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

[3.14] Initial configuration of programs does not use build-tool-depends #10692

Open
jasagredo opened this issue Dec 31, 2024 · 5 comments · May be fixed by #10731
Open

[3.14] Initial configuration of programs does not use build-tool-depends #10692

jasagredo opened this issue Dec 31, 2024 · 5 comments · May be fixed by #10731

Comments

@jasagredo
Copy link
Collaborator

Describe the bug
The store path in which the alex build tool is placed does not correspond with the one used later on to build Cabal-syntax.

To Reproduce

➜ PROJ=$(mktemp -d)
➜ cd $PROJ
➜ cabal init -n -d Cabal-syntax==3.14.1.0 -d base
...
➜ cabal-3.14.1.0 build
Resolving dependencies...
Build profile: -w ghc-9.6.6 -O1
In order, the following will be built (use -v for more details):
 - alex-3.5.2.0 (exe:alex) (requires build)
 - Cabal-syntax-3.14.1.0 (lib) (requires build)
 - tmp-Xg8Fj7x2N4-0.1.0.0 (exe:tmp-Xg8Fj7x2N4) (first run)
Starting     alex-3.5.2.0 (exe:alex)
Building     alex-3.5.2.0 (exe:alex)
Installing   alex-3.5.2.0 (exe:alex)
Completed    alex-3.5.2.0 (exe:alex)
Starting     Cabal-syntax-3.14.1.0 (lib)
Building     Cabal-syntax-3.14.1.0 (lib)

Failed to build Cabal-syntax-3.14.1.0.
Build log (
C:\Users\Javier\AppData\Local\cabal\logs\ghc-9.6.6\Cabal-syntax-3.14.1.0-cd5ebad5757c2e92bfba1d2f77f930f4e00add04.log
):
Preprocessing library for Cabal-syntax-3.14.1.0...
alex.exe: C:\Users\Javier\AppData\Local\cabal\store\ghc-9.6.6\alex-3.5.1.0-868184b46a75d3f1207acacf299a27ecc42d81d8\share/AlexTemplate.hs: openFile: does not exist (No such file or directory)
Error: [Cabal-7125]
Failed to build Cabal-syntax-3.14.1.0 (which is required by exe:tmp-Xg8Fj7x2N4 from tmp-Xg8Fj7x2N4-0.1.0.0). See the build log above for details.
➜ ls 'C:\Users\Javier\AppData\Local\cabal\store\ghc-9.6.6\' | grep alex
alex-3.5.2.0-291af4c4dce682a2c469bb7d8ce8566bb62a9bb1

I removed all the alex folders before starting this process. This doesn't happen on 3.12.1.0.

System information

  • Windows 11 24H2, MSYS2.
  • cabal: 3.14.1.0
  • ghc: 9.6.6

On Linux it does work:

javier@javier-VirtualBox:~$ PROJ=$(mktemp -d)
javier@javier-VirtualBox:~$ cd $PROJ
javier@javier-VirtualBox:/tmp/tmp.btAJnhl1KU$ cabal init -n -d Cabal-syntax==3.14.1.0 -d base
...
javier@javier-VirtualBox:/tmp/tmp.btAJnhl1KU$ cabal build
Resolving dependencies...
Build profile: -w ghc-9.4.8 -O1
In order, the following will be built (use -v for more details):
 - alex-3.5.2.0 (exe:alex) (requires download & build)
 - Cabal-syntax-3.14.1.0 (lib) (requires download & build)
 - tmp-btAJnhl1KU-0.1.0.0 (exe:tmp-btAJnhl1KU) (first run)
Downloading  alex-3.5.2.0
Downloaded   alex-3.5.2.0
Downloading  Cabal-syntax-3.14.1.0
Starting     alex-3.5.2.0 (exe:alex)
Downloaded   Cabal-syntax-3.14.1.0
Building     alex-3.5.2.0 (exe:alex)
Installing   alex-3.5.2.0 (exe:alex)
Completed    alex-3.5.2.0 (exe:alex)
Starting     Cabal-syntax-3.14.1.0 (lib)
Building     Cabal-syntax-3.14.1.0 (lib)
Installing   Cabal-syntax-3.14.1.0 (lib)
Completed    Cabal-syntax-3.14.1.0 (lib)
Configuring executable 'tmp-btAJnhl1KU' for tmp-btAJnhl1KU-0.1.0.0...
Preprocessing executable 'tmp-btAJnhl1KU' for tmp-btAJnhl1KU-0.1.0.0...
Building executable 'tmp-btAJnhl1KU' for tmp-btAJnhl1KU-0.1.0.0...
[1 of 1] Compiling Main             ( app/Main.hs, dist-newstyle/build/x86_64-linux/ghc-9.4.8/tmp-btAJnhl1KU-0.1.0.0/x/tmp-btAJnhl1KU/build/tmp-btAJnhl1KU/tmp-btAJnhl1KU-tmp/Main.o )
[2 of 2] Linking dist-newstyle/build/x86_64-linux/ghc-9.4.8/tmp-btAJnhl1KU-0.1.0.0/x/tmp-btAJnhl1KU/build/tmp-btAJnhl1KU/tmp-btAJnhl1KU
@jasagredo jasagredo changed the title Non-existent build-tool-depends dependency [3.14] Non-existent build-tool-depends dependency Dec 31, 2024
@jasagredo jasagredo changed the title [3.14] Non-existent build-tool-depends dependency [3.14] [Windows] Non-existent build-tool-depends dependency Dec 31, 2024
@jasagredo
Copy link
Collaborator Author

Cabal 3.14.1.0 finds the alex in the path, while Cabal 3.12.1.0 finds the build-tool alex:

> cabal-3.14.1.0 build Cabal-syntax --verbose
...
Using alex version 3.5.1.0 given by user at:
C:\Users\Javier\AppData\Roaming\cabal\bin\alex.exe

> cabal-3.12.1.0 build Cabal-syntax --verbose
...
Using alex version 3.5.2.0 found on system at:
C:\Users\Javier\AppData\Local\cabal\store\ghc-9.6.6\alex-3.5.2.0-efe6d76d1a54d29458569f5bc3280e088145510b\bin\alex.exe

I suspect this might be happening also on Linux.

@jasagredo
Copy link
Collaborator Author

It is this line the one that causes alex to be misconfigured https://github.com/haskell/cabal/pull/9967/files#diff-2adae381a2eb2d8c804d5396694a431e1e3babd816a5d2408a62213cf774a893R506.

It tries to configure all known programs before setting up the project, therefore searching only in the system PATH, without the build-tool-depends path. If an alex is found then it will not be re-configured with the build-tool-depends path.

cc @sheaf as the author of that line.

The verbose logs with markers before and after that call, mentioning "alex":

> $CABAL build Cabal-syntax --builddir blah --verbose=3 2>&1 | tee out-verb.txt
...
> rg "\balex\b" out-verb.txt | head
MARKER BEFORE (alex)
Searching for alex in path: [ProgramSearchPathDefault]
Found alex at C:\Users\Javier\AppData\Roaming\cabal\bin\alex.exe
Running: "C:\Users\Javier\AppData\Roaming\cabal\bin\alex.exe" "--version"
C:\Users\Javier\AppData\Roaming\cabal\bin\alex.exe is version 3.5.1.0
MARKER AFTER (alex)

@jasagredo jasagredo changed the title [3.14] [Windows] Non-existent build-tool-depends dependency [3.14] Non-existent build-tool-depends dependency Jan 2, 2025
@jasagredo jasagredo changed the title [3.14] Non-existent build-tool-depends dependency [3.14] Initial configuration of programs does not use build-tool-depends Jan 2, 2025
@sheaf
Copy link
Collaborator

sheaf commented Jan 2, 2025

Thanks for opening this ticket and taking the time to investigate. I suspect this is probably the same issue as #10633?

To explain my change:

finalProgDb <- liftIO $ configureAllKnownPrograms verbosity progdb''

The problem is that the recompilation checking logic inside which that code is (rerunIfChanged) uses Binary instances, and the Binary instance for ProgramDb simply drops all unconfigured programs. This means that one would get different behaviour:

  1. If doing the build the first time (in which case we continue to have all unconfigured programs available).
  2. If using a cache, in which case we re-load from the Binary instance and suddenly all unconfigured programs have disappeared from the ProgramDb.

I think there are only two ways of making this particular piece of code correct:

  1. Configure all the unconfigured programs (as I did in my previous commit), or
  2. Drop all unconfigured programs.

If (2) passes CI then I would be fine with that approach as well.
On the other hand, I would be opposed to simply reverting the change, because that would be re-introducing dangerously incorrect logic which causes the ProgramDb to be silently different in the first build or when using the cache.

@phadej
Copy link
Collaborator

phadej commented Jan 5, 2025

Using the different version from what specified in build-tool-depends is as far I can tell a critical issue.

build-tool-depends should override whatever default there is.

Perfectly, Cabal won't even configure alex, happy, hsc2hs etc by default, but require people to add build-tool-depends fields to make the dependency explicit. (For some future cabal-version).

Work towards making the builds hygienic.

@mpickering
Copy link
Collaborator

I've been assigned this issue, I have an idea about what's happening and will work on a fix tomorrow.

@mpickering mpickering self-assigned this Jan 8, 2025
mpickering added a commit to mpickering/cabal that referenced this issue Jan 9, 2025
In configureCompiler the call to configureAllKnownPrograms was too
eager. When called it selected the version of tools from PATH (such as
alex), and then when a package was configured these tools were passed
using `--with-alex` options to configure, which meant that the
build-tool-depends versions were not used. (See haskell#10692)

Why was this call introduced in the first place? Because
configureCompiler would a different result depending on whether:

* It is run for the first time, the `ProgramDb` will contain
  unconfigured programs.
* It is run subsequently, `ProgramDb` is read from disk, it does not
  contain unconfigured programs.

A surgical way to fix this is to avoid configuring the programs, and
manually adding back the builtinPrograms to the ProgramDb, so the
ProgramDb returned by configureCompiler always contains all the
unconfigured programs.

The testcase is not so easy to write because

* The bug only surfaces when the build-tool you are depending on is
  known (ie alex, happy etc)
* But then it is tricky to write a test, as we can't depend on the known
  tools or bundle the source for them.
* So we create a fake "alex", which cabal then invokes on a fake ".x"
  file. This is maybe a bit fragile if the way cabal invokes alex
  changes in future, but then the test can be modified as well.

Also see haskell#2241 and haskell#9840

Fixes haskell#10692
@mpickering mpickering linked a pull request Jan 9, 2025 that will close this issue
6 tasks
mpickering added a commit to mpickering/cabal that referenced this issue Jan 9, 2025
In configureCompiler the call to configureAllKnownPrograms was too
eager. When called it selected the version of tools from PATH (such as
alex), and then when a package was configured these tools were passed
using `--with-alex` options to configure, which meant that the
build-tool-depends versions were not used. (See haskell#10692)

Why was this call introduced in the first place? Because
configureCompiler would a different result depending on whether:

* It is run for the first time, the `ProgramDb` will contain
  unconfigured programs.
* It is run subsequently, `ProgramDb` is read from disk, it does not
  contain unconfigured programs.

A surgical way to fix this is to avoid configuring the programs, and
manually adding back the builtinPrograms to the ProgramDb, so the
ProgramDb returned by configureCompiler always contains all the
unconfigured programs.

The testcase is not so easy to write because

* The bug only surfaces when the build-tool you are depending on is
  known (ie alex, happy etc)
* But then it is tricky to write a test, as we can't depend on the known
  tools or bundle the source for them.
* So we create a fake "alex", which cabal then invokes on a fake ".x"
  file. This is maybe a bit fragile if the way cabal invokes alex
  changes in future, but then the test can be modified as well.

Also see haskell#2241 and haskell#9840

Fixes haskell#10692
mpickering added a commit to mpickering/cabal that referenced this issue Jan 13, 2025
This reverts commit 8bdda9c.

In configureCompiler the call to configureAllKnownPrograms was too
eager. When called it selected the version of tools from PATH (such as
alex), and then when a package was configured these tools were passed
using `--with-alex` options to configure, which meant that the
build-tool-depends versions were not used. (See haskell#10692)

Why was this call introduced in the first place? Because
configureCompiler would a different result depending on whether:

* It is run for the first time, the `ProgramDb` will contain
  unconfigured programs.
* It is run subsequently, `ProgramDb` is read from disk, it does not
  contain unconfigured programs.

Reverting this commit rexposes the bug that the serialised ProgramDb
will not contain UnconfiguredPrograms (in the case where reconfiguring
is avoided).

However, there are no code paths which require this logic in
`cabal-install` currently. The configuration phase happens again each
time that `Cabal` is called, with a populated `ProgramDb`. We will
fix this before the next major `cabal-install` release, but it would not
be suitable to backport.

In the future we will fix this properly by refactoring
`configureCompiler` so that `ProgramDb` is not serialised. The general
approach will be to make `configCompilerEx` return a pair of configured
programs (`ghc` and `ghc-pkg`) and expose an additional function from
`Cabal` which uses these two programs to perform the modifications to
the `ProgramDb` which `configCompilerEx` performs.

Also see haskell#2238 and haskell#9840

Fixes haskell#10692
mpickering added a commit to mpickering/cabal that referenced this issue Jan 13, 2025
The testcase is not so easy to write because

* The bug only surfaces when the build-tool you are depending on is
  known (ie alex, happy etc)
* But then it is tricky to write a test, as we can't depend on the known
  tools or bundle the source for them.
* So we create a fake "alex", which cabal then invokes on a fake ".x"
  file. This is maybe a bit fragile if the way cabal invokes alex
  changes in future, but then the test can be modified as well.

Ticket haskell#10692
mpickering added a commit to mpickering/cabal that referenced this issue Jan 13, 2025
Whilst fixing haskell#10692, I realised there was also this bug where
extra-prog-path would not be honoured for specific packages.

The idea behind extra-prog-path is that each local package can use a
different version of a preprocessor if desired.
mpickering added a commit to mpickering/cabal that referenced this issue Jan 13, 2025
Whilst fixing haskell#10692, I realised there was also this bug where
extra-prog-path would not be honoured for specific packages.

The idea behind extra-prog-path is that each local package can use a
different version of a preprocessor if desired.
mpickering added a commit to mpickering/cabal that referenced this issue Jan 13, 2025
The testcase is not so easy to write because

* The bug only surfaces when the build-tool you are depending on is
  known (ie alex, happy etc)
* But then it is tricky to write a test, as we can't depend on the known
  tools or bundle the source for them.
* So we create a fake "alex", which cabal then invokes on a fake ".x"
  file. This is maybe a bit fragile if the way cabal invokes alex
  changes in future, but then the test can be modified as well.

Ticket haskell#10692
mpickering added a commit to mpickering/cabal that referenced this issue Jan 13, 2025
Whilst fixing haskell#10692, I realised there was also this bug where
extra-prog-path would not be honoured for specific packages.

The idea behind extra-prog-path is that each local package can use a
different version of a preprocessor if desired.
sheaf pushed a commit to mpickering/cabal that referenced this issue Jan 15, 2025
This reverts commit 8bdda9c.

In configureCompiler the call to configureAllKnownPrograms was too
eager. When called it selected the version of tools from PATH (such as
alex), and then when a package was configured these tools were passed
using `--with-alex` options to configure, which meant that the
build-tool-depends versions were not used. (See haskell#10692)

Why was this call introduced in the first place? Because
configureCompiler would a different result depending on whether:

* It is run for the first time, the `ProgramDb` will contain
  unconfigured programs.
* It is run subsequently, `ProgramDb` is read from disk, it does not
  contain unconfigured programs.

Reverting this commit rexposes the bug that the serialised ProgramDb
will not contain UnconfiguredPrograms (in the case where reconfiguring
is avoided).

However, there are no code paths which require this logic in
`cabal-install` currently. The configuration phase happens again each
time that `Cabal` is called, with a populated `ProgramDb`. We will
fix this before the next major `cabal-install` release, but it would not
be suitable to backport.

In the future we will fix this properly by refactoring
`configureCompiler` so that `ProgramDb` is not serialised. The general
approach will be to make `configCompilerEx` return a pair of configured
programs (`ghc` and `ghc-pkg`) and expose an additional function from
`Cabal` which uses these two programs to perform the modifications to
the `ProgramDb` which `configCompilerEx` performs.

Also see haskell#2238 and haskell#9840

Fixes haskell#10692
sheaf pushed a commit to mpickering/cabal that referenced this issue Jan 15, 2025
The testcase is not so easy to write because

* The bug only surfaces when the build-tool you are depending on is
  known (ie alex, happy etc)
* But then it is tricky to write a test, as we can't depend on the known
  tools or bundle the source for them.
* So we create a fake "alex", which cabal then invokes on a fake ".x"
  file. This is maybe a bit fragile if the way cabal invokes alex
  changes in future, but then the test can be modified as well.

Ticket haskell#10692
sheaf pushed a commit to mpickering/cabal that referenced this issue Jan 15, 2025
Whilst fixing haskell#10692, I realised there was also this bug where
extra-prog-path would not be honoured for specific packages.

The idea behind extra-prog-path is that each local package can use a
different version of a preprocessor if desired.
sheaf pushed a commit to mpickering/cabal that referenced this issue Jan 15, 2025
This reverts commit 8bdda9c.

In configureCompiler the call to configureAllKnownPrograms was too
eager. When called it selected the version of tools from PATH (such as
alex), and then when a package was configured these tools were passed
using `--with-alex` options to configure, which meant that the
build-tool-depends versions were not used. (See haskell#10692)

Why was this call introduced in the first place? Because
configureCompiler would a different result depending on whether:

* It is run for the first time, the `ProgramDb` will contain
  unconfigured programs.
* It is run subsequently, `ProgramDb` is read from disk, it does not
  contain unconfigured programs.

Reverting this commit rexposes the bug that the serialised ProgramDb
will not contain UnconfiguredPrograms (in the case where reconfiguring
is avoided).

However, there are no code paths which require this logic in
`cabal-install` currently. The configuration phase happens again each
time that `Cabal` is called, with a populated `ProgramDb`. We will
fix this before the next major `cabal-install` release, but it would not
be suitable to backport.

In the future we will fix this properly by refactoring
`configureCompiler` so that `ProgramDb` is not serialised. The general
approach will be to make `configCompilerEx` return a pair of configured
programs (`ghc` and `ghc-pkg`) and expose an additional function from
`Cabal` which uses these two programs to perform the modifications to
the `ProgramDb` which `configCompilerEx` performs.

Also see haskell#2238 and haskell#9840

Fixes haskell#10692
sheaf pushed a commit to mpickering/cabal that referenced this issue Jan 15, 2025
The testcase is not so easy to write because

* The bug only surfaces when the build-tool you are depending on is
  known (ie alex, happy etc)
* But then it is tricky to write a test, as we can't depend on the known
  tools or bundle the source for them.
* So we create a fake "alex", which cabal then invokes on a fake ".x"
  file. This is maybe a bit fragile if the way cabal invokes alex
  changes in future, but then the test can be modified as well.

Ticket haskell#10692
sheaf pushed a commit to mpickering/cabal that referenced this issue Jan 15, 2025
Whilst fixing haskell#10692, I realised there was also this bug where
extra-prog-path would not be honoured for specific packages.

The idea behind extra-prog-path is that each local package can use a
different version of a preprocessor if desired.
sheaf pushed a commit to mpickering/cabal that referenced this issue Jan 15, 2025
Whilst fixing haskell#10692, I realised there was also this bug where
extra-prog-path would not be honoured for specific packages.

The idea behind extra-prog-path is that each local package can use a
different version of a preprocessor if desired.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants