Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: os: treat all non-symlink reparse points as ModeIrregular on Windows #61893

Open
qmuntal opened this issue Aug 9, 2023 · 12 comments
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows Proposal
Milestone

Comments

@qmuntal
Copy link
Contributor

qmuntal commented Aug 9, 2023

fs.ModeIrregular is useful to know whether a file is a reparse point or not. It is a signal that it needs more processing to fully understand how to handle it. Reparse points started to be treated as fs.ModeIrregular in CL 460595 (submitted by @bcmills), landed in go1.21.

Unfortunately, it did not mark all reparse points as irregular, only those that are not identified with ModeSymlink | ModeDir | ModeNamedPipe | ModeDevice | ModeCharDevice. This limits the usefulness of fs.ModeIrregular, as it is not possible to know, for example, if a directory is a reparse point or not. One can currently infer that all fs.ModeSymlink are reparse points, which currently includes IO_REPARSE_TAG_MOUNT_POINT, the most popular directory reparse point, but that's not a good reason to also properly support less common directory reparse ponts, such as IO_REPARSE_TAG_LX_SYMLINK or vendors defined reparse points.

I propose to treat all reparse points as fs.ModeIrregular, and to modify its definition from non-regular file; nothing else is known about this file to non-regular file; may need special handling.

Worth noting that this will help identifying mounted directories (aka IO_REPARSE_TAG_MOUNT_POINT) as reparse points if we manage to not treat them as symbolic links, that is fixing this TODO.

Edit: I'm opening this issue to discuss potential concerns.

@golang/windows

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 9, 2023
@mknyszek mknyszek added this to the Backlog milestone Aug 9, 2023
@gopherbot
Copy link

Change https://go.dev/cl/517815 mentions this issue: os: treat all non-symlink reparse points as ModeIrregular on Windows

@bcmills
Copy link
Member

bcmills commented Aug 9, 2023

Hmm. Lstat never follows reparse points, right? Can that be used to determine whether an arbitrary path is a reparse point?

Or does Lstat report, say, ModeDir for reparse points that are also directory-like?

@qmuntal
Copy link
Contributor Author

qmuntal commented Aug 9, 2023

Hmm. Lstat never follows reparse points, right? Can that be used to determine whether an arbitrary path is a reparse point?

Or does Lstat report, say, ModeDir for reparse points that are also directory-like?

Lstat reports fs.ModeDir for directory-like non-symlink reparse points. Unfortunately it doesn't currently set fs.ModeIrregular on that case. That's why I submitted this proposal.

@bcmills
Copy link
Member

bcmills commented Aug 9, 2023

Hmm. I do think it makes sense to mark directories as irregular if they may require additional special handling, although I also think it makes sense to continue to omit the irregular bit for reparse points that are “just” symlinks, since the os package already does the special handling needed for those.

@qmuntal
Copy link
Contributor Author

qmuntal commented Aug 9, 2023

I gave this another thought, and now I think I've given the wrong solution to the issue of consistently identifing directory reparse points.

IMO we shouldn't be treating any reparse points as a directory, trying to follow them while recursively walking directories is a recipe for problems. In fact, we almost have it right now: symlinks and mount points never have the directory bit set thanks to CL 41830. If we do the same for all reparse points, then the current logic will already mark them as irregular files.

require additional special handling, although I also think it makes sense to continue to omit the irregular bit for reparse points that are “just” symlinks, since the os package already does the special handling needed for those.

Good point, agree.

@gopherbot
Copy link

Change https://go.dev/cl/536655 mentions this issue: os,path/filepath: treat all non-symlink reparse points as irregular files

@jakebailey
Copy link

jakebailey commented Oct 20, 2023

Is it correct to say that the above change would start causing junctions to stop showing up as symlink'd directories, and start showing up as irregular files?

Given real symlinks are locked behind Developer Mode without elevation, loads of tools use junctions to emulate symlinks (e.g. yarn, pnpm in the JS ecosystem). I'm a little worried that some tooling out there will stop working properly.

Right now, WSL sees junctions as symlinks (when observed via /mnt/c or something). I know the WSL FS works via 9p; given the name/protocol I had kinda assumed it was in Go but maybe not. git-bash also treats junctions as symlinked dirs on Windows (though curiously git itself thinks they are regular directories, something I had tried to change in git-for-windows/git#4383 as it totally breaks recursive symlinks, unfortunately with the justification that Go treated them like symlinks.... 🙃 ).

@qmuntal
Copy link
Contributor Author

qmuntal commented Oct 21, 2023

Is it correct to say that the above change would start causing junctions to stop showing up as symlink'd directories, and start showing up as irregular files?

Junctions are currently only identified as symlinks, not directories. This prevents the infinite recursion trap for users blindly walking directories. If CL 536655 is approved, then junctions won't be identified as symlinks, but as irregular files.

I'm planning to submit a companion proposal to make query the file reparse tag easier , so users can take informed decisions when implementing different FS operations. Something like this: #63429 (comment).

@bcmills bcmills changed the title os: treat all non-symlink reparse points as ModeIrregular on Windows proposal: os: treat all non-symlink reparse points as ModeIrregular on Windows Oct 24, 2023
@bcmills bcmills modified the milestones: Backlog, Proposal Oct 24, 2023
@rsc
Copy link
Contributor

rsc commented Oct 27, 2023

Per discussion on #63429, even if we adopt a general rule that all reparse points are Irregular, we should also add an exception for dedup points, which are (1) regular enough, and (2) common enough to be worth special casing.

@rsc
Copy link
Contributor

rsc commented Oct 27, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@bcmills
Copy link
Member

bcmills commented Oct 31, 2023

Just to clarify, for a MOUNT_POINT reparse point:

  • os.Lstat would return a FileInfo for the reparse point itself, with the ModeIrregular bit set and the ModeDir bit unset. (Before this proposal, it would instead be reported as ModeSymlink and not ModeDir.)

  • Because IO_REPARSE_TAG_MOUNT_POINT has the surrogate bit set, os.Stat would return a FileInfo for the underlying directory joined at the mount point, with the ModeDir bit set and the ModeIrregular bit unset. (No change in behavior from today.)

@rsc
Copy link
Contributor

rsc commented Dec 6, 2023

What is the current proposal and is there general agreement for it yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows Proposal
Projects
Status: Active
Development

No branches or pull requests

6 participants