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

--output doesn't always work properly if run multiple times on same directory #53

Closed
JohnCMoon opened this issue Mar 13, 2024 · 1 comment

Comments

@JohnCMoon
Copy link
Contributor

As alluded to in issue #51, the --output argument to CARGO doesn't work properly when invoked multiple times with the same target directory.

Take the following example which you can run on a new project created with cargo new --bin foo:

VERSION 0.8

IMPORT github.com/earthly/lib/rust AS rust

install:
  FROM rust:latest
  RUN apt-get update -qq
  RUN apt-get install --no-install-recommends -qq autoconf autotools-dev libtool-bin clang cmake bsdmainutils
  DO rust+INIT --keep_fingerprints=true

source:
  FROM +install
  RUN echo asdf
  COPY --keep-ts Cargo.toml Cargo.lock ./
  COPY --keep-ts --dir src ./

build:
  FROM +source
  RUN rm -rf target
  DO rust+CARGO --args="build --release --bins" --output="release/[^/\.]+"
  RUN ./target/release/foo

test:
  FROM +build
  DO rust+CARGO --args="build --release --tests" --output="release/deps/foo-[^/\.]+"
  RUN ./target/release/deps/foo-*

all:
  BUILD +build
  BUILD +test

earthly +all yields the following:

...
              +build | --> RUN rm -rf target
              +build | --> RUN if [ ! -n "$EARTHLY_CACHE_PREFIX" ]; then echo "+INIT has not been called yet in this build environment" ; exit 1; fi;
              +build | --> RUN if [ ! -n "$EARTHLY_CACHE_PREFIX" ]; then echo "+INIT has not been called yet in this build environment" ; exit 1; fi;
              +build | --> mkdir /run
              +build | --> IF [ "$EARTHLY_KEEP_FINGERPRINTS" = "false" ]
              +build | --> RUN set -e; cargo $args; cargo sweep -r -t $EARTHLY_SWEEP_DAYS; cargo sweep -r -i;
              +build |     Finished release [optimized] target(s) in 0.00s
              +build | [INFO] Searching recursively for Rust project folders
              +build | [INFO] Cleaned 0.00 B from "/target"
              +build | [INFO] Searching recursively for Rust project folders
              +build | [INFO] Using all installed toolchains: ["1.76.0-x86_64-unknown-linux-gnu"]
              +build | [INFO] Cleaned 0.00 B from "/target"
              +build | --> mkdir /run
              +build | --> IF [ "$output" != "" ]
                +all | --> BUILD +test
              +build | --> RUN if [ ! -n "$EARTHLY_RUST_TARGET_CACHE" ]; then echo "+SET_CACHE_MOUNTS_ENV has not been called yet in this build environment" ; exit 1; fi;
              +build | --> RUN if [ -n "$output" ]; then echo "Copying output files" ; mkdir -p $TMP_FOLDER; cd target; find . -type f -regextype posix-egrep -regex "./$output" -exec cp --parents {} $TMP_FOLDER \; ; cd ..; fi;
              +build | Copying output files
              +build | --> RUN mkdir -p target; mv $TMP_FOLDER/* target 2>/dev/null || echo "no files found within ./target matching the provided output regexp";
              +build | --> RUN ./target/release/foo
              +build | Hello, world!
               +test | --> RUN if [ ! -n "$EARTHLY_CACHE_PREFIX" ]; then echo "+INIT has not been called yet in this build environment" ; exit 1; fi;
               +test | --> RUN if [ ! -n "$EARTHLY_CACHE_PREFIX" ]; then echo "+INIT has not been called yet in this build environment" ; exit 1; fi;
               +test | --> mkdir /run
               +test | --> IF [ "$EARTHLY_KEEP_FINGERPRINTS" = "false" ]
               +test | --> RUN set -e; cargo $args; cargo sweep -r -t $EARTHLY_SWEEP_DAYS; cargo sweep -r -i;
               +test |     Finished release [optimized] target(s) in 0.00s
               +test | [INFO] Searching recursively for Rust project folders
               +test | [INFO] Cleaned 0.00 B from "/target"
               +test | [INFO] Searching recursively for Rust project folders
               +test | [INFO] Using all installed toolchains: ["1.76.0-x86_64-unknown-linux-gnu"]
               +test | [INFO] Cleaned 0.00 B from "/target"
               +test | --> mkdir /run
               +test | --> IF [ "$output" != "" ]
               +test | --> RUN if [ ! -n "$EARTHLY_RUST_TARGET_CACHE" ]; then echo "+SET_CACHE_MOUNTS_ENV has not been called yet in this build environment" ; exit 1; fi;
               +test | --> RUN if [ -n "$output" ]; then echo "Copying output files" ; mkdir -p $TMP_FOLDER; cd target; find . -type f -regextype posix-egrep -regex "./$output" -exec cp --parents {} $TMP_FOLDER \; ; cd ..; fi;
               +test | Copying output files
               +test | --> RUN mkdir -p target; mv $TMP_FOLDER/* target 2>/dev/null || echo "no files found within ./target matching the provided output regexp";
               +test | no files found within ./target matching the provided output regexp
               +test | --> RUN ./target/release/deps/foo-*
               +test | /bin/sh: 1: ./target/release/deps/foo-*: not found
               +test | ERROR Earthfile line 26:2
               +test |       The command
               +test |           RUN ./target/release/deps/foo-*
               +test |       did not complete successfully. Exit code 127

The main warning I'm looking at is +test | no files found within ./target matching the provided output regexp. This is unexpected since cargo build --release --tests should produce an output that matches that regex.

If I simply add rm -rf target in the test target, it works:

test:
  FROM +build
  RUN rm -rf target
  DO rust+CARGO --args="build --release --tests" --output="release/deps/foo-[^/\.]+"
  RUN ./target/release/deps/foo-*
               +test | Copying output files
               +test | --> RUN mkdir -p target; mv $TMP_FOLDER/* target 2>/dev/null || echo "no files found within ./target matching the provided output regexp";
               +test | --> RUN ./target/release/deps/foo-*

               +test | running 0 tests

               +test | test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

This is because the output copy logic tries a blind mv of the $TMP_FOLDER contents to the target without checking to see if a directory already exists:

    RUN mkdir -p target; \
        mv $TMP_FOLDER/* target 2>/dev/null || echo "no files found within ./target matching the provided output regexp";

If I simply remove the stderr redirect to /dev/null, the error message shows what's going on:

mv: inter-device move failed: '/tmp/earthly/lib/rust/release' to 'target/release'; unable to remove target: Directory not empty
idelvall pushed a commit that referenced this issue Mar 13, 2024
Currently, when --output is used multiple times on the same directory,
the underlying command which copies the output can fail with a
misleading error message. Even if your regular expression matches a file
in the target directory, you're presented with:

"no files found within ./target matching the provided output regexp"

If the "mv" command's stderr is printed, it shows:

"mv: inter-device move failed: '/tmp/earthly/lib/rust/release' to
'target/release'; unable to remove target: Directory not empty"

Since "mv" doesn't handle existing directories the way we may want here,
let's use "cp -ruT" to overlay the files into the target directory
(regardless of what's already present there) and then remove the
temporary directory.
@JohnCMoon
Copy link
Contributor Author

Fixed by #54.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

1 participant