cabal icon indicating copy to clipboard operation
cabal copied to clipboard

only check for compiler when project file has conditionals

Open gbaz opened this issue 3 years ago • 3 comments

Should be a simple fix for https://github.com/haskell/cabal/issues/8352 -- first check that a project file has conditionals before attempting to fetch the compiler/arch etc info for use in evaluating them. That way, as long as a project file does not have conditionals, actions which did not require ghc to be available (such as sdist) will remain not requiring ghc.

gbaz avatar Aug 10 '22 22:08 gbaz

@mergify rebase

Mikolaj avatar Aug 11 '22 11:08 Mikolaj

rebase

✅ Branch has been successfully rebased

mergify[bot] avatar Aug 11 '22 11:08 mergify[bot]

Added a test, but it succeeds even when it should fail because I can't set the env to not have ghc in the path properly somehow. Setting it to Nothing makes the system scream about how there's no path at all, but setting it to Just "" doesn't seem to kill the path, just to like not append to it? sigh.

Also I discovered in working on this that sdist's fix to respect project options in https://github.com/haskell/cabal/pull/8109 was incomplete and I made a hopefully totally safe change to fix that too...

gbaz avatar Aug 11 '22 20:08 gbaz

(live testing as requested in cabal-dev meet)

I can confirm that on my dev machine, with GHC not in PATH, cabal update:

  • in master complains about missing GHC;
  • in gb/compiler-check-logic does not.

ffaf1 avatar Aug 25 '22 17:08 ffaf1

As per our discussion in the meeting, we don't seem to be able to test with an altered path missing ghc, so I've rolled back the not-working tests and we'll rely on manual verification. After I fix up docs and changelog this is ready for merge.

gbaz avatar Aug 25 '22 19:08 gbaz

Last minute idea about a test to show that GHC is not needed... I see a bunch of withEnv [("WITH_GHC", Just ghc_path)] in the test suite right now. Perhaps, if you have garbage in ghc_path, it will fall if and only if it's trying to call GHC?

ulysses4ever avatar Aug 25 '22 19:08 ulysses4ever

good thought, but i just tried that and that didn't work either. Terribly confusing.

gbaz avatar Aug 25 '22 22:08 gbaz

i suppose it relies in the PATH anyways, maybe a withEnv [("PATH", Just ${pathWithoutGHC})] would do the trick

jneira avatar Aug 26 '22 16:08 jneira

@jneira Gershom tried it, see above:

setting it to Just "" doesn't seem to kill the path, just to like not append to it? sigh

ulysses4ever avatar Aug 26 '22 17:08 ulysses4ever

@gbaz: would you prefer a merge_me or a squash_me?

Mikolaj avatar Sep 12 '22 13:09 Mikolaj

Certainly a squash -- I was hoping to update the docs first, but will not be able to get to it until next week.

gbaz avatar Sep 14 '22 23:09 gbaz

Let me squash if for you, in that case. :)

Mikolaj avatar Sep 16 '22 08:09 Mikolaj

Mergify probably balks at the merge delay passed label being tweaked manually (due to mergify script revisions), so we've got one more chance to edit this PR. @gbaz: are we merging or would you like to update the docs first?

Mikolaj avatar Sep 19 '22 09:09 Mikolaj

@Mikolaj added changelog and docs. Please do squash/merge!

gbaz avatar Sep 20 '22 18:09 gbaz

With pleasure.

Mikolaj avatar Sep 20 '22 18:09 Mikolaj

@mergify rebase

Mikolaj avatar Sep 20 '22 18:09 Mikolaj

rebase

✅ Branch has been successfully rebased

mergify[bot] avatar Sep 20 '22 18:09 mergify[bot]