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

github-runner: Fix labels for different nixpkgs versions #1143

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

YorikSar
Copy link
Contributor

@YorikSar YorikSar commented Nov 4, 2024

Changes to escapeShellArg introduced in NixOS/nixpkgs#333744 made different versions of nixpkgs behave differently. If current nix-darwin is used with nixpkgs before that change, labels end up having labels quoted twice (see #1085), but without changes from #1055, with new nixpkgs, labels end up not quoted at all, and ShellCheck ends up complaining that commas might have been used as array item separator (see https://www.shellcheck.net/wiki/SC2054).
Use the old version of escapeShellArg to always escape the list of labels and make nix-darwin work with both old and new versions of nixpkgs.

Fixes #1085

Cc @Gabriella439

Enzime
Enzime previously approved these changes Nov 6, 2024
Copy link
Collaborator

@Enzime Enzime left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for the contribution

@Enzime
Copy link
Collaborator

Enzime commented Nov 6, 2024

Can you rebase this on top of master to get the tests to run?

@YorikSar YorikSar force-pushed the fix-github-runner-labels branch from 41df9a8 to 4e3368b Compare November 7, 2024 06:24
Changes to escapeShellArg introduced in
NixOS/nixpkgs#333744 made different versions of
nixpkgs behave differently. If current nix-darwin is used with nixpkgs
before that change, labels end up having labels quoted twice
(see LnL7#1085), but without
changes from LnL7#1055, with new
nixpkgs, labels end up not quoted at all, and ShellCheck ends up
complaining that commas might have been used as array item separator
(see https://www.shellcheck.net/wiki/SC2054).

Use the old version of escapeShellArg to always escape the list of
labels and make nix-darwin work with both old and new versions of
nixpkgs.

Fixes LnL7#1085
@YorikSar
Copy link
Contributor Author

YorikSar commented Nov 7, 2024

Oof. I don't know what came over me. Fixed now.

Copy link
Collaborator

@Enzime Enzime left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for the contribution

@Enzime Enzime merged commit fa7b46f into LnL7:master Nov 7, 2024
6 checks passed
@YorikSar YorikSar deleted the fix-github-runner-labels branch November 7, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

github-runner: Labels are not passed correctly
2 participants