Skip to content

setupHooks: replace sed with ripgrep for reference removal - #1094

Open
cyclic-pentane wants to merge 1 commit into
ipetkov:masterfrom
cyclic-pentane:refs-sed-ripgrep
Open

cyclic-pentane wants to merge 1 commit into
ipetkov:masterfrom
cyclic-pentane:refs-sed-ripgrep

Conversation

@cyclic-pentane

Copy link
Copy Markdown

Motivation

I was debugging an issue where the reference nuking step (aka removeReferencesToRustToolchainHook) and (removeReferencesToVendoredSources) taking a very long time during a crane build. The underlying issue was that debug symbols were turned on for release builds too, which made the binaries >500MB in size, and meant that sed was taking quite a while (>20 minutes in my case) to find-and-replace the store paths, since it's relatively unoptimised.

This PR replaces sed with rg, which is highly optimised for that usecase. In my case, it cut down the 20 minutes sed find-and-replace to less than 10 seconds.

Tested by verifying that the byte contents of the output files stay the exact same:

$ nix flake init -t github:ipetkov/crane#quick-start
$ nix build -v -L --out-link result-old
$ sha256sum result-old/bin/*
977e2d98bdcc13dc6872f930c65e8ec5c45279c61ef68a09db45f98ea8fd1252  result-old/bin/quick-start
$ nix build -v -L --override-input crane github:cyclic-pentane/crane/refs-sed-ripgrep --out-link result-new
$ sha256sum result-old/bin/*
977e2d98bdcc13dc6872f930c65e8ec5c45279c61ef68a09db45f98ea8fd1252  result-new/bin/quick-start

I'm not sure whether this is the best way to do this - in particular, the way this PR uses head -c 1 to cut off the trailing newline emitted by rg feels quite hacky.

Checklist

  • added tests to verify new behavior
  • added an example template or updated an existing one
  • updated docs/API.md (or general documentation) with changes
  • updated CHANGELOG.md

This branch has not been deployed

No deployments
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.

1 participant