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

[17.06] Fix handling of remote "git@" notation #100

Merged
merged 2 commits into from Jul 12, 2017

Conversation

Projects
None yet
2 participants
@andrewhsu
Collaborator

andrewhsu commented Jul 6, 2017

Backport fix from:

By cherry-pick of just the first commit, moby/moby@913eb99:

$ git cherry-pick -s -x -Xsubtree=components/engine 913eb99
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 6, 2017

Member

CI is failing because of;

19:50:27 # github.com/docker/docker/pkg/gitutils
19:50:27 pkg/gitutils/gitutils_test.go:19: undefined: require in require.NoError
19:50:27 pkg/gitutils/gitutils_test.go:20: undefined: assert in assert.NotEmpty
19:50:27 pkg/gitutils/gitutils_test.go:21: undefined: assert in assert.Equal
19:50:27 pkg/gitutils/gitutils_test.go:24: undefined: require in require.NoError
19:50:27 pkg/gitutils/gitutils_test.go:25: undefined: assert in assert.NotEmpty
19:50:27 pkg/gitutils/gitutils_test.go:26: undefined: assert in assert.Equal
19:50:27 pkg/gitutils/gitutils_test.go:29: undefined: require in require.NoError
19:50:27 pkg/gitutils/gitutils_test.go:30: undefined: assert in assert.NotEmpty
19:50:27 pkg/gitutils/gitutils_test.go:31: undefined: assert in assert.Equal
19:50:27 pkg/gitutils/gitutils_test.go:34: undefined: require
19:50:27 pkg/gitutils/gitutils_test.go:34: too many errors

Because it's not yet using stretchify (added in this refactor; moby/moby@b9d85ac)

Probably just adding these to imports should be sufficient;

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Member

thaJeztah commented Jul 6, 2017

CI is failing because of;

19:50:27 # github.com/docker/docker/pkg/gitutils
19:50:27 pkg/gitutils/gitutils_test.go:19: undefined: require in require.NoError
19:50:27 pkg/gitutils/gitutils_test.go:20: undefined: assert in assert.NotEmpty
19:50:27 pkg/gitutils/gitutils_test.go:21: undefined: assert in assert.Equal
19:50:27 pkg/gitutils/gitutils_test.go:24: undefined: require in require.NoError
19:50:27 pkg/gitutils/gitutils_test.go:25: undefined: assert in assert.NotEmpty
19:50:27 pkg/gitutils/gitutils_test.go:26: undefined: assert in assert.Equal
19:50:27 pkg/gitutils/gitutils_test.go:29: undefined: require in require.NoError
19:50:27 pkg/gitutils/gitutils_test.go:30: undefined: assert in assert.NotEmpty
19:50:27 pkg/gitutils/gitutils_test.go:31: undefined: assert in assert.Equal
19:50:27 pkg/gitutils/gitutils_test.go:34: undefined: require
19:50:27 pkg/gitutils/gitutils_test.go:34: too many errors

Because it's not yet using stretchify (added in this refactor; moby/moby@b9d85ac)

Probably just adding these to imports should be sufficient;

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@andrewhsu

This comment has been minimized.

Show comment
Hide comment
@andrewhsu

andrewhsu Jul 6, 2017

Collaborator

Added another commit to this PR with those imports. Looks like the unit tests now pass:

$ make shell # enter the docker-dev env where gopath is setup properly
root@7af0b04b51b4:/go/src/github.com/docker/docker# go test github.com/docker/docker/pkg/gitutils
ok      github.com/docker/docker/pkg/gitutils   0.040s
Collaborator

andrewhsu commented Jul 6, 2017

Added another commit to this PR with those imports. Looks like the unit tests now pass:

$ make shell # enter the docker-dev env where gopath is setup properly
root@7af0b04b51b4:/go/src/github.com/docker/docker# go test github.com/docker/docker/pkg/gitutils
ok      github.com/docker/docker/pkg/gitutils   0.040s

thaJeztah and others added some commits Jun 15, 2017

Fix handling of remote "git@" notation
`docker build` accepts remote repositories
using either the `git://` notation, or `git@`.

Docker attempted to parse both as an URL, however,
`git@` is not an URL, but an argument to `git clone`.

Go 1.7 silently ignored this, and managed to
extract the needed information from these
remotes, however, Go 1.8 does a more strict
validation, and invalidated these.

This patch adds a different path for `git@` remotes,
to prevent them from being handled as URL (and
invalidated).

A test is also added, because there were no
tests for handling of `git@` remotes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 913eb99)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
add go imports to enable specific cherry-pick of 913eb99
So that we don't have bring in the entire commit d3d1aab

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>

@andrewhsu andrewhsu modified the milestone: 17.06.1 Jul 12, 2017

@thaJeztah

LGTM

@andrewhsu andrewhsu merged commit aaf863d into docker:17.06 Jul 12, 2017

3 checks passed

ce-tests Jenkins build docker-ce-17.06-pr 165 has succeeded
Details
ce-tests-WoW-RS1 Jenkins build docker-ce-17.06-pr-WoW-RS1 163 has succeeded
Details
dco-signed All commits are signed

@andrewhsu andrewhsu deleted the andrewhsu:fix-build-git branch Jul 12, 2017

@MikeVL MikeVL referenced this pull request Dec 21, 2017

Open

Upgrade docker package #723

docker-jenkins pushed a commit that referenced this pull request Mar 31, 2018

Merge pull request #100 from seemethere/update_readme
Add Bionic to README
Upstream-commit: 9e33949813b2589271f7a4535008b209d95200ab
Component: packaging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment