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

status: try reading working directory path from $PWD first #85

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christoph-heiss
Copy link

@christoph-heiss christoph-heiss commented Dec 2, 2024

Hi,

this tries to fix #11, which I now also run into.
Seems like bit of a hack, so not sure if this is acceptable. But I'm using scmpuff now w/ the patch applied and it seems to fix the problem.

Happy to try other ways tho, if there are better.

According to POSIX, $PWD should always be set in POSIX-compliant environments, so seems safe to rely on, at least.

PWD
This variable shall be set as specified in the DESCRIPTION. If an application sets or unsets the value of PWD , the behavior of cd is unspecified.

Cheers!

@mroth
Copy link
Owner

mroth commented Dec 3, 2024

Interesting! Thanks for looking into to this. I'm trying to piece together why $PWD gets a different result than os.Getwd -- if you check the source code for that (https://cs.opensource.google/go/go/+/refs/tags/go1.23.3:src/os/getwd.go;l=22), it's also checking $PWD in some situations, but has a lot of edge case handling. What am I missing?

Sidenote: The risk with any "hacky" (to use your words) path munging is we might be introducing other subtle edge case errors by handling this exact situation -- I'm going to take a think through if there is a reasonable way to put together a test harness for the path resolution so we can safely introduce.

(On another sidenote, I'm increasingly thinking that finally taking on #33 might resolve all these various path parsing issues all at once.)

@christoph-heiss
Copy link
Author

First, thanks for taking a look!

if you check the source code for that (https://cs.opensource.google/go/go/+/refs/tags/go1.23.3:src/os/getwd.go;l=22), it's also checking $PWD in some situations, but has a lot of edge case handling. What am I missing?

I think generally, as long as os.Getwd() is documented as

If the current directory can be reached via multiple paths (due to symbolic links), Getwd may return any one of them.

the result cannot really be "trusted" anyway IMHO.
Reading the code a bit, it seems to me it really tries to find the correct path (whether that may be for Go), as it e.g. even stat() the path in $PWD and checks if it's the same directory as .?

Really weird in any case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent relative paths with symlinks in scmpuff expand
2 participants