pre-commit check hook

Bug #102747 reported by Martin Pool
4
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
Wishlist
Nam Nguyen

Bug Description

It would be nice if a hook could get the chance to examine the to-be-committed tree and eg run 'make check'. This would catch things like simply broken revisions, inconsistent selective commits, files that are needed but not added, and things that don't work in a clean rebuild.

* Pre-commit check. If this hook is defined, it needs to be handled
  specially: create a temporary directory containing the tree as it
  will be after the commit. This means excluding any ignored/unknown
  files, and respecting selective commits. Run the pre-commit check
  (e.g. compile and run test suite) in there.

  Possibly this should be done by splitting the commit function into
  several parts (under a single interface). It is already rather
  large. Decomposition:

   - find tree modifications and prepare in-memory inventory

   - export that inventory to a temporary directory

   - run the test in that temporary directory

   - if that succeeded, continue to actually finish the commit

  What should be done with the text of modified files while this is
  underway? I don't think we want to count on holding them in memory
  and we can't trust the working files to stay in one place so I
  suppose we need to move them into the text store, or otherwise into
  a temporary directory.

  If the commit does not actually complete, we would rather the
  content was not left behind in the stores.

Martin Pool (mbp)
Changed in bzr:
importance: Undecided → Wishlist
status: Unconfirmed → Confirmed
John A Meinel (jameinel)
Changed in bzr:
assignee: nobody → bitsink+launchpad
status: Confirmed → In Progress
Revision history for this message
John A Meinel (jameinel) wrote :

I would say that step 1 is to have a "pre-commit" hook as part of the Branch.hooks. Which will be called after we have figured out everything that we need, but before the Branch.last_revision() has been updated.
This ensures that you have a stable view of the tree.

What would also be nice is if the pre-commit hook takes the list of changes that commit worked out. This way, if you want to do something like "do all modified files have only spaces and no tabs", you can do that without having to look at the full tree. (Also works for checking line endings, etc)

At that point, the plugin/hook itself can decide whether it wants to build a complete tree in a temporary area, and run something like 'make check' there.
I would make sure we have enough helpers to make that easy for a plugin (something like bzrlib.transform.build_tree). But I wouldn't expect the pre-commit hook to do it by itself.

Though, I could also see if you had multiple pre-commit hooks that wanted to work on a pristine tree, then you would want a way they could share it, so you don't have problems.

Note, that if you put the hook at this place, you cannot modify the tree in the pre-commit hook. All you can do is ensure that the tree is 'correct'.

An alternative "pre-pre-commit" hook would be invoked (perhaps before any data gathering?). But that can also be easily done in a plugin by installing its own 'cmd_commit'. So I'm not sure we need a pre-pre-commit hook.

Revision history for this message
Nam Nguyen (bitsink+launchpad) wrote :

I also concur with you that pre-commit should preferably only reports a list of modified or added files or directories to the plugin. If needed, the plugin could request a whole tree in a temporary area.

There are a few issues though.

o If a branch is large enough, each time a whole tree is requested, there will be space wastage.
o If a list of plugins is specified and each of them requests a tree for itself, do we share a same tree, or create separate trees?

It might be a solution if we create a new branch in a temp directory, apply the merge, then give it back to the plugin. When the plugin finishes using the tree, it must release it (a refcount will tell if the branch has been released). At this time, the tree is reverted, the commiting merge is re-applied, and the tree is given to the next plugin in line.

This solution can also save us some space and time because we only need one temporary space, and a pull will keep it up-to-date with the current tree.

As to the question whether a plugin can modify the tree in pre-commit hook, I think it is free to do whatever it requires but the tree will be discarded (reverted) when it is released back from the plugin. Now, this is another problem because if the plugin intentionally makes conflicting changes, and commits to that, we won't be able to do a pull on that tree to keep it updated anymore (or do we?). What if the plugin deletes the whole temporary tree? In these cases (a pull fails, or the pre-commit tree is gone), we have to create it again.

Revision history for this message
Nam Nguyen (bitsink+launchpad) wrote :

commited from revision 2659

Changed in bzr:
status: In Progress → Fix Committed
Vincent Ladeuil (vila)
Changed in bzr:
milestone: none → 0.92
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.