You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2010/04/16 00:19:00 UTC
svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Author: stsp
Date: Thu Apr 15 22:18:59 2010
New Revision: 934627
URL: http://svn.apache.org/viewvc?rev=934627&view=rev
Log:
Reintegrate the svn-patch-improvements branch into trunk.
This adds support for deleting directories which are empty after
patching. See the branch log messages for details.
Modified:
subversion/trunk/ (props changed)
subversion/trunk/subversion/libsvn_client/patch.c
subversion/trunk/subversion/tests/cmdline/patch_tests.py
Propchange: subversion/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Apr 15 22:18:59 2010
@@ -24,7 +24,7 @@
/subversion/branches/reintegrate-improvements:873853-874164
/subversion/branches/subtree-mergeinfo:876734-878766
/subversion/branches/svn-mergeinfo-enhancements:870119-870195,870197-870288
-/subversion/branches/svn-patch-improvements:929288,931140
+/subversion/branches/svn-patch-improvements:918519-934609
/subversion/branches/svnpatch-diff:865738-876477
/subversion/branches/svnraisetc:874709-875149
/subversion/branches/svnserve-logging:869828-870893
Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=934627&r1=934626&r2=934627&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Thu Apr 15 22:18:59 2010
@@ -36,6 +36,7 @@
#include "svn_path.h"
#include "svn_pools.h"
#include "svn_props.h"
+#include "svn_sorts.h"
#include "svn_subst.h"
#include "svn_wc.h"
#include "client.h"
@@ -157,6 +158,10 @@ typedef struct {
/* True if the target's parent directory exists. */
svn_boolean_t parent_dir_exists;
+ /* True if the target's parent directory is being deleted
+ * as a result of the patching process. */
+ svn_boolean_t parent_dir_deleted;
+
/* The keywords of the target. */
apr_hash_t *keywords;
@@ -515,6 +520,7 @@ init_patch_target(patch_target_t **patch
target->had_rejects = FALSE;
target->deleted = FALSE;
target->eof = FALSE;
+ target->parent_dir_deleted = FALSE;
target->pool = result_pool;
target->lines = apr_array_make(result_pool, 0,
sizeof(svn_stream_mark_t *));
@@ -1273,7 +1279,7 @@ install_patched_target(patch_target_t *t
{
if (target->deleted)
{
- if (! dry_run)
+ if (! dry_run && ! target->parent_dir_deleted)
{
/* Schedule the target for deletion. Suppress
* notification, we'll do it manually in a minute.
@@ -1441,6 +1447,290 @@ install_patched_target(patch_target_t *t
return SVN_NO_ERROR;
}
+/* Baton for find_existing_children() */
+struct status_baton
+{
+ apr_array_header_t *existing_targets;
+ const char *parent_path;
+ apr_pool_t *result_pool;
+};
+
+/* Implements svn_wc_status_func4_t. */
+static svn_error_t *
+find_existing_children(void *baton,
+ const char *abspath,
+ const svn_wc_status3_t *status,
+ apr_pool_t *pool)
+{
+ struct status_baton *btn = baton;
+
+ if (status->text_status != svn_wc_status_none
+ && status->text_status != svn_wc_status_deleted
+ && strcmp(abspath, btn->parent_path))
+ {
+ APR_ARRAY_PUSH(btn->existing_targets,
+ const char *) = apr_pstrdup(btn->result_pool,
+ abspath);
+ }
+
+ return SVN_NO_ERROR;
+}
+
+/* Indicate in *EMPTY whether the directory at LOCAL_ABSPATH has any
+ * versioned or unversioned children. Consider any DELETED_TARGETS,
+ * as well as paths listed in DELETED_ABSPATHS_LIST (which may be NULL)
+ * as already deleted. Use WC_CTX as the working copy context.
+ * Do temporary allocations in SCRATCH_POOL. */
+static svn_error_t *
+check_dir_empty(svn_boolean_t *empty, const char *local_abspath,
+ svn_wc_context_t *wc_ctx,
+ apr_array_header_t *deleted_targets,
+ apr_array_header_t *deleted_abspath_list,
+ apr_pool_t *scratch_pool)
+{
+ struct status_baton btn;
+ svn_opt_revision_t revision;
+ svn_error_t *err;
+ int i;
+
+ btn.existing_targets = apr_array_make(scratch_pool, 0,
+ sizeof(patch_target_t *));
+ btn.parent_path = local_abspath;
+ btn.result_pool = scratch_pool;
+ revision.kind = svn_opt_revision_unspecified;
+
+ err = svn_wc_walk_status(wc_ctx, local_abspath, svn_depth_immediates,
+ TRUE, TRUE, TRUE, NULL, find_existing_children,
+ &btn, NULL, NULL, NULL, NULL, scratch_pool);
+ if (err)
+ {
+ if (err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
+ {
+ /* The patch has deleted the entire working copy. */
+ svn_error_clear(err);
+ *empty = FALSE;
+ return SVN_NO_ERROR;
+ }
+ else
+ svn_error_return(err);
+ }
+
+ *empty = TRUE;
+
+ /* Do we delete all targets? */
+ for (i = 0; i < btn.existing_targets->nelts; i++)
+ {
+ int j;
+ const char *found;
+ svn_boolean_t deleted;
+
+ deleted = FALSE;
+ found = APR_ARRAY_IDX(btn.existing_targets, i, const char *);
+
+ for (j = 0; j < deleted_targets->nelts; j++)
+ {
+ patch_target_t *target;
+
+ target = APR_ARRAY_IDX(deleted_targets, j, patch_target_t *);
+ if (! svn_path_compare_paths(found, target->abs_path))
+ {
+ deleted = TRUE;
+ break;
+ }
+ }
+ if (! deleted && deleted_abspath_list)
+ {
+ for (j = 0; j < deleted_abspath_list->nelts; j++)
+ {
+ const char *abspath;
+
+ abspath = APR_ARRAY_IDX(deleted_abspath_list, j, const char *);
+ if (! svn_path_compare_paths(found, abspath))
+ {
+ deleted = TRUE;
+ break;
+ }
+ }
+ }
+ if (! deleted)
+ {
+ *empty = FALSE;
+ break;
+ }
+ }
+
+ return SVN_NO_ERROR;
+}
+
+/* Push a copy of EMPTY_DIR, allocated in RESULT_POOL, onto the EMPTY_DIRS
+ * array if no directory matching EMPTY_DIR is already in the array. */
+static void
+push_if_unique(apr_array_header_t *empty_dirs, const char *empty_dir,
+ apr_pool_t *result_pool)
+{
+ svn_boolean_t is_unique;
+ int i;
+
+ is_unique = TRUE;
+ for (i = 0; i < empty_dirs->nelts; i++)
+ {
+ const char *empty_dir2;
+
+ empty_dir2 = APR_ARRAY_IDX(empty_dirs, i, const char *);
+ if (strcmp(empty_dir, empty_dir2) == 0)
+ {
+ is_unique = FALSE;
+ break;
+ }
+ }
+
+ if (is_unique)
+ APR_ARRAY_PUSH(empty_dirs, const char *) = apr_pstrdup(result_pool,
+ empty_dir);
+}
+
+/* Delete all directories from the working copy which are left empty
+ * by deleted TARGETS. Use client context CTX.
+ * If DRY_RUN is TRUE, do not modify the working copy.
+ * Do temporary allocations in SCRATCH_POOL. */
+static svn_error_t *
+delete_empty_dirs(apr_array_header_t *targets, svn_client_ctx_t *ctx,
+ svn_boolean_t dry_run, apr_pool_t *scratch_pool)
+{
+ apr_array_header_t *empty_dirs;
+ apr_array_header_t *deleted_targets;
+ apr_pool_t *iterpool;
+ svn_boolean_t again;
+ int i;
+
+ /* Get a list of all deleted targets. */
+ deleted_targets = apr_array_make(scratch_pool, 0, sizeof(patch_target_t *));
+ for (i = 0; i < targets->nelts; i++)
+ {
+ patch_target_t *target;
+
+ target = APR_ARRAY_IDX(targets, i, patch_target_t *);
+ if (target->deleted)
+ APR_ARRAY_PUSH(deleted_targets, patch_target_t *) = target;
+ }
+
+ /* We have nothing to do if there aren't any deleted targets. */
+ if (deleted_targets->nelts == 0)
+ return SVN_NO_ERROR;
+
+ /* Look for empty parent directories of deleted targets. */
+ empty_dirs = apr_array_make(scratch_pool, 0, sizeof(const char *));
+ iterpool = svn_pool_create(scratch_pool);
+ for (i = 0; i < targets->nelts; i++)
+ {
+ svn_boolean_t parent_empty;
+ patch_target_t *target;
+ const char *parent;
+
+ svn_pool_clear(iterpool);
+
+ target = APR_ARRAY_IDX(targets, i, patch_target_t *);
+ parent = svn_dirent_dirname(target->abs_path, iterpool);
+ SVN_ERR(check_dir_empty(&parent_empty, parent, ctx->wc_ctx,
+ deleted_targets, NULL, iterpool));
+ if (parent_empty)
+ {
+ APR_ARRAY_PUSH(empty_dirs, const char *) =
+ apr_pstrdup(scratch_pool, parent);
+ }
+ }
+
+ /* We have nothing to do if there aren't any empty directories. */
+ if (empty_dirs->nelts == 0)
+ {
+ svn_pool_destroy(iterpool);
+ return SVN_NO_ERROR;
+ }
+
+ /* Determine the minimal set of empty directories we need to delete. */
+ do
+ {
+ apr_array_header_t *empty_dirs_copy;
+
+ svn_pool_clear(iterpool);
+
+ /* Rebuild the empty dirs list, replacing empty dirs which have
+ * an empty parent with their parent. */
+ again = FALSE;
+ empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
+ apr_array_clear(empty_dirs);
+ for (i = 0; i < empty_dirs_copy->nelts; i++)
+ {
+ svn_boolean_t parent_empty;
+ const char *empty_dir;
+ const char *parent;
+
+ empty_dir = APR_ARRAY_IDX(empty_dirs_copy, i, const char *);
+ parent = svn_dirent_dirname(empty_dir, iterpool);
+ SVN_ERR(check_dir_empty(&parent_empty, parent, ctx->wc_ctx,
+ deleted_targets, empty_dirs_copy,
+ iterpool));
+ if (parent_empty)
+ {
+ again = TRUE;
+ push_if_unique(empty_dirs, parent, scratch_pool);
+ }
+ else
+ push_if_unique(empty_dirs, empty_dir, scratch_pool);
+ }
+ }
+ while (again);
+
+ /* Mark all children of empty parent directories. */
+ for (i = 0; i < targets->nelts ; i++)
+ {
+ patch_target_t *target;
+ int j;
+
+ target = APR_ARRAY_IDX(targets, i, patch_target_t *);
+ for (j = 0; j < empty_dirs->nelts; j++)
+ {
+ const char *empty_dir;
+
+ empty_dir = APR_ARRAY_IDX(empty_dirs, j, const char *);
+ if (svn_dirent_is_ancestor(empty_dir, target->abs_path))
+ {
+ target->parent_dir_deleted = TRUE;
+ break;
+ }
+ }
+ }
+
+ /* Finally, delete empty directories. */
+ for (i = 0; i < empty_dirs->nelts; i++)
+ {
+ const char *empty_dir;
+
+ svn_pool_clear(iterpool);
+
+ if (ctx->cancel_func)
+ SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
+
+ empty_dir = APR_ARRAY_IDX(empty_dirs, i, const char *);
+ if (ctx->notify_func2)
+ {
+ svn_wc_notify_t *notify;
+
+ notify = svn_wc_create_notify(empty_dir, svn_wc_notify_delete,
+ iterpool);
+ (*ctx->notify_func2)(ctx->notify_baton2, notify, iterpool);
+ }
+ if (! dry_run)
+ SVN_ERR(svn_wc_delete4(ctx->wc_ctx, empty_dir, FALSE, FALSE,
+ NULL, NULL, /* suppress cancellation */
+ NULL, NULL, /* suppress notification */
+ iterpool));
+ }
+ svn_pool_destroy(iterpool);
+
+ return SVN_NO_ERROR;
+}
+
/* Baton for apply_patches(). */
typedef struct {
/* The path to the patch file. */
@@ -1540,6 +1830,9 @@ apply_patches(void *baton,
}
while (patch);
+ /* Delete directories which are empty after patching, if any. */
+ SVN_ERR(delete_empty_dirs(targets, btn->ctx, btn->dry_run, scratch_pool));
+
/* Install patched targets into the working copy. */
for (i = 0; i < targets->nelts; i++)
{
@@ -1554,8 +1847,10 @@ apply_patches(void *baton,
if (! target->skipped)
SVN_ERR(install_patched_target(target, btn->abs_wc_path,
btn->ctx, btn->dry_run, iterpool));
- SVN_ERR(send_patch_notification(target, btn->ctx, iterpool));
- SVN_ERR(svn_diff_close_patch(target->patch));
+ if (! target->parent_dir_deleted)
+ SVN_ERR(send_patch_notification(target, btn->ctx, iterpool));
+ if (target->patch)
+ SVN_ERR(svn_diff_close_patch(target->patch));
}
SVN_ERR(svn_io_file_close(patch_file, iterpool));
Modified: subversion/trunk/subversion/tests/cmdline/patch_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/patch_tests.py?rev=934627&r1=934626&r2=934627&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/patch_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/patch_tests.py Thu Apr 15 22:18:59 2010
@@ -1001,6 +1001,107 @@ def patch_add_new_dir(sbox):
None, # expected err
1, # check-props
1) # dry-run
+def patch_remove_empty_dirs(sbox):
+ "patch deleting all children of a directory"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ patch_file_path = make_patch_path(sbox)
+
+ # Contents of B:
+ # A/B/lamba
+ # A/B/F
+ # A/B/E/{alpha,beta}
+ # Before patching we've deleted F, which means that B is empty after patching and
+ # should be removed.
+ #
+ # Contents of H:
+ # A/D/H/{chi,psi,omega}
+ # Before patching, chi has been removed by a non-svn operation which means it has
+ # status missing. The patch deletes the other two files but should not delete H.
+
+ unidiff_patch = [
+ "Index: psi\n",
+ "===================================================================\n",
+ "--- A/D/H/psi\t(revision 0)\n",
+ "+++ A/D/H/psi\t(revision 0)\n",
+ "@@ -1 +0,0 @@\n",
+ "-This is the file 'psi'.\n",
+ "Index: omega\n",
+ "===================================================================\n",
+ "--- A/D/H/omega\t(revision 0)\n",
+ "+++ A/D/H/omega\t(revision 0)\n",
+ "@@ -1 +0,0 @@\n",
+ "-This is the file 'omega'.\n",
+ "Index: lambda\n",
+ "===================================================================\n",
+ "--- A/B/lambda\t(revision 0)\n",
+ "+++ A/B/lambda\t(revision 0)\n",
+ "@@ -1 +0,0 @@\n",
+ "-This is the file 'lambda'.\n",
+ "Index: alpha\n",
+ "===================================================================\n",
+ "--- A/B/E/alpha\t(revision 0)\n",
+ "+++ A/B/E/alpha\t(revision 0)\n",
+ "@@ -1 +0,0 @@\n",
+ "-This is the file 'alpha'.\n",
+ "Index: beta\n",
+ "===================================================================\n",
+ "--- A/B/E/beta\t(revision 0)\n",
+ "+++ A/B/E/beta\t(revision 0)\n",
+ "@@ -1 +0,0 @@\n",
+ "-This is the file 'beta'.\n",
+ ]
+
+ svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
+
+ F_path = os.path.join(wc_dir, 'A', 'B', 'F')
+ svntest.actions.run_and_verify_svn("Deleting F failed", None, [],
+ 'rm', F_path)
+ svntest.actions.run_and_verify_svn("Update failed", None, [],
+ 'up', wc_dir)
+
+ # We should be able to handle one path beeing missing.
+ os.remove(os.path.join(wc_dir, 'A', 'D', 'H', 'chi'))
+
+ expected_output = [
+ 'D %s\n' % os.path.join(wc_dir, 'A', 'B'),
+ 'D %s\n' % os.path.join(wc_dir, 'A', 'D', 'H', 'psi'),
+ 'D %s\n' % os.path.join(wc_dir, 'A', 'D', 'H', 'omega'),
+ ]
+
+ expected_disk = svntest.main.greek_state.copy()
+ expected_disk.remove('A/D/H/chi')
+ expected_disk.remove('A/D/H/psi')
+ expected_disk.remove('A/D/H/omega')
+ expected_disk.remove('A/B/lambda')
+ expected_disk.remove('A/B/E/alpha')
+ expected_disk.remove('A/B/E/beta')
+
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.add({'A/D/H/chi' : Item(status='! ', wc_rev=1)})
+ expected_status.add({'A/D/H/omega' : Item(status='D ', wc_rev=1)})
+ expected_status.add({'A/D/H/psi' : Item(status='D ', wc_rev=1)})
+ expected_status.add({'A/B' : Item(status='D ', wc_rev=1)})
+ expected_status.add({'A/B/E' : Item(status='D ', wc_rev=1)})
+ expected_status.add({'A/B/E/beta' : Item(status='D ', wc_rev=1)})
+ expected_status.add({'A/B/E/alpha' : Item(status='D ', wc_rev=1)})
+ expected_status.add({'A/B/lambda' : Item(status='D ', wc_rev=1)})
+ expected_status.add({'A/B/F' : Item(status='D ', wc_rev=1)})
+
+ expected_skip = wc.State('', { })
+
+ svntest.actions.run_and_verify_patch(wc_dir,
+ os.path.abspath(patch_file_path),
+ expected_output,
+ expected_disk,
+ expected_status,
+ expected_skip,
+ None, # expected err
+ 1, # check-props
+ 1) # dry-run
+
def patch_reject(sbox):
"patch which is rejected"
@@ -2397,6 +2498,7 @@ test_list = [ None,
patch_strip1,
patch_no_index_line,
patch_add_new_dir,
+ patch_remove_empty_dirs,
patch_reject,
patch_keywords,
patch_with_fuzz,
Re: svn commit: r934627 - in /subversion/trunk: ./ subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> On Fri, Apr 16, 2010 at 11:38:41AM +0100, Philip Martin wrote:
>> Julian Foad <ju...@wandisco.com> writes:
>> > Just write "arr->nelts = 0;" (which is the entire body of
>> > apr_array_clear()) and don't introduce an ifdef.
>
> I'm a bit sceptical.
> Do we really want to rely on implementation details like
> "arr->nelts = 0" in the APR code?
Two examples where we already set nelts:
$ grep nelts\ =[^=] ../src/subversion/libsvn_*/*c
../src/subversion/libsvn_fs_base/dag.c: copy_ids->nelts = i_out;
../src/subversion/libsvn_fs_fs/caching.c: manifest->nelts = data_len / sizeof(apr_off_t);
--
Philip
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 16, 2010 at 10:50:22AM -0500, Hyrum K. Wright wrote:
> This was causing some of the buildbots not to be able to build, so I went
> ahead and committed a fix in r934969.
Thanks!
Stefan
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Fri, Apr 16, 2010 at 7:39 AM, Stefan Sperling <st...@elego.de> wrote:
> On Fri, Apr 16, 2010 at 11:38:41AM +0100, Philip Martin wrote:
> > Julian Foad <ju...@wandisco.com> writes:
> > > Just write "arr->nelts = 0;" (which is the entire body of
> > > apr_array_clear()) and don't introduce an ifdef.
>
> I'm a bit sceptical.
> Do we really want to rely on implementation details like
> "arr->nelts = 0" in the APR code? We don't even do this in our own code.
>
> > I agree that we don't want #if like that in the main code. Put it in
> > one of our private header files if you want to use it.
>
> I can make a private macro or function for this, sure.
>
This was causing some of the buildbots not to be able to build, so I went
ahead and committed a fix in r934969. Feel free to review/update.
-Hyrum
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 16, 2010 at 11:38:41AM +0100, Philip Martin wrote:
> Julian Foad <ju...@wandisco.com> writes:
> > Just write "arr->nelts = 0;" (which is the entire body of
> > apr_array_clear()) and don't introduce an ifdef.
I'm a bit sceptical.
Do we really want to rely on implementation details like
"arr->nelts = 0" in the APR code? We don't even do this in our own code.
> I agree that we don't want #if like that in the main code. Put it in
> one of our private header files if you want to use it.
I can make a private macro or function for this, sure.
Stefan
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Fri, Apr 16, 2010 at 5:38 AM, Philip Martin
<ph...@wandisco.com>wrote:
> Julian Foad <ju...@wandisco.com> writes:
>
> > Stefan Sperling wrote:
> >> On Fri, Apr 16, 2010 at 10:08:15AM +0100, Philip Martin wrote:
> >> > stsp@apache.org writes:
> >> >
> >> > > Author: stsp
> >> > > Date: Thu Apr 15 22:18:59 2010
> >> > > New Revision: 934627
> >> > >
> >> > > URL: http://svn.apache.org/viewvc?rev=934627&view=rev
> >> > > Log:
> >> > > Reintegrate the svn-patch-improvements branch into trunk.
> >> >
> >> > > + /* Rebuild the empty dirs list, replacing empty dirs which
> have
> >> > > + * an empty parent with their parent. */
> >> > > + again = FALSE;
> >> > > + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
> >> > > + apr_array_clear(empty_dirs);
> >> > > + for (i = 0; i < empty_dirs_copy->nelts; i++)
> >> >
> >> > apr_array_clear is only available in APR 1.3.x and later.
> >> >
> >> > We could write svn_array_clear, or perhaps even apr_array_clear, or we
> >> > could simply reset empty_dirs->nelts directly.
> >>
> >> Hmmm...
> >>
> >> I'll use apr_array_clear in an #ifdef APR_VERSION(...), and clear the
>
> APR_VERSION_AT_LEAST
>
When using this macro, don't forget to include svn_dep_compat.h.
APR_VERSION_AT_LEAST wasn't introduced into APR itself until 1.3.0.
> >> array manually in the #else clause.
> >
> > Just write "arr->nelts = 0;" (which is the entire body of
> > apr_array_clear()) and don't introduce an ifdef.
>
> I agree that we don't want #if like that in the main code. Put it in
> one of our private header files if you want to use it.
>
+1
-Hyrum
Re: svn commit: r934627 - in /subversion/trunk: ./ subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@wandisco.com> writes:
> Stefan Sperling wrote:
>> On Fri, Apr 16, 2010 at 10:08:15AM +0100, Philip Martin wrote:
>> > stsp@apache.org writes:
>> >
>> > > Author: stsp
>> > > Date: Thu Apr 15 22:18:59 2010
>> > > New Revision: 934627
>> > >
>> > > URL: http://svn.apache.org/viewvc?rev=934627&view=rev
>> > > Log:
>> > > Reintegrate the svn-patch-improvements branch into trunk.
>> >
>> > > + /* Rebuild the empty dirs list, replacing empty dirs which have
>> > > + * an empty parent with their parent. */
>> > > + again = FALSE;
>> > > + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
>> > > + apr_array_clear(empty_dirs);
>> > > + for (i = 0; i < empty_dirs_copy->nelts; i++)
>> >
>> > apr_array_clear is only available in APR 1.3.x and later.
>> >
>> > We could write svn_array_clear, or perhaps even apr_array_clear, or we
>> > could simply reset empty_dirs->nelts directly.
>>
>> Hmmm...
>>
>> I'll use apr_array_clear in an #ifdef APR_VERSION(...), and clear the
APR_VERSION_AT_LEAST
>> array manually in the #else clause.
>
> Just write "arr->nelts = 0;" (which is the entire body of
> apr_array_clear()) and don't introduce an ifdef.
I agree that we don't want #if like that in the main code. Put it in
one of our private header files if you want to use it.
--
Philip
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Julian Foad <ju...@wandisco.com>.
Stefan Sperling wrote:
> On Fri, Apr 16, 2010 at 10:08:15AM +0100, Philip Martin wrote:
> > stsp@apache.org writes:
> >
> > > Author: stsp
> > > Date: Thu Apr 15 22:18:59 2010
> > > New Revision: 934627
> > >
> > > URL: http://svn.apache.org/viewvc?rev=934627&view=rev
> > > Log:
> > > Reintegrate the svn-patch-improvements branch into trunk.
> >
> > > + /* Rebuild the empty dirs list, replacing empty dirs which have
> > > + * an empty parent with their parent. */
> > > + again = FALSE;
> > > + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
> > > + apr_array_clear(empty_dirs);
> > > + for (i = 0; i < empty_dirs_copy->nelts; i++)
> >
> > apr_array_clear is only available in APR 1.3.x and later.
> >
> > We could write svn_array_clear, or perhaps even apr_array_clear, or we
> > could simply reset empty_dirs->nelts directly.
>
> Hmmm...
>
> I'll use apr_array_clear in an #ifdef APR_VERSION(...), and clear the
> array manually in the #else clause.
Just write "arr->nelts = 0;" (which is the entire body of
apr_array_clear()) and don't introduce an ifdef.
- Julian
Re: svn commit: r934627 - in /subversion/trunk: ./
subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 16, 2010 at 10:08:15AM +0100, Philip Martin wrote:
> stsp@apache.org writes:
>
> > Author: stsp
> > Date: Thu Apr 15 22:18:59 2010
> > New Revision: 934627
> >
> > URL: http://svn.apache.org/viewvc?rev=934627&view=rev
> > Log:
> > Reintegrate the svn-patch-improvements branch into trunk.
>
> > + /* Rebuild the empty dirs list, replacing empty dirs which have
> > + * an empty parent with their parent. */
> > + again = FALSE;
> > + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
> > + apr_array_clear(empty_dirs);
> > + for (i = 0; i < empty_dirs_copy->nelts; i++)
>
> apr_array_clear is only available in APR 1.3.x and later.
>
> We could write svn_array_clear, or perhaps even apr_array_clear, or we
> could simply reset empty_dirs->nelts directly.
Hmmm...
I'll use apr_array_clear in an #ifdef APR_VERSION(...), and clear the
array manually in the #else clause.
Thanks for catching this,
Stefan
--
printf("Eh???/n");
Re: svn commit: r934627 - in /subversion/trunk: ./ subversion/libsvn_client/patch.c subversion/tests/cmdline/patch_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
stsp@apache.org writes:
> Author: stsp
> Date: Thu Apr 15 22:18:59 2010
> New Revision: 934627
>
> URL: http://svn.apache.org/viewvc?rev=934627&view=rev
> Log:
> Reintegrate the svn-patch-improvements branch into trunk.
> + /* Rebuild the empty dirs list, replacing empty dirs which have
> + * an empty parent with their parent. */
> + again = FALSE;
> + empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
> + apr_array_clear(empty_dirs);
> + for (i = 0; i < empty_dirs_copy->nelts; i++)
apr_array_clear is only available in APR 1.3.x and later.
We could write svn_array_clear, or perhaps even apr_array_clear, or we
could simply reset empty_dirs->nelts directly.
--
Philip