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

Propagating MAKEFLAGS to support tagless builds #46606

Merged
merged 10 commits into from
Sep 25, 2024
Merged

Conversation

doggydogworld
Copy link
Contributor

@doggydogworld doggydogworld commented Sep 13, 2024

Passing MAKEFLAGS as an environment variable to docker run commands to support propagation of MAKEFLAGS to "submake" run in a container.

As an example of what this aims to fix, when calling a "submake" through our buildbox containers

docker run $(BUILDBOX) /usr/bin/make ...

The flags passed in to the parent make aren't being propagated. To fix this the MAKEFLAGS variable will be passed in as an environment variable to the container. This matches the behavior of how make handles submakes.

@doggydogworld doggydogworld marked this pull request as ready for review September 13, 2024 16:46
Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@doggydogworld doggydogworld added the no-changelog Indicates that a PR does not require a changelog entry label Sep 13, 2024
Copy link
Contributor

@camscale camscale left a comment

Choose a reason for hiding this comment

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

I think the PR description should be more precise in the problem being solved here. Make normally passes MAKEFLAGS to a submake by always putting it in the environment and the submake accepts that from the environment. When we are layering through docker, the environment is not passed so this breaks. But it can work if we pass the right args to docker run.

So I think this should be changed to run docker run -e MAKEFLAGS ... to propagate $MAKEFLAGS into the container and leave it at that. This also has the advantage that whatever is in $MAKEFLAGS may not be safe for the command line as there is no quoting of special characters, but the environment does not go via the command line so that should be safer.

I ran a local test and this does seem to work and be sufficient to propagate MAKEFLAGS.

Merged via the queue into master with commit b85dcb6 Sep 25, 2024
45 of 46 checks passed
@doggydogworld doggydogworld deleted the gus/nightly-release branch September 25, 2024 18:21
@public-teleport-github-review-bot

@doggydogworld See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed
branch/v16 Create PR

doggydogworld added a commit that referenced this pull request Sep 25, 2024
* Propagating makeflags for nightly release

* Trying recusrive submake in container

* Propagating makeflags another way

* More propagation

* Trying MAKEFLAGS as an env var

* Have specify MAKEFLAGS

* Typod a paranthesis

* Moving makeflags env flag for docker to the targets

* Typo

* Previous try had a typo didn't catch but should work now
doggydogworld added a commit that referenced this pull request Sep 25, 2024
* Propagating makeflags for nightly release

* Trying recusrive submake in container

* Propagating makeflags another way

* More propagation

* Trying MAKEFLAGS as an env var

* Have specify MAKEFLAGS

* Typod a paranthesis

* Moving makeflags env flag for docker to the targets

* Typo

* Previous try had a typo didn't catch but should work now
github-merge-queue bot pushed a commit that referenced this pull request Sep 30, 2024
* Propagating makeflags for nightly release

* Trying recusrive submake in container

* Propagating makeflags another way

* More propagation

* Trying MAKEFLAGS as an env var

* Have specify MAKEFLAGS

* Typod a paranthesis

* Moving makeflags env flag for docker to the targets

* Typo

* Previous try had a typo didn't catch but should work now
github-merge-queue bot pushed a commit that referenced this pull request Sep 30, 2024
* Propagating makeflags for nightly release

* Trying recusrive submake in container

* Propagating makeflags another way

* More propagation

* Trying MAKEFLAGS as an env var

* Have specify MAKEFLAGS

* Typod a paranthesis

* Moving makeflags env flag for docker to the targets

* Typo

* Previous try had a typo didn't catch but should work now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants