Resolving Pull Requests in TeamCity context

[nulltoken]

Fix #31

[nulltoken]

@andreasohlund @SimonCropp

I've started to work on fixing #31. In order to not introduce any regression, I've chosen to first cover the existing GitFlowVersionFinder implementation with tests.

In order to work against a coherent repo, I've reproduced the layout of the describe repo in the article A usccesssful Git branching model and created a test asbm test repository.

Using the existing code, I've covered the retrieval of a version from the existing Master/Develop branches and from a new Feature.

I can't make the test covering the creation of a new Hotfix branch pass (most probably because I don't completely understand what's the lifecycle of a hotfix branch).

I've reproduced the behavior that one can observe in TeamCity when dealing with a pull request pointed at by a detached HEAD.

I'm going to go further with this, but In order to do it correctly, I'd need you to help me

• Making this hotfix test pass by pointing at what's incorrectly set in my test
• Adding some more passing tests by describing me the corner case usages and expectations

FWIW I'll rebase those new tests to come so that they appear before the future refactoring.

[nulltoken]

A hotfix is always branched from the master

@andreasohlund I've fixed this. The hotfix is created from master and named hotfix/1.0.2

This is still failing with the following message

System.Exception : There must be a tag on a hotfix branch with a version the same as the version from the branch name i.e. 1.0.2

Creating an annotated tag named 1.0.2 pointing at the tip of the hotfix branch fails with the following message

System.Exception : If a stability is defined on a hotfix branch the pre-release number must also be defined.

[andreasohlund]

You need to name the tag 1.0.2-Beta1

@SimonCropp didn't we remove this requirement?

Sent from my iPhone

On 17 nov 2013, at 19:51, nulltoken notifications@github.com wrote:

A hotfix is always branched from the master

@andreasohlund I've fixed this. The hotfix is created from master and named hotfix/1.0.2

This is still failing with the following message

System.Exception : There must be a tag on a hotfix branch with a version the same as the version from the branch name i.e. 1.0.2
Creating an annotated tag named 1.0.2 pointing at the tip of the hotfix branch fails with the following message

System.Exception : If a stability is defined on a hotfix branch the pre-release number must also be defined.
—
Reply to this email directly or view it on GitHub.

[andreasohlund]

Some more background on that tag requirement:

We couldn't find a stable way to count the "number of commits since the hotfix branch was created" so we enforced a tag pointing to the branch point.

Ie tag= 1.0.2-Beta1 => first build == 1.0.2-Beta1 and as more commit are added 1.0.2.Beta1.NumberOfCommitsAheadOfTHeTag

That said we agreed to relax this and just do a 1.0.2-Alpha0 for the first one and then let the users tag as needed.

[andreasohlund]

So I guess if you have any ideas on how we can reliably count number of commits since the branch got created without using the reflog you're our hero!

[nulltoken]

@andreasohlund The hotfix test now pass. Thanks for the support!

So I guess if you have any ideas on how we can reliably count number of commits since the branch got created without using the reflog you're our hero!

I'm not sure if that's what you're after, but I've added 56d953e which performs this count in the hotfix branch. Maybe have you met some more complex cases where that wouldn't work? If that's the case, I'd be interested if you could share those with me.

[andreasohlund]

Ifaik(simon?) we had issue when we lost count if a release branch was merged back to develop before the release was completed. This obviously is no issue for hotfix branches

Sent from my iPhone

On 17 nov 2013, at 21:07, nulltoken notifications@github.com wrote:

@andreasohlund The hotfix test now pass. Thanks for the support!

So I guess if you have any ideas on how we can reliably count number of commits since the branch got created without using the reflog you're our hero!

I'm not sure if that's what you're after, but I've added 56d953e which performs this count in the hotfix branch. Maybe have you met some more complex cases where that wouldn't work? If that's the case, I'd be interested if you could share those with me.

—
Reply to this email directly or view it on GitHub.

[andreasohlund]

@SimonCropp can you review and pull?

[nulltoken]

This one is still a WIP. It's not ready to be merged yet.

#36 should be ready to be reviewed, though.

[andreasohlund]

Sorry, got the numbers mixed up:)

On Thu, Nov 21, 2013 at 7:47 PM, nulltoken notifications@github.com wrote:

This one is still a WIP. It's not ready to be merged yet.

#36 #36 should be
ready to be reviewed, though.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/32#issuecomment-29011744
.

[nulltoken]

Ok, so I've been thinking a bit about this. Below my current proposal:

Proposed changes

• Update the task so that, when being run in a TeamCity environment
  • Ensure that one (and only one) remote is defined
    • None or more -> Fails
  • Perform a Fetch (refs/*:refs/*). This will retrieve all refs (branches and potential pulls)
  • Create missing local branches from remotes branches
  • If HEAD is detached
    • Find among the refs which one points to the commit
      • None -> Fails
    • Use the first ref and make HEAD point to it
• Drop Repository.GetBranch() extension and make xxxFinder types only work against a local repository

Those changes should lead to the following benefits:

• Ease the testability of GitFlow (by making its behavior more predictable/focused)
• Remove the need to manually update the environment variable from the build property (ie. Fix Fix the pull request hack needed to build pulls in TeamCity #31)
• Circumvent TeamCity behavior which only fetches the monitored branch
• Prepare the terrain for later enforcing some constraints (Repository topology constraints related questions #37)

[nulltoken]

Use the first ref and make HEAD point to it

Another option would be to fail if we find two refs pointing to the commit being built. Although quite unlikely, this may occur, for instance, if two pull requests are opened targeting the same commit. Maybe failing in this case would be more "safe" rather than generating a potentially wrong version...

[andreasohlund]

Sounds good!!

Sent from my iPhone

On 21 nov 2013, at 20:34, nulltoken notifications@github.com wrote:

Use the first ref and make HEAD point to it

Another option would be to fail if we find two refs pointing to the commit being built. Although quite unlikely, this may occur, for instance, if two pull requests are opened targeting the same commit. Maybe failing in this case would be more "safe" rather than generating a potentially wrong version...

—
Reply to this email directly or view it on GitHub.

[nulltoken]

So I think this should now be ready for review

• Add asbm test repository which reproduce the layout from http://nvie.com/posts/a-successful-git-branching-model/
• Add some tooling in order to ease working against temporary git repositories
• Ensure basic topology constraints
  • The processed repository bears both "master" and "develop" branches
  • A feature branch is always created from the develop branch
  • A hotfix branch is always created from the master branch
  • A pull request is always created from the develop branch
• When running under TeamCity, ensure the processed repository knows exactly one remote
• When running in standard mode, ensure the processed repository HEAD isn't detached
• When running under TeamCity, perform some normalization
  • Dynamically create local branches from remote tracking references
  • Create fake local branch when building a GitHub Pull Request
    • Fetches the remote tips to find which reference points to the commit pointed at by HEAD
    • Throws if not exactly one points at it
    • Throws if doesn't look like a valid pull request
• Drop .GetBranch() extension method
• Drop Teamcity.IsBuildingAPullRequest() method
• Drop Teamcity.CurrentPullRequestNo() method
• Make FeatureVersionFinder no longer depend on a local ref log
• Tests

Note: Some tests previously relying on MockRepository have been rewritten in order to benefit from built-in LibGit2Sharp capabilities (identification of ancestor, for instance).

Things to maybe consider later:

• Remove the tagging requirement of a hotfix branch by leveraging the CommitLog querying capabilities of LibGit2Sharp to count the commits on the hotfix branch
• Currently, when GitFlowVersionFinder.FindVersion() hasn't been able to detect which kind of branch the HEAD points to, it defaults to FeatureVersionFinder.FindVersion(). However, this behavior isn't covered by any test. How about throwing an exception?

[andreasohlund]

Regarding relaxing that tagging requirement: #33

[andreasohlund]

Raised and issue for throwing when the branch type can't be detected

#43

[andreasohlund]

@nulltoken seems like there is test failures on the buildserver http://builds.particular.net/viewLog.html?buildId=28556&buildTypeId=GitFlowVersion_1Buil

[nulltoken]

@andreasohlund

Can you please revert this merge while I investigate?

Off the top of my head:
Two kinds of failures:

• GitFlowVersion behaves now differently when running on a TeamCity agent. 10 failures out of 18 can be explained because of this. I'll work on a proposal to make the GitFlow tests able to run without any interference even when being run in a TeamCity agent.
• Checkout() related failures. Those one bother me a bit more. Would there be a way to actually make the CI server monitor the GitFlowVersion pull requests as well? That would be very helpful to remotely troubleshoot such kind of issues.

[andreasohlund]

I've enabled pull building and GH feedback now

[nulltoken]

I've enabled pull building and GH feedback now

Awesome!

@nulltoken seems like there is test failures on the buildserver http://builds.particular.net/viewLog.html?buildId=28556&buildTypeId=GitFlowVersion_1Buil

Those failures are now fixed by #45