Skip to content

Update dependencies#1036

Merged
imjasonh merged 3 commits intogoogle:mainfrom
imjasonh:deps
Jun 3, 2021
Merged

Update dependencies#1036
imjasonh merged 3 commits intogoogle:mainfrom
imjasonh:deps

Conversation

@imjasonh
Copy link
Copy Markdown
Contributor

@imjasonh imjasonh commented Jun 3, 2021 •
edited
Loading 

go get -u ./...
go mod vendor
./hack/update-codegen.sh

This updates our containerd dep to v1.5.2, which contains containerd/containerd@363f2c3 to only check CPU arch on-demand the first time it's needed, instead of at init-time.

Fixes #916

@imjasonh imjasonh requested a review from jonjohnsonjr June 3, 2021 16:46
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 3, 2021 •
edited
Loading

Codecov Report

Merging #1036 (a7ca1b5) into main (9b2cec9) will not change coverage.
The diff coverage is n/a.

❗ Current head a7ca1b5 differs from pull request most recent head 4923d47. Consider uploading reports for the commit 4923d47 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1036   +/-   ##
=======================================
  Coverage   75.03%   75.03%           
=======================================
  Files         107      107           
  Lines        5072     5072           
=======================================
  Hits         3806     3806           
  Misses        720      720           
  Partials      546      546

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b2cec9...4923d47. Read the comment docs.

@imjasonh
Copy link
Copy Markdown
Contributor Author

imjasonh commented Jun 3, 2021

Needed to add a lock around defaultKeychain.Resolve to avoid a data race, seemingly newly coming from docker/cli#3025 (Thanks to @jonjohnsonjr for digging that up)

@jonjohnsonjr
Copy link
Copy Markdown
Collaborator

jonjohnsonjr commented Jun 3, 2021

How would you feel about sync.Onceing the Load?

@imjasonh
Copy link
Copy Markdown
Contributor Author

imjasonh commented Jun 3, 2021

How would you feel about sync.Onceing the Load?

I'm not opposed in theory, but I might want to make that as a separate change. The mutex is a quick hack to avoid the data race (which seems relatively innocuous anyway).

Skipping the load each time is probably good for performance, especially for short-lived CLI use cases, but something that uses ggcr in a long-running, potentially multi-tenant scenario might unknowingly depend on the load each time, and could be surprised in a bad way if we optimized it.

@imjasonh imjasonh merged commit 162d96e into google:main Jun 3, 2021
@imjasonh imjasonh mentioned this pull request Jun 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonjohnsonjr jonjohnsonjr jonjohnsonjr approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Docker version upgrade?

3 participants

@imjasonh @codecov-commenter @jonjohnsonjr