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

Allow arbitrary expressions in attribute_path. #4

Merged
merged 3 commits into from
Dec 26, 2017
Merged

Conversation

Fuuzetsu
Copy link
Collaborator

This allows more advanced attributes to be used. Motivating use-case
is ghcWithPackages.

@Fuuzetsu Fuuzetsu requested a review from mboes December 14, 2017 16:03
Copy link
Member

@mboes mboes left a comment

Choose a reason for hiding this comment

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

Tests fail. Needs a unit test.

# example of this is haskellPackages.ghcWithPackages which is
# unusable with -A.
res = ctx.execute(["nix-build", path, "-E",
"(import <nixpkgs> {{}}).{0}".format(attr_path),
Copy link
Member

Choose a reason for hiding this comment

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

Also, if an attribute path is now really an expression, then the name should be changed and it should be documented.

@Fuuzetsu
Copy link
Collaborator Author

[shana@lenalee:~/programming/rules_nixpkgs]$ nix-shell -p bazel
these paths will be fetched (136.46 MiB download, 243.04 MiB unpacked):
  /nix/store/0s51xhmdnpy3kzndind3a9dgbdskw4p1-openjdk-8u152b16
  /nix/store/nlinmdc3fzh5nnh41bj4ydswqndn1cv5-openjdk-8u152b16-jre
  /nix/store/wl9x15jfc56rn4l0xbbb90pdaw56w3ac-bazel-0.8.0
copying path '/nix/store/nlinmdc3fzh5nnh41bj4ydswqndn1cv5-openjdk-8u152b16-jre' from 'http://cache.nixos.org'...
copying path '/nix/store/0s51xhmdnpy3kzndind3a9dgbdskw4p1-openjdk-8u152b16' from 'http://cache.nixos.org'...
copying path '/nix/store/wl9x15jfc56rn4l0xbbb90pdaw56w3ac-bazel-0.8.0' from 'http://cache.nixos.org'...

[nix-shell:~/programming/rules_nixpkgs]$ bazel clean --expunge
INFO: Starting clean (this may take a while). Consider using --expunge_async if the clean takes more than several minutes.

[nix-shell:~/programming/rules_nixpkgs]$ bazel run //tests:run-hello
.........
INFO: Analysed target //tests:run-hello (16 packages loaded).
INFO: Found 1 target...
Target //tests:run-hello up-to-date:
  bazel-bin/tests/run-hello
INFO: Elapsed time: 5.417s, Critical Path: 0.02s
INFO: Build completed successfully, 4 total actions

INFO: Running command line: bazel-bin/tests/run-hello external/hello/nix/bin/hello
Executing:  external/hello/nix/bin/hello
Hello, world!

???

@Fuuzetsu
Copy link
Collaborator Author

I changed the implementation a bit to allow arbitrary expressions and added a couple of tests. I hope they pass now.

@Fuuzetsu
Copy link
Collaborator Author

Hm, I think nix 1.12 is needed for -I to nix-build, not great…

@mboes
Copy link
Member

mboes commented Dec 15, 2017

???

The oracle here is CircleCI. Green on master but red on this branch. Can't merge until this one goes green.

@Fuuzetsu
Copy link
Collaborator Author

Turned out to be dumb mistake, should be green, PATAL.

"""Generate a nix expression that picks a package from nixpkgs.
"""
if ctx.attr.attribute and ctx.attr.expression:
fail("'attribute' and 'expression' are mutually exclusive.")
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the two should be mutually exclusive. -A and -E are not mutually exclusive flags to nix-build. When used together, -E e -A some.path means e.some.path. We should reproduce the same semantics here.

This allows us to be much more flexible when we want to be while
maintaining convenience for simple attribute cases.
@Fuuzetsu
Copy link
Collaborator Author

Addressed. Didn't realise you could combine -E and -A.

We really are passing an attribute *path*, not a Nix attribute. The
Nix documentation defines an attribute path as a dot separated
sequence of attributes.
@mboes mboes merged commit 86eb290 into master Dec 26, 2017
@mboes mboes deleted the allow-expressions branch December 26, 2017 23:18
@mboes
Copy link
Member

mboes commented Dec 27, 2017

Note: for now we're documenting attributes in README, not using inline docstrings.

wolfd pushed a commit to wolfd/rules_nixpkgs that referenced this pull request Sep 15, 2021
Move custom code out of nixpkgs subfolder. Wrap nix_cc with macro to use supports_nix constraint.
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.

2 participants