Code review comment for lp:~parthm/bzr/173274-export-hooks

144 +# FIXME: As export is a directory based module (as against a py file), our
145 +# lazy registering doesn't see 'hooks' in the first tuple. To work around
146 +# this directly list __init__ below. This can possibly be cleaner.
147 +known_hooks.register_lazy(('bzrlib.export.__init__', 'hooks'),
148 + 'bzrlib.export', 'ExportHooks')

I don't understand why this is necessary... oh, I see. There's a bug in _LazyObjectGetter. I've submitted <https://code.edge.launchpad.net/~spiv/bzr/hooks-refactoring/+merge/36101> to fix this, and also make known_hooks registration a little neater. Until that lands this is an ok workaround.

36 - return _exporters[format](tree, dest, root, subdir, filtered=filtered,
37 - per_file_timestamps=per_file_timestamps)
38 + for hook in hooks['pre_export']:
39 + hook(ExportHookParams(tree, dest, format, root, subdir,
40 + filtered, per_file_timestamps))
41 + _exporters[format](tree, dest, root, subdir, filtered=filtered,
42 + per_file_timestamps=per_file_timestamps)
43 + for hook in hooks['post_export']:
44 + hook(ExportHookParams(tree, dest, format, root, subdir,
45 + filtered, per_file_timestamps))

Is this actually sufficient to provide the functionality requested in bug 173274? Specifically, it requests a hook that allows modifying the tree to export, e.g. by running autoconf, and having the new, unversioned (and probably ignored) files that generates be included, and possibly include modifications to versioned files too. I don't think this implementation allows that, as I believe that regardless of the pre_export hook it will still export "the last committed revision", as "bzr help export" says.

Well, the pre-hook could commit a new revision with the changes, and the post-hook could uncommit it... that would be a really ugly hack, and a plugin could already wrap cmd_export to do that sort of hack.

So, two changes I'd like to see:

 * support for somehow allowing selected unversioned files in the tree to be included in the export. Perhaps by changing the Tree object that is being exported?
 * tests that demonstrate that support, so that we can be confident the use case in bug 173274 is satisfied :)