Skip to content

git: add missing deps for filter-branch etc #27221

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

Merged
merged 3 commits into from
Jul 11, 2017
Merged

Conversation

aschmolck
Copy link
Contributor

@aschmolck aschmolck commented Jul 7, 2017

Motivation for this change

Several git commands are implemented as shell scripts that run awk, sed, grep
and perl as well as coreutils such as cut, wc, basedir and dirname. There is some existing patching in the postinstall for perl to rewrite
it to an absolute reference to pkgs.perl, but several other packages are both
missing as a dependency and have no rewrite logic.

In particular git filter-branch depends on sed and grep.

Additionally, the perl logic also seds git-am, which is now a binary not a shell
script anymore (see <github.com/git/git/blob/master/builtin/am.c>), so this part
was obsolete.

I tested this by grepping all shell scripts for the relevant commands and then
comparing the diffs of the new version to what is produced in master. All
changes in the scripts seem good to me. I'll attach the patch in a comment. I als

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip" (tried to but I got fatal: reference is not a tree: 3d89df6)
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@aschmolck
Copy link
Contributor Author

aschmolck commented Jul 7, 2017

Patch of the differences in the shell scripts.

@aschmolck aschmolck force-pushed the fix-git-deps branch 4 times, most recently from eefd0f4 to 736ef3a Compare July 10, 2017 15:58

Unverified

This user has not yet uploaded their public signing key.
Several git commands are implemented as shell scripts that run awk, sed, grep
and perl. There is some existing patching in the postinstall for perl to rewrite
it to an absolute reference to pkgs.perl, but several other packages are both
missing as a dependency and have no rewrite logic.

In particular git filter-branch depends on sed and grep.

Additionally, the perl logic also seds git-am, which is now a binary not a shell
script anymore (see <github.com/git/git/blob/master/builtin/am.c>), so this part
was obsolete.

I tested this by grepping all shell scripts for the relevant commands and then
comparing the diffs of the new version to what is produced in master. All
changes in the scripts seem good to me.
# Fix references to the perl, sed, awk and various coreutil binaries used by
# shell scripts that git calls (e.g. filter-branch)
perl -0777 -i -pe \
"BEGIN{@a=('${perl}/bin/perl', '${gnugrep}/bin/grep', '${gnused}/bin/sed', '${gawk}/bin/awk',"\
Copy link
Member

Choose a reason for hiding this comment

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

Odd indentation here. Is there a reason for that?

Copy link
Member

Choose a reason for hiding this comment

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

@aschmolck nix strips the prefix on multi-line strings.

I pushed a change to make it a bit more readable in c6322fa. Needs testing

@aschmolck
Copy link
Contributor Author

@zimbatm: thanks for cleaning up the formatting!

@zimbatm
Copy link
Member

zimbatm commented Jul 11, 2017

@aschmolck cheers :) can you build and test the changes on linux to confirm that it's still working?

@aschmolck
Copy link
Contributor Author

@zimbatm broke something, debugging (must be trivial though).

@aschmolck
Copy link
Contributor Author

aschmolck commented Jul 11, 2017

@zimbatm 2 hours of my life gone, but I found the problem and it was trivial: read -d '' works, in that it sets the variable but returns a non-zero exit code which interrupts the install. I've replaced it with $(cat <<'EOS'

@zimbatm
Copy link
Member

zimbatm commented Jul 11, 2017

oh no, sorry for pushing that onto you.

@zimbatm zimbatm merged commit 2c1097a into NixOS:master Jul 11, 2017
@MP2E
Copy link

MP2E commented Jul 11, 2017

Thank you both for your work on this! Very much appreciated :)

@aschmolck
Copy link
Contributor Author

@zimbatm haha -- not your fault :) Was just tricky to debug because it looked OK, pasting it into the shell prompt (correcting for nix-double-single-quote-escapes) seemingly worked OK, but somehow it still blew up.

NeQuissimus pushed a commit that referenced this pull request Aug 11, 2017
Several git commands are implemented as shell scripts that run awk, sed, grep
and perl. There is some existing patching in the postinstall for perl to rewrite
it to an absolute reference to pkgs.perl, but several other packages are both
missing as a dependency and have no rewrite logic.

In particular git filter-branch depends on sed and grep.

Additionally, the perl logic also seds git-am, which is now a binary not a shell
script anymore (see <github.com/git/git/blob/master/builtin/am.c>), so this part
was obsolete.

I tested this by grepping all shell scripts for the relevant commands and then
comparing the diffs of the new version to what is produced in master. All
changes in the scripts seem good to me.

(cherry picked from commit 2c1097a)
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
adrianpk added a commit to adrianpk/nixpkgs that referenced this pull request May 31, 2024
Several git commands are implemented as shell scripts that run awk, sed, grep
and perl. There is some existing patching in the postinstall for perl to rewrite
it to an absolute reference to pkgs.perl, but several other packages are both
missing as a dependency and have no rewrite logic.

In particular git filter-branch depends on sed and grep.

Additionally, the perl logic also seds git-am, which is now a binary not a shell
script anymore (see <github.com/git/git/blob/master/builtin/am.c>), so this part
was obsolete.

I tested this by grepping all shell scripts for the relevant commands and then
comparing the diffs of the new version to what is produced in master. All
changes in the scripts seem good to me.

(cherry picked from commit 2c1097a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants