Closed GoogleCodeExporter closed 9 years ago
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
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:
Original comment by jokeserver
on 30 Sep 2010 at 2:43
Original comment by jokeserver
on 30 Sep 2010 at 2:43
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
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
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
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:
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
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
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
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
The change has been submitted for review in Issue 193.
Original comment by jokeserver
on 8 Oct 2010 at 5:35
The change has been committed to trunk.
Original comment by jokeserver
on 12 Oct 2010 at 3:12
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
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
Apologies for that. Fixed.
Original comment by jokeserver
on 12 Oct 2010 at 3:45
I still get the same error. Was the change deployed?
Original comment by melis.ga...@gmail.com
on 13 Oct 2010 at 1:11
I just asked amstan, should be in now.
Original comment by jokeserver
on 13 Oct 2010 at 3:33
Original issue reported on code.google.com by
sharpbla...@gmail.com
on 11 Sep 2010 at 11:35