Re: [PATCH] add ability to export a specific revision when a working copy path is supplied (v2)

[Philip Martin]

Daniel Shahaf  writes:

> Philip Martin wrote on Tue, 18 Jul 2017 17:01 +0100:
>> Committed as r1802316.  Thanks!
>
> What do you think about backporting this to 1.9.7, Philip?  While at the API
> level this adds a new promise to the docstring, at the CLI level this fixes
> an assertion error in a valid instance of usage #2 of 'export'.

Agreed, I think it meets the criteria for a backport.

-- 
Philip

1.10.0-alpha3 CHANGES

[Daniel Shahaf]

Hi,

I'm working on CHANGES for alpha3.  Could I please get a hand on the
following six changes.  For each of them: does it need to be listed in
CHANGES?  How'd you summarize the user-visible (or API-consumer-visible)
change in a sentence?

(In one or two case, I also wonder whether backport nominations are in
order, but that's orthogonal to the CHANGES work.)

> 
> r1783879 | stsp | 2017-02-21 12:14:01 + (Tue, 21 Feb 2017) | 14 lines
> 
> Stop recommending resolution options which follow incoming moves for merges.
> 
> It makes sense for update and switch. If merging, however, the user may want
> to apply an incoming edit to the node's old location, e.g. when backporting
> a file edit to an older release branch where the move should not be applied.
> Recommending that the move be applied without user interaction is not helpful
> in such a case. Instead, the resolver should offer two options:
> apply the move + edits, or apply edits at the old location.
> The latter option is not yet implemented.
> 
> * subversion/libsvn_client/conflicts.c
>   (init_wc_move_targets): Only recommend options which follow incoming moves
>for conflicts raised by update and switch operations.

> 
> r1791794 | stefan2 | 2017-04-18 14:50:07 + (Tue, 18 Apr 2017) | 11 lines
> 
> Fix inconsistent handling of FS-related config flags in mod_dav_svn.
> 
> * subversion/mod_dav_svn/mod_dav_svn.c
>   (dav_svn__get_fulltext_cache_flag,
>dav_svn__get_revprop_cache_flag,
>dav_svn__get_nodeprop_cache_flag,
>dav_svn__get_block_read_flag): Consistently use the get_conf_flag
>   function and default to FSFS's current
>   defaults.
> 
> Found by: julianfoad

> 
> r1795116 | stefan2 | 2017-05-14 20:22:49 + (Sun, 14 May 2017) | 17 lines
> 
> Fix issue #4677.
> https://issues.apache.org/jira/browse/SVN-4677 "svn up fails after a file is 
> added, moved, deleted, replaced by a directory, and a file in the directory 
> is deleted"
> 
> When non-exitent paths are allowed as a result of a DAG-walk
> (as opposed to creating a "path not found" error on those),
> we may encounter file nodes in the "parent" path.  Those are
> not an error but simply an indication that there won't be any
> sub-paths.
> 
> * subversion/libsvn_fs_fs/tree.c
>   (open_path): If n/a paths are allowed, non-dir parents are, too.
> 
> * subversion/libsvn_fs_x/dag_cache.c
>   (svn_fs_x__get_dag_path): Same.
> 
> * subversion/tests/libsvn_fs/fs-test.c
>   (closest_copy_test_svn_4677): Add regression test.
>   (test_funcs): Register new test.

> 
> r1795727 | stefan2 | 2017-05-21 20:22:51 + (Sun, 21 May 2017) | 8 lines
> 
> Address a minor initialization lib issue in svnmucc.
> 
> Clients should always explictly initialize the libraries that they are using.
> This prevents issues during the application tear-down phase.
> 
> * subversion/svnmucc/svnmucc.c
>   (sub_main): Explicitly initialize the RA layer just like the CL client does.

> 
> r1797908 | stsp | 2017-06-07 10:40:57 + (Wed, 07 Jun 2017) | 2 lines
> 
> * tools/dev/unix-build/Makefile.svn: Add a way to start a write-through proxy.

> 
> r1800836 | philip | 2017-07-04 22:37:52 + (Tue, 04 Jul 2017) | 15 lines
> 
> Remove some casts when using APR 2 and fix a "missing volatile" bug that
> was uncovered.
> 
> * subversion/include/private/svn_dep_compat.h
>   (svn_atomic_casptr, svn_atomic_xchgptr): New.
> 
> * subversion/libsvn_fs/fs-loader.c
>   (struct fs_type_defn): Add missing volatile to vtable element, change
>type to void * to avoid cast (we lose no type safety since we never
>access the value as any other type).
>   (get_library_vtable_direct): Remove explicit cast.
> 
> * subversion/libsvn_subr/utf.c
>   (atomic_swap): Remove explicit cast.

Last but not least, thanks to Evgeny for writing detailed introductory
paragraphs in his log messages, which made it very easy to understand
the high-level behaviour changes (as opposed to the symbol-level
changes, which are less important in the context of writing CHANGES).

Thanks,

Daniel

Re: [PATCH] add ability to export a specific revision when a working copy path is supplied (v2)

[Daniel Shahaf]

Philip Martin wrote on Tue, 18 Jul 2017 17:01 +0100:
> Doug Brown  writes:
> 
> > Thanks for your feedback! That change makes perfect sense. Here is a
> > second attempt at this patch that I think addresses everything you
> > listed. Patch with log message is attached.
> 
> Committed as r1802316.  Thanks!

What do you think about backporting this to 1.9.7, Philip?  While at the API
level this adds a new promise to the docstring, at the CLI level this fixes
an assertion error in a valid instance of usage #2 of 'export'.

Re: New conflict resolver bug in tracing history

[Julian Foad]

Stefan Sperling wrote:
> Thanks for finding this, Julian! Could you please file an issue for this?

https://issues.apache.org/jira/browse/SVN-4688

- Julian


> I expect that we'll find several such bugs. The related path hunting code
> needs thorough review and testing. It passes the tests we have, but it
> seems likely that it will still get confused in some situations.
> 
> That's one of the reason why I think we should issue an alpha release
> (and eventually release 1.10.0) to get sufficient testing.
> I don't expect this feature to be fully stable for the .0 release.

Re: svn commit: r1801970 - in /subversion/branches/shelve-checkpoint/subversion: libsvn_client/shelve.c svn/shelve-cmd.c

[Julian Foad]

Thanks for the review, Bert. I agree on all points.

- Julian


Bert Huijben wrote:
>> +  for (hi = apr_hash_first(scratch_pool, dirents); hi; hi = 
>> apr_hash_next(hi))
>>  {
> 
> I know it is just a prototype, but some stable sorting would be nice.

>> +  printf("%-30s %6d mins old %10ld bytes\n",
>> + name, age, (long)dirent->filesize);
> 
> And in general we try to avoid the standard print functions as these might 
> not be as compatible as expected. (UTF-8 support, buffering, etc.)

>> +  system(apr_psprintf(scratch_pool, "diffstat %s 2> /dev/null", 
>> path));
>> +  printf("\n");
> 
> And this will certainly fail on Windows (but the error is ignored anyway). I 
> would recommend flushing stdout before calling system() on other platforms.

Re: New conflict resolver bug in tracing history

[Stefan Sperling]

On Tue, Jul 18, 2017 at 12:27:42PM +0100, Julian Foad wrote:
> I just ran into the following bug. It appears that the conflict resolver
> doesn't properly follow branch (copy) history when searching for details.
> 
> [[[
> $ svn sw ^/subversion/branches/shelve-checkpoint3
> [...]
> 
> $ svn merge ^/subversion/trunk@1802151
> --- Merging r1801593 through r1802151 into '.':
> [...]
>C tools/dist/templates/nightly-candidates.ezt
> --- Recording mergeinfo for merge of r1801593 through r1802151 into '.':
>  U   .
> Summary of conflicts:
>   Tree conflicts: 1
> Searching tree conflict details for
> 'tools/dist/templates/nightly-candidates.ezt' in repository:
> Checking r1801838...svn: warning: apr_err=SVN_ERR_FS_NOT_FOUND
> svn: warning: W160013:
> '/repos/asf/!svn/rvr/1802151/subversion/branches/shelve-checkpoint3/tools/dist/templates'
> path not found
> /home/julianfoad/src/subversion-c/subversion/svn/merge-cmd.c:553,
> /home/julianfoad/src/subversion-c/subversion/svn/resolve-cmd.c:157:
> (apr_err=SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE)
> svn: E155027: Failure occurred resolving one or more conflicts
> ]]]
> 
> The conflict is a delete-delete: the file 'nightly-candidates.ezt' was
> deleted in branch shelve-checkpoint@r1801823 (note: that's a different
> branch) and also in trunk@r1801838. That branch was branched to
> shelve-checkpoint3 in r1802284.
> 
> The problem is that the look-up of the parent full-path (including
> branch name) fails because the branch 'shelve-checkpoint3' didn't exist
> in r1802151.
> 
> - Julian

Thanks for finding this, Julian! Could you please file an issue for this?

I expect that we'll find several such bugs. The related path hunting code
needs thorough review and testing. It passes the tests we have, but it
seems likely that it will still get confused in some situations.

That's one of the reason why I think we should issue an alpha release
(and eventually release 1.10.0) to get sufficient testing.
I don't expect this feature to be fully stable for the .0 release.

Re: [PATCH] add ability to export a specific revision when a working copy path is supplied (v2)

[Philip Martin]

Doug Brown  writes:

> Thanks for your feedback! That change makes perfect sense. Here is a
> second attempt at this patch that I think addresses everything you
> listed. Patch with log message is attached.

Committed as r1802316.  Thanks!

-- 
Philip

RE: svn commit: r1801970 - in /subversion/branches/shelve-checkpoint/subversion: libsvn_client/shelve.c svn/shelve-cmd.c

[Bert Huijben]

> -Original Message-
> From: julianf...@apache.org [mailto:julianf...@apache.org]
> Sent: vrijdag 14 juli 2017 16:22
> To: comm...@subversion.apache.org
> Subject: svn commit: r1801970 - in /subversion/branches/shelve-
> checkpoint/subversion: libsvn_client/shelve.c svn/shelve-cmd.c
> 
> Author: julianfoad
> Date: Fri Jul 14 14:21:35 2017
> New Revision: 1801970
> 
> URL: http://svn.apache.org/viewvc?rev=1801970=rev
> Log:
> On the 'shelve-checkpoint' branch: make 'svn shelve --list' more verbose.
> 
> Print the patch file age and size, and (unless '-q') also a diffstat.
> 
> * subversion/libsvn_client/shelve.c
>   (svn_client_shelves_list): Retrieve more stats including file size and
> mtime.
> 
> * subversion/svn/shelve-cmd.c
>   (shelves_list): Print each patch's age and size, and run 'diffstat' if
> found in the path.
>   (svn_cl__shelves,
>svn_cl__shelves): Update callers.
> 
> Modified:
> subversion/branches/shelve-checkpoint/subversion/libsvn_client/shelve.c
> subversion/branches/shelve-checkpoint/subversion/svn/shelve-cmd.c
> 
> Modified: subversion/branches/shelve-
> checkpoint/subversion/libsvn_client/shelve.c
> URL: http://svn.apache.org/viewvc/subversion/branches/shelve-
> checkpoint/subversion/libsvn_client/shelve.c?rev=1801970=1801969
> =1801970=diff
> ==
> 
> --- subversion/branches/shelve-
> checkpoint/subversion/libsvn_client/shelve.c (original)
> +++ subversion/branches/shelve-
> checkpoint/subversion/libsvn_client/shelve.c Fri Jul 14 14:21:35 2017
> @@ -314,7 +314,7 @@ svn_client_shelves_list(apr_hash_t **dir
> 
>SVN_ERR(svn_wc__get_shelves_dir(_dir, ctx->wc_ctx,
> local_abspath,
>scratch_pool, scratch_pool));
> -  SVN_ERR(svn_io_get_dirents3(dirents, shelves_dir, TRUE
> /*only_check_type*/,
> +  SVN_ERR(svn_io_get_dirents3(dirents, shelves_dir, FALSE
> /*only_check_type*/,
>result_pool, scratch_pool));
> 
>/* Remove non-shelves */
> 
> Modified: subversion/branches/shelve-checkpoint/subversion/svn/shelve-
> cmd.c
> URL: http://svn.apache.org/viewvc/subversion/branches/shelve-
> checkpoint/subversion/svn/shelve-
> cmd.c?rev=1801970=1801969=1801970=diff
> ==
> 
> --- subversion/branches/shelve-checkpoint/subversion/svn/shelve-cmd.c
> (original)
> +++ subversion/branches/shelve-checkpoint/subversion/svn/shelve-cmd.c
> Fri Jul 14 14:21:35 2017
> @@ -51,21 +51,36 @@ get_shelf_name(const char **shelf_name,
>  /* Display a list of shelves */
>  static svn_error_t *
>  shelves_list(const char *local_abspath,
> + svn_boolean_t diffstat,
>   svn_client_ctx_t *ctx,
> - apr_pool_t *pool)
> + apr_pool_t *scratch_pool)
>  {
>apr_hash_t *dirents;
>apr_hash_index_t *hi;
> 
> -  SVN_ERR(svn_client_shelves_list(, local_abspath, ctx, pool, pool));
> +  SVN_ERR(svn_client_shelves_list(, local_abspath,
> +  ctx, scratch_pool, scratch_pool));
> 
> -  for (hi = apr_hash_first(pool, dirents); hi; hi = apr_hash_next(hi))
> +  for (hi = apr_hash_first(scratch_pool, dirents); hi; hi = 
> apr_hash_next(hi))
>  {

I know it is just a prototype, but some stable sorting would be nice.


>const char *name = apr_hash_this_key(hi);
> +  svn_io_dirent2_t *dirent = apr_hash_this_val(hi);
> +  int age = (apr_time_now() - dirent->mtime) / 100 / 60;
> 
> -  if (strstr(name, ".patch"))
> +  if (! strstr(name, ".patch"))
> +continue;
> +
> +  printf("%-30s %6d mins old %10ld bytes\n",
> + name, age, (long)dirent->filesize);

And in general we try to avoid the standard print functions as these might not 
be as compatible as expected. (UTF-8 support, buffering, etc.)
> +
> +  if (diffstat)
>  {
> -  printf("%s\n", name);
> +  char *path = svn_path_join_many(scratch_pool,
> +  local_abspath, ".svn/shelves", 
> name,
> +  SVN_VA_NULL);
> +
> +  system(apr_psprintf(scratch_pool, "diffstat %s 2> /dev/null", 
> path));
> +  printf("\n");

And this will certainly fail on Windows (but the error is ignored anyway). I 
would recommend flushing stdout before calling system() on other platforms.

Bert
>  }
>  }
> 
> @@ -94,7 +109,9 @@ svn_cl__shelve(apr_getopt_t *os,
>if (os->ind < os->argc)
>  return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, 0, NULL);
> 
> -  SVN_ERR(shelves_list(local_abspath, ctx, pool));
> +  SVN_ERR(shelves_list(local_abspath,
> +   ! opt_state->quiet /*diffstat*/,
> +   ctx, pool));
>return SVN_NO_ERROR;
>  }
> 
> @@ -192,7 +209,7 @@ svn_cl__shelves(apr_getopt_t *os,
>  return 

New conflict resolver bug in tracing history

[Julian Foad]

I just ran into the following bug. It appears that the conflict resolver
doesn't properly follow branch (copy) history when searching for details.

[[[
$ svn sw ^/subversion/branches/shelve-checkpoint3
[...]

$ svn merge ^/subversion/trunk@1802151
--- Merging r1801593 through r1802151 into '.':
[...]
   C tools/dist/templates/nightly-candidates.ezt
--- Recording mergeinfo for merge of r1801593 through r1802151 into '.':
 U   .
Summary of conflicts:
  Tree conflicts: 1
Searching tree conflict details for
'tools/dist/templates/nightly-candidates.ezt' in repository:
Checking r1801838...svn: warning: apr_err=SVN_ERR_FS_NOT_FOUND
svn: warning: W160013:
'/repos/asf/!svn/rvr/1802151/subversion/branches/shelve-checkpoint3/tools/dist/templates'
path not found
/home/julianfoad/src/subversion-c/subversion/svn/merge-cmd.c:553,
/home/julianfoad/src/subversion-c/subversion/svn/resolve-cmd.c:157:
(apr_err=SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE)
svn: E155027: Failure occurred resolving one or more conflicts
]]]

The conflict is a delete-delete: the file 'nightly-candidates.ezt' was
deleted in branch shelve-checkpoint@r1801823 (note: that's a different
branch) and also in trunk@r1801838. That branch was branched to
shelve-checkpoint3 in r1802284.

The problem is that the look-up of the parent full-path (including
branch name) fails because the branch 'shelve-checkpoint3' didn't exist
in r1802151.

- Julian