Handle source, docs artifacts correctly for Ivy #1016
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the detailed overview description. I will run the integration tests on it when I get a chance. My initial impressions without reviewing the code are: a) I believe sources/docs configurations are added by Ivy when the metadata source is a pom.xml |
|
Thanks! I'll try to answer some of these.
How do you run the integration tests, is it just |
|
|
Ok. I'll change those back to classes, as I just noticed that I can achieve what I want through UpdateReport.substitute. On Tue, Dec 3, 2013 at 3:57 PM, Mark Harrah notifications@github.com
|
added 7 commits
December 4, 2013 13:52
…ks in conjunction with ArtifactTypeFilter
…ve artifacts defined in Ivy publications
|
I've made the changes. Do you think it looks alright? |
|
Yes, I think so. What I still have to do is run the integration tests and I'd like to have another integration test for this functionality. I'm busy today, but will try to do this tomorrow. If you have time, you can run them and see if any fail. If you need help, post the output here. If you can't figure out an integration test, post to sbt-dev or provide a sample project and the commands to run on it and I'll turn it into an integration test. So, this LGTM overall, but let's get the testing checked out before merging. |
|
Alright, thanks a lot! I also added a late patch, as I recently realized that |
|
Ok, you can individual tests like |
|
Both tests that failed for me fail in exactly the same way for |
|
Thanks. I've deleted |
|
Thank you. Oh.. I see. I mistook the [error] it showed for "test failed". On Tue, Dec 10, 2013 at 5:02 PM, Mark Harrah notifications@github.com
|
harrah
added a commit
that referenced
this pull request
Dec 13, 2013
Handle source, docs artifacts correctly for Ivy
|
Ran the tests and they pass, so LGTM. Merged to 0.14. |
4 tasks
dwijnand
added a commit
to dwijnand/sbt
that referenced
this pull request
Apr 5, 2017
I believe this was accidentally not forward-ported from the 0.14 branch to the 1.0.x branch. Notice the change was present in sbt#1016: https://github.com/sbt/sbt/pull/1016/files#diff-6373e7f7122325e753b75fe1cc76ff5fL576 and missing in sbt#2478: https://github.com/sbt/sbt/pull/2478/files#diff-6373e7f7122325e753b75fe1cc76ff5fR680
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following our discussion at #1004, I've decided to handle the retrieval of source/docs artifacts by their artifact types.
The steps I took are:
runtime. Why runtime? No reason specifically, and it might change, but for now it seemed like a sensible default.sourceArtifactTypesanddocArtifactTypes: Sets which contain the artifact types that would qualify them as either source or doc artifacts.ArtifactTypeFilterwhich lets you apply the filter normally or inversed.updatetask (specifically, I added it toUpdateConfiguration) to allow "all artifact types except those found in eithersourceArtifactTypesordocArtifactTypes"updateClassifiers/updateSbtClassifiers, invert the filter from theUpdateConfiguration: it will therefore only let through sources and docsUpdateConfigurationtoIvyActions.updateand from there to the Ivy ResolveOptions inIvyActions.resolveupdateClassifiersto not filter out dependencies whose classifiers were deemed "not found" and thus excluded - they might still contain src/doc artifacts published in their ivysModuleIDs (and thus theDependencyDescriptorsthey convert to) that will allow Ivy to merge the explicitly given artifacts (where classifier=sources or javadoc) with the publications from theModuleDescriptor(i.e. the artifacts published in an ivy)updateClassifiers, map over all artifacts to set their classifier to either sources or javadoc, if it's not already set, according to a reverse mapping from the types in(source|doc)ArtifactTypesto "source" and "javadoc" respectively (well,Artifact.SourceClassifierandArtifact.DocClassifier, to be precise). This is because the artifacts retrieved from Ivy publications might well not have this attribute set, and it's the unfortunate case that currently IDE plugins depend exactly on this value to classify the artifacts.Hopefully, after this, plugins could just look at the artifact's type and see in which of the two sets contains it. That would also resolve a great deal of pain surrounding "e:classifier". For instance, I've discovered that it's impossible to use that attribute in an ivy repository if its value is not either "sources" or "javadoc" (e.g. if it's src), because plugins expect it to be one of those two values as mentioned before.
updateClassifiers of course still works as before to retrieve sources/docs artifacts from Maven repositories.