Fix run command environment

[xsebek]

• use elabExeDependencyPaths to get run test dependencies
• closes cabal test sees the executable, but cabal run does not. #8391

QA Notes

Calling cabal run should run the executable with dependencies on PATH.

Run a test that uses the built executable (e.g. fourmolu or HLS) that it depends on via build-tool-depends.
For example in the case of func-test in HLS, the following invocations should now all pass:

cabal run func-test -- -p indefinite
cabal test func-test --test-options="-p indefinite"
cabal exec $(cabal list-bin func-test) -- -p indefinite

---

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.
• Manual QA notes have been included.

[geekosaur]

A test and a changelog.d entry would be nice.

[xsebek]

@geekosaur this is still a Draft PR and I am aware there will be more steps to get this merged. 🙂

---

More importantly this does not work yet. Printing out the added environment shows just:

[("GHC_ENVIRONMENT",Just "/Users/xsebek/Git/haskell-language-server/dist-newstyle/tmp/environment.-20638/.ghc.environment.aarch64-darwin-9.2.8")]

The file does not exist, so I assume it's creation was skipped in run.

I would be grateful for any tips.

[gbaz]

I think this PR is confused. env files are ghc package environment env files.

The ticket seems to be about just setting the PATH shell environment variable -- a totally different usage of the name "environment".

The issue (I think) is that the test action is invoked via the setup wrapper, which sets that shell environment stuff automatically:

cabal/cabal-install/src/Distribution/Client/ProjectBuilding.hs

Line 1798 in 9f37325

setup

(that in turn uses withEnv to set the path).

The run action is invoked directly, so doesn't have that logic, which will need to be shared/replicated. (This is because there's a runghc Setup.hs test path but no such thing for run which doesn't exist for the Cabal library but only the `cabal-install executable).

[xsebek]

@gbaz I overlooked where exec modifies PATH, it's here:

cabal/cabal-install/src/Distribution/Client/CmdExec.hs

Line 168 in 6ac9de4

extraPaths <- pathAdditions verbosity baseCtx buildCtx

[gbaz]

Hrm... porting that from exec to run could likely work!

[xsebek]

@gbaz please take a look now. 🙂

Not sure if there is a good place for pathAdditions/binDirectories, maybe the latter should go to Distribution.Client.ProjectPlanning.Types? Looking at it, there is elabExeDependencyPaths already... 🤔

[xsebek]

I switched to using the existing elabExeDependencyPaths as it adds just what was explicitly required to PATH.

[gbaz]

Looks good, we should probably give it a user acceptance test at least.

[xsebek]

@gbaz should I prepare something for the test or are the notes enough? 🙂

[mpickering]

Can you please add a test?

[xsebek]

@mpickering sure, I will get to it next week and see if there is a similar test that I could modify. 🙂

[xsebek]

@mpickering as far as I can see, the only integration test that actually runs any Cmd.* action is testHaddockProjectDependencies. (I searched for Cmd\S*\.\S*Action in test directory.) Am I missing something?

Reading the test README leaves me rather confused, as there are no shell scripts that run commands there.
_{EDIT: I originally linked to the commit where the README was last modified, so there were shell scripts 7 years ago.}

Otherwise, I guess I could write an integration test for elabExeDependencyPaths, to see that it really gives the run command the paths to executable directories we would expect.

There are "golden tests" in unit tests that check the cabal files in cabal-install/tests/fixtures/init/golden/exe which do contain build-tool-depends, but I am not sure if they test elabExeDependencyPaths. (I have no idea how much mkStanza [mkExeStanza ...] tests.)

If there is some up-to-date guidance on how to write new tests in the repo, please point me to it. 🙂

[mpickering]

@xsebek You should write a test in cabal-testsuite/PackageTests which directly ensures that running the sequence of commands in the ticket description works.

I can write this test for you if you would like me to.

[ulysses4ever]

@xsebek hey! Any news on this? The last bit remaining is a test. And Matt suggested he could do it for you if you’re having trouble. What do you think?

[jasagredo]

I could absorb this PR into #9527. Thoughts? @ulysses4ever

[ulysses4ever]

@jasagredo I'm good with absorbing!

[jasagredo]

Closing as it was absorbed by #9527