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

Dockerfile: don't delete /go #3957

Merged
merged 1 commit into from
Nov 20, 2024
Merged

Dockerfile: don't delete /go #3957

merged 1 commit into from
Nov 20, 2024

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Nov 20, 2024

We're creating /go and then deleting it later on, which doesn't make sense. Stop deleting it to fix the use case of Prow using the cosa image as a buildroot.

We're creating `/go` and then deleting it later on, which doesn't make
sense. Stop deleting it to fix the use case of Prow using the cosa image
as a buildroot.
jlebon added a commit to jlebon/release that referenced this pull request Nov 20, 2024
jlebon added a commit to jlebon/release that referenced this pull request Nov 20, 2024
@dustymabe
Copy link
Member

hmm. does it get populated with a bunch of stuff that we don't need to ship, though?

@jlebon
Copy link
Member Author

jlebon commented Nov 20, 2024

No, it's empty. It's created higher up in the Dockerfile.

@dustymabe
Copy link
Member

No, it's empty. It's created higher up in the Dockerfile.

yeah though it's not super clear that none of the steps in between when it is created and the end of the Dockerfile store things in /go my preference would be to move the creation of the directory to below the RUN ./build.sh... lines, which are what lead to the ambiguity IMO.

so this PR looks great, but would prefer if we moved the mkdir to below those lines.

@jlebon jlebon merged commit dcd60cf into coreos:main Nov 20, 2024
5 checks passed
@jlebon jlebon deleted the pr/dont-nuke-go branch November 20, 2024 20:13
@jlebon
Copy link
Member Author

jlebon commented Nov 20, 2024

Follow-up in #3958

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

Successfully merging this pull request may close these issues.

2 participants