Skip to content

copy-tarballs: drop perl bindings #361700

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

copy-tarballs: drop perl bindings #361700

wants to merge 1 commit into from
+58 −20

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Dec 4, 2024

This hopefully makes it easier to re-write this script in a language that people understand. Because it's shelling out, it's likely slower but hopefully still fast enough for our purposes.

Things done

I only tested local dry run without any credentials...

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@Mic92
Copy link
Member Author

Mic92 commented Dec 4, 2024

@vcunat can you give me some code how to test it?

@Mic92 Mic92 mentioned this pull request Dec 4, 2024
Merged
13 tasks
@Mic92
Copy link
Member Author

Mic92 commented Dec 4, 2024

Ok. Looks like I also should replace hashFile

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Dec 4, 2024
roberth
roberth reviewed Dec 4, 2024
View reviewed changes
@Mic92 Mic92 force-pushed the copy-tarballs branch 3 times, most recently from 9a65523 to 32beb24 Compare December 4, 2024 09:24
@vcunat
Copy link
Member

vcunat commented Dec 4, 2024

Nit: is it worth using --argstr when you can directly anti-quote the values into the nix expression? (at least I do assume that it's at least as easy in Perl as in Bash)

@Mic92 Mic92 force-pushed the copy-tarballs branch 2 times, most recently from 35bfeb9 to f9fcb5e Compare December 4, 2024 10:03
@Mic92
copy-tarballs: drop perl bindings
89093ba 
This hopefully makes it easier to re-write this script in a language
that people understand. Because it's shelling out, it's likely slower
but hopefully still fast enough for our purposes.
@Mic92
Copy link
Member Author

Mic92 commented Dec 4, 2024

Nit: is it worth using --argstr when you can directly anti-quote the values into the nix expression? (at least I do assume that it's at least as easy in Perl as in Bash)

It's less mental overhead for me to have to think about potential escaping issues, so I find it easier to read this way.

@roberth roberth mentioned this pull request Dec 4, 2024
Open
2 tasks
@mweinelt
Copy link
Member

mweinelt commented Jun 7, 2025 •
edited
Loading

Tested and runs into the following error on 4792576

error:
       … while calling the 'map' builtin
         at /home/tarball-mirror/nixpkgs/maintainers/scripts/find-tarballs.nix:26:17:
           25|
           26|   uniqueFiles = map (x: x.file) (genericClosure {
             |                 ^
           27|     startSet = map (file: {
       … while calling the 'genericClosure' builtin
         at /home/tarball-mirror/nixpkgs/maintainers/scripts/find-tarballs.nix:26:34:
           25|
           26|   uniqueFiles = map (x: x.file) (genericClosure {
             |                                  ^
           27|     startSet = map (file: {
       (stack trace truncated; use '--show-trace' to show the full, detailed trace)
       error: attribute 'outputHashAlgo' missing
       at /home/tarball-mirror/nixpkgs/maintainers/scripts/find-tarballs.nix:38:12:
           37|     isPatch = (drv ? postFetch && drv.postFetch != "");
           38|     type = drv.outputHashAlgo;
             |            ^
           39|     name = drv.name;
./maintainers/scripts/copy-tarballs.pl: evaluation failed

@mweinelt
Copy link
Member

Bisected and fixed in #414869.

Now I applied this patch for nixos/infra to pluto.nixos.org and mirror-tarballs.service works again.

@mweinelt mweinelt moved this to In Progress in Infra operational issues Jun 11, 2025
mweinelt added a commit to NixOS/infra that referenced this pull request Jun 12, 2025
@mweinelt
tarball-mirror: apply github.com/NixOS/nixpkgs/pull/361700
2882768
mweinelt added a commit to NixOS/infra that referenced this pull request Jun 12, 2025
@mweinelt
tarball-mirror: apply github.com/NixOS/nixpkgs/pull/361700
1dec544 
With NixOS 25.05 there is no nix_2_18 anymore and now we're on an newer
version and the perl bindings changed.
github-merge-queue bot pushed a commit to NixOS/infra that referenced this pull request Jun 12, 2025
@mweinelt
Merge pull request #732 from NixOS/tarball-mirror-patch
4c043c8 
tarball-mirror: apply github.com/NixOS/nixpkgs/pull/361700
@mweinelt mweinelt marked this pull request as ready for review June 12, 2025 16:21
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 13, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 3, 2025
@mweinelt
Copy link
Member

Can you rebase?

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

@roberth roberth roberth left review comments

@mweinelt mweinelt mweinelt approved these changes

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.
Projects
Infra operational issues
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Mic92 @vcunat @mweinelt @roberth @wegank