[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] lock and unlock hooks completeness

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Mon, 21 Jul 2008 14:34:39 -0400

"Chia-liang Kao" <clkao_at_clkao.org> writes:
> Karl,
>
> Thanks for the review. attached is the revised patch.

+1 to commit. But in the log message, make sure to write out each
symbol. That is, don't do "svn_repos__hooks_pre_{lock,unlock}", instead
write both names out in full, so future searchers can find them.

Oh, and in the new doc strings, can you please specify whether the new
(char *) arguments can be NULL, and what happens if they are? I realize
that some of the existing arguments don't document this, and that's a
problem, but we should avoid adding to that problem.

Thanks,
-Karl

> 2008/7/16 Karl Fogel <kfogel_at_red-bean.com>:
>> [I'm sending this through the GMail interface due to some lossage with my
>> regular smtp server; I hope the formatting comes through okay...]
>>
>> "Chia-liang Kao" <clkao_at_clkao.org> writes:
>>> [[[
>>>Make pre-lock hook take comment and steal_lock as arguments, and
>>> make pre-unlock take token and break_lock as arguments.
>>>
>>> * subversion/libsvn_repos/hooks.c
>>> (svn_repos__hooks_pre_lock): takes comment and steal_lock, pass
>>> to run_hook_cmd.
>>> (svn_repos__hooks_pre_unlock): takes token and break_lock, pass
>>> to run_hook_cmd.
>>>
>>> * subversion/libsvn_repos/repos.h: Adjust prototype for
>>> svn_repos__hooks_pre_{lock,unlock}
>>>
>>> * subversion/libsvn_repos/fs-wrap.c:
>>> (svn_repos_fs_lock, svn_repos_fs_unlock): Call
>>> svn_repos__hooks_pre_{lock,unlock} with the above mentioned
>>> new arguments.
>>>
>>> * subversion/libsvn_repos/repos.c: Document the new arguments for
>>> the hooks in their templates.
>>> ]]]
>
> === subversion/libsvn_repos/fs-wrap.c
> ==================================================================
> --- subversion/libsvn_repos/fs-wrap.c (revision 32139)
> +++ subversion/libsvn_repos/fs-wrap.c (local)
> @@ -467,7 +467,8 @@
>
> /* Run pre-lock hook. This could throw error, preventing
> svn_fs_lock() from happening. */
> - SVN_ERR(svn_repos__hooks_pre_lock(repos, path, username, pool));
> + SVN_ERR(svn_repos__hooks_pre_lock(repos, path, username, comment,
> + steal_lock, pool));
>
> /* Lock. */
> SVN_ERR(svn_fs_lock(lock, repos->fs, path, token, comment, is_dav_comment,
> @@ -511,7 +512,8 @@
>
> /* Run pre-unlock hook. This could throw error, preventing
> svn_fs_unlock() from happening. */
> - SVN_ERR(svn_repos__hooks_pre_unlock(repos, path, username, pool));
> + SVN_ERR(svn_repos__hooks_pre_unlock(repos, path, username, token,
> + break_lock, pool));
>
> /* Unlock. */
> SVN_ERR(svn_fs_unlock(repos->fs, path, token, break_lock, pool));
> === subversion/libsvn_repos/hooks.c
> ==================================================================
> --- subversion/libsvn_repos/hooks.c (revision 32139)
> +++ subversion/libsvn_repos/hooks.c (local)
> @@ -768,6 +768,8 @@
> svn_repos__hooks_pre_lock(svn_repos_t *repos,
> const char *path,
> const char *username,
> + const char *comment,
> + svn_boolean_t steal_lock,
> apr_pool_t *pool)
> {
> const char *hook = svn_repos_pre_lock_hook(repos, pool);
> @@ -779,13 +781,15 @@
> }
> else if (hook)
> {
> - const char *args[5];
> + const char *args[7];
>
> args[0] = hook;
> args[1] = svn_path_local_style(svn_repos_path(repos, pool), pool);
> args[2] = path;
> args[3] = username;
> - args[4] = NULL;
> + args[4] = comment ? comment : "";
> + args[5] = steal_lock ? "1" : "0";
> + args[6] = NULL;
>
> SVN_ERR(run_hook_cmd(SVN_REPOS__HOOK_PRE_LOCK, hook, args, NULL, pool));
> }
> @@ -837,6 +841,8 @@
> svn_repos__hooks_pre_unlock(svn_repos_t *repos,
> const char *path,
> const char *username,
> + const char *token,
> + svn_boolean_t break_lock,
> apr_pool_t *pool)
> {
> const char *hook = svn_repos_pre_unlock_hook(repos, pool);
> @@ -848,13 +854,15 @@
> }
> else if (hook)
> {
> - const char *args[5];
> + const char *args[7];
>
> args[0] = hook;
> args[1] = svn_path_local_style(svn_repos_path(repos, pool), pool);
> args[2] = path;
> args[3] = username ? username : "";
> - args[4] = NULL;
> + args[4] = token ? token : "";
> + args[5] = break_lock ? "1" : "0";
> + args[6] = NULL;
>
> SVN_ERR(run_hook_cmd(SVN_REPOS__HOOK_PRE_UNLOCK, hook, args, NULL,
> pool));
> === subversion/libsvn_repos/repos.c
> ==================================================================
> --- subversion/libsvn_repos/repos.c (revision 32139)
> +++ subversion/libsvn_repos/repos.c (local)
> @@ -537,6 +537,8 @@
> "# [1] REPOS-PATH (the path to this repository)" NL
> "# [2] PATH (the path in the repository about to be locked)" NL
> "# [3] USER (the user creating the lock)" NL
> +"# [4] COMMENT (the comment of the lock)" NL
> +"# [5] STEAL-LOCK (1 if the user is trying to steal the lock, else 0)" NL
> "#" NL
> "# The default working directory for the invocation is undefined, so" NL
> "# the program should set one explicitly if it cares." NL
> @@ -618,6 +620,8 @@
> "# [1] REPOS-PATH (the path to this repository)" NL
> "# [2] PATH (the path in the repository about to be unlocked)" NL
> "# [3] USER (the user destroying the lock)" NL
> +"# [4] TOKEN (the lock token to be destoryed)" NL
> +"# [5] BREAK-UNLOCK (1 if the user is breaking the lock, else 0)" NL
> "#" NL
> "# The default working directory for the invocation is undefined, so" NL
> "# the program should set one explicitly if it cares." NL
> === subversion/libsvn_repos/repos.h
> ==================================================================
> --- subversion/libsvn_repos/repos.h (revision 32139)
> +++ subversion/libsvn_repos/repos.h (local)
> @@ -220,11 +220,15 @@
> /* Run the pre-lock hook for REPOS. Use POOL for any temporary
> allocations. If the hook fails, return SVN_ERR_REPOS_HOOK_FAILURE.
>
> - PATH is the path being locked, USERNAME is the person doing it. */
> + PATH is the path being locked, USERNAME is the person doing it,
> + COMMENT is the comment of the lock, and STEAL-LOCK is a flag if the
> + user is stealing the lock. */
> svn_error_t *
> svn_repos__hooks_pre_lock(svn_repos_t *repos,
> const char *path,
> const char *username,
> + const char *comment,
> + svn_boolean_t steal_lock,
> apr_pool_t *pool);
>
> /* Run the post-lock hook for REPOS. Use POOL for any temporary
> @@ -241,11 +245,15 @@
> /* Run the pre-unlock hook for REPOS. Use POOL for any temporary
> allocations. If the hook fails, return SVN_ERR_REPOS_HOOK_FAILURE.
>
> - PATH is the path being unlocked, USERNAME is the person doing it. */
> + PATH is the path being unlocked, USERNAME is the person doing it,
> + TOKEN is the lock token to be unlocked, and BREAK-LOCK is a flag if
> + the user is breaking the lock. */
> svn_error_t *
> svn_repos__hooks_pre_unlock(svn_repos_t *repos,
> const char *path,
> const char *username,
> + const char *token,
> + svn_boolean_t break_lock,
> apr_pool_t *pool);
>
> /* Run the post-unlock hook for REPOS. Use POOL for any temporary
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-21 20:35:04 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.