Skip to content

DynamicRepo - NextVersion.txt file when no /target branch arg #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dazinator opened this issue Jan 17, 2015 · 18 comments
Closed

DynamicRepo - NextVersion.txt file when no /target branch arg #351

dazinator opened this issue Jan 17, 2015 · 18 comments

Comments

@dazinator
Copy link
Member

When testing the new dynamic repo functionality (thanks for the new feature by the way!), I noticed that the NextVersion.txt file in my repo was not being honoured and therefore my attempts to bump the version number using that file were not working.

To replicate,

  1. Add a NextVersion.txt file to the target repo and set a bumped version number
  2. Execute GitVersion.exe using the -url -u and -p args which will invoke the new dynamic repo functionality in GitVersion.

Note that the version number GitVersion outputs won't be bumped as per the text file.

I believe if you also pass in an -targetbranch arg to GitVersion then it works but that's an assumption from reading the code not any actual testing.

@GeertvanHorrik
Copy link
Contributor

Let's explain how this works. You have a repository with a default branch. If you follow GitFlow, you probably have set the default branch to develop. If you haven't, you are probably on master. From now on, I will call the default branch default.

When you clone a dynamic repository, it checks the default branch (you can change that in the server settings, for example in GitHub). If you have pushed NextVersion.txt to the develop branch, but your default branch is master, it cannot find NextVersion.txt thus it will be ignored.

Please make sure that the NextVersion.txt is also available on the default branch of your repository. Otherwise it will only work when you specify the branch on which you committed the NextVersion.txt file.

As an additional note: adding NextVersion.txt to the master branch doesn't make much sense. I use that feature only to specify the next version on the develop branch so I can create 2.0.0-unstable123 for a repository that is on 1.2.0 on the master branch.

@dazinator
Copy link
Member Author

@GeertvanHorrik my default branch is master. It has a NextVersion.txt file. So I can't see that I have done anything wrong there.

If no one else can replicate this I will happily assume user error.

However the code you showed me here https://github.com/ParticularLabs/GitVersion/blob/ca002ed570b03f2f3a610523ab79adf34848fdc1/GitVersionExe/GitPreparer.cs#L69 seems to only do the checkout of nextversion.txt if there is a targetbranch arg present - and that is what i am seeing when testing.

@GeertvanHorrik
Copy link
Contributor

No, if you read the code correctly, it does this:

  1. Get repository (default branch) (note that this code should already have the latest head)
  2. If target specified, try to get the new head (will be null if nothing specified)
  3. If we have a new head, switch to it (will not be executed if null)
  4. Check out the file if it exists (always)

Can you write a failing unit test? Then I am willing to look into this.

@dazinator dazinator mentioned this issue Jan 17, 2015
@dazinator
Copy link
Member Author

Hmm, I am always willing to accept my own fallibility. But, I'm reading this:

            if (!string.IsNullOrWhiteSpace(arguments.TargetBranch))
            {
                    ... shortened for brevity...

                    repository.CheckoutFilesIfExist("NextVersion.txt");
             }

Irrespective of what I am reading, the issue was raised off the back of actually running GitVersion.exe locally - but I appreciate your caution - no one likes to have there time wasted.

There doesn't appear to be any existing test coverage for this otherwise you could just point to the existing test and tell me to run it - nevermind - I will have a go at creating one.

In order to carry out the unit test, i'll need the ability to commit a NextVersion.txt file to the local repo used for the unit test. I'll have to get familiar with libgit2sharp before I can do that - I'll work on that as soon as I can. If anyone knows of an existing function in the code base that commits a file to a Repo - i would be greatful if they could point me to that.

Cheers

@GeertvanHorrik
Copy link
Contributor

You are right, I missed the argument check at the top. I think it can be removed, let me double check.

@dazinator
Copy link
Member Author

Thanks @GeertvanHorrik - glad we are on the same page :)

I still think a test for regression purposes would still be handy as the NextVersion.txt file is a pretty important feature of GitVersion - but if you don't have time for that I can look into that as it will give me an excuse to get more familiar with the repo as well as libgit2sharp!

@GeertvanHorrik
Copy link
Contributor

btw. I do recommend to always pass in the branch name since you should never assume the default branch name of a repository. But will fix this anyway.

@JakeGinnivan
Copy link
Contributor

GitPreparerTests.WorksCorrectlyWithRemoteRepository is the test covering this.

We have a bunch of extension methods on the fixture.Repository.

As for NextVersion.txt this is being completely replaced by GitVersionConfig.yaml. So Whatever you do for NextVersion.txt please add support for GitVersionConfig.yaml at the same time. v3 is dropping NextVersion.txt and only using the config file

@dazinator
Copy link
Member Author

@JakeGinnivan - yep I found that test and those extension methods. I couldn't see anything that commits an actual file - but leave that to me, I only briefly looked at this stage. Thanks for letting me know about the yaml file I wasn't aware of that!

@GeertvanHorrik
Copy link
Contributor

The first fix is easy:

var targetBranch = arguments.TargetBranch;
if (string.IsNullOrWhiteSpace(targetBranch))
{
    targetBranch = repository.Head.Name;
}

Will look into GitVersionConfig.yaml support after that.

@JakeGinnivan
Copy link
Contributor

@dazinator wondering if you had time to create a wiki page for the dynamic repositories feature in general.

I think it would be handy to include tips on how to get the currently building branch names from different CI environments.

@dazinator
Copy link
Member Author

@JakeGinnivan - nope I have not yet created a wiki page for this feature, but I'll happily give that a shot!

I have been looking at the existing page here: https://github.com/ParticularLabs/GitVersion/wiki/Command-Line-Tool which does already describe these arguments, but I don't think there is anywhere that explains this particular feature in detail, and how, for example, it enables CI's to work that don't automatically checkout the repo - for example Team City builds that are set to "checkout on agent".

Are you thinking having some documentation page per CI system would be the way to go because I like that idea.

@JakeGinnivan
Copy link
Contributor

I was thinking have a Dynamic Respositories wiki page. We can link it from the command line args page and also the FAQ.

The info per CI system would be more how to get the branch name to pass to the argument so GitVersion calculates for the correct branch (otherwise it will always calculate master version even if building something else).

@dazinator
Copy link
Member Author

@JakeGinnivan - Ah Ok I Get ya.
I have created a wiki page here: https://github.com/ParticularLabs/GitVersion/wiki/Feature:-Dynamically-Obtained-Repository feel free to have a read and edit however you like.

I get the impression I may not understand the full implications of this feature just because my Git skills are weak. For example would it be common to have an scenario where you are building your repo - but you want the version number to be calculated from another repo or another branch? Because if that scenario is common in the Git world, then I can see how this feature might also enable that - I didn't want to speculate on this so the wiki page doesn't mention anything like this at present.

@GeertvanHorrik
Copy link
Contributor

I think the /b should nearly be made mandatory, one may never rely on the default branch.

@JakeGinnivan
Copy link
Contributor

For example would it be common to have an scenario where you are building your repo - but you want the version number to be calculated from another repo or another branch?

I don't think this would be common. This is mainly for build servers who do not check out a proper git repo.

Good work on the wiki page. I think we should say to always specify /b with note about the fallback.
Also should say something like ' in TeamCity the current branch can be accessed with the %system.current_vcs_branch% (or whatever the variable actually is)

Nice work

Sent from my Windows Phone


From: Darrellmailto:[email protected]
Sent: ý18/ý01/ý2015 16:22
To: ParticularLabs/GitVersionmailto:[email protected]
Cc: Jake Ginnivanmailto:[email protected]
Subject: Re: [GitVersion] DynamicRepo - NextVersion.txt file when no /target branch arg (#351)

@JakeGinnivanhttps://github.com/JakeGinnivan - Ah Ok I Get ya.
I have created a wiki page here: https://github.com/ParticularLabs/GitVersion/wiki/Feature:-Dynamically-Obtained-Repository feel free to have a read and edit however you like.

I get the impression I may not understand the full implications of this feature just because my Git skills are weak. For example would it be common to have an scenario where you are building your repo - but you want the version number to be calculated from another repo or another branch? Because if that scenario is common in the Git world, then I can see how this feature might also enable that - I didn't want to speculate on this so the wiki page doesn't mention anything like this at present.


Reply to this email directly or view it on GitHubhttps://github.com//issues/351#issuecomment-70414351.

@dazinator
Copy link
Member Author

Ok - I have made those changes that you both suggested. Feel free to make any further tweaks.

@JakeGinnivan
Copy link
Contributor

Going to close this.

See #367 for the enforcing the branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants