Images not output when mixing FROM +target and BUILD +target (on the same target) when --wait-block feature is used

[JayyyXp]

When using a monorepo where Earthfiles in subdirs ouput images built in WAIT blocks, all the images marked for the output are not exported to host, but marked for push. Also this issue seems to flaky and effected by the Earthly volume cache.

Example code and output:

#/earthly_test/Earthfile
VERSION --shell-out-anywhere 0.6
 
FROM armdocker.rnd.ericsson.se/dockerhub-ericsson-remote/python:3.10.6-buster
 
all:
    BUILD ./component1/+all
    BUILD ./component2/+all
 
PUSH_ECHO1:
    COMMAND
 
    RUN --push echo "push component1"
PUSH_ECHO2:
    COMMAND
 
    RUN --push echo "push component2"
 
VERSION --shell-out-anywhere --wait-block 0.6
 
#/earthly_test/component2/Earthfile
FROM armdocker.rnd.ericsson.se/dockerhub-ericsson-remote/python:3.10.6-buster
 
all:
    WAIT
    BUILD +image
    END
    DO ../+PUSH_ECHO2
 
image:
    FROM ../component1/+image
    RUN --no-cache echo "hello component2"
    SAVE IMAGE --push component2/image:0.1
 
VERSION --shell-out-anywhere --wait-block 0.6
 
#/earthly_test/component1/Earthfile
FROM armdocker.rnd.ericsson.se/dockerhub-ericsson-remote/python:3.10.6-buster
 
all:
    WAIT
    BUILD +image
    END
    DO ../+PUSH_ECHO1
 
image:
    FROM armdocker.rnd.ericsson.se/dockerhub-ericsson-remote/python:3.10.7-buster
    RUN --no-cache echo "hello component1"
    SAVE IMAGE --push component1/image:0.1
 
 
OUTPUT:
3. Push ⏫ (disabled)
————————————————————————————————————————————————————————————————————————————————
 
To enable pushing use earthly --push
Did not push image component1/image:0.1
Did not push image component2/image:0.1
 
 4. Local Output 🎁
————————————————————————————————————————————————————————————————————————————————
 
Image ./component2+image output as component2/image:0.1
 
 
========================== 🌍 Earthly Build  ✅ SUCCESS ==========================

[alexcb]

@JayyyXp I tried to see if I could reproduce this with a smaller case, but unfortunately I can't. and succeeded roughly 10-20% of the time.

I added a similar directory structure under https://github.com/alexcb/earthly-test/tree/wait-no-local-export
but without any COMMANDs; however it does only set the --wait-block feature in the two sub directories, and doesn't set it in the root earthfile.

here's a single-line repro case to run:

cd /tmp && git clone git@github.com:alexcb/earthly-test.git && cd earthly-test && git checkout wait-no-local-export && earthly +all

and then it shows:

 4. Local Output 🎁
————————————————————————————————————————————————————————————————————————————————

Image ./othersubdir+othersubdirimage output as othersubidrimage:test
Image ./subdir+subdirimage output as subidrimage:test

about 90% of the time, but when it does fail, it only outputs a single image:

 4. Local Output 🎁
————————————————————————————————————————————————————————————————————————————————

Image ./othersubdir+othersubdirimage output as othersubidrimage:test

I wonder if there's anything we can adjust to increase the chance of it not outputting all images -- I note that you mention the bug appears intermittently. alexcb/earthly-test@8759a48 did the trick

[alexcb]

Here's a reproduction case that occurs all the time:

VERSION --wait-block 0.6
common-base:
    FROM alpine
    SAVE IMAGE common:base

use-common-base:
    FROM +common-base
    RUN echo "this won't trigger the SAVE IMAGE in common-base"

test:
    BUILD +use-common-base
    FROM alpine
    IF sleep 10
        RUN echo doing explicit build
        BUILD +common-base
    END

running earthly-v0.6.27 --no-cache +test won't ever produce the image.

The bug occurs due to FROM +common-base being triggered before BUILD +common-base occuring.

If however the BUILD +common-base occurs before the FROM +common-base occurs, the build will happen. (the original bug report involved multiple threads which explains the difficulty in reproducing it)

The --wait-block feature incorrectly made the assumption that a target is only referenced from a single location; as a result the're a race condition where the FROM +common-base doesn't perform an export (introduced in v0.6; see https://docs.earthly.dev/docs/earthfile#description-6 for details under the "What is being output and pushed" section).

It's interesting to note that the above reproduction case doesn't explicitly have a WAIT / END; however under the hood the --wait-block feature wraps the referenced target (+test from the cli in this example), with an implicit WAIT / END block.

In order to fix this bug, I suspect we'll need to change the converter code to save waitItems on the SingleTarget (effectively changing the waitItem interface to be similar to a future/promise pattern). Then we'll change SetDoSaves() and SetDoPushes() functions to attach waitItems on to the current WAIT block -- potentially supporting a single waitItem on multiple WAIT blocks.

[JayyyXp]

That's a really interesting finding, thank you for digging deep into this! It was nice to get some validation on this that I wasn't just going crazy :D

[alexcb]

fixed in #2444