Hi Team,
I've just pushed cleaned up versions of the previous reviews,
introducing the API (https://git.eclipse.org/r/35476) then the
support for the pre-commit (https://git.eclipse.org/r/35477),
commit-msg (https://git.eclipse.org/r/35790), post-commit
(https://git.eclipse.org/r/35791) and post-rewrite
(https://git.eclipse.org/r/35792) hooks into JGit.
I have a few question though... First, the four reviews for the
hooks themselves are dependent on one-another, in a line... would
you rather I make these commits independent, having one "base"
review (the API) then the four commits depending on it each
introducing its own hook? (This will make the independant "hook"
commits conflicting with each other though, so I'll have to rebase
each one manually after one is accepted and submitted in the code
base).
Second... the "commit-msg" hook support makes three unit test case
because I trim the commit message when re-reading it after the hook
has done its job... and the failing unit cases use a commit message
with a trailing LF. Should I remove the trailing white space from
the unit test, or would you rather we accept these white spaces?
(cgit trims the commit message.)
The "post-rewrite" hook has test failures I'm currently fixing.
Laurent Goubet
Obeo
On 24/10/2014 16:53, Laurent Goubet
wrote:
Hi Andrey, team,
Just to let you know, I've just pushed two (pretty rough) drafts
introducing the support for git hooks in JGit. "pre-commit",
"commit-msg" and "post-commit" are supported as tested on both
unix and windows 8 Cygwin, running on Java 7 (but should work on
Java 5 and higher).
These patches are not meant to be reviewed as-is since they need a
lot of cleanup, but if you are willing to test on your platforms
if these three hooks work in your use cases, that would be a great
help to ensure I'm not overlooking something.
As a side note, I have some trouble wrapping my mind over how (or
rather, when) the "prepare-commit-msg" should be called. The
staging view for example is pretty much a "live" view of the
workspace, index and possible commits... Trying to call this hook
whenever there's something added to the index to update the
staging view seems... overkill. Likewise, the "commit" dialog
(Team > commit) will display a message... but the user might
change what's in the index from that pop-up too, potentially
changing what the "prepare-commit-msg" hook would have returned
(as opposed to the command line commit editor which only supports
message edition and not index modifications). I think I'll prepare
a first version of the git hook support that does not trigger
every possible existing hook, as I guess trying to find the best
way to support some of them will be far from trivial.
Laurent Goubet
Obeo
On 20/10/2014 15:13, Laurent Goubet
wrote:
Hi Andrey,
All right, I'll start a first implementation following these
notes. It would be nice if you are able to test the reviews once
they are in though :) (I'm on windows 8 / cygwin, so at least
this configuration will be tested).
Laurent Goubet
Obeo
On 20/10/2014 13:24, Andrey
Loskutov wrote:
Laurent,
regarding collaboration: I will be busy next two weeks,
so if you are in harry, just go ahead.
Hi
Andrey, Team,
We are also interested at Obeo about this support
for the git hooks, and we'd also be interested to
contribute this into E|JGit .
We also think that the "simplest possible"
solution outlined here would be sufficient for
most cases... since implementing something more
complex would involve unreliable solutions : the
hooks must be defined "per repository"; so they
must be somehow read directly from the repository.
If we implemented them in Java so that EGit and
JGit can have their own hooks API, we'll need to
involve class loaders and that too often proves
too bug-prone for too litle gain. Reading and
executing the hooks as cgit does seems like the
most reliable solution, even if not the most
portable.
As I gather, there are currently 12 client-side
hooks available to command line users :
- pre-commit
- prepare-commit-msg
- commit-msg
- post-commit
- post-rewrite
- applypatch-msg
- pre-applypatch
- post-applypatch
- pre-rebase
- post-checkout
- post-merge
- pre-auto-gc
They're named with these hard-coded strings as
their file name and are set as executable, and
located in the <git repo>/.git/hooks
folder. Only one of each is allowed (though that
"one" can be specifically designed to chain
others). They can be written in just about any
language available on the user machine.
These work well on linux but will fail on
windows, unless executed through msysgit or a
unix-emulator such as cygwin... I think that
having the same limitation in E|JGit is not
really an issue. The implementation should
probably use FS#runInShell(...) instead of
Runtime.exec(...) to make use of cygwin when
detected though.
Andrey didn't receive any answer to his
proposal to contribute last month. Should we
implement that proposal and push a patch to
gerrit? Andrey, would you be interested in
splitting the effort somehow?
Laurent Goubet
Obeo
On 16/10/2014 16:10,
Andrey Loskutov wrote:
Hi,
We at Advantest see the need of adding pre-commit/pre-receive etc hooks to [e|j]git so that egit UI behavior "matches" CLI git behavior as defined in [1].
I think I found related bug [2] and asked there what the smallest possible implementation should do, but so far no one responded. Therefore I would like to ask same question here again.
Is support for pre-/post- etc hooks on anyone plan for [e|j]git 3.6?
If not, does it make sense to add it to?
I'm offering to contribute simplest possible "Runtime.exec()" based solution which would "just work", but I would like to know what is the common agreement on the requested functionality.
IMHO the smallest possible implementation would need:
* somehow read all the available hooks
* run the appropriate git hook before/after appropriate jgit command using "Runtime.exec()" or platform-dependent "execution" API hook (similat to FS hooks)
* allow to have egit UI callbacks for prompting/displaying hook output etc
Anything I'm missing here (for the *basic* feature set)?
[1] https://www.kernel.org/pub/software/scm/git/docs/githooks.html
[2] https://bugs.eclipse.org/bugs/show_bug.cgi?id=299315
Kind regards,
Andrey Loskutov
http://google.com/+AndreyLoskutov
_______________________________________________
jgit-dev mailing list
jgit-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jgit-dev
_______________________________________________
jgit-dev mailing list
jgit-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jgit-dev
_______________________________________________
jgit-dev mailing list
jgit-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jgit-dev
|