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

Include the compiler ABI tag in nix-style package hashes #9325

Closed
wants to merge 1 commit into from

Conversation

sol
Copy link
Member

@sol sol commented Oct 12, 2023

  • This allows you to use several API incompatible builds of the same version of GHC without corrupting the cabal store
  • This relies on "Project Unit Id" which is available since GHC 9.8.1, older versions of GHC will not benefit from this change
  • I set compiler-abi-tag to unknown for pre-9.8.1 GHSs. This invalidates store entries that have been populated by previous versions of cabal. Another option would be to leave out compiler-abi-tag for pre-9.8.1 GHSs.

@sol sol marked this pull request as ready for review October 12, 2023 08:15
@sol sol requested a review from hasufell October 12, 2023 08:56
@hasufell hasufell requested a review from bgamari October 12, 2023 09:04
compilerId = CompilerId GHC ghcVersion

compilerAbiTag :: AbiTag
compilerAbiTag = maybe NoAbiTag AbiTag (Map.lookup "Project Unit Id" ghcInfoMap >>= stripPrefix (prettyShow compilerId <> "-"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bgamari is "Project Unit Id" the proper ABI identifier?

I see ghc-9.8.1-f7d8 in mine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me understand. GHC does not actually offer anything called ABI tag/identifier/hash? We are going to use this Project Unit Id? Is this the right thing to do?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Project Unit Id is indeed a reasonable thing to use here. Indeed it was specifically added to avoid mixing compilation artifacts from different compilers.

@hasufell
Copy link
Member

My question is: why don't we separate the directories in .cabal/store/ by the ABI hash instead of or in addition to adding it to the nix store? I want to be able to delete all the artifacts for a specific GHC bindist (which is essentially what ABI means for GHC).

@sol
Copy link
Member Author

sol commented Oct 12, 2023

My question is: why don't we separate the directories in .cabal/store/ by the ABI hash instead of or in addition to adding it to the nix store? I want to be able to delete all the artifacts for a specific GHC bindist (which is essentially what ABI means for GHC).

Yeah, I think that makes sense. Initially, I thought this would require to change the CompilerId type and would get somewhat involved. However, #9326 doesn't look so bad.

@andreabedini
Copy link
Collaborator

I am leaving comments on both PRs :)

I set compiler-abi-tag to unknown for pre-9.8.1 GHSs. This invalidates store entries that have been populated by previous versions of cabal. Another option would be to leave out compiler-abi-tag for pre-9.8.1 GHSs.

I suggest we follow this second option: it does not invalidate previous cache (not that big deal but the least we change the better), "unknown" in the path does not communicate anything to the user (they won't know what it "unknown").

@andreabedini
Copy link
Collaborator

andreabedini commented Oct 16, 2023

Copying here my comment in the other PR:

Edit: On a second thought. I think I'd prefer adding the platform to the path, just like we do with dist-newstyle (dist-newstyle/build/x86_64-linux/ghc-9.4.7/....), and adding the ghc abi hash to cabal hash, rather that to the store path. This has a simple structure and a clear meaning to the user.

A power user can still delete all cached entries from a particular GHC by grepping for the GHC ABI tag. cabal-install could also provide some store-management commands to help.

See other thread.

@sol
Copy link
Member Author

sol commented Oct 25, 2023

Closing in favor of #9326.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants