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

Nested attribute merging may or may not work as intended #11268

Closed
rhendric opened this issue Aug 8, 2024 · 4 comments · Fixed by #11294
Closed

Nested attribute merging may or may not work as intended #11268

rhendric opened this issue Aug 8, 2024 · 4 comments · Fixed by #11294
Labels
bug language The Nix expression language; parser, interpreter, primops, evaluation, etc

Comments

@rhendric
Copy link
Member

rhendric commented Aug 8, 2024

Describe the bug

I would like to be able to say the following:

  • A binding to an attribute path a.b.c = 1; is equivalent to a = { b = { c = 1; }; };
  • Bindings with the same name can be combined if both values are attribute set literals (modulo the rec keyword, maybe)

These two simple rules would explain both the common

{ a.b = 1; a.c = 2; }
# => { a = { b = 1; c = 2; }; }

and the less-common

{ a = { b = 1; }; a = { c = 2; }; }
# => { a = { b = 1; c = 2; }; }

without appealing to additional rules.

This unfortunately fails to hold for deeper attribute paths:

{ a.b.c = 1; a.b.d = 2; } # is okay
{
  a = { b = { c = 1; }; };
  a = { b = { d = 2; }; };
}
#error: attribute 'b' already defined at «string»:3:9
#       at «string»:2:9:
#            1| {
#            2|   a = { b = { c = 1; }; };
#             |         ^
#            3|   a = { b = { d = 2; }; };

I don't know if this is a bug (if so, it should be fixable without any breaking changes) or if this is an as-intended aspect of the language that I haven't found a succinct way to describe yet.

Other remarkable comparisons:

{ a.b = { c = 1; }; a.b = { d = 2; }; } # works
{ a = { b.c = 1; }; a = { b.d = 2; }; } # doesn't
{ a = { b = { c = 1; }; }; a.b.d = 2; } # works
{ a.b.c = 1; a = { b = { d = 2; }; }; } # doesn't

Steps To Reproduce

Try to evaluate

{
  a = { b = { c = 1; }; };
  a = { b = { d = 2; }; };
}

Expected behavior

Same as { a.b.c = 1; a.b.d = 2; }.

nix-env --version output

nix-env (Nix) 2.24.0

Additional context

Knowing whether this is a bug or not is a minor blocker for me writing up a specification of how bindings work as part of #10970. It's fine if it's a bug that hasn't been fixed yet; that can be noted in the documentation. It's also fine if this is how things are supposed to work; I can write the more complicated specification.

Priorities

Add 👍 to issues you find important.

@rhendric rhendric added the bug label Aug 8, 2024
@roberth roberth added the language The Nix expression language; parser, interpreter, primops, evaluation, etc label Aug 11, 2024
@roberth roberth added this to Nix team Aug 11, 2024
@github-project-automation github-project-automation bot moved this to To triage in Nix team Aug 11, 2024
@roberth roberth removed this from Nix team Aug 12, 2024
@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-08-12-nix-team-meeting-minutes-168/50561/1

@roberth
Copy link
Member

roberth commented Aug 13, 2024

Discussed during aforementioned Nix maintainers meeting:

The code that handles these syntax combinations is fairly complex and would probably need to be refactored in order to support this properly.
Most of the complexity comes from the combinations with dynamic attributes (${name} = v;)

If supporting this properly proves infeasible, we might deprecate some variations, but not remove it, because they've been supported for around 2 years.

I don't know if this is a bug (if so, it should be fixable without any breaking changes) or if this is an as-intended aspect of the language

These restrictions are not intentional. It used to be that explicit attrsets in repeated attrs could not be merged at all. ~2 years ago we've started supporting that, but apparently the support is not complete.

Perhaps you could document it, but add a caveat that not all combinations are currently supported.

Would you be interested in improving support for these cases?

@rhendric
Copy link
Member Author

Would you be interested in improving support for these cases?

I'm interested in taking a stab at it, if the refactoring doesn't prove to be too difficult!

@rhendric
Copy link
Member Author

rhendric commented Aug 14, 2024

WRT #9020, I assume that you'll want me to keep monstrosities like this working?

nix-repl> { a = rec { b = 1; }; a.c = b + 1; }.a.c        
2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants