earthly / lib

Mozilla Public License 2.0
7 stars 11 forks source link

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

Closed JohnCMoon closed 5 months ago

JohnCMoon commented 5 months ago

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
JohnCMoon commented 5 months ago

Fixed by #54.