You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Chia-liang Kao <cl...@clkao.org> on 2008/07/16 17:05:33 UTC
[PATCH] lock and unlock hooks completeness
[[[
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.
]]]
Re: [PATCH] lock and unlock hooks completeness
Posted by Karl Fogel <kf...@red-bean.com>.
"Chia-liang Kao" <cl...@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 <kf...@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" <cl...@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@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] lock and unlock hooks completeness
Posted by Chia-liang Kao <cl...@clkao.org>.
Karl,
Thanks for the review. attached is the revised patch.
2008/7/16 Karl Fogel <kf...@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" <cl...@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.
>> ]]]
Re: [PATCH] lock and unlock hooks completeness
Posted by Karl Fogel <kf...@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" <cl...@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,7 @@
>
> /* 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 +511,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));
> + fprintf(stderr, ".... %s\n", token);
> + SVN_ERR(svn_repos__hooks_pre_unlock(repos, path, username, token, break_lock, pool));
Looks good to me, except the lines are now longer than 80 columns (add
line breaks).
> /* 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 (if the user is trying to steal the lock, 1 or 0)" NL
> "#" NL
A better way to say this would be:
"1 if the user is trying to steal the lock, else 0"
(Formally, what you wrote means that the value can be 1 or 0 if the user
is trying to steal the lock, otherwise it can be... whatever :-) I'm
sure most readers would figure it out, but let's not make them work.)
> "# 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 (if the user is breaking the lock, 1 or 0)" NL
> "#" NL
Same point applies here.
> === subversion/libsvn_repos/repos.h
> ==================================================================
> --- subversion/libsvn_repos/repos.h (revision 32139)
> +++ subversion/libsvn_repos/repos.h (local)
> @@ -225,6 +225,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);
>
> /* Run the post-lock hook for REPOS. Use POOL for any temporary
> @@ -246,6 +248,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);
Need to update their doc strings in libsvn_repos/repos.h too.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org