You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2012/03/30 15:44:54 UTC

Re: svn commit: r1292246 - /subversion/trunk/subversion/libsvn_repos/hooks.c

Bert,

rhuijben@apache.org wrote on Wed, Feb 22, 2012 at 11:43:35 -0000:
> Author: rhuijben
> Date: Wed Feb 22 11:43:34 2012
> New Revision: 1292246
> 
> URL: http://svn.apache.org/viewvc?rev=1292246&view=rev
> Log:
> Revert r1240999. This patch causes an unexpected behavior change on Windows,
> where before this patch the %PATH% variable was still available for hooks.
> 
> This unbreaks the svnlook tests on the Windows-RA buildbot.
> 

Does the patch below result in pre-r1240999 behaviour on Windows?  (The
mod_dav_svn part is needed anyway to make INHERIT_VALUE(hooks_env) be
able to return either the parent's or the child's hooks_env.)

[[[
Index: subversion/mod_dav_svn/mod_dav_svn.c
===================================================================
--- subversion/mod_dav_svn/mod_dav_svn.c	(revision 1307158)
+++ subversion/mod_dav_svn/mod_dav_svn.c	(working copy)
@@ -195,7 +195,7 @@ create_dir_config(apr_pool_t *p, char *dir)
     conf->root_dir = svn_urlpath__canonicalize(dir, p);
   conf->bulk_updates = CONF_FLAG_ON;
   conf->v2_protocol = CONF_FLAG_ON;
-  conf->hooks_env = apr_hash_make(p);
+  conf->hooks_env = NULL;
 
   return conf;
 }
@@ -543,6 +543,9 @@ SVNHooksEnv_cmd(cmd_parms *cmd, void *config, cons
       dir_conf_t *conf = config;
       const char *name;
       const char *val;
+
+      if (! conf->hooks_env)
+        conf->hooks_env = apr_hash_make(cmd->pool);
 
       name = apr_pstrdup(apr_hash_pool_get(conf->hooks_env),
                          APR_ARRAY_IDX(var, 0, const char *));
Index: subversion/libsvn_repos/hooks.c
===================================================================
--- subversion/libsvn_repos/hooks.c	(revision 1307158)
+++ subversion/libsvn_repos/hooks.c	(working copy)
@@ -172,7 +172,7 @@ env_from_env_hash(apr_hash_t *env_hash,
   const char **env;
   const char **envp;
 
-  if (!env_hash || apr_hash_count(env_hash) == 0)
+  if (!env_hash)
     return NULL;
 
   env = apr_palloc(result_pool,
]]]

> After this patch Apr cleans the entire environment and svnlook.exe is unable
> to find shared libraries like libapr-1.dll.
> 
> I think apr should be fixed to behave the same way on Windows as on unix
> and we -as subversion project- should check if removing %PATH% on Windows
> is a breaking change.
> 
> * subversion/libsvn_repos/hooks.c
>   (env_from_env_hash): Revert to returning NULL for a NULL hash.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_repos/hooks.c
> 
> Modified: subversion/trunk/subversion/libsvn_repos/hooks.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/hooks.c?rev=1292246&r1=1292245&r2=1292246&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/hooks.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/hooks.c Wed Feb 22 11:43:34 2012
> @@ -172,7 +172,7 @@ env_from_env_hash(apr_hash_t *env_hash,
>    const char **env;
>    const char **envp;
>    
> -  if (!env_hash)
> +  if (!env_hash || apr_hash_count(env_hash) == 0)
>      return NULL;
>  
>    env = apr_palloc(result_pool,
> 
> 

Re: svn commit: r1292246 - /subversion/trunk/subversion/libsvn_repos/hooks.c

Posted by Daniel Shahaf <da...@elego.de>.
Committed in rr1307479 after IRC discussion.  See also
http://subversion.tigris.org/issues/show_bug.cgi?id=4113#desc3

Daniel Shahaf wrote on Fri, Mar 30, 2012 at 16:44:54 +0300:
> Bert,
> 
> rhuijben@apache.org wrote on Wed, Feb 22, 2012 at 11:43:35 -0000:
> > Author: rhuijben
> > Date: Wed Feb 22 11:43:34 2012
> > New Revision: 1292246
> > 
> > URL: http://svn.apache.org/viewvc?rev=1292246&view=rev
> > Log:
> > Revert r1240999. This patch causes an unexpected behavior change on Windows,
> > where before this patch the %PATH% variable was still available for hooks.
> > 
> > This unbreaks the svnlook tests on the Windows-RA buildbot.
> > 
> 
> Does the patch below result in pre-r1240999 behaviour on Windows?  (The
> mod_dav_svn part is needed anyway to make INHERIT_VALUE(hooks_env) be
> able to return either the parent's or the child's hooks_env.)

Re: svn commit: r1292246 - /subversion/trunk/subversion/libsvn_repos/hooks.c

Posted by Daniel Shahaf <da...@elego.de>.
Committed in rr1307479 after IRC discussion.  See also
http://subversion.tigris.org/issues/show_bug.cgi?id=4113#desc3

Daniel Shahaf wrote on Fri, Mar 30, 2012 at 16:44:54 +0300:
> Bert,
> 
> rhuijben@apache.org wrote on Wed, Feb 22, 2012 at 11:43:35 -0000:
> > Author: rhuijben
> > Date: Wed Feb 22 11:43:34 2012
> > New Revision: 1292246
> > 
> > URL: http://svn.apache.org/viewvc?rev=1292246&view=rev
> > Log:
> > Revert r1240999. This patch causes an unexpected behavior change on Windows,
> > where before this patch the %PATH% variable was still available for hooks.
> > 
> > This unbreaks the svnlook tests on the Windows-RA buildbot.
> > 
> 
> Does the patch below result in pre-r1240999 behaviour on Windows?  (The
> mod_dav_svn part is needed anyway to make INHERIT_VALUE(hooks_env) be
> able to return either the parent's or the child's hooks_env.)