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

hackage-db: support cabal-install 3.10.1.0 XDG paths. #597

Merged
merged 4 commits into from
Apr 10, 2023

Conversation

athas
Copy link
Contributor

@athas athas commented Mar 10, 2023

From 3.10.1.0 onwards, cabal-install uses XDG directories to store its state and cache components. cabal2nix expects to find the Hackage tarball in ~/.cabal, where it may no longer be located.

This commit changes cabal2nix to look in ~/.cabal if that directory exists (which matches the behaviour of cabal-install itself), but otherwise looks in the appropriate XDG directory.

@athas athas changed the title cabal2nix: support cabal-install 3.10.1.0 XDG paths. hackage-db: support cabal-install 3.10.1.0 XDG paths. Mar 10, 2023
@sternenseemann
Copy link
Member

Thanks! Can you also link to the upstream code that implements this in the comment for reference?

This also shows that we should respect the CABAL_DIR environment variable, but we can also do that in a separate change, as we weren't even respecting it in the first place.

From 3.10.1.0 onwards, cabal-install uses XDG directories to store its
state and cache components.  cabal2nix expects to find the Hackage
tarball in ~/.cabal, where it may no longer be located.

This commit changes cabal2nix to look in ~/.cabal if that directory
exists (which matches the behaviour of cabal-install itself), but
otherwise looks in the appropriate XDG directory.
@athas
Copy link
Contributor Author

athas commented Mar 13, 2023

Link added.

The Right Solution is perhaps to depend on cabal to use exactly the same logic. However, cabal2nix got away with not doing exactly what cabal does so far (in particular, not respecting CABAL_CONFIG or CABAL_DIR), so it probably does not matter in practice.

@sternenseemann
Copy link
Member

Seems like cabal-install has made our lives harder than it should be – namely you can also configure arbitrary location for the package db location. What's worse this is apparently rendered into the configuration file by default, so even if people move their config to xdg paths, it may still show up under ~/.cabal.

Not quite sure yet how to proceed – we'll want to avoid depending on cabal-install I think, since it is quite the dependency. One nice thing is that we support a wide range of Cabal libraries, but supporting a wide range of cabal-install is likely very painful.

@athas
Copy link
Contributor Author

athas commented Mar 24, 2023

The issue with paths being stored in the cabal config file is not new. It's been like that for a while - maybe forever. This PR does not make the situation worse.

@sternenseemann
Copy link
Member

True, but it'll be more likely to be a problem now. In any case that we'll have to prefer ~/.cabal anyways will get us out of 90% of the issues hopefully.

For anything else I'll implement CABAL_DIR and people will have to work around it manually.

Just like cabal-install, if CABAL_DIR is set, use that for all
operations. The environment variable does of course not support any
split between $XDG_CONFIG_HOME/cabal and $XDG_CACHE_HOME/cabal which
seems like an oversight on upstream's part, but we don't need to care:
we only ever care about the state dir, so users can just need to set
that.
Since nixpkgs unstable now ships cabal-install 3.10.1.0 (23.05 will as
well), we should get this feature released soon.
Copy link
Member

@maralorn maralorn left a comment

Choose a reason for hiding this comment

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

Excellent! I left to small comments, but can also be merged as is.

-- resides at @"$HOME\/.cabal\/packages\/hackage.haskell.org\/00-index.tar"@.
-- Running the command @"cabal update"@ will keep that file up-to-date.

-- resides in @$HOME\/.cabal\/packages\/hackage.haskell.org\/@.
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to adapt this path to the new default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's necessary.

Comment on lines +45 to +48
Nothing ->
if dotCabalExists
then pure dotCabal
else getXdgDirectory XdgCache "cabal"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Nothing ->
if dotCabalExists
then pure dotCabal
else getXdgDirectory XdgCache "cabal"
Nothing | dotCabalExists -> pure dotCabal
Nothing -> getXdgDirectory XdgCache "cabal"

Really unimportant. Just suggesting it to learn whether you also like it better and if not, why not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was copy-pasted from the cabal source code which doesn't seem to use guards much.

Copy link
Member

Choose a reason for hiding this comment

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

I think having the control flow more explicit is more readable here (at least to me) and it also makes the diff easier to discern. If you'd want to make the code nice, you'd probably use Alternative or something, but can't be bothered, really :)

@sternenseemann sternenseemann merged commit 690a283 into NixOS:master Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants