-
Notifications
You must be signed in to change notification settings - Fork 697
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
cabal new-install allows linking against multiple versions of the same package when internal libraries are used #5782
Comments
Would it be possible to come up with a small repro case? |
I had something similar caused by executable depending on an internal library from its package and the whole (anonymous) library of this package as well (the internal library depended on the whole library as well). In the result, the build used two versions of the whole library (I suppose, it took the whole library already installed previously and then probably compiled the internal library again). Note the double
If there is interest, I can create a branch to repro, but I doubt I could come up with a small example. |
A repro case would surely be nice to have. |
I'm not sure if the errors I'm getting are because I've broken my cabal DB and I don't want to rebuild all the packages to check, but I guess these are the steps to reproduce:
If it still doesn't fail, run Edit: yes, after the first
|
I wasn't able to reproduce the multiple versions error with LambdaHack. I tried running the first set of commands in #5782 (comment) with both cabal-install-2.4.1.0 and HEAD (27cf519 and 6316102), with GHC 8.6.4 and Cabal listed five LambdaHack components to be built (main library, three internal libraries, and an executable), which seems correct:
Here is the end of the log, which corresponds to the log in the previous comment. I noticed that the components were built in a different order, with
There was one directory for each LambdaHack component in the store afterwards. Which version of cabal caused the error? Were any other packages installed? Do you know what index state was used or have a full log? |
@grayjay: thank you for trying to repro. That was cabal version 3.0.0.0, somewhat older than my bug report. I can't tell the exact date. All deps were already installed. Can you try re-installing? I've just tried again with cabal from 6316102 and here's the full log: https://gist.github.com/Mikolaj/5661f4e69635e5c4f3c040ef211b1d68. Note how "indirectly depends on multiple" only appears when I run cabal again. Let me know if I can help any more. |
@Mikolaj Thanks for the log. I tried installing the dependencies separately, as well as making code changes to trigger a partial rebuild, but all of the commands succeeded. I noticed that the first new-install in the log failed with some profiling errors:
How is profiling enabled? I tried installing LambdaHack with |
Profiling is enabled through my ~/.cabal/config file: https://gist.github.com/Mikolaj/da6494a16d6f121eefad1930394de699 |
I do remember meddling with cabal package DB before originally posting about the error. Some |
I'm now even getting this:
which looks like some file corruption to me. |
@Mikolaj I don't see any |
Well, yes, the instance is here: but the anonymous library depends on the so it should be included. Edit: Which is why it compiles fine most of the time. |
Oh, I see.
yet
if it's reproducible, at least on your machine. Could you upload here the |
@phadej, @grayjay: Yes, it's reproducible, but only once. Then, on a second run (even after
it will succeed. Then set
The ending of the log:
|
Thanks! I was able to reproduce the issue by enabling profiling in ~/.cabal/config and rebuilding. I'll look into it further when I have time. |
I think I have a minimal reproduction for the issue, which doesn't involve profiling:
cabal.project: packages: ./ README.md:
issue5782.cabal: cabal-version: 2.2
name: issue5782
version: 0.1
build-type: Simple
data-files: README.md
library a
hs-source-dirs: src
exposed-modules: Module
build-depends: base
default-language: Haskell2010
library
hs-source-dirs: src
build-depends: a, base
default-language: Haskell2010
library b
hs-source-dirs: src
build-depends: a, base
default-language: Haskell2010
library c
hs-source-dirs: src
build-depends: issue5782, b, base
default-language: Haskell2010 src/Module.hs: module Module where
f = 0
Output from the two install commands:
Removing any of the four components removes the "multiple versions" warning, presumably because it breaks the diamond shape in the dependencies, but I didn't yet test whether cabal still uses the old version of one of the components. |
After testing by importing a definition directly and indirectly through packages that produce the warning, it seems that the warning is a false positive - at least in projects that don't use backpack features (where the warning comes from). I've also noticed that the The question then becomes: is the code that checks before outputting the warning correct for some cases when backpack features are used, or is it simply overzealous in general? A number of |
Weakly related: #7311 (and the GHC issue mentioned in there). |
I can definitely believe there is a bug here in GHC after reading the code but I can't reproduce using @Mikolaj's example. |
@mpickering I think it's |
BTW, the wonderful @grayjay 's repro no longer works with cabal 3.5 and GHC 9.0.1 (and probably many older setups). I've played with it and the following two look buggy and similar [edit: but don't repro the original problem]. To be run with something like test2.couldNotLoad.tar.gz Edit: actually the two tests only trigger wrong "Perhaps you need to add" hints (one that hints to add a dep that is already there and another that hints to add a dep that does not help at all) and not any other problem, the original bug in particular. I was also wrong about implicit re-exports, so I removed the remark. Edit2; reported at https://gitlab.haskell.org/ghc/ghc/-/issues/19777 |
My failure to reproduce @grayjay's example with cabal 3.5 and GHC 9.0.1 leads to me to believe that at least the implicit reexporting of internal libraries a library depends on has been fixed and, perhaps, the original bug has been fixed as a side-effect. I will try to reproduce using my own much bigger reproducer from earlier comments. Here is the closest rendering of @grayjay's example .cabal file that doesn't fail for unrelated reasons and that, now, compiles fine and then recompiles (without the offending warning) fine after a change (the other files are as in the archives I attached earlier, the command is
Edit: and the following slightly changed variant, which should probably pass, fails with
which may or may not be related.
|
I've performed my original consistent repro, LambdaHack with profiling, and with cabal 3.5 and GHC 9.0.1 there's neither the warning nor the error. It's quite possible the bug has been fixed. @vmchale, everybody, did you see such problems lately? If not, can we close this issue (and a dozen related ones)? |
Nobody reports that this issue is reproducible with current stable cabal or newer, so, if @emilypi, @fgaz and others don't object, I'm going to close this soon, together with half a dozen related issues (some of them mentioned in GHC tickets, so the noise is considerable). BTW, thank you everybody for providing repros and comments --- I'm sure they were helpful in keeping the work on the improvements in the area going, even if we eventually missed the point when the problem was finally fixed for good and the issues didn't get closed with proper thanks to all involved. |
I am for closing optimistically issues, but would be great to know what was the cause of the bug, what commits fixed it and what regression tests will ensure it will not arise again. |
@jneira: no, these are reasonable aspirations and an especially good point about adding tests. Sadly, the only reasonably small original repro, by @grayjay, no longer works for unrelated reasons (many things changed since then). We could use my first test from #5782 (comment) (it can be simplified a bit, but I tried to keep it close to the original) but the problem is it has never failed (and I'm not sure it would be accepted in cabal version old enough to trigger the original error --- could you perhaps try to iterate through versions and find one where it fails for the right reason?). Generally, this and the related issues were hard to repro, where many initial repros non-deterministic, etc., so it's a lot of extra work especially after such a long time. I'm afraid we don't have the capacity ATM, unless somebody helps. Volunteers? |
Yeah this is one of the really fun kind of bugs. Maybe if we only would know what pr or commits are the main candidates for having been the ones which have fixed it... |
You are right, this issue may have been more widespread than meets the eye. Therefore, if somebody would volunteer to bisect cabal commits history using my test from #5782 (comment), that would be very welcome. Also, any running any likely reproduction, successfully or not, with an up to date cabal version would help. I will ask on the other issues related to this one [edit: will limit myself to cabal issues, there's too many in other repos, e.g., changing internal libraries to common stanzas] and wait for a week. |
BTW, Oleg says in #6483 (comment) that possibly the commit fixing #6410 also fixed this set of problems and he confirms he can't repro his original examples either. Perhaps worth testing the commit just before the one that fixes #6410 and then the one, e.g., with my test above? Edit: And the commit in question is backported twice, one time as far back as to v3.0.1.0: 9089b0e which would suggest original v3.0 had the problem, but an updated v.3.0 didn't and v3,2 didn't have the problem from the start. |
Data point: my test compiles fine (two runs, with changed code in Module.hs in between) with cabal 3.3 and GHC 8.6.5. |
Data point 2: but it fails in the following way, matching the problem we are after, with cabal 3.0.2.0 and GHC 8.6.5. Which either means it's not deterministic, after all (but I think these little tests are) or the fix for #6410 did not yet (fully) solve the problem [edit: not so, I mixed up Cabal and cabal-install versions, see next comment].
Edit: yet another explanation might be that some change tweaked the non-determinism so that the problem does not manifest in the small tests, but still manifests in huge ones that involve strange timing of file writes, etc. |
Data point 3: my test passes fine with cabal 3.2 and GHC 8.6.5. BTW, I've double-checked and the fix to #6410 seems to be included in cabal-install 3.0.1.0, which was never released until cabal-install 3.2. I don't see any evidence it was included in Cabal 3.0.2.0, only in Cabal 3.2.0.0 (alpha). So, in the end, this is consistent with Oleg's hunch which commit fixed it. Edit: even though it passes fine and there are no warnings "indirectly depends on multiple versions", the executable gives a wrong result, ignoring the change to the file and the second compilation. So 3.2 probably fixed depending on multiple version, but the single version the executable depends on is the wrong one (the older one). |
Data point 4: cabal 3.4 (with GHC 8.6.5) fixes this for good, both no longer depending on multiple versions of an internal library and actually depending on the last compiled version, not the first compiled version. That's as far as I have capacity to go. If anybody would like to bisect to find the relevant commit(s), please volunteer before or after I close the issue and share here in a comment. The great outcome of @jneira aspirations and this series of tests is that we now know what to include as regression test (and that we need to run the executable and check the result or carefully inspect the install plan --- grepping for a warning is not enough). Any volunteer to add the test to our test suite? It can still be simplified, I'm sure. |
wow, impressive summary of the state of things, many thanks |
Nobody volunteered nor dissuaded me, so I'm adding the test for this issue myself. I can't find a similar test. All have too few internal libraries to form a diamond. Directory |
Add regression test for #5782, which requires extending test harness to v2-install
We have the regression test in place and it fails with cabal 3.2 (and even more often with 3.0) and works with 3.4, Closing. Thank you all. |
Due to the
vector
package upgrade, the following happened:I believe that this is due to the fact that
quaalude
(an internal prelude forats-pkg
) depends ondhall
. As the build report above attests,quaalude
was not rebuilt in light of thevector
upgrade; I'm guessing thatcabal
tried to link the old cachedquaalude
against the newdhall
.The text was updated successfully, but these errors were encountered: