Skip to content

Ensure shadow copying doesn't prevent from probing #517

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

Merged
merged 3 commits into from
Oct 5, 2013

Conversation

nulltoken
Copy link
Member

No description provided.

@nulltoken
Copy link
Member Author

Hmmm... Passes on Windows. Fails on Mono.

LibGit2Sharp.Tests.ShadowCopyFixture.CanProbeForNativeBinariesFromAShadowCopiedAssembly

mono: /home/travis/build/libgit2/libgit2sharp/libgit2/src/global.c:162: git__global_state: Assertion `_tls_init' failed.
Stacktrace:
  at (wrapper managed-to-native) LibGit2Sharp.Core.Nativethods.git_repository_open (LibGit2Sharp.Core.Handles.RepositorySafeHandle&,LibGit2Sharp.Core.FilePath) <0xffffffff>
  at LibGit2Sharp.Core.Proxy.git_repository_open (string) <0x0005f>
  at LibGit2Sharp.Repository..ctor (string,LibGit2Sharp.RepositoryOptions) <0x000b7>
  at LibGit2Sharp.Tests.StageFixture.CanStageAnUnknownFileWithLaxUnmatchedExplicitPathsValidation (string,LibGit2Sharp.FileStatus) <0x0008b>

@nulltoken
Copy link
Member Author

@phkelley @ethomson @jamill @ben @arrbee This test pass on Windows, but fails on Mono/Linux here.

I'm a bit confused here. Is this a threading issue or limitation? Does it only pass on Windows by chance?

I'd appreciate if you guys could give a peek at this 🙏❤️

@carlosmn
Copy link
Member

I just tested with mono 3.0.6 (Debian sid) and it passes (once I ask it to load git2 rather than the one with the hash).

@nulltoken
Copy link
Member Author

@carlosmn Thanks thousands for the hand! ❤️

(once I ask it to load git2 rather than the one with the hash)

Huh? You lost me there. Which version of libgit2 did you make it pass against? The one from the submodule? Why have you been compelled to rename it?

@carlosmn
Copy link
Member

It ran against libgit2/libgit2@8821c9aa, which is development from a few days ago, built with threadsafe. I changed LibGit2Sharp.Core.NativeDllName.Name to a plain git2 as that's the libgit2 that I have installed in my computer.

@nulltoken
Copy link
Member Author

AWESOME!.

@nulltoken
Copy link
Member Author

However, I can't find, among the changes, what made the test pass....

@carlosmn
Copy link
Member

The change isn't a libgit2 one. It also succeeds with libgit2/libgit2@32e4992. What I was pointing out was that a newer mono version to the one on Travis' version of Ubuntu behaves correctly.

@nulltoken
Copy link
Member Author

Duh... I read "and it passes (once I ask it to load git2 rather than the one with the hash)" backwards.

That leads us to a very interesting topic: Which version of Mono should we officially support?

From what I know 2.10 is no longer supported (announced by Miguel de Icaza at Monkeyspace 2013).

  • TeamCity Mono build runs against a Mono 3.x
  • Travis build runs against Mono 2.10

/cc @dragan

@dragan
Copy link

dragan commented Sep 30, 2013

@nulltoken I'd say you should just support 3.x since it's now the stable path. On the TeamCity Mono Build Agent, we support mono versions 3.0.0, 3.0.7, 3.0.12, 3.2.1, and 3.2.3. The reason we can do this is with mope. All you have to do in your build script is set an environment variable to the version you want to run the build under. Most do this in the build configuration, but with libgit2sharp's circumstances with the native library, it's probably better to set it in your build script.

export MOPE_VERSION="3.2.3"

Then, to setup the runtime correctly, just use mope to find where mono is installed.

MONO_PREFIX="$(mope prefix)"
export DYLD_FALLBACK_LIBRARY_PATH="${MONO_PREFIX}/lib:/usr/local/lib:/usr/lib"
export LD_LIBRARY_PATH="${MONO_PREFIX}/lib:/usr/lib"
export C_INCLUDE_PATH="${MONO_PREFIX}/include:/usr/local/include"
export ACLOCAL_PATH="${MONO_PREFIX}/share/aclocal"
export PKG_CONFIG_PATH="${MONO_PREFIX}/lib/pkgconfig:/usr/local/pkgconfig"
export PATH="${MONO_PREFIX}/bin:$PATH"

I'd like to do the above in mope, but haven't gotten around to it.

@nulltoken
Copy link
Member Author

Well. Some gain and some pain...

Gain:

  • New RequiresDotNetOrMonoGreaterThanOrEqualTo(Version v) test helper method which will skip the test if the Mono version isn't high enough

Pain:

  • I've got super weird test failures (First some IO UnauthorizedAccessExceptions then a ton of AccessViolationExceptions) on Windows (x86 and x64) when launching the full suite. I've stared at this code for too long, and now I'm "blind"... I'd need some help troubleshooting them. @ben, @jamill, @phkelley That would be awesome if you could peek at this.

@ben ben mentioned this pull request Oct 3, 2013
@ben
Copy link
Member

ben commented Oct 4, 2013

Pretty sure we found the cause of these test failures.

  1. The first call into Libgit2Sharp triggered a call to git_threads_init
  2. The ShadowCopiesFixture spun up a new AppDomain, which caused another call to git_threads_init
  3. Shutting that AppDomain down caused a call to git_threads_shutdown.
  4. More tests ran, expecting the threading setup to still be in place.

That last one isn't an unreasonable assumption, but the libgit2 code was written with only single calls in mind, so step 3 screwed up everything for future callers. I'll be fixing this in libgit2/libgit2#1890.

@nulltoken nulltoken merged commit 467fbd0 into vNext Oct 5, 2013
@nulltoken nulltoken deleted the ntk/test/shadow-copy branch October 5, 2013 18:39
@nulltoken
Copy link
Member Author

@ben @phkelley @arrbee Thanks for all the hard work ❤️

@dragan Once this was fixed on Windows, I tried again against Mono 2.10.8.1. The test now pass as well on this version 🎉

@ben
Copy link
Member

ben commented Oct 6, 2013

🎊

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

Successfully merging this pull request may close these issues.

5 participants