haskell / cabal

Official upstream development repository for Cabal and cabal-install
https://haskell.org/cabal
Other
1.61k stars 691 forks source link

Work with GHC_PACKAGE_PATH #3728

Open ttuegel opened 8 years ago

ttuegel commented 8 years ago

At the moment, Cabal will die if run with the GHC_PACKAGE_PATH environment variable set. I think this is an arbitrary restriction. Instead, it should

  1. Parse the search path in GHC_PACKAGE_PATH,
  2. treat these paths as if they were passed to the configure command by --package-db, and
  3. unset GHC_PACKAGE_PATH before invoking GHC.

Cabal should do the same for GHCJS_PACKAGE_PATH.

See also #2711.

I am implementing this in reconfigure.

/cc @ezyang @23Skidoo

ezyang commented 8 years ago

OK, that sounds fine to me.

dcoutts commented 8 years ago

@ttuegel this patch is likely to break cabal new-build since it changes getInstalledPackages but not getInstalledPackagesMonitorFiles. But just changing getInstalledPackagesMonitorFiles would also be wrong for new-build because the decision of whether or not to use the GHC_PACKAGE_PATH can't be taken locally like that, we'd need to decide for the old code path or the new code path differently whether we want to use it or not, and for new-build if we do then we need to track the monitored files.

In fact this will really break cabal new-build since we only want to get the installed packages for the global db and ignore user and any other ones. So unless we think that GHC_PACKAGE_PATH is specifying the "global" db (which nix does) then we do want to ignore it.

23Skidoo commented 8 years ago

@ttuegel @dcoutts Can we add a failing test for this?

ttuegel commented 8 years ago

In fact this will really break cabal new-build since we only want to get the installed packages for the global db and ignore user and any other ones. So unless we think that GHC_PACKAGE_PATH is specifying the "global" db (which nix does) then we do want to ignore it.

GHC_PACKAGE_PATH can really only specify an immutable global database: the paths are added before the global database, can't be cleared from the command line, and can't be installation targets. If I update getInstalledPackagesMonitorFiles accordingly, I don't see why this would break new-build.

ttuegel commented 8 years ago

This is blocked on #3651 (Support Nix in cabal-install). To really do this properly, cabal-install needs to pay attention to when certain environment variables change and reconfigure; this will break the usual workflow for Nix users unless I implement Nix integration first.

ttuegel commented 8 years ago

I have reverted the partial solution I applied before in lieu of a complete solution. Therefore, this issue is still fully open.

ezyang commented 8 years ago

Hmm. I guess nix-local-build doesn't have this problem, because the passed in package databases are recorded as the part of the project shared config; when it changes the cache is invalidated.

ttuegel commented 8 years ago

I guess nix-local-build doesn't have this problem, because the passed in package databases are recorded as the part of the project shared config; when it changes the cache is invalidated.

Right now, neither builder respects GHC_PACKAGE_PATH and so neither has this problem.

I'm thinking that, to be truly correct, we should also reconfigure if other variables change, like PATH.

ezyang commented 8 years ago

The rabbit hole goes very deep. Not only should we rebuild if PATH changes, but we should rebuild if the binary pointed to inside path changes hash, or if any of the dynamic libraries that it would be linked against would change.

Furthermore, for precision, we should only rebuild if we would have been affected by a change in PATH; e.g. if a user adds a path which doesn't affect any of the path lookups, that should not force us to relookup. (This would even be "easy" to do for Cabal's lookups on PATH, but if ghc independently tries to call out to the program you have more problems.)

Gotta stop somewhere.

ttuegel commented 8 years ago

That's a good point. To keep consistency with Cabal's current behavior, I think I will not reconfigure if GHC_PACKAGE_PATH changes; therefore, I will need to capture it during configuration and set the captured value before each call to GHC.

fgaz commented 1 year ago

I think this can be solved in a similar fashion as #7676.

Maybe since it's less explicit than a flag and it could cause problems without the user knowing it's set, we could have a config option use-ghc-package-path that can be set to use, dont-use, or error, defaulting to error which would be the current behaviour plus a longer error message explaining the options.

The rabbit hole goes very deep. Not only should we rebuild if PATH changes, but we should rebuild if the binary pointed to inside path changes hash, or if any of the dynamic libraries that it would be linked against would change.

I think that with proper warnings we can ignore this, like we do with --package-db. We don't perform that check on the global db either after all, and it can change too (eg. ghc.withPackages in nix). Finally, I predict this is mostly going to be useful together with all-or-nothing stuff like nix, ie. not using the store at all.

nmeum commented 9 months ago

I believe this issue to be the source of a major annoyance for Guix users, a nix-like functional package manager. Contrary to nix, Guix makes heavy use of GHC_PACKAGE_PATH to have GHC pickup Haskell packages installed via the Guix package manager [1]. Since Guix itself does not rely directly on Cabal for building packages this is not much of an issue, but is an annoyance when using Guix to create development environments for Haskell projects (e.g. using guix shell) as it won't be possible to invoke Cabal from these environment.

For example:

$ guix shell ghc cabal-install ghc-tasty ghc-tasty-hunit
guix$ echo $GHC_PACKAGE_PATH
/gnu/store/mgh46pipslhhfsmwx4rgmpzni01cz6ww-profile/lib/ghc-9.2.5/ghc-tasty-1.4.3.conf.d:/gnu/store/mgh46pipslhhfsmwx4rgmpzni01cz6ww-profile/lib/ghc-9.2.5/ghc-tasty-hunit-0.10.0.3.conf.d:/gnu/store/mgh46pipslhhfsmwx4rgmpzni01cz6ww-profile/lib/ghc-9.2.5/package.conf.d
guix$ cabal build
cabal: Use of GHC's environment variable GHC_PACKAGE_PATH is incompatible with
Cabal. Use the flag --package-db to specify a package database (it can be used
multiple times).

For this use case, it would be very handy if Cabal would expand elements in GHC_PACKAGE_PATH to --package-db options as outlined in this issue. Hence, I would like to see this issue being resolved. However, presently, I don't understand why the two commits by @ttuegel only constitute a “partial solution” is it just that the config option proposed in https://github.com/haskell/cabal/issues/3728#issuecomment-1490218447 is lacking?

What would have to be implemented in order to get this issue resolved?

Mikolaj commented 9 months ago

I don't understand why the two commits by @ttuegel only constitute a https://github.com/haskell/cabal/issues/3728#issuecomment-246205564 is it just that the config option proposed in https://github.com/haskell/cabal/issues/3728#issuecomment-1490218447 is lacking?

What would have to be implemented in order to get this issue resolved?

Bump. Any ideas anybody? @ttuegel, do you perchance remember after only 8 years?

nmeum commented 8 months ago

Looking at the revert commit message it seems that it was reverted because it was deemed necessary to cache/store the GHC_PACKAGE_PATH value in LocalBuildInfo during cabal configure. Doing that is, easy but the getInstalledPackages and getInstalledPackagesMonitorFiles functions (modified in the original patch) do not have access to the LocalBuildInfo. So I guess that approach requires a different entrypoint.

If storing the GHC_PACKAGE_PATH environment variable value in LocalBuildInfo is still deemed necessary, where would it best be picked up again?

mpickering commented 8 months ago

I have also run into this today. I want to use the GHC_PACKAGE_PATH variable to indicate toghc the location of the global package database that it should use. The fact that cabal ignores this option seems like quite a shortcoming.

mpickering commented 8 months ago

The patches by Thomas Tuegal are not sufficient for GHC_PACKAGE_PATH support. For a start, they do not obey the semantics of GHC_PACKAGE_PATH when it comes to whether to further search global/user package databases by default. Secondly there is a restriction about GlobalPackageDb having to appear first in a search path, which I believe can be dropped. There are also issues with tools like ghc-pkg inheriting GHC_PACKAGE_PATH and giving incorrect output for some commands. There are also calls to ghcProgram which don't respect GHC_PACKAGE_PATH which need to be modified

I am working on exploring how invasive/feasible proper GHC_PACKAGE_PATH support is.

nmeum commented 2 months ago

FYI: Guix now patches Cabal with a variant of the patch proposed by Thomas Tuegal: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=9236047476e728b9ae1746d56b8547e023f72307 for the reasons outlined above. Would still be cool to get a proper patch integrated into Cabal itself.