Re: [PATCH] add sub command 'youngest' to svn [Was: Re: [PATCH] add sub command 'youngest' to svnrdump]

[Masaru Tsuchiyama]

Hi.

This is the log message for the patch.

[[[
   issue #4299: add sub command 'youngest' to svn

   * subversion/svn/cl.h
 (svn_cl__opt_state_t): Add no_newline member.
 (): Add svn_cl__youngest to the command procedure list.
 
   * subversion/svn/svn.c
 (svn_cl__options): Add --new-line option.
 (svn_cl__cmd_table): Add 'youngest' entry.
 (sub_main): Add 'n' option.
   * subversion/svn/youngest-cmd.c
 (svn_cl__youngest): implement the 'youngest' sub command handler.
   ]]]


-- 
Masaru Tsuchiyama 

> Hi.
> 
> I implement 'youngest' sub command to svn, and attach a patch.
> 
> -- 
> Masaru Tsuchiyama 
> 
> 
> > On 07/19/2013 08:37 PM, Masaru Tsuchiyama wrote:
> > > Hi.
> > >
> > > I attach a patch to add sub command 'youngest' to svnrdump.
> > >
> > > [[[
> > > add sub command 'youngest' to svnrdump
> > >
> > > * subversion/svnrdump/svnrdump.c
> > >   (svnrdump__cmd_table): Add an entry for youngest sub command.
> > >   (main): Print youngest revision and exit when setting youngest 
> > > option.
> > > ]]]
> > >
> > >
> > 
> > Note that this patch could be the resolution for issue #4299 
> > (http://subversion.tigris.org/issues/show_bug.cgi?id=4299).  But that 
> > simply begs the question of whether the functionality would have been 
> > better implemented in 'svn' itself where it could enjoy the dual flavors of 
> > targets found therein:  URL or WC-PATH.

[PATCH] add sub command 'youngest' to svn [Was: Re: [PATCH] add sub command 'youngest' to svnrdump]

[Masaru Tsuchiyama]

Hi.

I implement 'youngest' sub command to svn, and attach a patch.

-- 
Masaru Tsuchiyama 


> On 07/19/2013 08:37 PM, Masaru Tsuchiyama wrote:
> > Hi.
> >
> > I attach a patch to add sub command 'youngest' to svnrdump.
> >
> > [[[
> > add sub command 'youngest' to svnrdump
> >
> > * subversion/svnrdump/svnrdump.c
> >   (svnrdump__cmd_table): Add an entry for youngest sub command.
> >   (main): Print youngest revision and exit when setting youngest option.
> > ]]]
> >
> >
> 
> Note that this patch could be the resolution for issue #4299 
> (http://subversion.tigris.org/issues/show_bug.cgi?id=4299).  But that simply 
> begs the question of whether the functionality would have been better 
> implemented in 'svn' itself where it could enjoy the dual flavors of targets 
> found therein:  URL or WC-PATH.


youngest.patch
Description: Binary data

Re: [PATCH] add sub command 'youngest' to svnrdump

[C. Michael Pilato]

On 07/19/2013 08:37 PM, Masaru Tsuchiyama wrote:

Hi.

I attach a patch to add sub command 'youngest' to svnrdump.

[[[
add sub command 'youngest' to svnrdump

* subversion/svnrdump/svnrdump.c
  (svnrdump__cmd_table): Add an entry for youngest sub command.
  (main): Print youngest revision and exit when setting youngest option.
]]]




Note that this patch could be the resolution for issue #4299 
(http://subversion.tigris.org/issues/show_bug.cgi?id=4299).  But that 
simply begs the question of whether the functionality would have been 
better implemented in 'svn' itself where it could enjoy the dual flavors 
of targets found therein:  URL or WC-PATH.

[PATCH] add sub command 'youngest' to svnrdump

[Masaru Tsuchiyama]

Hi.

I attach a patch to add sub command 'youngest' to svnrdump.

[[[
   add sub command 'youngest' to svnrdump

   * subversion/svnrdump/svnrdump.c
 (svnrdump__cmd_table): Add an entry for youngest sub command.
 (main): Print youngest revision and exit when setting youngest option.
   ]]]


-- 
Masaru Tsuchiyama 


svnrdump.patch
Description: Binary data

RE: svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h [2]

[Bert Huijben]

> -Original Message-
> From: stef...@apache.org [mailto:stef...@apache.org]
> Sent: vrijdag 19 juli 2013 22:46
> To: comm...@subversion.apache.org
> Subject: svn commit: r1505006 - in /subversion/trunk: ./
> subversion/include/svn_io.h subversion/libsvn_client/patch.c
> subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c
> 
> Author: stefan2
> Date: Fri Jul 19 20:46:22 2013
> New Revision: 1505006
> 
> URL: http://svn.apache.org/r1505006
> Log:
> Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
> These introduce svn_io_file_create_empty and svn_io_file_create_binary
> as alternatives to svn_io_file_create.
> 
> A few conflicts had to be resolved.


Second reply:

>  {
>apr_file_t *f;
>apr_size_t written;
> @@ -1178,16 +1179,28 @@ svn_error_t *svn_io_file_create(const ch
> (APR_WRITE | APR_CREATE | APR_EXCL),
> APR_OS_DEFAULT,
> pool));
> -  if (contents && *contents)
> -err = svn_io_file_write_full(f, contents, strlen(contents),
> - &written, pool);
> -
> +  if (length)
> +err = svn_io_file_write_full(f, contents, length, &written, pool);
> 
>return svn_error_trace(
>  svn_error_compose_create(err,
>   svn_io_file_close(f, 
> pool)));

Not really related to this patch,  but it is probably better to try to delete 
the new file when writing the initial data failed here.

Otherwise we will just leave a file on disk where nobody will look after, 
because the user of this function can't really determine in which stage the 
create failed.

Bert 

RE: svn commit: r1505055 - /subversion/trunk/subversion/tests/libsvn_subr/string-test.c

[Bert Huijben]

> -Original Message-
> From: stef...@apache.org [mailto:stef...@apache.org]
> Sent: zaterdag 20 juli 2013 00:29
> To: comm...@subversion.apache.org
> Subject: svn commit: r1505055 -
> /subversion/trunk/subversion/tests/libsvn_subr/string-test.c
> 
> Author: stefan2
> Date: Fri Jul 19 22:29:21 2013
> New Revision: 1505055
> 
> URL: http://svn.apache.org/r1505055
> Log:
> Add a test case for base36 string conversion.
> 
> * subversion/tests/libsvn_subr/string-test.c
>   (sub_test_base36): new two-way conversion test
>   (test_base36): test the conversion with a few different values
>   (test_funcs): register the new test
> 
> Modified:
> subversion/trunk/subversion/tests/libsvn_subr/string-test.c
> 
> Modified: subversion/trunk/subversion/tests/libsvn_subr/string-test.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_su
> br/string-test.c?rev=1505055&r1=1505054&r2=1505055&view=diff
> ==
> 
> --- subversion/trunk/subversion/tests/libsvn_subr/string-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_subr/string-test.c Fri Jul 19
> 22:29:21 2013
> @@ -537,7 +537,42 @@ test24(apr_pool_t *pool)
>SVN_TEST_ASSERT(length == 20);
>SVN_TEST_STRING_ASSERT(buffer, "18446744073709551615");
> 
> -  return test_stringbuf_unequal("abc", "abb", pool);
> +  return SVN_NO_ERROR;
> +}

It appears you accidentally removed the last line from the previous test?

(Problem resolving conflict?)


> +
> +static svn_error_t *
> +sub_test_base36(apr_uint64_t value, const char *base36)
> +{
> +  char buffer[SVN_INT64_BUFFER_SIZE];
> +  apr_size_t length;
> +  apr_size_t expected_length = strlen(base36);
> +  const char *end = buffer;
> +  apr_uint64_t result;
> +
> +  length = svn__ui64tobase36(buffer, value);
> +  SVN_TEST_ASSERT(length == expected_length);
> +  SVN_TEST_STRING_ASSERT(buffer, base36);
> +
> +  result = svn__base36toui64(&end, buffer);
> +  SVN_TEST_ASSERT(end - buffer == length);
> +  SVN_TEST_ASSERT(result == value);
> +
> +  result = svn__base36toui64(NULL, buffer);
> +  SVN_TEST_ASSERT(result == value);
> +
> +  return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *
> +test_base36(apr_pool_t *pool)
> +{
> +  SVN_ERR(sub_test_base36(0, "0"));
> +  SVN_ERR(sub_test_base36(1234567890ull, "kf12oi"));
> +  SVN_ERR(sub_test_base36(0x7fffull, "1y2p0ij32e8e7"));
> +  SVN_ERR(sub_test_base36(0x8000ull, "1y2p0ij32e8e8"));
> +  SVN_ERR(sub_test_base36(0xull, "3w5e11264sgsf"));
> +
> +  return SVN_NO_ERROR;
>  }
> 
>  static svn_error_t *
> @@ -770,6 +805,8 @@ struct svn_test_descriptor_t test_funcs[
> "compare stringbufs; same length, different content"),
>  SVN_TEST_PASS2(test24,
> "verify i64toa"),
> +SVN_TEST_PASS2(test_base36,
> +   "verify base36 conversion"),

Usually we try to add new tests at the end of the list to avoid renumbering.
(or there must be good reasons to keep more of them together)

Bert

Re: svn commit: r1505055 - /subversion/trunk/subversion/tests/libsvn_subr/string-test.c

[Ivan Zhakov]

On Sat, Jul 20, 2013 at 2:29 AM,   wrote:
> Author: stefan2
> Date: Fri Jul 19 22:29:21 2013
> New Revision: 1505055
>
> URL: http://svn.apache.org/r1505055
> Log:
> Add a test case for base36 string conversion.
>
> * subversion/tests/libsvn_subr/string-test.c
>   (sub_test_base36): new two-way conversion test
>   (test_base36): test the conversion with a few different values
>   (test_funcs): register the new test
>
> Modified:
> subversion/trunk/subversion/tests/libsvn_subr/string-test.c
>
> Modified: subversion/trunk/subversion/tests/libsvn_subr/string-test.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/string-test.c?rev=1505055&r1=1505054&r2=1505055&view=diff
> ==
> --- subversion/trunk/subversion/tests/libsvn_subr/string-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_subr/string-test.c Fri Jul 19 
> 22:29:21 2013
> @@ -537,7 +537,42 @@ test24(apr_pool_t *pool)
>SVN_TEST_ASSERT(length == 20);
>SVN_TEST_STRING_ASSERT(buffer, "18446744073709551615");
>
> -  return test_stringbuf_unequal("abc", "abb", pool);
> +  return SVN_NO_ERROR;
> +}
Looks like unwanted or undocumented change. Isn't it?

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

RE: svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h subversion/libsvn_client/patch.c subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c

[Bert Huijben]

> -Original Message-
> From: stef...@apache.org [mailto:stef...@apache.org]
> Sent: vrijdag 19 juli 2013 22:46
> To: comm...@subversion.apache.org
> Subject: svn commit: r1505006 - in /subversion/trunk: ./
> subversion/include/svn_io.h subversion/libsvn_client/patch.c
> subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c
> 
> Author: stefan2
> Date: Fri Jul 19 20:46:22 2013
> New Revision: 1505006
> 
> URL: http://svn.apache.org/r1505006
> Log:
> Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
> These introduce svn_io_file_create_empty and svn_io_file_create_binary
> as alternatives to svn_io_file_create.
> 
> A few conflicts had to be resolved.

Can you combine the original log messages to get a proper description here with 
the function additions. This makes it much easier to see when code was 
introduced when looking through the file history.

Looking at just this merge it is not clear why svn_io_file_create_binary() 
needs to be added, and what additional features these functions have over 
svn_io_file_create(), which already creates a text file with binary encoding.

With the diff it is clear, but the reason why belongs in the log message.


Looking at the diff, I understand the value of svn_io_file_create_binary(), but 
svn_io_file_create_empty() still looks like a convenience function for creating 
empty files, while you could just pass "", 0 to svn_io_file_create_binary().

And looking at the performance of the old working copy format on Windows, I'm 
certainly not recommending new code to create many empty files at once.
(File creations have very bad performance characteristics on Windows and 
network shares)

Bert

Re: svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h subversion/libsvn_client/patch.c subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c

[Ivan Zhakov]

On Sat, Jul 20, 2013 at 12:46 AM,   wrote:
> Author: stefan2
> Date: Fri Jul 19 20:46:22 2013
> New Revision: 1505006
>
> URL: http://svn.apache.org/r1505006
> Log:
> Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
> These introduce svn_io_file_create_empty and svn_io_file_create_binary
> as alternatives to svn_io_file_create.
>
[...]

>
> +svn_error_t *svn_io_file_create(const char *file,
> +const char *contents,
> +apr_pool_t *pool)
> +{
> +  return svn_error_trace(svn_io_file_create_binary(file, contents,
> +   strlen(contents), pool));
> +}
> +
> +svn_error_t *svn_io_file_create_empty(const char *file,
> +  apr_pool_t *pool)
> +{
> +  return svn_error_trace(svn_io_file_create_binary(file, "", 0, pool));
> +}
You may just pass NULL to svn_io_file_create_binary for zero length
data. Also I'm not sure that svn_io_file_create_empty() is really
needed.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

Re: svn commit: r1505046 - in /subversion/trunk: ./ subversion/include/private/svn_string_private.h subversion/libsvn_subr/string.c

[Ivan Zhakov]

On Sat, Jul 20, 2013 at 2:03 AM,   wrote:
> Author: stefan2
> Date: Fri Jul 19 22:03:34 2013
> New Revision: 1505046
>
> URL: http://svn.apache.org/r1505046
> Log:
> Merge revisions r1453590,1454307 from branches/fsfs-format7.
> These introduce base36 conversion routines.
>
> Modified:
> subversion/trunk/   (props changed)
> subversion/trunk/subversion/include/private/svn_string_private.h
> subversion/trunk/subversion/libsvn_subr/string.c
>
> Propchange: subversion/trunk/
> --
>   Merged /subversion/branches/fsfs-format7:r1453590,1454307
>
> Modified: subversion/trunk/subversion/include/private/svn_string_private.h
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_string_private.h?rev=1505046&r1=1505045&r2=1505046&view=diff
> ==
> --- subversion/trunk/subversion/include/private/svn_string_private.h 
> (original)
> +++ subversion/trunk/subversion/include/private/svn_string_private.h Fri Jul 
> 19 22:03:34 2013
> @@ -175,6 +175,32 @@ svn__ui64toa_sep(apr_uint64_t number, ch
>  char *
>  svn__i64toa_sep(apr_int64_t number, char seperator, apr_pool_t *pool);
>
> +
> +/** Writes the @a number as base36-encoded string into @a dest. The latter
> + * must provide space for at least #SVN_INT64_BUFFER_SIZE characters.
> + * Returns the number chars written excluding the terminating NUL.
> + *
> + * @note The actual maximum buffer requirement is much shorter than
> + * #SVN_INT64_BUFFER_SIZE but introducing yet another constant is only
> + * marginally useful and may open the door to security issues when e.g.
> + * switching between base10 and base36 encoding.
> + */
> +apr_size_t
> +svn__ui64tobase36(char *dest, apr_uint64_t number);
> +
> +/** Returns the value of the base36 encoded unsigned integer starting at
> + * @a source.  If @a next is not NULL, @a *next will be set to the first
> + * position after the integer.
> + *
> + * The data in @a source will be considered part of the number to parse
> + * as long as the characters are within the base36 range.  If there are
> + * no such characters to begin with, 0 is returned.  Inputs with more than
> + * #SVN_INT64_BUFFER_SIZE digits will not be fully parsed, i.e. the value
> + * of @a *next as well as the return value are undefined.
> + */
> +apr_uint64_t
> +svn__base36toui64(const char **next, const char *source);
> +
Hi Stefan,

Simple tests would be useful IMHO.



-- 
Ivan Zhakov

Re: svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h subversion/libsvn_client/patch.c subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c

[Ivan Zhakov]

On Sat, Jul 20, 2013 at 2:05 AM, Bert Huijben  wrote:
>> -Original Message-
>> From: stef...@apache.org [mailto:stef...@apache.org]
>> Sent: vrijdag 19 juli 2013 22:46
>> To: comm...@subversion.apache.org
>> Subject: svn commit: r1505006 - in /subversion/trunk: ./
>> subversion/include/svn_io.h subversion/libsvn_client/patch.c
>> subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c
>>
>> Author: stefan2
>> Date: Fri Jul 19 20:46:22 2013
>> New Revision: 1505006
>>
>> URL: http://svn.apache.org/r1505006
>> Log:
>> Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
>> These introduce svn_io_file_create_empty and svn_io_file_create_binary
>> as alternatives to svn_io_file_create.
>>
>> A few conflicts had to be resolved.
>
> Can you combine the original log messages to get a proper description here 
> with the function
> additions. This makes it much easier to see when code was introduced when 
> looking
> through the file history.
I agree with Bert that including original log message would be really
nice to review such merges properly. Actually I prefer to have full
log messages for merges describing all changes like they did to trunk
without committing to branch. Because I don't follow feature branches,
but want to review trunk commits.


-- 
Ivan Zhakov

Re: Filename encoding in working copy (was: Check-out fails with LANG=C)

[Stefan Sperling]

On Fri, Jul 19, 2013 at 05:27:11PM +0200, Vincent Lefevre wrote:
> [Cc to the dev@ list]
> 
> On 2013-07-19 16:50:49 +0200, Stefan Sperling wrote:
> > For this, we need answers to questions like:
> > How can the client detect whether the stored encoding name matches
> > the on-disk encoding? What does it do when they differ? How can users
> > re-encode filenames in the working copy when on-disk encoding has changed? 
> 
> I don't understand what you mean here. If the chosen encoding for the
> filenames is stored in the working copy: The on-disk encoding has been
> chosen by Subversion (at checkout time). So, initially it matches the
> stored encoding. This normally doesn't change...

Apparently it can change if files are copied by some copying tools,
or through network filesystem like samba. This was mentioned before
in the users@ thread you responded to.

Filename encoding in working copy (was: Check-out fails with LANG=C)

[Vincent Lefevre]

[Cc to the dev@ list]

On 2013-07-19 16:50:49 +0200, Stefan Sperling wrote:
> On Fri, Jul 19, 2013 at 04:32:50PM +0200, Vincent Lefevre wrote:
> > [...]
> > 
> > Actually I think that the encoding needs to be stored somewhere in the
> > working copy. Otherwise even if the user never changes the encoding,
> > problems may occur, and this is also true with the current behavior.
> > Indeed it was said in the past that USB keys were supported. So, move
> > a USB key to a different computer, where the encoding specified by the
> > environment is different... and see what happens if you try to do an
> > "svn update"...
> 
> Simply storing the encoding doesn't really solve anything. Sure, it
> remembers the LC_CTYPE value as the time the working copy was created.
> But then... what?

At least it would work better. And in some cases, one wouldn't notice
any problem. For instance, a repository (and therefore the corresponding
working copies too) may contain mostly filenames with only US-ASCII
characters, and when moving a USB key to another computer, one may be
interested only in a part of the working copy with such filenames.

> We also need to specify some new behaviour that increases user
> convenience for such a new feature to have any value.
> 
> For this, we need answers to questions like:
> How can the client detect whether the stored encoding name matches
> the on-disk encoding? What does it do when they differ? How can users
> re-encode filenames in the working copy when on-disk encoding has changed? 

I don't understand what you mean here. If the chosen encoding for the
filenames is stored in the working copy: The on-disk encoding has been
chosen by Subversion (at checkout time). So, initially it matches the
stored encoding. This normally doesn't change... unless the user has
explicitly chosen to re-encode the filenames globally with some tool.
In such a case, the user also needs to modify the stored encoding, and
Subversion can provide a command for that. Subversion can also provide
a command to re-encode the filenames of a working copy and update the
stored encoding.

The main problem is when a working copy is moved to another machine,
on which the chosen encoding is not supported on the new machine. But
I don't think Subversion can do anything about it. The user needs to
make sure that the chosen encoding is supported on the new machine.
This should be the case for ASCII (+ escaping mechanism) and UTF-8.

> I'm very much interested in enhancements in this area, but at this
> point we don't need to rehash all the problems there are. We need
> to design solutions. This discussion needs a change of direction towards
> being more constructive, or it will die with no results. The discussion
> is also increasingly off-topic for the users@ list.
> 
> In other words, I'm happy to continue this discussion on the dev@ list
> and review your proposed design specs and patches there.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: svnsync crashes on a huge commit

[Philip Martin]

Philip Martin  writes:

> I see those values with 1.8.x but the problem appears to be fixed on
> trunk where the memory use is remains more or less constant at 100MB
> virtual, 25MB resident.

It's r1499863, the rewrite of ra_serf's replay code, that fixes the
memory use.  Reverse merging that revision to trunk causes the problem
to come back.

-- 
Philip Martin | Subversion Committer
WANdisco | Non-Stop Data