prepend rather than append extra prog path

[gbaz]

resolves #6304

• Any changes that could be relevant to users have been recorded in the changelog.

[gbaz]

Not sure how one would add tests for this. Also, @Mistuke care to take a look / give it a whirl?

[gbaz]

(local testing by passing an extra-prog-path with a dummy pkg-config inside shows this works)

[ulysses4ever]

It's not impossible to write a test for it, is it? Create a tree:

❯ tree
.
├── bad
│   └── prog
└── good
    └── prog

(bad/prog always fails and good/prog succeeds). Put bad in PATH and good in extra-prog-path. Create a Custom setup that just calls prog. Voila…

[jneira]

hmm such changes have high chances of produce unintended consequences, at minimum i would add tests proving it fixes the issue and a Windows based one if possible

it seems to me it could break lot of user configurations and it should include a big warning in the changelog

[mouse07410]

such changes have high chances of produce unintended consequences

I disagree. The whole point of specifying location of extra programs is preferring them over what's in the standard location. Thus, looking there first is quite natural. IMHO.

[Mistuke]

such changes have high chances of produce unintended consequences

I disagree. The whole point of specifying location of extra programs is preferring them over what's in the standard location. Thus, looking there first is quite natural. IMHO.

Agreed, hence this request. At this moment it's hard to manage which install of things is "active".

@gbaz sure, do we make tarballs for PRs too or should I build from source?

[ulysses4ever]

@Mistuke just FYI: binaries for any PR are available on the CI page. For this PR it's here. Very bottom of it.

[Mikolaj]

@Mistuke: in case you missed @ulysses4ever's comment ^^^.

[Mistuke]

@Mistuke: in case you missed @ulysses4ever's comment ^^^.

I saw it. Sorry for my silence on this. I'm under a very tight deadline for work so will be quite unresponsive for the next 3 weeks still..

But feel free to merge without me checking and will do so afterwards

[gbaz]

Does anyone care to add the test as proposed to this? I'd appreciate a hand here...

[Mistuke]

@gbaz Sorry for the delay, I've been giving this a test and doesn't seem to work quite yet.

PS C:\Users\WDAGUtilityAccount\Desktop> cp C:\Users\WDAGUtilityAccount\AppData\Roaming\cabal\bin\can-i-have-a-pony.exe .\pkg-config.exe
PS C:\Users\WDAGUtilityAccount\Desktop> $Env:PATH+=";C:\Users\WDAGUtilityAccount\Desktop\"
PS C:\Users\WDAGUtilityAccount\Desktop> where.exe .\pkg-config.exe
INFO: Could not find files for the given pattern(s).
PS C:\Users\WDAGUtilityAccount\Desktop> where.exe pkg-config
C:\Users\WDAGUtilityAccount\Desktop\pkg-config.exe
PS C:\Users\WDAGUtilityAccount\Desktop> ls C:\tools\msys64\mingw64\bin


    Directory: C:\tools\msys64\mingw64\bin


Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a----         11/5/2022   8:22 AM          60356 libwinpthread-1.dll
-a----        11/23/2020   8:25 AM         677710 pkg-config.exe
-a----        11/23/2020   8:25 AM         677710 x86_64-w64-mingw32-pkg-config.exe


PS C:\Users\WDAGUtilityAccount\Desktop> cabal user-config diff :extra-prog-path
+C: \tools\msys64\usr\bin
+ extra-include-dirs: C:\tools\msys64\mingw64\include
- extra-prog-path: C:\Users\WDAGUtilityAccount\AppData\Roaming\cabal\bin
+ extra-prog-path: C:\tools\msys64\mingw64\bin,
+ install-method: copy
PS C:\Users\WDAGUtilityAccount\Desktop> .\cabal.exe install sdl2
Warning: this is a debug build of cabal-install with assertions enabled.
Warning: cannot determine version of
C:\Users\WDAGUtilityAccount\Desktop\pkg-config.exe :
"\n \\\n \\\n \\\\\n \\\\\n >\\/7\n _.-(6' \\\n (=___._/` \\\n ) \\ |\n / /
|\n / > /\n j < _\\\n _.-' : ``.\n \\ r=._\\ `.\n <`\\\\_ \\ .`-.\n \\ r-7 `-.
._ ' . `\\\n \\`, `-.`7 7) )\n \\/ \\| \\' / `-._\n || .'\ncjr \\\\ (\n10mar02
>\\ >\n ,.-' >.'\n <.'_.''\n <'\n\n"
Warning: cannot determine version of
C:\Users\WDAGUtilityAccount\Desktop\pkg-config.exe :
"\n \\\n \\\n \\\\\n \\\\\n >\\/7\n _.-(6' \\\n (=___._/` \\\n ) \\ |\n / /
|\n / > /\n j < _\\\n _.-' : ``.\n \\ r=._\\ `.\n <`\\\\_ \\ .`-.\n \\ r-7 `-.
._ ' . `\\\n \\`, `-.`7 7) )\n \\/ \\| \\' / `-._\n || .'\ncjr \\\\ (\n10mar02
>\\ >\n ,.-' >.'\n <.'_.''\n <'\n\n"
Resolving dependencies...

It's still reading PATH before extra-prog-path.
For the record

PS C:\Users\WDAGUtilityAccount\Desktop> mv pkg-config.exe pkg-config2.exe
PS C:\Users\WDAGUtilityAccount\Desktop> .\cabal.exe install sdl2

works.

[gbaz]

I just tested it and it works. Maybe that build-product binary thing is janky? Either way, added a test as well, and also confirmed that command line and cabal config file both behave the same way. Alternately, could be a windows specific thing about how search paths work?

Note this test needs to skip on windows, since it inserts the phony pkg-config as a shell script. I hope btw that git preserves the executable bit of the script, or the test may need more tweaking.

[jneira]

This seems to not works on windows , so it would fix #6304 only partially and the issue should not be closed but updated

[gbaz]

what do you mean it doesn't work on windows? i see no reason it shouldn't?

the test is skipped on windows because using a shell script with an executable bit doesn't work out of the box for a dummy executable -- comment to that effect is fine to add I suppose.

[jneira]

what do you mean it doesn't work on windows? i see no reason it shouldn't?

oh sorry maybe i did not understand @Mistuke comment:

I've been giving this a test and doesn't seem to work quite yet.
....
It's still reading PATH before extra-prog-path.

And then you responded:

I just tested it and it works. Maybe that build-product binary thing is janky? Either way, added a test as well, and also confirmed that command line and cabal config file both behave the same way. Alternately, could be a windows specific thing about how search paths work?

Did you test it in windows and it did not reproduce? From "could be a windows specific thing" i inferred you tested it in linux only (like tests already does)
If there is no evidence the fix works in windows (and we already have a evidence it does not work) i would not close the issue.

To workaround the windows limitation we could create a tiny executable and upload it to the repo or generate the executable in the test itself compiling it from source with ghc.

[gbaz]

I think its extremely likely that changing the path will work universally, and that the wrong binary was used. I don't think that we have evidence it doesn't work, because this is not the only time I've seen someone attempt to use a downloaded intermediate binary instead of building themselves, and not get the expected behavior -- I think that workflow is very likely messed up. That said, I'm fine not closing the related ticket until someone tests on windows. In the meantime, could you please approve this so it can merge at all?

[jneira]

sure, only want to track the need of testing it in windows