Extra bits returned by the Permissions struct on unix

[pietroalbini]

Until September, PermissionsExt::mode on unix returned the permissions of the file masked with 0o777, which was hiding the set{u,g}id/sticky bits. Then, #44147 and #44624 changed the standard library to avoid masking the permissions at all.

The fix indeed exposed the missing bits, but it also exposed some extra ones, like the ones specifying if the node is a file or a directory. For example, now the permissions of a directory are like 0o40755.

I think those bits are out of place in the permissions, as the information is provided by the Metadata struct already. Can we revert to masking with 0o7777 to preserve the set{u,g}id/sticky but discard the extra ones?

[joshlf]

The name mode for a method like this implies to me that it returns the full mode including all "non-permissions" bits. Perhaps a new method ought to be added (e.g., perm_mode), but I think having mode return only some of the mode bits would be surprising behavior.

[pietroalbini]

But that's not the behavior the method had in the previous releases. Isn't better to add a full_mode method which returns the unmasked mode? This way existing code doesn't break.

[joshlf]

Sure, but it sounds like what you're proposing is still a backwards-breaking change, right? The set{g,u}id/sticky bit wasn't exposed before either.

[pietroalbini]

Yeah, even that is a breaking change, but it affects way less use cases (things breaks only if the node has the set{g,u}id/sticky bit, which isn't so much common), while the current code affects all the calls to the method. Also, I think the motivation in #44147 is still valid, and that needs only a 0o7777 mask.

I'm ok with either masking to 0o7777 or reverting to the previous behavior, it's just a matter of preferring consistency or backward compatibility.

[joshlf]

@tmerr As the original reporter on #44147, what's your feeling on this?

[tmerr]

Hmmm I don't have a strong preference either keeping it unmasked or switching to 0o7777. Our current behavior associates Permissions more closely with Metadata since it exposes its st_mode bits, while this proposed behavior associates Permissions more closely with set_permissions, since we could think of mode() as the portion of the file mode that can be set by set_permissions aka a chmod. Either way makes sense to me, which is why I went with @sfackler's slight preference for changing to no mask for the PR.

Are there any known cases where the new behavior breaks existing code?

[pietroalbini]

There was a small breakage in a test of one of my projects, where I wanted to check if the permissions of a file were exactly the ones I wanted. It's not too much, but it's annoying anyway.

I think a decision on this should be made, either keeping the current behavior or reverting to the old one: the next release is going to be in a month, and it doesn't make sense to change the behavior of a method in a release and revert the change in the next one.