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

EARTHLY_PUSH is always false when set in the base target #3919

Open
nrdxp opened this issue Mar 18, 2024 · 8 comments
Open

EARTHLY_PUSH is always false when set in the base target #3919

nrdxp opened this issue Mar 18, 2024 · 8 comments
Labels
type:question Not an issue. Just a question.

Comments

@nrdxp
Copy link

nrdxp commented Mar 18, 2024

What went wrong?
When using ARG EARTHLY_PUSH in the base target of an Earthfile, the value seems to always contain false whether --push was passed on the cli or not.

What should have happened?

It should accurately reflect whether push was passed or not

Other Helpful Information
Repro:

VERSION 0.8
FROM ubuntu
WORKDIR /build

ARG EARTHLY_PUSH
RUN --no-cache echo $EARTHLY_PUSH

check: 
  ARG EARTHLY_PUSH
  RUN --no-cache echo $EARTHLY_PUSH

out of earthly --push +check:

              +check | --> FROM +base
              ubuntu | --> Load metadata ubuntu linux/amd64
               +base | --> FROM ubuntu
               +base | [----------] 100% FROM ubuntu
               +base | *cached* --> WORKDIR /build
               +base | --> RUN --no-cache echo $EARTHLY_PUSH
               +base | false
               +base | --> COPY Cargo.* rust-toolchain.toml .rustfmt.toml .
              +check | --> RUN --no-cache echo $EARTHLY_PUSH
              +check | true
@nrdxp nrdxp added the type:bug Something isn't working label Mar 18, 2024
@idodod
Copy link
Contributor

idodod commented Mar 25, 2024

I looked into this and noticed that flag is intentionally set to false when the target is not referened directly in the cli or by a BUILD statement. So for example earthly --push +base will return true.
I'll discuss with the team how we want to address this.

@alexcb
Copy link
Collaborator

alexcb commented Mar 25, 2024

This appears to be working as intended, as it follows the same logic of when a SAVE IMAGE --push ... is actually pushed.

from https://docs.earthly.dev/docs/earthfile#what-is-being-output-and-pushedpushed :

What is being output and pushed

In Earthly v0.6+, what is being output and pushed is determined either by the main target being invoked on the command-line directly, or by targets directly connected to it via a chain of BUILD calls. Other ways to reference a target, such as FROM, COPY, WITH DOCKER --load etc, do not contribute to the final set of outputs or pushes.

If you are referencing a target via some other command, such as COPY and you would like for the outputs or pushes to be included, you can issue an equivalent BUILD command in addition to the COPY. For example

Copy
my-target:
COPY --platform=linux/amd64 (+some-target/some-file.txt --FOO=bar) ./
Should be amended with the following additional BUILD call:

Copy
my-target:
BUILD --platform=linux/amd64 +some-target --FOO=bar
COPY --platform=linux/amd64 (+some-target/some-file.txt --FOO=bar) ./
This, however, assumes that the target +my-target is itself connected via a BUILD chain to the main target being built. If that is not the case, additional BUILD commands should be issued higher up the hierarchy.

any targets that don't have an explicit FROM will have an implicit FROM +base, which prevents the push from occurring in the base layer.

Maybe there's a use case that we didn't think about when this decision was made -- can you share some details on why you need to perform a push in the base target?

@alexcb alexcb added type:question Not an issue. Just a question. and removed type:bug Something isn't working labels Mar 25, 2024
@nrdxp
Copy link
Author

nrdxp commented Mar 31, 2024

I need to know if --push is passed so I can run a release build instead of a debug build. The former is much heavier and I only want to run it in CI if we are actually pushing the artifact somewhere

@alexcb
Copy link
Collaborator

alexcb commented Apr 1, 2024

I'm still trying to understand your use-case.

If you have an intermediate target that needs to perform some sort of optimization, (e.g. using the -O3 flag if you're compiling c), maybe you could rework your Earthfile to look like:

VERSION 0.8
FROM alpine
RUN apk add gcc

binaries:
    ARG OPTIMIZE=false
    IF [ "$OPTIMIZE" = "true" ]
        RUN gcc -O3 ...
    ELSE
        RUN gcc-g .... # debug symbols instead
    END

myimg:
    ARG EARTHLY_PUSH
    FROM +binaries --optimize="$EARTHLY_PUSH"
    SAVE IMAGE --push ...

@nrdxp
Copy link
Author

nrdxp commented Apr 1, 2024

that is essentially what I'm trying to do, except that EARTHLY_PUSH is false when it shouldn't be, leading to this not working as I expected it to. I guess I don't understand the reasoning behind the current behavior, if you are proporting it to be correct. At the very least it violates the principle of least suprise as that is not what most people would expect.

It is a simple boolean behavior (I either passed the flag or I didn't) and the variable should be equally as simple (reflecting whether the flag was passed or not)

@alexcb
Copy link
Collaborator

alexcb commented Apr 1, 2024

The reasoning (as documented under "What is being output and pushed"), can be summarised by this example:

VERSION 0.8
FROM alpine
RUN apk add gcc

foo:
    RUN echo foo > /foo
    SAVE IMAGE --push user/example:foo

bar:
    FROM +foo
    RUN echo bar > /bar
    SAVE IMAGE --push user/example:bar

when you run earthly --push +bar, earthly will only push the user/example:bar image.

The EARTHLY_PUSH value needs to stay consistent with the existing SAVE IMAGE --push behaviour.

@nrdxp
Copy link
Author

nrdxp commented Apr 1, 2024

I get that, for implementation, that it may be necessary to keep the current behavior, but it's a lot more cognitive overload to deal with, and I still don't fully understand why my code wasn't working. I have a +ci target which explicitly calls BUILD +build before BUILD +docker, yet for some reason EARTHLY_PUSH was still false during the +build target even when I passed --push

Unfortunately I can't give you the exact code that wasn't working because I already removed it from my worktree (since it wasn't working), but I will play around with it and see if I can get a better understanding.

@alexcb
Copy link
Collaborator

alexcb commented Apr 2, 2024

that's interesting that even with the explicit BUILD +target, it didn't set the target to be a push.

The current behaviour might contain a data race, for example consider calling earthly --push +bar:

VERSION 0.8
FROM alpine

foo:
    ARG EARTHLY_PUSH
    RUN echo foo has EARTHLY_PUSH=$EARTHLY_PUSH # this will be false since it's triggered via "FROM +foo"

bar:
    FROM +foo
    ARG EARTHLY_PUSH
    RUN echo bar has EARTHLY_PUSH=$EARTHLY_PUSH

where as if foo is only triggered via a BUILD +foo, then both will be true:

VERSION 0.8
FROM alpine

foo:
    ARG EARTHLY_PUSH
    RUN echo foo has EARTHLY_PUSH=$EARTHLY_PUSH # this will be true since it's triggered via a BUILD

bar:
    BUILD +foo
    ARG EARTHLY_PUSH
    RUN echo bar has EARTHLY_PUSH=$EARTHLY_PUSH

However the race occurs if it's triggered first by a FROM, and then again via a BUILD, e.g.

VERSION 0.8
FROM alpine
RUN echo bust1

foo:
    ARG EARTHLY_PUSH
    RUN echo foo has EARTHLY_PUSH=$EARTHLY_PUSH # this will be false, since it is first triggered by a FROM

bar:
    FROM +foo
    BUILD +foo
    ARG EARTHLY_PUSH
    RUN echo bar has EARTHLY_PUSH=$EARTHLY_PUSH

but if you switch the order of FROM and BUILD, both end up being true:

VERSION 0.8
FROM alpine
RUN echo bust1

foo:
    ARG EARTHLY_PUSH
    RUN echo foo has EARTHLY_PUSH=$EARTHLY_PUSH # true since first triggered via a BUILD

bar:
    BUILD +foo
    FROM +foo
    ARG EARTHLY_PUSH
    RUN echo bar has EARTHLY_PUSH=$EARTHLY_PUSH

I agree this is very confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:question Not an issue. Just a question.
Projects
Status: Icebox
Development

No branches or pull requests

3 participants