Skip to content

lib, treewide: introduce repoRevToName and use it to cleanup most fetch* functions#316668

Merged
Lassulus merged 3 commits intoNixOS:masterfrom
oxij:tree/repo-to-name-part-1
Jun 10, 2025
Merged

lib, treewide: introduce repoRevToName and use it to cleanup most fetch* functions#316668
Lassulus merged 3 commits intoNixOS:masterfrom
oxij:tree/repo-to-name-part-1

Conversation

@oxij
Copy link
Member

@oxij oxij commented Jun 2, 2024

This a first half of #49862. Basically, this is a noop cleanup that keeps all the fetch* names as they are now, while introducing the new config.fetchedSourceNameDefault option.

Changes

This patch adds lib.repoRevToName function that generalizes away most of the code used for derivation name generation by fetch* functions (fetchzip, fetchFromGitHub, etc, except those which are delayed until latter commits for mass-rebuild reasons).

It's first argument controls how the resulting name will look (see below).

Since lib has no equivalent of Nixpkgs' config, this patch adds config.fetchedSourceNameDefault option to Nixpkgs and then re-exposes lib.repoRevToName config.fetchedSourceNameDefault expression as pkgs.repoRevToNameMaybe which is then used in fetch* derivations.

The result is that different values of config.fetchedSourceNameDefault now control how the src derivations produced by fetch* functions are to be named, e.g.:

  • fetchedSourceNameDefault = "source" (the default):

      $ nix-instantiate -A fuse.src
      /nix/store/<hash>-source.drv
    
  • fetchedSourceNameDefault = "versioned":

      $ nix-instantiate -A fuse.src
      /nix/store/<hash>-libfuse-2.9.9-source.drv
    
  • fetchedSourceNameDefault = "full":

      $ nix-instantiate -A fuse.src
      /nix/store/<hash>-libfuse-2.9.9-github-source.drv
    

See documentation of config.fetchedSourceNameDefault for more info.

@oxij oxij requested a review from infinisil as a code owner June 2, 2024 13:40
@github-actions github-actions bot added 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 6.topic: lib The Nixpkgs function library labels Jun 2, 2024
@oxij oxij changed the title lib, treewide: introduce repoRevToName and use to cleanup most fetch* functions lib, treewide: introduce repoRevToName and use it to cleanup most fetch* functions Jun 2, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jun 2, 2024
@oxij oxij force-pushed the tree/repo-to-name-part-1 branch from 38ffa76 to a09a1e7 Compare June 5, 2024 11:48
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 10, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
@oxij oxij force-pushed the tree/repo-to-name-part-1 branch from a09a1e7 to 6fc7913 Compare March 19, 2025 12:24
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 19, 2025
@nix-owners nix-owners bot requested review from hsjobeki and philiptaron March 19, 2025 12:26
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

A few curious questions as I read through this PR.

@github-actions github-actions bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Mar 19, 2025
@oxij oxij force-pushed the tree/repo-to-name-part-1 branch from 6fc7913 to 3b81d58 Compare March 19, 2025 12:49
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2025
@oxij
Copy link
Member Author

oxij commented Mar 19, 2025

Okay, so I edited this to use fetchedSourceNameDefault instead of nameSourcesPrettily, and changed the type to be a simple enum.

`nixfmt` check fails, but I have no idea on which file. All files I think are ok to be touched pass `nixfmt` without changes locally. I don't think re-formatting of files under `pkgs/top-level` should be done in this PR. /cc @infinisil

A simple way to see which files fail the checks, except to run those checks yourself, would be nice, though.

@oxij oxij requested a review from AndersonTorres March 19, 2025 13:28
@oxij oxij force-pushed the tree/repo-to-name-part-1 branch from 3b81d58 to 08d03fa Compare March 19, 2025 14:28
@oxij
Copy link
Member Author

oxij commented Mar 31, 2025

ping @AndersonTorres specifically, since your review requested changes that were since resolved, IMHO.

Also, somewhat relatedly, #391322 is a follow-up to 6620964 which changed the version without changing the hash, which would not have happened if @mweinelt used this PR with fetchedSourceNameDefault = "versioned" set. :]

Such errors, how frequent are they? 🤔

Just saying.

Copy link
Member

@AndersonTorres AndersonTorres left a comment

Choose a reason for hiding this comment

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

I would complain about nixfmt not having its own commit. But nothing relevant is being lost.

So, LGTM.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 31, 2025
@mweinelt
Copy link
Member

Such errors, how frequent are they? 🤔

The cause is limitations in the python bulk updater. These commits are under my name, but update-python-libraries created them.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@oxij oxij force-pushed the tree/repo-to-name-part-1 branch from 08d03fa to dd3352f Compare April 2, 2025 18:20
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@oxij
Copy link
Member Author

oxij commented Apr 2, 2025

@AndersonTorres Thanks! Also, mass-reformat now ate those nixfmt changes anyway.

@mweinelt Well, okay, but it would have been caught by this infra.

Also, rebased onto master to fix conflicts.

@oxij
Copy link
Member Author

oxij commented Apr 4, 2025

Ping. What's holding this up?

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 17, 2025
@Lassulus Lassulus force-pushed the tree/repo-to-name-part-1 branch from dd3352f to 6404a0f Compare May 20, 2025 19:48
@arianvp
Copy link
Member

arianvp commented May 20, 2025

Took the liberty with @Lassulus to rebase this to solve the merge conflict

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label May 20, 2025
@oxij
Copy link
Member Author

oxij commented May 21, 2025

Thanks. Rebase LGTM.

oxij added 3 commits May 31, 2025 10:01
…ctions

This patch adds `lib.repoRevToName` function that generalizes away most of the
code used for derivation name generation by `fetch*` functions (`fetchzip`,
`fetchFromGitHub`, etc, except those which are delayed until latter commits
for mass-rebuild reasons).

It's first argument controls how the resulting name will look (see below).

Since `lib` has no equivalent of Nixpkgs' `config`, this patch adds
`config.fetchedSourceNameDefault` option to Nixpkgs and then re-exposes
`lib.repoRevToName config.fetchedSourceNameDefault` expression as
`pkgs.repoRevToNameMaybe` which is then used in `fetch*` derivations.

The result is that different values of `config.fetchedSourceNameDefault` now
control how the `src` derivations produced by `fetch*` functions are to be
named, e.g.:

- `fetchedSourceNameDefault = "source"` (the default):

  ```
  $ nix-instantiate -A fuse.src
  /nix/store/<hash>-source.drv
  ```

- `fetchedSourceNameDefault = "versioned"`:

  ```
  $ nix-instantiate -A fuse.src
  /nix/store/<hash>-libfuse-2.9.9-source.drv
  ```

- `fetchedSourceNameDefault = "full"`:

  ```
  $ nix-instantiate -A fuse.src
  /nix/store/<hash>-libfuse-2.9.9-github-source.drv
  ```

See the documentation of `config.fetchedSourceNameDefault` for more info.
@oxij oxij force-pushed the tree/repo-to-name-part-1 branch from 6404a0f to 7c8a4ef Compare May 31, 2025 10:02
@oxij
Copy link
Member Author

oxij commented Jun 3, 2025

Rebased again to fix conflicts. And it is now stuck at Darwin check again. And the previous time it was stuck right there for a month. So, I don't think it's likely this will get any better anytime soon.

@Lassulus Lassulus merged commit 78f051e into NixOS:master Jun 10, 2025
17 of 18 checks passed
@oxij
Copy link
Member Author

oxij commented Jun 23, 2025

Thanks @Lassulus!

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

Labels

6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 6.topic: lib The Nixpkgs function library 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: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants