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

[api/client] Fix build when context dir is symlink #15039

Merged
merged 1 commit into from Jul 27, 2015

Conversation

jlhawn
Copy link
Contributor

@jlhawn jlhawn commented Jul 27, 2015

Symbolic links in the context directory path are now evaluated.

fixes #15037

@crosbymichael
Copy link
Contributor

@jlhawn does this need to go into 1.8?

@icecrime icecrime added this to the 1.8.0 milestone Jul 27, 2015
@icecrime
Copy link
Contributor

@crosbymichael I guess it does: looks like a regression from the docker cp notary PR.

@jlhawn jlhawn force-pushed the fix_build_context_is_symlink branch 2 times, most recently from a44542f to 77244c0 Compare July 27, 2015 17:46
@jlhawn
Copy link
Contributor Author

jlhawn commented Jul 27, 2015

I've added a second commit with a test simple test case. With the test case commit alone the test case fails with the same error that @crosbymichael saw in #15037 but with the api/client/build.go patch commit it passes.

@jlhawn jlhawn force-pushed the fix_build_context_is_symlink branch from 77244c0 to f40b1a5 Compare July 27, 2015 18:58
@calavera
Copy link
Contributor

@jlhawn can you squash them together when it's ready? it will be easier to track/cherry-pick the changes.

Symbolic links in the context directory path are now evaluated.

Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
@jlhawn jlhawn force-pushed the fix_build_context_is_symlink branch from f40b1a5 to 01d570a Compare July 27, 2015 19:01
@jlhawn
Copy link
Contributor Author

jlhawn commented Jul 27, 2015

@calavera done.

@jlhawn
Copy link
Contributor Author

jlhawn commented Jul 27, 2015

hmm, there are still some CI errors, but they seem unrelated.

@calavera
Copy link
Contributor

LGTM.

@crosbymichael
Copy link
Contributor

LGTM

crosbymichael added a commit that referenced this pull request Jul 27, 2015
[api/client] Fix build when context dir is symlink
@crosbymichael crosbymichael merged commit bdc55be into moby:master Jul 27, 2015
@jlhawn
Copy link
Contributor Author

jlhawn commented Jul 27, 2015

hmm, this test still failed on Windows:

FAIL: docker_cli_build_test.go:4745: DockerSuite.TestBuildRenamedDockerfile

docker_cli_build_test.go:4771:
    c.Fatal(err)
... Error: "build -f files\\Dockerfile -t test2 ." failed with errors: exit status 1 : "unable to prepare context: The Dockerfile (C:\\Windows\\TEMP\\fake-context137025938\\files\\Dockerfile) must be within the build context (.)\n")

@calavera
Copy link
Contributor

@jlhawn I'm not sure symlinks work on windows. We might need to extract that into platform specific files.

/cc @jhowardmsft

@lowenna
Copy link
Member

lowenna commented Jul 27, 2015

Yes, I believe that will be needed.

@jlhawn
Copy link
Contributor Author

jlhawn commented Jul 27, 2015

@calavera @jhowardmsft I've opened #15060 to try and fix that test case on Windows. There's no need to use symlink.FollowSymlinkInScope here anyway.

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.

build: Cannot locate specified Dockerfile: Dockerfile
7 participants