From 0fd4ccab4d0e5340ea95b887d229178526675725 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Mon, 15 Aug 2016 16:35:29 -1000 Subject: [PATCH 1/4] improve the deploy jar creation --- scala/scala.bzl | 19 ++---- .../io/bazel/rulesscala/jar/JarCreator.java | 11 ++- .../io/bazel/rulesscala/jar/JarHelper.java | 68 ++++++++++++++----- 3 files changed, 67 insertions(+), 31 deletions(-) diff --git a/scala/scala.bzl b/scala/scala.bzl index 1820c0ec3..35bfa8040 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -233,25 +233,15 @@ def _compile_or_empty(ctx, jars, srcjars, buildijar): return struct(ijar=ijar, class_jar=ctx.outputs.jar) def _build_deployable(ctx, jars): - cmd = "rm -rf {out}_tmp\n" - cmd += "mkdir -p {out}_tmp\n" - for jar in jars: - cmd += "unzip -o {jar} -d {{out}}_tmp >/dev/null\n".format(jar=jar.path) - cmd += "{java} -jar {jar} -m {manifest} {out} {out}_tmp\n" - cmd += "rm -rf {out}_tmp\n" - - cmd = cmd.format( - out=ctx.outputs.deploy_jar.path, - jar=_get_jar_path(ctx.files._jar), - java=ctx.file._java.path, - manifest=ctx.outputs.manifest.path) + args = ["-m", ctx.outputs.manifest.path, ctx.outputs.deploy_jar.path] + args.extend([j.path for j in jars]) ctx.action( inputs=list(jars) + ctx.files._jdk + ctx.files._jar + [ctx.outputs.manifest], outputs=[ctx.outputs.deploy_jar], - command=cmd, + executable=ctx.executable._jar_bin, mnemonic="ScalaDeployJar", progress_message="scala deployable %s" % ctx.label, - arguments=[]) + arguments=args) def write_manifest(ctx): # TODO(bazel-team): I don't think this classpath is what you want @@ -471,6 +461,7 @@ _implicit_deps = { "_java": attr.label(executable=True, default=Label("@bazel_tools//tools/jdk:java"), single_file=True, allow_files=True), "_javac": attr.label(executable=True, default=Label("@bazel_tools//tools/jdk:javac"), single_file=True, allow_files=True), "_jar": attr.label(executable=True, default=Label("//src/java/io/bazel/rulesscala/jar:jar_deploy.jar"), allow_files=True), + "_jar_bin": attr.label(executable=True, default=Label("//src/java/io/bazel/rulesscala/jar")), "_jdk": attr.label(default=Label("//tools/defaults:jdk"), allow_files=True), } diff --git a/src/java/io/bazel/rulesscala/jar/JarCreator.java b/src/java/io/bazel/rulesscala/jar/JarCreator.java index 231404561..96641b2e8 100644 --- a/src/java/io/bazel/rulesscala/jar/JarCreator.java +++ b/src/java/io/bazel/rulesscala/jar/JarCreator.java @@ -70,6 +70,9 @@ public void addDirectory(String directory) { addDirectory(null, new File(directory)); } + public void addZip(String name) { + jarEntries.put(name, (new File(name)).getAbsolutePath()); + } /** * Adds the contents of a directory to the Jar file. All files below this * directory will be added to the Jar file using the prefix and the name @@ -189,7 +192,13 @@ public static void main(String[] args) { JarCreator createJar = new JarCreator(output); createJar.setManifestFile(manifestFile); for (int i = (idx+1); i < args.length; i++) { - createJar.addDirectory(args[i]); + String thisName = args[i]; + if (JarHelper.isJar(thisName)) { + createJar.addZip(thisName); + } + else { + createJar.addDirectory(thisName); + } } createJar.setCompression(true); createJar.setNormalize(true); diff --git a/src/java/io/bazel/rulesscala/jar/JarHelper.java b/src/java/io/bazel/rulesscala/jar/JarHelper.java index 62536023d..b842edb2d 100644 --- a/src/java/io/bazel/rulesscala/jar/JarHelper.java +++ b/src/java/io/bazel/rulesscala/jar/JarHelper.java @@ -16,15 +16,18 @@ import java.io.File; import java.io.FileInputStream; +import java.io.InputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.nio.file.Files; +import java.util.Enumeration; import java.util.HashSet; import java.util.Set; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.jar.JarOutputStream; import java.util.zip.CRC32; +import java.util.zip.ZipException; /** * A simple helper class for creating Jar files. All Jar entries are sorted alphabetically. Allows @@ -61,6 +64,10 @@ public JarHelper(String filename) { jarFile = filename; } + public static boolean isJar(String name) { + return name.endsWith(".jar"); + } + /** * Enables or disables the Jar entry normalization. * @@ -189,24 +196,53 @@ protected void copyEntry(String name, File file) throws IOException { System.err.println("adding " + file); } // Create a new entry - long size = isDirectory ? 0 : file.length(); - JarEntry outEntry = new JarEntry(name); - long newtime = normalize ? normalizedTimestamp(name) : file.lastModified(); - outEntry.setTime(newtime); - outEntry.setSize(size); - if (size == 0L) { - outEntry.setMethod(JarEntry.STORED); - outEntry.setCrc(0); - out.putNextEntry(outEntry); - } else { - outEntry.setMethod(storageMethod); - if (storageMethod == JarEntry.STORED) { - outEntry.setCrc(hashFile(file)); + if (JarHelper.isJar(name)) { + byte[] buffer = new byte[2048]; + JarFile nameJf = new JarFile(file); + for (Enumeration e = nameJf.entries(); e.hasMoreElements();) { + try { + JarEntry existing = e.nextElement(); + JarEntry outEntry = new JarEntry(existing.getName()); + outEntry.setTime(existing.getTime()); + outEntry.setSize(existing.getSize()); + out.putNextEntry(outEntry); + InputStream in = nameJf.getInputStream(existing); + while (0 < in.available()){ + int read = in.read(buffer); + out.write(buffer, 0, read); + } + in.close(); + out.closeEntry(); + } catch (ZipException ze) { + if (ze.getMessage().contains("duplicate entry")) { + // We ignore these and just take the last. I hope the consumers have tests! + } + else { + throw ze; + } + } + } + } + else { + long size = isDirectory ? 0 : file.length(); + JarEntry outEntry = new JarEntry(name); + long newtime = normalize ? normalizedTimestamp(name) : file.lastModified(); + outEntry.setTime(newtime); + outEntry.setSize(size); + if (size == 0L) { + outEntry.setMethod(JarEntry.STORED); + outEntry.setCrc(0); + out.putNextEntry(outEntry); + } else { + outEntry.setMethod(storageMethod); + if (storageMethod == JarEntry.STORED) { + outEntry.setCrc(hashFile(file)); + } + out.putNextEntry(outEntry); + Files.copy(file.toPath(), out); } - out.putNextEntry(outEntry); - Files.copy(file.toPath(), out); + out.closeEntry(); } - out.closeEntry(); } } } From 0e043546fecf7185c1ab7eb42a9386c9abe5093f Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Tue, 16 Aug 2016 11:01:51 -1000 Subject: [PATCH 2/4] address review comments --- scala/scala.bzl | 5 +- .../io/bazel/rulesscala/jar/JarHelper.java | 54 ++++++++++--------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/scala/scala.bzl b/scala/scala.bzl index 35bfa8040..69ac17ddf 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -233,10 +233,13 @@ def _compile_or_empty(ctx, jars, srcjars, buildijar): return struct(ijar=ijar, class_jar=ctx.outputs.jar) def _build_deployable(ctx, jars): + # -m is the argument to pass a manifest to our jar creation code + # the next argument is the manifest itself, then the target jars + # finally the list of jars to merge args = ["-m", ctx.outputs.manifest.path, ctx.outputs.deploy_jar.path] args.extend([j.path for j in jars]) ctx.action( - inputs=list(jars) + ctx.files._jdk + ctx.files._jar + [ctx.outputs.manifest], + inputs=list(jars) + [ctx.outputs.manifest], outputs=[ctx.outputs.deploy_jar], executable=ctx.executable._jar_bin, mnemonic="ScalaDeployJar", diff --git a/src/java/io/bazel/rulesscala/jar/JarHelper.java b/src/java/io/bazel/rulesscala/jar/JarHelper.java index b842edb2d..4fbcc8908 100644 --- a/src/java/io/bazel/rulesscala/jar/JarHelper.java +++ b/src/java/io/bazel/rulesscala/jar/JarHelper.java @@ -65,7 +65,7 @@ public JarHelper(String filename) { } public static boolean isJar(String name) { - return name.endsWith(".jar"); + return name.endsWith(".jar") && ((new File(name)).isFile()); } /** @@ -177,6 +177,33 @@ protected void writeManifestEntry(byte[] content) throws IOException { } } + /** + * This copies the contents of jarFile into out + * This is a static method to make it clear what is mutated (and it + * was written by someone who really likes to minimize state changes). + */ + static private void copyJar(JarFile nameJf, Set names, JarOutputStream out) throws IOException { + byte[] buffer = new byte[2048]; + for (Enumeration e = nameJf.entries(); e.hasMoreElements();) { + JarEntry existing = e.nextElement(); + String name = existing.getName(); + if (!names.contains(name)) { + JarEntry outEntry = new JarEntry(name); + outEntry.setTime(existing.getTime()); + outEntry.setSize(existing.getSize()); + out.putNextEntry(outEntry); + InputStream in = nameJf.getInputStream(existing); + while (0 < in.available()) { + int read = in.read(buffer); + out.write(buffer, 0, read); + } + in.close(); + out.closeEntry(); + names.add(name); + } + } + } + /** * Copies file or directory entries from the file system into the jar. * Directory entries will be detected and their names automatically '/' @@ -197,31 +224,8 @@ protected void copyEntry(String name, File file) throws IOException { } // Create a new entry if (JarHelper.isJar(name)) { - byte[] buffer = new byte[2048]; JarFile nameJf = new JarFile(file); - for (Enumeration e = nameJf.entries(); e.hasMoreElements();) { - try { - JarEntry existing = e.nextElement(); - JarEntry outEntry = new JarEntry(existing.getName()); - outEntry.setTime(existing.getTime()); - outEntry.setSize(existing.getSize()); - out.putNextEntry(outEntry); - InputStream in = nameJf.getInputStream(existing); - while (0 < in.available()){ - int read = in.read(buffer); - out.write(buffer, 0, read); - } - in.close(); - out.closeEntry(); - } catch (ZipException ze) { - if (ze.getMessage().contains("duplicate entry")) { - // We ignore these and just take the last. I hope the consumers have tests! - } - else { - throw ze; - } - } - } + copyJar(nameJf, names, out); } else { long size = isDirectory ? 0 : file.length(); From d6c791a5992044bb601bd6e403540dcc1242ad45 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Tue, 16 Aug 2016 11:45:45 -1000 Subject: [PATCH 3/4] minor cleanup --- src/java/io/bazel/rulesscala/jar/JarCreator.java | 15 ++++++++------- src/java/io/bazel/rulesscala/jar/JarHelper.java | 6 +++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/java/io/bazel/rulesscala/jar/JarCreator.java b/src/java/io/bazel/rulesscala/jar/JarCreator.java index 96641b2e8..5c9f0bac9 100644 --- a/src/java/io/bazel/rulesscala/jar/JarCreator.java +++ b/src/java/io/bazel/rulesscala/jar/JarCreator.java @@ -66,12 +66,12 @@ public boolean addEntry(String entryName, String fileName) { * * @param directory the directory to add to the jar */ - public void addDirectory(String directory) { - addDirectory(null, new File(directory)); + public void addDirectory(File directory) { + addDirectory(null, directory); } - public void addZip(String name) { - jarEntries.put(name, (new File(name)).getAbsolutePath()); + public void addZip(String name, File file) { + jarEntries.put(name, file.getAbsolutePath()); } /** * Adds the contents of a directory to the Jar file. All files below this @@ -193,11 +193,12 @@ public static void main(String[] args) { createJar.setManifestFile(manifestFile); for (int i = (idx+1); i < args.length; i++) { String thisName = args[i]; - if (JarHelper.isJar(thisName)) { - createJar.addZip(thisName); + File f = new File(thisName); + if (JarHelper.isJar(thisName, f)) { + createJar.addZip(thisName, f); } else { - createJar.addDirectory(thisName); + createJar.addDirectory(f); } } createJar.setCompression(true); diff --git a/src/java/io/bazel/rulesscala/jar/JarHelper.java b/src/java/io/bazel/rulesscala/jar/JarHelper.java index 4fbcc8908..4c1cb1658 100644 --- a/src/java/io/bazel/rulesscala/jar/JarHelper.java +++ b/src/java/io/bazel/rulesscala/jar/JarHelper.java @@ -64,8 +64,8 @@ public JarHelper(String filename) { jarFile = filename; } - public static boolean isJar(String name) { - return name.endsWith(".jar") && ((new File(name)).isFile()); + public static boolean isJar(String name, File file) { + return name.endsWith(".jar") && (file.isFile()); } /** @@ -223,7 +223,7 @@ protected void copyEntry(String name, File file) throws IOException { System.err.println("adding " + file); } // Create a new entry - if (JarHelper.isJar(name)) { + if (JarHelper.isJar(name, file)) { JarFile nameJf = new JarFile(file); copyJar(nameJf, names, out); } From d21b4a73cca657e3eef44ffeed60542103f34a97 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Tue, 16 Aug 2016 18:09:48 -1000 Subject: [PATCH 4/4] clean the code a bit --- scala/scala.bzl | 13 +++++++++++-- src/java/io/bazel/rulesscala/jar/JarCreator.java | 8 ++++---- src/java/io/bazel/rulesscala/jar/JarHelper.java | 6 +++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/scala/scala.bzl b/scala/scala.bzl index 69ac17ddf..69d458209 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -233,9 +233,18 @@ def _compile_or_empty(ctx, jars, srcjars, buildijar): return struct(ijar=ijar, class_jar=ctx.outputs.jar) def _build_deployable(ctx, jars): + # the _jar_bin program we call below expects one optional argument: # -m is the argument to pass a manifest to our jar creation code - # the next argument is the manifest itself, then the target jars - # finally the list of jars to merge + # the next argument is the path manifest itself + # the manifest is set up by methods that call this function (see usages + # of _build_deployable and note that they always first call write_manifest). + # that is what creates the manifest content + # + # following the manifest argument and the manifest, the next argument is + # the output path for the target jar + # + # finally all the rest of the arguments are jars to be flattened into one + # fat jar args = ["-m", ctx.outputs.manifest.path, ctx.outputs.deploy_jar.path] args.extend([j.path for j in jars]) ctx.action( diff --git a/src/java/io/bazel/rulesscala/jar/JarCreator.java b/src/java/io/bazel/rulesscala/jar/JarCreator.java index 5c9f0bac9..fd2281dce 100644 --- a/src/java/io/bazel/rulesscala/jar/JarCreator.java +++ b/src/java/io/bazel/rulesscala/jar/JarCreator.java @@ -70,8 +70,8 @@ public void addDirectory(File directory) { addDirectory(null, directory); } - public void addZip(String name, File file) { - jarEntries.put(name, file.getAbsolutePath()); + public void addJar(File file) { + jarEntries.put(file.getAbsolutePath(), file.getAbsolutePath()); } /** * Adds the contents of a directory to the Jar file. All files below this @@ -194,8 +194,8 @@ public static void main(String[] args) { for (int i = (idx+1); i < args.length; i++) { String thisName = args[i]; File f = new File(thisName); - if (JarHelper.isJar(thisName, f)) { - createJar.addZip(thisName, f); + if (JarHelper.isJar(f)) { + createJar.addJar(f); } else { createJar.addDirectory(f); diff --git a/src/java/io/bazel/rulesscala/jar/JarHelper.java b/src/java/io/bazel/rulesscala/jar/JarHelper.java index 4c1cb1658..d51956972 100644 --- a/src/java/io/bazel/rulesscala/jar/JarHelper.java +++ b/src/java/io/bazel/rulesscala/jar/JarHelper.java @@ -64,8 +64,8 @@ public JarHelper(String filename) { jarFile = filename; } - public static boolean isJar(String name, File file) { - return name.endsWith(".jar") && (file.isFile()); + public static boolean isJar(File file) { + return file.getName().endsWith(".jar") && (file.isFile()); } /** @@ -223,7 +223,7 @@ protected void copyEntry(String name, File file) throws IOException { System.err.println("adding " + file); } // Create a new entry - if (JarHelper.isJar(name, file)) { + if (JarHelper.isJar(file)) { JarFile nameJf = new JarFile(file); copyJar(nameJf, names, out); }