BDirectory::IsRootDirectory() returns true for root of active packages

[ambroff]

I was ​fixing the BDirectory tests and noticed what I think is a minor regression.

BDirectory::IsRootDirectory is supposed to return true if the directory is the root of a volume. And its implementation is pretty simple.

bool
BDirectory::IsRootDirectory() const
{
	// compare the directory's node ID with the ID of the root node of the FS
	bool result = false;
	node_ref ref;
	fs_info info;
	if (GetNodeRef(&ref) == B_OK && fs_stat_dev(ref.device, &info) == 0)
		result = (ref.node == info.root);
	return result;
}

So it gets the fs_info filed out by the filesystem that owns the current entry, and then compares the inodes with the inode of the root of the filesystem. Makes sense.

The problem is that this means the root of packages are now going to return true. For example, /boot/system is the root of the system package, and that now returns true for IsRootDirectory().

This feels like a regression to me, but I'm curious what you all think.

This doesn't seem that easy to fix without maybe adding an additional flag to fs_info::flags, like B_FS_IS_PACKAGE or B_FS_IS_BIND_MOUNTED or something like that. With a flag like that then BDirectory::IsRootDirectory can check for such a flag and fall back to some other method of figuringing out the root directory.

[pulkomandy]

That seems correct behavior for me. packagefs is a separate filesystem with its own root. Why would we want to hide that fact and lie to applications about it?

[ambroff]

Yeah that makes sense. I filed this ticket because I wasn't sure whether or not it was intentional.

I'm totally fine with the current behavior. It just caught me off guard because I didn't expect it so I wasn't sure it was right.

[waddlesplash]

"No change required" is more appropriate here :)

[ambroff]

Ah you're right thank you.