cabal2nix icon indicating copy to clipboard operation
cabal2nix copied to clipboard

How to deal with packages who's name is not a valid Nix identifier?

Open peti opened this issue 9 years ago • 21 comments

  • The Haskell package assert conflicts with the reserved keyword in Nix.

  • The Haskell package type conflicts with the internal attribute of the same name used in Nix derivations.

  • Packages like 4Blocks cannot be passed as arguments to other builds because 4Blocks is not a valid identifier. In other words, packages that depend on 4Blocks and friends cannot be built.

peti avatar Apr 16 '15 10:04 peti

I'd like to be able to have quoted attribute names in function bindings. But without that, probably the only way to do this is to do args@{ ... } and explicitly pass in 4Blocks to everything that references it :unamused:

shlevy avatar Apr 16 '15 10:04 shlevy

What about munging the package names? Say 4Blocks becomes x-4Blocks (and x- becomes x2-, x3- x4-, and so on, although hopefully hackage will never have any packages starting with x-). Then add an extra pass at the end to assign the proper attributes their proper values.

Mathnerd314 avatar Aug 18 '15 01:08 Mathnerd314

@edolstra any objection to { "4Blocks" }@args: args."4Blocks" being valid nix?

shlevy avatar Aug 18 '15 12:08 shlevy

I'm not really opposed to it being valid, but I don't think it's a good idea to use it in Nixpkgs because it's pretty ugly. Munging the package name (like _915resolution) is also ugly, but doesn't require special syntax.

Another reason not to do this: the first name component that starts with a digit signifies the start of the version. So "4blocks" might be interpreted as a package with an empty name and a version of "4blocks".

edolstra avatar Aug 18 '15 12:08 edolstra

Any updates or any workarounds for this issue? I need to compile a package that depends on assert, and I'm not sure how to proceed.

bandali0 avatar Jul 23 '17 19:07 bandali0

@edolstra wrote:

[…] I don't think it's a good idea to use it in Nixpkgs because it's pretty ugly.

I don't understand this objection to optional double-quoting of troublesome names, on the basis of aesthetics. The alternative is that you have a package manager that's not fit-for-purpose.

liyang avatar Jul 24 '17 03:07 liyang

Some considerations:

  • It's not just aesthetics, it's also confusing. The "4Blocks" in { "4Blocks" } looks like a string, not a variable name.

  • You can't refer to such variables directly, so you need tricks like { ... }@self: ... self."4Blocks".

  • Even if we added support for this now, you wouldn't be able to use it for a while in Nixpkgs for compatibility reasons.

  • Most package managers have naming rules, so to say that having a few rejected packages names makes Nix unfit-for-purpose seems hyperbolic.

edolstra avatar Jul 24 '17 12:07 edolstra

@liyang and @edolstra thanks for chiming in!

While I do agree that the situation is unfortunate, I think having some workaround is better than none. And better late than never, I guess :) For the short term I can copy the stuff I need from assert into our codebase (giving proper credit to @liyang of course) but I don't like to literally duplicate code like that.

As for workarounds, I'd probably pick either adding an underscore (_) to the beginning of package names that are invalid identifiers (e.g. assert becomes _assert, or type becomes _type); or the quoting approach with @. I'm leaning towards the first one, but I'm not a seasoned nix user and I'm just putting the thought out there.

bandali0 avatar Jul 24 '17 14:07 bandali0

Hello @edolstra!

Firstly I must admit that I don't know Nix (the language), nor do I use Nix (the distro), so I have a few questions (and apologies if they're basic/obvious):

The "4Blocks" in { "4Blocks" } looks like a string, not a variable name.

I didn't realise these were ‘variables’. Why must this variable name coincide with the upstream name though, with no room for exceptions?

Most package managers have naming rules, so to say that having a few rejected packages names makes Nix unfit-for-purpose seems hyperbolic.

Sorry, that was purely to get your attention. :)

That said having Hackage packages that are unpackageable under Nix does seem very wrong to me. Is it just a lack of contributor effort to cabal2nix that's preventing us from having a simple consistent workaround for troublesome package names? Earlier you wrote:

Munging the package name (like _915resolution) is also ugly, but doesn't require special syntax.

If someone (not me; maybe @aminb) prepared a PR for cabal2nix to munge troublesomes names—in whatever way you find least objectionable—would you be prepared to merge it?

liyang avatar Jul 25 '17 04:07 liyang

https://github.com/NixOS/nix/issues/1602

domenkozar avatar Nov 22 '17 10:11 domenkozar

Can somebody remind me why we use the

{ package1, package2 }:
...
buildInputs = [
  package1
  package2
];

convention at all as opposed to

{ pkgs }:
...
buildInputs = [
  pkgs.package1
  pkgs.package2
];

?

I understand that the latter would address most of these problems (pkgs."package1" can be used), and it also has less repetition.

nh2 avatar Dec 06 '18 12:12 nh2

The latter approach is unidiomatic in Nixpkgs and virtually no expression in Nixpkgs does that. It's good practice to accept precisely those dependencies that you use and nothing else. Expressions are easier to read that way and they are also easier to modify without accidentally adding new dependencies that weren't there before.

peti avatar Dec 12 '18 20:12 peti

virtually no expression in Nixpkgs does that

You sit on a throne of lies

See for example: https://github.com/NixOS/nixpkgs/blob/0e8e02e686363de53ad80953c265cf11a95304f4/pkgs/misc/emulators/wine/base.nix https://github.com/NixOS/nixpkgs/blob/794c4ec7fb778c06c22e05291d0f7566897d2d93/pkgs/development/web/postman/default.nix https://github.com/NixOS/nixpkgs/blob/f4811789605ee3fb1c069fd6c569aae32cd27d1e/pkgs/development/python-modules/pywebkitgtk/default.nix (and a few other python packages)

This email is Eelco coming up with the current style. Basically he likes the formal parameters. IIRC there was also some microbenchmark that showed that style was slightly faster since the C++ code did the lookup in pkgs once instead of for each attribute, but that might have changed.

Mathnerd314 avatar Dec 13 '18 05:12 Mathnerd314

Can somebody remind me why we use the

{ package1, package2 }:
...
buildInputs = [
  package1
  package2
];

convention at all as opposed to

{ pkgs }:
...
buildInputs = [
  pkgs.package1
  pkgs.package2
];

?

I understand that the latter would address most of these problems (pkgs."package1" can be used), and it also has less repetition.

To me, this to make overriding easy. You can override just one or two inputs instead of the entire set.

ip1981 avatar Oct 03 '19 13:10 ip1981

Still an issue 7 years later...

Fuuzetsu avatar May 20 '22 03:05 Fuuzetsu

Indeed, it is hard (to impossible) to fix if we still want to leverage splicing to facilitate correct cross compilation as it works today.

Are you running into it for anything specific?

sternenseemann avatar May 20 '22 08:05 sternenseemann

assert package; what one can do is modify generated nix file to rename it to something like assert_pkg and ensure it's available in package set under that name

there are presumably multiple ways cabal2nix can work around this but seems in name of perfection, nothing is done instead

Fuuzetsu avatar May 22 '22 02:05 Fuuzetsu

@sternenseemann Can you explain what that means? Why can’t we just define an injective mapping from hackage names to non-keyword nix names and apply that consistently everywhere when generating the nix expressions?

maralorn avatar May 22 '22 08:05 maralorn

but seems in name of perfection, nothing is done instead

I want to repeat my question: Where are you running into this, specifically? I'm a bit surprised by your frustration, since my impression is that it hasn't been implemented so far because the amount of pain it causes is minimal, i.e. the list of packages that have to be removed is quite short:

https://github.com/NixOS/cabal2nix/blob/26939db1c6ea34ed60a289360dac01fccd47a3d2/hackage2nix/Main.hs#L81-L85

Picking up this issue again is possible of course if it's worth it.

Why can’t we just define an injective mapping from hackage names to non-keyword nix names and apply that consistently everywhere when generating the nix expressions?

I think so, this occurred to me as well after thinking about it for a bit. My response was in particular about the alternative approaches presented in this issue (taking everything from the package set) where splicing would be broken: callPackage only attaches the nativeDrv attribute, for example, for every individual derivation that is taken as an explicit argument.

I believe we could, following general nixpkgs policy, use such an escaping logic:

escapeAttrName :: String -> String
escapeAttrName a = if isValidIdentifier a then a else '_':a

unEscapeAttrName :: String -> String
unEscapeAttrName ('_':x) = x
unEscapeAttrName x = x

The fact that cabal package names can never contain underscores does come in handy here.

sternenseemann avatar May 22 '22 11:05 sternenseemann

Honestly, I think we should do this. This is not just a question of weird corner cases, but of correctness. Especially, this could go wrong again at any time when anyone uploads anything funky to hackage so it will at least prevent breakage at an inconvenient time.

maralorn avatar May 22 '22 11:05 maralorn

@sternenseemann Oh, I wasn't clear. We have our own package that directly depends on assert (assert was written by our former co-worker, it may have even been written while he was with the company but this is before my time – long story short, we use it).

Deleting stuff that depends on assert or type is very hacky and only works for hackage2nix, injective mapping sounds a lot better.

Fuuzetsu avatar May 22 '22 23:05 Fuuzetsu