On 27/12/12 17:24, Daniel Shahaf wrote:>> /* Acquire a lock (of sorts)
on the repository associated with the
>> * given RA SESSION. This lock is just a revprop change attempt in a
>> - * time-delay loop. This function is duplicated by svnrdump in
>> - * load_editor.c.
>> + * time-delay loop. A similar function used by svnrdump is defined in
>> + * svnrdump/load_editor.c
>
> Why did you s/duplicate/similar/? I think the former is better (for its
> "if you change me, change that other one too" implication).
>
> That's all. All in all I'm not sure whether to just apply it or wait
> for another iteration, I'll go for the latter. Thanks for the patch!
>
> Daniel
>
Hi Daniel,
Second iteration attached. I've copied the indentation style from one
of your commit messages.
With regard to the get_lock() functions, my reading was that the two
functions now had different arguments and different behaviour - to my
mind, in that case, "duplicate" isn't an appropriate description.
The relevant function bodies are attached at the bottom of this mail
for general convenience.
Apart from the arguments, the differences appear to be cosmetic until
the calls to svn_ra__get_operational_lock() where each call has
arguments has unique arguments provided. I haven't investigated the
significance of this difference.
Of course, the fact that I think this comment is improved by rewording
doesn't make it so - feel free to make the final call on this.
Gabriela
--
/* From subversion/svnrdump/load_editor.c */
static svn_error_t *
get_lock(const svn_string_t **lock_string_p,
svn_ra_session_t *session,
svn_cancel_func_t cancel_func,
void *cancel_baton,
apr_pool_t *pool)
{
svn_boolean_t be_atomic;
SVN_ERR(svn_ra_has_capability(session, &be_atomic,
SVN_RA_CAPABILITY_ATOMIC_REVPROPS,
pool));
if (! be_atomic)
{
/* Pre-1.7 servers can't lock without a race condition. (Issue
#3546) */
svn_error_t *err =
svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
_("Target server does not support atomic
revision "
"property edits; consider upgrading it to
1.7."));
svn_handle_warning2(stderr, err, "svnrdump: ");
svn_error_clear(err);
}
return svn_ra__get_operational_lock(lock_string_p, NULL, session,
SVNRDUMP_PROP_LOCK, FALSE,
10 /* retries */,
lock_retry_func, NULL,
cancel_func, cancel_baton, pool);
}
/* From subversion/svnsync/svnsync.c */
static svn_error_t *
get_lock(const svn_string_t **lock_string_p,
svn_ra_session_t *session,
svn_boolean_t steal_lock,
apr_pool_t *pool)
{
svn_error_t *err;
svn_boolean_t be_atomic;
const svn_string_t *stolen_lock;
SVN_ERR(svn_ra_has_capability(session, &be_atomic,
SVN_RA_CAPABILITY_ATOMIC_REVPROPS,
pool));
if (! be_atomic)
{
/* Pre-1.7 server. Can't lock without a race condition.
See issue #3546.
*/
err = svn_error_create(
SVN_ERR_UNSUPPORTED_FEATURE, NULL,
_("Target server does not support atomic revision property "
"edits; consider upgrading it to 1.7 or using an external "
"locking program"));
svn_handle_warning2(stderr, err, "svnsync: ");
svn_error_clear(err);
}
err = svn_ra__get_operational_lock(lock_string_p, &stolen_lock, session,
SVNSYNC_PROP_LOCK, steal_lock,
10 /* retries */, lock_retry_func,
NULL,
check_cancel, NULL, pool);
if (!err && stolen_lock)
{
return svn_cmdline_printf(pool,
_("Stole lock previously held by '%s'\n"),
stolen_lock->data);
}
return err;
}
Index: subversion/tests/svn_test_main.c
===================================================================
--- subversion/tests/svn_test_main.c (revision 1426182)
+++ subversion/tests/svn_test_main.c (working copy)
@@ -1,5 +1,5 @@
/*
- * tests-main.c: shared main() & friends for SVN test-suite programs
+ * svn_tests_main.c: shared main() & friends for SVN test-suite programs
*
* ====================================================================
* Licensed to the Apache Software Foundation (ASF) under one
Index: subversion/svndumpfilter/svndumpfilter.c
===================================================================
--- subversion/svndumpfilter/svndumpfilter.c (revision 1426182)
+++ subversion/svndumpfilter/svndumpfilter.c (working copy)
@@ -1,5 +1,5 @@
/*
- * main.c: Subversion dump stream filtering tool.
+ * svndumpfilter.c: Subversion dump stream filtering tool main file.
*
* ====================================================================
* Licensed to the Apache Software Foundation (ASF) under one
Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1426182)
+++ subversion/svn/cl.h (working copy)
@@ -288,13 +288,13 @@ svn_opt_subcommand_t
svn_cl__upgrade;
-/* See definition in main.c for documentation. */
+/* See definition in svn.c for documentation. */
extern const svn_opt_subcommand_desc2_t svn_cl__cmd_table[];
-/* See definition in main.c for documentation. */
+/* See definition in svn.c for documentation. */
extern const int svn_cl__global_options[];
-/* See definition in main.c for documentation. */
+/* See definition in svn.c for documentation. */
extern const apr_getopt_option_t svn_cl__options[];
Index: subversion/svn/svn.c
===================================================================
--- subversion/svn/svn.c (revision 1426182)
+++ subversion/svn/svn.c (working copy)
@@ -1,5 +1,5 @@
/*
- * main.c: Subversion command line client.
+ * svn.c: Subversion command line client main file.
*
* ====================================================================
* Licensed to the Apache Software Foundation (ASF) under one
Index: subversion/svnadmin/svnadmin.c
===================================================================
--- subversion/svnadmin/svnadmin.c (revision 1426182)
+++ subversion/svnadmin/svnadmin.c (working copy)
@@ -1,5 +1,5 @@
/*
- * main.c: Subversion server administration tool.
+ * svnadmin.c: Subversion server administration tool main file.
*
* ====================================================================
* Licensed to the Apache Software Foundation (ASF) under one
Index: subversion/svnlook/svnlook.c
===================================================================
--- subversion/svnlook/svnlook.c (revision 1426182)
+++ subversion/svnlook/svnlook.c (working copy)
@@ -1,5 +1,5 @@
/*
- * main.c: Subversion server inspection tool.
+ * svnlook.c: Subversion server inspection tool main file.
*
* ====================================================================
* Licensed to the Apache Software Foundation (ASF) under one
Index: subversion/svnsync/svnsync.c
===================================================================
--- subversion/svnsync/svnsync.c (revision 1426182)
+++ subversion/svnsync/svnsync.c (working copy)
@@ -329,8 +329,8 @@ lock_retry_func(void *baton,
/* Acquire a lock (of sorts) on the repository associated with the
* given RA SESSION. This lock is just a revprop change attempt in a
- * time-delay loop. This function is duplicated by svnrdump in
- * load_editor.c.
+ * time-delay loop. A similar function used by svnrdump is defined in
+ * svnrdump/load_editor.c
*/
static svn_error_t *
get_lock(const svn_string_t **lock_string_p,
Index: subversion/svnserve/svnserve.c
===================================================================
--- subversion/svnserve/svnserve.c (revision 1426182)
+++ subversion/svnserve/svnserve.c (working copy)
@@ -1,5 +1,5 @@
/*
- * main.c : Main control function for svnserve
+ * svnserve.c : Main control function for svnserve
*
* ====================================================================
* Licensed to the Apache Software Foundation (ASF) under one
Index: subversion/svnrdump/load_editor.c
===================================================================
--- subversion/svnrdump/load_editor.c (revision 1426182)
+++ subversion/svnrdump/load_editor.c (working copy)
@@ -501,7 +501,8 @@ get_shim_callbacks(struct revision_baton *rb,
/* Acquire a lock (of sorts) on the repository associated with the
* given RA SESSION. This lock is just a revprop change attempt in a
- * time-delay loop. This function is duplicated by svnsync in main.c.
+ * time-delay loop. A similar function used by svnsync is defined in
+ * svnsync/svnsync.c
*
* ### TODO: Make this function more generic and
* expose it through a header for use by other Subversion
[[[
Update code comments in a number of files which refer to "main.c", where
the file has been renamed.
Update to comments in subversion/svnsync/svnsync.c and
subversion/svnrdump/load_editor.c which referred to the function
(get_lock) being duplicated, where the prototypes differ slightly.
* subversion/svnrdump/load_editor.c:
(get_lock): Changed comment to refer to "similar" function.
in subversion/svnsync/svnsync.c
Changed comment to fix reference to "main.c
* subversion/svnsync/svnsync.c:
(get_lock): Changed comment to refer to "similar" function.
in subversion/svnsync/svnsync.c
* subversion/tests/svn_test_main.c:
(Opening comment): File name fixed
* subversion/svndumpfilter/svndumpfilter.c:
(Opening comment): File name fixed
* subversion/svn/cl.h:
(svn_cl__cmd_table[]): File name fixed in comment
(svn_cl__global_options[]): File name fixed in comment
(apr_getopt_option_t svn_cl__options[]): File name fixed in comment
* subversion/svn/svn.c:
(Opening comment): File name fixed
* subversion/svnadmin/svnadmin.c:
(Opening comment): File name fixed
* subversion/svnlook/svnlook.c:
(Opening comment): File name fixed
* subversion/svnserve/svnserve.c:
(Opening comment): File name fixed
]]]
