On 2014-08-16 22:55-0400 Hazen Babcock wrote:

> On 8/16/2014 1:50 PM, Alan W. Irwin wrote:
>> On 2014-08-16 06:50-0400 Hezekiah M. Carty wrote:
>> 
>>> I agree with Hazen here - adding hooks on day one seems overkill for
>>> the project.  [...]
>>> Given that the project and developers are overall new to git I think
>>> it would be premature to start stacking automated restrictions on top
>>> of our use of the tool.
>> 
>> That said, I appreciate you and Hazen are anxious to get started with
>> git for PLplot after such a long wait, and I also feel similarly even
>> though I am pretty much still a git newbie.  So I suggest we should
>> jointly review my decision (to not allow push to the official repo
>> until we get the required modification of the update hook figured out)
>> sometime late next week (say roughly in 6 days from now).  That gives
>> enough time to find out if Brad King is willing to share his
>> implementation, and also to pursue a lot further the backup plan of
>> modifying .git/hooks/update(.sample) appropriately ourselves.
>
> Unfortunately all I can report so far is that I have not had much luck with 
> the obvious google searches such as "git hook enforce merge only". So while 
> presumably this enforcement is possible, it also seems to be rare.

Just to give some background for those who are not aware of it yet,
hooks are executable scripts with specific names (from a list
recognized by git) in .git/hooks which can occur on the server side
(the repo that is cloned from, e.g., the official SF repo) or the
client side (the repo that is created by git clone from the server
side repo and which has generic sample files in .git/hooks/*.sample
rather than anything specific from the server side. git interacts with
these named hooked files at certain stages of the git process and with
certain arguments.  But the documentation (git help hooks) shows the
hooks system has grown like topsy with some rather different uses of
the same hook.  Also, I believe that documentation is seriously out of
date (i.e., missing some additional official hook names such as
"pre-merge" which google searches tell me has gone into git, but only
documented within the git code).


>From the CMake wiki they enforce their desired workflow result via the
update hook script (i.e., .git/hooks/update) on the server side (in
this case the official CMake repo hosted by Kitware). An advantage of
this technique is such enforcement cannot be gotten around by changing
.git/hooks/update on the client side.  The only way you can change it
is via ssh access to the server-side repo (in our case that normally
would be PLplot's official SF repo).  A drawback is the enforcement
only occurs on pushes substantially after the fact when the workflow
error actually occurred which makes the logic of the update hook
somewhat complicated because it needs to figure out whether the push
would change the server-side history into non-clean "shape of
history".  (I am still struggling with this logic by mentally
rearranging tinker toys corresponding to various DAGs consisting of
git commits that are infected with prior git merges to the wrong
branch).  Another drawback is that users who are not that familiar
with the workflow constraints may have trouble figuring out which
merge to the wrong branch (perhaps weeks previous) is causing the 
"non-clean shape of history" push rejection.

But there is a potential client-side complement to the push constraint
that probably avoids that last issue.  What is needed is a client side
hook for merge commits that immediately rejects any wrong moves such
as merging from master to any branch not named "next" (which
immediately stops all merging of master to topic branches), and from
next to any branch at all.  An advantage of such client-side
enforcement rules for the workflow is the logic a lot more
straightforward then in the update hook case since it consists simply
of identify the "from" and "to" branches involved in merge commits and
applying the rules considering their names.  However, a disadvantage
is users must apply these rules themselves.  In other words, if they
don't bother with installing these client-side hooks, then their bad
workflow is only caught at push time.  Nevertheless, since the logic
should be easier I am starting with the voluntary client hooks to see
exactly what is involved.

One current issue for me (see the explanation of the git hooks mess
above) is finding the right hook to use which will fire at merge
commit time for the client-side and which will have the needed
information available (especially the id of the merge commit so its
parents can be found and their associated branch names). Another
uncertainty is that the commit part of merge commits (which you are
guaranteed to get if all your merges are --no-ff) apparently does not
fire some/all of the commit hooks.  So my goal for today is to dig
through the git code to see all hooks that are actually available by
name (e.g., the undocumented pre-merge), do tests to make sure they
fire for merge commits, and do tests to make sure there is a way to
access the id of the merge commit.

Assuming I can get this client side idea to work, then we could start
with that and we could be in business, and then I could figure out the
complementary (but ultimately necessary I think) server-side update
hook that CMake relies on to enforce the workflow as time permitted.

> As an aside, an interesting client-side hook to develop would be one that 
> automatically runs uncrustify. I know that I'm not very good about doing this 
> manually.

I strongly support this excellent idea.  Of course, only a subset of
our developers would want to install this hook on their local disk
repo.  For example they need access to the right version of uncrustify
(the output from uncrustify depends strong on uncrustify version so
this version has to be compatible with the uncrustify version we use
in the current style script).  That version needs to be built on Unix
OS's and I understand the build does not work on Windows.
Furthermore, python is a requirement since a home-brew python script
is used to enforce C++ style // commentary on our C code since
uncrustify (at least the version we are using) does not have this
capability.  But once set up, this would be a very nice way to keep
our source code in good style.

Alan
__________________________
Alan W. Irwin

Astronomical research affiliation with Department of Physics and Astronomy,
University of Victoria (astrowww.phys.uvic.ca).

Programming affiliations with the FreeEOS equation-of-state
implementation for stellar interiors (freeeos.sf.net); the Time
Ephemerides project (timeephem.sf.net); PLplot scientific plotting
software package (plplot.sf.net); the libLASi project
(unifont.org/lasi); the Loads of Linux Links project (loll.sf.net);
and the Linux Brochure Project (lbproject.sf.net).
__________________________

Linux-powered Science
__________________________

------------------------------------------------------------------------------
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel

Reply via email to