commercialhaskell / stack

The Haskell Tool Stack
http://haskellstack.org
BSD 3-Clause "New" or "Revised" License
3.95k stars 842 forks source link

Source map hashes ignore Cabal flags that are enabled, in error #6565

Closed mpilgrem closed 2 months ago

mpilgrem commented 2 months ago

Reference: https://github.com/commercialhaskell/stack/issues/6564#issuecomment-2091945886

Stack.Build.Source.hashSourceMapData yields a hash from build options on the command line and a source map, making use of depPackageHashableContent for immutable dependencies. However, it appears to me that the latter contains a bug that means that dependencies' Cabal flags that are enabled (as opposed to those that are disabled) are excluded from the hash. This would mean that Cabal flags could differ but the same snapshot database would be used. If I am correct, this will have applied since Stack 2.1.1.

I think:

let flagToBs (f, enabled) =
      if enabled
        then ""
        else "-" <> fromString (C.unFlagName f)

was intended to be:

let flagToBs (f, enabled) =
         (if enabled then "" else "-")
      <> fromString (C.unFlagName f)

Pinging @qrilka (the author of https://github.com/commercialhaskell/stack/commit/6d57ab5b2ea509b18d23860c7a72904bd9b8948d in November 2018) and @theobat, in case they can offer a second opinion.

mpilgrem commented 2 months ago

To see how this manifests, consider a simple one-package project test6565 with a dependency on ansi-terminal-0.11.5 (snapshot: lts-21.25) and a package.yaml with spec-version: 0.36.0 (to avoid the complication of Paths_test6565).

stack build --flag *:-dummy1 followed by -dummy2, -dummy3 etc (disabled Cabal flags) results in a rebuild of the dependencies as different immuatable snapshot databases.

stack build --flag *:dummy1 followed by dummy2, dummy3 etc (enabled Cabal flags) results in 'no op' builds as regards the dependencies; the same immutable snapshot database is being used each time.

mpilgrem commented 2 months ago

The original reasoning may have been that an immutable package's identifier uniquely identies the set of Cabal flags and the Cabal flags that are disabled uniquely identifies the state (enabled or disabled) of that set of flags. However, that reasoning is not valid when --flag *:-<flag_name> adds a disabled Cabal flag for a package that does not exist in the package's Cabal file.

So perhaps the problem can be thought of differently: the problem is that --flag *:[-]<flag_name> adds a flag to packages where the flag name may not exist in the Cabal file for the package.

qrilka commented 2 months ago

Unfortunately @mpilgrem I don't quite remember the exact details around this commit from 5.5 years ago but it looks to me that your logic is quite right here and this use case of reenabling flags was just not taken into consideration, i.e. it's a bug.