Exclude LibGit2Sharp from ILRepack in NuGet.CommandLine #890

eatdrinksleepcode · 2016-06-03T05:37:27Z

Fixes #854

This is a short-term fix for NuGet.CommandLine on Mono. It excludes LibGit2Sharp from ILRepack, and copies LibGit2Sharp (and its config file) into the package instead. Only NuGet.CommandLine is affected; changing the other packages build by GitVersionExe might (or might not) make sense, but that decision is being deferred to get this change in, in order to unblock other work.

asbjornu · 2016-06-03T07:01:21Z

@eatdrinksleepcode Thanks! Do you think it's possible to test the output from this somehow, so we know that the final merged and package .exe works on all platforms?

eatdrinksleepcode · 2016-06-03T19:05:53Z

That's actually a bit of a complicated question.

We are a bit lucky in that regard on the Windows side: there is already a test of this package happening during the Windows build as a consequence of GitVersion dogfooding itself. If you look at the AppVeyor build for this PR:

Line 38 - solution is built
Line 854 - GitVersion.exe from NuGetCommandLine is executed to update GitVersion's version number
Line 955 - solution is built again with the new version number

If I run that same process locally after breaking the package (by commenting out line 154 of GitVersionExe.csproj so that the LibGit2Sharp files are not copied into the package), the build correctly fails; when I fix the package, it succeeds. So that confirms that the package works correctly on Windows.

Testing as part of the Linux build is unfortunately not possible right now. Package building currently does not occur during the Linux build at all; I don't know why it was disabled originally, but when I try to re-enable it, ILRepack fails with a NullReferenceException. I have debugged into that and found that the model that Mono.Cecil is generating for one of the methods in GitVersionCore.dll has a null operand for a brtrue.s instruction, which should not be possible. The IL for the method in question is correct, and monodis represents it correctly when disassembling it. So there seems to be a bug in Mono.Cecil, but why it affects this particular instruction I don't know yet. I am going to look further into that now that I have this PR up :)

In the meantime, I have manually validated the new package by installing it from the Windows build into a repository on OSX and confirming that it works correctly (and also that it doesn't work when I intentionally break it). Until we enable package building on Linux, I think that's the best we can do.

asbjornu · 2016-06-06T16:04:43Z

@eatdrinksleepcode I would love to await the work you do in #891 and have that merged as an updated Fody and working Linux build first, so we can get a test in this PR to ensure this functionality never regresses on Unix/Linux.

eatdrinksleepcode · 2016-06-09T17:12:22Z

I added a commit which fixes #891 (by updating the Fody.Caseless version), builds packages on Linux, and dogfoods the CommandLine package during the Travis build (just like the AppVeyor build). However, that dogfooding is failing during the Travis build (see below) for reasons that I don't understand and cannot reproduce locally. Can someone who knows more about the implementation details of GitVersion take a look?

asbjornu · 2016-06-15T09:37:25Z

@eatdrinksleepcode Awesome work! 👍 😄 I've run the test locally and don't see anything wrong there either, so it's just weird that Travis is failing the build. Do you think you can add a commit or do something to trigger a new build, so we can see if the error was temporal? If it's not, I'd rather not merge before we figure out why Travis is failing, since that would make all builds fail going forward.

eatdrinksleepcode · 2016-06-15T13:58:41Z

I triggered a new Travis build twice, once by force pushing a no-edit amend, and again by rebasing on master. Both Travis runs failed before starting the actual build while installing Mono with an error about gpg keys. Looks like something is wrong with the Travis setup. I will try again later today.

eatdrinksleepcode · 2016-06-15T20:47:22Z

The Mono/gpg problem didn't recur, but now the previous error is happening again when running GitVersion within the build.

asbjornu · 2016-06-16T13:14:44Z

@eatdrinksleepcode Ok, looks like this is what is failing the build:

mono ./build/NuGetCommandLineBuild/tools/GitVersion.exe -l -console -output buildserver -updateAssemblyInfo

It doesn't seem like -l console is outputting anything to Travis's log, so perhaps we can pass a -l <path> argument in there instead and then do cat <path> after? We need to know why that command fails to fix it.

eatdrinksleepcode · 2016-06-20T06:16:47Z

Adding some logging has revealed multiple problems. First, Travis performs a shallow clone, which is why GitVersion can't find some of the objects it is looking for (possibly something GitVersion should try to detect). Unfortunately there is no way to tell Travis NOT to perform a shallow clone; but I was able to perform a git fetch --unshallow which resolved the problem.

The next problem is that GitVersion is detecting a detached HEAD on Travis, whereas it detects the PR branch in AppVeyor. I can reproduce this behavior on my own machine by following the same sequence of Git commands from the build log. The strange thing is that it also reproduces locally when following the sequence from the AppVeyor log. So at this point I am not sure why AppVeyor works, or what to do to the Travis build to make it work also.

JakeGinnivan · 2016-06-20T06:52:13Z

With AppVeyor (and teamcity) the branch name is available through a variable.

GitVersion has a feature called dynamic repositories, it will checkout the git repo into a temp folder on the machine, it means that GitVersion doesn't need a git folder which the build server has created. We may have to use that on travis?

eatdrinksleepcode · 2016-06-20T15:13:10Z

GitVersion has a feature called dynamic repositories

That is a potential alternative to running git fetch --unshallow, right? I don't know enough about how dynamic repositories work to know which option is better; I am happy to implement either one for the Travis build. The --unshallow seems to be working at the moment.

With AppVeyor (and teamcity) the branch name is available through a variable.

So is the best option to implement Travis as a supported CI server in GitVersion?

eatdrinksleepcode · 2016-06-20T15:20:47Z

I'm a little confused because the TeamCity implementation of BuildServerBase overrides GetCurrentBranch to get the current branch from the "Git_Branch" environment variable. But AppVeyor doesn't override GetCurrentBranch; so how is GitVersion getting the branch from an environment variable in AppVeyor?

gep13 · 2016-06-20T19:50:03Z

@asbjornu said...
It doesn't seem like -l console is outputting anything to Travis's log, so perhaps we can pass a -l argument in there instead and then do cat after? We need to know why that command fails to fix it.

This sounds like the same reason that the Cake Build PR that I created isn't building correctly on Travis.

asbjornu · 2016-06-21T11:28:49Z

@eatdrinksleepcode:

So is the best option to implement Travis as a supported CI server in GitVersion?

Yes, I believe so.

... how is GitVersion getting the branch from an environment variable in AppVeyor?

Good question. @JakeGinnivan, do you know?

@gep13:

This sounds like the same reason that the Cake Build PR that I created isn't building correctly on Travis.

They might be related. I haven't looked into why the Cake build PR is failing, but it might be worth rebasing once this PR is merged to see if that helps.

JakeGinnivan · 2016-06-29T09:38:48Z

how is GitVersion getting the branch from an environment variable in AppVeyor

Actually looks like it isn't :P

Replace slash with dash in the command line argument switches for gitversion.exe, so it is more Unix friendly. It will still fail exeucting on OS X due to GitTools/GitVersion#890, but once that has been fixed, this should ensure that GitVersion executes successfully on OS X.

asbjornu · 2016-06-29T10:20:30Z

@eatdrinksleepcode The reason stuff works without the AppVeyor build server provider without it returning the branch name from an environment variable, is that GitVersion then defaults to its own heuristics by querying the remotes. This seems to work on AppVeyor, but not on Travis. So if we get a build server provider working for Travis, that should fix the problem, given that Travis actually expose the name of the branch it is building (something I have no idea whether it is or isn't).

Would you like to tackle the Travis build server provider in this or another PR?

RyanMelenaNoesis · 2016-06-29T13:45:44Z

Here is a little info I found regarding Travis and getting the currently building git branch: http://graysonkoonce.com/getting-the-current-branch-name-during-a-pull-request-in-travis-ci/

Unfortunately, it doesn't seem as straight-forward as one would hope. There is a Travis feature request that would simplify the process but it hasn't been addressed in 2 years: travis-ci/travis-ci#5731

eatdrinksleepcode · 2016-06-29T18:06:48Z

I think it has to be done in this PR, because there is no other way to test it. I think I can get that done this weekend.

On Jun 29, 2016, at 5:20 AM, Asbjørn Ulsberg notifications@github.com wrote:

@eatdrinksleepcode The reason stuff works without the AppVeyor build server provider without it returning the branch name from an environment variable, is that GitVersion then defaults to its own heuristics by querying the remotes. This seems to work on AppVeyor, but not on Travis. So if we get a build server provider working for Travis, that should fix the problem, given that Travis actually expose the name of the branch it is building (something I have no idea whether it is or isn't).

Would you like to tackle the Travis build server provider in this or another PR?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

asbjornu · 2016-06-30T08:10:42Z

@eatdrinksleepcode Brilliant, thanks!

Replace slash with dash in the command line argument switches for gitversion.exe, so it is more Unix friendly. It will still fail exeucting on OS X due to GitTools/GitVersion#890, but once that has been fixed, this should ensure that GitVersion executes successfully on OS X.

…-build/cake#1021.

nulltoken · 2016-07-06T18:28:12Z

Hmmm... Travis build outputs

$ cat .git/HEAD
3f157524e431602a827ac4a4c01c6ed95fc25175
$ cat .git/FETCH_HEAD
3f157524e431602a827ac4a4c01c6ed95fc25175        'refs/pull/890/merge' of https://github.com/GitTools/GitVersion

....

INFO [07/06/16 18:00:38:40] Initializing Repository at /home/travis/build/GitTools/GitVersion/.git
INFO [07/06/16 18:00:38:40] HEAD = ref: refs/heads/pull/890/merge
INFO [07/06/16 18:00:38:40] FETCH_HEAD = 3f157524e431602a827ac4a4c01c6ed95fc25175       'refs/pull/890/merge' of https://github.com/GitTools/GitVersion

whereas AppVeyor ouputs

type .git\HEAD
3f157524e431602a827ac4a4c01c6ed95fc25175
type .git\FETCH_HEAD
3f157524e431602a827ac4a4c01c6ed95fc25175        'refs/pull/890/merge' of https://github.com/GitTools/GitVersion

...

INFO [07/06/16 18:00:59:31] Initializing Repository at C:\projects\gitversion\.git
INFO [07/06/16 18:00:59:31] HEAD = ref: refs/heads/pull/890/merge
INFO [07/06/16 18:00:59:31] FETCH_HEAD = 6ce85b61133fc99c2efff296f7ecfb4a67c9c380   not-for-merge   branch 'AssemblyInfoFix' of https://github.com/GitTools/GitVersion.git
df18bd4b9584151f09ea9eb2d37317f69b4ca2e6    not-for-merge   branch 'CakeBuild' of https://github.com/GitTools/GitVersion.git
32eca24606b7202489a45c2330d7d16ac5c5edc1    not-for-merge   branch 'gh-pages' of https://github.com/GitTools/GitVersion.git
1d9e30cab5241115d7c0422b0145fc5850a451b2    not-for-merge   branch 'master' of https://github.com/GitTools/GitVersion.git
8f153aa85eda48cb131d820ee676689d2265c19d    not-for-merge   branch 'release/4.0.0' of https://github.com/GitTools/GitVersion.git

Although the raw content of HEAD and FETCH_HEAD are identical, the buildversion log for FETCH_HEAD is different.

INFO [07/06/16 18:00:38:40] FETCH_HEAD = 3f157524e431602a827ac4a4c01c6ed95fc25175       'refs/pull/890/merge' of https://github.com/GitTools/GitVersion

vs

INFO [07/06/16 18:00:59:31] FETCH_HEAD = 6ce85b61133fc99c2efff296f7ecfb4a67c9c380   not-for-merge   branch 'AssemblyInfoFix' of https://github.com/GitTools/GitVersion.git
df18bd4b9584151f09ea9eb2d37317f69b4ca2e6    not-for-merge   branch 'CakeBuild' of https://github.com/GitTools/GitVersion.git
32eca24606b7202489a45c2330d7d16ac5c5edc1    not-for-merge   branch 'gh-pages' of https://github.com/GitTools/GitVersion.git
1d9e30cab5241115d7c0422b0145fc5850a451b2    not-for-merge   branch 'master' of https://github.com/GitTools/GitVersion.git
8f153aa85eda48cb131d820ee676689d2265c19d    not-for-merge   branch 'release/4.0.0' of https://github.com/GitTools/GitVersion.git

I'm not sure to understand where does the content of the FETCH_HEAD on AppVeyor come from (considering the type call didn't show any of it earlier). The content does seem legit (those look like valid remote branches from the GitVersion repository).
Would there be some possibility that GitVersion could issue an additional Fetch behind the scene? Would that be the case, any chance you could deactivate it?
Another idea: could you issue yet another call to type|cat .git\FETCH_HEAD once GitVersion has run?

There's indeed something weird... somewhere. @eatdrinksleepcode Would you be able to create the tiniest possible repro of this? Would this portion of code actually put under the light something incosistent at the libgit2(sharp) level, I'm pretty sure @ethomson and @carlosmn would be happy to hear about it.

nulltoken · 2016-07-06T18:29:51Z

@eatdrinksleepcode Dammit! Cross-posting... Happy you eventually found the issue. Congrats on the hunt!

asbjornu · 2016-07-06T23:40:56Z

@eatdrinksleepcode: 🎊 Fantastic work! 🎉

Had I not been doing most of my GitVersion work between 12 and 4 am, I imagine I might have figured out what was going on a fair bit sooner.

You never know. I have a theory that programming while half asleep lets problems enter the subconsciousness easier, which is where they need to go to be solved anyway. 😄

That being said, I think this exposes a weakness in the way build servers are currently handled that we should try to address. I will write that up as a separate issue.

Indeed! Excellent!

Now I am onto making the tests pass (now that Travis is recognized as a build server, there needs to be extra logic in the tests to "unrecognize" it; another issue I will write up). After that, I will need to clean up all of the debugging cruft that has built up in this branch before it can be merged.

Include a rebase and this sounds like a cunning plan! Great work! 👍

…rp.dll.config in NuGet.CommandLine

eatdrinksleepcode · 2016-07-07T19:50:51Z

Have stripped everything back down to just the desired changes and rebased with master. I think this is ready for review.

asbjornu · 2016-07-07T20:12:38Z

.travis.yml

@@ -7,6 +7,7 @@ os:
  - linux
  - osx
 before_install: # We need to download nuget.exe due to: https://github.com/travis-ci/travis-ci/issues/5932
+  - git fetch --unshallow


Is this required? If so, can you please explain why with a comment?

asbjornu · 2016-07-07T20:27:14Z

@eatdrinksleepcode I think this looks like amazeballs! But given the amplitude of the change, I'd love if @pascalberger, @gep13 or @JakeGinnivan could have a look too. Big 👍 and ❤️ for the work you've put into this!

eatdrinksleepcode · 2016-07-08T06:14:45Z

Added some comments and fixed up some minor formatting issues.

pascalberger · 2016-07-08T22:10:35Z

@eatdrinksleepcode Thanks for your work! There still seems to be some indentation issues in several files (eg. AppVeyor.cs). Do you think you can fix them?

Since you also added support for Travis can you add documentation for that (here and in a new markdown file containing Travis specific documentation).

Beside that and some minor issues for which I added comments it looks good to me.

pascalberger · 2016-07-08T22:14:08Z

src/GitVersionCore/BuildServers/TeamCity.cs

@@ -4,9 +4,11 @@

    public class TeamCity : BuildServerBase
    {
-        public override bool CanApplyToCurrentContext()
+		public const string EnvironmentVariableName = "TEAMCITY_VERSION";


Minor issue: Wouldn't a static readonly variable fit better? For cases where this value is used in another assembly and the core assembly is replaced (But most probably this won't happen anyway :)

JakeGinnivan · 2016-07-09T02:48:04Z

Gonna because I want to rebase the Cake script on this.

JakeGinnivan · 2016-07-09T02:48:30Z

We can fix the tabs vs spaces in master

eatdrinksleepcode force-pushed the unpack branch from fafad0b to 6d4df58 Compare June 9, 2016 06:53

eatdrinksleepcode force-pushed the unpack branch 2 times, most recently from 5fcec7e to 8d19c7f Compare June 15, 2016 13:54

eatdrinksleepcode force-pushed the unpack branch from 8d19c7f to c8b51dd Compare June 15, 2016 19:38

asbjornu mentioned this pull request Jun 17, 2016

Node execution in GitVersionTfsTask #909

Closed

eatdrinksleepcode force-pushed the unpack branch from 070e99b to 967c341 Compare June 20, 2016 04:00

asbjornu mentioned this pull request Jun 29, 2016

GH1025: Replace the command line argument switches for GitVersion cake-build/cake#1021

Merged

asbjornu mentioned this pull request Jun 29, 2016

Add GitVersionTask nunit/NUnit.System.Linq#6

Merged

asbjornu added a commit to asbjornu/NUnit.System.Linq that referenced this pull request Jun 30, 2016

Don't run GitVersion on Unix, due to GitTools/GitVersion#890 and cake…

8dbdccc

…-build/cake#1021.

eatdrinksleepcode force-pushed the unpack branch from 363181b to 468c069 Compare July 6, 2016 18:27

eatdrinksleepcode force-pushed the unpack branch from 468c069 to f3bb006 Compare July 7, 2016 18:54

eatdrinksleepcode added 5 commits July 7, 2016 14:02

Exclude LibGit2Sharp from ILRepack to allow for the use of LibGit2Sha…

5866b66

…rp.dll.config in NuGet.CommandLine

Build packages and get version numbers via dogfooding on Travis build

5f000bb

Unshallow repo before building on travis

31ec96f

TravisCI build server integration

e81c4ec

Remove TRAVIS environment variable while testing

289c543

eatdrinksleepcode force-pushed the unpack branch from f3bb006 to 2f67087 Compare July 7, 2016 19:22

asbjornu reviewed Jul 7, 2016
View reviewed changes

eatdrinksleepcode added 2 commits July 8, 2016 00:29

Extract build server environment variable names to constants

b3a8eab

Comments and formatting

b57b9c7

eatdrinksleepcode force-pushed the unpack branch from 0f703e1 to b57b9c7 Compare July 8, 2016 05:30

pascalberger reviewed Jul 8, 2016
View reviewed changes

JakeGinnivan merged commit aa5bb19 into GitTools:master Jul 9, 2016

eatdrinksleepcode mentioned this pull request Jul 10, 2016

Fody Caseless corrupts GitVersionCore on OSX #891

Closed

eatdrinksleepcode deleted the unpack branch July 11, 2016 15:32

asbjornu mentioned this pull request Mar 30, 2017

GitVersion.CommandLine does not work on Linux #1097

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exclude LibGit2Sharp from ILRepack in NuGet.CommandLine #890

Exclude LibGit2Sharp from ILRepack in NuGet.CommandLine #890

eatdrinksleepcode commented Jun 3, 2016

asbjornu commented Jun 3, 2016

eatdrinksleepcode commented Jun 3, 2016

asbjornu commented Jun 6, 2016

eatdrinksleepcode commented Jun 9, 2016

asbjornu commented Jun 15, 2016

eatdrinksleepcode commented Jun 15, 2016

eatdrinksleepcode commented Jun 15, 2016

asbjornu commented Jun 16, 2016

eatdrinksleepcode commented Jun 20, 2016

JakeGinnivan commented Jun 20, 2016

eatdrinksleepcode commented Jun 20, 2016

eatdrinksleepcode commented Jun 20, 2016

gep13 commented Jun 20, 2016

asbjornu commented Jun 21, 2016

JakeGinnivan commented Jun 29, 2016

asbjornu commented Jun 29, 2016

RyanMelenaNoesis commented Jun 29, 2016

eatdrinksleepcode commented Jun 29, 2016

asbjornu commented Jun 30, 2016

nulltoken commented Jul 6, 2016

nulltoken commented Jul 6, 2016

asbjornu commented Jul 6, 2016

eatdrinksleepcode commented Jul 7, 2016

asbjornu Jul 7, 2016

asbjornu commented Jul 7, 2016

eatdrinksleepcode commented Jul 8, 2016

pascalberger commented Jul 8, 2016 •

edited

Loading

pascalberger Jul 8, 2016

JakeGinnivan commented Jul 9, 2016

JakeGinnivan commented Jul 9, 2016

Exclude LibGit2Sharp from ILRepack in NuGet.CommandLine #890

Exclude LibGit2Sharp from ILRepack in NuGet.CommandLine #890

Conversation

eatdrinksleepcode commented Jun 3, 2016

asbjornu commented Jun 3, 2016

eatdrinksleepcode commented Jun 3, 2016

asbjornu commented Jun 6, 2016

eatdrinksleepcode commented Jun 9, 2016

asbjornu commented Jun 15, 2016

eatdrinksleepcode commented Jun 15, 2016

eatdrinksleepcode commented Jun 15, 2016

asbjornu commented Jun 16, 2016

eatdrinksleepcode commented Jun 20, 2016

JakeGinnivan commented Jun 20, 2016

eatdrinksleepcode commented Jun 20, 2016

eatdrinksleepcode commented Jun 20, 2016

gep13 commented Jun 20, 2016

asbjornu commented Jun 21, 2016

JakeGinnivan commented Jun 29, 2016

asbjornu commented Jun 29, 2016

RyanMelenaNoesis commented Jun 29, 2016

eatdrinksleepcode commented Jun 29, 2016

asbjornu commented Jun 30, 2016

nulltoken commented Jul 6, 2016

nulltoken commented Jul 6, 2016

asbjornu commented Jul 6, 2016

eatdrinksleepcode commented Jul 7, 2016

asbjornu Jul 7, 2016

Choose a reason for hiding this comment

asbjornu commented Jul 7, 2016

eatdrinksleepcode commented Jul 8, 2016

pascalberger commented Jul 8, 2016 • edited Loading

pascalberger Jul 8, 2016

Choose a reason for hiding this comment

JakeGinnivan commented Jul 9, 2016

JakeGinnivan commented Jul 9, 2016

pascalberger commented Jul 8, 2016 •

edited

Loading