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

Symlink cargo build artifacts instead of copying them #322

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/cargoTarpaulin.nix
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
}:

{ cargoExtraArgs ? ""
, cargoTarpaulinExtraArgs ? "--skip-clean --out Xml --output-dir $out"
, cargoTarpaulinExtraArgs ? "--out Xml --output-dir $out"
, ...
}@origArgs:
let
Expand Down
2 changes: 2 additions & 0 deletions lib/setupHooks/inheritCargoArtifacts.nix
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{ makeSetupHook
, rsync
}:

makeSetupHook
{
name = "inheritCargoArtifactsHook";
propagatedBuildInputs = [ rsync ];
} ./inheritCargoArtifactsHook.sh

42 changes: 30 additions & 12 deletions lib/setupHooks/inheritCargoArtifactsHook.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,36 @@ inheritCargoArtifacts() {
elif [ -d "${preparedArtifacts}" ]; then
echo "copying cargo artifacts from ${preparedArtifacts} to ${cargoTargetDir}"

# NB: rustc doesn't like it when artifacts are either symlinks or hardlinks to the store
# (it tries to truncate files instead of unlinking and recreating them)
# so we're forced to do a full copy here :(
#
# Notes:
# - --no-target-directory to avoid nesting (i.e. `./target/target`)
# - preserve timestamps to avoid rebuilding
# - no-preserve ownership (root) so we can make the files writable
cp -r "${preparedArtifacts}" \
--no-target-directory "${cargoTargetDir}" \
--preserve=timestamps \
--no-preserve=ownership
# copy target dir but ignore crate build artifacts
rsync -r --chmod=Du=rwx,Dg=rx,Do=rx --exclude "release/build" --exclude "release/deps" --exclude "*/release/build" --exclude "*/release/deps" "${preparedArtifacts}/" "${cargoTargetDir}/"

link_build_artifacts() {
local artifacts="$1"
local target="$2"

if [ -d "${artifacts}/release/deps" ]; then
mkdir -p "${target}/release/deps"
for dep in $(ls "${artifacts}/release/deps"); do
ln -fs "${artifacts}/release/deps/$dep" "${target}/release/deps/$dep"
done
fi

if [ -d "${artifacts}/release/build" ]; then
mkdir -p "${target}/release/build"
for build in $(ls "${artifacts}/release/build"); do
ln -fs "${artifacts}/release/build/$build" "${target}/release/build/$build"
done
fi
}
Copy link
Owner

Choose a reason for hiding this comment

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

One thing worth noting is that this bit of logic is assuming that only release artifacts are being built. Although this is the default build behavior we apply, it's entirely possible a derivation can be configured to build any other profile (including custom ones).

Also the derivation could also be building for different targets (though I believe this is captured by the wildcards). I think a more robust approach might be to find out exactly which files cargo will tolerate as symlinks/hardlinks and use that instead (e.g. *.rlib, *.rmeta or */build/*, etc.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohhh. I 99% was testing it with debug build, as our crane-based derivations default to debug-inherited profile. That would explain the rebuilds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think a more robust approach might be to find out exactly which files cargo will tolerate as symlinks/hardlinks and use that instead (e.g. *.rlib, *.rmeta or /build/, etc.)

After a little bit of research, I think it's safer to just symlink the content-addressed artifacts (all files under build and deps), these are the artifacts that take most (almost all the) disk-space and I had some weird issues when testing something like this (create dirs recursively and then link every file from the nix store cargo target to the build target dir):

    find "${preparedArtifacts}" -type d -printf '%P\n' | sed '/^$/d' | xargs -d '\n' -n 100 -P 10 mkdir -p
    find "${preparedArtifacts}" -type f -printf '%P\n' | sed '/^$/d' | xargs -P 100 -d '\n' -I '##{}##' ln -fs "${preparedArtifacts}/##{}##" "##{}##"

Some .d files in the root of the build-target could not be overwritten, but others were overwritten...

I have found a way to make the code a little bit more concise and performant (symlinking of the amount of files can take some time, not much, but if it can be faster it should be faster :)) and now every build target and profile should be considered.

@dpc, can you try the latest commit?

Copy link
Owner

Choose a reason for hiding this comment

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

In the spirit of going as faster to be faster, we could even invoke mkdir -p on all directories with no child directories so we can do it in fewer spawned processes?

find "${preparedArtifacts}" -type d -links 2 ! -empty -printf '%P\n' | sed '/^$/d' | xargs -d '\n' -n 100 -P 10 mkdir -p

Copy link
Contributor

Choose a reason for hiding this comment

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

@Philipp-M Everything failing on c415996


# symlink crate build artifacts
link_build_artifacts "${preparedArtifacts}" "${cargoTargetDir}"

# for each build target as well
# all other directories are ignored in `link_build_artifacts`
for target in $(ls "${preparedArtifacts}"); do
link_build_artifacts "${preparedArtifacts}/$target" "${cargoTargetDir}/$target"
done

# Keep existing permissions (e.g. exectuable), but also make things writable
# since the store is read-only and cargo would otherwise choke
Expand Down