Clean up compile_anything.py

[GoogleCodeExporter]

compile_anything.py is a mess. The current code can be re-factored down to a 
5th of its size - using lots of if statements is never a pretty, efficient (and 
in some cases) correct way of doing something, and it won't scale as more 
languages are added in the future. We might as well get the code into a nice 
condition now rather than later.

I am willing to work on this ticket myself, I just thought I would create it to 
track progress and gather comments.

Original issue reported on code.google.com by sharpbla...@gmail.com on 11 Sep 2010 at 11:35

• Blocked on: #193

[GoogleCodeExporter]

The attached patch cleans up compile_anything.py
The comments at the top should explain how, but I will add my rationale here.

This patch adds several classes: ChmodCompiler, ExternalCompiler and 
GccCompiler. These are abstractions used to define a compile chain that will 
result in a compiled version of the bot. Java's compile chain looks like so:
(The flags and program are defined in the lansys dict)
"Java"   :(".jar",["*.class","*.jar"], 
                  [("*.java",ExternalCompiler(lansys["Java"][0])),
                    ("*.class",ExternalCompiler(lansys["Java"][1])),
                    ("MyBot.jar",ChmodCompiler())]),

This quite simply removes all existing *.class and *.jar files, runs "javac" on 
all the *.java files first then runs the "jar" command on all the resulting 
*.class files and then chmods MyBot.jar.
It should be fully compatible with the existing inputs and outputs - it returns 
the stderr and stdout of the programs and also actually adds check_file to the 
output, whereas before it simply did nothing.

Now, as a result of these changes, "compile_function" is only 17 lines, as 
opposed to a monster 115+.

Original comment by sharpbla...@gmail.com on 13 Sep 2010 at 8:48

[GoogleCodeExporter]

There were two small errors in the submitted patch, here is a fix.

Original comment by sharpbla...@gmail.com on 14 Sep 2010 at 10:22

Attachments:

• compile.patch

[GoogleCodeExporter]

Original comment by jokeserver on 30 Sep 2010 at 2:43

• Changed title: Clean up compile_anything.py
• Changed state: Started

[GoogleCodeExporter]

Original comment by jokeserver on 30 Sep 2010 at 2:43

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Bugs in compile_anything are preventing people from participating in the 
contest.  See in particular issue 187.  The patch here is more general than the 
one on 187, so assuming it works as advertised, the patch here is preferable.  
When it is accepted, 187 should be closed.

Original comment by jklan...@gmail.com on 5 Oct 2010 at 11:24

[GoogleCodeExporter]

It *should* work as advertised. I suggest testing it before applying it - I do 
not have the means to run a full test of it (Running windows with none of the 
compilers). 

Also any new languages monkeypatched to compile_anything() after this patch was 
submitted needs to be added, but that should be quite easy and self explanatory.

Original comment by sharpbla...@gmail.com on 6 Oct 2010 at 12:34

[GoogleCodeExporter]

It will not work as advertised. I started working it into the existing script 
by hand and have noticed and fixed a few errors, mostly related to iterating 
over a string instead of a list of strings.

I have also not had it high on my todo list because this was mainly a style 
change. As it will also fix the problems in issue 187, I will attempt to get a 
fix in within a few days.

Original comment by jokeserver on 6 Oct 2010 at 5:19

[GoogleCodeExporter]

Here is an alternative approach.  It keeps the existing code structure, but 
makes a few improvements:

1.  The exit code of spawned processes is checked.
2.  For compiles which take several steps, the process is stopped at the first 
failure
3.  The helper functions take a log object as a parameter, which simplifies the 
task of keeping track of outputs and errors

It is my opinion that a sequence of "ifs", one for each language, is easier to 
read and modify than an abstract class structure.  However, that is only my 
opinion, and my only goal here is to get issue 187 fixed, so I'll help with 
whatever approach gets us there fastest.

Since there are so many changes to the file, I have just sent my copy instead 
of a patch.  Also note there is a test script included in the tarball.

The test script includes some "hello world" programs and tests compiling them 
in a temporary directory using compile_anything.  The languages tested are:

Java
Haskell
C#
C++
C
OCaml
Lisp

I set up a local MySQL server and created a database with a languages table, 
etc., and the version of compile_anything included in the tarball passes all 
the tests in the test script.  Note that the test script should also work with 
any other implementation of compile_anything, so I suggest making it part of 
the codebase irrespective of what version of compile_anything is chosen.

However, the test script does not cover any of the interpreted languages, nor 
does it cover failure cases.  Clearly there is room for improvement.

In any case, let me know if there is anything I can do to get this issue (and 
187) closed.

Original comment by jklan...@gmail.com on 6 Oct 2010 at 6:47

Attachments:

• compile-replacement.tgz

[GoogleCodeExporter]

I actually finished (shortly before this last post) integrating sharpblade1's 
changes (with some fairly heavy modifications) to the compile script, and 
tomorrow I'll start testing it. You can see what I've done here: 
http://code.google.com/p/ai-contest/source/browse/branches/compile_cleanup/compi
le_anything.py?r=408.

I do like some of the ideas in your patch, namely the early fail-out and the 
use of a Log class to prune hefty function returns. I will add those in as 
well, if it doesn't take too long, and once I'm satisfied things are working 
right, I'll submit the result for review.

Original comment by jokeserver on 6 Oct 2010 at 7:20

[GoogleCodeExporter]

Having thought about it further, for scripting languages it is not necessary 
(or desirable) to chmod all the files, it is only necessary to ensure that the 
main program file is executable.  The compile_anything function already knows 
what the "main_code_file" is for each language, because that information is 
stored in the database.

So we can cut out all the redundant code related to "compilation" of scripting 
languages.  Just pass the main_code_file as an additional argument to 
compile_function, and then compile_function can "chmod 0644" it as its default 
behavior for all languages which don't have special compilation rules.

Then when new scripting languages are added to the language database, 
compile_anything.py code will handle them without any changes whatsoever.  
There is no need to repeat the logic "for scripting language X, the main 
program is called Y and should be executable."

In a final note, I noticed you are handling transformations like "foo.c" -> 
"foo.o" using string operations.  The os.path.splitext library function handles 
splitting a file name into its base name and extension, and I suggest using it 
unless there is a compelling reason not to.

Original comment by jklan...@gmail.com on 6 Oct 2010 at 3:24

[GoogleCodeExporter]

I am not sure what you mean. chmod 644 makes it rw-r--r--, ie. world readable, 
not executable. The scripts are run through their respective interpreter, I 
believe, according to another entry from the database. To be executable would 
require they each have a line at the top #!/usr/bin/env python or similar.

I think I would rather leave the "compilation" of scripting languages as-is for 
now, though it may be worth looking into later.

Original comment by jokeserver on 6 Oct 2010 at 3:39

[GoogleCodeExporter]

You are right about what 0644 does.  I wasn't thinking clearly, and I'm still 
confused about the purpose of the chmod code.  If anyone decides to work 
further on the compiler, please consider documenting the rationale for changing 
the file permissions, and in particular doing so only for certain files in the 
root directory of certain submissions.

Original comment by jklan...@gmail.com on 6 Oct 2010 at 5:35

[GoogleCodeExporter]

The change has been submitted for review in Issue 193.

Original comment by jokeserver on 8 Oct 2010 at 5:35

[GoogleCodeExporter]

The change has been committed to trunk.

Original comment by jokeserver on 12 Oct 2010 at 3:12

• Changed state: Fixed

[GoogleCodeExporter]

Got this after submitting:

Failure: output file MyBot.sbcl was not created.
Compilation failed.

The binary is named "MyBot" so the + ".sbcl" in the code is not needed.

Original comment by melis.ga...@gmail.com on 12 Oct 2010 at 9:06

[GoogleCodeExporter]

I believe that patch below fixes it, but I don't have MySQL set up so couldn't 
test it:

Index: compile_anything.py
===================================================================
--- compile_anything.py (revision 428)
+++ compile_anything.py (working copy)
@@ -211,8 +211,8 @@
   "Javascript"  : (".js",
                    [],
                    [(["*.js"], ChmodCompiler("Javascript"))]),
-  "Lisp"        : (".sbcl",
-                   [BOT, BOT + ".sbcl"],
+  "Lisp"        : ("",
+                   [BOT],
                    [([""], ExternalCompiler(comp_args["Lisp"][0]))]),
   "Lua"         : (".lua",
                    [],

Original comment by melis.ga...@gmail.com on 12 Oct 2010 at 3:29

[GoogleCodeExporter]

Apologies for that. Fixed.

Original comment by jokeserver on 12 Oct 2010 at 3:45

[GoogleCodeExporter]

I still get the same error. Was the change deployed?

Original comment by melis.ga...@gmail.com on 13 Oct 2010 at 1:11

[GoogleCodeExporter]

I just asked amstan, should be in now.

Original comment by jokeserver on 13 Oct 2010 at 3:33