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

rebar_dir: Avoid canonicalizing the CWD (get_cwd/0) #2925

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

the-mikedavis
Copy link

The unix BIF which ultimately implements file:get_cwd/0 uses getcwd(3) which returns a canonicalized path, resolving any symlinks. Instead we should try to preserve the non-canonicalized path by reading the $PWD environment variable set by Posix shells. We should prefer $PWD to file:get_cwd() when it canonicalizes to the same path. This emulates the behavior of pwd -L (the default behavior of pwd).

WhatsApp/erlang-language-platform#65 has more details on the motivation of this change. On my machine rebar3 experimental manifest emits canonicalized paths which confuse tools like ELP that comumnicate these paths with editors.

The unix BIF which ultimately implements `file:get_cwd/0` uses getcwd(3)
which returns a canonicalized path, expanding any symlinks. Instead we
should try to preserve the non-canonicalized path by reading the $PWD
environment variable set by Posix shells. We should prefer $PWD to
`file:get_cwd()` when it canonicalizes to the same path. This emulates
the behavior of `pwd -L` (the default behavior of `pwd`).
@the-mikedavis
Copy link
Author

Looks like this will need some more work, it breaks rebar3 eunit for me locally

@the-mikedavis the-mikedavis marked this pull request as draft November 27, 2024 19:34
@ferd
Copy link
Collaborator

ferd commented Dec 2, 2024

I think this is going to be a bigger changeset than you intend, because we make extensive use of filename:absname/1 in code:

  • In path variables used by external commands and possibly the C compiler
  • in fetching dependencies
  • setting up the base directory for Rebar3's own operations for all build artifacts (including compiled modules, but also code coverage), across multiple compilers and most test tools
  • in building escripts
  • in executing custom scripts in rebar3 shell
  • in path name canonicalization and retargetting, which may be used by plugins.

The definition of that function, however, calls to file:get_cwd() internally, which currently does not respect the behavior intended by this PR.

I would expect that inconsistent path expansion here is likely to cause some problems or bypass the fixes? We do rely on absolute paths by default, a whole lot, everywhere.

We'd probably need to supply our own absname function that does not do the symlink following, and then substitute it everywhere.

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.

2 participants