ihmcrobotics / ihmc-build

Composite build and IDE classpath seperation support for JVM Gradle projects.
Apache License 2.0
5 stars 4 forks source link

Support publishing to arbitrary urls, remove publishMode, add snapshotMode for CI #44

Closed calvertdw closed 6 years ago

calvertdw commented 6 years ago

Add vendor variable to publish to Artifactory proprietary-vendor and Bintray maven-vendor.

Maybe just add publishUrl override variable?

New property: snapshotMode=true for Bamboo (for dependency configuration only)

Else it's just gonna be the version that's declared.

New addPublishUrl() method:

ihmc {
   group = "us.ihmc"
   version = "0.1.0"
   vcsUrl = "some-url"
   openSource = true

   configureDependencyResolution()
   addPublishUrl(keywordOne, publishUrlOne)
   addPublishUrl(keywordTwo, publishUrlTwo, myUsernameProperty, myPasswordProperty)
   configurePublications()
}

So then those are used by calling publishToNameOne and publishToNameTwo.

Maybe add publishToBintray task to make more consistent? Gradle creates publish, which we've been using.

I think this is how it will go. User passes in: -PpublishUrl=<keyword|actualUrl> [-PpublishUsername=user -PpublishPassword=pass]

Where publishUrl is a keyword or an actual URL. There will be a few keywords predefined in configurePublications() with custom ones added with addPublishUrl(...):

Of course snapshotMode=true would do a whole host of other crazy stuff that I won't mention here.

publishToMavenLocal still works normally.

dljsjr commented 6 years ago

I think the solution should be flexible to allow for publishing to arbitrary targets. Maybe a closure that takes in collection of key:value pairs? key => name to use for a generated publish task and value => the Maven URL?

So a KV pair like "bintrayRelease", "https://bintray.com/ihmcrobotics/maven-release" would cause the plugin to create a publishToBintrayRelease maven publish task. Then you could make a list of such KV pairs in the ihmc block, and create a bunch of publish tasks.

Only problem I see with this is that we'd have to modify our Bamboo setup to call a specific publishTo[…] task instead of just using publish.

calvertdw commented 6 years ago

Bamboo is only publishing to snapshots repository, or proprietary-snapshots, only for security reasons for closed software. So, Bamboo wouldn't be a concern. publishMode gets overridden to snapshot.

I'm wondering if there is a use case for having more that one publish target declared at a time.

If you see my above solution, setting publishURL would set up gradle publish to go to that url when publishMode = release

dljsjr commented 6 years ago

I think it's good for the publish target to be a bit more explicit than having it magically change based on which "release" mode you're in. And it adds flexibility in the future if we end up in a situation where, say, we might have multiple redundant artifact hosts, or switch from one host to another, etc. But it's mostly the explicit thing. It's good for the user to know exactly where they are publishing to via the command they invoke I think. If the concern is that somebody might accidentally publish a release to a snapshot repo or vice versa then we could do validity checking based on release mode instead of target config based on release mode.

From the outside looking in it seems like a very low-cost feature to develop in exchange for a lot of flexibility but I might be underestimating the complexity involved. Like that whole thing about checking your release mode matches the repo type you're deploying to might actually be pretty hard.

calvertdw commented 6 years ago

What about an addPublishMode(name, url, username?, password?), where you set publishMode to the name it will use that one.

dljsjr commented 6 years ago

I still think it makes the plugin too opinionated to couple a publication target with the publication mode. If a user wants to send a stable release .jar somewhere other than Bintray they should be allowed to.

calvertdw commented 6 years ago

Okay I definitely see where you're coming from and I totally agree. I'm gonna pull #43 into this issue, since it's essentially a duplicate.

calvertdw commented 6 years ago

@dljsjr See my edits to the original post. Let me know how you like it.

dljsjr commented 6 years ago

Looks good. Just a note on implementation is that Gradle auto-creates a publishTo[…] for any maven targets that are defined. And gradle publish is just shorthand for "do all publications". The whole publishing -> publications -> mavenJava syntax is just a default but mavenJava is user defined. Adding multiple publish targets should be as straight forward as using the passed-in name to create multiple publications instead of having to do a lot of fancy dynamic footwork. For example, if a vanilla build script contained the following:

publishing {
    publications {
        theBestRepo(MavenPublication) {
            from components.kotlin
        }

        theWorstRepo(MavenPublication) {
            from components.java
        }
    }
}

Then you could separately run gradle publishToTheBestRepo or gradle publishToTheWorstRepo, and gradle publish does both.

TL;DR This is already built in to Gradle, the build plugin should just be a nice wrapper around adding new publish targets

calvertdw commented 6 years ago

Updated to reflect our conversation.

calvertdw commented 6 years ago

Should the composite publish task be called publishAll, compositePublish or something else?

It's currently called publishAll. But what is all? Maybe publish should just always publish everything under it and there is no special command. Unfortunately, I'm not sure if redefining publish is possible.

dljsjr commented 6 years ago

I think it depends on what it does.

If, for example, I go in to a project group like Open Robotics, and then I go inside a project like Java Toolkit, and I run gradle <the task>, what is the behavior?

If the behavior is to use the compositeSearchHeight property to publish all of Open Robotics even though I'm in Java Toolkit then I think we should call it compositePublish.

If the behavior is to publish everything related to Java Toolkit (in this specific example that would just be the main and test source sets) then publishAll is sufficient I believe.

calvertdw commented 6 years ago

If the behavior is to publish everything related to Java Toolkit (in this specific example that would just be the main and test source sets) then publishAll is sufficient I believe.

This is exactly what is covered by the standard publish task. It will by default publish all publications in the multi-project build. (ihmc-java-toollkit and ihmc-java-toolkit-test are configured as a multi-project build)

So it seems you would support compositePublish as calling publish on everything included by the compositeSearchHeight property. Which, in the case of your example, would publish ihmc-java-toolkit and only its dependencies, including transitives. So, you would almost never want to do that one.

I am having a thought that I wonder if aliasing certain compositeTask -PtaskName=blahs creates confusion, and it is better to always type it out. On one hand, typing it out may make users feel that it is not a well refined CLI, while aliases may seem magical and scary.

calvertdw commented 6 years ago

Fixed in 0.13.0.