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/18 23:19:17 UTC
[PATCH] Add "--non-recursive" flag for export
Rowell, Geoff wrote:
> Really could have used this feature recently when developing packaging
> scripts.
>
>
>
> I'd like to be able to export a single directory, without any
> housekeeping information or lower-level directories.
And here's a patch against trunk to do just that.
[[[
Add a --non-recursive option to "svn export"
* subversion/libsvn_client/export.c:
(svn_client_export3): deprecate and call svn_client_export4,
defaulting to old recursive behaviour.
(svn_client_export4): add a "recursive" flag which gets passed
through to svn_ra_do_update.
* subversion/clients/cmdlind/export-cmd.c:
(svn_cl__export): use new svn_client_export4 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
[PATCH] Add "--non-recursive" flag for export - v3
Posted by Daniel Patterson <da...@danpat.net>.
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
Posted by kf...@collab.net.
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.
-Karl
Daniel Patterson <da...@danpat.net> writes:
> kfogel@collab.net wrote:
> Ah, I hadn't noticed that it was new for 1.2. I've re-done the change
> adding a parameter to svn_client_export3 (and updated svn_client.h)
> instead of a new function. Patch attached.
>
> I'm out of time today, but when I get a chance, I'll submit a patch
> for the regression tests (when I figure out how they fit together).
>
> [[[
> 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
> (svn_client_export2): default to passing TRUE for new
> "recurse" parameter
>
> * 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
> ]]]
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 13058)
> +++ subversion/include/svn_client.h (working copy)
> @@ -1568,6 +1568,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.
> */
> svn_error_t *
> @@ -1578,6 +1581,7 @@
> const svn_opt_revision_t *revision,
> svn_boolean_t force,
> svn_boolean_t ignore_externals,
> + svn_boolean_t recurse,
> const char *native_eol,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);
> Index: subversion/libsvn_client/externals.c
> ===================================================================
> --- subversion/libsvn_client/externals.c (revision 13058)
> +++ subversion/libsvn_client/externals.c (working copy)
> @@ -244,7 +244,8 @@
> SVN_ERR (svn_client_export3 (NULL, new_item->url, path,
> &(new_item->revision),
> &(new_item->revision),
> - TRUE, FALSE, NULL, ib->ctx, ib->pool));
> + TRUE, FALSE, TRUE, NULL,
> + ib->ctx, ib->pool));
> else
> SVN_ERR (svn_client__checkout_internal (NULL, new_item->url, path,
> &(new_item->revision),
> Index: subversion/libsvn_client/export.c
> ===================================================================
> --- subversion/libsvn_client/export.c (revision 13058)
> +++ subversion/libsvn_client/export.c (working copy)
> @@ -744,6 +744,7 @@
> const svn_opt_revision_t *revision,
> svn_boolean_t force,
> svn_boolean_t ignore_externals,
> + svn_boolean_t recurse,
> const char *native_eol,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> @@ -851,7 +852,7 @@
> &reporter, &report_baton,
> revnum,
> "", /* no sub-target */
> - TRUE, /* recurse */
> + recurse, /* recurse */
> export_editor, edit_baton, pool));
>
> SVN_ERR (reporter->set_path (report_baton, "", revnum,
> @@ -929,7 +930,8 @@
> peg_revision.kind = svn_opt_revision_unspecified;
>
> return svn_client_export3 (result_rev, from, to, &peg_revision,
> - revision, force, FALSE, native_eol, ctx, pool);
> + revision, force, FALSE, TRUE,
> + native_eol, ctx, pool);
> }
>
>
> Index: subversion/clients/cmdline/export-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/export-cmd.c (revision 13058)
> +++ subversion/clients/cmdline/export-cmd.c (working copy)
> @@ -74,6 +74,7 @@
> err = svn_client_export3 (NULL, truefrom, to, &peg_revision,
> &(opt_state->start_revision),
> opt_state->force, opt_state->ignore_externals,
> + opt_state->nonrecursive ? FALSE : TRUE,
> opt_state->native_eol, ctx,
> pool);
> if (err && err->apr_err == SVN_ERR_WC_OBSTRUCTED_UPDATE && !opt_state->force)
> Index: subversion/clients/cmdline/main.c
> ===================================================================
> --- subversion/clients/cmdline/main.c (revision 13058)
> +++ subversion/clients/cmdline/main.c (working copy)
> @@ -322,7 +322,7 @@
> " If specified, PEGREV determines in which revision the target is "
> "first\n"
> " looked up.\n"),
> - {'r', 'q', svn_cl__force_opt, SVN_CL__AUTH_OPTIONS,
> + {'r', 'q', 'N', svn_cl__force_opt, SVN_CL__AUTH_OPTIONS,
> svn_cl__config_dir_opt, svn_cl__native_eol_opt,
> svn_cl__ignore_externals_opt} },
>
---------------------------------------------------------------------
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
Posted by Daniel Patterson <da...@danpat.net>.
kfogel@collab.net wrote:
> Hmmm, I don't see any svn_client.h changes in there. They would be
> needed, because you'd have to publish the new svn_client_export4 API,
> and deprecate the old svn_client_export3 API, in the usual way. (See
> examples in the other header files, see HACKING).
>
> However, since svn_client_export3 is new in 1.2 anyway, and 1.2 has
> not been released yet, there's no need to make svn_client_export4.
> Just change svn_client_export3 as you need (remember that its API docs
> will need updating in svn_client.h too).
>
> Also, while not absolutely required, a regression test for the new
> feature is a huge bonus, and gives a patch higher precedence.
>
> .......
>
> Good change, overall. Can you resubmit it as described above? (Or if
> you don't have time, let me know, and I can make the adjustments.)
Ah, I hadn't noticed that it was new for 1.2. I've re-done the change
adding a parameter to svn_client_export3 (and updated svn_client.h)
instead of a new function. Patch attached.
I'm out of time today, but when I get a chance, I'll submit a patch
for the regression tests (when I figure out how they fit together).
[[[
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
(svn_client_export2): default to passing TRUE for new
"recurse" parameter
* 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
Posted by kf...@collab.net.
Daniel Patterson <da...@danpat.net> writes:
> And here's a patch against trunk to do just that.
Thanks for the quick action, Daniel!
Comments on the patch:
> [[[
> Add a --non-recursive option to "svn export"
>
> * subversion/libsvn_client/export.c:
> (svn_client_export3): deprecate and call svn_client_export4,
> defaulting to old recursive behaviour.
> (svn_client_export4): add a "recursive" flag which gets passed
> through to svn_ra_do_update.
>
> * subversion/clients/cmdlind/export-cmd.c:
> (svn_cl__export): use new svn_client_export4 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
> ]]]
Hmmm, I don't see any svn_client.h changes in there. They would be
needed, because you'd have to publish the new svn_client_export4 API,
and deprecate the old svn_client_export3 API, in the usual way. (See
examples in the other header files, see HACKING).
However, since svn_client_export3 is new in 1.2 anyway, and 1.2 has
not been released yet, there's no need to make svn_client_export4.
Just change svn_client_export3 as you need (remember that its API docs
will need updating in svn_client.h too).
Also, while not absolutely required, a regression test for the new
feature is a huge bonus, and gives a patch higher precedence.
> Index: subversion/libsvn_client/export.c
> ===================================================================
> --- subversion/libsvn_client/export.c (revision 13057)
> +++ subversion/libsvn_client/export.c (working copy)
> @@ -737,13 +737,14 @@
> /*** Public Interfaces ***/
>
> svn_error_t *
> -svn_client_export3 (svn_revnum_t *result_rev,
> +svn_client_export4 (svn_revnum_t *result_rev,
> const char *from,
> const char *to,
> const svn_opt_revision_t *peg_revision,
> const svn_opt_revision_t *revision,
> svn_boolean_t force,
> svn_boolean_t ignore_externals,
> + svn_boolean_t recursive,
> const char *native_eol,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> @@ -851,7 +852,7 @@
> &reporter, &report_baton,
> revnum,
> "", /* no sub-target */
> - TRUE, /* recurse */
> + recursive, /* recurse */
> export_editor, edit_baton, pool));
>
> SVN_ERR (reporter->set_path (report_baton, "", revnum,
> @@ -913,6 +914,22 @@
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_client_export3 (svn_revnum_t *result_rev,
> + const char *from,
> + const char *to,
> + const svn_opt_revision_t *peg_revision,
> + const svn_opt_revision_t *revision,
> + svn_boolean_t force,
> + svn_boolean_t ignore_externals,
> + const char *native_eol,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + return svn_client_export4 (result_rev, from, to, peg_revision,
> + revision, force, ignore_externals, TRUE,
> + native_eol, ctx, pool);
> +}
The shape of the code change looks correct to me. I haven't actually
tested it yet (this is one reason a regression test is handy).
Shockingly, the 'recurse' parameter of svn_ra_do_update is not
documented right now, a fact which makes it more difficult than it
should be to verify your change. Nor is that parameter documented for
svn_client_checkout2. Grrr. Inspection of the code indicates that
the 'recurse' flag does what we all think it does, anyway.
I'll fix these documentation bugs. They're not related to your patch,
except insofar as your patch caused them to be noticed.
Good change, overall. Can you resubmit it as described above? (Or if
you don't have time, let me know, and I can make the adjustments.)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org