libgit2 / libgit2sharp

Git + .NET = ❤
http://libgit2.github.com
MIT License
3.21k stars 889 forks source link

Repository.Clone doesn't work with local repositories #247

Closed darind closed 11 years ago

darind commented 12 years ago

I am unable to clone a local repository using Repository.Clone method. I have tried with different source urls without success:

// var sourceUrl = "file:///c:\work\REPO.git\";
var sourceUrl = @"c:\work\REPO.git\";
Repository.Clone(sourceUrl, @"c:\work\targetDir", bare: true) 

The following exception is raised:

Unhandled Exception: LibGit2Sharp.LibGit2SharpException: An error was raised by libgit2. Category = Config (Error).
Failed to parse config file: Unexpected end of file while parsing multine var (in c:/work/targetDir/config:23, column 0)

Cloning with the HTTP transport works fine:

var sourceUrl = @"https://github.com/libgit2/libgit2sharp";
Repository.Clone(sourceUrl, @"c:\work\targetDir", bare: true) 
nulltoken commented 12 years ago

Thanks a lot for reporting this!

Local cloning/fetching is (just) around the corner and should be shortly made available through LibGit2Sharp. It's already part of libgit2.

Unexpected end of file while parsing multiline var (in c:/work/targetDir/config:23, column 0)

Would you be able to share you config file? Or maybe recreate a test repo that would generate the same issue if your current config file contains sensitive information?

Related information:

/cc @ben

darind commented 12 years ago

@nulltoken, thanks for looking into this issue.

The thing is that the folder shown in the exception stacktrace "c:/work/targetDir" is never created. There's no such directory and no config file inside. So the exception is a bit weird because it mentions a "config" file that doesn't exist.

I am working with the sample repository used in the LibGit2Sharp unit tests: https://github.com/libgit2/libgit2sharp/tree/vNext/LibGit2Sharp.Tests/Resources/testrepo.git.

nulltoken commented 12 years ago

Ok. That's weird ;) That also might be a bogus error message. We'll track this down as well.

However, there's still some config parsing happening, How about your source repository in c:\work?

/cc @carlosmn

darind commented 12 years ago

The source repository in c:\work is the test repo used in the LibGit2Sharp unit tests. Here's the config file: https://github.com/libgit2/libgit2sharp/blob/vNext/LibGit2Sharp.Tests/Resources/testrepo.git/config

nulltoken commented 12 years ago

Awesome, it should be quite easy to reproduce it then!

carlosmn commented 12 years ago

Are you positive that the dir is never created? The clone function is meant to simulate the git behaviour, so it deletes the newly-created repository on error.

The error in the first stacktrace shows that something failed to set an error, as that error message doesn't cause the function to return an error code, most likely something in the remote or transport part.

The second one's error message shows that we're not escaping the problematic string. The string you're passing ends in a backslash, which is git and general unix form for indicating that the "logical" line continues on the next line in the file. The configuration layer doesn't guard against setting this accidentally, which is a bug there.

In any case, you need to pass a path in the "scheme://path" form for the fetch to work.

darind commented 12 years ago

@carlosmn, I don't know if the folder is being created during the execution of the program. What I know for sure is that this folder no longer exists after the exception is thrown and the program has finished executing. So either the folder is never created or it is being cleaned after the exception. In both cases the expected result is not achieved.

Here's are my exact repro steps.

Open a Visual Studio command prompt and type the following commands (I assume that the git client is installed and configured to run in the command prompt):

C:>mkdir work
C:>cd work
C:\work>git clone https://github.com/libgit2/libgit2sharp.git
    Cloning into 'libgit2sharp'...
    remote: Counting objects: 7662, done.
    remote: Compressing objects: 100% (2089/2089), done.
    remote: Total 7662 (delta 5667), reused 7324 (delta 5423)
    Receiving objects: 100% (7662/7662), 36.28 MiB | 2.12 MiB/s, done.
    Resolving deltas: 100% (5667/5667), done.
C:\work>msbuild libgit2sharp\LibGit2Sharp\LibGit2Sharp.csproj
C:\work>copy con Program.cs
    using System;
    using LibGit2Sharp;    
    class Program
    {
        static void Main()
        {
            Repository.Clone(@"file:///c:\work\libgit2sharp\LibGit2Sharp.Tests\Resources\testrepo.git\", @"c:\work\targetDir", bare: true);
        }
    }^Z
        1 file(s) copied.
C:\work>xcopy libgit2sharp\lib\NativeBinaries\* NativeBinaries /I /S
    libgit2sharp\lib\NativeBinaries\amd64\git2.dll
    libgit2sharp\lib\NativeBinaries\amd64\git2.pdb
    libgit2sharp\lib\NativeBinaries\x86\git2.dll
    libgit2sharp\lib\NativeBinaries\x86\git2.pdb
   4 File(s) copied
C:\work>copy libgit2sharp\LibGit2Sharp\bin\Debug\LibGit2Sharp.dll .
    1 file(s) copied.
C:\work>csc /target:exe /reference:libgit2sharp\libgit2sharp\bin\debug\libgit2sharp.dll Program.cs
C:\work>Program.exe

    Unhandled Exception: LibGit2Sharp.LibGit2SharpException: An error was raised by libgit2. Category = Config (Error).
    Failed to parse config file: Unexpected end of file while parsing multine var (in c:/work/targetDir/config:23, column 0)
       at LibGit2Sharp.Core.Ensure.Success(Int32 result, Boolean allowPositiveResult)
       at LibGit2Sharp.Core.Proxy.git_clone_bare(String url, String workdir, git_transfer_progress_callback transfer_cb)
       at LibGit2Sharp.Repository.Clone(String sourceUrl, String workdirPath, Boolean bare, Boolean checkout, TransferProgressHandler onTransferProgress, CheckoutProgressHandler onCheckoutProgress)
       at Program.Main()

So the question remains: what is the correct url pattern to clone a local git repository using LibGit2Sharp?

nulltoken commented 11 years ago

I think this closely relates to @jamill's libgit2/libgit2#1140

/cc @ben

darind commented 11 years ago

Has this been resolved for libgit2sharp? I've seen that the https://github.com/libgit2/libgit2/issues/1140 issue has been closed. If so, what do I need to do in order to integrate/compile the new native bits? My C++ skills are close to 0 (if not negative). If I have to compile the native binary, what command line switches do I need to use in order to build for both x86 and x64 with all optimizations turned on in Release mode?

nulltoken commented 11 years ago

If I have to compile the native binary, what command line switches do I need to use in order to build for both x86 and x64 with all optimizations turned on in Release mode?

Thanks to @ben, this is now far easier. Running UpdateLibgit2ToSha.ps1 in a PowerShell console should do the hard work. Although this script is rather profiled to automate the production of "LibGit2 Upgrade commits" (cf. ed8c978b49ca5b5e5becd37cc8d71e086217672d for instance) it should fulfill your needs.

This requires CMake to be installed.

darind commented 11 years ago

I have tried the PowerShell script and it indeed generated both a x86 and x64 bit versions of git2.dll. I have placed the latest sources from the development branch in the libgit2 folder before running the script. The problem is that when I attempted to use with libgit2sharp and the Repository.Clone method I got the following exception:

Unable to find an entry point named 'git_clone_bare' in DLL 'git2'.

Do I need to pass some parameters to the PowerShell script in order to export functions or something?

nulltoken commented 11 years ago

I have tried the PowerShell script and it indeed generated both a x86 and x64 bit versions of git2.dll.

Great!

Unable to find an entry point named 'git_clone_bare' in DLL 'git2'.

Less great :-/

Indeed, the libgit2 API has been updated by @ben.... and I completely forgot about this while answering to your question. Sorry about this.

The problem is that the current interface is not very suitable for LibGit2Sharp as it relies on an in-memory remote.

Current interface:

GIT_EXTERN(int) git_clone(
        git_repository **out,
        git_remote *origin,
        const char *local_path,
        const git_clone_options *options);

Pull request libgit2/libgit2#1152 by @ben (ZOMG! He's everywhere!) should help fix.

Proposed interface:

 GIT_EXTERN(int) git_clone(
     git_repository **out,
     const char *url,
     const char *local_path,
     const git_clone_options *options);

Once it's merged, LibGit2Sharp will be updated with the newest version of libgit2 binaries.

Meanwhile, it might be worth trying to update libgit2 binaries up to libgit2/libgit2@44f5f777aeca736a3ccbd2e099a608cc2687b508 which contains @jamill's libgit2/libgit2#1140 fix. However, it's possible you may encounter other breaking changes in the libgit API.

./UpdateLibgit2ToSha.ps1 44f5f777aeca736a3ccbd2e099a608cc2687b508
ben commented 11 years ago

Yeah, sorry about this. Clone has been kind of my personal crusade, and I've made a couple of missteps.

Once https://github.com/libgit2/libgit2/issues/1152 is merged, I'll send up a PR to update libgit2sharp to work with the new bits.

nulltoken commented 11 years ago

Once libgit2/libgit2#1152 is merged, I'll send up a PR to update libgit2sharp to work with the new bits.

@ben :heart_decoration:

arse commented 11 years ago

Is there any update to this please?

nulltoken commented 11 years ago

Is there any update to this please?

@ben is already working on it. See #281

nulltoken commented 11 years ago

@darind FWIW the following tests now pass

private void AssertLocalClone(string path)
{
    var scd = BuildSelfCleaningDirectory();
    using (Repository clonedRepo = Repository.Clone(path, scd.RootedDirectoryPath))
    using (var originalRepo = new Repository(BareTestRepoPath))
    {
        Assert.NotEqual(originalRepo.Info.Path, clonedRepo.Info.Path);
        Assert.Equal(originalRepo.Head, clonedRepo.Head);

        Assert.Equal(originalRepo.Branches.Count(), clonedRepo.Branches.Count(b => b.IsRemote));
        Assert.Equal(1, clonedRepo.Branches.Count(b => !b.IsRemote));

        Assert.Equal(originalRepo.Tags.Count(), clonedRepo.Tags.Count());
        Assert.Equal(1, clonedRepo.Remotes.Count());
    }
}

[Fact]
public void CanCloneALocalRepositoryFromALocalUri()
{
    var uri = new Uri(BareTestRepoPath);
    AssertLocalClone(uri.AbsoluteUri);
}

[Fact]
public void CanCloneALocalRepositoryFromAStandardPath()
{
    AssertLocalClone(BareTestRepoPath);
}

@ben :+1:

nulltoken commented 11 years ago

The passing tests have been pushed into vNext.

darind commented 11 years ago

Sorry I didn't respond earlier, but I didn't have time to test the new version. I have downloaded the latest versions of libgit2 and libgit2sharp from their respective repositories and built the native library with the UpdateLibgit2ToSha.ps1 script. I have then integrated the new binaries into my test project (a console application).

Now if I run my console application in debug mode (F5) everything works great and the repository is getting cloned.

But If I run the console application without debugging (Ctrl+F5) or outside Visual Studio, I still get the same exception

Failed to parse config file: Unexpected end of file while parsing multine var (in c:/work/targetDir/config:21, column 0)

and if I attach a debugger, the cause of this exception seems to be related to some heap corruption issue in the unmanaged bits:

Untitled

Another interesting thing is that if I ignore the exception (I don't launch a debugger), the repository gets successfully cloned.

Any idea what could be the reason for this? I am a little bit reluctant of putting this code in my production application and wouldn't consider this issue as fixed (unless of course I did something wrong with my setup).

nulltoken commented 11 years ago

I have downloaded the latest versions of libgit2 and libgit2sharp from their respective repositories and built the native library with the UpdateLibgit2ToSha.ps1 script.

@darind As the libgit2 binaries are already included in Lib/NativeLibraries/ you shouldn't have to compile your own version of libgit2.

Failed to parse config file: Unexpected end of file while parsing multine var (in c:/work/targetDir/config:21, column 0)

Could you please remove the libgit2 binaries you've compiled and check if the problem happens again with the provided ones? Each commit of LibGit2Sharp depends on a specific version of libgit2. Working against a more recent version of the binaries may cause problems. Let's try to strike this out.

Provided the problem happens again with the embedded binaries, could you please check what you could share in order to help us fix this (repro case, call stack, config files, ...)?

/cc @carlosmn

I am a little bit reluctant of putting this code in my production application.

I can't agree more with you on this point :wink:

darind commented 11 years ago

@nulltoken, I have tried with the binaries included in Lib/NativeLibraries/ folder and still have the same issue. You can download my sample console application exhibiting the problem here: http://db.tt/GzxPsPLO

The libgit2 binaries were taken from the LibGit2Sharp distribution and the LibGit2Sharp assembly that I referenced from the Lib folder was compiled locally from the source code.

My test environment is Windows 7 x64, Visual Studio 2012 Ultimate and the Console Application is compiled in Debug mode targeting Any CPU.

Hope my description is enough to reproduce the issue.

nulltoken commented 11 years ago

Hope my description is enough to reproduce the issue.

Awesome :heart:

I was afraid this may be related to your global and/or system config files, but it turns out I can reproduce the issue. Working on it.

nulltoken commented 11 years ago

@darind I've found and fixed the root cause. I'll send a libgit2 PR later today.

darind commented 11 years ago

@nulltoken, thanks for solving the problem so rapidly. I am looking forward to get the new bits from the LibGit2Sharp project containing the fixed unmanaged binaries.

nulltoken commented 11 years ago

@darind vNext has been updated with the latest bits.

darind commented 11 years ago

I still get the same error. I have cloned the libgit2sharp project and copied the lib\NativeBinaries to my test application and got the same heap corruption error.

nulltoken commented 11 years ago

@darind Sorry to ask, you also copied the managed assembly as well, didn't you? That's weird. Could you please issue a git hash-object on the native libraries (x86 and amd64) your project rely against?

darind commented 11 years ago

Yes, I have recompiled the LibGit2Sharp project from the source and added the generated assembly to my sample application. And here are the hashes of the unmanaged libraries that I use:

C:\work\libgit2sharp\Lib\NativeBinaries\amd64>git hash-object git2.dll c08ef0ecca49a2854eb4843fa1480cc2d203b3e0

C:\work\libgit2sharp\Lib\NativeBinaries\x86>git hash-object git2.dll ecea5a1e8175bec7ed62c2481d57d3dba9e9f61b

nulltoken commented 11 years ago

@darind I cannot reproduce the issue. Can you give a try to the following project http://db.tt/x6vW8Ws5 on your computer?

This is a slightly modified version of yours which includes the Debug version of the managed assembly and the RelWithDebInfo version of the native binaries

darind commented 11 years ago

Yes, your code works fine. It turns out that the exception is thrown if you incorrectly generate the sourceUri variable (which is what I did in my example). If you want to reproduce the problem in your code replace:

var sourceUri = new Uri(sourceDirectory).AbsoluteUri;
// generates "file:///c:/work/LibGit2SharpTest/testrepo.git/"

with:

var sourceUri = string.Format("file:///{0}", sourceDirectory);
// generates "file:///c:\work\LibGit2SharpTest\testrepo.git\"

I agree that the first approach is the correct url to be specified but I don't agree with leaking handles if I pass by mistake some wrong url.

nulltoken commented 11 years ago

@darind Awesome! I'm glad we eventually nailed this issue.

darind commented 11 years ago

Me too, but are there plans to handle this error gracefully and throw a normal .NET exception if the uri is invalid instead of a heap corruption?

nulltoken commented 11 years ago

Are there plans to handle this error gracefully and throw a normal .NET exception if the uri is invalid instead of a heap corruption?

I'm on it. There are two nested issues. I already locally fixed one of them.

Beside this, from a .Net perspective, I wonder if accepting a string instead of a proper Uri is the right choice. @phkelley @cmn @jamill your opinion on this? (cf. http://git-scm.com/docs/git-clone#URLS)

nulltoken commented 11 years ago

I'm on it. There are two nested issues. I already locally fixed one of them.

And @carlosmn nailed the other one ;-) cf. libgit2/libgit2#1279

ethomson commented 11 years ago

Is there some occasion where accepting a string that is not a URL is possible? I fear it will encourage people to use the abomination that is the impossibly vague ssh-style host:/path syntax, when the only thing that can properly disambiguate between ssh-style host:/path and C:/Temp is a URI with a scheme.

carlosmn commented 11 years ago

A path of C:/Temp, this is a perfectly acceptable path for a computer named (even if only in your ssh configuration) C.

What I find problematic about the notion that you want the library to figure out what you mean by that path is that you're mixing what the git command-line tool does for you and what the underlying git system does. Just because git.git chose to do something under a particular command, it doesn't mean we have to emulate it 1-to-1, especially when the the behaviours are quite different.

The git-clone builtin does many different things depending on what parameters you pass and how you specify the paths. If you run git clone /some/path or git clone C:/something, what you're asking git is precisely not to perform a fetch (which is why you even want the path in the first place), but to link the new repository's object db with the old one's so as to speed up the operation and reduce the amount of disc space needed for the objects both already have. Then there is also the --shared option which sets up alternates.

While it's perfectly fine to have this db linking as a convenience function in the library, I see it as ill-advised to attempt to replicate whatever git.git does upstream by default in some command as it makes the implementation harder and reduces the flexibility which is the point of using the library instead of writing a small shell script to get git.git to do most of what you want and then fixing it up. Not to speak about how half of the clone options make no sense when you're not actually asking the remote for object but taking them and neither do the states.

So I'm just going to issue an edict to make it clear what the library should expect: pass a well-formed url into it, with a schema part and a local part. Otherwise it's a relative path; and if it has a colon it's a scp-style path, which gets grandfathered in because that's been there since the start.

If you want the --local behaviour from git.git, you need to link the object db, and that has little to do with a clone. As the first steps of initialising the repo and creating the remote are the same, it'd probably be good to expose this as its own function.

ethomson commented 11 years ago

@carlosmn so..... does this mean you think it should take a String ?

carlosmn commented 11 years ago

Yes, it should be a string. You should also not be surprised if passing C:/Temp doesn't work.

ethomson commented 11 years ago

I disagree completely. I don't think it's a stretch to say that most libgit2sharp users are on Windows, and that most Windows users would far far sooner expect C:\Temp or C:/Temp to be a local path before an SCP-style path. Uris have no ambiguity. Accepting a String means that the library does have to figure out what you meant.

Your edict says that one should pass a well-formed URL into it. Why should I pass a well-formed URL in as a String instead of as an object made to represent this?

I think that we should go a step further and have libgit2 reject anything that is not a well-formed URL. SCP style syntax is a hot mess that consumers and front-ends can worry about and should not be the library's job to touch ever. But that's a discussion for libgit2 not libgit2#.

carlosmn commented 11 years ago

The C# bindings can require a Uri and just refuse scp-style at all if you think that makes more sense. It stops people from passing C:\Temp and expecting it to work.

I'd love it if we could let the users handle scp-style and local paths and not worry about anything, but we can't really do that because the paths are stored in the config. Never mind that scp-style is what GitHub puts on the page for now and concentrate on the fact that the url/path is stored in the config we need to read whatever was put there by whatever tool and git implementation put it there.

We could stop the functions that take a url/path from the user from taking anything else and let the user sort out what the other things mean, but git_remote_load(); git_remote_download(); should bloody well work if the user initially cloned with git.git. The initial fetch implementation back in '11 only accepted proper uris, but we can't really ask the user to parse what's in the config for us.

nulltoken commented 11 years ago

@darind Both libgit2/libgit2#1076 and libgit2/libgit2#1280 have been merged. Thanks thousands for having helped us uncover those. The fixes will be made available to LibGit2Sharp the next time the binaries will be updated.

Meanwhile, provided you rely on a Uri normalized path, you should be pretty safe with current vNext.