Hello again,
Comments below :)
My
personal suggestions/opinions:
1)
I totally agree with the listeners suggestion
2)
Hooks have priority; if they reject a commit,
everything runs the same as now – afterwards listeners are
called. This way we don’t break any compat with c git
3)
Listeners (EGit) gets called thereafter, and
might or might not have something to say (depending on
validator registered with EGit).
Just to make sure we're on the same track:
any listener called after a hook that's allowed to interrupt the
current command will also be able to interrupt the command, et
inversely any listener called after a hook that's not allowed to
interrupt the current command will not be able to interrupt the
command (even if they violently throw exceptions or this kind of
things). Is that right?
4)
I don’t see any files involved in this –
everything is available in memory, and we’re on a track
that’s pretty far away from what plain GIT does.
Yes, everything is available in memory, we'll
just have to be careful about what these listeners could do to
the currently executing command if they have access to it,
which seems hard to avoid:
It seems likely that some listeners will be allowed to change
part of the state of the current command (such as modifying
the commit message for instance), but they must not be allowed
certain side effects (such as interrupted the current command
at a point where it's not supposed to be interruptible).
It may also be worth to wonder who is going to implement such
listeners: it seems that understanding EGit and JGit APIs will
be a requirement.
More opinions? :)
BTW, I /really/ like the productive discussion
now! :) Keep going!
Cheers,
Markus
I think JGit
should allow "listeners" to be added for each "hook event"
(pre-commit, commit-msg, post-commit, pre-rebase, etc.)
EGit could then provide one or several extension point(s) to
dynamically register such listeners, and register the
listeners from the extensions in the JGit commands it runs.
As to the API of such a validator, that is the question!
The way git handles that is by transferring every data
through files. For example, commit-msg can use and modify
the content of the commit message by reading/writing to a
specific temporary file used for that purpose. This way of
working removes the necessity of defining an API...
Other hooks receive info via stdin, and I guess there may be
more mechanisms. So more analysis is required here I'm
afraid.
Another question while I'm at it: How would such listener
interact with hooks? Suppose a hook and 2 listeners are
"registered", which should run first?
Hope this helps anyway.
Laurent
Delaigue (@laurentdelaigue)
Tel. : (+33) 2 51 13 54 18
Obeo - Model-Driven Company
In this case, maybe multiple points should
be hookable – before and after the commit has been done.
My only concern here is the “how/where to register
hooks”, and the “what should hooks return, and how to
present it”.
We need JGit’s help (definitely) to be able
to get called during the commit process, at the same
locations as other hooks. But do we register each
validator directly with JGit (how?), or only register
EGit hooks with JGit somehow (API?), and EGit
distributes to individual validators registered through
... an extension point? ... or some API?
What should these validators return, and
what is their input? I opt for IStatus as return (can be
nested, provides possibility to transfer all required
information). For the input, at least the repository,
but in some hooks there is more information
available/required (commit-msg: the message,
post-commit: the commit object), so it is a little hard
to find a single generic interface for that... do you
agree?
Cheers,
Markus
Hi,
I agree with Andrey that EGit should not create markers.
I think it could be interesting to allow JGit to notify
listeners at particular moments in time (specifically,
in the places where hooks are called) to give the
opportunity to clients to register listeners to react.
I've been working on hooks lately and I think this
should be quite easy.
This would allow java listeners to be registered and
react at pre-commit, commit-msg, or any other step, with
the same side-effects as hooks have (especially in terms
of being able to cancel an operation).
I don't agree that validations should be pre-commit, git
allows some many different workflows that we should
support this flebility: if users want to be warned after
a commit is made, I think it's fine.
Best regards,
1)
IMHO Egit should not create markers since it
does not listen to the resource modificatons (is
the problem fixed?) and also does not contribute
builders to the projects.
The
problem with the markers is that user expects
that they disappear after the problem is
resolved or after "clean project", but if there
is no egit listener/builder on the project, they
will remain. We could add such a builder, but
this is going out of the original proposal's
scope (the builder will need to run pre-commit
validators over each changed file, so that user
could see the warnings even *before* commiting
the stuff). This is interesting idea and
deserves an extra discussion. Just imagine we
would see all the "trailing white space" errors
before they will appear in the gerrit.
What
we can initially do is just to run validators
each time an entry appears in the staging view
and also decorate entries in the staging view
with extra information (all icons/font/text
labels are possible), and even eventually
disabling the commit button while showing
"commit validation rule error on file xyz" error
description in the staging view.
2)
Matthias, me and Christian - we all understand
this validation as "pre-commit" thing, which
*prevents* commit to happen if something is not
OK. Markus proposal works *after the fact*, and
this is an important difference. From the user
perspective it does really not make sense to
fiddle with "bad" commit, it is not user
friendly. I guess no one of my coworkers ever
ameded a commit or tried to modify the history
in any way. So if implementing validators, they
must be *pre*-commit/pre-push etc.
Hey
all,
It's gotten quite silent about
this. Could somebody give any
feedback on the change? I'm OK
with "we won't accept it this way"
also ;) (maybe as long as there is
a viable alternative to do
something similar in quality for
the end-user).
Just as a demo on how I imagine
validators based on the change
(real world examples):
https://drive.google.com/folderview?id=0B05h8C3gLt38MVlvOFFaUlh4Yk0&usp=sharing
I
appreciate that you are looking
into more powerful validation.
I
think it makes sense to provide
more powerful validations which
are better integrated into
the
Eclipse workbench than what can be
done in scripted hooks.
I
think we should go one step
farther than what you propose.
I
wonder if validators should create
problem markers [1] and the git
commands like
the
commit command should check if
problems of certain types/severity
exist in order
to
decide if it's ok to create a
commit or if just to show there
are problems.
Which
problem types/severities should be
reported and/or block git commands
should be configurable.
This
way we would gain an integration
with other validators like build,
compiler warnings etc
without
imposing an EGit specific API for
integrating them.
So
you could e.g. configure that a
warning should be shown in the
commit dialog/staging view
if
there are compiler warnings of a
certain kind. Or commit could be
blocked if the code doesn't
compile.
We could add new validators for
git specific validations like a
commit message validator
checking
if formatting of the commit
message is ok, contains a link to
a bug etc.
Validations
could be also integrated with
other git commands, e.g. if you
want to prevent
that
large blobs are entering the git
repository the best place to block
that would be the AddCommand
since
as soon as you staged the big file
it's already in the object store
of your git repository.
We
could use the problem view to
display the problems and even
implement quick fixes for
git
specific new problem types.
_______________________________________________
egit-dev mailing list
egit-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/egit-dev
Salomon
Automation GmbH | Friesachstrasse 15 | 8114 Friesach bei
Graz | Austria
Registered Office: Friesach bei Graz | Commercial
Register: 49324 K | VAT no. ATU28654300
Commercial Court: Landesgericht für Zivilrechtssachen
Graz
Salomon Automation GmbH |
Friesachstrasse 15 | 8114 Friesach bei Graz | Austria
Registered Office: Friesach bei Graz | Commercial Register:
49324 K | VAT no. ATU28654300
Commercial Court: Landesgericht für Zivilrechtssachen Graz
|
