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

treewide: rename fetchPypi -> fetchFromPyPI #119896

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Synthetica9
Copy link
Member

@Synthetica9 Synthetica9 commented Apr 19, 2021

Motivation for this change

This rename does two things:

  1. Brings the capitization in line with the preferred capitization:
    PyPI is short for "Python Package Index" and references itself with
    this capitalized name.
  2. Adds "From". This is for consistency with other fetchers, which use
    from when fetching from a central or default repository:
    • fetchFromGitHub
    • fetchFromGitLab
    • fetchFromSourcehut
    • etc.
      A fetcher without "From" is used for fetchers without a central or
      default repository:
    • fetchurl
    • fetchpatch
    • fetchgit
    • fetchzip
    • etc.
      Since PyPI has a central repository, it should use the first pattern.

NB. This is intended to be a 0-rebuild change, so while it affects a large amount of files it should not cause a mass rebuild.

TODO:
  • Update release note
  • Add alias with warning for old fetchPypi
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.

This rename does two things:

1. Brings the capitization in line with the preferred capitization:
   PyPI is short for "Python Package Index" and references itself with
   this capitalized name.
2. Adds "From". This is for consistency with other fetchers, which use
   from when fetching from a central or default repository:
     * fetchFromGitHub
     * fetchFromGitLab
     * fetchFromSourcehut
     * etc.
   A fetcher without "From" is used for fetchers without a central or
   default repository:
     * fetchurl
     * fetchpatch
     * fetchgit
     * fetchzip
     * etc.
   Since PyPI has a central repository, it should use the first pattern.
@github-actions github-actions bot added 6.topic: python 6.topic: rust 8.has: documentation This PR adds or changes documentation labels Apr 19, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Apr 19, 2021
@stale
Copy link

stale bot commented Oct 22, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 22, 2021
@AndersonTorres
Copy link
Member

Can we redo this in a sprintable?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 17, 2021
@Synthetica9
Copy link
Member Author

I don't know how sprintable this is; it should just be merged in one shot I think and it's a fairly simple find-and-replace

@AndersonTorres
Copy link
Member

I am thinking in something like this:

  • Change the definition of fetchPypi to fetchFromPyPI
  • Create the alias - outside aliases.nix, because it is filtered out when enableAliases is disabled
  • Modify docs reflecting the new function
  • pull-request

After this PR be merged, the sprint can begin!

The sprint can be made in small steps, after all ripgrep returns 6400+ hits

After the sprint, move the alias to aliases.nix

@FRidh
Copy link
Member

FRidh commented Nov 18, 2021

If we're going to make a change I think it is preferable to have it instead generate an url that needs to explicitly be used with a fetcher (fetchurl), the reason being this function does not actually implement a fetcher, only a routine for computing an url.

@AndersonTorres
Copy link
Member

AndersonTorres commented Nov 18, 2021

If we're going to make a change I think it is preferable to have it instead generate an url that needs to explicitly be used with a fetcher (fetchurl), the reason being this function does not actually implement a fetcher, only a routine for computing an url.

You are suggesting something like this below?

src = fetchurl {
   url = pypiurl pname version;
   hash = "...";
}

I think it is not a useful distinction.

  • First, because for the end user, the only thing he thinks is "well, I have seen a new Python package on PyPI; so I will use fetchFromPyPI". The user do not need to know the inner functioning of fetchFromPyPI.
  • Second, because, well, under the hood, fetchFrom... uses the lower level fetchgit, fetchsvn,... fetchers.

@AndersonTorres AndersonTorres mentioned this pull request Nov 19, 2021
12 tasks
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 19, 2022
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python 6.topic: rust 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants