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

gitAndTools: move everything to the top level #109396

Merged
merged 1 commit into from Jan 14, 2021

Conversation

alyssais
Copy link
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.
Copy link
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
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
Member

ryantm commented Jan 27, 2021

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

@worldofpeace
Copy link
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants