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

./maintainers/scripts/check-by-name.sh: delete broken -I argument #282226

Closed
wants to merge 1 commit into from
Closed

./maintainers/scripts/check-by-name.sh: delete broken -I argument #282226

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 20, 2024

Running CI locally is broken becauses the -I argument:

  • Clobbers $NIX_PATH
  • Is wrong for two reasons:
    • Has too many .. elements, relative to the script's location
    • Isn't relative to the script's location (as with *.nix files), since shell scripts and POSIX in general interpret paths relative to the current working directory, not the canonical path of argv[0]
  • Is inconsistent, since this script has symlinks pointing at it from different depths in the repository

There is no way to set this flag statically in a way that will work everywhere. The caller needs to use $NIX_PATH, or the script needs to take the -I value as an argument.

This commit deletes the static -I flag.

Running CI locally is broken becauses the `-I` argument:

- Clobbers $NIX_PATH
- Is wrong for two reasons:
  - Has too many `..` elements, relative to the script's location
  - Isn't relative to the script's location (as with *.nix files),
    since shell scripts and POSIX in general interpret paths
    relative to the current working directory, not the canonical
    path of argv[0]
- Is inconsistent, since this script has symlinks pointing at it
  from different depths in the repository

There is no way to set this flag statically in a way that will work
everywhere.  The caller needs to use $NIX_PATH, or the script needs
to take the `-I` value as an argument.

This commit deletes the static `-I` flag.
@ghost ghost requested a review from infinisil as a code owner January 20, 2024 05:06
@ghost ghost changed the title ./maintainers/scripts/check-by-name.sh: fix broken -I argument ./maintainers/scripts/check-by-name.sh: delete broken -I argument Jan 20, 2024
@ghost ghost closed this Jan 23, 2024
@ghost ghost deleted the pr/by-name/fix branch January 23, 2024 06:47
@infinisil infinisil added this to the RFC 140 milestone Jan 25, 2024
@infinisil
Copy link
Member

Including this in #284765

infinisil added a commit that referenced this pull request Jan 29, 2024
Original commit from #282226,
message:

Running CI locally is broken becauses the `-I` argument:

- Clobbers $NIX_PATH
- Is wrong for two reasons:
  - Has too many `..` elements, relative to the script's location
  - Isn't relative to the script's location (as with *.nix files),
    since shell scripts and POSIX in general interpret paths
    relative to the current working directory, not the canonical
    path of argv[0]
- Is inconsistent, since this script has symlinks pointing at it
  from different depths in the repository

There is no way to set this flag statically in a way that will work
everywhere.  The caller needs to use $NIX_PATH, or the script needs
to take the `-I` value as an argument.

This commit deletes the static `-I` flag.
infinisil added a commit to NixOS/nixpkgs-vet that referenced this pull request Mar 18, 2024
Original commit from NixOS/nixpkgs#282226,
message:

Running CI locally is broken becauses the `-I` argument:

- Clobbers $NIX_PATH
- Is wrong for two reasons:
  - Has too many `..` elements, relative to the script's location
  - Isn't relative to the script's location (as with *.nix files),
    since shell scripts and POSIX in general interpret paths
    relative to the current working directory, not the canonical
    path of argv[0]
- Is inconsistent, since this script has symlinks pointing at it
  from different depths in the repository

There is no way to set this flag statically in a way that will work
everywhere.  The caller needs to use $NIX_PATH, or the script needs
to take the `-I` value as an argument.

This commit deletes the static `-I` flag.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant