Re: [Proposed] Split very long messages by paragraph for easy translate

[Dongsheng Song]

On Sun, Nov 14, 2010 at 01:19, Greg Hudson  wrote:
> On Sat, 2010-11-13 at 10:31 -0500, Daniel Shahaf wrote:
>> Sounds reasonable.
>>
>> What changes to the source code would be required?
>>
>> Do we just change
>>       N_("three\n\nparagraphs\n\nhere\n")
>> to
>>       N_("three\n") N_("paragraphs\n") N_("here\n")
>
> No, that would just result in evaluating gettext on the combined string,
> same as before.  I can see two options for help strings in particular:
>
> 1. Rev svn_opt_subcommand_desc2_t to include an array of help strings
> which are translated and displayed in sequence.
>
> 2. Change print_command_info2 to look at the help string and break it up
> at certain boundaries (such as blank lines or numbered list entries)
> before translating it.
>
> (Mercurial is written in Python, so it has different constraints.)
>

Change svn_opt_subcommand_desc2_t.help from 'const char *' to 'const
char **' maybe break the ABI,
I don't think we can do it in the near future.

Change print_command_info2 have the similarly issues, and it's more
complex to generate and store strings for translating.

I have another approach, introduce a new function to concatenate many
strings, e.g.
const char * svn_opt__string_concat(apr_pool_t *pool, const char *s1, ...)

The last parameter should be NULL to indicate this is the last
parameter, then the very long messages:
N_("para1" "para2" "para3" "..."  "paraN")

Can be changed to:
svn_opt__string_concat(N_("para1"), (N_("para2"), (N_("para3"), ...,
(N_("paraN"), NULL)

Why I recall this thread ? Because when I translating today, I found a
message which have 255 lines (svn/main.c:676 ),
It's supper terrible ! I can not image the maintenance work when there
have one line changed only!

Any comments?

--
Dongsheng Song

Re: [PATCH] New XFail test for the issue 3781

[Prabhu Gnana Sundar]

Hi Daniel,

Thank you for the comments... :)

On Friday 04 February 2011 08:12 AM, Daniel Shahaf wrote:

Prabhu Gnana Sundar wrote on Thu, Feb 03, 2011 at 16:53:54 +0530:

Hi all,

Currently, as per the issue 3781, "checkout" reads the authz file in
*case insensitive* way, whereas "commit" reads the authz file in *case
sensitive* way.

Here is what is observed:
1. Checkout is *case insensitive* about the repository name section and
also the path-inside-repo section.
2. Commit is *case sensitive* about the repository name section *but
case insensitive* about the path-inside-repo section.

Attached an XFail testcase and the log message with this mail. Please
share your views.



You didn't actually say what the intended behaviour is.



The intended behaviour is path based authorization should be case 
sensitive for all cases.

But it shows some inconsistencies currently...

It has been discussed in ,

http://mail-archives.apache.org/mod_mbox/subversion-dev/201101.mbox/%3c4d468561.3070...@collab.net%3E


Index: subversion/tests/cmdline/authz_tests.py
===
--- subversion/tests/cmdline/authz_tests.py (revision 1066732)
+++ subversion/tests/cmdline/authz_tests.py (working copy)
@@ -1061,6 +1061,67 @@
   [], 'ls', '-R',
   sbox.repo_url)

+
+def case_insensitive_authz(sbox):
+  "authz issue #3781, check case insensitiveness"
+
+  sbox.build()
+
+  # test the case-insensitivity of the path inside the repo
+  write_authz_file(sbox, {"/": "jrandom = r", "/A": "jrandom = r", "/a/Mu": 
"jrandom = rw"})
+
+  write_restrictive_svnserve_conf(sbox.repo_dir)
+
+  wc_dir = sbox.wc_dir
+
+  mu_path = os.path.join(wc_dir, 'A', 'mu')
+  mu_url = sbox.repo_url + '/A/mu'
+  svntest.main.file_append(mu_path, "hi")
+
+  # Create expected output tree.
+  expected_output = svntest.wc.State(wc_dir, {
+'A/mu' : Item(verb='Sending'),
+})
+
+  # Commit the file.
+  svntest.actions.run_and_verify_commit(wc_dir,
+expected_output,
+None,
+None,
+mu_path)

Hold on.  Why does this work?  You commit /A/mu and the authz file gives
you only 'r' access to /A!


It works because we have set 'rw' for the /a/Mu file.
And this must fail actually, because of the case-change in the path. 
Tweaked the test-case.

+  mixed_case_repo_dir = mixcases(os.path.basename(sbox.repo_dir))
+
+  # test the case-insensitivity of the repo name
+  write_authz_file(sbox, {}, sections = {mixed_case_repo_dir + ":/": "jrandom = 
r",
+ mixed_case_repo_dir + ":/A": "jrandom = 
r",
+ mixed_case_repo_dir + ":/A/mu": "jrandom = 
rw"})
+

Just for clarity, could you add sections for the correct-case repository
name to the authz file?


Sure :) added for clarity...

+  svntest.main.file_append(mu_path, "hi")
+  # Commit the file again.
+  svntest.actions.run_and_verify_commit(wc_dir,
+expected_output,
+None,
+None,
+mu_path)
+  svntest.actions.run_and_verify_svn2('No error',
+  svntest.verify.AnyOutput, [],
+  0, 'cat', mu_url)

Could you have here both a commit that fails and a commit that succeeds?

Added :)


I have attached the tweaked correct patch and the log message with this 
mail. Please share your views.




Thanks and regards
Prabhu
Index: subversion/tests/cmdline/authz_tests.py
===
--- subversion/tests/cmdline/authz_tests.py	(revision 1067090)
+++ subversion/tests/cmdline/authz_tests.py	(working copy)
@@ -1060,6 +1060,84 @@
  [], 'ls', '-R',
  sbox.repo_url)
 
+def case_insensitive_authz(sbox):
+  "authz issue #3781, check case insensitiveness"
+
+  sbox.build()
+
+  # test the case-insensitivity of the path inside the repo
+  write_authz_file(sbox, {"/": "jrandom = r", "/A": "jrandom = r", "/a/Mu": "jrandom = rw"})
+
+  write_restrictive_svnserve_conf(sbox.repo_dir)
+
+  wc_dir = sbox.wc_dir
+
+  mu_path = os.path.join(wc_dir, 'A', 'mu')
+  mu_url = sbox.repo_url + '/A/mu'
+  svntest.main.file_append(mu_path, "hi")
+
+  # Create expected output tree.
+  expected_output = svntest.wc.State(wc_dir, {
+'A/mu' : Item(verb='Sending'),
+})
+
+  expected_error = "Commit failed"
+
+  # Commit the file.
+  svntest.actions.run_and_verify_commit(wc_dir,
+None,
+None,
+expected_error,
+

Re: diff4-optimization-bytes

[Daniel Shahaf]

Johan Corveleyn wrote on Mon, Jan 31, 2011 at 02:47:50 +0100:
> On Fri, Jan 28, 2011 at 7:58 PM, Daniel Shahaf  
> wrote:
> > Johan Corveleyn wrote on Fri, Jan 28, 2011 at 14:04:07 +0100:
> >> On Fri, Jan 28, 2011 at 9:29 AM, Daniel Shahaf  
> >> wrote:
> >> > May I suggest that, if this code is to be released, then you validate
> >> > its correctnss?  For example, a minimal regression test that is written
> >> > to record trunk's pre-branch behaviour might be sufficient.
> >>
> >> ... I'll accept your suggestion. I'll try to take some time for that
> >> this weekend. If anyone beats me to it, that would be fine as well of
> >> course :).
> >>
> 
> Sorry, didn't get around to it yet. Maybe sometime during next week.
> 
> > Thanks :-).  I've looked into it, but it seems the output functions in
> > svn_diff.h are oriented to diff2/diff3 only (they don't have an
> > 'ancestor' enum or parameters); and in a quick test, I couldn't get
> > tools/diff/diff4 to output anything sensible:
> >
> > % for i in 1 2 3 4; do echo $i>$i; done
> > % ./tools/diff/diff4 1 2 3 4
> > 3
> > %
> 
> I think it's more or less sensible, although I'm not completely sure.
> If you look at the "usage" of diff4, you see:
> 
> Usage: /path/to/diff4
> 
> I think what you did is: take the difference between 2 and 3 (older
> and yours), and apply that to 1 (mine), using 4 as the common
> ancestor. Apparently that yields "3" :-).
> 
> Compare that with the use of diff3, with the files 1, 2 and 3:
> 
> Usage: /path/to/diff3   
> 
> $ diff3 1 2 3
> <<< 1
> 1
> ===
> 3
> >>> 3
> 
> Hmmm, that looks like a merge conflict, which seems logical (trying to
> apply the diff between 2 and 3, to the file 1 which doesn't contain a
> line '2').
> 

Agreed.

> So now I also don't really understand why diff4 successfully merges
> it, given the ancestor 4. Maybe the reasoning is as follows, given
> that 4 is the common ancestor:
> 
> - 1 has evolved out of 4 (so '1' is the mine's replacement for '4')
> 
> - 2 has evolved out of 4 (so in the merge source, '2' is the
> replacement for '4')
> 
> - So with "variance-adjusted patching", the diff between 2 and 3
> translates into: "replace whatever '4' was transformed into (in this
> case '1') into '3'". That would yield '3'.
> 
> Not sure though.
> 
> > How can we test diff4 then?  Do we have to add public diff4 APIs in
> > order to be able to test svn_diff_diff4_2()?
> 
> Maybe we should come up with a better example. In the meantime, I'm
> trying to understand diff4 (reading kfogel's article at [1], as
> referenced in diff4.c).
> 

That article was very helpful --- thanks for the pointer!

> > Daniel
> > (on the one hand I'd much prefer to test the API changes before
> > releasing them; on the other hand, adding API's related to an API
> > that no one (to our knowledge) uses seems odd)
> 
> Yes. I think we should give it some effort to come up with some
> minimal testing. But if it takes too long, we should probably not let
> this be blocking
> 

I've went ahead and made a patch out of the second example in that
article.  However, I admit that I got it to work by fiddling with the
order of the four parameters until the test passed :-)

Could you have a look? (attached)

Thanks,

Daniel

> Cheers,
> -- 
> Johan
> 
> [1] 
> http://svn.apache.org/repos/asf/subversion/trunk/notes/variance-adjusted-patching.html
Index: subversion/libsvn_subr/stream.c
===
--- subversion/libsvn_subr/stream.c	(revision 1065108)
+++ subversion/libsvn_subr/stream.c	(working copy)
@@ -1373,6 +1373,22 @@ svn_stream_for_stdout(svn_stream_t **out, apr_pool
 
 
 svn_error_t *
+svn_stream_for_stderr(svn_stream_t **err, apr_pool_t *pool)
+{
+  apr_file_t *stderr_file;
+  apr_status_t apr_err;
+
+  apr_err = apr_file_open_stderr(&stderr_file, pool);
+  if (apr_err)
+return svn_error_wrap_apr(apr_err, "Can't open stderr");
+
+  *err = svn_stream_from_aprfile2(stderr_file, TRUE, pool);
+
+  return SVN_NO_ERROR;
+}
+
+
+svn_error_t *
 svn_string_from_stream(svn_string_t **result,
svn_stream_t *stream,
apr_pool_t *result_pool,
Index: subversion/tests/libsvn_diff/diff-diff3-test.c
===
--- subversion/tests/libsvn_diff/diff-diff3-test.c	(revision 1065108)
+++ subversion/tests/libsvn_diff/diff-diff3-test.c	(working copy)
@@ -2050,6 +2050,41 @@ test_three_way_merge_conflict_styles(apr_pool_t *p
 
 
 static svn_error_t *
+test_diff4(apr_pool_t *pool)
+{
+  svn_diff_t *diff;
+  svn_stream_t *actual, *expected;
+  svn_boolean_t same;
+  svn_stream_t *err;
+
+  /* Usage: tools/diff/diff4 */
+  /* tools/diff/diff4 B2 T2 T3 T1 > B2new */
+  SVN_ERR(svn_diff_file_diff4(&diff, "T2", "B2", "T3", "T1", pool));
+
+  /* Sanity. */
+  SVN_TEST_ASSERT(! svn_diff_contains_conflicts(diff));
+  SVN_TEST_ASSERT(svn_diff_contains_diffs(diff));
+
+  /* Comparison. 

Re: svn commit: r989017 - /subversion/trunk/subversion/svn/main.c

[Dongsheng Song]

On Wed, Aug 25, 2010 at 19:26,   wrote:
> Author: julianfoad
> Date: Wed Aug 25 11:26:31 2010
> New Revision: 989017
>
> URL: http://svn.apache.org/viewvc?rev=989017&view=rev
> Log:
> * subversion/svn/main.c
>  (svn_cl__cmd_table): Mention that --keep-local implies --force in
>    the delete command.
>
> Modified:
>    subversion/trunk/subversion/svn/main.c
>
> Modified: subversion/trunk/subversion/svn/main.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/main.c?rev=989017&r1=989016&r2=989017&view=diff
> ==
> --- subversion/trunk/subversion/svn/main.c (original)
> +++ subversion/trunk/subversion/svn/main.c Wed Aug 25 11:26:31 2010
> @@ -543,7 +543,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
>      "    committed, are immediately removed from the working copy\n"
>      "    unless the --keep-local option is given.\n"
>      "    PATHs that are, or contain, unversioned or modified items will\n"
> -     "    not be removed unless the --force option is given.\n"
> +     "    not be removed unless the --force or --keep-local option is 
> given.\n"
>      "\n"
>      "  2. Each item specified by a URL is deleted from the repository\n"
>      "    via an immediate commit.\n"),
>

Is this still true ?

It's crazy, because I do not expected these PATHs will be deleted when
I give --keep-local option !

--
Dongsheng Song

Re: [PATCH] New XFail test for the issue 3781

[Daniel Shahaf]

Prabhu Gnana Sundar wrote on Thu, Feb 03, 2011 at 16:53:54 +0530:
> Hi all,
>
> Currently, as per the issue 3781, "checkout" reads the authz file in  
> *case insensitive* way, whereas "commit" reads the authz file in *case  
> sensitive* way.
>
> Here is what is observed:
> 1. Checkout is *case insensitive* about the repository name section and  
> also the path-inside-repo section.
> 2. Commit is *case sensitive* about the repository name section *but  
> case insensitive* about the path-inside-repo section.
>
> Attached an XFail testcase and the log message with this mail. Please  
> share your views.
>
>

You didn't actually say what the intended behaviour is.

> [[[
> XFail testcase for the issue 3781

Thanks for writing the XFail test first.

> Index: subversion/tests/cmdline/authz_tests.py
> ===
> --- subversion/tests/cmdline/authz_tests.py   (revision 1066732)
> +++ subversion/tests/cmdline/authz_tests.py   (working copy)
> @@ -1061,6 +1061,67 @@
>   [], 'ls', '-R',
>   sbox.repo_url)
>  
> +
> +def case_insensitive_authz(sbox):
> +  "authz issue #3781, check case insensitiveness"
> +
> +  sbox.build()
> +
> +  # test the case-insensitivity of the path inside the repo
> +  write_authz_file(sbox, {"/": "jrandom = r", "/A": "jrandom = r", "/a/Mu": 
> "jrandom = rw"})
> +
> +  write_restrictive_svnserve_conf(sbox.repo_dir)
> +
> +  wc_dir = sbox.wc_dir
> +
> +  mu_path = os.path.join(wc_dir, 'A', 'mu')
> +  mu_url = sbox.repo_url + '/A/mu'
> +  svntest.main.file_append(mu_path, "hi")
> +
> +  # Create expected output tree.
> +  expected_output = svntest.wc.State(wc_dir, {
> +'A/mu' : Item(verb='Sending'),
> +})
> +
> +  # Commit the file.
> +  svntest.actions.run_and_verify_commit(wc_dir,
> +expected_output,
> +None,
> +None,
> +mu_path)

Hold on.  Why does this work?  You commit /A/mu and the authz file gives
you only 'r' access to /A!

> +  svntest.actions.run_and_verify_svn2('No error',
> +  svntest.verify.AnyOutput, [],
> +  0, 'cat', mu_url)
> +
> +  def mixcases(repo_name):
> +mixed_repo_name = ''
> +for i in range(0, len(repo_name)):
> +  if i % 2 == 0:
> +mixed_val = repo_name[i].upper()
> +mixed_repo_name = mixed_repo_name + mixed_val
> +  else:
> +mixed_val = repo_name[i].lower()
> +mixed_repo_name = mixed_repo_name + mixed_val
> +return mixed_repo_name 
> +

Nice, but why not just pass NAME to sbox.build() and then hard-code
'bar' and 'baR' in the test?

> +  mixed_case_repo_dir = mixcases(os.path.basename(sbox.repo_dir))
> +
> +  # test the case-insensitivity of the repo name
> +  write_authz_file(sbox, {}, sections = {mixed_case_repo_dir + ":/": 
> "jrandom = r",
> + mixed_case_repo_dir + ":/A": 
> "jrandom = r",
> + mixed_case_repo_dir + ":/A/mu": 
> "jrandom = rw"})
> +

Just for clarity, could you add sections for the correct-case repository
name to the authz file?

> +  svntest.main.file_append(mu_path, "hi")
> +  # Commit the file again.
> +  svntest.actions.run_and_verify_commit(wc_dir,
> +expected_output,
> +None,
> +None,
> +mu_path)
> +  svntest.actions.run_and_verify_svn2('No error',
> +  svntest.verify.AnyOutput, [],
> +  0, 'cat', mu_url) 

Could you have here both a commit that fails and a commit that succeeds?

Re: [PATCH] - Fix for issue #3792

[Daniel Shahaf]

Noorul Islam K M wrote on Thu, Feb 03, 2011 at 14:15:48 +0530:
> 
> Attached is the patch to fix issue #3792. Please review and let me know
> your comments.
> 
> Log
> [[[
> 
> Fix for issue #3792. Make svn info to display information for
> excluded items.
> 

s/to//

> * subversion/include/private/svn_wc_private.h,
>   subversion/libsvn_wc/node.c
>   (svn_wc__node_depth_is_exclude): New helper function to find
> whether the node is set with depth 'exclude'.
> 
> * subversion/libsvn_client/info.c
>   (crawl_entries): Build minimal information if the node has
> depth set as 'exclude'.
> 
> * subversion/svn/info-cmd.c
>   (print_info): Print depth as 'exclude' for nodes having depth
> exclude. Hide node kind while printing exclude infomation.
> 

It's okay to print the node kind if it's known.  (and it probably is)

> * subversion/tests/cmdline/depth_tests.py
>   (test_list): Remove XFail marker from info_excluded test.
> 
> Patch by: Noorul Islam K M 
> ]]]
> 

> Index: subversion/tests/cmdline/depth_tests.py
> ===
> --- subversion/tests/cmdline/depth_tests.py   (revision 1066732)
> +++ subversion/tests/cmdline/depth_tests.py   (working copy)
> @@ -2841,7 +2841,7 @@
>excluded_path_misc_operation,
>excluded_receive_remote_removal,
>exclude_keeps_hidden_entries,
> -  XFail(info_excluded, issues=3792),
> +  info_excluded,
>tree_conflicts_resolved_depth_empty,
>tree_conflicts_resolved_depth_files,
>tree_conflicts_resolved_depth_immediates,
> Index: subversion/svn/info-cmd.c
> ===
> --- subversion/svn/info-cmd.c (revision 1066732)
> +++ subversion/svn/info-cmd.c (working copy)
> @@ -291,7 +291,8 @@
>break;
>  
>  case svn_node_none:
> -  SVN_ERR(svn_cmdline_printf(pool, _("Node Kind: none\n")));
> +  if (info->depth != svn_depth_exclude)
> +SVN_ERR(svn_cmdline_printf(pool, _("Node Kind: none\n")));

That doesn't look right.  Does the DB know the node kind of an excluded
node?  If so, it should print it; if not, it should return "unknown".

>break;
>  
>  case svn_node_unknown:
> @@ -363,6 +364,9 @@
>  SVN_ERR(svn_cmdline_printf(pool, _("Copied From Rev: %ld\n"),
> info->copyfrom_rev));
>  }
> +  
> +  if (info->depth == svn_depth_exclude)
> +SVN_ERR(svn_cmdline_printf(pool, _("Depth: exclude\n")));
>  

I know that there is already some other place in the code that prints
the Depth: line, why didn't you reuse that place?

>if (info->last_changed_author)
>  SVN_ERR(svn_cmdline_printf(pool, _("Last Changed Author: %s\n"),
> Index: subversion/include/private/svn_wc_private.h
> ===
> --- subversion/include/private/svn_wc_private.h   (revision 1066732)
> +++ subversion/include/private/svn_wc_private.h   (working copy)
> @@ -759,6 +759,15 @@
>   apr_pool_t *result_pool,
>   apr_pool_t *scratch_pool);
>  
> +/**
> + * Find whether @a local_abspath is set with depth exclude using @a wc_ctx. 
> + */
> +svn_error_t *
> +svn_wc__node_depth_is_exclude(svn_boolean_t *exclude,
> +  svn_wc_context_t *wc_ctx,
> +  const char *local_abspath,
> +  apr_pool_t *scratch_pool);
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> Index: subversion/libsvn_wc/node.c
> ===
> --- subversion/libsvn_wc/node.c   (revision 1066732)
> +++ subversion/libsvn_wc/node.c   (working copy)
> @@ -247,6 +247,30 @@
>  }
>  
>  svn_error_t *
> +svn_wc__node_depth_is_exclude(svn_boolean_t *exclude,
> +  svn_wc_context_t *wc_ctx,
> +  const char *local_abspath,
> +  apr_pool_t *scratch_pool)
> +{
> +  svn_wc__db_status_t status;
> +  svn_error_t *err;
> +  
> +  *exclude = FALSE;
> +
> +  err = svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL,
> + wc_ctx->db, local_abspath, scratch_pool,
> + scratch_pool);
> +
> +  if ((! err) && (status == svn_wc__db_status_excluded))
> +*exclude = TRUE;
> +
> +  return svn_error_return(err);

Haven't reviewed this part, but I'll have to do so before committing.

> +}
> +
> +svn_error_t *
>  svn_wc__node_get_depth(svn_depth_t *depth,
> svn_wc_context_t *wc_ctx,
> const char *local_abspath,
>

Re: SQLite and callbacks

[Daniel Shahaf]

(test script attached --- hope it goes through)

Stefan Sperling wrote on Wed, Feb 02, 2011 at 18:53:39 +0100:
> I've made svn proplist issue per-directory queries in r1066541.
> Reviews of this change are most welcome.
> Also, if someone has a nice way of testing performance impacts of
> this change it would be interesting to see the results.
> I have not done any benchmarking myself yet.
> 

Working copy of /tags/1.6.*, no local mods, r1066541:
[[[
==> pristine-t1-1.out <==
233.55user 78.55system 5:12.08elapsed 100%CPU (0avgtext+0avgdata 
32176maxresident)k
0inputs+0outputs (0major+6199minor)pagefaults 0swaps

==> pristine-t1-2.out <==
237.68user 79.22system 5:16.88elapsed 100%CPU (0avgtext+0avgdata 
32176maxresident)k
0inputs+0outputs (0major+6200minor)pagefaults 0swaps

==> pristine-t1-3.out <==
232.96user 76.04system 5:08.98elapsed 100%CPU (0avgtext+0avgdata 
32176maxresident)k
0inputs+0outputs (0major+6197minor)pagefaults 0swaps
]]]

Ditto, r1066540:
[[[
==> pristine-t2-1.out <==
253.41user 82.90system 5:36.27elapsed 100%CPU (0avgtext+0avgdata 
30480maxresident)k
0inputs+0outputs (0major+6099minor)pagefaults 0swaps

==> pristine-t2-2.out <==
252.31user 82.52system 5:34.81elapsed 100%CPU (0avgtext+0avgdata 
30480maxresident)k
0inputs+0outputs (0major+6090minor)pagefaults 0swaps

==> pristine-t2-3.out <==
234.95user 75.25system 5:10.15elapsed 100%CPU (0avgtext+0avgdata 
30480maxresident)k
0inputs+0outputs (0major+6092minor)pagefaults 0swaps
]]]

So, 5% more memory, but (excluding the t2-3 result) post-change is 7%
faster than post-change.


Ditto, with local prop mods on all *.c/*.py files:
[[[
==> pset-Rpyc-t1-1.out <==
219.41user 67.76system 4:47.13elapsed 100%CPU (0avgtext+0avgdata 
32192maxresident)k
0inputs+0outputs (0major+6200minor)pagefaults 0swaps

==> pset-Rpyc-t1-2.out <==
218.48user 65.74system 4:44.18elapsed 100%CPU (0avgtext+0avgdata 
32192maxresident)k
0inputs+0outputs (0major+6205minor)pagefaults 0swaps

==> pset-Rpyc-t1-3.out <==
219.14user 67.83system 4:46.94elapsed 100%CPU (0avgtext+0avgdata 
32192maxresident)k
0inputs+0outputs (0major+6197minor)pagefaults 0swaps
]]]

Ditto, r1066540:
[[[
==> pset-Rpyc-t2-1.out <==
217.34user 64.90system 4:42.20elapsed 100%CPU (0avgtext+0avgdata 
30480maxresident)k
0inputs+0outputs (0major+6097minor)pagefaults 0swaps

==> pset-Rpyc-t2-2.out <==
215.01user 67.10system 4:42.08elapsed 100%CPU (0avgtext+0avgdata 
30464maxresident)k
0inputs+0outputs (0major+6097minor)pagefaults 0swaps

==> pset-Rpyc-t2-3.out <==
212.82user 63.68system 4:36.46elapsed 100%CPU (0avgtext+0avgdata 
30480maxresident)k
0inputs+0outputs (0major+6094minor)pagefaults 0swaps
]]]

Pre-change is faster.


> Stefan

Daniel
(with thanks to svn-qavm)
#!/bin/sh -ex

dirs="$HOME/src/svn/t1 $HOME/src/svn/t2"
wc=$1
nick=$2
[ "$#" -gt 2 ] && exit 1

for i in $dirs; do
  svn=$i/subversion/svn/svn
  $svn pl -qvR $wc >/dev/null
  for j in 1 2 3; do
f=$nick-`basename $i`-$j.out
rm -f $f
time $svn pl -qvR $wc >/dev/null 2>$f
  done
done

Re: Windows Makefile (was: Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir))

[Daniel Shahaf]

Johan Corveleyn wrote on Thu, Feb 03, 2011 at 22:47:23 +0100:
> On Wed, Feb 2, 2011 at 5:53 AM, Daniel Shahaf  wrote:
> > Johan Corveleyn wrote on Tue, Feb 01, 2011 at 13:28:24 +0100:
> >> So: I've tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py
> >> (put it in comment, ran "nmake config" and rebuilt everything), then
> >> ran update_tests.py again: same result. It still crashes, and shows
> >> the ugly blocking popup.
> >>
> >
> > I don't use Windows as my day-to-day OS any more, but I suppose it'll be
> > a good idea to adhere to my old promise to commit that Makefile once it
> > has seen some testing :-).  Johan, could you do that?  Or send it to me
> > and I'll do it.
> 
> Hi Daniel,
> 
> Yes, great idea. This could definitely help get other people working
> on Windows interested in svn development (it helped me getting
> started). Here is the Makefile I'm currently using, but I don't think
> it's ready to be committed right away, with my local paths in it and
> all, and lots of custom tweaks to make it work for my needs.
> 

r1067056, and I added some TODO's in README.  We can handle them on an
as-needed basis.

I added some tweaks --- to avoid requiring a 'test.exe' binary, etc ---
I guess you'll see them when you diff the files.  (I also diffed your
Makefile against the one I'd mailed the list a while ago.)

I also reverted some of your 'don't require version number for
dependencies' changes... but thinking again I think that might not have
a good move.  So feel free to add them back!

> If I have some time, I'll try to clean it up a little. But right now,
> attaching it to this mail is all I can do :-).
> 
> Some changes I made to the original you sent me (from memory):
> - Fiddled around with the dependencies, until they worked for me.
> - Added Programs\diff, Programs\diff3 and Programs\diff4 to the "all1"
> and "all2" targets, because I needed them (for the diff-opitimizations
> testing). Added corresponding copy statement in the package target.
> 

On Unix, 'make' now implies 'make tools' (as of a couple of months ago).
We should do the same on Windows.

> Maybe other things, but I don't remember (right now, I'm just copying
> this Makefile around if I need to build a new branch that I checked
> out (only need to change the paths in the beginning of the file)).
> 
> Cheers,
> Johan
> 

Thanks!

Daniel

> > Belatedly,
> >
> > Daniel
> > (as sibling to stsp's tools/dev/unix-build/ I suggest)
> >
> >> Anyone else who recently built trunk on Windows seeing this, when
> >> running update_tests.py?
> >>
> >> --
> >> Johan
> >

Re: Conflict storage in 1.7

[Paul Burba]

On Mon, Dec 13, 2010 at 10:41 AM, Hyrum K. Wright
 wrote:
> [Apologies to Bert if my earlier comments were a bit pointed.]
>
> On Sat, Dec 11, 2010 at 8:33 AM, Stefan Sperling  wrote:
>> On Fri, Dec 10, 2010 at 01:14:25PM -0800, Hyrum K. Wright wrote:
>>> On Fri, Dec 10, 2010 at 12:07 PM, Bert Huijben  wrote:
>>> >
>>> >
>>> >> -Original Message-
>>> >> From: hy...@hyrumwright.org [mailto:hy...@hyrumwright.org] On Behalf Of
>>> >> Hyrum K. Wright
>>> >> Sent: vrijdag 10 december 2010 17:11
>>> >> To: Subversion Development
>>> >> Subject: Re: Conflict storage in 1.7
>>> >>
>>> >> Hearing no objections, I'm going to mark this as DONE.
>>> >
>>> > I don't think we can properly mark unversioned obstructions with just the
>>> > current datastore. We need that storage to allow an update to complete
>>> > regardless of obstructions.
>>> > (And we can't keep the original behavior of 1.6 of unversioned 
>>> > obstructions
>>> > as an unversioned directory obstructing a different directory wouldn’t be
>>> > noticed with a single db).
>>>
>>> Why can't we keep the original behavior of 1.6?  Is there a behavior
>>> regression (in which case, it wouldn't be the original behavior of
>>> 1.6)?  Could you elaborate on the case you mention about?  Or better
>>> yet, author a test case to illustrate it?
>>
>> Some observations:
>>
>> In 1.7, you get a tree conflict if a directory obstructs an addition
>> of another directory during update:
>> $ svn up
>> Updating '.' ...
>>   C foo
>> At revision 3.
>> Summary of conflicts:
>>  Tree conflicts: 1
>> $ svn st
>> ?     C foo
>>      >   local unversioned, incoming add upon update
>> Summary of conflicts:
>>  Tree conflicts: 1
>>
>> IIRC we once decided that obstructions should not cause tree conflicts.
>> Only versioned item should cause tree conflicts, and obstructions should
>> be skipped. Apparently, this decision has not been enforced consistently
>> throughout the code since we still have an "unversioned" tree conflict 
>> reason.
>>
>> 1.6 behaved as follows:
>> $ svn up
>> subversion/libsvn_wc/update_editor.c:2335: (apr_err=155000)
>> svn: Failed to add directory 'foo': an unversioned directory of the same 
>> name already exists
>>
>> I like the current 1.7 behaviour better than the 1.6 behaviour because the
>> update isn't instantly aborted when an obstructing directory is found.
>> But maybe 1.7 should skip adding the directory instead of flagging a
>> conflict? I don't really care, except that the behaviour in obstruction
>> cases should be consistent (either skip, or flag conflict).
>
> Agreed.  Do we have a matrix somewhere of the various combinations of
> state and what the current and/or expected behaviors should be?  Such
> a document would make testing, as well as ensuring, consistency much
> easier.
>
>>
>> Another scenario:
>> Update brings in a change for a file within a directory which has been
>> replaced without telling svn about this (rm -rf dir; mkdir dir):
>>
>> 1.7 restores missing files inside the directory:
>> $ svn up
>> Updating '.' ...
>> Restored 'epsilon/zeta'
>> U    epsilon/zeta
>> Updated to revision 3.
>>
>> 1.6 errors out like this:
>> $ svn up
>> subversion/libsvn_wc/lock.c:947: (apr_err=155005)
>> svn: Directory 'epsilon/.svn' containing working copy admin area is missing
>>
>> I suppose Bert would like 1.7 to error out (or skip the directory, or
>> flag a tree conflict), instead of assuming the directory was versioned.
>> FWIW, mercurial (another "single-db" system) behaves like 1.7 in this case.
>> So the current behaviour isn't necessarily bad or something people won't
>> expect. Then again, unlike Subversion, mercurial and git explicitly don't
>> treat directories as versioned objects.
>> I suppose the most important question here is whether we can change this
>> behaviour later without breaking backwards compat if we released 1.7 as is.
>>
>>> > I would really like to see more of the enhanced conflict storage model in
>>> > 1.7.
>>>
>>> Will you then commit to implementing this behavior in a timely manner,
>>> so as to not further delay 1.7?
>>>
>>> > And until we get the code to a proper atomic behavior in the more common
>>> > code paths (which I would call a show stopper) this can be implemented in
>>> > parallel just fine.
>>> >
>>> >
>>> > And I don't understand how you call something that hasn't been implemented
>>> > 'DONE'. Maybe 'postponed' or something like that, but certainly not 
>>> > 'done'.
>>>
>>> The bits about moving conflict information to the victim is done.  Of
>>> course there is always future work to be done, but my intent was to
>>> call the bits required for 1.7 as DONE (particularly on roadmap.html).
>>
>> Just to make sure I understand your point: Moving TC data to the victim
>> node should address the upgrade problem Bert described in
>> http://svn.haxx.se/dev/archive-2010-08/0554.shtml, right?
>> Is this what you are talking about?
>
> Yes.  Since we've put conflict information on the victim no

Re: svn commit: r1067014 - /subversion/trunk/CHANGES

[Hyrum K Wright]

FYI: I plan on going through the rest of the 1.7.0 issues tomorrow,
and adding their content to CHANGES.

A number of these are bugs that we fixed by wc-ng, but since they've
been long-standing bugs, and not issues which were reported *because*
of wc-ng, I decided to go ahead and include them, on the chance that
people are interested in a particular bug by issue number.  I didn't
do the same with other features, like 'svn patch'.

-Hyrum

On Thu, Feb 3, 2011 at 4:42 PM,   wrote:
> Author: hwright
> Date: Thu Feb  3 22:42:15 2011
> New Revision: 1067014
>
> URL: http://svn.apache.org/viewvc?rev=1067014&view=rev
> Log:
> Start populating the 1.7.0 CHANGES entry.  This commit adds entries for issues
> with the 1.7.0 milestone older than 3300.
>
> Note: Not all issues with the milestone are included, and folks should please
> add, edit, remove, and otherwise modify these items.
>
>  * CHANGES:
>   (1.7.0): Add issues < 3300.
>
> Modified:
>    subversion/trunk/CHANGES
>
> Modified: subversion/trunk/CHANGES
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/CHANGES?rev=1067014&r1=1067013&r2=1067014&view=diff
> ==
> --- subversion/trunk/CHANGES (original)
> +++ subversion/trunk/CHANGES Thu Feb  3 22:42:15 2011
> @@ -6,21 +6,65 @@ http://svn.apache.org/repos/asf/subversi
>   - General:
>
>   - Major new features:
> +    * Less verbose HTTP-based repository access protocol (issue #1161, #3371)
> +    * Rewritten working copy metadata storage (issue #3357)
> +    * 'svn patch' and git extensions for 'svn diff'
>
>   - Minor new features and improvements:
> +    * Better handling of HTTP redirects (issue #2779)
> +    * Improved and much more consistent path handling (issue #2028, and 
> others)
> +    * 'svnadmin load' rewrites changed revnums in mergeinfo (issue #3020)
> +    * Error message improvements
>
>   - Client-side bugfixes:
> +    * 'svn cp A B; svn mv B C' is equavalient to 'svn cp A C' (issue #756)
> +    * revert fetches missing directories from the server (issue #1040)
> +    * allow subdirs of moved dirs to be moved and committed (issue #1259)
> +    * improved performance of 'svn mv' with whole directories (issue #1284)
> +    * 'svn rm B; svn cp A B' now works (issue #1516)
> +    * 'svn diff URL1 URL2' now reverse of 'svn diff URL2 URL1) (issue #2333)
> +    * error if relocating to an unused URL (issue #2531)
> +    * 'svn blame -rWORKING' is now supported (issue #2544)
> +    * improve correctness of commit on a relocated wc over ra_dav (issue 
> #2578)
> +    * add early error to 'svn add --auto-props' with mixed eols (issue #2713)
> +    * allow 'svn diff' to accept symlinks as targets (issue #2716)
> +    * don't lose props for replaced items (issue #2743)
> +    * handle mergeinfo for subtrees removed outside of svn (issue #2915)
> +    * add ability to force 'svn diff' to use internal diff (issue #3701)
> +    * correctly recover a schedule-for-delete rm'd outside of svn (issue 
> #3106)
> +    * don't create self-referential mergeinfo from own history (issue #3157)
> +    * improve 'svn log -g' handling of bad mergeinfo source paths (issue 
> #3270)
> +    * fixed: wc-to-wc copy of a switch source (issue #1802)
> +    * fixed: 'svn st' reports symlinks as obstructed items (issue #2284)
> +    * fixed: 'cd e:\; svn up e:\' fails (issue #2556)
> +    * fixed: removing a dir scheduled for deletion corrupts wc (issue #2741)
> +    * fixed: 'svn cleanup' fails on obstructed paths (issue #2867)
> +    * fixed: case-only renames resulting from merges don't work (issue #3115)
> +    * fixed: 'svn mergeinfo' ignores peg rev for wc target (issue #3180)
> +    * fixed: unable to merge to wc of deleted branch (issue #3221)
>
>   - Server-side bugfixes:
> +    * mod_dav_svn is less strict about auto-merging for commits (issue #1704)
> +    * allow SVNListParentPath to be used with authz (issue #2753)
> +    * allow nav to repo list from repo top with SVNListParentPath (issue 
> #3159)
> +    * fixed: 'svnadmin hotcopy' does not duplicate symlinks (issue #2591)
> +    * fixed: post-revprop-change errors cancel commit (issue #2990)
> +    * fixed: mod_dav_svn runs pre-revprop-change hook twice (issue #3085)
> +    * fixed: mod_dav_svn doesn't return stderr to user on failure (issue 
> #3112)
>
> -  - Contributed tools improvements and bugfixes:
> +  - Other tool improvements and bugfixes:
> +    * svnsync now takes the '--config-option' argument (issue #2027)
>
>  Developer-visible changes:
>   - General:
>
>   - API changes:
> +    * don't crash svn_client_copy if ctx->log_msg_func is NULL (issue #3234)
> +    * fixed: svn_ra_local__get_file() leaks file descriptors (issue #3290)
>
>   - Bindings:
> +    * New JavaHL package: org.apache.subversion
> +    * Deprecate the SVNClientSynchronized class in JavaHL (issue #2755)
>
>
>  Version 1.6.15
>
>
>

Re: Windows Makefile (was: Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir))

[Daniel Shahaf]

Johan Corveleyn wrote on Thu, Feb 03, 2011 at 22:47:23 +0100:
> - Added Programs\diff, Programs\diff3 and Programs\diff4 to the "all1"
> and "all2" targets, because I needed them (for the diff-opitimizations
> testing).

stsp added 'make tools' to the default unix build, which builds these
too.  We should do the same for the Windows build if it's not already so.

> Added corresponding copy statement in the package target.

With respect to the rest: cool, thanks, one of us can clean it up and commit it.

Re: Windows Makefile (was: Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir))

[Johan Corveleyn]

On Wed, Feb 2, 2011 at 5:53 AM, Daniel Shahaf  wrote:
> Johan Corveleyn wrote on Tue, Feb 01, 2011 at 13:28:24 +0100:
>> So: I've tried removing SVN_USE_WIN32_CRASHHANDLER from gen_win.py
>> (put it in comment, ran "nmake config" and rebuilt everything), then
>> ran update_tests.py again: same result. It still crashes, and shows
>> the ugly blocking popup.
>>
>
> I don't use Windows as my day-to-day OS any more, but I suppose it'll be
> a good idea to adhere to my old promise to commit that Makefile once it
> has seen some testing :-).  Johan, could you do that?  Or send it to me
> and I'll do it.

Hi Daniel,

Yes, great idea. This could definitely help get other people working
on Windows interested in svn development (it helped me getting
started). Here is the Makefile I'm currently using, but I don't think
it's ready to be committed right away, with my local paths in it and
all, and lots of custom tweaks to make it work for my needs.

If I have some time, I'll try to clean it up a little. But right now,
attaching it to this mail is all I can do :-).

Some changes I made to the original you sent me (from memory):
- Fiddled around with the dependencies, until they worked for me.
- Added Programs\diff, Programs\diff3 and Programs\diff4 to the "all1"
and "all2" targets, because I needed them (for the diff-opitimizations
testing). Added corresponding copy statement in the package target.

Maybe other things, but I don't remember (right now, I'm just copying
this Makefile around if I need to build a new branch that I checked
out (only need to change the paths in the beginning of the file)).

Cheers,
Johan

> Belatedly,
>
> Daniel
> (as sibling to stsp's tools/dev/unix-build/ I suggest)
>
>> Anyone else who recently built trunk on Windows seeing this, when
>> running update_tests.py?
>>
>> --
>> Johan
>

Re: Assertion failure during update_tests.py 58 (XFAIL: update a nonexistent child of a copied dir)

[Johan Corveleyn]

On Wed, Feb 2, 2011 at 7:38 AM, Lieven Govaerts  wrote:
> Johan,
>
> On Tue, Feb 1, 2011 at 11:16 PM, Johan Corveleyn  wrote:
>>
>> On Tue, Feb 1, 2011 at 10:10 PM, Johan Corveleyn 
>> wrote:
>> > it continues until test nr 58, and then gives the popup.
>> >
>> > Hm, I'm confused. I guess I'm going to fire up my debugger and set a
>> > breakpoint or something to see what happens and why ...
>>
>> Ok, I'm starting to understand. If I do this with a Debug build, it
>> crashes without the popup. If I'm running the test with a Release
>> build, it gives the annoying popup, blocking the rest of the test
>> suite.
>>
>> Is it supposed to be that way? I guess I should just as well be able
>> to run the full test suite, unattended, with a Release build, no?
>
> The idea was that people who have Visual Studio installed might be more
> interested in seeing the code resulting in a crash in their debugger than as
> a textual stacktrace.
> This choice is hardcoded in
> subversion/libsvn_subr/win32_crashrpt.c:svn__unhandled_exception_filter
> (lines 723-724).
> I'm not against using SVN_DBG_STACKTRACES_TO_STDERR also as a flag to
> trigger this, or add (yet) another flag.

I don't understand. I'm not running it from inside a debugger.

I'm just running the tests from the commandline:

python win-tests.py --verbose --cleanup
--bin=C:\research\svn\client_build\svn_branch_diff-opt\dist\bin
--debug -f fsfs -t update_tests.py

If I run this with a Debug build, the crash in test 58 is no problem
(textual stacktrace in tests.log file).

However, if I run it with a Release build, that crash causes the Windows popup.

But there is no debugger involved in either case.

Anyway, I guess it would indeed help if SVN_DBG_STACKTRACES_TO_STDERR
would avoid the popup, both with Debug builds as with Release builds.
Or some other way of avoiding this problem ...

Cheers,
-- 
Johan

Re: Roadmap : 1.7 Release Status : Test Review : XFails

[Daniel Becroft]

Thanks, Daniel. This issue shouldn't block 1.7 - the problem existed in 1.6.x 
(and probably 1.5.x and 1.4.x as well).

Will hopefully get the fix patch finished when I get back to my machine.

Sent from my iPhone.

On 02/02/2011, at 14:21, Daniel Shahaf  wrote:

>> LISTING: merge_tests.py
>> 
> 
> You missed:
> 
> merge_tests.py:  
> XFail(SkipUnless(merge_change_to_file_with_executable,
> merge_tests.py-svntest.main.is_posix_os)),
> 
> which Daniel Becroft (CC'd) recently added, and has submitted a patch
> that seeks to fix the underlying problem.
> 
> Issue #3686, r1061020.
> 
> 
> Daniel: please look at Paul's email (to which I am replying) if you
> haven't already and consider yourself assigned to this XFail. :-)

Re: [PATCH] issue #3719 fix slow large checkouts on Windows

[Stefan Sperling]

On Thu, Feb 03, 2011 at 04:18:32PM +, Neil Bird wrote:
> 
>   This is a patch for discussion which I submitted to issue 3719
> (“Extremely slow checkout on Windows”).  I shan't repeat everything
> that's there;  essentially it fixes *really* slow checkouts to NTFS
> of directories with large numbers of files with properties.
> 
> 
>   I originally modified it slightly to address my second comment on
> that issue, namely that just using rand() will leave it with
> slightly less functionality than the previous 9 loop.
> 
>   However, that was relying upon my Linux man page telling me
> RAND_MAX was 32767;  it's actually 2147483647.  I think it's still
> smaller on Windows, though, so it would suffer that limitation.
> Should we include a small, custom int-max rand() just for this?
> 
>   Otherwise it might be a #if on RAND_MAX.
> 
> 
>   A supplementary question is:  should I worry about where rand() is
> included from?  It “just worked” on my Linux build, but I've not
> tried for Windows, etc.
> 
> 
>   Apologies if I haven't formatted all this correctly.
> 
> 
> [[[
> Fix issue #3719: prevent logarithmic slow down when checking out
> large directories onto Windows NTFS
> 
> * subversion/libsvn_subr/io.c
>   svn_io_open_unique_file3: don't call svn_io_open_uniquely_named(),
> but instead use a copy of that routine

Which routine, precisely?

> that uses rand() to get the
> next potential free number instead if simply incrementing,
> minimising clashes on repeated calls.

Have you taken a look at the implementation of svn_io_open_unique_file3()
in Subversion's trunk code? This has already been changed, and it looks
like it already solves the performance problem. It also has a special case
for windows -- see the temp_file_create() helper function.

I think it would be better to backport that code to 1.6.x, instead of
changing svn_io_open_unique_file3() for 1.6.x in some other way.

Stefan

[PATCH] issue #3719 fix slow large checkouts on Windows

[Neil Bird]

  This is a patch for discussion which I submitted to issue 3719 
(“Extremely slow checkout on Windows”).  I shan't repeat everything that's 
there;  essentially it fixes *really* slow checkouts to NTFS of directories 
with large numbers of files with properties.



  I originally modified it slightly to address my second comment on that 
issue, namely that just using rand() will leave it with slightly less 
functionality than the previous 9 loop.


  However, that was relying upon my Linux man page telling me RAND_MAX was 
32767;  it's actually 2147483647.  I think it's still smaller on Windows, 
though, so it would suffer that limitation.  Should we include a small, 
custom int-max rand() just for this?


  Otherwise it might be a #if on RAND_MAX.


  A supplementary question is:  should I worry about where rand() is 
included from?  It “just worked” on my Linux build, but I've not tried for 
Windows, etc.



  Apologies if I haven't formatted all this correctly.


[[[
Fix issue #3719: prevent logarithmic slow down when checking out large 
directories onto Windows NTFS


* subversion/libsvn_subr/io.c
  svn_io_open_unique_file3: don't call svn_io_open_uniquely_named(), but 
instead use a copy of that routine that uses rand() to get the next 
potential free number instead if simply incrementing, minimising clashes on 
repeated calls.


]]]

--
[neil@fnx ~]# rm -f .signature
[neil@fnx ~]# ls -l .signature
ls: .signature: No such file or directory
[neil@fnx ~]# exit
Index: subversion/libsvn_subr/io.c
===
--- subversion/libsvn_subr/io.c	(revision 1066367)
+++ subversion/libsvn_subr/io.c	(working copy)
@@ -466,11 +466,140 @@
  apr_pool_t *result_pool,
  apr_pool_t *scratch_pool)
 {
-  return svn_io_open_uniquely_named(file, temp_path,
-dirpath, "tempfile", ".tmp",
-delete_when, result_pool, scratch_pool);
+  const char *path;
+  unsigned int i;
+  struct temp_file_cleanup_s *baton = NULL;
+  char *filename = "tempfile";
+  char *suffix   = ".tmp";
+
+  SVN_ERR_ASSERT(file || temp_path);
+
+  if (dirpath == NULL)
+SVN_ERR(svn_io_temp_dir(&dirpath, scratch_pool));
+  if (filename == NULL)
+filename = "tempfile";
+  if (suffix == NULL)
+suffix = ".tmp";
+
+  path = svn_path_join(dirpath, filename, scratch_pool);
+
+  if (delete_when == svn_io_file_del_on_pool_cleanup)
+{
+  baton = apr_palloc(result_pool, sizeof(*baton));
+
+  baton->pool = result_pool;
+  baton->name = NULL;
+
+  /* Because cleanups are run LIFO, we need to make sure to register
+ our cleanup before the apr_file_close cleanup:
+
+ On Windows, you can't remove an open file.
+  */
+  apr_pool_cleanup_register(result_pool, baton,
+temp_file_plain_cleanup_handler,
+temp_file_child_cleanup_handler);
 }
 
+  for (i = 1; i <= RAND_MAX; i++)
+{
+  const char *unique_name;
+  const char *unique_name_apr;
+  apr_file_t *try_file;
+  apr_status_t apr_err;
+  apr_int32_t flag = (APR_READ | APR_WRITE | APR_CREATE | APR_EXCL
+  | APR_BUFFERED | APR_BINARY);
+
+  if (delete_when == svn_io_file_del_on_close)
+flag |= APR_DELONCLOSE;
+
+  /* Special case the first attempt -- if we can avoid having a
+ generated numeric portion at all, that's best.  So first we
+ try with just the suffix; then future tries add a number
+ before the suffix.  (A do-while loop could avoid the repeated
+ conditional, but it's not worth the clarity loss.) */
+  if (i == 1)
+unique_name = apr_psprintf(scratch_pool, "%s%s", path, suffix);
+  else
+  {
+/* Use rand instead of counting up from 1 to prevent logarithmic
+ * slowdown with large directory checkouts when continually looking
+ * for new temp files for the files' properties. */
+unsigned int ir = rand();
+unique_name = apr_psprintf(scratch_pool, "%s.%u%s", path, ir, suffix);
+  }
+
+  /* Hmmm.  Ideally, we would append to a native-encoding buf
+ before starting iteration, then convert back to UTF-8 for
+ return. But I suppose that would make the appending code
+ sensitive to i18n in a way it shouldn't be... Oh well. */
+  SVN_ERR(cstring_from_utf8(&unique_name_apr, unique_name, scratch_pool));
+
+  apr_err = file_open(&try_file, unique_name_apr, flag,
+  APR_OS_DEFAULT, FALSE, result_pool);
+
+  if (APR_STATUS_IS_EEXIST(apr_err))
+continue;
+  else if (apr_err)
+{
+  /* On Win32, CreateFile fails with an "Access Denied" error
+ code, rather than "File Already Exists", if the colliding
+ name belongs to a directory. */
+  if (APR_STATUS_IS_EACCES(

Re: NODES.properties == null for added node?

[Stefan Sperling]

On Wed, Feb 02, 2011 at 06:02:55PM -0800, Julian Foad wrote:
> Hi Stefan.
> 
> On IRC
>  I
> see you wrote:
> 
> > are newly added files supposed to have a NULL properties column in
> > wc_db?
> > they have props in ACTUAL_NODE
> > but in NODES the column is NULL
> 
> There is no pristine version of an added node. The comment in
> ACTUAL_NODES doesn't define this, but the pristine properties column
> being null (rather than an empty list of props) makes sense to me.

OK, thanks for clarifying.

> > we have several loops in the code that do things like this:
> > while (have_row && !svn_sqlite__column_is_null(stmt, 0))
> 
> I was curious and looked, but can't see any such loops.  Where?

Hmmm... you're right. What probably happened is that I modeled
the while loop after an if statement condition that uses this.

Re: [PATCH] New XFail test for the issue 3781

[Prabhu Gnana Sundar]

On Thursday 03 February 2011 05:23 PM, Noorul Islam K M wrote:

Prabhu Gnana Sundar  writes:


Hi all,

Currently, as per the issue 3781, "checkout" reads the authz file in
*case insensitive* way, whereas "commit" reads the authz file in *case
sensitive* way.

Here is what is observed:
1. Checkout is *case insensitive* about the repository name section
and also the path-inside-repo section.
2. Commit is *case sensitive* about the repository name section *but
case insensitive* about the path-inside-repo section.

Attached an XFail testcase and the log message with this mail. Please
share your views.




Thanks and regards
Prabhu

[[[
XFail testcase for the issue 3781

* subversion/tests/cmdline/authz_tests.py
   (case_insensitive_authz, test_list): New XFail test for the issue 3781

Patch by: Prabhu Gnana Sundar
Suggested by: Kamesh Jayachandran

Index: subversion/tests/cmdline/authz_tests.py
===
--- subversion/tests/cmdline/authz_tests.py (revision 1066732)
+++ subversion/tests/cmdline/authz_tests.py (working copy)
@@ -1061,6 +1061,67 @@
   [], 'ls', '-R',
   sbox.repo_url)

+
+def case_insensitive_authz(sbox):
+  "authz issue #3781, check case insensitiveness"
+
+  sbox.build()
+
+  # test the case-insensitivity of the path inside the repo
+  write_authz_file(sbox, {"/": "jrandom = r", "/A": "jrandom = r", "/a/Mu": 
"jrandom = rw"})
+

I am not sure whether this is relevant. I think for consistency lines
should be of 79 characters.


wrapped the text for the consistency :)
Attached the patch with this mail...
Thanks Noorul for your feedback over the "indentation".


Thanks and regards
Prabhu
Index: subversion/tests/cmdline/authz_tests.py
===
--- subversion/tests/cmdline/authz_tests.py	(revision 1066732)
+++ subversion/tests/cmdline/authz_tests.py	(working copy)
@@ -1061,6 +1061,69 @@
  [], 'ls', '-R',
  sbox.repo_url)
 
+
+def case_insensitive_authz(sbox):
+  "authz issue #3781, check case insensitiveness"
+
+  sbox.build()
+
+  # test the case-insensitivity of the path inside the repo
+  write_authz_file(sbox, {"/": "jrandom = r",
+  "/A": "jrandom = r",
+  "/a/Mu": "jrandom = rw"})
+
+  write_restrictive_svnserve_conf(sbox.repo_dir)
+
+  wc_dir = sbox.wc_dir
+
+  mu_path = os.path.join(wc_dir, 'A', 'mu')
+  mu_url = sbox.repo_url + '/A/mu'
+  svntest.main.file_append(mu_path, "hi")
+
+  # Create expected output tree.
+  expected_output = svntest.wc.State(wc_dir, {
+'A/mu' : Item(verb='Sending'),
+})
+
+  # Commit the file.
+  svntest.actions.run_and_verify_commit(wc_dir,
+expected_output,
+None,
+None,
+mu_path)
+  svntest.actions.run_and_verify_svn2('No error',
+  svntest.verify.AnyOutput, [],
+  0, 'cat', mu_url)
+
+  def mixcases(repo_name):
+mixed_repo_name = ''
+for i in range(0, len(repo_name)):
+  if i % 2 == 0:
+mixed_val = repo_name[i].upper()
+mixed_repo_name = mixed_repo_name + mixed_val
+  else:
+mixed_val = repo_name[i].lower()
+mixed_repo_name = mixed_repo_name + mixed_val
+return mixed_repo_name 
+
+  mixed_case_repo_dir = mixcases(os.path.basename(sbox.repo_dir))
+
+  # test the case-insensitivity of the repo name
+  write_authz_file(sbox, {}, sections = {mixed_case_repo_dir + ":/": "jrandom = r",
+ mixed_case_repo_dir + ":/A": "jrandom = r",
+ mixed_case_repo_dir + ":/A/mu": "jrandom = rw"})
+
+  svntest.main.file_append(mu_path, "hi")
+  # Commit the file again.
+  svntest.actions.run_and_verify_commit(wc_dir,
+expected_output,
+None,
+None,
+mu_path)
+  svntest.actions.run_and_verify_svn2('No error',
+  svntest.verify.AnyOutput, [],
+  0, 'cat', mu_url) 
+
 
 # Run the tests
 
@@ -1093,6 +1156,8 @@
   Skip(wc_wc_copy_revert, svntest.main.is_ra_type_file),
   Skip(authz_recursive_ls,
svntest.main.is_ra_type_file),
+  XFail(Skip(case_insensitive_authz,
+ svntest.main.is_ra_type_file)),
  ]
 serial_only = True
 
[[[
XFail testcase for the issue 3781

* subversion/tests/cmdline/authz_tests.py
  (case_insensitive_

Re: [PATCH] New XFail test for the issue 3781

[Noorul Islam K M]

Prabhu Gnana Sundar  writes:

> Hi all,
>
> Currently, as per the issue 3781, "checkout" reads the authz file in
> *case insensitive* way, whereas "commit" reads the authz file in *case
> sensitive* way.
>
> Here is what is observed:
> 1. Checkout is *case insensitive* about the repository name section
> and also the path-inside-repo section.
> 2. Commit is *case sensitive* about the repository name section *but
> case insensitive* about the path-inside-repo section.
>
> Attached an XFail testcase and the log message with this mail. Please
> share your views.
>
>
>
>
> Thanks and regards
> Prabhu
>
> [[[
> XFail testcase for the issue 3781
>
> * subversion/tests/cmdline/authz_tests.py
>   (case_insensitive_authz, test_list): New XFail test for the issue 3781
>
> Patch by: Prabhu Gnana Sundar 
> Suggested by: Kamesh Jayachandran 
>
> Index: subversion/tests/cmdline/authz_tests.py
> ===
> --- subversion/tests/cmdline/authz_tests.py   (revision 1066732)
> +++ subversion/tests/cmdline/authz_tests.py   (working copy)
> @@ -1061,6 +1061,67 @@
>   [], 'ls', '-R',
>   sbox.repo_url)
>  
> +
> +def case_insensitive_authz(sbox):
> +  "authz issue #3781, check case insensitiveness"
> +
> +  sbox.build()
> +
> +  # test the case-insensitivity of the path inside the repo
> +  write_authz_file(sbox, {"/": "jrandom = r", "/A": "jrandom = r", "/a/Mu": 
> "jrandom = rw"})
> +

I am not sure whether this is relevant. I think for consistency lines
should be of 79 characters. 

> +  write_restrictive_svnserve_conf(sbox.repo_dir)
> +
> +  wc_dir = sbox.wc_dir
> +
> +  mu_path = os.path.join(wc_dir, 'A', 'mu')
> +  mu_url = sbox.repo_url + '/A/mu'
> +  svntest.main.file_append(mu_path, "hi")
> +
> +  # Create expected output tree.
> +  expected_output = svntest.wc.State(wc_dir, {
> +'A/mu' : Item(verb='Sending'),
> +})
> +
> +  # Commit the file.
> +  svntest.actions.run_and_verify_commit(wc_dir,
> +expected_output,
> +None,
> +None,
> +mu_path)
> +  svntest.actions.run_and_verify_svn2('No error',
> +  svntest.verify.AnyOutput, [],
> +  0, 'cat', mu_url)
> +
> +  def mixcases(repo_name):
> +mixed_repo_name = ''
> +for i in range(0, len(repo_name)):
> +  if i % 2 == 0:
> +mixed_val = repo_name[i].upper()
> +mixed_repo_name = mixed_repo_name + mixed_val
> +  else:
> +mixed_val = repo_name[i].lower()
> +mixed_repo_name = mixed_repo_name + mixed_val
> +return mixed_repo_name 
> +
> +  mixed_case_repo_dir = mixcases(os.path.basename(sbox.repo_dir))
> +
> +  # test the case-insensitivity of the repo name
> +  write_authz_file(sbox, {}, sections = {mixed_case_repo_dir + ":/": 
> "jrandom = r",
> + mixed_case_repo_dir + ":/A": 
> "jrandom = r",
> + mixed_case_repo_dir + ":/A/mu": 
> "jrandom = rw"})
> +

Line break ?

> +  svntest.main.file_append(mu_path, "hi")
> +  # Commit the file again.
> +  svntest.actions.run_and_verify_commit(wc_dir,
> +expected_output,
> +None,
> +None,
> +mu_path)
> +  svntest.actions.run_and_verify_svn2('No error',
> +  svntest.verify.AnyOutput, [],
> +  0, 'cat', mu_url) 
> +
>  
>  # Run the tests
>  
> @@ -1093,6 +1154,8 @@
>Skip(wc_wc_copy_revert, svntest.main.is_ra_type_file),
>Skip(authz_recursive_ls,
> svntest.main.is_ra_type_file),
> +  XFail(Skip(case_insensitive_authz,
> + svntest.main.is_ra_type_file)),
>   ]
>  serial_only = True
>  

Thanks and Regards
Noorul

[PATCH] New XFail test for the issue 3781

[Prabhu Gnana Sundar]

Hi all,

Currently, as per the issue 3781, "checkout" reads the authz file in 
*case insensitive* way, whereas "commit" reads the authz file in *case 
sensitive* way.


Here is what is observed:
1. Checkout is *case insensitive* about the repository name section and 
also the path-inside-repo section.
2. Commit is *case sensitive* about the repository name section *but 
case insensitive* about the path-inside-repo section.


Attached an XFail testcase and the log message with this mail. Please 
share your views.





Thanks and regards
Prabhu
[[[
XFail testcase for the issue 3781

* subversion/tests/cmdline/authz_tests.py
  (case_insensitive_authz, test_list): New XFail test for the issue 3781

Patch by: Prabhu Gnana Sundar 
Suggested by: Kamesh Jayachandran 
]]]
Index: subversion/tests/cmdline/authz_tests.py
===
--- subversion/tests/cmdline/authz_tests.py	(revision 1066732)
+++ subversion/tests/cmdline/authz_tests.py	(working copy)
@@ -1061,6 +1061,67 @@
  [], 'ls', '-R',
  sbox.repo_url)
 
+
+def case_insensitive_authz(sbox):
+  "authz issue #3781, check case insensitiveness"
+
+  sbox.build()
+
+  # test the case-insensitivity of the path inside the repo
+  write_authz_file(sbox, {"/": "jrandom = r", "/A": "jrandom = r", "/a/Mu": "jrandom = rw"})
+
+  write_restrictive_svnserve_conf(sbox.repo_dir)
+
+  wc_dir = sbox.wc_dir
+
+  mu_path = os.path.join(wc_dir, 'A', 'mu')
+  mu_url = sbox.repo_url + '/A/mu'
+  svntest.main.file_append(mu_path, "hi")
+
+  # Create expected output tree.
+  expected_output = svntest.wc.State(wc_dir, {
+'A/mu' : Item(verb='Sending'),
+})
+
+  # Commit the file.
+  svntest.actions.run_and_verify_commit(wc_dir,
+expected_output,
+None,
+None,
+mu_path)
+  svntest.actions.run_and_verify_svn2('No error',
+  svntest.verify.AnyOutput, [],
+  0, 'cat', mu_url)
+
+  def mixcases(repo_name):
+mixed_repo_name = ''
+for i in range(0, len(repo_name)):
+  if i % 2 == 0:
+mixed_val = repo_name[i].upper()
+mixed_repo_name = mixed_repo_name + mixed_val
+  else:
+mixed_val = repo_name[i].lower()
+mixed_repo_name = mixed_repo_name + mixed_val
+return mixed_repo_name 
+
+  mixed_case_repo_dir = mixcases(os.path.basename(sbox.repo_dir))
+
+  # test the case-insensitivity of the repo name
+  write_authz_file(sbox, {}, sections = {mixed_case_repo_dir + ":/": "jrandom = r",
+ mixed_case_repo_dir + ":/A": "jrandom = r",
+ mixed_case_repo_dir + ":/A/mu": "jrandom = rw"})
+
+  svntest.main.file_append(mu_path, "hi")
+  # Commit the file again.
+  svntest.actions.run_and_verify_commit(wc_dir,
+expected_output,
+None,
+None,
+mu_path)
+  svntest.actions.run_and_verify_svn2('No error',
+  svntest.verify.AnyOutput, [],
+  0, 'cat', mu_url) 
+
 
 # Run the tests
 
@@ -1093,6 +1154,8 @@
   Skip(wc_wc_copy_revert, svntest.main.is_ra_type_file),
   Skip(authz_recursive_ls,
svntest.main.is_ra_type_file),
+  XFail(Skip(case_insensitive_authz,
+ svntest.main.is_ra_type_file)),
  ]
 serial_only = True
 

[PATCH] - Fix for issue #3792

[Noorul Islam K M]

Attached is the patch to fix issue #3792. Please review and let me know
your comments.

Log
[[[

Fix for issue #3792. Make svn info to display information for
excluded items.

* subversion/include/private/svn_wc_private.h,
  subversion/libsvn_wc/node.c
  (svn_wc__node_depth_is_exclude): New helper function to find
whether the node is set with depth 'exclude'.

* subversion/libsvn_client/info.c
  (crawl_entries): Build minimal information if the node has
depth set as 'exclude'.

* subversion/svn/info-cmd.c
  (print_info): Print depth as 'exclude' for nodes having depth
exclude. Hide node kind while printing exclude infomation.

* subversion/tests/cmdline/depth_tests.py
  (test_list): Remove XFail marker from info_excluded test.

Patch by: Noorul Islam K M 
]]]

Index: subversion/tests/cmdline/depth_tests.py
===
--- subversion/tests/cmdline/depth_tests.py (revision 1066732)
+++ subversion/tests/cmdline/depth_tests.py (working copy)
@@ -2841,7 +2841,7 @@
   excluded_path_misc_operation,
   excluded_receive_remote_removal,
   exclude_keeps_hidden_entries,
-  XFail(info_excluded, issues=3792),
+  info_excluded,
   tree_conflicts_resolved_depth_empty,
   tree_conflicts_resolved_depth_files,
   tree_conflicts_resolved_depth_immediates,
Index: subversion/svn/info-cmd.c
===
--- subversion/svn/info-cmd.c   (revision 1066732)
+++ subversion/svn/info-cmd.c   (working copy)
@@ -291,7 +291,8 @@
   break;
 
 case svn_node_none:
-  SVN_ERR(svn_cmdline_printf(pool, _("Node Kind: none\n")));
+  if (info->depth != svn_depth_exclude)
+SVN_ERR(svn_cmdline_printf(pool, _("Node Kind: none\n")));
   break;
 
 case svn_node_unknown:
@@ -363,6 +364,9 @@
 SVN_ERR(svn_cmdline_printf(pool, _("Copied From Rev: %ld\n"),
info->copyfrom_rev));
 }
+  
+  if (info->depth == svn_depth_exclude)
+SVN_ERR(svn_cmdline_printf(pool, _("Depth: exclude\n")));
 
   if (info->last_changed_author)
 SVN_ERR(svn_cmdline_printf(pool, _("Last Changed Author: %s\n"),
Index: subversion/include/private/svn_wc_private.h
===
--- subversion/include/private/svn_wc_private.h (revision 1066732)
+++ subversion/include/private/svn_wc_private.h (working copy)
@@ -759,6 +759,15 @@
  apr_pool_t *result_pool,
  apr_pool_t *scratch_pool);
 
+/**
+ * Find whether @a local_abspath is set with depth exclude using @a wc_ctx. 
+ */
+svn_error_t *
+svn_wc__node_depth_is_exclude(svn_boolean_t *exclude,
+  svn_wc_context_t *wc_ctx,
+  const char *local_abspath,
+  apr_pool_t *scratch_pool);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
Index: subversion/libsvn_wc/node.c
===
--- subversion/libsvn_wc/node.c (revision 1066732)
+++ subversion/libsvn_wc/node.c (working copy)
@@ -247,6 +247,30 @@
 }
 
 svn_error_t *
+svn_wc__node_depth_is_exclude(svn_boolean_t *exclude,
+  svn_wc_context_t *wc_ctx,
+  const char *local_abspath,
+  apr_pool_t *scratch_pool)
+{
+  svn_wc__db_status_t status;
+  svn_error_t *err;
+  
+  *exclude = FALSE;
+
+  err = svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL,
+ wc_ctx->db, local_abspath, scratch_pool,
+ scratch_pool);
+
+  if ((! err) && (status == svn_wc__db_status_excluded))
+*exclude = TRUE;
+
+  return svn_error_return(err);
+}
+
+svn_error_t *
 svn_wc__node_get_depth(svn_depth_t *depth,
svn_wc_context_t *wc_ctx,
const char *local_abspath,
Index: subversion/libsvn_client/info.c
===
--- subversion/libsvn_client/info.c (revision 1066732)
+++ subversion/libsvn_client/info.c (working copy)
@@ -405,20 +405,31 @@
   /* Check for a tree conflict on the root node of the info, and if there
  is one, send a minimal info struct. */
   const svn_wc_conflict_description2_t *tree_conflict;
+  svn_boolean_t exclude = FALSE;
 
   SVN_ERR(svn_wc__get_tree_conflict(&tree_conflict, ctx->wc_ctx,
 local_abspath, pool, pool));
 
-  if (tree_conflict)
+  /* Check whether depth of the node is set as exclude. */
+  if (! tree_conflict)
+SV