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.

Sign up for GitHub

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

Jump to bottom

Backports misc. fixes for the new ofborg eval time checks #48692

Merged
merged 2 commits into from Oct 20, 2018
Merged

Backports misc. fixes for the new ofborg eval time checks #48692

merged 2 commits into from Oct 20, 2018

Conversation

samueldr
Copy link
Member

Motivation for this change

The new ofborg check (named grahamcofborg-eval-package-list-no-aliases) currently fails on release-18.09.

image

This PR aims to correct this.

Things done
  • ✔️ Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • ✔️ NixOS
    • ⬜ macOS
    • ⬜ other Linux distributions
  • ✔️ time env -i nix-env --query --available --json --file . --arg config '{ allowAliases = false; }'
  • ✔️ Fits CONTRIBUTING.md.

cc @ryantm

in f91a79e there were changes to pkgs/misc/vim-plugins/default.nix which didn't seem to be needed. I have not backported those changes. Any explanations to help figure out whether they should have been needed?

@ryantm @samueldr
treewide: fix allowAliases = false evaluation problems
b02c65a 
(cherry picked from commit 856fbc8)

 Conflicts:
	pkgs/os-specific/linux/roccat-tools/default.nix

File isn't in release-18.09
@ryantm @samueldr
treewide: fix config.allowAliases = false evaluation
9dc7c31 
(cherry picked from commit f91a79e)

 Conflicts:
	pkgs/misc/vim-plugins/default.nix

That file has NOT been touched in this cherry pick.
@samueldr samueldr requested a review from ryantm October 19, 2018 01:18
@samueldr
Copy link
Member Author

samueldr commented Oct 19, 2018 •
edited

🎉 it eval'd!

Since the rebuild count is 0 (zero) this should be fine, a zero-sum change.

@samueldr samueldr mentioned this pull request Oct 19, 2018
Merged
9 tasks
@ryantm
Copy link
Member

ryantm commented Oct 19, 2018

The vim changes are only necessary if you want to do some evaluation inside of vimPackages with allowAliases = false set. So it's probably less of a priority to backport that, but still relevant if you want allowAliases to always work. I believe we did the changes in a way that there isn't any incompatibility, except if you use aliases inside of the vimPlugins expressions in nixpkgs it will throw an error.

Mic92
Mic92 reviewed Oct 19, 2018
View reviewed changes
pkgs/top-level/all-packages.nix

terraform-provider-ibm = callPackage ../applications/networking/cluster/terraform-provider-ibm {};


Copy link
Member

Choose a reason for hiding this comment

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

Where did those go?

Copy link
Member Author

@samueldr samueldr Oct 19, 2018 •
edited

Choose a reason for hiding this comment

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

Hmm, overlooked when asking @ryantm

This is verbatim from the cherry-picked commit.

They are in terraform-providers as ibm and libvirt; both already had an alias in aliases.nix as of release-18.09 right now.

@samueldr
Copy link
Member Author

This is merged without the equivalent change to the vim-plugins as that change is not required for fixing ofborg evals.

If the ofborg checks are updated to dig-in deeply in the attribute sets, then I guess it'll have to be ported, but in all cases, it's not a matter of backporting, there is a merge conflict.

@samueldr samueldr merged commit 35421cf into NixOS:release-18.09 Oct 20, 2018
@samueldr samueldr deleted the fix/disallow-aliases branch February 12, 2019 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@ryantm ryantm Awaiting requested review from ryantm

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@samueldr @ryantm @Mic92 @GrahamcOfBorg