git-commit-id / git-commit-id-maven-plugin

Maven plugin which includes build-time git repository information into an POJO / *.properties). Make your apps tell you which version exactly they were built from! Priceless in large distributed deployments... :-)
http://www.kto.so
GNU Lesser General Public License v3.0
1.61k stars 298 forks source link

Git describe command prefers the newer tag if more than one is on the same hash #39

Closed fredcooke closed 11 years ago

fredcooke commented 12 years ago

And you take the oldest or first. I much prefer the newest one because then I can do something tricky:

Let maven tag as artifact-version during release, release gets build with git describe showing release version, etc. Tag on the same hash after maven is done as the current/next snapshot version, future versions will include the snapshot tag in describe output so as not to confuse users "Yeah, I'm on 0.0.2" when they really mean "I'm on 0.0.3-SNAPSHOT-38commits-some hash".

ktoso commented 12 years ago

Tricky use case indeed! Definitely makes sense to use the newest then :)

fredcooke commented 12 years ago

Right, because if they say "I'm on 0.0.3" you can say "No, you're not! Tell us more!" ;-)

ktoso commented 12 years ago

Note to self, the contract here is:

For each committish supplied, git describe will first look for a tag which tags exactly that commit. Annotated tags will always be preferred over lightweight tags, and tags with newer dates will always be preferred over tags with older dates. If an exact match is found, its name will be output and searching will stop.

I've started hacking on it. It'll take a bit though, so it won't end up in a total mess ;-) Thanks for pointing out this contract for tags :-)

fredcooke commented 12 years ago

Hmm, according to that a tag that is newer, but further away would be preferred. Is that incomplete? For example, if I am manually tagging, and forget to tag a version for some reason, and retrospectively tag it later, after a newer release, will I get the older one or the closest one?

fredcooke commented 12 years ago

The missing text from man git-describe is below:

"If an exact match was not found, git describe will walk back through the commit history to locate an ancestor commit which has been tagged. The ancestor's tag will be output along with an abbreviation of the input committish's SHA1."

fredcooke commented 12 years ago

This is now officially a blocker for me to push commits. My old tags are standard and make for a super ugly title bar, and the snapshot tags don't show up in describe output, yet :-) I'll keep working privately and rebase -i my work once 2.1.1 is available. If you have snapshots you'd like me to test for you pre releasing that, let me know. Cheers.

ktoso commented 12 years ago

Hey and thanks for the feedback. I've been silent this week because of a long-awaited vacation. I'll be back hacking on this during the next week - Thanks for testing it out and letting me know about that. It's a priotity on my todo's.

fredcooke commented 12 years ago

Sorry! I didn't mean to pester you on a holiday! Enjoy the last of your well-earned time off :-)

ktoso commented 12 years ago

Not a all - I'm quite happy bout people really using/reporting stuff about software ;-) Just made the holiday more awesome. I'll be back soon and push this forward fast :+1:

fredcooke commented 12 years ago

I totally understand! It's hard to get people to complain about stuff! Seems strange, but it's true. I blame the developers that cry and complain when they hear that, god forbid, their work isn't perfect ;-)

fredcooke commented 12 years ago

I likely shouldn't be using vim+browser to do this, but I can't see how to get a date out of the refs. This is probably due to the fact that my git knowledge is purely superficial, unlike yours. If you have pointers, I can take a crack at this too.

fredcooke commented 11 years ago

What I'm going to do for this, unless you find some time before I get around to it, is a dirty dirty hack, and release my own 2.1.0.1-HACKED version on my own repo for my use. My hack will be to look at the tags and prefer the one with SNAPSHOT in the name over one without. That will give the correct behaviour in my use case, but is dirty dirty DIRTY. If that's the way it's gotta be, no stress. I also need to release private versions of a number of other plugins in a similar way with various fixes, so don't feel bad! Thanks for an awesome tool :-)

ktoso commented 11 years ago

Hey Fred :) I'm currently (and have been doing so the week before too) traveling and giving some talks at conferences so didn't get time to work on it. The good news though is that that conferences "iteration" is over this weekend, and I'll be mostly free to work on the plugin again :]

Thanks a lot for the feedback - it really keeps me motivated and really glad I open source stuff. :-) (Nice to have some chit-chat in issues now and then too ;-))

Let's see how long I'll be keeping you on a "-hacked" fork - I hope not too long ;]

fredcooke commented 11 years ago

No stress! :-) The best part of writing software is seeing people use it and like it. Yours does something that I need, so I'm happy! I'm also fussy! But not too fussy, hence -hacked :-)

ktoso commented 11 years ago

Moving to 2.1.3, 2.1.2 included an easier fix.

fredcooke commented 11 years ago

I'm glad that it's not JUST me that found that API hard to figure out! Looking forward to 2.1.3 :-)

BenFenner commented 11 years ago

Hey ktoso, just making sure you don't forget about this. I get to play again on my favorite app once you fix this. =D

fredcooke commented 11 years ago

You know, I had a friend look at this, and he couldn't reproduce it. So I just tried myself, and neither can I now! Hmmm. I wonder if you changed something in between. I'm on a newer version than I was then.

fredcooke commented 11 years ago

This IS still an issue. I had a code one-liner fixing the issue for me. Just remembered it, commented it out, and yes, it's broken. This is, therefore, still a blocker for me using this for production builds. Let me know if I can assist.

fredcooke commented 11 years ago

The reason I couldn't reproduce is this:

I was trying with 0.0.X and 0.0.Y-SNAPSHOT where Y = X + 1 and where the snapshot tag is more recent.

This case works. What doesn't work is the more general case:

Name-0.0.X vs 0.0.Y-SNAPSHOT - in my case name starts with O, but I suspect letters beat numbers in ordering, somehow.

Hope this helps! :-)

PS, I pinged @MrOnion about it too. Maybe he can help again :-)

ktoso commented 11 years ago

I think I'll finish it today. The "get the most recent" logic is done, having tests fail on "commits since last tag" lookups.

BTW: collections ops in java are a pain... Still thinking about migrate the project to Scala.

fredcooke commented 11 years ago

Fantastic, I really appreciate your time and effort! :-) I also really appreciate the test-first approach. It gives me faith!

Re scala: if you migrate it then I'll still test, however it'd be nice if there was a pre-migration version in central with most stuff fixed first, just in case the migration goes haywire ;-)

ktoso commented 11 years ago

I've just pushed an impl I believe to be valid. Before I push the stable version, I'd like you to check it out on your project where you encontered this issue @fredcooke :-)

A snapshot with the latest code is available here: http://oss.sonatype.org/content/repositories/snapshots/pl/project13/maven/git-commit-id-plugin/2.1.4-SNAPSHOT

fredcooke commented 11 years ago

I'll get right on it! :-)

fredcooke commented 11 years ago

I got this output, but still a wrong result:

datedRevTags = [DatedRevTag{id=8ac9446fc4b495775f445335c92cc2bd113945d5, tagName='0.0.1-SNAPSHOT', date=13 November 2011 6:36:59 PM}]
datedRevTags = [DatedRevTag{id=db075c9cbdfe9b43b6ff0a50343d91705bdeed4d, tagName='0.0.3-SNAPSHOT', date=6 September 2012 5:49:10 PM}]
datedRevTags = [DatedRevTag{id=05f364326dcf3ae8f93638c7c021bcfd396706fb, tagName='0.0.2-SNAPSHOT', date=20 November 2011 4:17:34 AM}]
datedRevTags = [DatedRevTag{id=8fcfe7bcef73dfcefa07155b7b947e64b214597a, tagName='OpenLogViewer-0.0.2', date=27 December 2011 1:22:41 AM}]
datedRevTags = [DatedRevTag{id=79e8a33f0276db00174d6aa25a22521c1dbb1dad, tagName='OpenLogViewer-0.0.1', date=20 November 2011 2:53:54 AM}]

But still this:

<entry key="git.commit.id.describe">OpenLogViewer-0.0.2-138-g0162ccf-DEV</entry>

I double checked that the tags both point at the same hash, they do.

Dates look good, though, so you MUST be close! :-)

SHA1 of plugin, to be sure to be sure:

12b0448ca90b79f103d11f29bc579f3554511f4a  /home/fred/.m2/repository/pl/project13/maven/git-commit-id-plugin/2.1.4-SNAPSHOT/git-commit-id-plugin-2.1.4-20130113.174454-1.jar
ktoso commented 11 years ago

Now this has been a really interesting bug!

Turns out Tags may also point to Tags. As seen here:

[info] Resolved tag [0.0.3-SNAPSHOT] [...], points at [tag 8fcfe7bcef73dfcefa07155b7b947e64b214597a ------] 

In this case, our "group by" doesn't make sense since the id here is a tag id, and not the commit id we need. A simple unwraping until we get the commit id solved this edge case.

Thanks a lot for the testing, I'll push to MC soon. :+1:

fredcooke commented 11 years ago

You're a hero! Thanks for all of your work on this! :-)