Skip to content
This repository was archived by the owner on Oct 13, 2023. It is now read-only.

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

Merged
andrewhsu merged 2 commits intodocker-archive:17.06from
andrewhsu:fix-build-git
Jul 12, 2017
Merged

[17.06] Fix handling of remote "git@" notation#100
andrewhsu merged 2 commits intodocker-archive:17.06from
andrewhsu:fix-build-git

Conversation

@andrewhsu
Copy link
Contributor

@andrewhsu 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
Copy link
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
Copy link
Contributor Author

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

@andrewhsu andrewhsu mentioned this pull request Jul 11, 2017
Open
thaJeztah and others added 2 commits July 11, 2017 23:00
@thaJeztah @andrewhsu
Fix handling of remote "git@" notation
425242f 
`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>
@andrewhsu
add go imports to enable specific cherry-pick of 913eb99
7fe174d 
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
thaJeztah approved these changes Jul 12, 2017
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewhsu andrewhsu merged commit aaf863d into docker-archive:17.06 Jul 12, 2017
@andrewhsu andrewhsu deleted the fix-build-git branch July 12, 2017 21:43
@thaJeztah thaJeztah mentioned this pull request Jul 13, 2017
Closed
@MikeVL MikeVL mentioned this pull request Dec 21, 2017
Closed
docker-jenkins pushed a commit that referenced this pull request Mar 31, 2018
@seemethere
Merge pull request #100 from seemethere/update_readme
861c350 
Add Bionic to README
Upstream-commit: 9e33949
Component: packaging
docker-jenkins pushed a commit that referenced this pull request Oct 31, 2018
@andrewhsu
Merge pull request #100 from thaJeztah/18.09_backport_log_error_spelling
e0d454c 
[18.09 backport] Fix incorrect spelling in error message
Upstream-commit: 66bfae52bca82e6e3055ea5ce0203758af5364e9
Component: engine
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 30, 2020
@seemethere
Merge pull request docker-archive#100 from seemethere/update_readme
9e33949 
Add Bionic to README
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Feb 3, 2020
@andrewhsu
Merge pull request docker-archive#100 from andrewhsu/fix-build-git
119ebf1 
[17.06] Fix handling of remote "git@" notation
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@thaJeztah thaJeztah thaJeztah approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

17.06.1

Development

Successfully merging this pull request may close these issues.

2 participants

@andrewhsu @thaJeztah