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

[SPARK-21397][BUILD]Maven shade plugin adding dependency-reduced-pom.xml to … #18619

Closed
wants to merge 1 commit into from

Conversation

zuotingbing
Copy link

@zuotingbing zuotingbing commented Jul 13, 2017

…base directory

What changes were proposed in this pull request?

  1. run ./build/mvn -DskipTests clean package and we get many files 'dependency-reduced-pom.xml'
    under the source directory

  2. run ./build/mvn clean, the files dependency-reduced-pom.xml can not be cleaned.

As a trivial Improvement, i suggest to specify dependencyReducedPomLocation to 'target/' to store these files.

How was this patch tested?

manual tests

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@srowen
Copy link
Member

srowen commented Jul 13, 2017

I like the idea, but see this note: https://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#dependencyReducedPomLocation . I'm not clear whether this has side-effects.

@zuotingbing
Copy link
Author

zuotingbing commented Jul 13, 2017

@srowen Thanks for your time. I am not sure the open issue, which is mentioned in the note, whether has bean resolved. as the note said

setting a value for this parameter with a directory other than ${basedir} will change the value of ${basedir}

,but after running mvn checkstyle:check , i get the right path of ${basedir}/target/checkstyle-output.xml

@srowen
Copy link
Member

srowen commented Jul 13, 2017

That's not the problem this would create -- it changes basedir after execution of the shade plugin, so it might affect things like the release build. Is it really a problem? gitignore already ignores these files, even though they're a little annoying.

@zuotingbing
Copy link
Author

I just use mvn checkstyle:check ,which contains output files with basedir, to check whether the basedir is correct with this PR.
Could you please specify the affect exactly as you said? what is your mean about release build? or does it the output package is not right?

@zuotingbing
Copy link
Author

zuotingbing commented Jul 14, 2017

Yes this PR is a trivial Improvement for me. Thanks.

@srowen
Copy link
Member

srowen commented Jul 14, 2017

Meaning I think you'd have to run the release process to verify the artifacts still work, because it's the process that uses shading

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.

3 participants