Skip to content

pin vndr to 9909bb2b8a0b7ea464527b376dc50389c90df587#318

Merged
vieux merged 1 commit into
docker:masterfrom
thaJeztah:pin-vndr
Jul 11, 2017
Merged

pin vndr to 9909bb2b8a0b7ea464527b376dc50389c90df587#318
vieux merged 1 commit into
docker:masterfrom
thaJeztah:pin-vndr

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

This make updating vndr a deliberate action, and prevents updates to vndr from making the vendor validation fail in CI.

Pinning to 9909bb2b8a0b7ea464527b376dc50389c90df587, which is current "master", and the same version as is used in moby/moby#34047

ping @tonistiigi @tiborvass @dnephin PTAL

This make updating vndr a deliberate action, and
prevents updates to vndr from making the vendor
validation fail in CI.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 10, 2017

Codecov Report

Merging #318 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #318   +/-   ##
=======================================
  Coverage   48.67%   48.67%           
=======================================
  Files         186      186           
  Lines       12416    12416           
=======================================
  Hits         6044     6044           
  Misses       5997     5997           
  Partials      375      375


RUN go get github.com/LK4D4/vndr && \
cp /go/bin/vndr /usr/bin && \
ARG VNDR_COMMIT=9909bb2b8a0b7ea464527b376dc50389c90df587
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should go in hack/dockerfile/binaries-commits

hack/dockerfile/install-binaries.sh will override this version otherwise

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@mlaventure this is the docker/cli repo, there's no hack/dockerfile/binaries-commits, so I just put it in the dockerfile

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤦‍♂️

Indeed 😅

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Jul 11, 2017

LGTM

@vieux vieux merged commit b7b6805 into docker:master Jul 11, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.07.0 milestone Jul 11, 2017
@thaJeztah thaJeztah deleted the pin-vndr branch July 11, 2017 14:28
RUN git clone https://github.com/LK4D4/vndr.git "/go/src/github.com/LK4D4/vndr" && \
cd "/go/src/github.com/LK4D4/vndr" && \
git checkout -q "$VNDR_COMMIT" && \
go build -v -o /usr/bin/vndr . && \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this going to break as soon as vndr adds a dependency, since we don't have a go get anywhere here?

go get will also get all the git files, so I think we can use go get to fetch, and still git checkout <sha>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Isn't this going to break as soon as vndr adds a dependency, since we don't have a go get anywhere here?

I assume it would vendor that dependency, but it's a ood point; I took the same approach as is used in moby/moby, so we may want to modify the approach there as well

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.

6 participants