8000 fix: Docker tox-bootstrapd hash update failing when using BuildKit by nurupo · Pull Request #2447 · TokTok/c-toxcore · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: Docker tox-bootstrapd hash update failing when using BuildKit #2447

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

Merged
merged 2 commits into from
Dec 2, 2023

Conversation

nurupo
Copy link
Member
@nurupo nurupo commented Dec 2, 2023

Docker is defaulting to using BuildKit for building images since version
23.0 (2023-02-01), which is not compatible with this script. The script
was fishing the hash of the intermediate build container in which the
build has failed, in order to run the sha256sum in that image, however
with BuildKit there are no longer any intermediate build containers,
which breaks the script.

The legacy builder is supposedly getting removed in a future version of
Docker, which is why we embrace BuildKit instead of reverting to the
legacy builder via DOCKER_BUILDKIT=0:

$ DOCKER_BUILDKIT=0 docker build ...
DEPRECATED: The legacy builder is deprecated and will be removed in a
            future release. BuildKit is currently disabled; enable it
            by removing the DOCKER_BUILDKIT=0 environment-variable.

While DOCKER_BUILDKIT=1 is unnecessary on Docker >= 23.0, it's needed
for anyone running older Docker, so it makes sense to have it in for
now, while everyone transitions.


This change is Reviewable

@nurupo nurupo changed the title Fix Docker tox-bootstrapd hash update failing when using BuildKit fix: Docker tox-bootstrapd hash update failing when using BuildKit Dec 2, 2023
Copy link
codecov bot commented Dec 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7cfe35d) 74.37% compared to head (1b6dee7) 74.38%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2447   +/-   ##
=======================================
  Coverage   74.37%   74.38%           
=======================================
  Files          87       87           
  Lines       26255    26255           
=======================================
+ Hits        19528    19529    +1     
+ Misses       6727     6726    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nurupo nurupo force-pushed the bootstrapd-fix-docker-buildkit branch from 27d3469 to a59144a Compare December 2, 2023 06:41
Docker is defaulting to using BuildKit for building images since version
23.0 (2023-02-01), which is not compatible with this script. The script
was fishing the hash of the intermediate build container in which the
build has failed, in order to run the sha256sum in that image, however
with BuildKit there are no longer any intermediate build containers,
which breaks the script.

The legacy builder is supposedly getting removed in a future version of
Docker, which is why we embrace BuildKit instead of reverting to the
legacy builder via DOCKER_BUILDKIT=0:

  $ DOCKER_BUILDKIT=0 docker build ...
  DEPRECATED: The legacy builder is deprecated and will be removed in a
              future release. BuildKit is currently disabled; enable it
              by removing the DOCKER_BUILDKIT=0 environment-variable.

While DOCKER_BUILDKIT=1 is unnecessary on Docker >= 23.0, it's needed
for anyone running older Docker, so it makes sense to have it in for
now, while everyone transitions.
@nurupo nurupo force-pushed the bootstrapd-fix-docker-buildkit branch from a59144a to 1b6dee7 Compare December 2, 2023 06:50
@nurupo nurupo marked this pull request as ready for review December 2, 2023 07:53
@nurupo
Copy link
Member Author
nurupo commented Dec 2, 2023

I'm actually surprised no one has fixed the script before me. Did no one encounter the issue?

iphydf
iphydf 7C46 approved these changes Dec 2, 2023
@nurupo nurupo added this to the v0.2.19 milestone Dec 2, 2023
@nurupo nurupo merged commit 1b6dee7 into TokTok:master Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0