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

Display docker command line output. This also ensures the process output... #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

skullmaggots
Copy link

Changes to the DockerNativeClient so that the process buffer does not get full and hang the process.waitFor() blocking call for large docker builds. It also shows the progress of the docker build during its execution.

skullmaggots added 2 commits August 27, 2014 16:43
…put buffer does not get full and hang the process.waitFor() blocking call for large docker builds.
…gets copied if newer. This means incremental docker builds will make full use of the cache.
@skullmaggots
Copy link
Author

Added another minor change to this pull request to use ant copy task instead of built in copy. This is done so only files that have been modified are copied across from gradle build dir to docker stage if they have changed. Doing this means you will get full use of docker cache on ADD command, which also means subsequent docker commands after that will also get benefit of the cache.

@sgoings
Copy link

sgoings commented Aug 28, 2014

Sweet! I've been wanting to see this!

Any reason why you used ant's copy instead of the Sync task in Gradle? (or why you didn't wire up the inputs/outputs for a copy task such that Gradle won't copy the file if it doesn't need to?)

@sgoings
Copy link

sgoings commented Aug 28, 2014

Note: provides a fix for #26...

@skullmaggots
Copy link
Author

No technical reason - just looking for something other than gradle copy task which always copies the file whether it has changed or not.

@sgoings
Copy link

sgoings commented Aug 29, 2014

Part of the problem is that it was using the copy command in Gradle, not the full-fledged copy task, where the latter doesn't know about inputs/outputs so it doesn't benefit from any up-to-date checks. (see: http://www.gradle.org/docs/current/userguide/working_with_files.html#copy)

@sfitts
Copy link
Contributor

sfitts commented Sep 4, 2014

I agree that what we probably want to do is fully wire this to Gradle's task inputs/outputs, but my sense right now is that's a bit more work (see #20 for some related issues). For now I think avoiding the copy when not necessary is a good step. I'll do a quick sanity check and then merge to dev.

@sfitts
Copy link
Contributor

sfitts commented Sep 4, 2014

Unfortunately I'm seeing a failure in one of the tests -- testAddFileWithDir. I think the ant copy task has an issue copying directories. The error I see is:

: Use a resource collection to copy directories.
    at org.apache.tools.ant.taskdefs.Copy.validateAttributes(Copy.java:684)
    at org.apache.tools.ant.taskdefs.Copy.execute(Copy.java:442)

@sfitts
Copy link
Contributor

sfitts commented Sep 11, 2014

FYI -- I swiped the first part of this (to display the output as we go) and included it in my recent commit to dev.

@magnus-larsson
Copy link

Any plans to make a release with this issue resolved?
We are kind of blocked on issue #37 "docker build task is hanging" and I guess that the code committed to dev as described above solves this issue.

Regards,
Magnus.

@marcoivich
Copy link

I am running into this issue in Windows. Any updates into when this fix will be released?

Thank you,

Marco

@davidresnick
Copy link

We're also having this problem - on Windows systems, the build hangs. We still have a lot of developers working on Windows laptops, and will for the foreseeable future. Any update on when the fix will be included in a release?

@khauser
Copy link

khauser commented Feb 28, 2016

+1

@amckee23
Copy link

+1 Is there a roadmap for new releases?

@bjornmagnusson
Copy link
Contributor

No roadmap, but a next release is near. Some final issue to look over and verification to be done.

@imkarrer
Copy link

This PR has been open for 3 years. Is this repo dead?

@ummershervani
Copy link

Any luck merging this PR?

@bjornmagnusson
Copy link
Contributor

Part of this is included from other features in the master branch. This is however not released yet

@dmarkhas
Copy link

Can you release it..?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet