Skip to content

Comments

splice.nix: add convenience functions #207652

Merged
Artturin merged 3 commits intoNixOS:masterfrom
Artturin:splicenixfmt
Jan 7, 2023
Merged

splice.nix: add convenience functions #207652
Artturin merged 3 commits intoNixOS:masterfrom
Artturin:splicenixfmt

Conversation

@Artturin
Copy link
Member

@Artturin Artturin commented Dec 25, 2022 •
edited
Loading

Replaces #105374

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Artturin
splice.nix: run nixpkgs-fmt
0a3d351 
the file was a bit hard to read
@github-actions github-actions bot added 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: steam Steam game store/launcher (store.steampowered.com) 6.topic: xfce The Xfce Desktop Environment labels Dec 25, 2022
Artturin
Artturin commented Dec 25, 2022
View reviewed changes
pkgs/top-level/splice.nix Outdated
Copy link
Member Author

@Artturin Artturin Dec 25, 2022

Choose a reason for hiding this comment

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

ideas for a better name?

Copy link
Contributor

@Madouura Madouura Dec 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

topSplices, topProjectedSplices or projectedSplices?
I'm not sure top really needs to be there.
Maybe even topSplicedScope.

Copy link
Member Author

@Artturin Artturin Dec 25, 2022

Choose a reason for hiding this comment

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

renamed to generateSplicesForMkScope

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Dec 25, 2022
@Artturin Artturin mentioned this pull request Dec 25, 2022
Closed
10 tasks
FRidh
FRidh reviewed Dec 25, 2022
View reviewed changes
Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

Maybe use a single parameter name for all these steamPackagesAttr, pythonAttr, ... such as selfAttr?

Also, some spliced packages don't expose this attribute. That means it is not possible to add a variant through overriding as the overridden version will use the attribute corresponding to the original variant.

@symphorien
Copy link
Member

symphorien commented Dec 25, 2022

could you document what you add? And if you document makeScopeWithSplicing at the same time it would be wonderful.

@Artturin Artturin force-pushed the splicenixfmt branch 2 times, most recently from afa3edb to 250170b Compare December 25, 2022 12:00
@Artturin Artturin merged commit f56de23 into NixOS:master Jan 7, 2023
@Artturin Artturin deleted the splicenixfmt branch January 7, 2023 15:42
@Ericson2314
Copy link
Member

Ericson2314 commented Apr 30, 2024

I just lost some time because this broke on a dot in an attribute name. Yes, that is a preverse edge case, but splitAttrByPath taking a list is a good thing, and I think we should just do that rather than try to pas a string with dot notation.

@Ericson2314
Copy link
Member

Ericson2314 commented Apr 30, 2024

e7ea78e this commit (part of an not yet merged PR) make it take a list too

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

Reviewers

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

2 more reviewers

@FRidh FRidh FRidh left review comments

@Madouura Madouura Madouura left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: steam Steam game store/launcher (store.steampowered.com) 6.topic: xfce The Xfce Desktop Environment 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Artturin @symphorien @Ericson2314 @FRidh @Madouura