Proper pull, fetch, merge support #65

Closed
synhershko opened this Issue Sep 18, 2011 · 107 comments

Comments

Projects
None yet

Currently there doesn't seem to be a way to do a pull, or even a separate fetch-merge operation. Will be nice, and quite useful, to have this...

Member

nulltoken commented Sep 19, 2011

Hello Itamar,

Will be nice, and quite useful, to have this...

I fully agree with you :) @carlosmn is working hard to make this happen. Once the feature is available and tested in libgit2, we'll make sure that LibGit2Sharp benefit from it quickly.

BTW, from an API standpoint, how would you like to use it? What would be the client code you'd like to write?

Looking forward to it...

I guess new Repository(path).Pull(origin) and new
Repository(path).Fetch(origin) ?

Either this or through the Remotes API. I'm not familiar with your API
enough, but I think pulling made on a repository given a remote name makes
the most sense.

On Mon, Sep 19, 2011 at 11:49 AM, nulltoken <
reply@reply.github.com>wrote:

Hello Itamar,

Will be nice, and quite useful, to have this...

I fully agree with you :) @carlosmn is working hard to make this happen.
Once the feature is available and tested in libgit2, we'll make sure that
LibGit2Sharp benefit from it quickly.

BTW, from an API standpoint, how would you like to use it? What would be
the client code you'd like to write?

Reply to this email directly or view it on GitHub:
#65 (comment)

Member

nulltoken commented Sep 19, 2011

Looking forward to it...

I'm going to keep this issue open as a reminder. It will be updated as soon as something testable pops up.

Owner

carlosmn commented Sep 19, 2011

The library already supports fetching (not every efficiently, but that'll come in good time). Personally I don't like new Repository(path).Fetch(origin), because that can block (for a few minutes if you're unlucky). What the library has now (in C) is more like

Remote origin = new Remote(path).Remote("origin");
origin.Connect();
origin.Negotiate();
if (origin.NeedsPack) {
  origin.DownloadPack();
}

All of the network operations should be run in a worker thread and you probably don't want to create another repository object since you probably already have one somewhere.

Don't mind the new stuff, I was sketching code...

My problem with this syntax is it doesn't mimic the normal git operation.
The .NET wrapper is enough high level to allow that, IMHO.

Also, what about merges? in my scenario I'm not expecting conflicts, hence
I'm interested in using Pull.

On Mon, Sep 19, 2011 at 5:43 PM, Carlos Martn Nieto <
reply@reply.github.com>wrote:

The library already supports fetching (not every efficiently, but that'll
come in good time). Personally I don't like new Repository(path).Fetch(origin), because that can block (for a few minutes
if you're unlucky). What the library has now (in C) is more like

Remote origin = new Remote(path).Remote("origin");
origin.Connect();
origin.Negotiate();
if (origin.NeedsPack) {
 origin.DownloadPack();
}

All of the network operations should be run in a worker thread and you
probably don't want to create another repository object since you probably
already have one somewhere.

Reply to this email directly or view it on GitHub:
#65 (comment)

Owner

carlosmn commented Sep 19, 2011

It replicates how the git protocol works. The git command-line tool obviously does it all at once. The C# layer can certainly simplify the method all it wants to, and we may eventually take (some of) it into the library.

Merges need merge-base calculation support and the whole diff machinery, which will take a bit of time (help writing and reviewing code is always appreciated). I don't think there should be an explicit pull operation, but rather fetch + merge, both of these steps being explicitly run.

I'm too against doing just pull, but in some scenarios (like mine: auto deployment) that is more than acceptable to make. It should be easy to with with an extension function, so I don't really mind.

What is lacking in the merge and diff arena? there's plenty of OSS code that should get you jumpstarted on that one, no?

I'm too swamped with work and projects as it is, but I'd like to understand what needs to be done.

Feel free to e-mail me directly to prevent this thread from bloating.

I'm too against doing just pull, but in some scenarios (like mine: auto
deployment) that is more than acceptable to make. It should be easy to with
with an extension function, so I don't really mind.

What is lacking in the merge and diff arena? there's plenty of OSS code that
should get you jumpstarted on that one, no?

I'm too swamped with work and projects as it is, but I'd like to understand
what needs to be done.

Feel free to e-mail me directly to prevent this thread from bloating.

On Mon, Sep 19, 2011 at 6:23 PM, Carlos Martn Nieto <
reply@reply.github.com>wrote:

It replicates how the git protocol works. The git command-line tool
obviously does it all at once. The C# layer can certainly simplify the
method all it wants to, and we may eventually take (some of) it into the
library.

Merges need merge-base calculation support and the whole diff machinery,
which will take a bit of time (help writing and reviewing code is always
appreciated). I don't think there should be an explicit pull operation, but
rather fetch + merge, both of these steps being explicitly run.

Reply to this email directly or view it on GitHub:
#65 (comment)

Hey, any word on what needs to be done?

Member

nulltoken commented Oct 10, 2011

Hey, any word on what needs to be done?

Hello Itamar, sorry I've let this thread go cold.

First of all, the update of the working directory from a commit (checkout) doesn't work yet. So the Clone/Fetch experience will not be as rich as one could expect :)

Even if the API might change later (for instance, returning a [Fetch|Clone]Result or accepting callback parameters), for now, I'd go with the following pattern of usage.

using (var repo = new Repository(path))
{
   repo.Fetch(remoteAlias);
}

or

string fullPathToTheRepo = Repository.Clone(repoPath, remoteUrl);

I'd throw in some local and http Clone tests from a tiny repo within a SelfCleaningDirectory, asserting commits, refs, .... (To this purpose, I've forked @henon's TestGitRepository at https://github.com/nulltoken/TestGitRepository)

Then I'd dive in fetch.c example and bang my head in despair against the keyboard until the tests pass.

If this doesn't work, a more standard approach may help :)

  • Make sure fetch.c actually works (this will of course require to build libgit2 (see this and that)
  • Bind the extern'd methods in NativeMethods (depends.exe usually comes handy to troubleshoot potential issues)
  • Port the C code in C#
  • Make the tests pass
  • Refactor
  • Add some more tests
  • ...

Please note, however, that this kind of task might put under the light some bugs (features-to-be?) in libgit2. When/If you encounter an issue, provided you're damn sure of your C# code, you'll eventually have to go back to C and port your non-working test from C# to C in order to give the libgit2 team something to chew while troubleshooting/fixing...

Concerning the diff, we might wait for this issue to be closed or temporarily rely on diffplex (or whatever) once the clone/fetch works.

Hope this helps,

Em.

Yay, thanks. I will have a look as soon as I can.

Member

nulltoken commented Dec 10, 2011

Relevant information in libgit2 issue #501.

Contributor

uluhonolulu commented Jan 16, 2012

So I thought I'd give it a try, and the last line of this code gives me an AccessViolationException:

var path = @"F:\Projects\Fubu\Chpokk";
var result = NativeMethods.git_repository_open(out repository, PosixPathHelper.ToPosix(path));
Assert.NotNull(repository);
Assert.AreEqual(0, result);
result = NativeMethods.git_remote_new(out remote, repository, "git@github.com:uluhonolulu/Chpokk.git", null);
Assert.AreEqual(0, result);
result = NativeMethods.git_remote_connect(remote, NativeMethods.GIT_DIR_FETCH);

My native method declarations are:

    public const int GIT_DIR_FETCH = 0;

    [DllImport(libgit2)]
    public static extern int git_remote_new(
        out RemoteSafeHandle remote,
        RepositorySafeHandle repo,
        [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string url,
        [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string name);

    [DllImport(libgit2)]
    public static extern int git_remote_connect(
        RemoteSafeHandle remote, 
        int direction);

I should admit I was too lazy to build and run the C code.

Member

nulltoken commented Jan 16, 2012

@uluhonolulu AccessViolationException confirmed. I'm going to troubleshoot this.

Meanwhile, using git://github.com/libgit2/libgit2.git or http://github.com/libgit2/libgit2.git seems to be ok.

Owner

carlosmn commented Jan 16, 2012

The url git@github.com:uluhonolulu/Chpokk.git doesn't get handled by libgit2

Contributor

uluhonolulu commented Jan 17, 2012

Yay! it works!

Member

nulltoken commented Jan 17, 2012

;-)

Contributor

uluhonolulu commented Jan 17, 2012

Ok, now git_remote_download throws AccessViolationException.
The code is:

            var path = @"F:\Projects\Fubu\Chpokk";
            var result = NativeMethods.git_repository_open(out repository, PosixPathHelper.ToPosix(path));
            Assert.NotNull(repository);
            Assert.AreEqual(0, result);
            result = NativeMethods.git_remote_new(out remote, repository, "git://github.com/libgit2/libgit2.git", "origin");
            Assert.AreEqual(0, result);
            result = NativeMethods.git_remote_connect(remote, 0);
            Assert.AreEqual(0, result);
            string packname;
            result = NativeMethods.git_remote_download(out packname, remote);

Of course it might be a problem with how I declare the method:

    [DllImport(libgit2)]
    public static extern int git_remote_download(
        [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof (Utf8Marshaler))] out string filename,
        RemoteSafeHandle remote);

The original is:

GIT_EXTERN(int) git_remote_download(char **filename, git_remote *remote);

Member

nulltoken commented Jan 17, 2012

Ok, now git_remote_download throws AccessViolationException.

This reminds me of an AVE that @dahlbyk encountered in #77. Can you try and not declare filename as an out string and replace it with an IntPtr?

You can take a look at the way git_commit_message() (which also returns a string) is declared and used throughout the code. Beware that this case is slightly different as the returned commit message is a char* and the filename is a char**. You might (not sure about this) have to "unwrap" the IntPtr before invoking MarshallAsString().

@carlosmn How would you release from c# the allocated char array returned by git_remote_download()?

Contributor

uluhonolulu commented Jan 17, 2012

Do I still have to use "out" for IntPtr filename?

Contributor

uluhonolulu commented Jan 17, 2012

Both options still throw the same exception.

Member

nulltoken commented Jan 17, 2012

Both options still throw the same exception.

Hmm. You dropped the MarshalAs attribute, did you? Can you paste the stacktrace?

One last option might be to go the unsafe way with this last method by declaring a sbyte* filename and passing &filename as a parameter. You'd have to move the extern declaration from NativeMethods to UnsafeNativeMethods. You can peek at @tclem's Libgit2UnsafeHelper work for some inspiration.

Contributor

uluhonolulu commented Jan 17, 2012

    [DllImport(libgit2)]
    public static extern int git_remote_download(
        out IntPtr filename,
        RemoteSafeHandle remote);

TestCase 'LibGit2Sharp.Tests.RemoteFixture.TryToConnectToRemote'
failed: System.AccessViolationException : Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
at LibGit2Sharp.Core.NativeMethods.git_remote_download(IntPtr& filename, RemoteSafeHandle remote)
RemoteFixture.cs(47,0): at LibGit2Sharp.Tests.RemoteFixture.TryToConnectToRemote()

Contributor

uluhonolulu commented Jan 17, 2012

Gonna try this unsafe thing when I have more courage, err.. time.

Contributor

uluhonolulu commented Jan 18, 2012

Could you post the exact unsafe code you want me to try? I'm a complete C noob. Also, folks suggest trying various Charset and declaration types, but CDecl doesn't work (says no entry point), what you think?

Member

nulltoken commented Jan 18, 2012

Try something like this:

private unsafe string DownloadPackFileOrWhateverVeryNiceMethodNameYouCanThinkOf(RemoteSafeHandle remoteSafeHandle)
{
    sbyte* filename;

    int result = UnSafeNativeMethods.git_remote_download(&filename, remoteSafeHandle);
    Ensure.Success(result);

    return new string(filename);
}

Adding following declaration to UnSafeNativeMethods

[DllImport(libgit2)]
public static extern int git_remote_download(sbyte** filename, RemoteSafeHandle remoteSafeHandle);

It should work.

Contributor

uluhonolulu commented Jan 18, 2012

Well, it doesn't.

failed: System.AccessViolationException : Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
at LibGit2Sharp.Core.UnSafeNativeMethods.git_remote_download(SByte** filename, RemoteSafeHandle remoteSafeHandle)
Core\UnSafeNativeMethods.cs(25,0): at LibGit2Sharp.Core.UnSafeNativeMethods.DownloadPackFileOrWhateverVeryNiceMethodNameYouCanThinkOf(RemoteSafeHandle remoteSafeHandle)
RemoteFixture.cs(47,0): at LibGit2Sharp.Tests.RemoteFixture.TryToConnectToRemote()

Couldn't it be that the actual error happens inside the native function (just like it was with the git@ path)?

Member

nulltoken commented Jan 18, 2012

Couldn't it be that the actual error happens inside the native function (just like it was with the git@ path)?

You may be right. AccessViolationException doesn't smell good. I'm going to try and troubleshoot this this evening. In order for me to be able to reproduce your context, could you push a new branch in your forked repository with the current state of your code?

BTW, previous AVE has been fixed in core libgit2 (see libgit2/libgit2@d1317f1)

Owner

carlosmn commented Jan 18, 2012

It's perfectly possible that filename points to NULL. That's how the function tells you that you didn't need to download anything.

Member

nulltoken commented Jan 18, 2012

It's perfectly possible that filename points to NULL. That's how the function tells you that you didn't need to download anything.

@carlosmn Thanks for the clarification. I'll check this point as well.

Member

nulltoken commented Jan 19, 2012

@uluhonolulu I've pushed a new topic/fetch branch in my repo with a new binary release of libgit2 and a quick test which succeeds at downloading the libgit2 repo through the git protocol. It might help you go farther.

Note: I've discovered another slight issue testing the http protocol, dont't try it yet ;-)

Contributor

uluhonolulu commented Jan 19, 2012

I've pushed a new branch https://github.com/uluhonolulu/libgit2sharp/tree/fetch

The failing test is in the RemotFixture class. Had to make some methods/classes public in order to write it.

I'll check the new stuff later, got to leave.

Member

nulltoken commented Jan 19, 2012

@uluhonolulu I'd rather suggest you to pull from my repo and start from my topic\fetch branch. This will allow us to share a common passing test :)

Contributor

uluhonolulu commented Jan 19, 2012

Thanks, I finally got a chance to test it, got a green, moving on.

Member

nulltoken commented Jan 20, 2012

Thanks to @carlosmn, libgit2 is now able to download a packfile over http on Windows.

The current HEAD of Mono git repository has been "cloned" in 350 seconds with LibGit2Sharp!

Warning: no proxy support yet.

The nulltoken/topic/fetch has been accordingly updated.

Contributor

uluhonolulu commented Jan 20, 2012

Do you mean you can now fetch and clone with LibGit2Sharp?

Member

nulltoken commented Jan 21, 2012

Do you mean you can now fetch and clone with LibGit2Sharp?

Not yet ;-) I've only uncommented this line.

According to this C code sample, we still have to build the .idx file, rename the packfile, update the remote references and, of course, wrap this into a nice API . Still willing to give a a hand?

Contributor

uluhonolulu commented Jan 21, 2012

Absolutely.

Contributor

uluhonolulu commented Jan 22, 2012

Just thinkin,

Why do we implement Fetch() without implementing Clone() first?

Owner

carlosmn commented Jan 22, 2012

On Sun, 2012-01-22 at 05:00 -0800, Artem Smirnov wrote:

Just thinkin,

Why do we implement Fetch() without implementing Clone() first?

Fetch is a prerequisite for clone.

You can think of 'git clone git://github.com/libgit2/libgit2' as

mkdir libgit2 && cd libgit2 &&
git init -q && git remote add origin git://github.com/libgit2/libgit2 &&
git fetch origin && git checkout -tb master origin/master

It's a bit more complicated in a real clone command depending on options
and what the remote's default branch is, but the point is that it's not
a basic operation, and you need a working fetch to do it anyway.

cmn

Contributor

uluhonolulu commented Jan 25, 2012

Hmm I'm missing something here.

It looks like we've implemented everything for Pulling in our AwesomeTest, except for the last "checkout" command. However, I cannot run the checkout command manually (after hitting a breakpoint in the test, so no cleanup has been done yet).
git checkout -tb master origin/master tells me "updating paths is incompatible with switching branches."
git branch shows an empty list, although Bash shows me I'm on the master branch.
git checkout and git checkout -b master tells me "You are on a branch yet to be born".

Besides, how do I checkout with libgit2Sharp?

Owner

carlosmn commented Jan 25, 2012

On Wed, 2012-01-25 at 01:53 -0800, Artem Smirnov wrote:

Hmm I'm missing something here.

It looks like we've implemented everything for Pulling in our AwesomeTest, except for the last "checkout" command. However, I cannot run the checkout command manually (after hitting a breakpoint in the test, so no cleanup has been done yet).
git checkout -tb master origin/master tells me "updating paths is incompatible with switching branches."

[...]

git checkout and git checkout -b master tells me "You are on a branch yet to be born".

So you just run git init (or its equivalent) then? The repo is in a
delicate state. HEAD points to refs/heads/master, but that branch
doesn't exist. It will once you commit something (thus "yet to be
born"). It's a similar state to what you'd find yourself in if you run
git checkout --orphan somebrach.

You might find
http://thread.gmane.org/gmane.comp.version-control.git/188758
interesting.

In this case, git checkout can't do anything, because there is no
branch to do anything. git checkout -b master is probably failing
because you're telling git to create a branch 'master' based on the
current starting point, which doesn't exist.

Besides, how do I checkout with libgit2Sharp?

You can't yet. Support is coming in libgit2.

cmn

Member

nulltoken commented Jan 25, 2012

@uluhonolulu Have you tried "cheating" the context, before the checkout, by creating a refs/heads/master and make it point to the SHA of refs/remote/origin/HEAD?

$ git update-ref refs/heads/master refs/remotes/origin/HEAD
Owner

carlosmn commented Jan 25, 2012

Oh, right, that was following my command. Yeah, as I said it's a bit more complicated. The last step would be, once you've determined where the remote's HEAD points to (which you can only do during the fetch process), or some git ls-remote HEAD parsing, and stored it in $HEAD, relative to refs/heads/. Often this is master, but it could be anything.

git symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/$HEAD &&
git symbolic-ref HEAD $HEAD &&
git update-ref HEAD $(git rev-parse refs/remotes/origin/HEAD)
Member

nulltoken commented Jan 27, 2012

@uluhonolulu How are things going on the Fetch/Clone front? :)

Contributor

uluhonolulu commented Jan 27, 2012

Sorry I had no time for it. I'll try to figure what's going on on the weekend. Or do you mean change the return type to Repository?

As for Clone, I can't do it without libgit implementing Checkout.

Member

nulltoken commented Jan 27, 2012

As for Clone, I can't do it without libgit implementing Checkout.

The Checkout() method also lacks this feature. It currently only update the HEAD reference.
The Reset() also lacks this and only support Soft and Mixedmode.

Provided we add a // TODO: xxxx comment and a warning in the changelog, releasing the feature without this would still be very valuable.

Or do you mean change the return type to Repository?

Making Clone() return a Repository can be safely done as part of this PR.

Sorry I had no time for it. I'll try to figure what's going on on the weekend.

Great! Keep me updated should you have any concern.

Contributor

uluhonolulu commented Jan 28, 2012

I'm afraid I'm missing something here. Where's the Checkout method?

Member

nulltoken commented Jan 28, 2012

It's a method exposed by the BranchCollection type.

Contributor

uluhonolulu commented Jan 30, 2012

Something's wrong here. When I run the test:

            SelfCleaningDirectory scd = BuildSelfCleaningDirectory();
            string dir = Repository.Init(scd.DirectoryPath, false);

            using (var repo = new Repository(dir))
            {
                repo.Config.Set("remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*");
                repo.Config.Set("remote.origin.url", remoteLocation);

        repo.Fetch("origin");
//breakpoint here

and hit a breakpoint, the folder structure seems to be wrong. Instead of the refs\heads\master file, there's a refs\remotes\origin\refs\heads\master -- note that the origin folder contains the refs folder.

Now, when I move the master file to where it belongs and run the git checkout command manually, it works fine, and the files appear in my working tree.

Member

nulltoken commented Jan 30, 2012

There's a refs\remotes\origin\refs\heads\master -- note that the origin folder contains the refs folder.

Indeed, something's fishy here. I'll take a look at this this evening.

In order to help me reproduce the issue, can you confirm you ran the test against the tip of the topic/fetch branch in my repo? However, if you've made some additional changes, can you push them and point me to the commit? Thanks in advance.

Contributor

uluhonolulu commented Jan 30, 2012

No, it's the same old AwesomeTest, I just changed the repo to be non-bare so that I could compare it to the clone I did manually.

Member

nulltoken commented Jan 30, 2012

@uluhonolulu Reproduced. Thanks for having spotted this.

I'm migrating the test to good old C code (in order to dig a bit deeper) and will keep you updated.

Member

nulltoken commented Jan 31, 2012

@uluhonolulu The root cause has been found. @carlosmn is working on refactoring my piece of ... hack. Should land soon.

Instead of refs\remotes\origin\refs\heads\master, this should now generate refs\remotes\origin\master

Contributor

uluhonolulu commented Jan 31, 2012

I think I know how to write the files on disk. Should I create a separate branch for it? From which clone/branch?

Member

nulltoken commented Jan 31, 2012

I think I know how to write the files on disk.

You mean implementing the Checkout()? Wow! Beware, it's a tricky beast :)

Should I create a separate branch for it?

I think you can safely create a new branch from the tip of the vNext branch as this doesn't depend on the Init/Clone features. You could open the Standard repository from the test resources, checkout the branches/tags or event commit shas and assert the expected changes.

Contributor

uluhonolulu commented Jan 31, 2012

I'm sure Checkout is more than I know. For writing the files, I'm just taking a Tree and writing the Blob's recursively.

By the way, the first thing I tried was looking in the Index, but it was empty (I'm talking about the AwesomeTest). Is it by design?

Member

nulltoken commented Jan 31, 2012

By the way, the first thing I tried was looking in the Index, but it was empty (I'm talking about the AwesomeTest). Is it by design?

I think it's not updated yet. Off the top of my head, I'd say it's part of the Checkout responsibility. One way to do this would be, once the remotes references are set, to try and leverage the Reset() method in Mixed mode (still in the vNextbranch). This would populate the index with the Tree of a the commit object (see this test).

Owner

carlosmn commented Jan 31, 2012

A porcelainish checkout is not just grabbing a tree and putting it in the filesystem. That could lose data. It's more git read-tree -m $new && git checkout-index. The first part merges the tree you want to checkout with the existing data in the index/filesystem. Otherwise, you'd lose your uncommitted changes when you checked-out a different branch.

Contributor

uluhonolulu commented Jan 31, 2012

@carlosmn Thanks for the clarification. I'm trying to do a minimum that would make the Clone work, then I'll enhance it with other options/cases.

Do you have read-tree implemented in libgit?

@nulltoken So, do you recommend taking the list of files from the tree, or from the refreshed index? From what I understood, using the index is better in view of expected enhancements, right?

Member

nulltoken commented Jan 31, 2012

Do you have read-tree implemented in libgit?

@uluhonolulu Yes Sir ! Beware that it doesn't handle the merge. It blindly replaces the content of the Index. The Reset() method relies on this method to perform its task.

Take a look at this internal method from the Index

From what I understood, using the index is better in view of expected enhancements, right?

I think it would indeed be a better choice.

Owner

carlosmn commented Jan 31, 2012

You never[0] want to move data directly between the repo and the filesystem. In either direction, you have to go through the index. Doing otherwise is asking for trouble.

[0] There may be a few exceptions, but this is not one of them

Member

nulltoken commented Jan 31, 2012

@uluhonolulu

Instead of refs\remotes\origin\refs\heads\master, this should now generate refs\remotes\origin\master

It's now fixed, thanks to @carlosmn. topic/fetch branch has been updated with the latest binaries.

Contributor

uluhonolulu commented Feb 2, 2012

So, I'm reading filenames from the index, but file contents from the tree?

Member

nulltoken commented Feb 2, 2012

So, I'm reading filenames from the index, but file contents from the tree?

How about the following?

byte[] content = ((Blob)(tree["path/to/file"].Target)).Content;
Contributor

uluhonolulu commented Feb 2, 2012

Yes, that's what I thought.

Since it involves various stuff that logically belongs to the Repository class, how about moving the Checkout method to this class? Want me to create an issue?

Member

nulltoken commented Feb 2, 2012

Want me to create an issue?

Yes, please.

Contributor

uluhonolulu commented Feb 2, 2012

Need your advice on tests. I need a non-bare repo with two branches and a file that exists only in the test branch. Currently the standard test repo has only one branch. Adding a test branch broke 17 tests. I don't feel myself confident enough to fix them.

The easiest way would be to add another repo, but I'm not sure it's a good idea, cluttering the resources with multiple repositories. The ninja way would be to modify the existing repo inside the test, but we'll be testing too much (both commit and checkout) in a single test.

Actually the problem was with the way I added a branch -- I didn't know how to do it properly without changing the status of existing files (most of the test failures are related to file status). If you could tell me how to add a branch without ruining the current state, I think I could handle the rest.

Member

nulltoken commented Feb 2, 2012

I'd start by dynamically creating new branches targeting existing commits. The following test does just that and ensures that one of the branch (branch "one") lacks a file which exists in the other branch (branch "another").

[Test]
public void SmArtTest()
{
    TemporaryCloneOfTestRepo path = BuildTemporaryCloneOfTestRepo(StandardTestRepoPath);
    using (var repo = new Repository(path.RepositoryPath))
    {
        Branch one = repo.CreateBranch("one", "4a202b3");
        Branch another = repo.CreateBranch("another", "c47800c");

        Assert.IsNull(one.Tip["branch_file.txt"]);
        Assert.IsNotNull(another.Tip["branch_file.txt"]);                
    }
}
Contributor

uluhonolulu commented Mar 1, 2012

So, is Clone() implemented?

Member

nulltoken commented Mar 1, 2012

So, is Clone() implemented?

@uluhonolulu most of it, thanks to you ;-) The code currently sits in a branch in my repo. It's not complete yet as only the heads are being fetched. The tags are not.

@carlosmn has started to improve libgit2 code in order to make this happen. Once this works, the branch will be merged.

Contributor

uluhonolulu commented Mar 2, 2012

Great, I guess I'll use your branch in my project then.

How about Push, is there anything in libgit2 we could use?

Member

nulltoken commented Mar 2, 2012

Great, I guess I'll use your branch in my project then.

I'll ping you once the code is merged.

How about Push, is there anything in libgit2 we could use?

It's not available yet. As @carlosmn stated in the mlist "What we don't have is code to do a push because we'd need to bring over the object packer from git and plug it into the libgit2 methods, which is non-trivial."

Contributor

uluhonolulu commented Mar 2, 2012

The Fetch and Clone tests are failing for me. In particular, Fetch results in two branches, one is "refs/remotes/origin/HEAD", the other is "refs/remotes/origin/master". DO you want me to investigate this further?

Owner

carlosmn commented Mar 2, 2012

A fetch by itself shouldn't create refs/remotes/origin/HEAD, but a clone should.

Contributor

uluhonolulu commented Mar 4, 2012

Yes, but I presume HEAD is not a separate branch.

Owner

carlosmn commented Mar 4, 2012

refs/remotes/<remote>/HEAD is either a ref (if the remote is in detached HEAD) or a symbolic reference to the default branch on that remote.

Contributor

uluhonolulu commented Mar 4, 2012

Ok, but why after Fetch repo.Branches is two objects, both pointing to remote branches? Shouldn't it be one local branch?

Owner

carlosmn commented Mar 4, 2012

Fetch shouldn't be changing any local branches (you can force it to, but it's generally a bad idea). The default fetch refspec is refs/heads/*:refs/remotes/origin/*. If the remote has one branch, that gives you refs/remotes/origin/master and refs/remotes/origin/HEAD

Contributor

uluhonolulu commented Mar 4, 2012

What if I don't have the master branch locally, shouldn't fetch create it?

Owner

carlosmn commented Mar 4, 2012

That's not fetch's job. Fetch asks for the refs that match the left part of its fetch refpec(s) and stores them with the name given in the right part. If any local branches are created, such as in the case of git clone, that's something that happens after and independently[0] from the fetch.

[0] not completely, as we need the information that the remote end provided to guess which branch is the default one, but that information is the only thing that fetch does wrt creating a local branch.

Contributor

kaisellgren commented May 6, 2012

Short status update? (not trying to rush you guys :)

Member

nulltoken commented May 8, 2012

@kaisellgren

not trying to rush you guys :)

sure? ;p

The rework of the error handling mechanism in libgit2 kept things on hold for some time. @carlosmn has been hard at work with the streaming indexer.

As things are now a bit settled, we're fixing a few pending issues polishing the fetching code to make it perfect!

Contributor

uluhonolulu commented Jul 9, 2012

@kaisellgren for cloning, I'm using the code from @nulltoken's fork. Perfect or not, works for me. Unfortunately, I cannot submit an issue there ;).

Pushing is another issue as I understand. Needs to be implemented in libgit first. Big priority for me, but I'm incompatible with native code.

Contributor

kaisellgren commented Jul 9, 2012

@uluhonolulu @nulltoken how are/will the private keys be handled? Does the current cloning code at @nulltoken's fork work only with public repos?

Member

nulltoken commented Jul 9, 2012

NEWS

After he recently saved a cheerleader from squishy lizards and fuzzy rodents, the amazing @benstraub has decided to finish the job and save the world. At last. libgit2/libgit2#778 Should add native clone and checkout to libgit2.

Push is also being worked on. As @schu was a bit bored, he also decided to challenge his natural awesomeness and tackle this during the summer. With a blindfold. And a hand tied in his back. In order to keep this task a bit challenging to him.

Contributor

uluhonolulu commented Jul 10, 2012

We're going to have a pretty psychedelic library ;)

Contributor

uluhonolulu commented Jul 31, 2012

So, the libgit binaries containing the native clone implementation are now in LibGit2Sharp? Which branch?

Member

nulltoken commented Jul 31, 2012

@uluhonolulu It's not available yet. @benstraub Is still working on it. You can peek at his work by subscribing to the notifications of libgit2/libgit2#778.

Contributor

uluhonolulu commented Jul 31, 2012

Yes I'm watching it already, thought he's improving it in the main repo.

Member

ben commented Aug 1, 2012

I've been keeping this on my own fork, that seems to be more idiomatic of open-source projects on GitHub.

It's mostly ready; pull from my fork, checkout the clone branch, and try out the network example: ./git2 clone http://github.com/libgit2/libgit2 ./libgit2. Leave a note on the PR if you have feedback!

Contributor

uluhonolulu commented Aug 2, 2012

I'm a managed guy, the mere idea of having to build a native library scares me. If I could get my hands on the binary, I'd volunteer to write a managed wrapper around it. But I guess it doesn't make much sense until it's merged into the main repo.

Member

nulltoken commented Oct 17, 2012

The amazing @jamill did it! Fetch() has landed in vNext

Member

nulltoken commented Nov 12, 2012

Quick update, thanks to the amazing job done by @ben and @jamill, the following features are now available:

  • Fetch()
  • Clone()
  • Checkout()

jbjorge commented Feb 14, 2013

Are there any plans for merge support in the near future?

Member

nulltoken commented Feb 14, 2013

Are there any plans for merge support in the near future?

@jbjorge Related bits and pieces are regularly being committed by @ethomson

Owner

ethomson commented Feb 14, 2013

@jbjorge I can't answer that question without knowing your definition of "near". So instead I'll just give you a broad overview of where things are.

In short, though, yes, I hope so. It's an area that I'm actively involved in. Merge is being added to libgit2 libgit2/libgit2#1007. Once it lands there, obviously, it will land in libgit2sharp not too terribly long thereafter.

I needed to take a brief pause in order to help get the Visual Studio tooling (aka #git4vs) stuff out the door. While most everybody else was actually working, I did have to fix a few bugs in, well, merge, actually. (And I was out of the office last week.)

So you'll note an increase in activity here trying to get that branch integrated. I'm breaking git_merge_trees() out into its own PR at the moment, and you're seeing libgit2sharp reaction work like @nulltoken just pointed out. Having merge out in its own branch is a huge technical debt for us (me) and a significant source of pain for us (me) so it's certainly something that we're pushing for.

Any news here?

kierun commented Jul 9, 2013

What's the status of this?

Member

nulltoken commented Jul 9, 2013

@powercode, @kierun As stated by @ethomson above, merge is still a work in progress.

Some lower level features have already been merged in libgit2, though (cf. libgit2/libgit2#1592 , libgit2/libgit2#1389 , libgit2/libgit2#1185 , libgit2/libgit2#, libgit2/libgit2#1011 and some other minor ones).

Some of those bits have already been made available to LibGit2Sharp (see MergeFixture, CommitFixture, ConflictFixture, CheckoutFixture).

The main PR to follow is libgit2/libgit2#1007.

jorpheus commented Aug 7, 2013

I'm using LibGit2Sharp.0.13.0.0 from NuGet and understand that "Pull" isn't yet implemented, however is it possible to "mimick" a pull using different (plumbing) functions that are currently available in the API? or is my only option "shelling out" to do the pull? seems a shame when Clone() and Push() work as expected.

Member

dahlbyk commented Aug 7, 2013

fetch is supported (see FetchFixture), otherwise see @nulltoken's previous comment.

Member

jamill commented Aug 7, 2013

Merge is still being developed in libgit2 (which would be required to do "Pull"). I believe the PR to follow is still libgit2/libgit2#1007.

Member

nulltoken commented Jan 16, 2014

Merge has landed! See #608

nulltoken referenced this issue Feb 22, 2014

Closed

Merge? #630

Member

nulltoken commented Mar 6, 2014

@synhershko ~900 days later, it looks like we may close this feature request in a near future. It looks like @jamill is making Pull() happen in #643.

🎉

@nulltoken Amazing :)

Member

nulltoken commented Apr 10, 2014

Fixed by #643

nulltoken closed this Apr 10, 2014

nulltoken added this to the v0.17.0 milestone Apr 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment