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

Making use of Ivy source | javadoc configs to resolve sources and javadocs #1004

Closed
dansanduleac opened this issue Nov 25, 2013 · 8 comments
Closed
Milestone

Comments

@dansanduleac
Copy link
Contributor

Currently, sbt resolves "classifiers" (sources and javadocs) by taking the existing ModuleID (a sbt class representing a dependency) and creating sbt.Artifacts from it (in sbt.IvyActions.classifiedArtifacts) which have the e:classified set to sources or javadoc (the classifiers themselves come from the setting transitiveClassifiers :== List(sources, javadoc)).

However I'd argue that this is incorrect:

  • for one, it doesn't touch the current project's sources / javadoc configs that Ivy will see
  • it assumes sources and javadoc artifacts always follow this pattern, so setting transitiveClassifiers differently for a particular resolver (what if I want them to be "src" and "doc"?) is not possible
  • Ivy xml files (from sbt, but not only) often define a sources configuration and a source artifact under that configuration. For example, publishing sbt itself and looking in e.g. main, we can see that it mentions exactly what artifact it's publishing under the sources configuration. That e:classifier is "sources" here is just a coincidence / convention (it's sbt's default), but that's not necessarily the case everywhere.
<publications>
  ...
  <artifact name="main" type="src" ext="jar" conf="sources" e:classifier="sources"/>
</publications>

Thus, I think sbt should make its "Sources" configuration, which corresponds with the ivy sources config, depend on the sources configuration of each dependency (same with javadoc). In a published Ivy file this would look like <dependency ... conf="compile->default(compile);sources;javadoc"> as in, aside from the normal stuff, "our" sources depends on this dep's sources, and our javadoc depends on this dep's javadoc.

Only if such artifact/configuration isn't provided in a dependency's ivy file should sbt magically produce an artifact, hoping that the file exists and follows the default pattern.

@harrah
Copy link
Member

harrah commented Nov 25, 2013

The current behavior is the Maven way, which is for better or for worse the de facto correct way in the Java ecosystem. By default, Ivy will try to lookup these sources and javadocs classifiers when resolving a pom and include them in the respective configurations. However, this takes a lot of time, misses are expensive, and Ivy doesn't cache those misses well. So, sbt disables that and implements the Maven way via updateClassifiers, since most dependencies are published as pom.xml to a Maven repository. This is the right way to handle those.

It is true that Ivy dependencies aren't handled and that could be improved. Do you mean sbt itself should depend on sources/javadoc for dependencies or that it should make projects do that by default? (Also, I believe you'd need sources->sources.)

@dansanduleac
Copy link
Contributor Author

Sorry about the formatting.
Thanks for explaining! Indeed I had a hunch it had something to do with maven, since all the ivys in the cache that were built from from maven POMs seemed to have no sources artifacts defined. That of course seems perfectly fine as long as the ivy file doesn't explicitly specify source artifacts.
All I'm arguing is that when it does, they should be picked up. Specifically

  • The project's external dependencies should gain a "sources;javadoc" in their configurations attribute. I do mean sources->sources;javadoc->javadoc by that, but from what I've read online Ivy auto-expands that, just like it does with compile(*) to mean compile->compile(*).
  • The default update task (compile:update) should not resolve these (after all maybe we're on a prod server, and as you said it can be slow), whereas sources:update should resolve the source configurations of all deps transitively (same for javadoc).
  • (not necessarily urgent, but might be a good idea and make everything uniform) The dependencies themselves, when they come from Maven, still end up as Ivy files. Thus it would be good if their sources artifacts (as deduced by updateClassifiers) were encoded in the generated ivy file. Thus they can be automatically picked up, or not, depending on what configuration you call update on. This would make things uniform as you wouldn't need updateClassifiers anymore, and you wouldn't get the hit of downloading them either unless you specifically ask for it with sources:update.

I'm not sure exactly what you mean by "sbt itself should depend on sources/javadoc for dependencies". If it's that a sbt project's sources & javadoc ivy configs should depend on its dependencies' sources & javadoc ivy configs, then yes, I'm suggesting that.

Ultimately I'd like the source jars of all my project's deps to be brought to the ivy cache if I call something like sources:update.
I doubt that works as I'm expecting it to right now, as I've tried to declare some libraryDependencies in sources:
libraryDependencies in Sources += "org.abc" % "something" % "[1.0,)" % "sources"
but they don't get picked up at all.

@harrah
Copy link
Member

harrah commented Nov 25, 2013

  • Ivy uses defaultMappings (or something like that) to know what to do when there is no mapping specified. sbt makes this x->compile by default.
  • compile(*) is not valid on its own as far as I know. It can only be on the right side, where the () indicate what to do if compile does not exist. Having used this system for a while and thought about what a better dependency manager would do, it is my opinion that compile should always be the default and if you mean otherwise, you should state it explicitly. (Automatic processing might need this as a workaround due to limitations of the Ivy/Maven model.)
  • sbt has moved away from automatically adding dependency declarations as much as possible. This is somewhat unavoidable when trying to set up a good out-of-the-box Scala experience, but even there it has been minimized. Something similar to adding a dependency on sources and javadoc was done in 0.7 and earlier with withSources and withJavadoc (they explicitly declared a classifier, but the result was similar).
  • Yes, pom.xml is translated to ivy.xml, but in order to have the information in ivy.xml, Ivy has to do the lookup on classifiers at the time it downloads the pom.xml as I described before. This is a performance problem in practice.
  • There is only one update per project. If you run sources:update, that delegates to *:update. So, libraryDependencies in Sources isn't used. That's a known source of confusion. I've previously proposed not delegating for something specified on the command line (but still doing it when referenced programmatically) but I'm not sure what side effects it will have. You could open a bug for this if you agree.
  • updateClassifiers can certainly add the contents of what you declare in the sources configuration with the assumption that you add these declarations. However, that means that your Ivy dependencies also need to declare a sources configuration (and so on, transitively). These will also be resolved at the same time as normal dependencies due to there being one update by design. In theory they could be resolved by updateClassifiers instead. I suspect it will be messy in practice.
  • Another option is to do something similar to what updateClassifiers does with classifiers and automatically add dependencies on sources and javadoc configurations for all transitive dependencies resolved as an ivy.xml. The downside is you lose explicit control over the names of those configurations, but perhaps this could be a setting. In the simple case, not discriminating per dependency, transitiveSourceConfigurations and transitiveDocConfigurations.

@dansanduleac
Copy link
Contributor Author

  • Sorry about that, I got confused by some SO questions regarding this. Indeed one person here correctly points out how to set defaultConfMapping to make that work, but I didn't see that part before:

    It's possible to specify configurations defaultconfmapping="compile->master;runtime->default;sources->@;javadoc->@" and then just use dependencies with conf="compile,javadoc,sources" without unwanted extra mappings.

    I presume he means conf=compile;javadoc;sources but the idea was relevant. My bad

  • I agree that compile should be the default. Indeed I don't find a mention of compile(*) anywhere, I don't remember where that idea came from initially. The Ivy dependencies page doesn't seem to have it either.

  • I know of withSources and withJavadoc, and as far as I can see they function much in the same way like updateClassifiers -- they clone the default artifact and inject classifiers to create a new artifact, but that's not what I want as discussed before because it ignores configurations altogether.

  • I'm not suggesting that sbt do this lookup on classifiers ahead of time. It shouldn't matter to sbt exactly what configurations a dependency actually defines in their ivy file, and what artifacts it defines with what classifiers, until resolution. What I am suggesting:

    • There's a way to say "my sources maps to the dep's sources, or none" -- sources->sources(); & same for docs. With this, I wouldn't care if my dep doesn't define sources or docs, because I'd just do nothing then. I've just injected this into IvySbt.convertDependency and it seems to work: on update, it does get all my sources as well, ignoring deps which don't have either sources or docs defined; and if update were to only retrieve the necessary configurations to run (Configurations.defaultConfigurations) this looks like it would work just fine. Which leads me to:
    • Is there any reason update tries to resolve everything ? At the very least, sources,docs (not sure about pom) in Configurations.auxiliary should not make their way into ivyConfigurations by default. There could be an updateAuxiliary or something to update those.
    • Of course, this would be under the assumption that sources and docs always mean what they mean.
  • Not sure what to think of update having only one global definition regardless of the configuration scope -- it's probably ok to leave it as is if it got split in 2 as I mentioned before.

  • Regarding this:

    updateClassifiers can certainly add the contents of what you declare in the sources configuration with the assumption that you add these declarations.

    How? playing around with it, I found that it completely disregards my configuration. If I have a default artifact and another one like

<artifact name="myApp" type="jar" ext="jar" conf="default"/>
<artifact name="myApp" type="src" ext="jar" conf="sources" e:classifier="src"/>

then what I see updateClassifiers doing is creating a totally new artifact from scratch that will look like:

<artifact name="myApp" type="src" ext="jar" e:classifier="sources"/>

and I was baffled by this for a while. e:classifier would be constructed from the value in transitiveClassifiers and type=src ext=jar would just be defaults IIRC. I didn't see any way to use it to actually refer to my module's artifacts which I had defined under <publications>

  • We could add transitive dependencies, but this is certainly not mandatory (I could create a different issue for this in particular). It might have downsides - for external packages from Maven, this could bring in way more source jars than you want for a single dependency that you're using directly. However, inside an organisation, where you have complete control over how your Ivys will look like, the fact that you could resolve your own sources & docs configurations is enough - because you can then tailor your Ivys to include, or not include, further dependencies transitively.

@harrah
Copy link
Member

harrah commented Nov 26, 2013

  • Commas in configurations can be valid. They enable a shorthand in some situations, but I don't use or advertise them in sbt.
  • compile is a convention from Maven-land. Ivy doesn't have any conventions for configurations that I know of. It does have the special configuration default (at least I think it is special) that points to the default configuration.
  • I brought up with* only as an example of the user explicitly configuring sources/javadocs. I know you ultimately want configurations to work.
  • You suggested resolving classifiers ahead of time with "Thus it would be good if their sources artifacts (as deduced by updateClassifiers) were encoded in the generated ivy file." The generated Ivy file is created when the pom.xml is resolved and so that is when you have to check whether the artifacts exist.
  • Good to know that sources->sources() does what we want.
  • think of updateClassifiers as the second update that does what you want. It is named to only work with classifiers, but the IDE plugins all use it to get sources/javadocs. When I said "`updateClassifiers can add ..." I meant "we can make it add", sorry. It certainly doesn't do it now.
  • when I say add it transitively, I only mean for modules resolved via Ivy. If you want to focus on the situation where an organization controls all ivy.xmls and so transitive isn't necessary, that's fine. I do find that to be the more the exception these days. Most people seem to just have a filtering proxy.

@dansanduleac
Copy link
Contributor Author

  • i thought commas just group confs on either side of a ->, whereas ; separates these dependencies, like a,b->c,d;d would mean a and b both depend on the dependency's c and d, and that d depends on whatever defaultConfMapping says
  • yeah, after thinking through it I agree that you can't just put src/doc artefacts in the generated Ivys and assume they exist (initially I thought that would be the case but of course ivy complains when these don't actually exist)
  • ultimately, changing updateClassifiers seems like the way to go. However, and this is important:
  • I no longer believe it should use bespoke sources/docs configurations
    After talking to a colleague I realised that ivy configurations were not meant for this type of stuff. See the section "configurations of a module" in Ivy terminology
    Configurations were meant for grouping different sets of dependencies, a good example being: if you use slf4j for logging, you'd have 1 configuration per logger implementation (so log4j / util.logging / ...) that brings in the additional runtime dependencies for only that logger implementation. Note that you could have not just logging defined this way, but also db drivers, basically any API that has pluggable backends. By using the sources configuration to mean "all sources for this project", you can't express in ivy that sources should also depend on "the sources for whatever logging implementation I'm using", so its transitivity is inherently limited to fixed configurations
    • Instead, we should rely on the artifact type - have the main classes jars and their source artifacts and doc artifacts all be defined in the same configuration (default, or runtime), and differentiate them by type. I also think we should completely remove the Sources and Docs configs from sbt, and from Configurations.Auxiliary -- they do get published, but seem to serve no role whatsoever when retrieving artifacts.
    • An sbt setting would indicate which types should be treated as sources e.g. Set(src, source, sources) and which as docs, assuming that all the rest are for compilation, like the most common "jar" type.
    • update would only download artifacts whose types are not in the sets for sources and docs.
    • updateClassifiers would then retrieve the ones that are in those sets, and will know which is which, thus the editors will know as well.
    • the current stuff about assuming such artifacts exist with certain preset classifiers would be used as fallback, although ideally it would only run when we know that the ivy had been generated from a Pom (unless it's also helpful with misconfigured ivy files, but I assume most of those will define the source/doc artifacts).

Sorry for the sudden change of events, but hopefully you can see my point and that this is the recommended way to handle these kinds of artifacts.

@harrah
Copy link
Member

harrah commented Nov 27, 2013

  • Hmm, you are probably right about the commas.
  • This revised proposal generally sounds ok to me. However, note that the classifiers are the main use case. As I mentioned, there aren't that many pure Ivy users that I'm aware of. So, I agree that a) classifiers are always used when pom.xml is the origin b) never used if the ivy.xml declares sources/javadocs types c) but I think that at least for a (possibly long) transition period the other cases need to be pulled by classifiers for compatibility.

@harrah
Copy link
Member

harrah commented Dec 13, 2013

Fixed by #1016.

@harrah harrah closed this as completed Dec 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants