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

oh-my-git: fix strict eval #134281

Merged
merged 1 commit into from
Aug 16, 2021
Merged

oh-my-git: fix strict eval #134281

merged 1 commit into from
Aug 16, 2021

Conversation

r-burns
Copy link
Contributor

@r-burns r-burns commented Aug 16, 2021

libudev is an alias to udev

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

libudev is an alias to udev
@happysalada
Copy link
Contributor

This fails to build with nixpkgs-review with the following

error: anonymous function at /Users/raphael/.cache/nixpkgs-review/pr-134281-1/nixpkgs/pkgs/games/oh-my-git/default.nix:1:1 called without required argument 'libudev'

       at /Users/raphael/.cache/nixpkgs-review/pr-134281-1/nixpkgs/lib/customisation.nix:69:16:

           68|     let
           69|       result = f origArgs;
             |                ^
           70|

       … while evaluating 'makeOverridable'

       at /Users/raphael/.cache/nixpkgs-review/pr-134281-1/nixpkgs/lib/customisation.nix:67:24:

           66|   */
           67|   makeOverridable = f: origArgs:
             |                        ^
           68|     let

       … from call site

       at /Users/raphael/.cache/nixpkgs-review/pr-134281-1/nixpkgs/lib/customisation.nix:121:8:

          120|       auto = builtins.intersectAttrs (lib.functionArgs f) autoArgs;
          121|     in makeOverridable f (auto // args);
             |        ^
          122|

       … while evaluating 'callPackageWith'

       at /Users/raphael/.cache/nixpkgs-review/pr-134281-1/nixpkgs/lib/customisation.nix:117:35:

          116|   */
          117|   callPackageWith = autoArgs: fn: args:
             |                                   ^
          118|     let

       … from call site

       at /Users/raphael/.cache/nixpkgs-review/pr-134281-1/nixpkgs/pkgs/top-level/all-packages.nix:3072:15:

         3071|
         3072|   oh-my-git = callPackage ../games/oh-my-git { };
             |               ^
         3073|
https://github.com/NixOS/nixpkgs/pull/134281 failed to build

I'm checking where this could come from.

@r-burns
Copy link
Contributor Author

r-burns commented Aug 16, 2021

Yeah, you can't run nixpkgs-review when the before/after state fails eval. It needs a successful eval for both to compute the list of changed paths.

@happysalada
Copy link
Contributor

TIL !
Also the only platform supported is x86_64-linux, I wanted to help on that one, but I'm afraid I can't test it.
It looks right though.

@davidak
Copy link
Member

davidak commented Aug 16, 2021

Result of nixpkgs-review pr 134281 run on x86_64-linux 1

@davidak
Copy link
Member

davidak commented Aug 16, 2021

libudev is an alias to udev

what do you want to say? are aliases not allowed anymore? why does the check have an error and nixpkgs-review not? why does this fix the issue?

udev is an alias to sysdemd btw

i will merge asap if i can verify that the change makes sense

@r-burns
Copy link
Contributor Author

r-burns commented Aug 16, 2021

As I understand it, aliases are disallowed in ofborg eval (since NixOS/ofborg#250) and the purpose of the aliases is more for backwards compatibility so that we don't break e.g. users' configs/overlays.

@davidak
Copy link
Member

davidak commented Aug 16, 2021

I see. Then why does this change fix it? udev is an alias as well!

@veprbl veprbl merged commit 58abd73 into NixOS:master Aug 16, 2021
@r-burns r-burns deleted the oh-my-git-eval branch August 16, 2021 03:20
@r-burns
Copy link
Contributor Author

r-burns commented Aug 16, 2021

udev isn't a real "alias", it's just an expression with the same value as systemd.
Only aliases in top-level/aliases.nix have strict eval semantics.

@davidak davidak mentioned this pull request Aug 16, 2021
11 tasks
@Kranzes
Copy link
Member

Kranzes commented Aug 16, 2021

Sorry for the mess guys :(

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

Successfully merging this pull request may close these issues.

None yet

5 participants