Skip to content
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

Create builder/command, cut libcontainer dependency on integration-cli #10727

Merged
merged 2 commits into from Feb 12, 2015
Merged

Create builder/command, cut libcontainer dependency on integration-cli #10727

merged 2 commits into from Feb 12, 2015

Conversation

@ahmetb
Copy link
Contributor

@ahmetb ahmetb commented Feb 12, 2015

#10561 (d1e9d07) introduces a dependency to libcontainer and other daemon related packages through builder package in integration-cli tests. The only thing test needs is set of the Dockerfile commands. Extracting them to a separate

package: builder/commands.

This was causing CI tests to not to compile on non-Linux platforms due to indirect dependency to libcontainer from CLI test code. (Example: https://jenkins.dockerproject.com/job/Windows-PRs/2/console)

Signed-off-by: Ahmet Alp Balkan ahmetalpbalkan@gmail.com

Label: #windows
Cc: @duglin @tiborvass @erikh @unclejack

d1e9d07 introduces a dependency to libcontainer and other daemon
related packages through builder package. The only thing test needs
is set of the Dockerfile commands. Extracting them to a separate
package.

This was causing CI tests to not to compile on non-Linux platforms.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
@duglin
Copy link
Contributor

@duglin duglin commented Feb 12, 2015

Just wondering, why don't you have an issue with the docker_cli_daemon_test's ? I would think those would have a similar issue since they create daemons.

@ahmetb
Copy link
Contributor Author

@ahmetb ahmetb commented Feb 12, 2015

@duglin it has // +build daemon build tag. :)

@duglin
Copy link
Contributor

@duglin duglin commented Feb 12, 2015

so you skip those?

@ahmetb
Copy link
Contributor Author

@ahmetb ahmetb commented Feb 12, 2015

@duglin nope that file doesn't get compiled at all when DOCKER_CLIENTONLY=1 is set (the case for darwin/windows)

@tiborvass
Copy link
Collaborator

@tiborvass tiborvass commented Feb 12, 2015

LGTM

@tiborvass
Copy link
Collaborator

@tiborvass tiborvass commented Feb 12, 2015

Ping @duglin

@duglin
Copy link
Contributor

@duglin duglin commented Feb 12, 2015

LGTM

duglin added a commit that referenced this issue Feb 12, 2015
…-compile-fix

Create builder/command, cut libcontainer dependency on integration-cli
@duglin duglin merged commit 802802b into moby:master Feb 12, 2015
1 of 2 checks passed
@ahmetb
Copy link
Contributor Author

@ahmetb ahmetb commented Feb 12, 2015

supposedly this change should allow integration-cli to build on windows but apparently there's a GOPATH problem, @jfrazelle was gonna take a look.

@ahmetb ahmetb deleted the win-cli/integration-cli-compile-fix branch Feb 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants