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

Implement an Elisp binding for libgit2 #2959

Open
tarsius opened this Issue Jan 12, 2017 · 83 comments

Comments

@tarsius
Member

tarsius commented Jan 12, 2017

This description was taken from #2956. I intend to replace it with a more in-depth description at a later time.

Magit is slow and part of fixing that involves the use of libgit2, "a portable, pure C implementation of the Git core methods provided as a re-entrant linkable library with a solid API, allowing you to write native speed custom Git applications in any language which supports C bindings." Unfortunately nobody has written that for Elisp yet and since improving performance is a top priority now, I'll to it.

This will be named just libgit.el (or libgit2.el) and be pretty basic, i.e. just expose the functions provided by libgit2 to Elisp.


Older discussions: #2539, #2442 (comment), #1327 (comment). (Yes, this goes back a while, but note that doing this is only even possible since Emacs v25.1, which was released in September 2016.)


Some resources:

And of course...

@jwiegley

This comment has been minimized.

Show comment
Hide comment
@jwiegley

jwiegley May 30, 2017

Contributor

@tarsius If you have any libgit2 related questions, feel free to ask me. I've used it extensively, both for personal projects and in production, and I maintain the Haskell bindings to libgit2.

Contributor

jwiegley commented May 30, 2017

@tarsius If you have any libgit2 related questions, feel free to ask me. I've used it extensively, both for personal projects and in production, and I maintain the Haskell bindings to libgit2.

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius May 30, 2017

Member

That's awesome to hear! I suspect you also have some experience with the new module support. I intend to get started with this soon, but could definitely need some help.

Member

tarsius commented May 30, 2017

That's awesome to hear! I suspect you also have some experience with the new module support. I intend to get started with this soon, but could definitely need some help.

@jwiegley

This comment has been minimized.

Show comment
Hide comment
@jwiegley

jwiegley May 30, 2017

Contributor

I haven't yet looked into the module support, but this would be a great way to learn it. Count me in!

Contributor

jwiegley commented May 30, 2017

I haven't yet looked into the module support, but this would be a great way to learn it. Count me in!

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Sep 1, 2017

Contributor

@jwiegley I'm not current on emacs-devel, but have we thought at all about how modules will be distributed? Is package.el planning support for them?

I ask since they impact the structure of the implementation and I've been thinking about starting that project up again personally (now that magithub is stable-ish).

Contributor

vermiculus commented Sep 1, 2017

@jwiegley I'm not current on emacs-devel, but have we thought at all about how modules will be distributed? Is package.el planning support for them?

I ask since they impact the structure of the implementation and I've been thinking about starting that project up again personally (now that magithub is stable-ish).

@mgalgs

This comment has been minimized.

Show comment
Hide comment
@mgalgs

mgalgs Sep 2, 2017

Contributor

part of fixing that involves the use of libgit2

I would love to see some data to back up that claim. It sounds right to me, but it would be a shame for you to spend precious time on an optimization that might not bear fruit as anticipated. Perhaps we could start with just a minimal implementation of the libgit2 bindings and do some benchmarks to prove the concept.

I'm guessing you've already thought this through... But it would be nice to see the data. I can help out if there's any dividing and conquering that can be done.

Contributor

mgalgs commented Sep 2, 2017

part of fixing that involves the use of libgit2

I would love to see some data to back up that claim. It sounds right to me, but it would be a shame for you to spend precious time on an optimization that might not bear fruit as anticipated. Perhaps we could start with just a minimal implementation of the libgit2 bindings and do some benchmarks to prove the concept.

I'm guessing you've already thought this through... But it would be nice to see the data. I can help out if there's any dividing and conquering that can be done.

@jwiegley

This comment has been minimized.

Show comment
Hide comment
@jwiegley

jwiegley Sep 3, 2017

Contributor

This is a good question, and maybe we'll be the first ones to answer it for future package authors too. I just don't know yet. :)

Contributor

jwiegley commented Sep 3, 2017

This is a good question, and maybe we'll be the first ones to answer it for future package authors too. I just don't know yet. :)

@jwiegley

This comment has been minimized.

Show comment
Hide comment
@jwiegley

jwiegley Sep 3, 2017

Contributor

@mgalgs If someone can show me a set of commands that are being used by magit, and which are presumed to be slow, I can tell you how libgit2 might affect the performance there and why. It's possible too that we could use more caching, and more low-level Git commands (for example, direct tree manipulation) to defer going the libgit2 route.

Contributor

jwiegley commented Sep 3, 2017

@mgalgs If someone can show me a set of commands that are being used by magit, and which are presumed to be slow, I can tell you how libgit2 might affect the performance there and why. It's possible too that we could use more caching, and more low-level Git commands (for example, direct tree manipulation) to defer going the libgit2 route.

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Sep 3, 2017

Contributor

@mgalgs @tarsius From my memory of prior conversations, rev-parse is probably the biggest single hitter. See also ksjogo/emacs-libgit2#4.

Contributor

vermiculus commented Sep 3, 2017

@mgalgs @tarsius From my memory of prior conversations, rev-parse is probably the biggest single hitter. See also ksjogo/emacs-libgit2#4.

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Sep 3, 2017

Member

From my memory of prior conversations, rev-parse is probably the biggest single hitter.

... because we call it a lot. So it would be a good idea to implement support for that first.

If someone can show me a set of commands that are being used by magit, and which are presumed to be slow,

The problem isn't that certain git commands are slow on Windows, but that starting subprocesses is slow per se and Magit starts many.

It's possible too that we could use more caching,

We already do a lot of caching. Identical calls (same arguments and directory) during a single refresh (i.e. after every Magit command) get the value from a cache. I don't think there is much room for improvement here. Well there is--see #2982--but that goes much further than just a stupid cache.

and more low-level Git commands

There have been some reports that e.g. rebasing can be slow on Windows, I think. But we cannot do much about that--I certainly don't want to reimplement every Git command that is still implemented as a shell script.

However a few months ago a similar (but much less severe) instance of "starting a subprocess is slow" was fixed, but only on macOS/Darwin. I am hoping that something similar can be done on Windows.

Unfortunately I never got around asking the right people for help. We should dig up the old discussions and then bring those to their attention. The issue on macOS was that "the wrong fork" was being used and since that was being done for a very long time on macOS, the same thing might very well be true on Windows also.

But even if that gives us (not just Magit, but any package that uses many subprocesses) an amazing performance boost, I would still like to be able to use libgit from elisp.

Member

tarsius commented Sep 3, 2017

From my memory of prior conversations, rev-parse is probably the biggest single hitter.

... because we call it a lot. So it would be a good idea to implement support for that first.

If someone can show me a set of commands that are being used by magit, and which are presumed to be slow,

The problem isn't that certain git commands are slow on Windows, but that starting subprocesses is slow per se and Magit starts many.

It's possible too that we could use more caching,

We already do a lot of caching. Identical calls (same arguments and directory) during a single refresh (i.e. after every Magit command) get the value from a cache. I don't think there is much room for improvement here. Well there is--see #2982--but that goes much further than just a stupid cache.

and more low-level Git commands

There have been some reports that e.g. rebasing can be slow on Windows, I think. But we cannot do much about that--I certainly don't want to reimplement every Git command that is still implemented as a shell script.

However a few months ago a similar (but much less severe) instance of "starting a subprocess is slow" was fixed, but only on macOS/Darwin. I am hoping that something similar can be done on Windows.

Unfortunately I never got around asking the right people for help. We should dig up the old discussions and then bring those to their attention. The issue on macOS was that "the wrong fork" was being used and since that was being done for a very long time on macOS, the same thing might very well be true on Windows also.

But even if that gives us (not just Magit, but any package that uses many subprocesses) an amazing performance boost, I would still like to be able to use libgit from elisp.

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Sep 3, 2017

Contributor

For anyone who's curious, I've implemented a type of benchmark in this gist. What I've done is I've redirected magit-git-executable to a shell script that logs the input and times it. I've got some elisp also that processes that; once I'm done running errands, I'll be doing more processing of that log output so we can say with certainty how long we spend doing git commands.

Contributor

vermiculus commented Sep 3, 2017

For anyone who's curious, I've implemented a type of benchmark in this gist. What I've done is I've redirected magit-git-executable to a shell script that logs the input and times it. I've got some elisp also that processes that; once I'm done running errands, I'll be doing more processing of that log output so we can say with certainty how long we spend doing git commands.

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Sep 4, 2017

Contributor

More useful metrics would have to correlate these data with the timestamps of actual magit commands, but here are my own numbers (using the approach and code above) after using magit to review some history. First column is number of calls, second column is total time taken by that command.

231  1.6963  "rev-parse --show-toplevel"
196  1.4190  "rev-parse --show-cdup"
 19  0.2855  "show -p --cc --format=%n --no-prefix --numstat --stat --no-ext-diff <short-hash>^{commit} --"
 19  0.2591  "branch --merged <short-hash>"
 19  0.2483  "branch --contains <short-hash>"
 19  0.2398  "show --no-patch --format=%d --decorate=full <short-hash>^{commit} --"
 19  0.1996  "describe --contains <short-hash>"
 19  0.1855  "describe --long --tags <short-hash>"
 19  0.1669  "show --no-patch --format=Author:     %aN <%aE>\nAuthorDate: %ad\nCommit:     %cN <%cE>\nCommitDate: %cd\n <short-hash>^{commit} --"
 19  0.1601  "show --no-patch --format=%h %s <long-hash>^{commit} --"
 19  0.1584  "rev-parse --verify <short-hash>^{commit}"
 19  0.1535  "show --no-patch --format=%B <short-hash>^{commit} --"
 19  0.1506  "rev-parse <short-hash>^{commit}"
 19  0.1495  "rev-list -1 --parents <short-hash>"
 19  0.1485  "cat-file -t <short-hash>"
 19  0.1472  "notes show <short-hash>"
...
Contributor

vermiculus commented Sep 4, 2017

More useful metrics would have to correlate these data with the timestamps of actual magit commands, but here are my own numbers (using the approach and code above) after using magit to review some history. First column is number of calls, second column is total time taken by that command.

231  1.6963  "rev-parse --show-toplevel"
196  1.4190  "rev-parse --show-cdup"
 19  0.2855  "show -p --cc --format=%n --no-prefix --numstat --stat --no-ext-diff <short-hash>^{commit} --"
 19  0.2591  "branch --merged <short-hash>"
 19  0.2483  "branch --contains <short-hash>"
 19  0.2398  "show --no-patch --format=%d --decorate=full <short-hash>^{commit} --"
 19  0.1996  "describe --contains <short-hash>"
 19  0.1855  "describe --long --tags <short-hash>"
 19  0.1669  "show --no-patch --format=Author:     %aN <%aE>\nAuthorDate: %ad\nCommit:     %cN <%cE>\nCommitDate: %cd\n <short-hash>^{commit} --"
 19  0.1601  "show --no-patch --format=%h %s <long-hash>^{commit} --"
 19  0.1584  "rev-parse --verify <short-hash>^{commit}"
 19  0.1535  "show --no-patch --format=%B <short-hash>^{commit} --"
 19  0.1506  "rev-parse <short-hash>^{commit}"
 19  0.1495  "rev-list -1 --parents <short-hash>"
 19  0.1485  "cat-file -t <short-hash>"
 19  0.1472  "notes show <short-hash>"
...
@chriscool

This comment has been minimized.

Show comment
Hide comment
@chriscool

chriscool Sep 26, 2017

Be careful with libgit2 as I don't think it implements file locks in a compatible way with Git itself. For example if git gc is run in the background and libgit2 is doing things at the same time, there could be problems.

chriscool commented Sep 26, 2017

Be careful with libgit2 as I don't think it implements file locks in a compatible way with Git itself. For example if git gc is run in the background and libgit2 is doing things at the same time, there could be problems.

@jwiegley

This comment has been minimized.

Show comment
Hide comment
@jwiegley

jwiegley Sep 26, 2017

Contributor

Based on issues like libgit2/libgit2#2902, I think the developers both think about these sorts of issues, and would be open to bug reports about them.

Contributor

jwiegley commented Sep 26, 2017

Based on issues like libgit2/libgit2#2902, I think the developers both think about these sorts of issues, and would be open to bug reports about them.

@chriscool

This comment has been minimized.

Show comment
Hide comment
@chriscool

chriscool Sep 26, 2017

Maybe but in general I think libgit2 development is lagging behind Git development. See:
https://github.com/git/git/graphs/contributors
https://github.com/libgit2/libgit2/graphs/contributors
(Disclosure: I am a Git developer)

chriscool commented Sep 26, 2017

Maybe but in general I think libgit2 development is lagging behind Git development. See:
https://github.com/git/git/graphs/contributors
https://github.com/libgit2/libgit2/graphs/contributors
(Disclosure: I am a Git developer)

@jwiegley

This comment has been minimized.

Show comment
Hide comment
@jwiegley

jwiegley Sep 26, 2017

Contributor

I can certainly believe that.

Contributor

jwiegley commented Sep 26, 2017

I can certainly believe that.

@tarsius tarsius added the abstraction label Dec 10, 2017

@ubolonton

This comment has been minimized.

Show comment
Hide comment
@ubolonton

ubolonton Dec 23, 2017

Contributor

I started an experimental module for this https://github.com/ubolonton/magit-libgit2

It currently advises magit-rev-parse to use libgit2 where possible.

Some notes:

  • A quick benchmark on my laptop showed 40x speedup for that function. I'm going to check if the difference can be perceived in daily uses.
  • We should probably add some automated benchmarks, ideally integrated with CI, to identify slow parts.
  • Writing the module in Rust is quite nice. The tooling is good, and I got a live reloading setup going.
  • We can start with implementing only functionalities needed by magit. A generic libgit2.el can be extracted much later on.
Contributor

ubolonton commented Dec 23, 2017

I started an experimental module for this https://github.com/ubolonton/magit-libgit2

It currently advises magit-rev-parse to use libgit2 where possible.

Some notes:

  • A quick benchmark on my laptop showed 40x speedup for that function. I'm going to check if the difference can be perceived in daily uses.
  • We should probably add some automated benchmarks, ideally integrated with CI, to identify slow parts.
  • Writing the module in Rust is quite nice. The tooling is good, and I got a live reloading setup going.
  • We can start with implementing only functionalities needed by magit. A generic libgit2.el can be extracted much later on.
@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Dec 25, 2017

Member

@ubolonton I am taking a two week break, but am excited to look at this when I get back.

Member

tarsius commented Dec 25, 2017

@ubolonton I am taking a two week break, but am excited to look at this when I get back.

@tarsius tarsius added this to the 2.17.0 milestone Mar 29, 2018

@TheBB

This comment has been minimized.

Show comment
Hide comment
@TheBB

TheBB Apr 20, 2018

Hi everyone,

I was interested in working on this a bit. From what I see there are two cases of "prior art":

I played around with my own module here: https://github.com/TheBB/libegit2

Now since this is a relatively ambitious project, I'm wary to invite further fracturing, but in my defense, (a) I'm not comfortable with rust, (b) @ksjogo's repo seems abandoned, and (c) I had fun anyway.

I'm aiming for a thin wrapper. If you're familiar with PyQt, it's possible to read Qt's C++ documentation and translate directly to Python. That's the level I'd like to aim for: you can read libgit2's C documentation and use it directly from Emacs with no go-between.

I haven't yet tried to get magit to play with this module.

What's the current status on your side? Should I continue working on this?

TheBB commented Apr 20, 2018

Hi everyone,

I was interested in working on this a bit. From what I see there are two cases of "prior art":

I played around with my own module here: https://github.com/TheBB/libegit2

Now since this is a relatively ambitious project, I'm wary to invite further fracturing, but in my defense, (a) I'm not comfortable with rust, (b) @ksjogo's repo seems abandoned, and (c) I had fun anyway.

I'm aiming for a thin wrapper. If you're familiar with PyQt, it's possible to read Qt's C++ documentation and translate directly to Python. That's the level I'd like to aim for: you can read libgit2's C documentation and use it directly from Emacs with no go-between.

I haven't yet tried to get magit to play with this module.

What's the current status on your side? Should I continue working on this?

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Apr 20, 2018

Member

I'm aiming for a thin wrapper. [...] That's the level I'd like to aim for: you can read libgit2's C documentation and use it directly from Emacs with no go-between.

That's exactly what I was hoping for and would have done eventually if you didn't beat me to it. But it would probably have taken me much longer than someone more familiar with C.

So far this is pretty incomplete but it is very promising that you have already outlined your plans on what you do or don't intend to implement and that you have added documentation that allows others to contribute.

I think I am going to add this to Emacs.g very soon - not just to the magit-directors-cut branch, but master.

I haven't yet tried to get magit to play with this module.

I have only played with it a tiny bit. But that already confirms that this is easily installable (when using borg 😀 ). Also I already ran into the first problem: you probably want to expand-file-name all paths before handing them to libgit2 so that not every caller has to do it (git and libgit2 don't understand ~/).

I am already quite sure that this is what I am going to use in Magit (*). If you would like to do that too, then I would like to welcome this project into the magit "organization". Combined with your instructions, that might help encourage contributions.

Beside the need for greater coverage, I think the most important tasks ahead are:

  1. Reconsider the symbol prefix. While this is the package that most deserves the git- prefix (especially now that the git.el that used to be part of Git itself has been removed), this is going to lead to a lot of conflicts with many existing packages and that could lead to a lot of unnecessary work. What about libgit-, lid-, lgit- or egit? (lgit was the name of my own pre-magit git library, and egit is a very old abandoned magit competitor of sorts.)

  2. Make it easily installable from Melpa. Again its important to get this right or else the price has to be payed later in the form of having to help lots of lost users.


(*) I don't want to discourage other efforts though. (By the way, @ubolonton sorry for not getting back to you.) But I do favor this implementation not least because @TheBB has maintained other important Emacs projects before and because, as I said, his approach is pretty much what I had hoped for. It also has less of a proof-of-concept feel to it.

Member

tarsius commented Apr 20, 2018

I'm aiming for a thin wrapper. [...] That's the level I'd like to aim for: you can read libgit2's C documentation and use it directly from Emacs with no go-between.

That's exactly what I was hoping for and would have done eventually if you didn't beat me to it. But it would probably have taken me much longer than someone more familiar with C.

So far this is pretty incomplete but it is very promising that you have already outlined your plans on what you do or don't intend to implement and that you have added documentation that allows others to contribute.

I think I am going to add this to Emacs.g very soon - not just to the magit-directors-cut branch, but master.

I haven't yet tried to get magit to play with this module.

I have only played with it a tiny bit. But that already confirms that this is easily installable (when using borg 😀 ). Also I already ran into the first problem: you probably want to expand-file-name all paths before handing them to libgit2 so that not every caller has to do it (git and libgit2 don't understand ~/).

I am already quite sure that this is what I am going to use in Magit (*). If you would like to do that too, then I would like to welcome this project into the magit "organization". Combined with your instructions, that might help encourage contributions.

Beside the need for greater coverage, I think the most important tasks ahead are:

  1. Reconsider the symbol prefix. While this is the package that most deserves the git- prefix (especially now that the git.el that used to be part of Git itself has been removed), this is going to lead to a lot of conflicts with many existing packages and that could lead to a lot of unnecessary work. What about libgit-, lid-, lgit- or egit? (lgit was the name of my own pre-magit git library, and egit is a very old abandoned magit competitor of sorts.)

  2. Make it easily installable from Melpa. Again its important to get this right or else the price has to be payed later in the form of having to help lots of lost users.


(*) I don't want to discourage other efforts though. (By the way, @ubolonton sorry for not getting back to you.) But I do favor this implementation not least because @TheBB has maintained other important Emacs projects before and because, as I said, his approach is pretty much what I had hoped for. It also has less of a proof-of-concept feel to it.

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Apr 20, 2018

Member

Pinging some people who might be interested in contributing to this effort - @jwiegley @vermiculus @mgalgs @chriscool.

Member

tarsius commented Apr 20, 2018

Pinging some people who might be interested in contributing to this effort - @jwiegley @vermiculus @mgalgs @chriscool.

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Apr 20, 2018

Member

(I've added some useful resources to my initial post above.)

Member

tarsius commented Apr 20, 2018

(I've added some useful resources to my initial post above.)

@ylluminarious

This comment has been minimized.

Show comment
Hide comment
@ylluminarious

ylluminarious Oct 10, 2018

@TheBB Thanks for the info. I myself probably won't have much time to contribute to this in the near future (although I'd like to), but thanks a lot for pointing to the README. After looking at it more thoroughly (rather than glancing through it), you're right; it provides good info on how to get started. Hopefully this ball can get rolling again soon. I'm sure we're all looking forward to a faster Magit!

ylluminarious commented Oct 10, 2018

@TheBB Thanks for the info. I myself probably won't have much time to contribute to this in the near future (although I'd like to), but thanks a lot for pointing to the README. After looking at it more thoroughly (rather than glancing through it), you're right; it provides good info on how to get started. Hopefully this ball can get rolling again soon. I'm sure we're all looking forward to a faster Magit!

@brotzeit

This comment has been minimized.

Show comment
Hide comment
@brotzeit

brotzeit Oct 10, 2018

Contributor

I would like to do it, but I won't have the time :/
Maybe post it on reddit to see if somebody is interested in implementing it ? Worth a try...

Contributor

brotzeit commented Oct 10, 2018

I would like to do it, but I won't have the time :/
Maybe post it on reddit to see if somebody is interested in implementing it ? Worth a try...

@ylluminarious

This comment has been minimized.

Show comment
Hide comment
@ylluminarious

ylluminarious Oct 10, 2018

Agreed. A "Help Wanted" thread on reddit could be useful. /r/emacs would be the place to post it.

ylluminarious commented Oct 10, 2018

Agreed. A "Help Wanted" thread on reddit could be useful. /r/emacs would be the place to post it.

@luismbo

This comment has been minimized.

Show comment
Hide comment
@luismbo

luismbo Oct 10, 2018

Contributor

FWIW, I'm interested in either funding someone or being funded myself to do this.

Contributor

luismbo commented Oct 10, 2018

FWIW, I'm interested in either funding someone or being funded myself to do this.

@TheBB

This comment has been minimized.

Show comment
Hide comment
@TheBB

TheBB Oct 10, 2018

Thanks for the tip, I'll write a call to arms on reddit tonight.

TheBB commented Oct 10, 2018

Thanks for the tip, I'll write a call to arms on reddit tonight.

@brotzeit

This comment has been minimized.

Show comment
Hide comment
@brotzeit

brotzeit Oct 10, 2018

Contributor

@luismbo Sounds good. How much would you expect ?

Contributor

brotzeit commented Oct 10, 2018

@luismbo Sounds good. How much would you expect ?

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Oct 10, 2018

Member

I'm going to create a libgit branch in the repository over the next two days or so.

Member

tarsius commented Oct 10, 2018

I'm going to create a libgit branch in the repository over the next two days or so.

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Oct 10, 2018

Member

I just realized that libgit won't work over Tramp. It's quite amazing that I, and apparently nobody else, realized that before. That's very bad news. It means that for every magit function that we port to libgit we have to keep the old implementation around. And then we need wrappers to dispatch the proper implementation (probably based on the file-handler functionality). That's going to be a lot of work and the resulting bloat will be with us forever. Or we drop support for Tramp. We are stuck between a rock and a hard place.

Member

tarsius commented Oct 10, 2018

I just realized that libgit won't work over Tramp. It's quite amazing that I, and apparently nobody else, realized that before. That's very bad news. It means that for every magit function that we port to libgit we have to keep the old implementation around. And then we need wrappers to dispatch the proper implementation (probably based on the file-handler functionality). That's going to be a lot of work and the resulting bloat will be with us forever. Or we drop support for Tramp. We are stuck between a rock and a hard place.

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Oct 10, 2018

Contributor

Why can't libgit work over Tramp? Is that something that can be accounted for in the module?

Contributor

vermiculus commented Oct 10, 2018

Why can't libgit work over Tramp? Is that something that can be accounted for in the module?

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Oct 10, 2018

Member

For the same reason Tramp has to use the ls executable instead of the opendir() and readdir() functions to list the contents of a remote directory.

The emacs process runs on the local machine. Adding bindings for libgit2 teaches that process new tricks, but making system calls on another machine is not one of them. That may be possible with some academic Erlang OS, but not with Linux, *BSD, macOS, or Windows.

Currently when Magit needs to run git "over Tramp" it does that by running git on the other machine. That's how Tramp works -- it runs processes on the other machine and then does something with the output of those processes.

The equivalent with a libgit-enabled emacs would be to run another such emacs instance on the other machine. That would defeat the purpose of Tramp at least for one popular use-case: do stuff on another machine which doesn't have Emacs installed, from the comfort of the local Emacs. (And obviously starting many emacs processes would be much slower than starting many git processes, so this is a non-starter.)

Member

tarsius commented Oct 10, 2018

For the same reason Tramp has to use the ls executable instead of the opendir() and readdir() functions to list the contents of a remote directory.

The emacs process runs on the local machine. Adding bindings for libgit2 teaches that process new tricks, but making system calls on another machine is not one of them. That may be possible with some academic Erlang OS, but not with Linux, *BSD, macOS, or Windows.

Currently when Magit needs to run git "over Tramp" it does that by running git on the other machine. That's how Tramp works -- it runs processes on the other machine and then does something with the output of those processes.

The equivalent with a libgit-enabled emacs would be to run another such emacs instance on the other machine. That would defeat the purpose of Tramp at least for one popular use-case: do stuff on another machine which doesn't have Emacs installed, from the comfort of the local Emacs. (And obviously starting many emacs processes would be much slower than starting many git processes, so this is a non-starter.)

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Oct 10, 2018

Contributor

Couldn't the module run git remotely if it must? I'm thinking a normal remote execution over something like ssh. That should provide some output that could be parsed into the appropriate data structures.

Contributor

vermiculus commented Oct 10, 2018

Couldn't the module run git remotely if it must? I'm thinking a normal remote execution over something like ssh. That should provide some output that could be parsed into the appropriate data structures.

@phil-s

This comment has been minimized.

Show comment
Hide comment
@phil-s

phil-s Oct 11, 2018

Contributor

@vermiculus, are you suggesting that if it did that and returned data which was equivalent to what it would return if it could use libgit, then Magit wouldn't need to worry about maintaining multiple ways of doing things in the future?

I guess that might work, but it seems to me like it's just shifting the burden of the legacy approach from Magit (where it is already implemented and working) into the C module (where it is not implemented at all), so I'm not sure that's the best approach?

Contributor

phil-s commented Oct 11, 2018

@vermiculus, are you suggesting that if it did that and returned data which was equivalent to what it would return if it could use libgit, then Magit wouldn't need to worry about maintaining multiple ways of doing things in the future?

I guess that might work, but it seems to me like it's just shifting the burden of the legacy approach from Magit (where it is already implemented and working) into the C module (where it is not implemented at all), so I'm not sure that's the best approach?

@phil-s

This comment has been minimized.

Show comment
Hide comment
@phil-s

phil-s Oct 11, 2018

Contributor

As libgit can't work with a remote git repository, maybe the sensible approach is not to implement libgit support directly as an Emacs module, but instead to implement a separate program which uses libgit, and to which Emacs can communicate (whether locally or remotely) using a persistent process?

The idea being that this program could be installed locally (in which case a local Emacs can start it as a process to do local git things), and it could likewise be installed remotely (in which case I believe a local Emacs could talk to the remote process in order to do remote git things).

It would still be necessary for Magit to maintain non-libgit functionality (unless installing the program became a requirement); but this still seems like an improvement upon "Cannot take advantage of libgit over tramp at all" ?

Edit: Maybe such a program already exists? Regardless, it would clearly have applicability to more than just Emacs, so such a project might get wider support from the git community, compared to an Emacs-specific module?

Contributor

phil-s commented Oct 11, 2018

As libgit can't work with a remote git repository, maybe the sensible approach is not to implement libgit support directly as an Emacs module, but instead to implement a separate program which uses libgit, and to which Emacs can communicate (whether locally or remotely) using a persistent process?

The idea being that this program could be installed locally (in which case a local Emacs can start it as a process to do local git things), and it could likewise be installed remotely (in which case I believe a local Emacs could talk to the remote process in order to do remote git things).

It would still be necessary for Magit to maintain non-libgit functionality (unless installing the program became a requirement); but this still seems like an improvement upon "Cannot take advantage of libgit over tramp at all" ?

Edit: Maybe such a program already exists? Regardless, it would clearly have applicability to more than just Emacs, so such a project might get wider support from the git community, compared to an Emacs-specific module?

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Oct 11, 2018

Contributor

Yes, you understand correctly. I can see your point about shifting work though, but my gut instinct is that it would be more straightforward in C -- in some ways, this low-level parsing is what the language was built to do.

I think I might be on your same thought process -- it seems to me like we've identified a deficiency in libgit. Could that deficiency be appropriately resolved within libgit? In other words, can we teach libgit to operate over a network?

Contributor

vermiculus commented Oct 11, 2018

Yes, you understand correctly. I can see your point about shifting work though, but my gut instinct is that it would be more straightforward in C -- in some ways, this low-level parsing is what the language was built to do.

I think I might be on your same thought process -- it seems to me like we've identified a deficiency in libgit. Could that deficiency be appropriately resolved within libgit? In other words, can we teach libgit to operate over a network?

@brotzeit

This comment has been minimized.

Show comment
Hide comment
@brotzeit

brotzeit Oct 12, 2018

Contributor

Would it be really that much effort to provide libgit and tramp functionality ?

Contributor

brotzeit commented Oct 12, 2018

Would it be really that much effort to provide libgit and tramp functionality ?

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Oct 12, 2018

Contributor

Having concurrent implementations of some functionality is doable if there is a plan to remove the duplication. Otherwise, the two implementations will surely fall out of sync.

Contributor

vermiculus commented Oct 12, 2018

Having concurrent implementations of some functionality is doable if there is a plan to remove the duplication. Otherwise, the two implementations will surely fall out of sync.

@luismbo

This comment has been minimized.

Show comment
Hide comment
@luismbo

luismbo Oct 12, 2018

Contributor

It is undeniable that implementing two backends, git and libgit, will add complexity, particularly maintenance-wise. In terms of initial implementation, it might not be too different effort-wise, though. In fact, there might be some advantages to having two backends:

  1. For read-only operations, the git backend can be used to test the libgit backend and vice-versa. Both backends can be executed and have their outputs compared. Write operations would be trickier, but could be done with appropriately set up clones, if worthwhile.

  2. The git backend can be used whenever libgit2 and/or the libegit2 glue library are unavailable.

  3. And, of course, we can have TRAMP and fast local execution, particularly on Windows. (That is to say, this solution fits the requirements.)

The maintenance burden will be pretty annoying, no doubt. Whenever the backend API needs tweaking, you'll need to implement it twice. Perhaps we could think of the second implementation as a unit test of sorts?

Contributor

luismbo commented Oct 12, 2018

It is undeniable that implementing two backends, git and libgit, will add complexity, particularly maintenance-wise. In terms of initial implementation, it might not be too different effort-wise, though. In fact, there might be some advantages to having two backends:

  1. For read-only operations, the git backend can be used to test the libgit backend and vice-versa. Both backends can be executed and have their outputs compared. Write operations would be trickier, but could be done with appropriately set up clones, if worthwhile.

  2. The git backend can be used whenever libgit2 and/or the libegit2 glue library are unavailable.

  3. And, of course, we can have TRAMP and fast local execution, particularly on Windows. (That is to say, this solution fits the requirements.)

The maintenance burden will be pretty annoying, no doubt. Whenever the backend API needs tweaking, you'll need to implement it twice. Perhaps we could think of the second implementation as a unit test of sorts?

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Oct 12, 2018

Contributor

In my humble, non-maintainer opinion, I don't think having two concurrent and ongoing implementations is the right path for Magit – or indeed for any project. It's early here, so I may not be understanding fully/correctly, but (1) seems like an performance drain rather an improvement. Instead of replacing shell-outs, we would add to them with more in-process work. (2) is reasonable, but I don't think it's a good enough reason to maintain two implementations indefinitely. It will make it much harder for the project to move forward unless a simple, obvious, and robust system can be put in place that makes dual implementations truly easy (and makes it apparent when they've broken). I doubt such a system exists, but I'm willing to be proven wrong.

Contributor

vermiculus commented Oct 12, 2018

In my humble, non-maintainer opinion, I don't think having two concurrent and ongoing implementations is the right path for Magit – or indeed for any project. It's early here, so I may not be understanding fully/correctly, but (1) seems like an performance drain rather an improvement. Instead of replacing shell-outs, we would add to them with more in-process work. (2) is reasonable, but I don't think it's a good enough reason to maintain two implementations indefinitely. It will make it much harder for the project to move forward unless a simple, obvious, and robust system can be put in place that makes dual implementations truly easy (and makes it apparent when they've broken). I doubt such a system exists, but I'm willing to be proven wrong.

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Oct 12, 2018

Contributor

I've raised the possibility/question to libgit2. I do not know when/if the conversation will move to a more stable medium, but it's currently on their slack channel.


three hours later: there's some good conversation there

Contributor

vermiculus commented Oct 12, 2018

I've raised the possibility/question to libgit2. I do not know when/if the conversation will move to a more stable medium, but it's currently on their slack channel.


three hours later: there's some good conversation there

@luismbo

This comment has been minimized.

Show comment
Hide comment
@luismbo

luismbo Oct 12, 2018

Contributor

@vermiculus the main motivation for me (not necessarily for others) for using libgit is that launching n git processes for each magit operation is unbearably slow on Windows. (1) is an interesting point, though. At least some libgit calls should probably take place in a separate thread, and either Emacs threads can context switch at the call-to-C boundary or libegit2 needs to provide asynchronicity somehow.

Contributor

luismbo commented Oct 12, 2018

@vermiculus the main motivation for me (not necessarily for others) for using libgit is that launching n git processes for each magit operation is unbearably slow on Windows. (1) is an interesting point, though. At least some libgit calls should probably take place in a separate thread, and either Emacs threads can context switch at the call-to-C boundary or libegit2 needs to provide asynchronicity somehow.

@vermiculus

This comment has been minimized.

Show comment
Hide comment
@vermiculus

vermiculus Oct 12, 2018

Contributor

That's a main motivator for me as well, though I'm not following your thinking. What does async have to do with the current conversation?

Contributor

vermiculus commented Oct 12, 2018

That's a main motivator for me as well, though I'm not following your thinking. What does async have to do with the current conversation?

@luismbo

This comment has been minimized.

Show comment
Hide comment
@luismbo

luismbo Oct 12, 2018

Contributor

@vermiculus you mentioned that calling libgit2 meant more in-process work. I was just noticing that such in-process work will block the Emacs process and would become a problem for operations that aren't instantaneous. The most obvious offenders would be fetch and push.

Contributor

luismbo commented Oct 12, 2018

@vermiculus you mentioned that calling libgit2 meant more in-process work. I was just noticing that such in-process work will block the Emacs process and would become a problem for operations that aren't instantaneous. The most obvious offenders would be fetch and push.

@jwiegley

This comment has been minimized.

Show comment
Hide comment
@jwiegley

jwiegley Oct 12, 2018

Contributor

How many people rely on using Magit-via-tramp? I'd be happy to drop Tramp support, in order to gain libgit2.

Contributor

jwiegley commented Oct 12, 2018

How many people rely on using Magit-via-tramp? I'd be happy to drop Tramp support, in order to gain libgit2.

@ylluminarious

This comment has been minimized.

Show comment
Hide comment
@ylluminarious

ylluminarious Oct 12, 2018

@jwiegley I felt the same way at first, but a number of people on reddit have made it clear that they rely on Magit+TRAMP. I've been in their position many times, so I cannot in good conscience advocate for the disregard of these users.

ylluminarious commented Oct 12, 2018

@jwiegley I felt the same way at first, but a number of people on reddit have made it clear that they rely on Magit+TRAMP. I've been in their position many times, so I cannot in good conscience advocate for the disregard of these users.

@luismbo

This comment has been minimized.

Show comment
Hide comment
@luismbo

luismbo Oct 12, 2018

Contributor

Someone made a good point on the reddit thread: the libgit backend doesn't need to be complete. It can focus on performance critical operations and fallback to git elsewhere.

Contributor

luismbo commented Oct 12, 2018

Someone made a good point on the reddit thread: the libgit backend doesn't need to be complete. It can focus on performance critical operations and fallback to git elsewhere.

@jb55

This comment has been minimized.

Show comment
Hide comment
@jb55

jb55 Oct 12, 2018

jb55 commented Oct 12, 2018

@phst

This comment has been minimized.

Show comment
Hide comment
@phst

phst Oct 13, 2018

Contributor

I too rely on remote file support frequently. I think that remote file support is one of the basic features of Emacs that packages should aim to provide. Removing it seems like a significant step backwards.

Contributor

phst commented Oct 13, 2018

I too rely on remote file support frequently. I think that remote file support is one of the basic features of Emacs that packages should aim to provide. Removing it seems like a significant step backwards.

@leezu

This comment has been minimized.

Show comment
Hide comment
@leezu

leezu Oct 14, 2018

While editing and commiting on remote machines is nice, editing locally and pushing to the remote machine may be sufficient, given that we are talking about git. When push-to-checkout is set up, this allows conveniently pushing to a remote which will then directly check out the updated branch. It may be a good idea to add support to magit for configuring the remote repository to support push-to-checkout, which e.g. can be done via

# Setup git push_to_checkout on a remote server

ssh -t $1 "cd $2; git config receive.denyCurrentBranch updateInstead"
ssh -t $1 "cd $2; echo 'git read-tree -u -m HEAD \"\$1\"' > .git/hooks/push-to-checkout"
ssh -t $1 "cd $2; chmod +x .git/hooks/push-to-checkout"

leezu commented Oct 14, 2018

While editing and commiting on remote machines is nice, editing locally and pushing to the remote machine may be sufficient, given that we are talking about git. When push-to-checkout is set up, this allows conveniently pushing to a remote which will then directly check out the updated branch. It may be a good idea to add support to magit for configuring the remote repository to support push-to-checkout, which e.g. can be done via

# Setup git push_to_checkout on a remote server

ssh -t $1 "cd $2; git config receive.denyCurrentBranch updateInstead"
ssh -t $1 "cd $2; echo 'git read-tree -u -m HEAD \"\$1\"' > .git/hooks/push-to-checkout"
ssh -t $1 "cd $2; chmod +x .git/hooks/push-to-checkout"
@phil-s

This comment has been minimized.

Show comment
Hide comment
@phil-s

phil-s Oct 14, 2018

Contributor

The thing with tramp in general is that it's unlikely to be the most performant, and there's usually a 'better' way if you take the time to set it up, but it is incredibly convenient when you haven't set up something better (to the point where many people do indeed use it all the time, even if others would not consider working that way).

Tramp is awesome for this reason, and the fact that Magit works over tramp is awesome too. I think that eliminating that facility from Magit would be a tremendous shame (especially given how much effort has gone into making it work in the first place).

Contributor

phil-s commented Oct 14, 2018

The thing with tramp in general is that it's unlikely to be the most performant, and there's usually a 'better' way if you take the time to set it up, but it is incredibly convenient when you haven't set up something better (to the point where many people do indeed use it all the time, even if others would not consider working that way).

Tramp is awesome for this reason, and the fact that Magit works over tramp is awesome too. I think that eliminating that facility from Magit would be a tremendous shame (especially given how much effort has gone into making it work in the first place).

@tarsius

This comment has been minimized.

Show comment
Hide comment
@tarsius

tarsius Oct 14, 2018

Member

I do not intend to remove tramp support.

Member

tarsius commented Oct 14, 2018

I do not intend to remove tramp support.

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