-
Notifications
You must be signed in to change notification settings - Fork 0
Add javac support for mixed targets #2
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
Conversation
if (ops.javaFiles.length > 0) { | ||
StringBuilder cmd = new StringBuilder(); | ||
cmd.append(ops.javacPath); | ||
if (ops.jvmFlags != "") cmd.append(ops.jvmFlags); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these not be separate args to the Process builder rather than a string concat?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do that, it passes empty strings to the javac which then errors
rather than ignoring empty args.
It's great. I'm proud of this. ;)
On Mon, Sep 12, 2016 at 14:39 ianoc [email protected] wrote:
In src/java/io/bazel/rulesscala/scalac/ScalaCInvoker.java
#2 (comment):@@ -255,6 +255,37 @@ private static void processRequest(List args) throws Exception {
reporter.flush();
throw new RuntimeException("Build failed");
} else {
/**
\* See if there are java sources to compile
*/
if (ops.javaFiles.length > 0) {
StringBuilder cmd = new StringBuilder();
cmd.append(ops.javacPath);
if (ops.jvmFlags != "") cmd.append(ops.jvmFlags);
should these not be separate args to the Process builder rather than a
string concat?—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/ianoc/rules_scala/pull/2/files/d0d6658d1f6ff948949ac47ea1ab7f9f128c8606#r78479922,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEJduvOcPE3K8mTZEnbhLDkvK0h8Yvkks5qpfDCgaJpZM4J7LKQ
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Miss filter/flatMap don't you!)
ok this seems fine since it must work. (In some other of these sub process libs they can require that the args are passed to it and the 'process' is a executable to call than a string with args in it... indeed i believe scala's works like this.)
LGTM |
I really missed filter and map on this. Maybe I should look into the Java
|
commit d3ed208 Merge: 61b3b57 cd24529 Author: Ian O'Connell <[email protected]> Date: Mon Sep 12 21:21:53 2016 -0700 Merge pull request #3 from ianoc/oscar/worker-resources Add resource support, minor cleanups commit cd24529 Author: Oscar Boykin <[email protected]> Date: Mon Sep 12 18:05:01 2016 -1000 Add resource support, minor cleanups commit 61b3b57 Merge: 9bf9087 d0d6658 Author: P. Oscar Boykin <[email protected]> Date: Mon Sep 12 16:53:40 2016 -1000 Merge pull request #2 from ianoc/oscar/javac_scala_worker Add javac support for mixed targets commit d0d6658 Author: Oscar Boykin <[email protected]> Date: Mon Sep 12 14:29:18 2016 -1000 Add javac support for mixed targets commit 9bf9087 Merge: 617885e 30b0efc Author: Ian O'Connell <[email protected]> Date: Mon Sep 12 16:25:56 2016 -0700 Merge pull request #1 from ianoc/oscar/resident-compiler Factor out ScalaCInvoker option parsing commit 30b0efc Author: Oscar Boykin <[email protected]> Date: Mon Sep 12 13:22:56 2016 -1000 address review commit b28dcd1 Author: Oscar Boykin <[email protected]> Date: Mon Sep 12 12:35:14 2016 -1000 Factor out ScalaCInvoker option parsing commit 617885e Author: Ian O Connell <[email protected]> Date: Sun Sep 11 21:43:59 2016 -0700 Adding commit 821ab21 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 19:47:11 2016 -0700 wip commit f6bebe6 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 19:38:41 2016 -0700 wip commit 4ab6810 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 19:33:02 2016 -0700 wip commit cdc4995 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 17:55:40 2016 -0700 wip commit 51b9251 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 17:45:45 2016 -0700 WIP commit e4b53aa Author: Ian O Connell <[email protected]> Date: Sun Sep 11 17:29:09 2016 -0700 WIP commit 1cc779c Author: Ian O Connell <[email protected]> Date: Sun Sep 11 17:16:05 2016 -0700 Wip commit 1f9600c Author: Ian O Connell <[email protected]> Date: Sun Sep 11 14:13:33 2016 -0700 WIP commit d2dce89 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 14:11:14 2016 -0700 Undo commit ec05677 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 14:06:17 2016 -0700 WIP commit dcf82b4 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 12:19:19 2016 -0700 Some formatting so my editor is less cranky commit f11926a Author: Ian O Connell <[email protected]> Date: Sun Sep 11 12:12:29 2016 -0700 WIP -- have the jar creator code inlined commit 0a1ad35 Author: Ian O Connell <[email protected]> Date: Sat Sep 10 21:01:44 2016 -0700 wip commit 3a0c1ab Author: Ian O Connell <[email protected]> Date: Sat Sep 10 20:58:10 2016 -0700 WIP commit 15d02b1 Author: Ian O Connell <[email protected]> Date: Sat Sep 10 18:10:28 2016 -0700 Make deploy jar name easier to grep for commit d3823aa Author: Ian O Connell <[email protected]> Date: Tue Aug 9 11:40:40 2016 -0700 Use bind's for twitter scrooge so local repo's can override scrooge versions commit f11e814 Merge: f82800b 4146c9e Author: Ian O Connell <[email protected]> Date: Thu Jul 28 16:28:02 2016 -0700 Merge branch 'master' of github.com:bazelbuild/rules_scala commit f82800b Author: Ian O Connell <[email protected]> Date: Thu Jul 28 15:32:14 2016 -0700 Fix repl test commit 9c3f37c Author: Ian O Connell <[email protected]> Date: Thu Jul 28 15:30:46 2016 -0700 Update test rules
* Squashed commit of the following: commit d3ed208 Merge: 61b3b57 cd24529 Author: Ian O'Connell <[email protected]> Date: Mon Sep 12 21:21:53 2016 -0700 Merge pull request #3 from ianoc/oscar/worker-resources Add resource support, minor cleanups commit cd24529 Author: Oscar Boykin <[email protected]> Date: Mon Sep 12 18:05:01 2016 -1000 Add resource support, minor cleanups commit 61b3b57 Merge: 9bf9087 d0d6658 Author: P. Oscar Boykin <[email protected]> Date: Mon Sep 12 16:53:40 2016 -1000 Merge pull request #2 from ianoc/oscar/javac_scala_worker Add javac support for mixed targets commit d0d6658 Author: Oscar Boykin <[email protected]> Date: Mon Sep 12 14:29:18 2016 -1000 Add javac support for mixed targets commit 9bf9087 Merge: 617885e 30b0efc Author: Ian O'Connell <[email protected]> Date: Mon Sep 12 16:25:56 2016 -0700 Merge pull request #1 from ianoc/oscar/resident-compiler Factor out ScalaCInvoker option parsing commit 30b0efc Author: Oscar Boykin <[email protected]> Date: Mon Sep 12 13:22:56 2016 -1000 address review commit b28dcd1 Author: Oscar Boykin <[email protected]> Date: Mon Sep 12 12:35:14 2016 -1000 Factor out ScalaCInvoker option parsing commit 617885e Author: Ian O Connell <[email protected]> Date: Sun Sep 11 21:43:59 2016 -0700 Adding commit 821ab21 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 19:47:11 2016 -0700 wip commit f6bebe6 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 19:38:41 2016 -0700 wip commit 4ab6810 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 19:33:02 2016 -0700 wip commit cdc4995 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 17:55:40 2016 -0700 wip commit 51b9251 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 17:45:45 2016 -0700 WIP commit e4b53aa Author: Ian O Connell <[email protected]> Date: Sun Sep 11 17:29:09 2016 -0700 WIP commit 1cc779c Author: Ian O Connell <[email protected]> Date: Sun Sep 11 17:16:05 2016 -0700 Wip commit 1f9600c Author: Ian O Connell <[email protected]> Date: Sun Sep 11 14:13:33 2016 -0700 WIP commit d2dce89 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 14:11:14 2016 -0700 Undo commit ec05677 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 14:06:17 2016 -0700 WIP commit dcf82b4 Author: Ian O Connell <[email protected]> Date: Sun Sep 11 12:19:19 2016 -0700 Some formatting so my editor is less cranky commit f11926a Author: Ian O Connell <[email protected]> Date: Sun Sep 11 12:12:29 2016 -0700 WIP -- have the jar creator code inlined commit 0a1ad35 Author: Ian O Connell <[email protected]> Date: Sat Sep 10 21:01:44 2016 -0700 wip commit 3a0c1ab Author: Ian O Connell <[email protected]> Date: Sat Sep 10 20:58:10 2016 -0700 WIP commit 15d02b1 Author: Ian O Connell <[email protected]> Date: Sat Sep 10 18:10:28 2016 -0700 Make deploy jar name easier to grep for commit d3823aa Author: Ian O Connell <[email protected]> Date: Tue Aug 9 11:40:40 2016 -0700 Use bind's for twitter scrooge so local repo's can override scrooge versions commit f11e814 Merge: f82800b 4146c9e Author: Ian O Connell <[email protected]> Date: Thu Jul 28 16:28:02 2016 -0700 Merge branch 'master' of github.com:bazelbuild/rules_scala commit f82800b Author: Ian O Connell <[email protected]> Date: Thu Jul 28 15:32:14 2016 -0700 Fix repl test commit 9c3f37c Author: Ian O Connell <[email protected]> Date: Thu Jul 28 15:30:46 2016 -0700 Update test rules * Handle passing through jvm options correctly, fix up merges from master * Set bazel version to a tag * Bump bazel version * Remove cat * Review comments. Collapsing code, killing code * delete code, unused import statements. * Remove unused code/ unneeded comments * Add the jvm options back in for usage with javac * Review comments * Add comment describing the args to a worker cmd * Move compile java sources to its own method * Kill maven_jar of guava * Update readme * Readme edit
seems to work for the targets in the repo.
Enough to pass the tests, at least. Now just need resources.