[l10n] Translation status report for trunk r1055292
Translation status report for tr...@r1055292 lang trans untrans fuzzy obs -- de2053 126 239 199 es1990 189 273 338 fr2141 38 75 41 it1839 340 472 162 ja1982 197 344 584 ko2027 152 187 179 nb2043 136 197 307 pl2075 104 146 82 pt_BR1803 376 489 157 sv1759 420 513 163 zh_CN2175 4 11 88 zh_TW1739 440 551 225
Re: [PATCH] allow svnsync to translate non-UTF-8 log messages to UTF-8 (v. 3)
On Sun, 2011-01-02, Daniel Shahaf wrote:
Danny Trebbien wrote on Sun, Dec 26, 2010 at 15:39:05 -0800:
Attached are version 3 of the patch and corresponding log message.
Also attached is a TGZ archive of two Subversion repository dumps that
are used by a new test case.
[[[
Add a command line option (--source-encoding) to the svnsync init, sync, and
copy-revprops subcommands that allows the user to specify the character
encoding of translatable properties from the source repository. This is
needed
to allow svnsync to sync some older Subversion repositories that have
properties that were not encoded in UTF-8.
[...]
+1 to what danielsh said.
Seems to me it would be good for the name of the command-line option to
include property, e.g. --prop-source-encoding. I note that this
option is not intended to be used frequently so I don't imagine it's a
problem to have a long name.
Index: subversion/svnsync/sync.h
===
svnsync_normalize_revprops(apr_hash_t *rev_props,
- int *normalized_count,
+ int *normalized_le_count,
+ const char *encoding,
apr_pool_t *pool);
svnsync_get_sync_editor(const svn_delta_editor_t *wrapped_editor,
void *wrapped_edit_baton,
svn_revnum_t base_revision,
const char *to_url,
+const char *prop_encoding,
svn_boolean_t quiet,
const svn_delta_editor_t **editor,
void **edit_baton,
-int *normalized_node_props_counter,
+int *le_normalized_node_props_counter,
Can you use the same names for the counter parameters? (I realize one
is an output and the other is an in-out, but that doesn't appear to be
the reason why they differ.) Also we normally use EOL for end of
line; I haven't noticed the use of LE before.
Not directly related to this change, but ...
- int *normalized_node_props_counter; /* Where to count normalizations? */
+ int *le_normalized_node_props_counter; /* Where to count line ending
+ normalizations? */
} edit_baton_t;
@@ -407,9 +425,10 @@ change_file_prop(void *file_baton,
if (svn_prop_needs_translation(name))
{
svn_boolean_t was_normalized;
It would be a good idea to rename this variable also, otherwise this is
a bit confusing.
- SVN_ERR(normalize_string(value, was_normalized, pool));
+ SVN_ERR(normalize_string(value, was_normalized, eb-prop_encoding,
+ pool, pool));
if (was_normalized)
-(*(eb-normalized_node_props_counter))++;
+(*(eb-le_normalized_node_props_counter))++;
}
This kind of code can be simplified (removing one level of indirection)
by making the baton hold the actual counter instead of a pointer to it.
- Julian
Re: diff-optimizations-bytes: how to make diff3 work with prefix/suffix scanning
On Sat, 2011-01-01, Johan Corveleyn wrote:
[ Taking a privately-started discussion with danielsh to the list, in
case others have inspiration/insight about this. Question at hand: I'm
having trouble making diff3 work with prefix/suffix scanning of the
diff-optimizations-bytes branch. Any feedback is highly appreciated
:-). ]
On Fri, Dec 31, 2010 at 2:38 PM, Daniel Shahaf d...@daniel.shahaf.name
wrote:
[...snip...]
In diff3 with prefix enabled, I was seeing failures like this:
EXPECTED:
line1
line2
===
line2 modified
ACTUAL:
line1
line2
===
line1
line2 modified
so I assumed that when the prefix code gobbled the shared prefix. How
to fix it from there was purely guesswork on my part (and I haven't
implemented it).
These failures would go away if I prepended a line0 left and line0
right to the file.
Yes, I know. That's indeed because the prefix-scanning code gobbled up
the identical prefix. That problem is also there in diff2.
In diff2, I fixed this by simply pre-pending a piece of common diff
to the diff linked list, in the function diff.c#svn_diff__diff.
From r1037371:
[[[
--- subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff.c
(original)
+++ subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff.c
Sun Nov 21 02:36:07 2010
@@ -43,6 +43,22 @@ svn_diff__diff(svn_diff__lcs_t *lcs,
svn_diff_t *diff;
svn_diff_t **diff_ref = diff;
+ if (want_common (original_start 1))
+{
+ /* we have a prefix to skip */
+ (*diff_ref) = apr_palloc(pool, sizeof(**diff_ref));
+
+ (*diff_ref)-type = svn_diff__type_common;
+ (*diff_ref)-original_start = 0;
+ (*diff_ref)-original_length = original_start - 1;
+ (*diff_ref)-modified_start = 0;
+ (*diff_ref)-modified_length = modified_start - 1;
+ (*diff_ref)-latest_start = 0;
+ (*diff_ref)-latest_length = 0;
+
+ diff_ref = (*diff_ref)-next;
+}
Hi Johan.
I know this is work in progress but that whole if block of code is a
good example of where a comment above the block would be *really* useful
to tell the reader what it's for. :-)
(Also, all that dereferencing is unnecessary in that particular block,
but I looked at the rest of the function and saw that that style is used
in two other blocks where it is not unnecessary. Personally I would
find the following style more readable throughout the function:
{
svn_diff_t *new_diff = apr_palloc(pool, sizeof(*new_diff));
new_diff-type = ...
...
*diff_ref = new_diff;
diff_ref = new_diff-next;
}
)
while (1)
{
if (original_start lcs-position[0]-offset
]]]
Naively, I wanted to do the same for diff3 (along with some other
tricks, to make the algorithm aware of the prefix_lines), but it
doesn't work. See the patch in attachment.
I mean: it works partly: diff-diff3-test.exe passes with the attached
patch, but merge_reintegrate_tests.py (1, 2, 10, 13 and 14) and
merge_tests.py (61) don't. I haven't studied the test failures in
detail (I probably should, but I currently can't wrap my head around
those tests).
I'm now pondering another approach, to pre-pend an lcs piece to the
lcs linked list, and let the rest of the algorithm do its work as
normal. Inspiration for this came when I was reading the code of
diff3.c#svn_diff__resolve_conflict, where a similar trick is used
around line 121:
[[[
/* If there were matching symbols at the start of
* both sequences, record that fact.
*/
if (common_length 0)
{
lcs = apr_palloc(subpool, sizeof(*lcs));
lcs-next = NULL;
lcs-position[0] = start_position[0];
lcs-position[1] = start_position[1];
lcs-length = common_length;
lcs_ref = lcs-next;
}
]]]
Maybe I can even integrate this logic into lcs.c#svn_diff__lcs, where
I'm already passing the prefix_lines as a new parameter. If that would
work out, I could probably remove the special common-diff-injecting
code in diff.c#svn_diff__diff.
Thoughts?
Just from reading what you say here, that last solution sounds much
better.
- Julian
It will take me some time/experimentation to work out the details, but
I think that should work ...
Cheers,
Re: [PATCH] make diff against copy-source by default
On Thu, 2010-12-30, Prabhu Gnana Sundar wrote: [...] After a few discussions about the inconsistent behaviour of 'svn diff' in different scenarios, Kamesh suggested that let 'svn diff' be done against the copy-source by default, making use of the copyfrom info. Now this patch would do 'diff' against the copy-source by default, whereas the prevailing default behaviour(of showing a *modified copy* as all adds) can be achieved by using the '--show-copies-as-adds' switch(that already exists in the code-base!). I like this change, in principle. We will have to decide whether it is backwards-compatible enough or whether we need to provide an additional compatibility mode. In order to help us decide, it would be very useful if you could provide a summary of the behavioural changes. For example, maybe some tables something like this would be a good way to summarize the changes: Type of diff shown BEFORE this patch, WITHOUT --show-copies-as-adds: +---+---+--+---+ | | copied only | copied and modified | ... | +---+---+--+---+ | | | | | | WC-WC diff | | | | | | | | | | WC-repo diff | | | | | | | | | | repo-repo diff | | | | | | | | | +---+---+--+---+ Type of diff shown BEFORE this patch, WITH --show-copies-as-adds: +---+---+--+---+ Type of diff shown AFTER this patch, WITHOUT --show-copies-as-adds: +---+---+--+---+ Type of diff shown AFTER this patch, WITH --show-copies-as-adds: +---+---+--+---+ Or whatever rows and columns best convey the information. Are there any differences with different (old) versions of Subversion server? (I seem to recall that copy-from data was not always supplied, but I am not sure of the details.) I was quickly in to it and after spending some time in the process I came to know that several existing 'test cases' *failed* due to the change in the behaviour of the 'svn diff'. Later found that the 'wc editor' needs to be taught to handle this behaviour (handling the copyfrom info and retrieving it from the repository as against working copy text-base). This took quite some amount of time... See [1] below of the mailer.py usecase which tries to get the same output as my patch do. May be that's the behaviour of the 'diff' that is prefered ? :) I have attached the patch and the log message with this mail. Please review and respond. I only had a very quick look at the patch. The first thing that catches my eye is this: * subversion/libsvn_wc/diff.c (): included the svn_ra.h header file. (): persisted the ra_session, copyfrom file, copyfrom path and pristine properties in the edit_baton. (make_edit_baton) : introduced the ra_session. (get_file_from_ra): newly added to fetch file from repository. (add_file): support copyfrom by making use of the copyfrom info. (apply_textdelta) : makes use of the copyfrom_temp file making use of the copyfrom info. (close_file) : makes use of the copyfrom info and tweaked the file_changed to display the copyfrom revision number. (svn_wc_get_diff_editor6) : introduced the ra_session. pass NULL for ra_session to make_edit_baton. Isn't it a layering violation for libsvn_wc to know about libsvn_ra? Maybe this needs to use callbacks or something, so that all the RA knowledge remains in libsvn_client. - Julian
[PATCH] '--version --quiet' should display only the version info in svnadmin, svnlook, svnsync, svndumpfilter, svnversion, svnserve
Hi all,
Currently svnadmin, svnlook, svnsync, svndumpfilter, svnversion,
svnserve show the entire version information even if '--version --quiet'
is given as command.
The '--quiet' option has *not* been handled so far.
This patch would make svnadmin, svnlook, svnsync, svndumpfilter,
svnversion, svnserve '--version --quiet' to display *only* the version
number(info) just like 'svn --version --quiet' does.
I have attached the patch and the log message with this mail. Please
give your views on this.
Thanks and regards
Prabhu
[[[
svnadmin, svnlook, svnserve, svnversion, svnsync, svndumpfilter, svnrdump would now show the version documentation in quiet mode similar to 'svn'.
* subversion/svndumpfilter/main.c
(subcommand_help): now accepts the 'quiet' option.
(main): added a 'break' statement.
help subcommand handles the '--quiet' option along with '--version'.
* subversion/svnversion/main.c
(version): accepts the 'quiet' option and displays the version in quiet mode also.
(main) : added booleans 'quiet' and 'is_version'.
added 'quiet' to the 'options'.
handles the 'quiet' case.
* subversion/svnadmin/main.c
(subcommand_help): handles the 'quiet' option.
(main): help subcommand handles the '--quiet' option along with '--version'.
* subversion/svnlook/main.c
(): added 'quiet' to the 'options_table'.
(struct svnlook_opt_state): added boolean 'quiet'.
(subcommand_help): handles the 'quiet' option.
(main): handled the 'quiet' case.
help subcommand handles the '--quiet' option along with '--version'.
* subversion/svnsync/main.c
(help_cmd): handles the 'quiet' option.
(main): help subcommand handles the '--quiet' option along with '--version'.
* subversion/svnserve/main.c
(): added 'quiet' to the svnserve__options.
(version): now accepts 'quiet' argument and handles the 'quiet' option.
(main): added booleans 'quiet' and 'is_version'.
handles the 'quiet' case.
pass the 'quiet' option to version() function.
* subversion/svnrdump/svnrdump.c
(version): now accepts the 'baton' argument.
svn_opt_print_help3() function handles the 'quiet' option.
(main): help subcommand handles the '--quiet' option along with '--version'.
Patch by: Prabhu Gnana Sundar prabhugs{_AT_}collab.net
Suggested by: Kamesh Jayachandran kamesh{_AT_}collab.net
]]]
Index: subversion/svndumpfilter/main.c
===
--- subversion/svndumpfilter/main.c (revision 1055432)
+++ subversion/svndumpfilter/main.c (working copy)
@@ -1054,7 +1054,7 @@
SVN_ERR(svn_opt_print_help3(os, svndumpfilter,
opt_state ? opt_state-version : FALSE,
- FALSE, NULL,
+ opt_state ? opt_state-quiet : FALSE, NULL,
header, cmd_table, options_table, NULL,
NULL, pool));
@@ -1336,6 +1336,7 @@
break;
case svndumpfilter__version:
opt_state.version = TRUE;
+ break;
case svndumpfilter__quiet:
opt_state.quiet = TRUE;
break;
@@ -1385,6 +1386,7 @@
static const svn_opt_subcommand_desc2_t pseudo_cmd =
{ --version, subcommand_help, {0}, ,
{svndumpfilter__version, /* must accept its own option */
+ svndumpfilter__quiet,
} };
subcommand = pseudo_cmd;
Index: subversion/svnversion/main.c
===
--- subversion/svnversion/main.c (revision 1055432)
+++ subversion/svnversion/main.c (working copy)
@@ -32,9 +32,9 @@
static svn_error_t *
-version(apr_pool_t *pool)
+version(svn_boolean_t quiet, apr_pool_t *pool)
{
- return svn_opt_print_help3(NULL, svnversion, TRUE, FALSE, NULL, NULL,
+ return svn_opt_print_help3(NULL, svnversion, TRUE, quiet, NULL, NULL,
NULL, NULL, NULL, NULL, pool);
}
@@ -128,6 +128,8 @@
apr_getopt_t *os;
svn_node_kind_t kind;
svn_wc_context_t *wc_ctx;
+ svn_boolean_t quiet = FALSE;
+ svn_boolean_t is_version = FALSE;
const apr_getopt_option_t options[] =
{
{no-newline, 'n', 0, N_(do not output the trailing newline)},
@@ -135,6 +137,8 @@
{help, 'h', 0, N_(display this help)},
{version, SVNVERSION_OPT_VERSION, 0,
N_(show program version information)},
+ {quiet, 'q', 0,
+ N_(no progress (only errors) to stderr)},
{0, 0, 0, 0}
};
@@ -193,12 +197,14 @@
case 'c':
committed = TRUE;
break;
+case 'q':
+ quiet = TRUE;
+ break;
case 'h':
help(options, pool);
break;
case SVNVERSION_OPT_VERSION:
- SVN_INT_ERR(version(pool));
- exit(0);
+ is_version = TRUE;
RE: Any idea why public function like svn_fspath__dirname have double __ in its name?
On Thu, 2010-12-30, Kamesh Jayachandran wrote: While we've mandated that __ must be used for semi-public functions, we've never said that it can't be used for private ones. Kamesh is talking about public, not private, functions. I.e., ones where we actually do have an ABI promise to keep. I looked at svn_error__locate last week. It's really only useful in --enable-maintainer-mode, but the way it's implemented, it ends up being used by macros in the public (non-maintainer) ABI, so even if we eliminate those callers, we have to supply the function itself forever. I've addressed this as best we can, in r1053469. Thanks Supplying a given ABI forever is the sort of thing I thought we didn't have to do for __ functions. I think that's Kamesh's question too. Yes. Following is my understanding, correct me if I am wrong. my understanding Any function/macro inside the header *directly* under subversion/include should only have a public functions(ones that starts with svn_ and do *not* have __). If somebody differs from this convention then there should be *subversion dev* community independent convention here may be like these. * doc string of such functions(semi-public a.k.a intra library functions/private(should we even have such one in headers?)) should promptly say so in the headers. * start with __svn like we see in some of the libc headers. The vague __ starting would signal about the scope/ABI nature of the API. /my understanding I added the svn_fspath__* functions. My understanding and my intention is: * The double underscore means this function is not for public use and does not have compatibility guarantees even though it is physically exported. * Functions that are not for public use should normally be in 'subversion/include/private/*.h'. These functions are in 'subversion/include/svn_dirent_uri.h' because they are analogous to other functions there, but we could move them. With regards Kamesh Jayachandran [PS] Unless I am mistaken svn_fspath__* can be used in libsvn_repos too instead of svn_path_* wherever applicable and hence a chance to become public. I don't quite understand what you mean here. - Julian
Re: diff-optimizations-bytes: how to make diff3 work with prefix/suffix scanning
On Wed, Jan 5, 2011 at 12:11 PM, Julian Foad julian.f...@wandisco.com wrote:
On Sat, 2011-01-01, Johan Corveleyn wrote:
[ Taking a privately-started discussion with danielsh to the list, in
case others have inspiration/insight about this. Question at hand: I'm
having trouble making diff3 work with prefix/suffix scanning of the
diff-optimizations-bytes branch. Any feedback is highly appreciated
:-). ]
On Fri, Dec 31, 2010 at 2:38 PM, Daniel Shahaf d...@daniel.shahaf.name
wrote:
[...snip...]
In diff3 with prefix enabled, I was seeing failures like this:
EXPECTED:
line1
line2
===
line2 modified
ACTUAL:
line1
line2
===
line1
line2 modified
so I assumed that when the prefix code gobbled the shared prefix. How
to fix it from there was purely guesswork on my part (and I haven't
implemented it).
These failures would go away if I prepended a line0 left and line0
right to the file.
Yes, I know. That's indeed because the prefix-scanning code gobbled up
the identical prefix. That problem is also there in diff2.
In diff2, I fixed this by simply pre-pending a piece of common diff
to the diff linked list, in the function diff.c#svn_diff__diff.
From r1037371:
[[[
---
subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff.c
(original)
+++
subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff.c
Sun Nov 21 02:36:07 2010
@@ -43,6 +43,22 @@ svn_diff__diff(svn_diff__lcs_t *lcs,
svn_diff_t *diff;
svn_diff_t **diff_ref = diff;
+ if (want_common (original_start 1))
+ {
+ /* we have a prefix to skip */
+ (*diff_ref) = apr_palloc(pool, sizeof(**diff_ref));
+
+ (*diff_ref)-type = svn_diff__type_common;
+ (*diff_ref)-original_start = 0;
+ (*diff_ref)-original_length = original_start - 1;
+ (*diff_ref)-modified_start = 0;
+ (*diff_ref)-modified_length = modified_start - 1;
+ (*diff_ref)-latest_start = 0;
+ (*diff_ref)-latest_length = 0;
+
+ diff_ref = (*diff_ref)-next;
+ }
Hi Johan.
I know this is work in progress but that whole if block of code is a
good example of where a comment above the block would be *really* useful
to tell the reader what it's for. :-)
Ok, you're absolutely right. If I can't throw this block away (as
suggested below), I'll add some more comments.
(Also, all that dereferencing is unnecessary in that particular block,
but I looked at the rest of the function and saw that that style is used
in two other blocks where it is not unnecessary. Personally I would
find the following style more readable throughout the function:
{
svn_diff_t *new_diff = apr_palloc(pool, sizeof(*new_diff));
new_diff-type = ...
...
*diff_ref = new_diff;
diff_ref = new_diff-next;
}
)
Yep, that's definitely clearer. I just copy-pasted from the other
similar blocks in that function. But you're right.
Now, this style is also used in other similar places in diff3.c and
diff4.c. So if it's decided that it's best to change this to make it
more readable, I guess it should be changed in all those places
(preferably on trunk, it's not specific to the
diff-optimizations-bytes branch).
while (1)
{
if (original_start lcs-position[0]-offset
]]]
Naively, I wanted to do the same for diff3 (along with some other
tricks, to make the algorithm aware of the prefix_lines), but it
doesn't work. See the patch in attachment.
I mean: it works partly: diff-diff3-test.exe passes with the attached
patch, but merge_reintegrate_tests.py (1, 2, 10, 13 and 14) and
merge_tests.py (61) don't. I haven't studied the test failures in
detail (I probably should, but I currently can't wrap my head around
those tests).
I'm now pondering another approach, to pre-pend an lcs piece to the
lcs linked list, and let the rest of the algorithm do its work as
normal. Inspiration for this came when I was reading the code of
diff3.c#svn_diff__resolve_conflict, where a similar trick is used
around line 121:
[[[
/* If there were matching symbols at the start of
* both sequences, record that fact.
*/
if (common_length 0)
{
lcs = apr_palloc(subpool, sizeof(*lcs));
lcs-next = NULL;
lcs-position[0] = start_position[0];
lcs-position[1] = start_position[1];
lcs-length = common_length;
lcs_ref = lcs-next;
}
]]]
Maybe I can even integrate this logic into lcs.c#svn_diff__lcs, where
I'm already passing the prefix_lines as a new parameter. If that would
work out, I could probably remove the special common-diff-injecting
code in diff.c#svn_diff__diff.
Thoughts?
Just from reading what you say here, that last solution sounds much
better.
Yep. I hope to get this working soon. I did an attempt yesterday, but
ended up with causing an infinite loop somewhere/somehow, and gave up
around 2am :-). I'll
Re: [l10n] Translation status report for trunk r1055292
How is throwing cron jobs over a wall going to cause our translations to become written? There have been exactly 3 po commits this month, so I'm going to go ahead and filter these to /dev/null. Sorry, Daniel SVN DEV wrote on Wed, Jan 05, 2011 at 04:26:23 +: Translation status report for tr...@r1055292 lang trans untrans fuzzy obs -- de2053 126 239 199 es1990 189 273 338 fr2141 38 75 41 it1839 340 472 162 ja1982 197 344 584 ko2027 152 187 179 nb2043 136 197 307 pl2075 104 146 82 pt_BR1803 376 489 157 sv1759 420 513 163 zh_CN2175 4 11 88 zh_TW1739 440 551 225
Re: Bringing diff options of svnlook on par with svn?
Johan Corveleyn wrote on Wed, Jan 05, 2011 at 13:35:33 +0100: Hi, I'm wondering why svnlook doesn't support the same diff options (-x flags or others) as svn does. Is it just a question of tuits? Or there some specific reason why svnlook diff doesn't support '-x-p' (--show-c-function) and the new '--git'? If the answer is tuits, I'd like to give it a shot when I have some time. Rationale for my personal interest in this: we use svnlook diff for producing the diff for post-commit emails (still using a perl script, instead of mailer.py, because we have no python bindings installed). It's annoying that I can't use the -x-p option. While I'm there, it may be doable to also include the new --git option (don't know how much work etc). In post-commit you can use 'svn'. (The transaction had already been committed and the FS layer finished his part in the show by that point; see svn_repos_fs_commit_txn().) Only in pre-commit are you constrained to 'svnlook' (-t), and --- for example, considering the -x-p option --- I suspect no one ever needed to use -x-p from pre-commit. I'm not sure that explanation covers all the missing options, but at least some of them. Cheers, -- Johan
Re: [l10n] Translation status report for trunk r1055292
By the way, as I said on wayita: +1 to Hyrum for heading the translation efforts; I am annoyed at having to receive mails which *for me* are information-free (because I follow the commits@ list), not at the larger picture of someone finally trying to make our translations move forward. Daniel (but I'll solve by problem by installing mail filtering on my end) Daniel Shahaf wrote on Wed, Jan 05, 2011 at 14:44:08 +0200: How is throwing cron jobs over a wall going to cause our translations to become written? There have been exactly 3 po commits this month, so I'm going to go ahead and filter these to /dev/null. Sorry, Daniel SVN DEV wrote on Wed, Jan 05, 2011 at 04:26:23 +: Translation status report for tr...@r1055292 lang trans untrans fuzzy obs -- de2053 126 239 199 es1990 189 273 338 fr2141 38 75 41 it1839 340 472 162 ja1982 197 344 584 ko2027 152 187 179 nb2043 136 197 307 pl2075 104 146 82 pt_BR1803 376 489 157 sv1759 420 513 163 zh_CN2175 4 11 88 zh_TW1739 440 551 225
Re: svn commit: r1055462 - /subversion/trunk/subversion/tests/cmdline/dav-mirror-autocheck.sh
On 01/05/2011 09:01 AM, julianf...@apache.org wrote: Author: julianfoad Date: Wed Jan 5 14:01:56 2011 New Revision: 1055462 URL: http://svn.apache.org/viewvc?rev=1055462view=rev Log: * subversion/tests/cmdline/dav-mirror-autocheck.sh Set C locale so as to run all tests in a known locale. A follow-up to r1055065 which did the same for davautocheck.sh and svnserveautocheck.sh. Well, there goes all the fun of chasing random environment-specific test failures. Thanks aLOT, Julian. ;-) -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: svn commit: r1055462 - /subversion/trunk/subversion/tests/cmdline/dav-mirror-autocheck.sh
On Wed, 2011-01-05 at 09:05 -0500, C. Michael Pilato wrote: On 01/05/2011 09:01 AM, julianf...@apache.org wrote: Author: julianfoad Date: Wed Jan 5 14:01:56 2011 New Revision: 1055462 URL: http://svn.apache.org/viewvc?rev=1055462view=rev Log: * subversion/tests/cmdline/dav-mirror-autocheck.sh Set C locale so as to run all tests in a known locale. A follow-up to r1055065 which did the same for davautocheck.sh and svnserveautocheck.sh. Well, there goes all the fun of chasing random environment-specific test failures. Thanks aLOT, Julian. ;-) Heh. Actually, thanks to Philip for debugging and making the fix. I just copied it. - Julian
Re: [PATCH] Remove redundant member skip_descendants from libsvn_wc/update_editor.c
Philip Martin philip.mar...@wandisco.com writes: I'll take a look at your patch, it looks like a useful step. Committed in r1055463. Thanks! -- Philip
Re: [PATCH] Inconsistent behavior in cat command
Noorul Islam K M noo...@collab.net writes:
Julian Foad julian.f...@wandisco.com writes:
On Thu, 2010-12-02, Noorul Islam K M wrote:
Noorul Islam K M noo...@collab.net writes:
When I was trying to come up with a patch for issue 3713, I observed the
following.
For example I have two files 1.txt and 2.txt in a repository located at
file:///tmp/testrepo
svn cat behaves differently for local paths and URLs. See the
illustration below.
noo...@noorul:/tmp/wc/testrepo$ svn cat 1.txt 2.txt
1
2
A) Local non-existent target followed by existing target
noo...@noorul:/tmp/wc/testrepo$ svn cat 3.txt 1.txt
svn: warning: '/tmp/wc/testrepo/3.txt' is not under version control
1
B) Non-existent URL followed by existing URL
noo...@noorul:/tmp/wc/testrepo$ svn cat ^/3.txt ^/1.txt
svn: File not found: revision 1, path '/3.txt'
In case A, even though the first target was non-existent it performs cat
operation on the second target but in the case of B, svn errors out at
the first failure itself.
I am not sure about behavior of other svn commands which accepts
multiple targets. When I discussed this Julian in IRC, he said a
discussion is needed to come up with standardized behavior across svn
commands. Any thoughts?
Any updates?
Hi Noorul.
A good way to start a discussion of this sort is to:
list the possible solutions;
find out and describe how the other subcommands behave;
I checked 'svn info' and it behaves the same way for both wc and URL.
say what you think is good and bad about each possible solution;
say which solution you think we should choose.
That will make it much easier for readers to respond.
I figured out that for non-existent URL path, svn_client_cat2 returns
SVN_ERR_FS_NOT_FOUND which needs to be caught by svn_cl__try. Therefore
I passed this code as one the arguments.
Attached is the patch. All tests pass with this patch.
Log
[[[
Make 'svn cat' not to error out when one of the URL targets do not
exist.
* subversion/svn/cat-cmd.c
(svn_cl__cat): Pass SVN_ERR_FS_NOT_FOUND to svn_cl__try in order to
catch the error, print warning and proceed with other targets.
Patch by: Noorul Islam K M noorul{_AT_}collab.net
]]]
Thanks and Regards
Noorul
Index: subversion/svn/cat-cmd.c
===
--- subversion/svn/cat-cmd.c (revision 1053010)
+++ subversion/svn/cat-cmd.c (working copy)
@@ -78,6 +78,7 @@
SVN_ERR_UNVERSIONED_RESOURCE,
SVN_ERR_ENTRY_NOT_FOUND,
SVN_ERR_CLIENT_IS_DIRECTORY,
+ SVN_ERR_FS_NOT_FOUND,
SVN_NO_ERROR));
}
svn_pool_destroy(subpool);
Initially this thread was monitored by Julian and now I found out that
he is busy. It will be great if someone else can take a look at this
one.
Thanks and Regards
Noorul
Re: svn commit: r1055452 - /subversion/trunk/tools/po/l10n-report.py
On 05.01.2011 14:00, danie...@apache.org wrote:
Author: danielsh
Date: Wed Jan 5 13:00:07 2011
New Revision: 1055452
URL: http://svn.apache.org/viewvc?rev=1055452view=rev
Log:
Add some helpful headers to l10-report.py's output.
* tools/po/l10n-report.py
(svn:keywords): Set to 'all'.
(_rev): New helper.
(main): Include a couple of extra headers.
Modified:
subversion/trunk/tools/po/l10n-report.py (contents, props changed)
Modified: subversion/trunk/tools/po/l10n-report.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/tools/po/l10n-report.py?rev=1055452r1=1055451r2=1055452view=diff
==
--- subversion/trunk/tools/po/l10n-report.py (original)
+++ subversion/trunk/tools/po/l10n-report.py Wed Jan 5 13:00:07 2011
@@ -1,5 +1,6 @@
#!/usr/bin/env python
#
+# $Id$
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
@@ -39,6 +40,10 @@ import os
import re
import subprocess
+def _rev():
+ dollar = $Revision$
+ return int(re.findall('[0-9]+', dollar)[0]);
+
def usage_and_exit(errmsg=None):
Print a usage message, plus an ERRMSG (if provided), then exit.
If ERRMSG is provided, the usage message is printed to stderr and
@@ -185,9 +190,16 @@ def main():
email_to = To: %s % to_email_id
email_sub = Subject: [l10n] Translation status report for %s r%s \
% (branch_name, wc_version)
+x_headers = .join([
+ X-Mailer: l10n-report.py r%ld\n % _rev(),
+ Reply-To: d...@subversion.apache.org\n,
+ Mail-Followup-To: d...@subversion.apache.org\n,
+ #
http://www.iana.org/assignments/auto-submitted-keywords/auto-submitted-keywords.xhtml
+ Auto-Submitted: auto-generated\n,
+];
-msg = %s\n%s\n%s\n%s\n%s\n%s\n%s % (email_from, email_to,\
- email_sub, title, format_head, format_line, body)
+msg = \n.join((email_from, email_to, email_sub, x_headers,
+title, format_head, format_line, body))
server.sendmail(email_from, email_to, msg)
print(The report is sent to '%s' email id. % to_email_id)
RFC822 messages should properly have CRLF line endings, so I'd suggest
\r\n.join(...) for both the headers and the body.
-- Brane
Re: Diff optimizations and generating big test files
Johan Corveleyn jcor...@gmail.com writes: Thanks for the script, it gives me some good inspiration. However, it doesn't fit well with the optimization that's currently being done on the diff-optimizations-bytes branch, because the differing lines are spread throughout the entire file. I thought you were working on two different prefix problems, but if it's all the same problem that's fine. It's why I want *you* to write the script, then I can test your patches on my machine. When you are thinking of replacing function calls with macros that's very much hardware/OS/compiler specific and testing on more than one platform is important. -- Philip
Re: [PATCH] svn info - changing hard coded error message to specific one.
On Sat, 2010-12-25, Noorul Islam K M wrote:
Julian Foad julian.f...@wandisco.com writes:
On Fri, 2010-12-24, C. Michael Pilato wrote:
On 12/23/2010 07:27 AM, Julian Foad wrote:
From IRC:
Bert julianf: We changed error codes in libsvn_wc all over the place.
I don't think we see strict error codes as part of the documented
behavior, unless it is in the function documentation
I agree with this. Any time a specific error code is intended for use by
an
API function as a specific message to the caller, we document that fact and
that behavior becomes part of the contract. Undocumented error codes are
fair game for changing over time.
OK, then that's fine by me too.
Noorul wrote:
Daniel Shahaf wrote:
Why do you have to place the svn: prefix here explicitly?
Normally
svn_handle_error2() would do that for you. This is a red flag (is
a wheel being reinvented here?) for me.
We are actually consuming the error here to print it and proceed with
the other targets.
Noorul, svn_handle_error2 has a fatal flag that controls whether it
terminates the program or not; setting fatal=FALSE would enable you to
continue processing the other targets. But it prints the error stack
which is not what you want here - we want just a single error message.
Try using svn_handle_warning2 instead. That function appears to do
almost exactly what you want here; the only differences I can see is it
inserts warning: before the message, which I think is perfect for
this usage, and it doesn't print the extra \n, which is easily
rectified.
Please find updated patch attached. I used svn_handle_warning2 with
svn: as prefix because it is used in this fashion everywhere. All
tests pass with this patch.
Thanks, Noorul.
I see that this patch contains two logically separate changes:
1. Fix the regression that svn info has stopped skipping unversioned
paths, which is caused by WC-NG producing a new error code.
2. Display the specific error message from the API instead of a generic
(Not a valid URL) or (Not a versioned resource).
I split your patch into two, and committed part 1 in r1055484 and part 2
in r1055494.
Index: subversion/svn/info-cmd.c
===
--- subversion/svn/info-cmd.c (revision 1051915)
+++ subversion/svn/info-cmd.c (working copy)
@@ -566,29 +566,14 @@
{
/* If one of the targets is a non-existent URL or wc-entry,
don't bail out. Just warn and move on to the next target. */
- if (err-apr_err == SVN_ERR_UNVERSIONED_RESOURCE
- || err-apr_err == SVN_ERR_ENTRY_NOT_FOUND)
+ if (err-apr_err == SVN_ERR_WC_PATH_NOT_FOUND ||
+ err-apr_err == SVN_ERR_RA_ILLEGAL_URL)
{
- SVN_ERR(svn_cmdline_fprintf
- (stderr, subpool,
- _(%s: (Not a versioned resource)\n\n),
- svn_path_is_url(truepath)
- ? truepath
- : svn_dirent_local_style(truepath, pool)));
+ svn_handle_warning2(stderr, err, svn: );
+ svn_error_clear(svn_cmdline_fprintf(stderr, subpool, \n));
}
- else if (err-apr_err == SVN_ERR_RA_ILLEGAL_URL)
-{
- SVN_ERR(svn_cmdline_fprintf
- (stderr, subpool,
- _(%s: (Not a valid URL)\n\n),
- svn_path_is_url(truepath)
- ? truepath
- : svn_dirent_local_style(truepath, pool)));
-}
else
-{
return svn_error_return(err);
-}
I kept those braces, for symmetry with the if clause.
svn_error_clear(err);
err = NULL;
- Julian
Re: Any idea why public function like svn_fspath__dirname have double __ in its name?
With regards Kamesh Jayachandran [PS] Unless I am mistaken svn_fspath__* can be used in libsvn_repos too instead of svn_path_* wherever applicable and hence a chance to become public. I don't quite understand what you mean here. I meant subversion/libsvn_repos/commit.c:delete_entry() uses deprecated svn_path_join which can be made to use svn_fspath__join. If my guess is true, we can as well make svn_fspath__join as svn_fspath_join and make it public as it is used by 2 modules(libsvn_fs and libsvn_repos). With regards Kamesh Jayachandran
Re: [oss-security] CVE request for subversion
- Original Message - On Tue, Jan 4, 2011 at 10:02 AM, Jan Lieskovsky jlies...@redhat.com wrote: Hello Kurt, Josh, vendors, Josh Bressers wrote: - Original Message - Unspecified vulnerability in the server component in Apache Subversion 1.6.x before 1.6.15 allows remote attackers to cause a denial of service via unknown vectors, related to a several bug fixes, including two which can cause client-initiated crashes on the server. [1] http://svn.haxx.se/dev/archive-2010-11/0475.shtml Cc-ed Hyrum to shed more light into this one. [1] mentions two issues: begin quote ... several bug fixes, including two which can cause client-initiated crashes on the server. /end quote Further look at: [2] http://svn.apache.org/repos/asf/subversion/tags/1.6.15/CHANGES suggest: A, * prevent crash in mod_dav_svn when using SVNParentPath (r1033166) being the first one. Upstream changeset: http://svn.apache.org/viewvc?view=revisionrevision=1033166 and after discussion with Joe Orton, Joe suggested: B, * fix server-side memory leaks triggered by 'blame -g' (r1032808) References: http://svn.haxx.se/dev/archive-2010-11/0102.shtml Upstream changeset: http://svn.apache.org/viewvc?view=revisionrevision=1032808 being the second one as denial of service attack (by memory consumption) against svnserve. Questions: -- Hyrum, could you confirm A, and B, issues are those two, mentioned in [2] to be able to cause client-initiated crashes on the server? I can confirm that A and B are the two issues mentioned in [2]. I admit, this isn't obvious, so let's use CVE-2010-4539 for now. We can split it if needed once more information is known. Josh, since CVE-2010-4539 was assigned. Once Hyrum confirms, can we consider CVE-2010-4539 to be a CVE identifier for A, issue and request yet another / second one for B, issue? We didn't initially reserve CVEs for these vulnerabilities, but will be happy to update our documentation to reflect them. (See http://subversion.apache.org/security/ ) The two issues really are orthogonal, so B should probably not be included in a CVE for A. I've CC'd dev@subversion.apache.org to help coordinate advisory authoring. OK, let's split the CVE id then. So for A, * prevent crash in mod_dav_svn when using SVNParentPath (r1033166) Upstream changeset: http://svn.apache.org/viewvc?view=revisionrevision=1033166 Let's use CVE-2010-4539. For B, * fix server-side memory leaks triggered by 'blame -g' (r1032808) References: http://svn.haxx.se/dev/archive-2010-11/0102.shtml Upstream changeset: http://svn.apache.org/viewvc?view=revisionrevision=1032808 Let's use CVE-2010-4644. Thanks. -- JB
Re: Any idea why public function like svn_fspath__dirname have double __ in its name?
On 01/05/2011 11:05 AM, Kamesh Jayachandran wrote: With regards Kamesh Jayachandran [PS] Unless I am mistaken svn_fspath__* can be used in libsvn_repos too instead of svn_path_* wherever applicable and hence a chance to become public. I don't quite understand what you mean here. I meant subversion/libsvn_repos/commit.c:delete_entry() uses deprecated svn_path_join which can be made to use svn_fspath__join. If my guess is true, we can as well make svn_fspath__join as svn_fspath_join and make it public as it is used by 2 modules(libsvn_fs and libsvn_repos). Our public API is no longer defined by what needs to be used by more than one module. It's defined by what functionality is required by or believed to be generally useful to third-party consumers of our libraries. Sometimes a function is used only inside a single module. We keep it module-private in that case. Sometimes a function is used by multiple modules, but isn't of any general utility to third-party consumers. We put it into include/private, so our own modules can use it, but it isn't part of our published public API (which has versioning promises and guarantees, etc.). (The svn_fspath__* functions might very well be examples of such functions.) And sometimes a function is used by one or more modules, but is clearly useful to third-party consumers of the Subversion libraries. Those functions live in include/ proper, and make up our public API. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: [PATCH] support printing changed properties in svnlook
Hi,
I have now redone the patch to fix some of the comments given in this
thread and I'm attaching the current working state of it so that we
can have something real to discuss around. This does not include any
changes to svnlook but have a test case to verify that it works as it
should (which it at the moment doesn't, see 10 below).
Further more, I've tried to enumerate all cases that I could think of
were properties are changed and how they should be handled. For some
of the cases I've changed my mind compared to what I've said in the
rest of the thread. Hopefully this is a complete list.
mod_props is the per node hash with properties (see svn_repos_node2_t).
1. Add node w/o props = mod_props is NULL
2. Add node w/ props = mod_props contains all props
Pretty straight-forward; mod_props contains what can be expected and
action is A for all props.
3. Delete node w/o props = mod_props is NULL
4. Delete node w/ props = mod_props is NULL
For case 4 it could be argued that mod_props should contain all props
that the node had prior to being deleted. First I thought it should,
but after thinking some more I now have the opinion that it isn't of
any real use. The node is gone after all, and it doesn't matter if the
node had e.g. svn:eol-style set or not.
5. Change (add, delete or modify) node props = mod_props contains all
changed props
Also straight-forward. Action is A, D or M depending on type of
change. Properties that haven't changed are not included in mod_props.
6. Copy (i.e. add-with-history) node w/o props = mod_props is NULL
7. Copy node w/ props = mod_props is NULL
Case 7 is somewhat similar to case 4 in that one could argue that
mod_props should contain all props that were copied with the node. But
as nothing has been changed and text_mod and prop_mod are both FALSE
in this case I think that mod_props should be NULL.
8. Copy node w/ changed props [1] = mod_props contains all changed props
In this case one or several properties are added, deleted or modified
between coping the node and committing. Unmodified props are not
included in mod_props (case 7).
9. Replace (without history) node (w/ or w/o props) with node w/o
props = mod_props is NULL
We don't include any props from the deleted node (case 3 and 4) and no
props are added for the new node (case 1).
10. Replace node (w/ or w/o props) with node w/ props = mod_props
contains all props
We don't include any props from the deleted node (case 3 and 4) but
new props are added with the new node (case 2). The test for this
fails as new props are marked as M if they existed in the deleted
node. Don't know a good solution for this yet. Suggestions welcome.
11. Replace (with history) node (w/ or w/o props) with copied node w/
or w/o props = mod_props is NULL
We don't include any props from the deleted node (case 3 and 4) nor
any props from the unmodified copy (case 6 and 7).
12. Replace node (w/ or w/o props) with copied node w/ modified props
= mod_props contains all modified props
We don't include any props from the deleted node (case 3 and 4) but
the modified props from the copied node (case 8).
// Erik
--
Erik Johansson
Home Page: http://ejohansson.se/
PGP Key: http://ejohansson.se/erik.asc
Index: subversion/tests/libsvn_repos/repos-test.c
===
--- subversion/tests/libsvn_repos/repos-test.c (revision 1055565)
+++ subversion/tests/libsvn_repos/repos-test.c (working copy)
@@ -367,6 +367,53 @@
}
+/* Debug helper */
+static void
+node_tree_print(const svn_repos_node2_t *node,
+int indent,
+apr_pool_t *pool)
+{
+ char action;
+
+ if (!node)
+return;
+
+ /* Make bubble-up nodes have action 'r'. */
+ action = node-action;
+ if (action == 'R' ! node-text_mod ! node-prop_mod)
+action = 'r';
+
+ printf([%c]%*c%s: (copy %s:%ld) [%s mods text:%d prop:%d]\n,
+ action, indent + 1, ' ', node-name,
+ (node-copyfrom_path ? node-copyfrom_path : none),
+ node-copyfrom_rev,
+ svn_node_kind_to_word(node-kind),
+ node-text_mod, node-prop_mod);
+
+ if (node-mod_props)
+{
+ apr_hash_index_t *hi;
+ for (hi = apr_hash_first(pool, node-mod_props); hi;
+ hi = apr_hash_next(hi))
+{
+ const void *key;
+ void *val;
+ svn_repos_node_prop_t *prop;
+
+ apr_hash_this(hi, key, NULL, val);
+ prop = val;
+ printf( %*c[%c] %s (copy %d)\n,
+ indent + 1 + 2, ' ', prop-action,
+ (const char *)key,
+ prop-action == 'M' node-copyfrom_path);
+}
+}
+
+ node_tree_print(node-child, indent + 2, pool);
+ node_tree_print(node-sibling, indent, pool);
+}
+
+
static svn_error_t *
node_tree_delete_under_copy(const svn_test_opts_t *opts,
apr_pool_t *pool)
@@ -448,6 +495,182 @@
}
+static svn_error_t *
+node_tree_props(const svn_test_opts_t *opts,
+
problem with svnsync and repository locks...
I have a master server, and a slave server configured with pass-thru proxy. Off the top of my head, I believe they're both 1.6.12, but I'll double check if that is an important detail. A user at the slave site does get lock on a file. She gets the lock successfully. She makes a change, tries to commit. Commit fails because file is locked in repository. (What? Yeah.) She asked me for help, and I ensured she did NOT lock in one WC and then try to commit from another WC. All of these operations are happening in a single WC, using the slave server for the URL. She tries to unlock. Cannot unlock because file is not locked. She tries to lock. File is already locked. On another computer, I try to lock her file. Cannot lock - lock belongs to her. (I did not force steal the lock.) I try to unlock her file. Cannot unlock, file is not locked. I double-checked the revs of the master slave. Both matching (we are not waiting for an in-progress svnsync to finish from master to slave.) The workaround was this: I made a connection directly to the master and forced the unlock. Then she was able to commit. Clearly, the presence of a repository lock is not properly communicated between master slave. I double-checked my master server configuration, and ensured there is both a post-commit hook, and a post-revprop-change hook. Both of which have been working flawlessly for months. If necessary, I can reproduce this in a precisely documented way. But I didn't document it that thoroughly yet because I didn't think that's necessarily necessary. Is this simply a bug that was accidentally overlooked in the master/slave design? Is it a known issue?
