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

Find build-tool installed programs before programs in path #9762

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

alt-romes
Copy link
Collaborator

@alt-romes alt-romes commented Mar 1, 2024

We must consider the path to the installed build-tool before the path to existing versions of the build tool in paths such as extra-prog-path or in the system path.

This was previously fixed by #8972 but undone by #9527.

This also renames appendProgramSearchPath to
prependProgramSearchPath to describe correctly what that function does.

Fixes #9756

@alt-romes alt-romes requested review from jasagredo and gbaz March 1, 2024 14:41
@alt-romes
Copy link
Collaborator Author

alt-romes commented Mar 1, 2024

@gbaz here is the fix.

It still needs a test, I'm trying to figure out how to do one for this specifically.

-- We are testing if the build-tools program is found in path before programs e.g. in extra-prog-path or the system path
-- For that, we need
--  * A repo with a build tool that is up to date
--  * An older version of the build tool in the extra-prog-path
--  * A project that requires the more up-to-date version of the build-tool

Happy to take any ideas

@alt-romes
Copy link
Collaborator Author

(BTW I've tested this locally on Andreas' reproducer successfully)

@alt-romes alt-romes force-pushed the wip/romes/9756 branch 2 times, most recently from e710313 to 12c2bdd Compare March 1, 2024 16:40
@alt-romes
Copy link
Collaborator Author

Test added, ready to merge.

@alt-romes
Copy link
Collaborator Author

If this should make it back to 3.10 it needs to be backported @Mikolaj @gbaz

@gbaz
Copy link
Collaborator

gbaz commented Mar 1, 2024

Thank you so much for moving quickly on this. Given how quickly this has been done, and that the only key change is line 1239 of configure.hs, I definitely think we should backport.

@alt-romes
Copy link
Collaborator Author

I've added the comment you requested @gbaz. I'm also going to put forward a backport MR

@alt-romes alt-romes added the merge me Tell Mergify Bot to merge label Mar 1, 2024
alt-romes added a commit to alt-romes/cabal that referenced this pull request Mar 1, 2024
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Mar 3, 2024
We must consider the path to the installed build-tool before the path to
existing versions of the build tool in paths such as `extra-prog-path`
or in the system path.

This was previously fixed by haskell#8972 but undone by haskell#9527.

This also renames `appendProgramSearchPath` to
`prependProgramSearchPath` to describe correctly what that function
does.

Fixes haskell#9756
@mergify mergify bot merged commit 3c06125 into haskell:master Mar 4, 2024
53 checks passed
@Mikolaj
Copy link
Member

Mikolaj commented Mar 4, 2024

@backport 3.12

@Mikolaj
Copy link
Member

Mikolaj commented Mar 4, 2024

@mergify backport 3.12

Copy link
Contributor

mergify bot commented Mar 4, 2024

backport 3.12

✅ Backports have been created

@Mikolaj
Copy link
Member

Mikolaj commented Mar 4, 2024

@backport --- apologies, wrong ping, but you must be used to it by now, aren't you? :)

mergify bot added a commit that referenced this pull request Mar 4, 2024
Find build-tool installed programs before programs in path (backport #9762)
Kleidukos pushed a commit that referenced this pull request Mar 11, 2024
…9767)

* Find build-tool installed programs before programs in path (BP)

A backport of 443c890 (#9762)

---------

Co-authored-by: brandon s allbery kf8nh <allbery.b@gmail.com>
Co-authored-by: Gershom Bazerman <gershom@arista.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention: needs-backport 3.10 merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in 3.10 cabal build: the extra-prog-path ends up first in the PATH, shadowing build-tools
6 participants