search

haskell / cabal

Official upstream development repository for Cabal and cabal-install
https://haskell.org/cabal
Other
1.62k stars 693 forks source link

`cabal install`: fails remotely, succeeds locally (with `build-tools: alex`) #7808

Open andreasabel opened 2 years ago

andreasabel commented 2 years ago

TL;DR It seems that cabal install BNFC-2.8.3 does not run alex, but cabal get BNFC-2.8.3 && cd BNFC-2.8.3 && cabal install does so. Consequently GHC-9.2.1-build fails in the former case but succeeds in the latter case.
What is then the truth value of BNFC-2.8.3 builds with GHC-9.2.1 ?

This question came up in the wild, when I was investigating whether BNFC-2.8.3 builds with GHC 9.2.1.

1. Fails when directly installing from hackage

$ cabal install -w ghc-9.2.1 BNFC-2.8.3
...
[ 16 of 104] Compiling LexBNF           ( dist/build/bnfc/bnfc-tmp/LexBNF.hs, dist/build/bnfc/bnfc-tmp/LexBNF.o )

dist/build/bnfc/bnfc-tmp/LexBNF.hs:405:50: error:
    • Couldn't match expected type ‘Int16#’ with actual type ‘Int#’
    • In the fifth argument of ‘alex_scan_tkn’, namely ‘sc’

2. Succeeds when first downloading

$ cabal get BNFC-2.8.3
Unpacking to BNFC-2.8.3/
$ cd BNFC-2.8.3
$ cabal install -w ghc-9.2.1 
...
[ 16 of 104] Compiling LexBNF           ( dist/build/bnfc/bnfc-tmp/LexBNF.hs, dist/build/bnfc/bnfc-tmp/LexBNF.o )
...
Linking dist/build/bnfc/bnfc ...

WAT?

Installation succeeds when LexBNF.x is processed by alex-3.2.6, which is my installed version:

$ alex --version
Alex version 3.2.6, (c) 2003 Chris Dornan and Simon Marlow

The plan for the local install indeed contains alex-3.2.6:

$ cabal-plan
...
alex-3.2.6
 [alex-3.2.6:exe:"alex"]
 ├─ array-0.5.4.0 ┄┄
 ├─ base-4.16.0.0 ┄┄
 ├─ containers-0.6.5.1 ┄┄
 └─ directory-1.3.6.2 ┄┄
...

These are the versions of LexBNF.hs in the directory structure:

$ find . -name "LexBNF.hs" -exec ls -l {} \;
76062 Aug 27  2019 ./dist/build/unit-tests/unit-tests-tmp/LexBNF.hs
76062 Aug 27  2019 ./dist/build/bnfc/bnfc-tmp/LexBNF.hs
76275 Nov  9 22:42 ./dist-newstyle/build/x86_64-osx/ghc-9.2.1/BNFC-2.8.3/x/bnfc/build/bnfc/bnfc-tmp/LexBNF.hs
69884 Aug 27  2019 ./src/LexBNF.hs

The third has been created during build by alex-3.2.6.

Further analysis of remote install

Note that constraining alex == 3.2.6 does not help with the remote install:

$ cabal install -w ghc-9.2.1 BNFC-2.8.3 --constraint='any.alex==3.2.6'
((SAME RESULT)

It is noteworthy that the package contains both:

More precisely, the tarball has:

BNFC-2.8.3/dist/build/unit-tests/unit-tests-tmp/LexBNF.hs
BNFC-2.8.3/dist/build/bnfc/bnfc-tmp/LexBNF.hs
BNFC-2.8.3/src/LexBNF.x
BNFC-2.8.3/src/LexBNF.hs

Some excerpts from -v3:

Resolving dependencies...
targets: BNFC
constraints: 
...
  any.alex ==3.2.6 (command line flag)
...
[_41] trying: BNFC:alex:exe.alex-3.2.6 (dependency of BNFC)
...
[_56] done
Elaborating the install plan...
Component graph for alex-3.2.6: component exe:alex
component lx-3.2.6-12b0a501
    include array-0.5.4.0
    include base-4.16.0.0
    include containers-0.6.5.1
    include directory-1.3.6.2
unit lx-3.2.6-12b0a501
    include array-0.5.4.0
    include base-4.16.0.0
    include containers-0.6.5.1
    include directory-1.3.6.2
...

Here, we break to remember that the Grinch stole the vowels from alex, just leaving us lx (#7209).

Configured alex-3.2.6 (lx-3.2.6-12b0a501)
  array-0.5.4.0
  base-4.16.0.0
  containers-0.6.5.1
  directory-1.3.6.2
...
Configured BNFC-2.8.3 (BNFC-2.8.3-87063006)
...
  lx-3.2.6-12b0a501
...
Installed alex-3.2.6 (lx-3.2.6-12b0a501)
...
((REPETITIONS of this...))
...
Build profile: -w ghc-9.2.1 -O1
In order, the following will be built:
 - BNFC-2.8.3-87063006 (exe:bnfc) (requires build)
 - BNFC-2.8.3-a710c271 (lib) (requires build)
...
Finalized package description:
...
build-depends:
array >=0 && ==0.5.4.0,
base (>=4.6 && <5) && ==4.16.0.0
...
executable bnfc
main-is: Main.hs
build-tools: alex >=0, happy >=0

We break to wonder why no constraint on alex made it into the final description.

...
Searching for alex in path.
Found alex at /Users/abel/.cabal/store/ghc-9.2.1/lx-3.2.6-12b0a501/bin/alex
/Users/abel/.cabal/store/ghc-9.2.1/lx-3.2.6-12b0a501/bin/alex --version
/Users/abel/.cabal/store/ghc-9.2.1/lx-3.2.6-12b0a501/bin/alex is version 3.2.6
...
Environment: ... ("PATH","/Users/abel/.cabal/store/ghc-9.2.1/lx-3.2.6-12b0a501/bin:/Users/abel/.cabal/store/ghc-9.2.1/hppy-1.20.0-d456cdf9/bin:...
...
Using alex version 3.2.6 found on system at:
/Users/abel/.cabal/store/ghc-9.2.1/lx-3.2.6-12b0a501/bin/alex
...
creating dist/build/bnfc/bnfc-tmp
...
/usr/local/bin/ghc-9.2.1 --make ... -outputdir dist/build/bnfc/bnfc-tmp ... src/Main.hs Paths_BNFC LexBNF ...
...
[ 15 of 104] Compiling ErrM             ( src/ErrM.hs, dist/build/bnfc/bnfc-tmp/ErrM.o )
[ 16 of 104] Compiling LexBNF           ( dist/build/bnfc/bnfc-tmp/LexBNF.hs, dist/build/bnfc/bnfc-tmp/LexBNF.o )

dist/build/bnfc/bnfc-tmp/LexBNF.hs:405:50: error:
    • Couldn't match expected type ‘Int16#’ with actual type ‘Int#’
...

I did not see an invocation of alex in -v3. It seems that LexBNF is taken from the shipped dist/build/bnfc/bnfc-tmp/.

phadej commented 2 years ago

IMHO, cabal check should warn:

And declare this issue as wontfix, as it's unclear what cabal build or install should do when there is an ambiguity or dist.

I have no idea why build works and install doesn't. I think both should fail. Probably we look for dist/ when installing due some old packages on Hackage, but really we shouldn't.

EDIT: I remember that alex itself did such trick for bootstrap, i.e. including preprocessed files. That didn't work very well, and we changed that so developers of alex need to preprocess inputs themselves when developing / before sdist. That's a slight inconvenience for alex developers but a huge win for everyone else. BNFC is even simpler: I think it shouldn't include preprocessed LexBNF.hs (in latest version) at all, and let alex do its job.

andreasabel commented 2 years ago

@phadej wrote:

  • when there is dist directory in the package

Well, this dist directory was added by the former self of cabal sdist. (It is not coming from extra-source-files etc.)

  • there is conflicting files (e.g. Foo.x and Foo.hs)

Same here. The cabal file just declares LexBNF in source-modules/other-modules and then cabal sdist added both the .x and the .hs file.

And declare this issue as wontfix, as it's unclear what cabal build or install should do when there is an ambiguity or dist.

So cabal is at the level of politicians that state: "I don't care about what I said yesterday." ? :-D

In the nature of things (meaning, hackage), cabal will have to handle the products of its former selves.

phadej commented 2 years ago

In the nature of things (meaning, hackage), cabal will have to handle the products of its former selves.

Yes, But before working around previous mistakes, we have to put fences so we don't repeat them. Otherwise people will rely on them and complexity only grows with time.

Then the problematic cases can be dealt one-by-one.

Well, this dist directory was added by the former self of cabal sdist?

Is it?

At least it's not added by cabal-install-3.6. (Try cabal get BNFC-2.8.3 && cd BNFC-2.8.3 && cabal sdist). I also tried with cabal-install-2.0 (released august 2017) and no dist/ in there. BNFC-2.8.3 is uploaded august 2019.

You can also upload BNFC-2.8.3.1 without dist directory, which would fix an issue for all, and that should work (try with candidate, installing from tarball url should work similarly to installing package from Hackage).

And in BNFC-2.9.3 there is Lex.x and Lex.hs, and that is unfortunate. That's why I suggested that a cabal check is added, so such packages couldn't be uploaded to Hackage in the future. Why you have Lex.hs in your source tree? next to Lex.x?

fgaz commented 2 years ago

Previous discussion about such a warning here and in later comments: https://github.com/haskell/cabal/issues/7251#issuecomment-763705185

phadej commented 2 years ago

And https://github.com/haskell/cabal/issues/7258 seems relevant. Even reported by @andreasabel, I still wonder why Andreas is polluting his source tree by running alex there, but I do agree that cabal check shouldn't succeed in that case. And I think we can make cabal sdist fail too, if it's going to include (haskell | preprocess) files which only differ in extension. (Different folders is harder, as these can be guarded by flags, but there could be ambiguity that way too).

andreasabel commented 2 years ago

@phadej wrote:

it's not added by cabal-install-3.6. (...). I also tried with cabal-install-2.0 (released august 2017) and no dist/ in there. BNFC-2.8.3 is uploaded august 2019.

Indeed. My apologies. I am very puzzled by this, but evidence suggests that it was my own former self that tampered with the tarball before uploading it to hackage. I have no recollection of this, but it could have been that I wanted to be "on the safe side" and have the generated parser source included in case alex/happy were not available to the installer or would generate code with a different interface as at the time of shipping. Maybe I took alex as an inspiration, see https://github.com/haskell-infra/hackage-trustees/issues/321#issue-1051382788: alex-3.2.1 also ships dist/ with pre-build scanner and parser.

It seems now that in some cases cabal does use the pre-build code (remote install) and in some cases it does not (local install). Thinking about it, this is maybe even working as intended.

Only that I have, ironically, become a victim of my own precautions.

You can also upload BNFC-2.8.3.1 without dist directory, which would fix an issue for all,

Excellent suggestion!

IMHO, cabal check should warn:

* when there is `dist` directory in the package

Ok, this would prevent boot-strapping tricks like used by alex-3.2.1, but maybe they are obsolete now?

In my case, cabal check would not have seen the dist/ directory as I likely added it manually later. But cabal upload could complain.

Why you have Lex.hs in your source tree? next to Lex.x?

To be able to load my code into ghci from a simple emacs haskell-mode.