Unnecessary rebuild in circular dep causes issues

[brandonchinn178]

Overview

At a high level, Stack allows packages to have circular dependencies if it's linearizable at the component level (Cabal flat out rejects to build this situation). However, Stack will fail to recognize that it can reuse the first library and rebuild, causing the second library to be using an old version of the first library.

This is similar to #3130 in that both issues stem from a library still using an old version of a dependent library. But in #3130, the problem is Stack not rebuilding enough, and in this issue, the problem is Stack rebuilds unnecessarily. In fact, #3130 is not an issue if you stack clean before every build, but this issue is still present even from a clean build.

Problems

In all of the situations below, we have the following project:

-- my-lib/my-lib.cabal
cabal-version: 3.0
name: my-lib
version: 0
build-type: Simple

library
  exposed-modules: MyLib
  hs-source-dirs: src
  build-depends: base

test-suite my-lib-tests
  type: exitcode-stdio-1.0
  main-is: Test.hs
  build-depends: base, my-lib, my-test-utils

-- my-test-utils/my-test-utils.cabal
cabal-version: 3.0
name: my-test-utils
version: 0
build-type: Simple

library
  exposed-modules: MyTestUtils
  hs-source-dirs: src
  build-depends: base, my-lib

This creates a dependency chain that looks like:

my-lib:lib
  │ └─> my-test-utils:lib
  │      ↓
  └─> my-lib:test-suite    

When running a stack test, Stack will do the following:

1. Build my-lib:lib
2. Build my-test-utils:lib against my-lib:lib (1)
3. Rebuild (!!) my-lib:lib
4. Build my-lib:test-suite against my-test-utils:lib and my-lib:lib (2)

Problem 1: compilation error with old data type

In this minimal repro, we can see that my-test-utils is still using the old library's Foo data type (as exposed by the foo function), as GHC complains that you're trying to compare an old Foo type with a new Foo type.

-- my-lib/src/MyLib.hs
module MyLib (foo) where

data Foo = Foo Int True
  deriving (Show, Eq)

foo :: Foo
foo = Foo 1 True

-- my-test-utils/src/MyTestUtils.hs
module MyTestUtils (foo) where

import MyLib

-- my-lib/Test.hs
import qualified MyLib
import qualified MyTestUtils

main :: IO ()
main = print $ MyLib.foo == MyTestUtils.foo

Expected

Works

Actual

my-lib/Test.hs:9:24: error: [GHC-83865]
    • Couldn't match expected type ‘MyLib.Foo’
                  with actual type ‘my-lib-0:MyLib.Foo’
      NB: ‘MyLib.Foo’ is defined in ‘MyLib’ in package ‘my-lib-0’
          ‘my-lib-0:MyLib.Foo’ is defined in ‘MyLib’ in package ‘my-lib-0’
    • In the second argument of ‘(==)’, namely ‘MyTestUtils.foo’
      In the second argument of ‘($)’, namely
        ‘MyLib.foo == MyTestUtils.foo’
      In a stmt of a 'do' block: print $ MyLib.foo == MyTestUtils.foo
  |
9 |   print $ MyLib.foo == MyTestUtils.foo
  |                        ^^^^^^^^^^^^^^^

Problem 2: surprising behavior with global IORef

This situation is the original problem I ran into, and it was surprising because it actually compiles successfully (unlike problem 1), but it has surprising behavior at runtime: the global IORef is actually initialized twice, once for my-lib:lib (1) and once for my-lib:lib (2). So writeRefFromTestUtils actually sets a different IORef than the one read from readRefFromLib.

-- my-lib/src/MyLib.hs
module MyLib (
  readRefFromLib,
  setRefForTests,
) where

import Data.IORef
import System.IO.Unsafe

myRef :: IORef Int
myRef = unsafePerformIO $ newIORef 0
{-# NOINLINE myRef #-}

readRefFromLib :: IO ()
readRefFromLib = do
  x <- readIORef myRef
  putStrLn $ "readRefFromLib: " <> show x

setRefForTests :: Int -> IO ()
setRefForTests x = do
  writeIORef myRef x
  putStrLn $ "setRefForTests: " <> show x

-- my-test-utils/src/MyTestUtils.hs
module MyTestUtils where

import Data.IORef
import MyLib

writeRefFromTestUtils :: IO ()
writeRefFromTestUtils =
  setRefForTests 123

-- my-lib/Test.hs
import MyLib
import MyTestUtils

main :: IO ()
main = do
  readRefFromLib
  writeRefFromTestUtils
  readRefFromLib

Expected

readRefFromLib: 0
setRefForTests: 123
readRefFromLib: 123

Actual

readRefFromLib: 0
setRefForTests: 123
readRefFromLib: 0

[tfausak]

This sounds similar to #3130.

[brandonchinn178]

Yeah, it's similar to #3130 in that the root cause is an old package and new package are both being used.

But it's slightly different in that one solutiom to #3130 is "invalidate more, cause a rebuild", but that would actually cause a circular build in this case, as rebuilding my-test-utils would invalidate my-lib, which would force rebuilding my-test-utils.

The better solution for this issue is actually the opposite of #3130: avoid rebuilding the library when it was already built and only build the test-suite the second time around.

[brandonchinn178]

@tfausak okay I've updated the issue with an even smaller repro. I've also mentioned how it differs from #3130.

[mpilgrem]

Is this something to do with the solution to #2904? That is, I understand that Stack rebuilds more than it would like to because Cabal does not assign unique ids to packages based on their contents.

[brandonchinn178]

#2904 seems to be the same as #3130, no? That is, it seems like the resolution of that ticket is "unregister more", where I want "unregister less".

One comment in the other issue says "the package ID should change when the contents of the package change", but that's not what's happening in this issue. Here, the source code of the package hasnt changed, but Stack is still rebuilding.

As I mentioned in the description, #2904 and #3130 both are solvable by stack clean before every build, but this issue is still present in a clean build. In other words, the main issue behind #2904 and #3130 is building after a change, but this issue is building with a specific dependency tree

[mpilgrem]

Adding output from stack --verbose --plan-in-log test (extracts only):

[debug] Constructing the build plan
[debug] (addDep) Package info for my-lib: ... 
[debug] (addDep) Package info for base: ...
[debug] (addDep) Package info for my-test-utils: ...
[debug] (installPackage) No test or bench component for my-test-utils so doing an all-in-one build.
[debug] (getCachedDepOrAddDep) Using cached result for base: ...
[debug] (checkCallStackAndAddDep) Detected cycle my-lib: ["my-test-utils","my-lib"]
[debug] (installPackage) Before trying cyclic plan, resetting lib result map to: fromList []
[debug] (addDep) Package info for base: ...
[debug] (getCachedDepOrAddDep) Using cached result for base: ...
[debug] (addDep) Package info for my-test-utils: ...
[debug] (installPackage) No test or bench component for my-test-utils so doing an all-in-one build.
[debug] (getCachedDepOrAddDep) Using cached result for base: ...
[debug] (getCachedDepOrAddDep) Using cached result for my-lib: ..
[debug] (getCachedDepOrAddDep) Using cached result for my-test-utils: ...
[debug] Checking if we are going to build multiple executables with the same name
[debug] Executing the build plan
[info] my-lib       > configure
[info] my-lib       > build
[info] my-lib       > copy/register
[info] my-test-utils> configure (lib)
[info] my-test-utils> build (lib)
[info] my-test-utils> copy/register
[info] my-lib       > configure (test)
[info] my-lib       > build (test)
[info] my-lib       > Preprocessing library for my-lib-0.1.0.0..
[info] my-lib       > Building library for my-lib-0.1.0.0..
[info] my-lib       > [1 of 2] Compiling MyLib
[info] my-lib       > [2 of 2] Compiling Paths_my_lib
[info] my-lib       > Preprocessing test suite 'my-lib-tests' for my-lib-0.1.0.0..
[info] my-lib       > Building test suite 'my-lib-tests' for my-lib-0.1.0.0..
[info] my-lib       > [1 of 2] Compiling Main
[warn] my-lib       > 
[warn] my-lib       > D:\Users\mike\Code\Haskell\my-project\my-lib\test\Test.hs:5:29: error:
[warn] my-lib       >     • Couldn't match expected type ‘MyLib.Foo’
[info] my-lib       > [2 of 2] Compiling Paths_my_lib
[warn] my-lib       >                   with actual type ‘my-lib-0.1.0.0:MyLib.Foo’
[warn] my-lib       >       NB: ‘MyLib.Foo’ is defined in ‘MyLib’ in package ‘my-lib-0.1.0.0’
[warn] my-lib       >           ‘my-lib-0.1.0.0:MyLib.Foo’
[warn] my-lib       >             is defined in ‘MyLib’ in package ‘my-lib-0.1.0.0’
[warn] my-lib       >     • In the second argument of ‘(==)’, namely ‘MyTestUtils.foo’
[warn] my-lib       >       In the second argument of ‘($)’, namely
[warn] my-lib       >         ‘MyLib.foo == MyTestUtils.foo’
[warn] my-lib       >       In the expression: print $ MyLib.foo == MyTestUtils.foo
[warn] my-lib       >   |
[warn] my-lib       > 5 | main = print $ MyLib.foo == MyTestUtils.foo
[warn] my-lib       >   |                             ^^^^^^^^^^^^^^^