first-parent problems - Githubissues

mandrachek commented 7 years ago

Since #46 was added, my builds are all messed up now.

We push code from development, to qc, to staging, and finally production. We add a tag when we're ready to increment the version number in development. That gets merged through the other branches.

git describe --first-parent -tags returns: on my development branch: 0.10.0-51 on my qc branch: 0.3.0-107 on my staging branch: 0.3.0-76 on my release branch: 0.3.0-71

The latest tag in my development branch is 0.12.0. But using --first-parent, it doesn't show up at all. When I merge code form development to QC, the new version is not picked up at all.

I'm going to have to roll back to an older version of the plugin (was using 0.5.2, which hopefully will still work), unless someone can explain how I can fix all my tags so that they will show up and work correctly across branches.

The purpose of this ticket is to request that either a CHANGELOG file be added to the project, or at the least notice in the README when potentially build breaking changes like this are made.

uschi2000 commented 7 years ago

Hej, thanks for reporting this. Can you provide a minimal test case that illustrates your branch and tag structure and the expected version strings in those situations? On Fri, May 5, 2017 at 8:13 AM Mark Andrachek notifications@github.com wrote:

Since #46 https://github.com/palantir/gradle-git-version/issues/46 was added, my builds are all messed up now.

We push code from development, to qc, to staging, and finally production. We add a tag when we're ready to increment the version number in development. That gets merged through the other branches.

git describe --first-parent -tags returns: on my development branch: 0.10.0-51 on my qc branch: 0.3.0-107 on my staging branch: 0.3.0-76 on my release branch: 0.3.0-71

The latest tag in my development branch is 0.12.0. But using --first-parent, it doesn't show up at all. When I merge code form development to QC, the new version is not picked up at all.

I'm going to have to roll back to an older version of the plugin (was using 0.5.2, which hopefully will still work), unless someone can explain how I can fix all my tags so that they will show up and work correctly across branches.

The purpose of this ticket is to request that either a CHANGELOG file be added to the project, or at the least notice in the README when potentially build breaking changes like this are made.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/palantir/gradle-git-version/issues/55, or mute the thread https://github.com/notifications/unsubscribe-auth/AGOdwYI5w1FW2pUATg1iElU2A_Lk_C4Eks5r2zxtgaJpZM4NSDH5 .

MatthewMaclean commented 7 years ago

I'm not sure --first-parent is the workflow we want to promote. The reason for adding this in https://github.com/palantir/gradle-git-version/issues/46 was for hotfixes. My understanding of gitflow is you only tag on master and not the hotfix branch. sources: 1, 2, 3. By tagging on master (rather than tagging the hotfix branch pre-merging into develop + master) you avoid changing the tag on develop to something like 1.0.0-hotfix-*.

The issue that --first-parent causes when following gitflow is that when you tag master and then merge that into develop, develop will not pick up the tag.

> mkdir t; cd t; git init
> touch a; git add .; git commit -m "first commit"; git tag 0.1
> git checkout -b develop; touch z; git add .; git commit -m "dev commit;
> git checkout master; touch b; git add .; git commit -m "second commit; git tag 0.2
> git checkout develop; git merge master;
> git log
_commit ac76f7b988b179330c95fa821754d6bd1690a378
Merge: f08e821 1c11d59
    Merge branch 'master' into develop

commit 1c11d5991288408f2ba87a2b171a4c37adc8f294
    second commit

commit f08e821b2e56c287ffb7f1bbe5eb31946799bf2c
    dev commit

commit f902c1858967ad5392cb35a63dd81f5015711f85
    first commit
> git describe --tags --first-parent
0.1-2-gac76f7b

By using --first-parent develop misses the 0.2 tag merged in from master. This would also apply to any feature branches.

PR with --first-parent removed: https://github.com/palantir/gradle-git-version/pull/64

Related open tickets to --first-parent: https://github.com/palantir/gradle-git-version/issues/63 - version of Git that supports this workflow https://github.com/palantir/gradle-git-version/issues/60 - README

If we do remove --first-parent not sure the best place to communicate the functionality change.

uschi2000 commented 7 years ago

I don't understand the example above: why are you interested in tags on develop if you're tagging and releasing on master?

uschi2000 commented 7 years ago

I believe that --first-parent is the standard way we use this plugin. Happy to make the behavior configurable though, defaulting to --first-parent.

MatthewMaclean commented 7 years ago

Products often release a latest which is based on develop. In this case you want a version on develop related to what's been merged in from master. My understanding is that this is the normal workflow vs managing tags independently on different branches with postfixes of "-dev" or "-hotfix". But that only matters if we are looking to take an opinionated approach and choose one. I'm happy to make this configurable as there are definitely different approaches to how tags are managed.

markelliot commented 7 years ago

Can you talk about your specific workflow? The ones we know about are supported well by this plugin.

On Jul 7, 2017, at 20:04, MatthewMaclean notifications@github.com<mailto:notifications@github.com> wrote:

Products often release a latest which is based on develop. In this case you want a version on develop related to what's been merged in from master. My understanding is that this is the normal workflow vs managing tags independently on different branches with postfixes of "-dev" or "-hotfix". But that only matters if we are looking to take an opinionated approach and choose one. I'm happy to make this configurable as there are definitely different approaches to how tags are managed.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub[github.com]https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_palantir_gradle-2Dgit-2Dversion_issues_55-23issuecomment-2D313766826&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=Vp81aOxWZvuVgVK_wp-VF3pIYG92B19LcCw6XKeYC0U&m=WPJW5nB8MO8LV9R0Pacna9gy12eh7Bl12gTcD5qkqNA&s=f1OWsCytoql6V00W-W1ZTRacTvFSajSghvFpeS0kf8E&e=, or mute the thread[github.com]https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AAp3Q78EBGv57AT4Iny1WxXSOuyTOje1ks5sLoETgaJpZM4NSDH5&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=Vp81aOxWZvuVgVK_wp-VF3pIYG92B19LcCw6XKeYC0U&m=WPJW5nB8MO8LV9R0Pacna9gy12eh7Bl12gTcD5qkqNA&s=44QJvRZDbTkhneWWrWipGVGhRouDHn-wE1f4X8i3n6c&e=.

MatthewMaclean commented 7 years ago

We manage tags on one branch (master) and merge it into develop to propagate it to other branches for usage. This allows us to release latest from develop and have it reflect the latest tags. What are the existing workflows + tooling that mesh with --first-parent that I can investigate transitioning to?

markelliot commented 7 years ago

Single primary branch, releases are tags that appear on that branch.

On Jul 8, 2017, at 00:36, MatthewMaclean notifications@github.com<mailto:notifications@github.com> wrote:

What are the existing workflows + tooling that mesh with --first-parent that I can investigate transitioning to?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub[github.com]https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_palantir_gradle-2Dgit-2Dversion_issues_55-23issuecomment-2D313816486&d=DwMCaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=Vp81aOxWZvuVgVK_wp-VF3pIYG92B19LcCw6XKeYC0U&m=xrkggVk8Y4Q6SySKV9bA41fV44n6FfozWoXmuli9Tt4&s=HRxmN2mH9TkBMMEBTEPDPlyCrzMs_q4EoKEZBW05egc&e=, or mute the thread[github.com]https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AAp3Q2ZuJdsPVIDJHV-5F22AF1F1la7ZZDks5sLsD7gaJpZM4NSDH5&d=DwMCaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=Vp81aOxWZvuVgVK_wp-VF3pIYG92B19LcCw6XKeYC0U&m=xrkggVk8Y4Q6SySKV9bA41fV44n6FfozWoXmuli9Tt4&s=AlEIRwditKi7tGjf6sslpHNVpVN6IuHqrdC83_WDMwA&e=.

nmiyake commented 7 years ago

I think we should try our best to be accommodating of different workflows, but would very much prefer not to make --first-parent configurable -- one of the major benefits we get from standardizing on gradle-git-version is that it produces well-defined, deterministic output for a given repository. Adding configuration that would make the output different based on it would make it such that this is no longer true, which is a pretty big cost in my eyes.

MatthewMaclean commented 7 years ago

Sorry for the late reply, closed my PR and will try out workflows that rely on --first-parent. If a similar style change occurs in the future to git-version, we should probably make it a major version change.

johnmshields commented 6 years ago

GitFlow is a very standard and widely used workflow. The use of --first-parent breaks this workflow and any other workflow that tags a release branch rather than the trunk (develop/master/etc.).

I'd like to also request that the use of --first-parent be configurable as well.

Thanks!

P.S. Happy to provide a PR if the maintainers agree to make it configurable.

peterfortuin commented 6 years ago

For me this is not workable. I would like to be able to configure the --first-parent behaviour. Until this is in I will remain at version 0.5.1.

nmiyake commented 6 years ago

Thanks for the feedback. As long as we keep the --first-parent behavior as the default, I think that, based on this feedback, exposing the ability to use configuration to turn this behavior off is not unreasonable.

@uschi2000 @iamdanfox any strong objections to this? I hold that the goal of gradle-git-version is to take a repository and generate a version in a known manner that conforms to a certain specification. I think it also makes sense for us to have an opinionated recommendation for how to tag releases etc. and to have the behavior of this plugin work with that opinionated recommendation by default.

However, I think it is a reality that requiring a specific mechanism for tagging and managing releases may be too restrictive, and exposing a limited set of knobs to allow this plugin to generate well-formed versions for repositories that use different (but still "sane") practices may make sense.

iamdanfox commented 6 years ago

@nmiyake my concerns here are:

internally we do not recommend gitflow for a few reasons:
- its long lived master & develop branches do not promote a world in which we can release often and at short notice.
- We also prefer small PRs which are squashed onto develop instead of gitflow's recommendation to use merge commits.
- I also have concerns about gitflow's complexity (which would lead to needless differences between teams),
- gitflow means you can't push push one button and get a release
- gitflow requires adjusting protected branches & required CI checks for every release/X.Y.Z branch
for the above reason, I'd prefer to continue to promote the --first-parent behaviour internally
every piece of additional logic we add to this plugin needs to be rock-solid, well tested and maintained (the jgit stuff is already scary enough).

Proposal

What if we published two plugins that both expose the same gradle API (versionDetails() etc), just with differing internal implementations:

com.palantir.git-version     // `git describe --tags --first-parent --always --prefix...`
com.palantir.gitflow-version // `git describe --tags --always --prefix...`

johnmshields commented 6 years ago

Thanks for taking the time to respond.

Branching models are darn close to religious opinions... :) So I don't want to belabor any debate about which model is "best". Also, I don't actually disagree with your points but there are many teams that use gitflow successfully. Also, several of your points don't apply to many teams and systems that don't share the exact same goals and priorities you mention.

On the technical side, I will note that the use of --first-parent also breaks the common branch for release and github flow models as well. AFAICT, using --first-parent only works if you tag your primary branch. Tagging releases on any other branch seems to break. So it only supports full trunk based development with tagged releases on the trunk.

The request here is to simply provide:

version = gitVersion(firstParent: false)

I'll add that what I would want aligns with git describe --tags --always --prefix.... It's only the --first-parent that is the problem.

Again, especially with groovy it's easy enough to fork out and run git describe in our build. But I like the ability to not require git in the path and I also like the ease your plugin provides to get the other version details.

Thanks!

nmiyake commented 6 years ago

@iamdanfox I agree with with points 1 and 2, but would argue that keeping --first-parent as the default behavior basically satisfies both of those cases -- keeping the flag as default will promote this as the recommended/endorsed behavior. 99% of users will just apply the plugin, get this expected behavior, and won't give it a second thought.

I agree with point 3 as well, but am not sure that releasing a separate plugin (com.palantir.gitflow-version) really addresses it. I would fully hope that we would maintain the same quality bar/standards for any com.palantir.gitflow-version plugin we would release, so it's not like making that plugin separate would lower our support burden. If a project owner applies this plugin to their project and they run into this issue, if our recommendation is "use this other plugin" versus "configure this setting on this plugin", I don't see what that buys us.

If we want to double down on the dogma and say "we believe --first-parent is the only valid behavior so that's all we're going to support in an effort to force people to use that release model", that's fine. However, if we're willing to concede that we may want to support other release models, then at that point I feel like exposing that configuration in this plugin makes more sense than publishing a separate plugin. Is there some other advantage of publishing a separate plugin that I'm missing?

iamdanfox commented 6 years ago

If we're gonna implement both in this repo, my preference for a separate plugin is:

it's easier to run automation internally to just grep all repos for com.palantir.gitflow-version and open PRs to switch to the recommended one
the decision to use first-parent or not seems like a repo level decision. People shouldn't be able to invoke versionDetails() / gitVersion() in a few places in their repo and accidentally forget to use the firstParent: false, e.g.:

version gitVersion(firstParent: false)

// ... some other file:
task publishSomething() {
    url "foo/bar/{versionDetails().gitHashFull}/some-artifact.json" // should be fine
    doFirst {
      println "publishing ${versionDetails()}" // 🌶 different to above
    }
}

nmiyake commented 6 years ago

Ah OK if there's not a way to enforce the setting "globally" on the plugin then I'm convinced.

johnmshields commented 6 years ago

A few comments:

It would be perfectly reasonable to use --first-parent on master for example, and not on develop. I'd say it is more of a branch level decision than a repo level decision.
Can you clarify why the git hash (versionDetails) would be impacted by the git describe (gitVersion) call? I see these as completely separate and the firstParent option only applying to gitVersion.
I agree there is a valid use case for making the setting "global".
- This could be done with standard gradle properties files to set it. But I also think this is out of scope for this particular ticket.
- However, I'm also not sure that the plugin authors should be concerned with the DRY-ness of my build script. If I do it wrong it and miss it somewhere I'll fix it. If I call a function with different arguments I should expect different results... ;)
- The same logic/argument applies to the current prefix argument AFAICT.
To be clear, the only option I'd like to see is the ability to switch the arguments to git describe to omit --first-parent. I don't see how that warrants a separate plugin.
I'd also want to have the option to use --first-parent, BTW! I don't think these things are as mutually exclusive as you are arguing.

Thanks again for a helpful plugin and considering these updates.

iamdanfox commented 6 years ago

Ah sorry you're absolutely right, gitHashFull would always be the same! Have updated the snippet above. My ulterior motive here is that I also don't like that prefix configurability (only two repos use this internally) and would kinda prefer for a versioning plugin to be a zero-config affair. This would then mean the 'configurability' you mention is just choosing which plugin you want to apply. Both would comply to the same interface (expose gitVersion() and versionDetails() functions)

dhs-rec commented 3 years ago

+1 for having --first-parent usage configurable.

dhs-rec commented 2 years ago

Another year has passed (nearly). Is this going somewhere?

pgodzwa commented 2 years ago

+1, would be great to have it implemented

palantir / gradle-git-version

first-parent problems #55

Proposal