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

cabal-install-3.8.1.0 update and sdist require ghc in PATH #8352

Closed
phadej opened this issue Aug 10, 2022 · 31 comments
Closed

cabal-install-3.8.1.0 update and sdist require ghc in PATH #8352

phadej opened this issue Aug 10, 2022 · 31 comments
Assignees

Comments

@phadej
Copy link
Collaborator

phadej commented Aug 10, 2022

Otherwise

Project settings changed, reconfiguring...
creating /__w/splitmix/splitmix/dist-newstyle/cache
creating /__w/splitmix/splitmix/dist-newstyle
creating /__w/splitmix/splitmix/dist-newstyle/cache
Compiler settings changed, reconfiguring...
CallStack (from HasCallStack):
  withMetadata, called at src/Distribution/Simple/Utils.hs:370:14 in Cabal-3.8.1.0-inplace:Distribution.Simple.Utils
Error: cabal-3.8.1.0: The program 'ghc' version >=7.0.1 is required but it

I guess I'm required to pass --with-compiler now?

@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

Error: cabal-3.8.1.0: The program 'ghc' version >=7.0.1 is required but it could not be found.

cabal sdist is not runnable without GHC in PATH, and cabal sdist doesn't have --with-compiler option.

This is major blocker for using cabal-install-3.8.1.0.

@Mikolaj
Copy link
Member

Mikolaj commented Aug 10, 2022

I don't know when it changed, but my guess would be that cabal sdist now requires GHC for some of its checks. @ffaf1: do you have any inkling?

Would enabling --with-compiler be a sufficient fix?

@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

Would enabling --with-compiler be a sufficient fix?

Sufficient but ugly. Even if cabal.project supported conditionals based on compiler version, sdist could still work without knowing anything about a compiler, taking union of all possibly available packages. sdist for single package doesn't need a compiler.

@RyanGlScott
Copy link
Member

Indeed, this prevents cabal-install-3.8.1.0 from being usable from within haskell-ci, which avoids putting compilers on the PATH by design.

@phadej phadej changed the title cabal-install-3.8.1.0 requires GHC in PATH cabal-install-3.8.1.0 update and sdist require GHC in PATH Aug 10, 2022
@phadej phadej changed the title cabal-install-3.8.1.0 update and sdist require GHC in PATH cabal-install-3.8.1.0 update and sdist require ghc in PATH Aug 10, 2022
@Mikolaj
Copy link
Member

Mikolaj commented Aug 10, 2022

Uh, oh, cabal update needing GHC sounds definitely like a bug.

@Mikolaj
Copy link
Member

Mikolaj commented Aug 10, 2022

That's a probably a release blocker. Too bad, we've already released. Are there any workarounds for haskell-ci?

A pity nobody tried 3.8.1.0-rc1 with haskell-ci. Is that reasonably easy to do for a haskell-ci user, for next time (3.8.1.0-rc1 was installable with ghcup)?

@Mikolaj Mikolaj added type: regression regression on master Regression that is unreleased and needs to be fixed before release labels Aug 10, 2022
@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

A pity nobody tried 3.8.1.0-rc1 with haskell-ci. Is that reasonably easy to do for a haskell-ci user, for next time (3.8.1.0-rc1 was installable with ghcup)?

I wasn't aware it was available through ghcup. It shouldn't be hard to test release candidates in the future. Though there is little insensitive as older cabal-install seems to work well enough. And that is a workaround for now: stick to using older cabal-install.

@ulysses4ever ulysses4ever added the can-workaround There is a (maybe partial) workaround for the issue or missing feature label Aug 10, 2022
@ffaf1
Copy link
Collaborator

ffaf1 commented Aug 10, 2022

Maybe 32259a1?

(compiler, Platform arch os, _) <- configureCompiler verbosity distDirLayout ((fst $ PD.ignoreConditions projectConfigSkeleton) <> cliConfig)

and

liftIO $ info verbosity "Compiler settings changed, reconfiguring..."
result@(_, _, progdb') <- liftIO $
Cabal.configCompilerEx
hcFlavor hcPath hcPkg
progdb verbosity

Follow the trail and you will end up with the dreaded requireProgramVersion.

@Mikolaj
Copy link
Member

Mikolaj commented Aug 10, 2022

But is this (rebuildProjectConfig) really called even in cabal update?

@ffaf1
Copy link
Collaborator

ffaf1 commented Aug 10, 2022

Yes, see ProjectOrchestration.hs

establishProjectBaseContextWithRoot
:: Verbosity
-> ProjectConfig
-> ProjectRoot
-> CurrentCommand
-> IO ProjectBaseContext
establishProjectBaseContextWithRoot verbosity cliConfig projectRoot currentCommand = do
cabalDir <- getCabalDir
let distDirLayout = defaultDistDirLayout projectRoot mdistDirectory
httpTransport <- configureTransport verbosity
(fromNubList . projectConfigProgPathExtra $ projectConfigShared cliConfig)
(flagToMaybe . projectConfigHttpTransport $ projectConfigBuildOnly cliConfig)
(projectConfig, localPackages) <-
rebuildProjectConfig verbosity

and

establishProjectBaseContext
:: Verbosity
-> ProjectConfig
-> CurrentCommand
-> IO ProjectBaseContext
establishProjectBaseContext verbosity cliConfig currentCommand = do
projectRoot <- either throwIO return =<< findProjectRoot Nothing mprojectFile
establishProjectBaseContextWithRoot verbosity cliConfig projectRoot currentCommand

and then CmdUpdate.hs

updateAction :: NixStyleFlags () -> [String] -> GlobalFlags -> IO ()
updateAction flags@NixStyleFlags {..} extraArgs globalFlags = do
let ignoreProject = flagIgnoreProject projectFlags
projectConfig <- withProjectOrGlobalConfig verbosity ignoreProject globalConfigFlag
(projectConfig <$> establishProjectBaseContext verbosity cliConfig OtherCommand)

and up to the original cabal update call.

@ulysses4ever
Copy link
Collaborator

Is it about the time to ping @gbaz?..

@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

I'd suspect changes related to --ignore-project implementation, not conditionals.

So possibly: #8109, but for be sure you shouldgit bisect.

@ffaf1
Copy link
Collaborator

ffaf1 commented Aug 10, 2022

git bisect agrees with my diagnosis: " 32259a1 is the first bad commit "

#7783

@gbaz
Copy link
Collaborator

gbaz commented Aug 10, 2022

Good diagnosis! Looking now at the logic there, I'm not sure if this change in behavior is wrong, though it is surprising.

In particular, that PR allows conditionals on ghc versions to be in project files. And... project files I believe have options that may affect even update and sdist? So If one does not have a ghc version, then when one reads a project file, as update and sdist must, then what should happen when a conditional is encountered?

A change would I think have to be some sort of "defaulting" in the specific cases of commands that don't strictly need ghc, which seems pretty strange.

It might be better to just change haskell-ci scripts to always make sure a ghc made available even for these commands?

@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

It might be better to just change haskell-ci scripts to always make sure a ghc made available even for these commands?

That is lazy approach (EDIT: and even then sdist command should have --with-compiler. haskell-ci wont put ghc in $PATH). Why can't you just fix cabal?

E.g. when haskell-ci calls cabal update there is no project file. What does cabal needs GHC then for. Why it spends cycles looking for it?

@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

And to expand on haskell-ci philosophy. Because haskell-ci doesn't put GHC in path, we found numerous bugs in cabal where it was looking for ghc and not using --with-compiler provided one. Also bugs in packages which expected ghc (or some other stuff) to be in PATH.

CI is a good place to work in highly hygienic environment, without humongous implicit state.

I'd appreciate if cabal supported such workflows in the future.

@gbaz
Copy link
Collaborator

gbaz commented Aug 10, 2022

"when haskell-ci calls cabal update there is no project file. What does cabal needs GHC then for. Why it spends cycles looking for it"

As I described, update and sdist may both be affected by options in project files. Project files may now have conditionals. Those conditionals may branch on version of ghc (or an arch). Knowing how to branch on a version of ghc or its supported arch requires finding the ghc available and querying its version.

A potential refactor would be to only query for ghc when such conditionals exist, but that would be extremely ad-hoc and also not at all fit with the current pipeline in terms of when things are pure and when they are not.

Your argument for not having a systemwide ghc but only using --with-compiler makes a lot of sense, as a way to shake out bugs. But that said, it seems like changing the haskell ci scripts to pass those flags to these two commands seems straightforward?

(edit: that said, its certainly an issue that sdist doesn't have a --with-compiler option, and I do think that would be an important fix, regardless)

@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

A potential refactor would be to only query for ghc when such conditionals exist, but that would be extremely ad-hoc and also not at all fit with the current pipeline in terms of when things are pure and when they are not.

It won't be ad-hoc. There is ProgramDb in Cabal was designed that you would configure a program at most once. The threading is just not applied through the cabal-install.

@gbaz
Copy link
Collaborator

gbaz commented Aug 10, 2022

What I mean is the following -- if we made that change, then using it, haskell-ci would work on most packages. But there would eventually be a package that had a conditional branch in its cabal.project, and haskell-ci would fail to work on that project, because in that specific instance it would need to figure out which ghc to use in order to fully parse the project.

So I think it is better to make a change that lets everything always work on all projects, and that change is extending sdist to have a --with-compiler flag.

@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

haskell-ci would work on most packages. But there would eventually be a package that had a conditional branch in its cabal.project,

haskell-ci always generates own cabal.project files, it doesn't use projects' own ones.

@phadej
Copy link
Collaborator Author

phadej commented Aug 10, 2022

... and as I said, even then taking an union of all branches should be enough for sdist. It doesn't need to know compiler.

@jneira
Copy link
Member

jneira commented Aug 11, 2022

... and as I said, even then taking an union of all branches should be enough for sdist. It doesn't need to know compiler.

hmm if you have

if impl(ghc >= x.y.z)
  packages: ./package1
else
  packages: ./package1 ./package2

cabal sdist all would not behave the same way having in account ghc versions or fusing all branches

EDIT: for cabal update the counterexample would be even clearer:

if impl(ghc >= x.y.z)
  active-repositories: repo1
else
  active-repositories: repo1, repo2

@fgaz
Copy link
Member

fgaz commented Aug 11, 2022

What about requiring ghc only if there are impl(ghc) branches?

@jneira
Copy link
Member

jneira commented Aug 11, 2022

cabal sdist --ignore-project should not query ghc neither and it fails with no ghc in PATH

@jneira
Copy link
Member

jneira commented Aug 11, 2022

haskell-ci always generates own cabal.project files, it doesn't use projects' own ones.

And there are no plans to generate cabal.project files with conditionals? at a first look it could be useful for haskell-ci.
To have even the chance to use them haskell-ci, the command would need -w and no remove the need for ghc (so remove the use of ghc conditionals)

@phadej
Copy link
Collaborator Author

phadej commented Aug 11, 2022

cabal sdist all would not behave the same way having in account ghc versions or fusing all branches

IMO it should not account for ghc ever. cabal sdist all should sdist all packages, regardless of which ghc happens to be in $PATH. Same applies for cabal update, updating more repositories won't destroy anything.

@roberth
Copy link

roberth commented Aug 21, 2022

IMO it should not account for ghc ever.

This is not just an opinion. cabal sdist is used for uploading to hackage, which means that the sdists must include the source files for all supported configurations, regardless of conditionals, including conditionals that involve ghc.

@jneira
Copy link
Member

jneira commented Aug 21, 2022

Hmm sdist is not used to upload to hackage diretly, you could use another tool to generate the tarball to be uploaded and you can use the tar generated by sdist for other purposes.
Otoh you dont upload cabal.project to hackage so changing manually the field packages in a cabal.project.local before a sdist would be equivalent to use one or another ghc in my example (or, he, upload only some of the tar generated or whatever) and hackage will never know why there are those packages and no other ones.

@roberth
Copy link

roberth commented Aug 21, 2022

Hmm sdist is not used to upload to hackage diretly

My release scripts do call sdist before uploading. IIRC cabal upload fails if you don't call cabal sdist first. If there's a better way to do it, I'm all ears.

Regardless, the current behavior is a breaking change.

Otoh you dont upload cabal.project to hackage so changing manually the field packages

Are you suggesting that the ghc dependency is only due to the cabal.project feature?
My first encounter with this bug was in a Nixpkgs feature that does not use cabal.project, so that was a little confusing. My release scripts don't rely on cabal.project either, so I guess I'm mostly unaffected then.

Shouldn't release commands always produce the same behavior regardless of the Haskell and OS platforms they run on?

hamishmack added a commit to input-output-hk/haskell.nix that referenced this issue Sep 2, 2022
@Mikolaj
Copy link
Member

Mikolaj commented Sep 5, 2022

@Mikolaj Mikolaj removed the regression on master Regression that is unreleased and needs to be fixed before release label Sep 8, 2022
@gbaz gbaz self-assigned this Sep 8, 2022
hamishmack added a commit to input-output-hk/haskell.nix that referenced this issue Sep 13, 2022
A bug was fixed in cabal 3.8 (haskell/cabal#6771) that haskell.nix relied on for `pkg-config` in support.  To work around this we added a dummy `pkg-config` that `cabal configure` now uses when haskell.nix runs it `cabal configure` internally.  That returns version information for all the `pkg-config` packages in `lib/pkgconfig-nixpkgs-map.nix`.  This mapping has also been expanded based on the results of searching nixpkgs for `*.pc` files.

This update also includes workarounds for:

haskell/cabal#8352
haskell/cabal#8370
haskell/cabal#8455
hamishmack added a commit to input-output-hk/haskell.nix that referenced this issue Sep 15, 2022
hamishmack added a commit to input-output-hk/haskell.nix that referenced this issue Sep 15, 2022
* Add cabal-issue-8352-workaround

See haskell/cabal#8352

* More script fixes

* More script fixes
@gbaz
Copy link
Collaborator

gbaz commented Sep 21, 2022

resolved by #8358

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants