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

Using cabal install in a project or install/repl without a project, duplicates global config and rebuilds libs twice in the store #6906

Closed
vituscze opened this issue Jun 13, 2020 · 17 comments · Fixed by #7753

Comments

@vituscze
Copy link
Contributor

To Reproduce
Make sure the global Cabal config has something that can be duplicated, for example ghc-options or extra-include-dirs. Then:

$ cabal v2-install tardis --lib
$ cabal get tardis
$ cd tardis-0.4.1.0
$ cabal v2-build

The mmorph package is built twice and the store now contains two mmorph directories. cabal-hash.txt of the first one:

extra-include-dirs: C:\tools\msys64\mingw64\include C:\tools\msys64\mingw64\include

cabal-hash.txt of the second one:

extra-include-dirs: C:\tools\msys64\mingw64\include

Same thing happens with cabal v2-repl --build-depends tardis.

Expected behavior
The mmorph package should be built just once and its cabal-hash.txt should contain just:

extra-include-dirs: C:\tools\msys64\mingw64\include

System information

  • cabal 3.2.0.0 (also tested with head), GHC 8.10.1, Windows 10
@vituscze
Copy link
Contributor Author

vituscze commented Jun 13, 2020

I believe the duplication happens here and here. withoutProject in installAction returns globalConfig <> cliConfig and then installAction runs establishDummyProjectBaseContext which adds another copy of globalConfig on top.

runAction and replAction have a similar problem.

@vituscze
Copy link
Contributor Author

cabal v2-install also does this inside a project directory, cabal v2-repl does not.

@treblacy
Copy link

treblacy commented Oct 3, 2020

Table summarizing the situation:

command in project/package outside
repl no dup dup
build no dup n/a
run no dup dup
install dup dup

"run" "outside" means, e.g., cabal run m.hs where m.hs a cabal script:

{- cabal:
build-depends: base, acme-dont
-}

main = putStrLn "hello"

@phadej
Copy link
Collaborator

phadej commented Oct 3, 2020

I have a hunch why this happens, I have yet no idea how hard this is to fix.

It might be related to how we build ProjectConfig "twice" in the scenarios mentioned in @treblacy table (thanks, that is great summary). Fixing #6977 should fix that, if we are careful to construct ProjectConfig only once, without it is tricky.

Relatedly, with cabal repl -b extra-package with cabal-install-3.2 I expect global config to be tripled. Related to investigation of #7081

@jneira jneira changed the title Using cabal install/repl without a project duplicates global config Using cabal install in a project or repl without a project, duplicates global config and rebuilding libs twice Oct 14, 2021
@jneira jneira changed the title Using cabal install in a project or repl without a project, duplicates global config and rebuilding libs twice Using cabal install in a project or install/repl without a project, duplicates global config and rebuilding libs twice Oct 14, 2021
@jneira jneira changed the title Using cabal install in a project or install/repl without a project, duplicates global config and rebuilding libs twice Using cabal install in a project or install/repl without a project, duplicates global config and rebuilds libs twice in the store Oct 14, 2021
@Mikolaj
Copy link
Member

Mikolaj commented Oct 14, 2021

Citing from #7745: "This seems like a very juicy and (let's hope) relatively low hanging fruit. Even just time saved diagnosing offshoots of this problems again and again is significant and could be used elsewhere."

@jneira
Copy link
Member

jneira commented Oct 14, 2021

@phadej suggestion was

It might be related to how we build ProjectConfig "twice" in the scenarios mentioned in @treblacy table (thanks, that is great summary). Fixing #6977 should fix that, if we are careful to construct ProjectConfig only once, without it is tricky.

Fixing #6977 (remove the need for fake-package) seems to be a deep change: what about fix duplicate entries just in the input info used to compute the package hash and avoid rebuilds without waiting for the proper fix?

I mean, a quick and dirty nub over those fields just where the package hash input is created.

In the meanwhile a possible workaround (i have to check it) to avoid rebuilds could be:

  • remove extra-include-dirs and extra-lib-dirs from the global config
  • add the equivalent config per project actually needing them, adding in cabal.project:
package *
  extra-include-dirs: ...
  extra-lib-dirs: ....
  • for no project workflows use the equivalent cli options -extra-include-dirs and --extra-lib-dirs

@jneira
Copy link
Member

jneira commented Oct 14, 2021

add the equivalent config per project actually needing them, adding in cabal.project:

i checked out that config in cabal.project does not trigger a rebuild

for no project workflows use the equivalent cli options -extra-include-dirs and --extra-lib-dirs

i am afraid that woud not work in the general case due to #2997 (cli options are only applied to local packages and no to dependencies)

@mouse07410 that workaround would be too cumbersome for your workflow?

@jneira
Copy link
Member

jneira commented Oct 14, 2021

Why those fields:

packageConfigExtraLibDirs :: [FilePath],
packageConfigExtraLibDirsStatic :: [FilePath],
packageConfigExtraFrameworkDirs :: [FilePath],
packageConfigExtraIncludeDirs :: [FilePath],

are not NubList Filepath instead [Filepath] (like packageConfigProgramPathExtra)?

Added in 92f018c by @dcoutts 5 years sgo 😅

I guess is hard to discern filepath equality without knowing for sure they are not fully normalized in a os independent way, but that did not hold for packageConfigProgramPathExtra 🤔
#6667 would make it easier. It does not talk about normalized paths but we are using them in lsp for example:

https://github.com/haskell/lsp/blob/ab1aec745268afadcbfe4404d61e230e4c188e98/lsp-types/src/Language/LSP/Types/Uri.hs#L154-L161

@Mikolaj
Copy link
Member

Mikolaj commented Oct 14, 2021

That is indeed a good question, because there are a couple of NubList in that commit. Perhaps it didn't matter at that time? Or just oversight?

@phadej, @fgaz, @gbaz, any ideas/recollections?

@jneira
Copy link
Member

jneira commented Oct 14, 2021

Well i guess it was known at that moment that packageConfigProgramPathExtra entries were fully normalized but there were not enough info about the other ones.
And adding possible duplicates did not seem too troublesome at that moment as cabal itself does not much work with them but pass them to gcc, ghc etc (and the perfomance was not visibly degraded neither)

But even remove duplicate entries on pure String equality seems to be an improvement, no?

@mouse07410
Copy link
Collaborator

@mouse07410 that workaround would be too cumbersome for your workflow?

If you mean - move the --extra-lib-dirs and such from ~/.cabal/config to every project's cabal.project, then I'd say yes - it's too cumbersome.

BTW, is this problem of rebuilding twice and installing the same package twice with two hashes (#6906) in any way related with the failure of cabal list-bin to locate the compiled binaries (#7679)?

@jneira
Copy link
Member

jneira commented Oct 14, 2021

@mouse07410 that workaround would be too cumbersome for your workflow?

If you mean - move the --extra-lib-dirs and such from ~/.cabal/config to every project's cabal.project, then I'd say yes - it's too cumbersome.

Well, to every package needing them for real, in my case I dont have many projects using c libraries f.e.
But it is not a easy workaround tbh

BTW, is this problem of rebuilding twice and installing the same package twice with two hashes (#6906) in any way related with the failure of cabal list-bin to locate the compiled binaries (#7679)?

hmm I would say it is not related at all but I did not investigate that bug

@mouse07410
Copy link
Collaborator

Well, to every package needing them for real, in my case I dont have many projects using c libraries f.e.
But it is not a easy workaround tbh

One reason for those extra dirs is Macports-installed OpenSSL, because MacOS comes with LibReSSL that one cannot link applications against. I may have an idea which of my projects may need OpenSSL (much less so wrt. dependencies pulled from Hackage) - but it's pain in the neck to track. Majority of what I'm doing involves some form of crypto, and crypto software. So, likely to be hit here.

Another reason - libiconv. There are two incompatible versions of it - one provided by MacOS itself, and that's what GHC et co. are all linked against. The other one is Macports-provided, and all the non-OS apps link against it. Here I've no way of knowing whether an app - either written my me or (likelier) pulled from Hackage - would want to link against libiconv. Tracking this - almost impossible. That means, I'd need to add this "workaround" literally to every package. No, thanks!

@jneira
Copy link
Member

jneira commented Oct 14, 2021

Why those fields:\n\nhttps://github.com/haskell/cabal/blob/c66a1260f86c9c2f16dc319604f766fd8f756045/cabal-install/src/Distribution/Client/ProjectConfig/Types.hs#L260-L263\n\nare not NubList Filepath instead [Filepath] (like packageConfigProgramPathExtra)?

well I have a theory, the order of include (-i, -I) and lib dirs (-l,-L) is significant so [dir1, dir2, dir1] could have diff behaviour than [dir2, dir1] or even [dir1, dir2]

It seems gcc scans the option from left to right so removing from the end of the list should not change the behaviour. If all possible c compilers follow that convention maybe we could do that, but are we sure about that?

https://stackoverflow.com/questions/41437648/specifying-order-in-gcc-and-g-include-and-lib-paths

@Mikolaj
Copy link
Member

Mikolaj commented Oct 14, 2021

well I have a theory

That's a sick theory, but sadly it's possible.

Which C compilers are possible for us? GCC for sure. Anything else? What's used with LLVM? We use GCC only for linking or also for something else?

@mouse07410
Copy link
Collaborator

Which C compilers are possible for us? GCC for sure. Anything else? What's used with LLVM?

MacOS must use Clang (and that's what comes with Xcode - Apple-provided compiler and IDE), which is LLVM-based (aka, used with LLVM).

@gbaz
Copy link
Collaborator

gbaz commented Oct 14, 2021

The correct thing to do is to not load cabal.config multiple times. I'm sure there's going to be a straightforward way to do so, with some care.

jneira added a commit to jneira/cabal that referenced this issue Oct 15, 2021
jneira added a commit to jneira/cabal that referenced this issue Oct 16, 2021
mergify bot pushed a commit that referenced this issue Oct 18, 2021
…#7759)

* Debug package hash inputs

* Add test skeleton for #6906

* Improve output of hash inputs

* Add tests over pks hash inputs

* Correct typos and formatting

* Add missing do

* Add missing cabal result arg

* Make consistent log about hash.txt

* Update log msg creating file

* Change var name to hashFileName
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.

7 participants