ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peter Reilly" <peter.kitt.rei...@gmail.com>
Subject Re: svn commit: r446979 - in /ant/core/trunk: CONTRIBUTORS WHATSNEW contributors.xml src/main/org/apache/tools/ant/taskdefs/Execute.java
Date Mon, 18 Sep 2006 14:17:12 GMT
Hi DD,
Thanks for the comments, it is always nice for people
to check the code - I mentioned that this patch was
the one that I was a little worried about.

1)
The code does not export the lower case version
of the env variable that the user passed in, the
lower case version is only used for comparision
purposes.
2)
Windows in case-insensitive as regards env names,
however before this patch, it was very easy to have
multiple env settings corresponding to the same name placed in
the env list passed to the subprocess.

I have done some testing with this change,
for example:
<project default="java" xmlns:ac="antlib: net.sf.antcontrib">
  <!-- hide the silliness of cmd -->
  <presetdef name="cmdshell">
    <ac:shellscript shell="cmd.exe" tmpsuffix=".bat">
      <arg value="/c"/>
      <arg value="call"/>
    </ac:shellscript>
  </presetdef>
  <target name="java">
    <property environment="env"/>
    <echo>Path is ${ env.PATH}</echo>
    <java classname="org.apache.tools.ant.launch.Launcher"
          classpath="${ant.home}/lib/ant-launcher.jar"
          fork="yes">
      <env key="PaTh" value="C:/apps;${env.PATH}"/>
      <arg value="shell"/>
    </java>
  </target>
  <target name="shell">
    <cmdshell>
      echo %PATH%
      ls
    </cmdshell>
  </target>
</project>

The one thing that is infurating is that "Path" can be "PATH" or "path"
or whatever is the whim of the  windows machine,  and
it is difficult to write an ant script that work correctly
when using <property environment="env"/>,
perhaps we could provide a touppercase=yes/no option to
<property environment="env">, or provide
a "${getenv:} prefix to ${} expansion to correspond
to java(1.1-, 5+) System.getenv(key).
Note that getenv() is case-insensitive:
  <target name="beanshell">
    <script language="beanshell">
      System.out.println(System.getenv ("pAth"));
    </script>
  </target>

Please try out the case-insensentive version of
<exec><env></exec>

One change that could be made would be
to ignore the casiness of the key in the <env key="">
and use the casiness of the key in the current env, it is present.


Peter

On 9/18/06, Dominique Devienne <ddevienne@gmail.com> wrote:
>
> On 9/16/06, peterreilly@apache.org <peterreilly@apache.org> wrote:
> > bugzilla 28874: make env case insensitive for windows
> > @@ -624,9 +627,17 @@
> >         for (int i = 0; i < env.length; i++) {
> >             // Get key including "="
> >             String key = env[i].substring(0, env[i].indexOf('=') + 1);
> > +            if (environmentCaseInSensitive) {
> > +                // Nb: using default locale as key is a env name
> > +                key = key.toLowerCase();
> > +            }
> >             int size = osEnv.size();
> >             for (int j = 0; j < size; j++) {
> > -                if (((String) osEnv.elementAt(j)).startsWith(key)) {
> > +                String osEnvItem = (String) osEnv.elementAt(j);
> > +                if (environmentCaseInSensitive) {
> > +                    osEnvItem = osEnvItem.toLowerCase();
> > +                }
> > +                if (osEnvItem.startsWith(key)) {
> >                     osEnv.removeElementAt(j);
> >                     break;
> >                 }
>
> Peter, can you please explain this patch a bit? I've also been bitten
> by the case of (PATH | Path | path) on Windows in the past, but solved
> it by assigning the Path's current env key and value to 2 properties,
> to ensure I was using the same case when assigning it for a
> sub-process.
>
> This is different than forcing env keys to be lower-case
> (case-insensitive), and that worries me a bit franckly... In my
> experience, the path *must* be assigned using the same case used by
> the Ant process, and going to all lower-case doesn't do that. This is
> critical when forking any process (<java>, <junit>, <exec>) that
> depends on Path (or PATH, or path) to be set properly to run.
>
> Thanks for any insight on this. Right-now I worried about this. --DD
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message