Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Truffle] mx clean and mx build work for JRuby #3813

Merged
merged 1 commit into from
May 5, 2016
Merged

[Truffle] mx clean and mx build work for JRuby #3813

merged 1 commit into from
May 5, 2016

Conversation

jtulach
Copy link
Contributor

@jtulach jtulach commented Apr 20, 2016

Majority of Graal/Truffle projects is using mx build tool. JRuby is an exception. The goal of this pull request is to provide an mx facade that delegates execution to existing mvn goals, but behaves like a normal mx from outside and thus simplifies including of JRuby in other mx based projects.

With the current changes I can run the build and it successfully connects Truffle with JRuby and builds both:

$ mx clean
$ mx build

# java executable that is used we unfortunately need to append it to the PATH
javaHome = os.getenv('JAVA_HOME')
if javaHome:
os.environ["PATH"] = os.environ["JAVA_HOME"] + '/bin' + os.pathsep + os.environ["PATH"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like something that should be done in run_maven

@gilles-duboscq
Copy link

gilles-duboscq commented Apr 20, 2016

In general, overriding build and clean is not a good idea: it breaks composing the suite with other suites.

The recommended way of achieving this is to make the thing you want to build a mx.Dependency, then you can implement Dependency.getBuildTask which returns a mx.BuildTask. You'll return an implementation of BuildTask that implements BuildTask.build and BuildTask.clean using the appropriate maven commands.
To get a mx.Dependency in the system, you can for example have a project in your suite.py with a class attribute: when such projects are loaded, this class name is looked up in your suite extension (here mx_jruby.py) and used as the class for the object representing this project. This class should be a subclass of mx.Project.

An example of this is how HotSpot is build in the jvmci suite: in suite.py you can see the hotspot project: it has a class attribute set to HotSpotProject. This class is found in mx_jvmci.py and in particular it overrides getBuildTask to return a HotSpotBuildTask which handles the details of building and cleaning hotspot.

This doesn't really answer the question of the resulting distributions but i guess we'd need some kind of JARDistribution that simply copies an existing jar (the one produced by maven).

@gilles-duboscq
Copy link

Regarding the distribution, i think we could add a class attribute to those as well.

@eregon
Copy link
Member

eregon commented Apr 20, 2016

The goal of this pull request is to provide an mx facade that delegates execution to existing mvn goals, but behaves like a normal mx from outside and thus simplifies including of JRuby in other mx based projects.

Actually, I would be interested in compilation by mx not depending on Maven, because it's much faster and more reliable when configured correctly. But this is of course conflicting with this PR...

@jtulach
Copy link
Contributor Author

jtulach commented Apr 20, 2016

Compilation with mx is not really conflicting, but certainly out of scope of what I am trying to do. If we want 100% compatibility with the way JRuby is currently built, then I am reluctant to compile with mx. Of course, if you guys want to use mx as primary build tool, then including in other mx projects would be piece of cake and we could close this PR. But I assume wrapper around mvn will be more gentle approach.

mx.run_maven(['-DskipTests', '-Dtruffle.version=' + truffle_commit], cwd=rubyDir)
mx.run_maven(['-Pcomplete', '-DskipTests', '-Dtruffle.version=' + truffle_commit], cwd=rubyDir)
# mx.run(['zip', '-d', 'maven/jruby-complete/target/jruby-complete-graal-vm.jar', 'META-INF/jruby.home/lib/*'], cwd=rubyDir)
mx.run(['bin/jruby', 'bin/gem', 'install', 'bundler', '-v', '1.10.6'], cwd=rubyDir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why installing bundler here, on every build?

@eregon
Copy link
Member

eregon commented Apr 20, 2016

@jtulach Right, let's begin with delegating to Maven and then maybe progressively adopt mx, as discussed with @gilles-duboscq.

@@ -23,14 +23,12 @@

"jruby-truffle" : {
"subDir" : "",
"class" : "MavenProject",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MavenProject and MavenBuildTask approach seems to be working quite well. The next step is to do something with the distribution. @gilles-duboscq, please push me in the right direction.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, for the distribution i'll add support for the class attribute in mx for distribution and then you can do the same an create something that looks like a JARDistribution that just copies the jar created by maven. I'll have a quick to see if it would help to make an abstract JARDistribution that you can extend that has all the "consumption" aspects of JARDistribution but none of the "creation" aspects.

@pitr-ch pitr-ch added this to the truffle-dev milestone Apr 20, 2016
@jtulach
Copy link
Contributor Author

jtulach commented Apr 22, 2016

The goal of this change is to create an mx distribution that will contain the whole content of a jre/language/ruby/ directory as is currently in the OTN build. It will be responsibility of you guys to maintain the content of this ZIP to contain everything you need.

],
},
"libraries": {
"RUBY_COMPLETE": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be called JRUBY_CORE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed the path to jruby-complete JAR at the end.

@jtulach
Copy link
Contributor Author

jtulach commented May 5, 2016

The current version seems to work OK for my mx build purposes. Please consider merging it.

"truffle:TRUFFLE_API",
"truffle:TRUFFLE_DEBUG",
],
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation seems wrong here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. See next comment...

@eregon
Copy link
Member

eregon commented May 5, 2016

Looks good 👍, please address my comments.
I think some names and descriptions can be improved, but I can do that later.

@eregon
Copy link
Member

eregon commented May 5, 2016

LGTM!

@eregon eregon merged commit 4f5fa8e into jruby:truffle-head May 5, 2016
@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants