Merge lp://staging/~parthm/bzr/173274-export-hooks into lp://staging/bzr

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 :)

> * 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 :)

Taking bug #619478 into account, maybe a --all or --unversioned flag for 'bzr export'? I can create a patch for that before we try to land this in case one of the above seems like a good solution. Thoughts?

Thanks for the review.

On 22 September 2010 01:21, Parth Malwankar <email address hidden> wrote:
>
>>  * 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 :)
>
> Taking bug #619478 into account, maybe a --all or --unversioned flag for 'bzr export'? I can create a patch for that before we try to land this in case one of the above seems like a good solution. Thoughts?

That sounds good. I'd used --uncommitted, to be consistent with merge.

--
Martin

>>>>> Martin Pool <email address hidden> writes:

<snip/>

    >> Taking bug #619478 into account, maybe a --all or --unversioned
    >> flag for 'bzr export'? I can create a patch for that before we
    >> try to land this in case one of the above seems like a good
    >> solution. Thoughts?

    > That sounds good. I'd used --uncommitted, to be consistent with merge.

+1

Handling 'junk' files may become possible in the near future and using
'--uncommitted' to encompass all the kinds of unversioned file seems to
be the best bet.

   Vincent

Just mentioning that "--uncommitted" sounds to me like "give me the
content of the *versioned* files that hasn't been committed yet". Which
is how merge uses it. Specifically, you don't get junk files when you
'merge --uncomitted'.

John A Meinel wrote:
> Just mentioning that "--uncommitted" sounds to me like "give me the
> content of the *versioned* files that hasn't been committed yet". Which
> is how merge uses it. Specifically, you don't get junk files when you
> 'merge --uncomitted'.

Yes, I think uncommitted-but-versioned isn't what the use case is asking for.
It is asking for a way to get some (all?) unversioned files included (and
probably uncommitted changes too).

I'm not sure exactly what would best suit the use case (bzr export of a tree
including 'autoconf' output). Typically you wouldn't want to export all ignored
files (e.g. usually you don't want to ship: tags, *.pyc, *.o, ...). Perhaps the
hypothetical 'precious' designation is good enough, but I'm not sure it's an
exact fit. So I suspect a complete, polished solution may involve some extra
complexity (perhaps just some config in bzrrules to define some files as
unversioned_exportable)...

But that's policy. In terms of mechanism, we need a hook or other API that
gives a plugin a way to:

  * cause selected unversioned files to be included in the export
  * cause uncommitted versions of versioned files to be included in the export

Then we should be able to support whatever exact behaviour is needed.

ping, this branch is now a pre-requisite of https://code.edge.launchpad.net/~gz/bzr/transport_post_connect_hook/+merge/38431

What's the status here ?

Ah, this one isn't a pre-requisite for me, I just used Parth as a reference and inspiration. :)