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

gawk: move gawkbug to gawkInteractive #314720

Merged
merged 1 commit into from
Aug 14, 2024
Merged

Conversation

tie
Copy link
Member

@tie tie commented May 26, 2024

Description of changes

This change removes gawkbug program from non-interactive build, similar to the bash{,Interactive} package.

Parent PR:

Things done

  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

@tie tie requested a review from alyssais May 26, 2024 01:04
@tie tie marked this pull request as ready for review May 26, 2024 01:04
@tie tie added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label May 26, 2024
@tie tie requested a review from Ericson2314 May 28, 2024 02:41
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label May 29, 2024
@tie
Copy link
Member Author

tie commented May 30, 2024

Hm, might as well move gawkbug to gawkInteractive, similar to bashInteractive, given that #314660 was merged.

@tie tie force-pushed the remove-gawkbug branch from 1df1c73 to 22ab825 Compare May 30, 2024 10:42
@tie tie changed the title gawk: remove gawkbug program gawk: move gawkbug to gawkInteractive May 30, 2024
@tie tie requested review from AndersonTorres and alyssais June 2, 2024 07:33
@tie
Copy link
Member Author

tie commented Jun 3, 2024

The PR originally removed gawkbug altogether, I’ve updated it to keep it only for gawkInteractive.

@wegank wegank added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Jun 10, 2024
@tie
Copy link
Member Author

tie commented Jul 16, 2024

@alyssais, gentle ping, is this good for merge?

@tie
Copy link
Member Author

tie commented Jul 19, 2024

@AndersonTorres, gentle ping, is this good for merge?

FWIW, this package is part of the stdenv and also doesn't have a maintainer now, #327779 😬

@tie tie requested a review from SuperSandro2000 August 7, 2024 03:10
@tie tie requested a review from philiptaron August 14, 2024 11:01
@tie
Copy link
Member Author

tie commented Aug 14, 2024

@philiptaron, added you as a reviewer if you don’t mind looking at this.

Comment on lines 72 to 75
remove-references-to -t "$NIX_CC" -- "$out"/bin/gawkbug
patchShebangs --host -- "$out"/bin/gawkbug
'' else ''
rm -- "$out"/bin/gawkbug
Copy link
Member

Choose a reason for hiding this comment

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

Also FYI: we don't need -- here because $out always starts with a slash.

Copy link
Member Author

@tie tie Aug 14, 2024

Choose a reason for hiding this comment

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

Fixed.

Note: I don’t think that’s documented in either Nix or Nixpkgs (i.e. that output paths are guaranteed to be absolute paths)…

This change removes gawkbug program from non-interactive build, similar
to the bash{,Interactive} package.
@tie tie added 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people and removed 12.approvals: 2 This PR was reviewed and approved by two reputable people labels Aug 14, 2024
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.

Result of nixpkgs-review pr 314720 run on x86_64-linux 1

2 packages built:
  • gawkInteractive
  • gawkInteractive.info (gawkInteractive.info.info)

@philiptaron philiptaron merged commit fa70b50 into NixOS:staging Aug 14, 2024
25 of 27 checks passed
@tie tie deleted the remove-gawkbug branch August 14, 2024 16:20
@tie
Copy link
Member Author

tie commented Aug 14, 2024

Thank you!

2 packages built:

Oh, I guess I could have rebased to master 😅

Well, the parent PR is targeting staging as well, so might be easier to rebase it though…

@philiptaron
Copy link
Contributor

No, I just used the -p flag to nixpkgs-review -- it was a lot otherwise.

@tie
Copy link
Member Author

tie commented Aug 14, 2024

Ah, OK. I generally manually copy-paste nixpkgs-review report and update the arguments accordingly.
Perhaps nixpkgs-review should include some arguments by default in the report (i.e. those that definitely cannot contain secrets)…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv This PR causes stdenv to rebuild 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants