Skip to content

Correct store-dir dir in ghc env files generated by cabal install --lib --package-env #6298

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

Merged
merged 2 commits into from
Oct 22, 2019

Conversation

jneira
Copy link
Member

@jneira jneira commented Oct 16, 2019

Please include the following checklist in your PR:

  • 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.

It should fix #5925 and help to fix DanielG/cabal-helper#83

Tested manually running the command and check that the package env file contains the custom store-dir:

PS D:\dev\ws\haskell\cabal-test> cabal install --lib --package-env .
Wrote tarball sdist to
D:\dev\ws\haskell\cabal-test\dist-newstyle\sdist\cabal-test-0.1.0.0.tar.gz
Resolving dependencies...
Up to date
PS D:\dev\ws\haskell\cabal-test> type .\.ghc.environment.x86_64-mingw32-8.6.5
clear-package-db
global-package-db
package-db C:\Users\atrey\AppData\Roaming\cabal\store\ghc-8.6.5\package.db
package-id ghc-8.6.5
....
PS D:\dev\ws\haskell\cabal-test> D:\bin\cabals\3.1.0.0\cabal install --lib --package-env .
Wrote tarball sdist to
D:\dev\ws\haskell\cabal-test\dist-newstyle\sdist\cabal-test-0.1.0.0.tar.gz
Resolving dependencies...
Up to date
PS D:\dev\ws\haskell\cabal-test> type .\.ghc.environment.x86_64-mingw32-8.6.5
clear-package-db
global-package-db
package-db C:\sd\ghc-8.6.5\package.db
....

@DanielG
Copy link
Collaborator

DanielG commented Oct 16, 2019

The fix itself looks good, but this makes me think the usage of globalFlags in v2 commands is a general code smell. Particularily mlogsDir = flagToMaybe (globalLogsDir globalFlags) just below your patch looks wrong too. ProjectOrchestration.hs uses projectConfigLogsDir when calling mkCabalDirLayout instead. I recon we should do the same thing here.

@jneira
Copy link
Member Author

jneira commented Oct 17, 2019

@DanielG agree, it is done!

@DanielG
Copy link
Collaborator

DanielG commented Oct 17, 2019

Windows build is failing, not sure what's going on there. @jneira can you try reproducing that locally since you're on windows anyways?

@jneira
Copy link
Member Author

jneira commented Oct 17, 2019

I don't know in deep cabal test suite but i've executed cabal v2-build T4288 inside cabal-testsuite\PackageTests\NewBuild\T4288 with the pr cabal version and the 3.0.0.0 release and the output is identical: https://gist.github.com/jneira/f13ad285b8aaf29ed7909236d64bc9dc

I've triggered an appveyor build in my repo: https://ci.appveyor.com/project/jneira/cabal/builds/28191270

@jneira
Copy link
Member Author

jneira commented Oct 17, 2019

The test was succesful in the appveyor build in my repo: https://ci.appveyor.com/project/jneira/cabal/builds/28191270#L1953

@phadej
Copy link
Collaborator

phadej commented Oct 18, 2019

Please, remove the Trigger ci build commit. There are people who are able to click "restart job" if needed.

Copy link
Collaborator

@phadej phadej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove empty commits.

@jneira
Copy link
Member Author

jneira commented Oct 18, 2019

Oh, sorry for that, commit removed

@jneira
Copy link
Member Author

jneira commented Oct 22, 2019

@phadej does the pr need some other change?

@DanielG DanielG merged commit 9e0ab3b into haskell:master Oct 22, 2019
@jneira
Copy link
Member Author

jneira commented Oct 22, 2019

Thanks for the feedback! Glad to having been able to contribute my 2 cents.

jneira added a commit to jneira/cabal that referenced this pull request Nov 5, 2019
…ckage-env` (haskell#6298)

Use project config instead of global one to get store-dir and logs-dir.

Fixes haskell#5925

(cherry picked from commit 9e0ab3b)
@phadej
Copy link
Collaborator

phadej commented Dec 13, 2019

This PR is a mess, is the 9e0ab3b right commit to backport? I'm waiting for the response until tomorrow.

@jneira
Copy link
Member Author

jneira commented Dec 13, 2019

@phadej yeah, 9e0ab3b has the needed changes

phadej pushed a commit to phadej/cabal that referenced this pull request Dec 13, 2019
…ckage-env` (haskell#6298)

Use project config instead of global one to get store-dir and logs-dir.

Fixes haskell#5925
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.

cabal-helper error with a custom cabal store-dir Environment file from cabal new-install ... --package-env . has wrong store dir.
3 participants