Vendor dependencies, add hacking guide by s-urbaniak · Pull Request #148 · opencontainers/image-spec

s-urbaniak · 2016-06-16T09:54:50Z

This PR vendors all OCI image spec Go dependencies using glide and glide-vc.

Additionally it adds a hacking guide since we accumulated quite some tools/prerequisites for various make targets. The hacking guide documents them.

Fixes #76, #162

s-urbaniak · 2016-06-16T10:11:37Z

@vbatts git-validation complains about blanks at EOF in vendored files:

vendor/github.com/pkg/errors/LICENSE:24: new blank line at EOF.
vendor/go4.org/LICENSE:202: new blank line at EOF.

Can I somehow tell it to ignore any files in vendor/**?

wking · 2016-06-16T21:52:13Z

On Thu, Jun 16, 2016 at 02:54:51AM -0700, Sergiusz Urbaniak wrote:

This PR vendors all OCI image spec Go dependencies using glide and
glide-vc.

Plugged in yesterday's Git Rev News 1: manul 2, which uses
submodules for vendoring. I'm unlikely to be a big Go consumer, and I
don't have meaninful personal experience with any Go vendoring tools,
but I thought I'd mention it because I don't like committing external
code to my own repositories ;).

s-urbaniak · 2016-06-17T13:43:28Z

Committing vendored code is quite idiomatic in Go. We do this in many other projects.

Using git submodules for vendoring has been tried before (including me) with mixed/bad results, see https://groups.google.com/forum/m/#!topic/go-package-management/KR2zrSIsRho for details.

jonboulle · 2016-06-17T13:49:46Z

I think we should just fix git-validation

On 17 June 2016 at 15:43, Sergiusz Urbaniak notifications@github.com
wrote:

Committing vendored code is quite idiomatic in Go. We do this in many
other projects.

Using git submodules for vendoring has been tried before (including me)
with mixed/bad results, see
https://groups.google.com/forum/m/#!topic/go-package-management/KR2zrSIsRho
for details.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#148 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ACewN6zlXu2RL47EaRxOVj-rO5htzqkqks5qMqSBgaJpZM4I3N3y
.

philips · 2016-06-17T16:17:29Z

@vbatts please help with git-validation

On Fri, Jun 17, 2016 at 6:49 AM Jonathan Boulle notifications@github.com
wrote:

I think we should just fix git-validation

On 17 June 2016 at 15:43, Sergiusz Urbaniak notifications@github.com
wrote:

Committing vendored code is quite idiomatic in Go. We do this in many
other projects.

Using git submodules for vendoring has been tried before (including me)
with mixed/bad results, see

https://groups.google.com/forum/m/#!topic/go-package-management/KR2zrSIsRho
for details.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<
#148 (comment)
,
or mute the thread
<
https://github.com/notifications/unsubscribe/ACewN6zlXu2RL47EaRxOVj-rO5htzqkqks5qMqSBgaJpZM4I3N3y

.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#148 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AACDCPQn6oagwIwSxbIBwFCVSLvuDFjIks5qMqX7gaJpZM4I3N3y
.

vbatts · 2016-06-21T17:53:26Z

man. Since it is using git show --check ... I am not seeing a flag to ignore paths in that git command. Apart from wholly reimplementing this functionality outside of git, it may be faster to either
A) fix the offending issues in a correction commit
B) turn off the whitespace check for a bit

wking · 2016-06-21T18:00:29Z

On Tue, Jun 21, 2016 at 10:53:28AM -0700, Vincent Batts wrote:

A) fix the offending issues in a correction commit

I think the validator checks each commit, so you'd probably need to
patch the initial vendoring commit.

philips · 2016-06-22T18:10:54Z

I think we should just merge it :-/

LGTM

Signed-off-by: Sergiusz Urbaniak <sur@coreos.com>

s-urbaniak · 2016-06-23T07:12:04Z

What a mess, I did some duck-taping to hopefully fix everything:

Apply some sed-magic in the update-deps Makefile target to remove EOL's for each file in the vendor directory: ecb2556#diff-b67911656ef5d18c4ae36cb6741b7965R97

I am a bit worried whether this EOL-sed-fix will work on OSX though.

Pinned gojsonschema to the last working version until upstream has a fix ready: ecb2556#diff-395f41b2a27b70ce44399c91c82158c2R11

@vbatts @philips PTAL

vbatts · 2016-06-23T13:39:04Z

@s-urbaniak are we concerned about OSX? 😉

wking · 2016-06-23T13:47:45Z

Makefile

+	glide update --strip-vcs --strip-vendor --update-vendored --delete
+	glide-vc --only-code --no-tests
+	# see http://sed.sourceforge.net/sed1line.txt
+	for f in $$(find vendor -type f); do sed -i -e :a -e '/^\n*$$/{$$d;N;ba' -e '}' $$f; done


Probably better to use find's -exec instead of the for loop (-execdir is safer, but not in POSIX). Although you may have to keep the for loop, because -i is not in POSIX for sed.

vbatts · 2016-06-26T20:16:57Z

LGTM

jonboulle · 2016-06-27T10:35:19Z

LGTM (but do we need to worry about #148 (comment) ? I will leave it to @vbatts or one of the OS X users to merge)

vbatts · 2016-06-27T13:07:27Z

For loops are generally fine, though this would break if there were a path that had a space in it. That's generally frowned upon though 🤓.
As for the sed -i, I'm OK waiting until that is an issue.

After mulling on opencontainers#148 perhaps removing the for loop is a bit more simple and safe. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

After mulling on opencontainers/image-spec#148 perhaps removing the for loop is a bit more simple and safe. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

s-urbaniak force-pushed the vendor branch from 14527d4 to 98a088b Compare June 16, 2016 10:02

This was referenced Jun 16, 2016

oci-image-tool: Drop http.FileSystem? #150

Closed

serialization: add test, make volumes null #151

Closed

wking mentioned this pull request Jun 17, 2016

Makefile: fix fmt target and format json files #158

Merged

wking mentioned this pull request Jun 21, 2016

master broken: unable to validate: open /config-schema.json: no such file or directory #162

Closed

Sergiusz Urbaniak added 3 commits June 23, 2016 08:51

glide: introduce glide.yaml

fab787e

Signed-off-by: Sergiusz Urbaniak <sur@coreos.com>

Makefile: implemented update-deps target, add hacking guide

ecb2556

Signed-off-by: Sergiusz Urbaniak <sur@coreos.com>

glide: vendor dependencies

a664feb

Signed-off-by: Sergiusz Urbaniak <sur@coreos.com>

s-urbaniak force-pushed the vendor branch from 98a088b to a664feb Compare June 23, 2016 07:02

wking reviewed Jun 23, 2016
View reviewed changes

vbatts merged commit ca58cdf into opencontainers:master Jun 27, 2016

wking mentioned this pull request Jun 27, 2016

image: Refactor to use cas/ref engines instead of walkers #159

Closed

vbatts added a commit to vbatts/oci-image-spec that referenced this pull request Jun 27, 2016

Makefile: comments and simplify

fb56f01

After mulling on opencontainers#148 perhaps removing the for loop is a bit more simple and safe. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts mentioned this pull request Jun 27, 2016

Makefile: comments and simplify #166

Merged

vbatts added a commit to vbatts/oci-image-spec that referenced this pull request Jul 11, 2016

Makefile: comments and simplify

ce633c1

After mulling on opencontainers#148 perhaps removing the for loop is a bit more simple and safe. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

jonboulle mentioned this pull request Dec 21, 2016

identity: add implementation of ChainID #486

Merged

7c00d pushed a commit to 7c00d/J1nHyeockKim that referenced this pull request Jun 6, 2022

Makefile: comments and simplify

7d44ee8

After mulling on opencontainers/image-spec#148 perhaps removing the for loop is a bit more simple and safe. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

7c00d added a commit to 7c00d/J1nHyeockKim that referenced this pull request Jun 6, 2022

Makefile: comments and simplify

ce2b8e8

After mulling on opencontainers/image-spec#148 perhaps removing the for loop is a bit more simple and safe. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

Conversation

s-urbaniak commented Jun 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

s-urbaniak commented Jun 16, 2016

Uh oh!

wking commented Jun 16, 2016

Uh oh!

s-urbaniak commented Jun 17, 2016

Uh oh!

jonboulle commented Jun 17, 2016

Uh oh!

philips commented Jun 17, 2016

Uh oh!

vbatts commented Jun 21, 2016

Uh oh!

wking commented Jun 21, 2016

Uh oh!

philips commented Jun 22, 2016 • edited by caniszczyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

s-urbaniak commented Jun 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbatts commented Jun 23, 2016

Uh oh!

wking Jun 23, 2016

Choose a reason for hiding this comment

Uh oh!

vbatts commented Jun 26, 2016 • edited by caniszczyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonboulle commented Jun 27, 2016 • edited by caniszczyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbatts commented Jun 27, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

s-urbaniak commented Jun 16, 2016 •

edited

Loading

philips commented Jun 22, 2016 •

edited by caniszczyk

Loading

s-urbaniak commented Jun 23, 2016 •

edited

Loading

vbatts commented Jun 26, 2016 •

edited by caniszczyk

Loading

jonboulle commented Jun 27, 2016 •

edited by caniszczyk

Loading