Skip to content

gitAndTools: move everything to the top level#109396

Merged
alyssais merged 1 commit intoNixOS:masterfrom
alyssais:gitAndTools
Jan 14, 2021
Merged

gitAndTools: move everything to the top level#109396
alyssais merged 1 commit intoNixOS:masterfrom
alyssais:gitAndTools

Conversation

@alyssais
Copy link
Copy Markdown
Member

Motivation for this change

The comment at the top of git-and-tools/default.nix said:

/* All git-relates tools live here, in a separate attribute set so that users
 * can get a fast overview over what's available.

but unfortunately that hasn't actually held up in practice.

Git-related packages have continued to be added to the top level, or
into gitAndTools, or sometimes both, basically at random, so having
gitAndTools is just confusing. In fact, until I looked as part of
working on getting rid of gitAndTools, one program (ydiff) was
packaged twice independently, once in gitAndTools and once at the top
level (I fixed this in 98c3490).

So I think it's for the best if we move away from gitAndTools, and
just put all the packages it previously contained at the top level.
I've implemented this here by just making gitAndTools an alias for the
top level -- this saves having loads of lines in aliases.nix. This
means that people can keep referring to gitAndTools in their
configuration, but it won't be allowed to be used within Nixpkgs, and
it won't be presented to new users by e.g. nix search.

The only other change here that I'm aware of is that
appendToName "minimal" is not longer called on the default git
package, because doing that would have necessitated having a private
gitBase variable like before. I think it makes more sense not to do
that anyway, and reserve the "minimal" suffix only for gitMinimal.

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 pkgs 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

The comment at the top of git-and-tools/default.nix said:

    /* All git-relates tools live here, in a separate attribute set so that users
     * can get a fast overview over what's available.

but unfortunately that hasn't actually held up in practice.

Git-related packages have continued to be added to the top level, or
into gitAndTools, or sometimes both, basically at random, so having
gitAndTools is just confusing.  In fact, until I looked as part of
working on getting rid of gitAndTools, one program (ydiff) was
packaged twice independently, once in gitAndTools and once at the top
level (I fixed this in 98c3490).

So I think it's for the best if we move away from gitAndTools, and
just put all the packages it previously contained at the top level.
I've implemented this here by just making gitAndTools an alias for the
top level -- this saves having loads of lines in aliases.nix.  This
means that people can keep referring to gitAndTools in their
configuration, but it won't be allowed to be used within Nixpkgs, and
it won't be presented to new users by e.g. nix search.

The only other change here that I'm aware of is that
appendToName "minimal" is not longer called on the default git
package, because doing that would have necessitated having a private
gitBase variable like before.  I think it makes more sense not to do
that anyway, and reserve the "minimal" suffix only for gitMinimal.
@alyssais alyssais requested review from Mic92 and cole-h January 14, 2021 20:00
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python Python is a high-level, general-purpose programming language. 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package labels Jan 14, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jan 14, 2021
Copy link
Copy Markdown
Contributor

@nicknovitski nicknovitski left a comment

Choose a reason for hiding this comment

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

I have no objections to this.

Copy link
Copy Markdown
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

Diff LGTM.

@alyssais alyssais merged commit 3893d6c into NixOS:master Jan 14, 2021
@alyssais alyssais deleted the gitAndTools branch January 14, 2021 21:34
@ryantm
Copy link
Copy Markdown
Member

ryantm commented Jan 27, 2021

Thank you for doing this. Probably going to win my favorite PR of year award ;-)

@worldofpeace
Copy link
Copy Markdown
Contributor

worldofpeace commented Feb 26, 2021

I'm also pleasantly surprised by this. thanks @alyssais

md-magenta added a commit to neic/dotfiles that referenced this pull request Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python Python is a high-level, general-purpose programming language. 8.has: clean-up This PR removes packages or removes other cruft 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants