Daniel Patterson <firstname.lastname@example.org> writes:
> email@example.com wrote:
> > There's a bug, though: when you do an export from a working copy
> > (instead of from a URL), the new -N flag is just ignored. It should
> > have the same effect when the source is a working copy as when the
> > source is a URL.
> Hmm, my bad, I've never used that feature, I didn't even know it
> Updated log message below, updated patch attached. Still no
> test cases, my Python literacy is not great.
Nice work! This passes my hand tests, and passes 'make check', and
I'd like to commit it. But there's one problem: it is inconsistent in
how it treats svn:externals.
Let's take the cvs2svn repository as an example, since it uses
svn:externals. If you do
$ svn co http://svn.collab.net/repos/cvs2svn/trunk/ cvs2svn-wc
$ svn export -N http://svn.collab.net/repos/cvs2svn/trunk/ URL-exported
$ svn export -N cvs2svn-wc WC-exported
then afterwards, 'URL-exported' will contain the 'svntest' subdir
(which comes from the svn:externals property), yet 'WC-exported' will
IMHO, the -N flag should prevent any externals from being exported.
In other words, your patch's from-wc behavior should happen in the
from-url case as well.
Do you agree with this proposal, and if so, would you be willing to
update the patch accordingly? In the meantime, I have filed
...as this is getting close enough to being committed that it's nice
to have a place to track the latest version of the patch at all
times. Feel free to comment on the issue, add attachments, etc.
Regarding the patch itself, a few minor comments:
> Add --non-recursive parameter to "svn export"
> * subversion/libsvn_client/export.c:
> (svn_client_export3): add new "recursive" parameter that
> gets passed through to svn_ra_do_update. Also, if exporting
> from a working copy, pass the recursive flag through to
> copy_versioned_files (which has been updated), see below.
> (svn_client_export2): default to passing TRUE for new
> "recurse" parameter
> (copy_versioned_files): add a recursive parameter and
> only do a recursive copy if it's true.
> * subversion/include/svn_client.h:
> (svn_client_export3): update public API to include new
> "recursive" parameter"
> * subversion/libsvn_client/externals.c:
> (handle_external_item_change): update call to
> svn_client_export3 to pass TRUE as the recursive value.
> When traversing externals, if we're doing a nonrecursive
> export, we should never get to this point, so TRUE should
> always be the case.
> * subversion/clients/cmdlind/export-cmd.c:
> (svn_cl__export): use updated svn_client_export3 function and pass
> inverse of --non-recursive command line through. Still defaults
> to the original behaviour of a recursive export.
> * subversion/clients/cmdline/main.c:
> (svn_cl__cmd_table): add "N" option to "export" command
Excellent log message. Might want to capitalize the first sentence in
each symbol summary, as we traditionally do, but no big deal. Also,
this is really an "option" not a "parameter" being added (at least,
that seems to be common usage among U.S. programmers, not sure about
elsewhere). And do give the "-N" short form too, in case anyone
searches the log messages on that instead of "--non-recursive".
> Index: subversion/include/svn_client.h
> --- subversion/include/svn_client.h (revision 13095)
> +++ subversion/include/svn_client.h (working copy)
> @@ -1588,6 +1588,9 @@
> * will use the standard eol marker. Any other value will cause the
> * SVN_ERR_IO_UNKNOWN_EOL error to be returned.
> + * @a recurse passes through to svn_ra_do_update to make exports of
> + * directories recursive if TRUE.
> + *
> * All allocations are done in @a pool.
A more verbose, but precise, description might be:
* If @a recurse is TRUE, then export subdirectories recursively, else
* just export @a from and its immediate non-directory children.
* Also, if @a recurse is TRUE, behave as if @a ignore_externals is
* also TRUE.
(I inserted a description of the proposed externals behavior just for
convenience, in case that's how we end up going.)
> Index: subversion/libsvn_client/export.c
> --- subversion/libsvn_client/export.c (revision 13095)
> +++ subversion/libsvn_client/export.c (working copy)
> @@ -852,7 +859,7 @@
> &reporter, &report_baton,
> "", /* no sub-target */
> - TRUE, /* recurse */
> + recurse, /* recurse */
> export_editor, edit_baton, pool));
I think leaving a comment "/* recurse */" next to a variable named
"recurse" qualifies as excessive redundancy :-).
Above minor nits notwithstanding, this was an excellent patch.
Looking forward to the next (and hopefully final) iteration.
To unsubscribe, e-mail: firstname.lastname@example.org
For additional commands, e-mail: email@example.com
Received on Tue Feb 22 04:37:00 2005