Skip to content
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

Replace git command update-ref by a LibGit2Sharp call to UpdateRef() #356

Merged
merged 3 commits into from
May 8, 2013

Conversation

pmiossec
Copy link
Member

PS : there is one more not replaced because we use "update-ref" with a comment.
Perhaps we should do the replacement anyway. If you judge, so, do it... ;)

@sc68cal
Copy link
Contributor

sc68cal commented Apr 23, 2013

😍

@spraints
Copy link
Member

@pmiossec this should finish off the other git update-ref. If it builds, could you push that commit onto your branch?

@pmiossec
Copy link
Member Author

@spraints I work quite some time on your commit (trying to correct the build) before figuring out what you trying to do...

  • you forgot to commit the update of the libgit2sharp submodule.
  • the method Append() that you try to call on the RefLogCollection is internal and couldn't be called :(
  • if we succeed on that, there is some stuff to fix.
  • if we update libgit2sharp submodule (I did it to the last version), there is lot of things to fix, too.

@nulltoken
Copy link

FWIW, I discussed this topic with @Saaman.

We don't plan on making the RefLogCollection.Append() method public.

However, the repo.Refs.UpdateTarget() will (soon-ish) accept an optional parameter logMessage which should make its behavior more compliant with what git update-ref actually does.

Hope this helps.

@spraints
Copy link
Member

@nulltoken - cool. Is someone working on the repo.refs.UpdateTarget() change, or is that something I could contribute (if I get to it soon-ish)? Also, this seems like it would be genrally useful. Should it be in libgit2?

@Saaman
Copy link

Saaman commented Apr 24, 2013

I intend to start this on friday. It should be finished for the middle of next week maximum I guess. I don't know if it is soon-ish enough for you...

@nulltoken
Copy link

Also, this seems like it would be generally useful. Should it be in libgit2?

Eventually, yes. Along with most of the reflog related work as well. The plan is to make it quickly available to .Net/Mono clients with a reasonable good test coverage. Let it live in the wild for a wild and ... fix bugs, add some more tests ;-)

Once it's stable enough, port the code to C, replace the C# implementation with call to native methods, ensuring that no regression has happened thanks to the LibGit2Sharp reflog tests.

@spraints
Copy link
Member

@Saaman sounds good to me!

@spraints
Copy link
Member

@pmiossec I updated my branch (using VS this time :trollface:), and Process.Start is gone for update-ref. It has some crlf 🔥 issues, though.

@nulltoken and @Saaman - Would it make sense to add an optional log message to Refs.Add(), too? Here's my code right now:

        public void UpdateRef(string gitRefName, string shaCommit, string message = null)
        {
            if (message == null)
            {
                _repository.Refs.Add(gitRefName, new ObjectId(shaCommit), true);
            }
            else
            {
                if (_repository.Refs[gitRefName] == null)
                {
                    _repository.Refs.Add(gitRefName, new ObjectId(shaCommit));
                }

                _repository.Refs.UpdateTarget(gitRefName, shaCommit, message);
            }
        }

I'm not sure that Add+UpdateTarget is going to actually make me a reflog entry for the first commit. Would it be reasonable make Add look like public DirectReference Add(string name, ObjectId targetId, bool allowOverwrite = false, string logMessage = null)? This should make git-tfs's UpdateRef look like this:

        public void UpdateRef(string gitRefName, string shaCommit, string message = null)
        {
            _repository.Refs.Add(gitRefName, newObjectId(shaCommit), allowOvewrite: true, message: message);
        }

@Saaman
Copy link

Saaman commented Apr 30, 2013

I see your point. I tested it with git.git, it seems to create the reference and log it into the reflog, so Refs.Add() should do the same, following your signature proposal.

I will implement it in the next few days.

@spraints spraints merged commit 09606b2 into git-tfs:master May 8, 2013
spraints referenced this pull request May 8, 2013
Replace Process.Start with libgit2sharp. (closes #360)
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.

None yet

5 participants