ajoberstar / gradle-git

Git plugin for Gradle
Apache License 2.0
561 stars 89 forks source link

Support Maven style SNAPSHOT versions #57

Closed quidryan closed 10 years ago

quidryan commented 10 years ago

The current InferredVersion doesn't seem to have the ability to have a stage/scope combination that ends with -SNAPSHOT. If I use build metadata, it'll always be appended with a "+", e.g. "+SNAPSHOT". If I name my stage SNAPSHOT, it'll get commit count appended to it, always, e.g. "-SNAPSHOT.3". I would have to use a stage of "final" to avoid the target version, but clearly that's not the best stage for a SNAPSHOT build.

ajoberstar commented 10 years ago

I think SNAPSHOT's would have to be handled as a special case to avoid the unique increment or commit count being added.

bmuschko commented 10 years ago

@ajoberstar Do you have a preferred way of implementing this feature?

ajoberstar commented 10 years ago

Initial thought was to add something inside the untagged stages block here something like this:

} else if (untaggedStages.contains(stage)) {
  if (stage == 'SNAPSHOT') {
    // set to SNAPSHOT
  } else {
    // use commit count
  }
} //...

That way people have to explicitly say that SNAPSHOTs are a valid stage for their project. Open to other thoughts though too.

bmuschko commented 10 years ago

@ajoberstar @quidryan I took a stab at this based on Andrew's comment (see the referenced commit). Could you both please have a look at it and discuss if that's how you imagined the feature to look like?

ajoberstar commented 10 years ago

That looks good to me. I hadn't planned on providing a method to add snapshot, but I could see that being a good way to make it clear its available as an option. Thanks!

quidryan commented 10 years ago

I would think another List called "snapshotStages" fits the general API better, I already get to specify my untagged and tagged as arbitrary names. It would be implied that snapshotStages are untagged.

bmuschko commented 10 years ago

@quidryan @ajoberstar Looks better? If yes, I am going to open a pull request.

quidryan commented 10 years ago

I think so.

bmuschko commented 10 years ago

OK, opened pull request https://github.com/ajoberstar/gradle-git/pull/62.

ajoberstar commented 10 years ago

Sorry for taking a while to get back to you guys on this. My main problem with the new approach is that untaggedStages and taggedStages are exclusive and complementary options, while snapshotStages is a subset of untaggedStages. It seems weird to put it at the same level in the API.

With the current API, the only option I can think of is a flag for uniqueUntaggedVersions which defaults to true. There would be no special handling of SNAPSHOT, users would need to provide that as an untaggedStage and decide that all of their untaggedStages would be non-unique.

However, I'm a little uncomfortable with non-unique versions, not that a commit count is is any guarantee of uniqueness. Since this is meant to be an opinionated plugin, I'm a little hesitant to rework the API to support SNAPSHOTs, which seem slightly at odds with semantic versioning.

What do you think of the uniqueUntaggedVersions approach? Any other ideas?

bmuschko commented 10 years ago

@quidryan Do you have any thoughts on this?

quidryan commented 10 years ago

SNAPSHOTs are a reality of the "maven repository" world, I think supporting them greatly broadens the applicability of this plugin. I definitely wouldn't encourage their use, but basic support would go a long way. My primary use of them would be for local builds, in which case just using the hash doesn't help because a commit is not happening on every build, additionally without SNAPSHOT we'd be building non-unique version without any indication in the version string that they're not unique (which is what SNAPSHOT provides). Our other use-case is our CI-builds, which can be so frequent that unique publication would overwhelm our binary repository, SNAPSHOTs help by indicating that one one of the multiple copies need to be kept.

BTW, snapshotStages could be exclusive to untaggedStages if that helps.

@ajoberstar With uniqueUntaggedVersions would I still be able to specify createBuildMetadata? I'd still want to be able to do this, since I squeeze feature branch names in here. Are you thinking that just appends SNAPSHOT to all untaggedStages? If so, that's fine by me.

ajoberstar commented 10 years ago

All good points. As an example of what I was proposing, you would configure something like below. All uniqueUntaggedVersions=false would do is turn off the commit count.

release {
  version {
    untaggedStages = ['SNAPSHOT']
    uniqueUntaggedVersions = false
  }
}

Without build metadata, you'd have something like 1.2.3-SNAPSHOT. With build metadata, you should have something like 1.2.3-SNAPSHOT+featurebranch.

Would that meet your needs? If not, do you have some examples of what you would want the versions to look like?

ajoberstar commented 10 years ago

To get around a bunch of these configurability options, I'm working through a new implementation of the release plugin. It still won't be the default to have SNAPSHOT's enabled, but it will be an option.

bmuschko commented 10 years ago

@ajoberstar Can you give us a update on where this feature stands at the moment? Is there anything I can help to get this in asap?

ajoberstar commented 10 years ago

I'm incorporating this into a rewrite of the release plugin in the new-release branch. I'm still finalizing the extension API and then need to add some test coverage. If you have any thoughts on the API as it sits, let me know. I'm hoping it won't be too much longer before it's ready.

ajoberstar commented 10 years ago

I still need to write up the documentation, but I have a test suite and the plugins done. There are two new plugins replacing the original:

Until I update the documentation, you'll probably have to dig around a little in the code to see how it works now.

Take a look at the Strategies class for some samples (including one for SNAPSHOTs).

ajoberstar commented 10 years ago

New version is 0.11.0-milestone.1+7d44943 (don't have the metadata for the plugin portal in there yet, don't plan on adding that until the release)

ajoberstar commented 10 years ago

I've added documentation in the wiki, but I still need to flesh out the Groovydoc for additional detail on the specifics.

I'm closing the issue for now, but feel free to reopen if you test and it doesn't meet your needs.

bmuschko commented 10 years ago

Thanks, Andrew. I played around with the latest version of your plugin in an example project. Is it expected that the SNAPSHOT strategy does not actually create a tag or is this a bug? Changing the strategy to FINAL does exactly that.

ajoberstar commented 10 years ago

Yes, that is intentional. See the GroovyDoc and source.

My reasoning is that, while repositories may make the snapshots unique via a timestamp, as far as Gradle is concerned they are not. However, it's fairly trivial to make it create tags:

// instead of 
defaultVersionStrategy = Strategies.SNAPSHOT
// use
defaultVersionStrategy = Strategies.SNAPSHOT.copyWith(createTag: true)