Re: svn commit: r603856 - in /lucene/java/trunk/contrib/benchmark: ./ src/java/org/apache/lucene/benchmark/byTask/feeds/

[Erik Hatcher <er...@ehatchersolutions.com>]

On Dec 13, 2007, at 12:58 AM, doronc@apache.org wrote:
> +12/13/07
> +  LUCENE-1086: DocMakers setup for the "docs.dir" property
> +  fixed to properly handle absolute paths. (Shai Erera via Doron  
> Cohen)
> +

I haven't looked at the details of this beyond the commit messages  
that went by, but if this is because of an Ant property that is  
sending in a relative versus absolute path, you can ensure the path  
is always absolute by using :

	<property name="whatever.dir" file="relative/path"/>

instead of using the <property name="..." value="..."/> variant.   If  
the file="..." value is not already absolute, it'll make it absolute  
based on the base directory.

(couldn't help myself) After a quick peek at the contrib/benchmark/ 
build.xml, this may be the culprit:

    <property name="working.dir" value="work"/>

use file instead of value, and you can always assume the path is  
absolute from that default, and mandate anyone overriding that  
property specify an absolute path.  And there are some other  
properties (the ones pointing to JAR files, for example) that should  
be using the file variant as well.

My rule is that all Ant properties that point to any file or  
directory use the "file" variant of <property>.

And another candidate for refactoring in the benchmark build.xml is  
this:

    <arg line="${working.dir} ${basedir}/conf/micro-standard- 
config.xml"/>

The relative path can be converted into an absolute path using two  
<arg file="...."/> elements instead of one <arg line="..."/>.   My  
rule here is to avoid using "line", rather:

    <arg file="${working.dir}"/>
    <arg file="conf/micro-standard-config.xml"/>

No need to use ${basedir} for that second argument as that is  
implicit in using <arg file="..."/> if the value is not already an  
absolute path.

	Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org

Re: svn commit: r603856 - in /lucene/java/trunk/contrib/benchmark: ./ src/java/org/apache/lucene/benchmark/byTask/feeds/

[Doron Cohen <cd...@gmail.com>]

Thanks for the review Erik!

LUCENE-1086 solved a problem for people trying to run the benchmark
from e.g. Eclipse rather than by using ant, and specifying absolute path for
the docs dir. Problem was that the benchmark Java code assumed that
the path for docs dir is relative, under "work", and when someone wanted
to run the benchmark on "his" collection, which is under "work", it just
didn't work. So the fix solved that, allowing one to supply either a
relative path or an absolute one.

A I understand your comments you suggest to modify build.xml
to use "file" rather than "value". This way absolute file-paths would
be passed to Java. So, complementary to this, the Java code
should be modified to not assume any relative paths, and actually
become unaware of the "work-dir", but rather obtain absolute paths
for whatever is required - docs, alg, index-dir, whatever.

Is this correct so far?

Assuming yes, I like this approach - I think it is much cleaner. So I'll
create an issue to fix this (and the arg line issue).

One implication of this change would be that anyone invoking the
benchmarks not with build.xml would have to specify all the folders
involved - data-dir, alg-file, index-dir. Well, only 3 if I am not
forgetting anything, guess this is not too bad?

Thanks,
Doron

On Dec 13, 2007 2:00 PM, Erik Hatcher <er...@ehatchersolutions.com> wrote:

>
> On Dec 13, 2007, at 12:58 AM, doronc@apache.org wrote:
> > +12/13/07
> > +  LUCENE-1086: DocMakers setup for the "docs.dir" property
> > +  fixed to properly handle absolute paths. (Shai Erera via Doron
> > Cohen)
> > +
>
> I haven't looked at the details of this beyond the commit messages
> that went by, but if this is because of an Ant property that is
> sending in a relative versus absolute path, you can ensure the path
> is always absolute by using :
>
>        <property name="whatever.dir" file="relative/path"/>
>
> instead of using the <property name="..." value="..."/> variant.   If
> the file="..." value is not already absolute, it'll make it absolute
> based on the base directory.
>
> (couldn't help myself) After a quick peek at the contrib/benchmark/
> build.xml, this may be the culprit:
>
>    <property name="working.dir" value="work"/>
>
> use file instead of value, and you can always assume the path is
> absolute from that default, and mandate anyone overriding that
> property specify an absolute path.  And there are some other
> properties (the ones pointing to JAR files, for example) that should
> be using the file variant as well.
>
> My rule is that all Ant properties that point to any file or
> directory use the "file" variant of <property>.
>
> And another candidate for refactoring in the benchmark build.xml is
> this:
>
>    <arg line="${working.dir} ${basedir}/conf/micro-standard-
> config.xml"/>
>
> The relative path can be converted into an absolute path using two
> <arg file="...."/> elements instead of one <arg line="..."/>.   My
> rule here is to avoid using "line", rather:
>
>    <arg file="${working.dir}"/>
>    <arg file="conf/micro-standard-config.xml"/>
>
> No need to use ${basedir} for that second argument as that is
> implicit in using <arg file="..."/> if the value is not already an
> absolute path.
>
>        Erik
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org
>
>