You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Patterson <da...@danpat.net> on 2005/02/21 09:16:06 UTC
[PATCH] Add "--non-recursive" flag for export - v3
kfogel@collab.net wrote:
> Thanks for the new patch.
>
> 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
existed.
Updated log message below, updated patch attached. Still no
test cases, my Python literacy is not great.
[[[
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
]]]
Re: [PATCH] Add "--non-recursive" flag for export - v3
Posted by kf...@collab.net.
Daniel Patterson <da...@danpat.net> writes:
> > http://subversion.tigris.org/issues/show_bug.cgi?id=2228
>
> New patch ready, including the above behaviour and the minor
> nits corrected. Now, all I need is for someone to update my
> email address for tigris.org so I can reset my password and I'll
> attach it to the issue :-) (I've sent email to feedback@tigris.org
> to make that happen).
Done :-).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add "--non-recursive" flag for export - v3
Posted by kf...@collab.net.
Daniel Patterson <da...@danpat.net> writes:
> > http://subversion.tigris.org/issues/show_bug.cgi?id=2228
>
> New patch ready, including the above behaviour and the minor
> nits corrected. Now, all I need is for someone to update my
> email address for tigris.org so I can reset my password and I'll
> attach it to the issue :-) (I've sent email to feedback@tigris.org
> to make that happen).
Committed in r13121.
So, what's next? :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add "--non-recursive" flag for export - v3
Posted by Daniel Patterson <da...@danpat.net>.
kfogel@collab.net wrote:
> 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
>
Yep, makes sense to me.
> http://subversion.tigris.org/issues/show_bug.cgi?id=2228
New patch ready, including the above behaviour and the minor
nits corrected. Now, all I need is for someone to update my
email address for tigris.org so I can reset my password and I'll
attach it to the issue :-) (I've sent email to feedback@tigris.org
to make that happen).
daniel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Add "--non-recursive" flag for export - v3
Posted by kf...@collab.net.
Daniel Patterson <da...@danpat.net> writes:
> kfogel@collab.net 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
> existed.
>
> 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
not.
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
http://subversion.tigris.org/issues/show_bug.cgi?id=2228
...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,
> revnum,
> "", /* 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.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org