Re: FSFS locks questions and a concern
C. Michael Pilato wrote on Fri, Nov 12, 2010 at 09:40:49 -0500: On 11/11/2010 04:25 PM, Daniel Shahaf wrote: Philip observed today that, since 'hotcopy' copies the $REPOS/db/locks/???/* files using a simple svn_io_copy_dir_recursively(), the correctness of the copy is not guaranteed if locks are being added/removed while the hotcopy is ongoing: the hotcopy might contain only an arbitrary subset of the path-prefix-digest-files. (It might contain any subset of the three digest files md5(/), md5(/trunk/iota), md5(/trunk). The lock removal removes/updates those files in depth-first order.) So, a few questions: Maybe, just maybe, you can find some of the information you seek in the history of http://subversion.tigris.org/issues/show_bug.cgi?id=3660. I had to study the FSFS lock storage and usage algorithms some time ago to fix a bug. (Though, don't expect to find hotcopy strategies there.) Indeed, I don't find hotcopy strategies there, but I do find there discussion on whether the FSFS locks directory should, in each non-immediate ancestor of a locked file, a pointer to that file's digest file or a pointer to the next-ancestor-in-the-chain (i.e., /trunk to /trunk/A)'s digest file. I guess this answers my original point (2) by It has always been this way. Following if it ain't broken, don't fix it, I'll leave this bit untouched until somebody complains (that 'svn lock' takes too long). Thanks for the pointer, Daniel
Re: FSFS locks questions and a concern
Philip Martin wrote on Fri, Nov 12, 2010 at 11:02:14 +: Daniel Shahaf d...@daniel.shahaf.name writes: 3. What's a reasonable strategy to hotcopy the locks? I've started on a patch that uses walk_digest_files() (I modified that function today), but currently that patch simply does a walk starting at the digest file md5(/), and consequently it misses copying the unreferenced digest file md5(/trunk). It appears you have to read/copy md5(/) and get a list of locks, then read/copy each lock file that still exists, then reconstruct the intermediate files for the locks that were copied. So, for example: - read md5(/) * read md5(/trunk/A/mu) + compute md5(/trunk/A) + compute md5(/trunk) * read md5(/trunk/iota) + compute md5(/trunk) where the 'compute()' steps follow those of normal lock creation: read the digest file, update list of children, write it back. -- Philip
Re: issue 3486 should be reopened?
Peter Parker wrote on Sat, Nov 13, 2010 at 05:55:06 +0100: Hello, I still have still issues with fixed issue: #3486 (svn: Attempt to add tree conflict that already exists) See attached script for provoking this error. With trunk, I get: [[[ + /home/daniel/src/svn/trunk/subversion/svn/svn merge '^/Source/branches/3.0' -r5:9 subversion/libsvn_client/merge.c:8923: (apr_err=195016) svn: Cannot merge into mixed-revision working copy [7:8]; try updating first ]]] and when I add an '$svn up' before the '$svn merge', I get: [[[ + /home/daniel/src/svn/trunk/subversion/svn/svn merge '^/Source/branches/3.0' -r5:9 --- Merging r6 into '.': C dir/foo.c Summary of conflicts: Tree conflicts: 1 subversion/libsvn_wc/tree_conflicts.c:403: (apr_err=155016) svn: Attempt to add tree conflict that already exists at '/tmp/sandbox-scripts/wc/Source/trunk/dir/foo.c' ]]] Attached the non-windows version of your script I used for testing. Daniel Same results with webdav except you get additional error: svn: Attempt to add tree conflict that already exists at 'dir\foo.c' svn: Error reading spooled REPORT request response I used subversion 1.6.13 on windows (script should be easily adapted to unix(change line 2 and 8)) Biggest problem is the bail out on merge leaving all other merges unprocessed. Best regards, Peter
Re: FSFS locks questions and a concern
Daniel Shahaf wrote on Sat, Nov 13, 2010 at 14:08:12 +0200:
Philip Martin wrote on Fri, Nov 12, 2010 at 11:02:14 +:
Daniel Shahaf d...@daniel.shahaf.name writes:
3. What's a reasonable strategy to hotcopy the locks?
I've started on a patch that uses walk_digest_files() (I modified
that function today), but currently that patch simply does a walk
starting at the digest file md5(/), and consequently it misses
copying the unreferenced digest file md5(/trunk).
It appears you have to read/copy md5(/) and get a list of locks, then
read/copy each lock file that still exists, then reconstruct the
intermediate files for the locks that were copied.
So, for example:
- read md5(/)
* read md5(/trunk/A/mu)
+ compute md5(/trunk/A)
+ compute md5(/trunk)
* read md5(/trunk/iota)
+ compute md5(/trunk)
where the 'compute()' steps follow those of normal lock creation: read
the digest file, update list of children, write it back.
Attached patch implements this. Comments?
http://people.apache.org/~danielsh/hot.logmsg
http://people.apache.org/~danielsh/hot.diff
[[[
FSFS: teach 'hotcopy' the semantics of the locks/ directory.
Found by: philip
* notes/fsfs:
Document another limitation of rsync-style backup, which 'hotcopy' lacks.
* subversion/libsvn_fs_fs/fs_fs.c
(svn_fs_fs__hotcopy):
Use svn_fs_fs__hotcopy_locks() instead of svn_io_copy_dir_recursively().
* subversion/libsvn_fs_fs/lock.c
(svn_fs_fs__hotcopy_locks): New declaration.
* subversion/libsvn_fs_fs/lock.c
(digest_dir_path_from_digest):
New, like svn_dirent_dirname(digest_path_from_digest()).
(hotcopy_walker): New callback.
(svn_fs_fs__hotcopy_locks): New definition.
]]]
[[[
Index: notes/fsfs
===
--- notes/fsfs (revision 1034745)
+++ notes/fsfs (working copy)
@@ -248,6 +248,11 @@ manually removed. svnadmin hotcopy does not cop
transactions from an FSFS repository, although that might need to
change if Subversion starts making use of long-lived transactions.
+Naively copying an FSFS repository might also copy an inconsistent view
+of the locks directory, if locks are being added or removed as the copy
+operation is running. svnadmin hotcopy copies and generates the locks
+digest files properly.
+
So, if you are using standard backup tools to make backups of a FSFS
repository, configure the software to copy the current file before
the numbered revision files, if possible, and configure it not to copy
Index: subversion/libsvn_fs_fs/fs_fs.c
===
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1034745)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -1767,9 +1767,7 @@ svn_fs_fs__hotcopy(const char *src_path,
src_subdir = svn_dirent_join(src_path, PATH_LOCKS_DIR, pool);
SVN_ERR(svn_io_check_path(src_subdir, kind, pool));
if (kind == svn_node_dir)
-SVN_ERR(svn_io_copy_dir_recursively(src_subdir, dst_path,
-PATH_LOCKS_DIR, TRUE, NULL,
-NULL, pool));
+SVN_ERR(svn_fs_fs__hotcopy_locks(src_path, dst_path, pool));
/* Now copy the node-origins cache tree. */
src_subdir = svn_dirent_join(src_path, PATH_NODE_ORIGINS_DIR, pool);
Index: subversion/libsvn_fs_fs/lock.c
===
--- subversion/libsvn_fs_fs/lock.c (revision 1034756)
+++ subversion/libsvn_fs_fs/lock.c (working copy)
@@ -121,6 +121,19 @@ err_corrupt_lockfile(const char *fs_path, const ch
/*** Digest file handling functions. ***/
+/* Return the path of the lock/entries directory for which DIGEST is the
+ hashed repository relative path. */
+static const char *
+digest_dir_path_from_digest(const char *fs_path,
+const char *digest,
+apr_pool_t *pool)
+{
+ return svn_dirent_join_many(pool, fs_path, PATH_LOCKS_DIR,
+ apr_pstrmemdup(pool, digest, DIGEST_SUBDIR_LEN),
+ NULL);
+}
+
+
/* Return the path of the lock/entries file for which DIGEST is the
hashed repository relative path. */
static const char *
@@ -893,6 +906,65 @@ unlock_body(void *baton, apr_pool_t *pool)
}
+/*** Hotcopying locks. ***/
+
+/* Implements walk_digests_callback_t. */
+static svn_error_t *
+hotcopy_walker(void *baton,
+ const char *fs_path,
+ const char *digest_path,
+ apr_hash_t *children /* ignored */,
+ svn_lock_t *lock /* ignored */,
+ svn_boolean_t have_write_lock,
+ apr_pool_t *pool)
+{
+ const char *dst_fs_path = baton;
+ const char *digest;
+ const char *src_digest_dir, *dst_digest_dir;
+ const char *perms_reference = digest_path;
+
+ digest = svn_dirent_basename(digest_path,
Re: issue 3486 should be reopened?
Daniel Shahaf wrote on Sat, Nov 13, 2010 at 14:35:13 +0200: Attached the non-windows version of your script I used for testing. [[[ #!/bin/sh -ex rm -rf repo wc REPO_PATH=$(pwd)/repo $svnadmin create repo $svn co file://$REPO_PATH wc pushd wc mkdir -p Server/trunk/dir echo x Server/trunk/dir/foo.c echo x Server/trunk/dir/bar.c $svn add Server $svn commit -m 1 $svn move Server Source $svn commit -m 2 mkdir -p Source/branches $svn add Source/branches $svn commit -m 3 $svn copy Source/trunk Source/branches/3.0 $svn commit -m 4 $svn delete Source/trunk/dir/foo.c $svn commit -m 5 # few checkins echo xxx Source/branches/3.0/dir/foo.c $svn commit -m 6 echo xxx Source/branches/3.0/dir/bar.c $svn commit -m 7 # merge down bar.c only pushd Source/trunk # wtf - 6:7 is exclusive on the left side? -r 7 or -r 7:7 does nothing $svn merge ^/Source/branches/3.0 -r 6:7 $svn up $svn commit -m 8 popd echo xxx Source/branches/3.0/dir/foo.c $svn commit -m 9 pushd Source/trunk test -n $1 $svn up $svn merge ^/Source/branches/3.0 -r5:9 popd popd ]]]
[Proposed] Split very long messages by paragraph for easy translate
Hi folks, subversion.pot have some very long translated message, for example: Apply a patch to a working copy.\n usage: patch PATCHFILE [WCPATH]\n \n Apply a unidiff patch in PATCHFILE to the working copy WCPATH.\n If WCPATH is omitted, '.' is assumed.\n \n A unidiff patch suitable for application to a working copy can be\n produced with the 'svn diff' command or third-party diffing tools.\n Any non-unidiff content of PATCHFILE is ignored.\n \n Changes listed in the patch will either be applied or rejected.\n If a change does not match at its exact line offset, it may be applied\n earlier or later in the file if a match is found elsewhere for the\n surrounding lines of context provided by the patch.\n A change may also be applied with fuzz, which means that one\n or more lines of context are ignored when matching the change.\n If no matching context can be found for a change, the change conflicts\n and will be written to a reject file with the extension .svnpatch.rej.\n \n For each patched file a line will be printed with characters reporting\n the action taken. These characters have the following meaning:\n \n A Added\n D Deleted\n U Updated\n C Conflict\n G Merged (with local uncommitted changes)\n \n Changes applied with an offset or fuzz are reported on lines starting\n with the '' symbol. You should review such changes carefully.\n \n If the patch removes all content from a file, that file is scheduled\n for deletion. If the patch creates a new file, that file is scheduled\n for addition. Use 'svn revert' to undo deletions and additions you\n do not agree with.\n From the translator's point of view, this very hard for translate and maintain. So I proposed we should split these long message like mercurial. -- Dongsheng
Re: [Proposed] Split very long messages by paragraph for easy translate
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) ? Dongsheng Song wrote on Sat, Nov 13, 2010 at 23:18:08 +0800: Hi folks, subversion.pot have some very long translated message, for example: Apply a patch to a working copy.\n usage: patch PATCHFILE [WCPATH]\n \n Apply a unidiff patch in PATCHFILE to the working copy WCPATH.\n If WCPATH is omitted, '.' is assumed.\n \n A unidiff patch suitable for application to a working copy can be\n produced with the 'svn diff' command or third-party diffing tools.\n Any non-unidiff content of PATCHFILE is ignored.\n \n Changes listed in the patch will either be applied or rejected.\n If a change does not match at its exact line offset, it may be applied\n earlier or later in the file if a match is found elsewhere for the\n surrounding lines of context provided by the patch.\n A change may also be applied with fuzz, which means that one\n or more lines of context are ignored when matching the change.\n If no matching context can be found for a change, the change conflicts\n and will be written to a reject file with the extension .svnpatch.rej.\n \n For each patched file a line will be printed with characters reporting\n the action taken. These characters have the following meaning:\n \n A Added\n D Deleted\n U Updated\n C Conflict\n G Merged (with local uncommitted changes)\n \n Changes applied with an offset or fuzz are reported on lines starting\n with the '' symbol. You should review such changes carefully.\n \n If the patch removes all content from a file, that file is scheduled\n for deletion. If the patch creates a new file, that file is scheduled\n for addition. Use 'svn revert' to undo deletions and additions you\n do not agree with.\n From the translator's point of view, this very hard for translate and maintain. So I proposed we should split these long message like mercurial. -- Dongsheng
Re: [Proposed] Split very long messages by paragraph for easy translate
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.)
Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 3)
The only difference between translate_newline() and translate_newline_alt()
is the lines of code from here to the end of the function. I think you
could keep translate_newline() as is, and just move these checks elsewhere
(to a new function or to the caller; I haven't thought about this in
detail).
In any case, please avoid the code duplication if possible. (Function
pointers are nice, but sometimes just a way to shoot yourself in the
foot.) If you're unsure what approach to take, do brainstorm on-list
before sending a new iteration of the patch. (That saves time to you
and to us)
After thinking about this problem some more, I thought about using a
macro solution. I define a macro that is able to output the
definitions of the two variants of translate_newline():
translate_newline() and translate_newline_alt(). This way, duplicate
code is avoided, but it is like having duplicate code where each copy
is slightly different.
Do you like this idea? I have attached C code that I have tested. In
this sample, the macro that outputs the definitions of
translate_newline() and translate_newline_alt() is called
DEFINE_TRANSLATE_NEWLINE_FN.
/* A boolean expression that evaluates to true if the string STR, having length
STR_LEN, is one of the end-of-line strings LF, CR, or CRLF; to false
otherwise. */
#define STRING_IS_EOL(str, str_len) (((str_len) == 2 \
(str)[0] == '\r' \
(str)[1] == '\n') || \
((str_len) == 1 \
((str)[0] == '\n' || (str)[0] == '\r')))
/* A boolean expression that evaluates to true if the end-of-line string EOL1,
having length EOL1_LEN, and the end-of-line string EOL2, having length
EOL2_LEN, are different, assuming that EOL1 and EOL2 are both from the
set {\n, \r, \r\n}; to false otherwise.
Given that EOL1 and EOL2 are either \n, \r, or \r\n, then if
EOL1_LEN is not the same as EOL2_LEN, then EOL1 and EOL2 are of course
different. If EOL1_LEN and EOL2_LEN are both 2 then EOL1 and EOL2 are both
\r\n. Otherwise, EOL1_LEN and EOL2_LEN are both 1. We need only check the
one character for equality to determine whether EOL1 and EOL2 are the same
in that case. */
#define DIFFERENT_EOL_STRINGS(eol1, eol1_len, eol2, eol2_len) \
(((eol1_len) != (eol2_len)) || (*(eol1) != *(eol2)))
static svn_error_t *
translate_newline_alt(const char *eol_str,
apr_size_t eol_str_len,
char *src_format,
apr_size_t *src_format_len,
const char *newline_buf,
apr_size_t newline_len,
svn_stream_t *dst,
struct translation_baton *b);
#define DEFINE_TRANSLATE_NEWLINE_FN(fn, is_alt)\
static svn_error_t * \
fn(const char *eol_str,\
apr_size_t eol_str_len, \
char *src_format, \
apr_size_t *src_format_len, \
const char *newline_buf,\
apr_size_t newline_len, \
svn_stream_t *dst, \
struct translation_baton *b)\
{ \
SVN_ERR_ASSERT(STRING_IS_EOL(eol_str, eol_str_len)); \
SVN_ERR_ASSERT(STRING_IS_EOL(newline_buf, newline_len)); \
\
/* If we've seen a newline before, compare it with our cache to \
check for consistency, else cache it for future comparisons. */ \
if (*src_format_len) \
{ \
/* Comparing with cache. If we are inconsistent and \
we are NOT repairing the file, generate an error! */ \
if ((! b-repair) \
((*src_format_len != newline_len) || \
(strncmp(src_format, newline_buf, newline_len \
return svn_error_create(SVN_ERR_IO_INCONSISTENT_EOL, NULL, NULL); \
} \
else
