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

Installed executables take precedence over build-tool-depends #8951

Closed
sol opened this issue May 18, 2023 · 4 comments · Fixed by #8972
Closed

Installed executables take precedence over build-tool-depends #8951

sol opened this issue May 18, 2023 · 4 comments · Fixed by #8972
Labels
re: build-tool Concerning `build-tools` and `build-tool-depends` re: extra-prog-path Concerning the `extra-prog-path` configuration option regression in 3.10

Comments

@sol
Copy link
Member

sol commented May 18, 2023

Describe the bug
Executables from ~/.local/bin1 shadow executables that are specified via build-tool-depends.

Why is this a problem? How can this corrupt your builds in non-obvious ways? How much effort is it to debug this? I'm not going into rant mode now, please use your own judgment.

Instead, I will describe the underlying reasons for this bug and possible solutions in a follow-up comment.

To Reproduce
A minimal skeleton that can be used to reproduce the issue is at https://github.com/sol/cabal-bug/tree/build-tools

You may need to adapt this according to your OS and whether you have an existing ~/.cabal or not. So make sure that you fully understand what it is doing before you run it.

System information
Operating system: Linux
cabal: 3.10.1.0
ghc: 9.6.1

Footnotes

  1. Note that this may be ~/.cabal/bin on your system.

@sol
Copy link
Member Author

sol commented May 18, 2023

Underlying reasons

  1. When you first run cabal it adds

    extra-prog-path: ~/.local/bin

    to ~/.config/cabal/config.

    return [dir </> ".local" </> "bin"]

  2. cabal-install ensures that build tools (specified via build-tool-depends) are on the PATH.

  3. Cabal prepends extra-prog-path to the PATH1. At this point executables from ~/.local/bin take precedence over build tools, resulting in the bug.

Footnotes

  1. Related: prepend rather than append extra prog path #8506

@sol
Copy link
Member Author

sol commented May 18, 2023

Possible solutions

I see two ways to address this issue. Note that they are not mutually exclusive. We could (and probably should) do both:

  1. Don't add extra-prog-path to ~/.config/cabal/config in the first place1. While this will not change anything for existing users, it will improve the situation for new users or if you start from a clean system. (implemented in Installed executables take precedence over build-tool-depends #8951)
  2. Make sure that build tools take precedence over --extra-prog-path.

(1) Is a trivial change. This is why I would do that first.

(2) Is more involved due to the split responsibilities between cabal-install / Cabal.

Footnotes

  1. Note that this was added 10 years ago (https://github.com/haskell/cabal/commit/66a1bbf9eca4aa95cda5476bcd031401bf1e3f46), pre-dating build-tool-depends. With build-tool-depends at our disposal I think the original motivation no longer holds much weight.

sol added a commit to sol/cabal that referenced this issue May 18, 2023
@gbaz
Copy link
Collaborator

gbaz commented May 24, 2023

Thanks for the reproducer. This is a regression in 3.10 likely induced by #8506 -- the code is very gnarly around how this stuff works, so need to think carefully on best way to fix it.

@gbaz
Copy link
Collaborator

gbaz commented May 24, 2023

Reverting the specific change to Configure.hs here resolves this issue, but then breaks the other behavior that 8506 intended to introduce:

https://github.com/haskell/cabal/pull/8506/files#diff-fff9e57c2727835086925d886619deb3fafd33a06e1af760d60d485ae07cec68R853

The deeper diagnosis is that invoke in SetupWrapper.hs is called by v2-build to run the actual build, performed by Cabal-the-library. It sets a PATH including the paths of tools created in build-tool-depends at the head. Then the Setup in Cabal-the-library in turn adds the extra-prog-path stuff even further, in front of that.

My proposed, janky-as-heck, solution is to have the path from extra-prog-path eagerly added at the end of the useExtraPathEnv option in SetupScriptOptions.

We would then also need to still revert the above, so it also gets added at the end (in the case we're not in a -v2 build). This means that the usefulness of 8506 is lost in the non v2-build case, but I suppose its the least bad thing I can imagine. Ugh!

sol added a commit to sol/cabal that referenced this issue Jun 2, 2023
Mikolaj pushed a commit to sol/cabal that referenced this issue Jun 20, 2023
Mikolaj pushed a commit to sol/cabal that referenced this issue Jun 20, 2023
mergify bot added a commit that referenced this issue Jun 20, 2023
Don't add `extra-prog-path` to `~/.config/cabal/config` (#8951)
mergify bot pushed a commit that referenced this issue Jun 20, 2023
(when initially creating it)

(cherry picked from commit ea55955)

# Conflicts:
#	cabal-install/src/Distribution/Client/Config.hs
@mergify mergify bot closed this as completed in #8972 Jul 1, 2023
Kleidukos added a commit that referenced this issue Jul 12, 2023
…port #8952) (#9050)

Don't add `extra-prog-path` to `~/.config/cabal/config` (#8951)

(when initially creating it)

(cherry picked from commit ea55955)

# Conflicts:
#	cabal-install/src/Distribution/Client/Config.hs

* fix conflict

* Merge branch '3.10' into mergify/bp/3.10/pr-8952
mergify bot added a commit that referenced this issue Jul 13, 2023
Don't add `extra-prog-path` to `~/.config/cabal/config` (#8951) (backport #8952)
@andreasabel andreasabel added re: build-tool Concerning `build-tools` and `build-tool-depends` re: extra-prog-path Concerning the `extra-prog-path` configuration option labels Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
re: build-tool Concerning `build-tools` and `build-tool-depends` re: extra-prog-path Concerning the `extra-prog-path` configuration option regression in 3.10
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants