Feature Build Process Changes

fronobulax said:
I see class files that are in jar files in lib/jar that somehow end up in KoLmafia.jar. I would like to understand how they get there and why the same technique cannot be done for the rhino jar.
Click to expand...
Daily, after it finishes its dependencies, calls two tasks: compile and jar. The compile task compiles the java files into class fiies in the normal way via javac. The jar task creates the jar from the output of the compile task, and includes lib/jar/*.jar in the KolMafia-${version}.jar output.

In order to use a jar in an include (as this project does), it would need to be in the compile task's classpath.
Code: 
import org.mozilla.javascript.BaseFunction;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.NativeJavaObject;
import org.mozilla.javascript.Scriptable;
Jars KM builds are available at compile time and run-time. Jars KM includes are available at run-time only.

There are (at least) four ways to deal with this:
1: add all the lib/jar jars to the classpath
2: add the source to the libs directory and build it.
3: add just the lib we need to the classpath.
4: refactor the code to not need the dependency at compile time
 
Oh, and to get deeper into the build weeds, the lib jars are available when compiling the lib directory but are not when compiling the src directory.
 
MCroft said:
Daily, after it finishes its dependencies, calls two tasks: compile and jar. The compile task compiles the java files into class fiies in the normal way via javac. The jar task creates the jar from the output of the compile task, and includes lib/jar/*.jar in the KolMafia-${version}.jar output.

In order to use a jar in an include (as this project does), it would need to be in the compile task's classpath.
Code: 
import org.mozilla.javascript.BaseFunction;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.NativeJavaObject;
import org.mozilla.javascript.Scriptable;
Jars KM builds are available at compile time and run-time. Jars KM includes are available at run-time only.

There are (at least) four ways to deal with this:
1: add all the lib/jar jars to the classpath
2: add the source to the libs directory and build it.
3: add just the lib we need to the classpath.
4: refactor the code to not need the dependency at compile time
Click to expand...
This makes sense. My patch took approach 1. Approach 2 has been taken for some libraries already in Mafia. On 3, I am not sure why one would not want to add all the jars to the classpath; there may be reasons that I'm not seeing here. I don't think 4 is really feasible.
 
Maybe we should make a new directory for jars that should be available at compile time? Feels better to me to make a generic solution.
 
MCroft said:
There are (at least) four ways to deal with this:
1: add all the lib/jar jars to the classpath
2: add the source to the libs directory and build it.
3: add just the lib we need to the classpath.
4: refactor the code to not need the dependency at compile time
Click to expand...

Where is this classpath cited in 1) and 3) ?

Is it relevant that I can delete .classpath and still build KoLmafia with "and daily"?

Something is including classes from jars in \lib\jar in the KoLmafia jar and I cannot figure out how the ant task knows how beyond their presence in \lib\jar.

2) is a maintenance nightmare unless we are willing and able to automate and idiot proof the process of updating the source from its repository.

4) I don't understand something about how rhino is being used which would make 4 even possible.

With 1 and 3 left, 3 is the smaller change although if 1 can be done in such a way that we will never have to solve a similar problem by doing anything besides dropping a jar file in \lib\jar I'd explore it, if only because I thought that was already happening.
 
Fronobulax, I think what you're seeing is that much of the code in /lib uses the jars. The jars are in the CLASSPATH for /lib, but not in the CLASSPATH for /src. You can see that in the current build.xml. I think that is why you see compile errors when you delete all the jars. When I try to reproduce that (by deleting jars), all the errors I see are from source files in /lib. So my understanding is that, before my patch, no current code in /src used jars - instead, some of it used code in /lib which in turn used the jars. I don't think "ant debug" and "ant daily" do anything substantially different in terms of jars or CLASSPATH.

Here is the entry for /lib:
XML: 
        <javac
            compiler="modern"
            source="1.8"
            target="1.8"
            release="8"
            srcdir="${lib}"
            destdir="${build}"
            failonerror="false"
            debug="on"
            debuglevel="lines,vars,source"
            deprecation="off"
            encoding="utf-8"
            nowarn="on"
            includeantruntime="false"
            errorProperty="compile.failed">
            <classpath>
                <pathelement path="${classpath}"/>
                <fileset dir="${lib}/jar">
                    <include name="**/*.jar"/>
                </fileset>
            </classpath>
        </javac>

And for /src:
XML: 
        <javac
            compiler="modern"
            source="1.8"
            target="1.8"
            release="8"
            srcdir="${src}"
            destdir="${build}"
            classpath="${build}"
            failonerror="false"
            debug="on"
            debuglevel="lines,vars,source"
            deprecation="off"
            encoding="utf-8"
            nowarn="on"
            includeantruntime="false"
            errorProperty="compile.failed" />

The jars are all copied into the final mafia jar in this build step.
XML: 
        <jar
            destfile="${jarfile}"
            basedir="${build}"
            update="true"
            index="true"
            compress="true">

            <manifest>
                <attribute name="Main-Class" value="${main}" />
            </manifest>
            <zipgroupfileset dir="${lib}/jar">
                <include name="*.jar"/>
            </zipgroupfileset>
        </jar>

I'm still pretty sure that the .classpath file is not doing anything to the build.
 
fronobulax said:
Where is this classpath cited in 1) and 3) ?
Click to expand...
In the javac task of the compile target for lib
Code: 
            <classpath>
                <pathelement path="${classpath}"/>
                <fileset dir="${lib}/jar">
                    <include name="**/*.jar"/>
                </fileset>
            </classpath>

and the javac task of the compile target for src
Code: 
            classpath="${build}"

fronobulax said:
Is it relevant that I can delete .classpath and still build KoLmafia with "and daily"?
Click to expand...
according to notes in the pinned "build" thread in this forum, this .classpath file is only used by eclipse.
fronobulax said:
Something is including classes from jars in \lib\jar in the KoLmafia jar and I cannot figure out how the ant task knows how beyond their presence in \lib\jar.
Click to expand...
The daily target includes the jar task. It adds the files from ${lib}/jar/*.jar with a fileset, in this case a zipgroupfileset.
Code: 
        <jar...>
            <zipgroupfileset dir="${lib}/jar">
                <include name="*.jar"/>
            </zipgroupfileset>
        </jar>
fronobulax said:
2) is a maintenance nightmare unless we are willing and able to automate and idiot proof the process of updating the source from its repository.
Click to expand...
Agreed. I think that outweighs the advantage of not changing the build model, but I didn't want to not discuss it. It works for other projects.
fronobulax said:
4) I don't understand something about how rhino is being used which would make 4 even possible.
Click to expand...
I don't think it's viable for this project. It is viable for something much less tightly coupled, like the DarkMode project I'm still working. I think the functional difference is you can't include a class unless the jar is in the classpath.
fronobulax said:
With 1 and 3 left, 3 is the smaller change although if 1 can be done in such a way that we will never have to solve a similar problem by doing anything besides dropping a jar file in \lib\jar I'd explore it, if only because I thought that was already happening.
Click to expand...
I would implement #3 by adding a build.classpath.addons to build.properties and adding Rhino-XXX.jar to it, then changing the classpath in the compile javac task to include "classpath="${build};${build.classpath.addons}". or similar...

You can drop a jar file in lib\jar as long as you don't want to import anything from it, just call it at runtime.
 
fronobulax said:
With 1 and 3 left, 3 is the smaller change although if 1 can be done in such a way that we will never have to solve a similar problem by doing anything besides dropping a jar file in \lib\jar I'd explore it, if only because I thought that was already happening.
Click to expand...
My concern with #1 is that I don't feel sure it won't cause namespace collisions that we don't currently have by introducing a bunch of jars we previously haven't had in the build space. If we go with #3, we can make other exceptions the same way we make this one. But I've compiled with basically #1 for the last two versions and didn't see any issues, so I could be convinced it is safe.
 
I am worried that splitting this out into a separate thread on build process risks overcomplicating the otherwise small change that @ikzann has made in his other thread. Perhaps anyone who is interested can continue this discussion and the javascript changes could be limited to exclusively the rhino jar.
 
Thanks for your patience. I do not understand why dropping a jar in /lib/jar does not make all the right things happen because of the wildcards in defining the zipgroupfileset.

BUT

The last time I applied the patch I was able to compile and run. Nothing I did knew about the rhino jar. But I did not actually try and execute a js script.

So is the rhino jar something that is only needed at run time and only needed if a user tries to run a Kolmafia script written in JS?

If the answer is YES then we have found the source of all my confusion. But I would then ask why are we messing with the build and not following the example of ash dependencies (if we want to put this on the script writer and open the doors to Version Hell) or looking to the SVN repo file or the TCRS files as a model for distribution?
 
It is not only needed at runtime. The code to run JS is deeply intertwined with Rhino and needs to know about Rhino changes at both compile time and runtime.

Re: MCroft, I think the risk of namespace collisions is pretty limited. Everything still needs to be imported via its fully-qualified name, anyway, and all those jars end up in the final Mafia jar anway. I can't think of any other potential issues arising from having the jars available at compile time.
 
The problem with certain ways of reading posts is that things get missed.

I now note the reminder that there are two compile operations - one for "our" code and one for third party code where we have just grabbed the source.

While the assertion that nothing in "our" source depends upon anything in /lib/jar is counter intuitive to me, it explains the source of my confusion. I should also be reminded that the test target uses jars that mafia doesn't need.

If my newfound insight is correct then my knee jerk response is to make the src build also use /lib/jar and just drop the rhino jar in there.

It's a good thing I have made peace with being publicly wrong and trying to learn from the experience..
 
OK. I got the patch that started this all to work. I made my own changes to build.xml so I understood what I was doing. What worked for me was copying the zipfileset to the src javac task and then dropping the rhino jar into /lib/jar.

My recommendation at this point is to create a new directory /src/lib or /src/jar. Then I would modify the src javac task to include any jars there. I would do that by copying the <classpath> from the lib javac task and change the directory. Then, as KoLmafia needs external jars they can be placed there and no additional changes to the ant build would be needed.

If we agree on this I might consider doing directory creation and the build change as a commit by themselves. Then a separate commit with rhino and then the JavaScript patch is pretty much only Java code.

In the spirit of fixing what is not broken, it would be interesting to review/clean up /lib/jar and the /lib source. I think there are jar files that are no longer used. But that can be a later project :-)
 
I think you're correct and I'm OK with making this change. I tested the 10-year-old instructions @Veracity has for making a release and the only thing that doesn't work doesn't work on HEAD, either. (I'll make a new defect for that).

We'd need to make sure that the daily and dist tasks moved the included jars correctly, but that's a testing task.
 
MCroft said:
We'd need to make sure that the daily and dist tasks moved the included jars correctly, but that's a testing task.
Click to expand...

My one test used daily and the rhino jar was unpacked from a separate jar and the class files placed in the KoLmafia jar..

Might be a vocabulary thing - back in the day the code sometimes had to know where a jar file was going to be because security models sometimes required a different or separate classloader. Lazy coders, such as myself, liked all jars in one place and liked classes that were in the same jar as the main, even better.
 
fronobulax said:
My one test used daily and the rhino jar was unpacked from a separate jar and the class files placed in the KoLmafia jar..

Might be a vocabulary thing - back in the day the code sometimes had to know where a jar file was going to be because security models sometimes required a different or separate classloader. Lazy coders, such as myself, liked all jars in one place and liked classes that were in the same jar as the main, even better.
Click to expand...
I just want to make sure the build output puts Rhino and all the others lib/jar/*.jar files in the single release jar, in each of the ways we normally make a jar.
 
MCroft said:
I just want to make sure the build output puts Rhino and all the others lib/jar/*.jar files in the single release jar, in each of the ways we normally make a jar.
Click to expand...

If we create src/jar and put rhino there would you be modifying your expectations?
 
If the jar packaging commands all add that to the zipfilesetgroup.

on review: -v7 does what I was looking for, which is that the jar task packages the Rhino jar.
 
Last edited:
In the interest of moving ahead, lets accept the src/jar convention. I'll create the directories and check in rhino. I'm OK with the build.xml changes in the Javascript patch so I will extract them and check them in separately from the rest of the JavaScript.

The daily task made the same jar with the directory and rhino present as it did without, using the changed build so breaking things apart doesn't seem to hurt anything or even make a difference until Javascript is added.
 
You must log in or register to reply here.
Back
Top