Skip to content

Conversation

@ydubreuil
Copy link

Current implementation doesn't work when builds are running on a slave node: code is checking that slave directory exists on master node, not on slave node. This patch fixes this and enhance logs to help diagnose further issues if any.

@reviewbybees

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

Choose a reason for hiding this comment

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

Can isSubDirectory(basePath, sourcePath)) ever be false? Since we said FilePath sourcePath = basePath.withSuffix(subdirectory).absolutize();

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it could be false if subdirectory contains something like "../non-workspace-dir". I guess that's what the original author wanted to check.

@varmenise
Copy link

🐝

@ghost
Copy link

ghost commented Jul 1, 2015

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be
reviewed by other CloudBees employees before we seek to have the change accepted. If you want to find out
more about our process please see this note

@ydubreuil
Copy link
Author

@reviewbybees done

@ghost
Copy link

ghost commented Jul 8, 2015

This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging.

@JohnBloom
Copy link

Any luck that this will get merged in? We are waiting for it upstream and feeling the pain. awslabs#28

@ydubreuil
Copy link
Author

@Suryanarayanan Would you mind merging this PR? Thanks

surybala added a commit that referenced this pull request Aug 27, 2015
@surybala surybala merged commit f8ab6c7 into jenkinsci:master Aug 27, 2015
@JohnBloom
Copy link

Does this build the plugin or does another PR need to be made to the awslabs project?

@surybala
Copy link

I did create another PR and merged into parent awslabs repo.

@JohnBloom
Copy link

Thanks a ton!

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.

5 participants