Additional flag settings are ignored if the original ones are in a conditional block

[michaelpj]

Describe the bug
This is a weird one. What I'm observing is that this:

package foo
   flags: +bar

package foo
   flags: -bar

builds foo with bar disabled (correct), but that this:

-- Assume we're using GHC > 9 so this conditional is satisfied
if impl(ghc>9)
   package foo
       flags: +bar

pacakge foo
   flags: -bar

builds foo with bar enabled (incorrect).

To Reproduce

• Checkout https://github.com/haskell/lsp/tree/mpj/cabal-bug
• cabal build lsp (with GHC > 9 so the conditional is satisfied)
  • Observe that cabal declares that it is going to build exe:lsp-demo-simple-server
• Remove the conditional around the flag setting in cabal.project
• cabal build lsp
  • Observe that cabal does not declare that it is going to build exe:lsp-demo-simple-server

Expected behavior

The second flag setting should override the first regardless of whether the first one is conditional.

System information

• NixOS
• `cabal-3.10.1.01
• ghc-9.2.7

Additional context

I don't think this is #8699, because:

1. I can reproduce it even if I wipe out dist-newstyle before building; and
2. The problem is not the conditional section being ignored, but rather the later unconditional section being ignored.

[michaelpj]

Workaround:

package foo 
   flags: +bar

-- flipped conditional
if impl(ghc<9)
   package foo
       flags: -bar

pacakge foo
   flags: -bar

Now the override works where you want it to.

[gbaz]

The reason for the funny semantics is we first process all elements of a file except for the conditionals. Then we determine the compiler and platform, which may actually have been set by the file, and then having done so, we evaluate the conditionals.

At a minimum this should be documented. I'm not sure how we might get a different semantics here without doing significant violence to the structure of the code, and perhaps creating other confusing semantic issues.

[michaelpj]

The platform can't be set by the file, right? It's really just the compiler that's problematic, I think.

So it sounds like we're worried about files like:

if os(windows)
  with-compiler: ghc-9.2
else
  with-compiler: ghc-9.6

or

-- no fixpoint!
if impl(ghc < 9.2)
  with-compiler: ghc-9.4
else
  with-compiler: ghc-9.0

However, the ability to override settings in cabal.project.local is pretty important, an it's quite bad if conditionals break that.

---

I think we probably want the fixpoint semantics, but the fixpoint might not exist (see the loopy example above), unless we restrict the inputs we allow.

The best idea I can think of is to ban with-compiler from appearing inside a conditional. Then you can evaluate once to get the compiler, and then again to get the final version of everything.

This seems straightforward and fairly effective, OTOH it bans the first example above, which is maybe a reasonable thing to want.

[geekosaur]

The loopy fixpoint was the first thing I thought of when I saw this ticket. I would ban with-compiler from if impl(…) blocks, and similarly for any other conditionals that might be affected by settings (but I don't think there are any). This might still allow looping in more complex cases, though (although if impl is the only one that can be affected by conditionals then probably not), so we would also need to set a (probably small: 2 should be enough) limit on number of passes.

[michaelpj]

This might still allow looping in more complex cases

I'm pretty sure that if with-compiler doesn't appear in a conditional then you can't loop and you're guaranteed to reach fixpoint in exactly two iterations.

[gbaz]

We currently don't allow with-compiler in conditionals, for the reasons described. However we didn't think to run the whole processing twice to resolve issues such as that initially reported. Its a smart idea, and could even arguably clean up the processing if done right, because we wouldn't have to extract and run conditionals only in the second pass (as we now do) -- rather we could just run a full pass, conditionals and all, twice. Definitely would welcome a PR.