search

libgit2 / libgit2

A cross-platform, linkable library implementation of Git that you can use in your application.
https://libgit2.org/
Other
9.7k stars 2.41k forks source link

filters (smudge / clean) mis-called when using "git_index_add_bypath" #2479

Closed ccleaud closed 10 years ago

ccleaud commented 10 years ago

Hello,

I use custom filter driver for encryption purpose (git-crypt) and I see a behavior difference between 2 TortoiseGit options in "Advanced settings":

According to TortoiseGit code, the "mis-working" function is "git_index_add_bypath". @see: https://code.google.com/p/tortoisegit/source/diff?spec=svn1b5d73d62cd4bc2de35301589415688a1c698bf7&r=1b5d73d62cd4bc2de35301589415688a1c698bf7&format=side&path=/src/TortoiseProc/GitProgressList.cpp at line 1757.

I firstly created an issue to TortoiseGit. The problem seems NOT to be from TortoiseGit. So I create this ticket....

You can check my initial TortoiseGit ticket for more details about how you could reproduce the problem. @see: https://code.google.com/p/tortoisegit/issues/detail?can=2&start=0&num=100&q=&colspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary&groupby=&sort=&id=2224

Thanks for fixing!

C. CLEAUD

arrbee commented 10 years ago

I've read over your TortoiseGit issue, but I'm still a little uncertain if I my understanding is correct.

Is the problem that libgit2 is just not invoking the git-crypt.exe external application? If so, that is by design; libgit2 will not shell out to an external command for you. If TortoiseGit wants to support running external filters, it is possible to write glue code that uses the filter hooks in libgit2 to invoke an external program, but libgit2 won't do this out of the box.

If anyone from TortoiseGit is following and is interested in working on this, I can probably help with some example code to bridge from libgit2's filter support to invoke the shell command, at least with an example that would work on Linux.

Another issue you will have, however, is that libgit2 does not currently support textconv for diffs, which would be a problem for the example you gave.

linquize commented 10 years ago

AFAIK, jgit (as a library) doesn't call external executable either.

ccleaud commented 10 years ago

Yes, the problem is libgit2 not invoking external application. As far as I know, TortoiseGit developpers though this action was offered by libgit2. I'll show them your answer and let them handle the issue (maybe they'll get in touch we you). Thanks for your answer.

YueLinHo commented 10 years ago

If anyone from TortoiseGit is following and is interested in working on this, I can probably help with some example code to bridge from libgit2's filter support to invoke the shell command, at least with an example that would work on Linux.

@arrbee I am interested in working on this. I want to try it. So, please help me. Thank you. ^_^

arrbee commented 10 years ago

@YueLinHo That is great! You'll want to read three places:

Starting with tests/filter/custom.c may be easiest, since that has a concrete example. You allocate a git_filter object and set the check, apply, and shutdown callbacks. You will also set the attributes field to the string "filter" indicating that you want your custom filter to be called on any file that has the "filter" attribute. Then you pass that filter object to git_filter_register to add it to the libgit2 global registry.

When a file needs to be filtered, if it has the filter attribute set then your check function will be invoked and passed the value of the filter attribute in the attr_values input. You can then look up that value in the git config and if it is present you return 0 (and optionally cache anything you might need, like the command-line from the config file, in the payload which is an in-out paramter).

If check returned 0, then the apply callback will be invoked with the unfiltered data in the from buffer. It needs to write the data to the to buffer. You can write the from to temp file and then construct the command line and shell out to the user's application, or whatever you need to do to actually apply the filter. You will know whether to run the clean or smudge command by using the git_filter_source and using the git_filter_source_mode accessor function.

Anyhow, read the docs in the headers and look at the examples in the test code and let me know if you encounter any problems! I wrote the above stuff mostly off the top of my head, so I may have made some errors. I'm sorry if I did!

To be totally honest, I don't think anyone has actually fully implemented this before, so I'm not 100% certain that you won't encounter a design flaw, but we tried to account for this scenario.

Another issue you will have, unfortunately, is that there is no support for diff textconv filtering, so if you do this it will likely mess up diffs. But we would love feedback on that part of the library and more experience with the filter API so we can make sure to do textconv right.

YueLinHo commented 10 years ago

@arrbee Thank you so much. You are nice. I am not familiar with libgit2, but I will try. Good luck to me. ^_^

arrbee commented 10 years ago

@YueLinHo one other thing we can do is look for other libgit2 users who might be interested in this functionality to help you out. If you are not familiar with libgit2, this isn't a very easy first project.

We can see if any other users who like this feature want to chime in. If not, I'm afraid I don't have time at the moment to help you with coding it, but feel free to ask questions or run ideas by us and someone will get back to you.

YueLinHo commented 10 years ago

@arrbee It's OK if you don't have time. You'd give me so many hints. I think that if I finish it, I will gain rich experience. right? That's good. ^_^ So, all I need to do is step out. Thanks again.

I am not a smart one, and anyone who is interested in working on this, feel free to take it. Whoever finish it, I will keep following it. And I will learn something from it. That is what I want. O_O

csware commented 10 years ago

TGit now has a working "filter" filter (https://code.google.com/p/tortoisegit/issues/detail?id=2224#c17).

Btw. Does libgit2 support diff filters/textconv (if yes: how to implement)?

I think this issue can be closed then.

arrbee commented 10 years ago

@csware That is awesome news! Is the code available somewhere? I'd love to see what it took to get it all working and, if possible, see if we could build an example of how to do it in libgit2 based on what's there. (I realize that TGit is GPLv2 so we'd have to get agreement to see if that is possible.)

Unfortunately, there is no support for diff filters/textconv in libgit2 right now. I've talked it through with a couple of folks, but nothing has been implemented so far. It would be good to tackle that problem since filters are much less useful without it. Sorry!

csware commented 10 years ago

@arrbee: It still has some glitches when used in combination with sh.exe. But I found that this seems to be a common problem.

Right now TortoiseGit is a "nice" bundle of libgit2 and git.exe calls. For diffing we only use git.exe, so it's not a big issue so far. However, we started using git_indexadd* for adding files to index and here some of our users reported issues with missing filters.

arrbee commented 10 years ago

Thanks @csware - that's good to understand. I'm not a great user of Google Code, but I tried to search around for anything where there was a libgit2 filter plugin and couldn't find it. Is it on a branch or just not pushed yet?

csware commented 10 years ago

@arrbee We still have some specific msys issues to solve (see https://code.google.com/p/tortoisegit/issues/detail?id=2224#c17).

I'll provide a link as soon as the code is cleaned up, rebased and merged ;) - A basic working version can be found here: https://github.com/csware/TortoiseGit/commit/63b2d90032d21070afc74af2310b3413faaf61ee

csware commented 10 years ago

TortoiseGit merge commit (sh.exe is also working now): https://github.com/TortoiseGit/TortoiseGit/commit/8aff4e99e9eb937b822c3a4d99feaf7d3c206ea3

csware commented 10 years ago

I think this can be closed.