Skip to content

8347755: Support static library in jmod #46

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

Open
wants to merge 8 commits into
base: hermetic-java-runtime
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2891,7 +2891,7 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, JVMFlagOrigin
} else {
log_info(hermetic)(
"Use hermetic JDK from %s, hermetic packaged modules image starts at: "
PTR_FORMAT ", size: %zu",
JULONG_FORMAT_X ", size: %zu",
_hermetic_jdk_image_path,
_hermetic_jdk_jimage_offset,
_hermetic_jdk_jimage_size);
Expand Down
19 changes: 14 additions & 5 deletions src/java.base/share/classes/jdk/internal/jmod/JmodFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,14 @@
public class JmodFile implements AutoCloseable {
// jmod magic number and version number
private static final int JMOD_MAJOR_VERSION = 0x01;
private static final int JMOD_MINOR_VERSION = 0x00;
private static final int JMOD_MINOR_VERSION = 0x01;
private static final byte[] JMOD_MAGIC_NUMBER = {
0x4A, 0x4D, /* JM */
JMOD_MAJOR_VERSION, JMOD_MINOR_VERSION, /* version 1.0 */
JMOD_MAJOR_VERSION, JMOD_MINOR_VERSION, /* version 1.1 */
};
private static final byte[] JMOD_MAGIC_NO_STATIC_LIB = {
0x4A, 0x4D, /* JM */
JMOD_MAJOR_VERSION, 0x00, /* version 1.0 */
};

public static void checkMagic(Path file) throws IOException {
Expand Down Expand Up @@ -79,7 +83,8 @@ public static enum Section {
LEGAL_NOTICES("legal"),
MAN_PAGES("man"),
NATIVE_LIBS("lib"),
NATIVE_CMDS("bin");
NATIVE_CMDS("bin"),
STATIC_LIBS("static-lib");

private final String jmodDir;
private Section(String jmodDir) {
Expand Down Expand Up @@ -182,8 +187,12 @@ public JmodFile(Path file) throws IOException {
this.zipfile = new ZipFile(file.toFile());
}

public static void writeMagicNumber(OutputStream os) throws IOException {
os.write(JMOD_MAGIC_NUMBER);
public static void writeMagicNumber(OutputStream os, boolean withStaticLib) throws IOException {
if (withStaticLib) {
os.write(JMOD_MAGIC_NUMBER);
} else {
os.write(JMOD_MAGIC_NO_STATIC_LIB);
}
Comment on lines +190 to +195
Copy link
Collaborator

Choose a reason for hiding this comment

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

This conditionality seems a bit strange. Isn't the idea for the magic to be changed when the format got extended to do more things? I.e. shouldn't this be micro version 0x01 in both cases? Then when dealing with version JMOD 1.1 version you'd be checking if static-libs is there.

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 idea is without static lib, it's essential the same as the old format, using the old version so that old tools can be used if static is not included.

Copy link
Member

Choose a reason for hiding this comment

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

I thought the idea was that the jmod format was left intentionally undefined, to not think you could rely on anything else than the corresponding JDK tools to process it? That would mean backwards compatibility should not be a concern.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Just to be clear, this is about the JMOD format, not the jimage format. The jimage format is JDK internal and we are free to change it from release to release, even build to build, without compatibility concerns.)

You are right that the JMOD format is not documented but care is required when rev'ing the format because there are JDK tools that produce and consume this format. It should be possible for a project to produce a module in JMOD format with the jmod tool from JDK N, and put the package module on the module path when creating a run-time image with JDK N+1 jlink. So what Henry has looks right. It means the jmod tool will produce a 1.0 format when creating a package module that doesn't have static libs, and a JMOD in the new format when the module has static libs. This seems preferable to adding a --release option.

Choose a reason for hiding this comment

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

I guess its worth asking how would the format evolve in the future if more than /static-lib were added? Just as a strawman: say /docs and /sources were added in some sequence. Would this grow to say

if (<nothing new>) {
   // 1.0
}
else if (<just static-lib>) {
   // 1.1
}
else {
   // 1.2
}

What if /sources (and corresponding generation of src.zip) were added first and /docs second?

if (<nothing new>) {
   // 1.0
}
else if (<just static-lib>) {
   // 1.1
}
else if (<just sources +/- static-lib>) {
   // 1.2
}
else {
   // 1.3
}

Or would, at that point, versioning move to be closer to what is done for classfiles.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess its worth asking how would the format evolve in the future if more than /static-lib were added?

Unlike the JAR format, the JMOD format requires an update when new sections are added. We don't need to get hung up on any of the details around this now, most of the work to support this will be in jlink, not the jmod tool.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm ok with this for the leyden branch.

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 am thinking just to support -1 version. So 1.2 jmod can produce 1.2 with latest feature and 1.1 for previous version.
Anyway, as Alan pointed out, we don't need to figure this out right now.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,20 @@ class JmodOutputStream extends OutputStream implements AutoCloseable {
* This method creates (or overrides, if exists) the JMOD file,
* returning the output stream to write to the JMOD file.
*/
static JmodOutputStream newOutputStream(Path file, LocalDateTime date, int compressLevel) throws IOException {
static JmodOutputStream newOutputStream(Path file, LocalDateTime date, int compressLevel, boolean withStaticLib) throws IOException {
OutputStream out = Files.newOutputStream(file);
BufferedOutputStream bos = new BufferedOutputStream(out);
return new JmodOutputStream(bos, date, compressLevel);
return new JmodOutputStream(bos, date, compressLevel, withStaticLib);
}

private final ZipOutputStream zos;
private final LocalDateTime date;
private JmodOutputStream(OutputStream out, LocalDateTime date, int compressLevel) {
private JmodOutputStream(OutputStream out, LocalDateTime date, int compressLevel, boolean withStaticLib) {
this.zos = new ZipOutputStream(out);
this.zos.setLevel(compressLevel);
this.date = date;
try {
JmodFile.writeMagicNumber(out);
JmodFile.writeMagicNumber(out, withStaticLib);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand Down
17 changes: 15 additions & 2 deletions src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ private static String getMessageOrKey(String key, Object... args) {
private static final Path CWD = Paths.get("");

private Options options;
private boolean withStaticLibs = false;
private PrintWriter out = new PrintWriter(System.out, true);
void setLog(PrintWriter out, PrintWriter err) {
this.out = out;
Expand Down Expand Up @@ -155,6 +156,7 @@ static class Options {
List<Path> cmds;
List<Path> configs;
List<Path> libs;
List<Path> staticLibs;
List<Path> headerFiles;
List<Path> manPages;
List<Path> legalNotices;;
Expand Down Expand Up @@ -440,7 +442,7 @@ private boolean create() throws IOException {
Path target = options.jmodFile;
Path tempTarget = jmodTempFilePath(target);
try {
try (JmodOutputStream jos = JmodOutputStream.newOutputStream(tempTarget, options.date, options.compressLevel)) {
try (JmodOutputStream jos = JmodOutputStream.newOutputStream(tempTarget, options.date, options.compressLevel, withStaticLibs)) {
jmod.write(jos);
}
Files.move(tempTarget, target);
Expand All @@ -465,6 +467,7 @@ private static Path jmodTempFilePath(Path target) throws IOException {
private class JmodFileWriter {
final List<Path> cmds = options.cmds;
final List<Path> libs = options.libs;
final List<Path> staticLibs = options.staticLibs;
final List<Path> configs = options.configs;
final List<Path> classpath = options.classpath;
final List<Path> headerFiles = options.headerFiles;
Expand Down Expand Up @@ -495,6 +498,7 @@ void write(JmodOutputStream out) throws IOException {
processSection(out, Section.MAN_PAGES, manPages);
processSection(out, Section.NATIVE_CMDS, cmds);
processSection(out, Section.NATIVE_LIBS, libs);
processSection(out, Section.STATIC_LIBS, staticLibs);

}

Expand Down Expand Up @@ -1026,7 +1030,7 @@ private void updateJmodFile(Path target, Path tempTarget,
{

try (JmodFile jf = new JmodFile(target);
JmodOutputStream jos = JmodOutputStream.newOutputStream(tempTarget, options.date, options.compressLevel))
JmodOutputStream jos = JmodOutputStream.newOutputStream(tempTarget, options.date, options.compressLevel, withStaticLibs))
{
jf.stream().forEach(e -> {
try (InputStream in = jf.getInputStream(e.section(), e.name())) {
Expand Down Expand Up @@ -1399,6 +1403,11 @@ private void handleOptions(String[] args) {
.withRequiredArg()
.withValuesConvertedBy(DirPathConverter.INSTANCE);

OptionSpec<List<Path>> staticLibs
= parser.accepts("static-libs", getMessage("main.opt.static-libs"))
.withRequiredArg()
.withValuesConvertedBy(DirPathConverter.INSTANCE);

OptionSpec<List<Path>> legalNotices
= parser.accepts("legal-notices", getMessage("main.opt.legal-notices"))
.withRequiredArg()
Expand Down Expand Up @@ -1490,6 +1499,10 @@ private void handleOptions(String[] args) {
options.excludes = opts.valuesOf(excludes); // excludes is repeatable
if (opts.has(libs))
options.libs = getLastElement(opts.valuesOf(libs));
if (opts.has(staticLibs)) {
options.staticLibs = getLastElement(opts.valuesOf(staticLibs));
withStaticLibs = !options.staticLibs.isEmpty();
}
if (opts.has(headerFiles))
options.headerFiles = getLastElement(opts.valuesOf(headerFiles));
if (opts.has(manPages))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ main.opt.help-extra=Print help on extra options
main.opt.version=Version information
main.opt.class-path=Application jar files|dir containing classes
main.opt.libs=Location of native libraries
main.opt.static-libs=Location of native static libraries
main.opt.cmds=Location of native commands
main.opt.config=Location of user-editable config files
main.opt.extractDir=Target directory for extract
Expand Down
64 changes: 60 additions & 4 deletions test/jdk/tools/jmod/JmodTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public class JmodTest {
static final String CLASSES_PREFIX = "classes/";
static final String CMDS_PREFIX = "bin/";
static final String LIBS_PREFIX = "lib/";
static final String STATIC_LIBS_PREFIX = "static-lib/";
static final String CONFIGS_PREFIX = "conf/";

@BeforeTest
Expand All @@ -88,6 +89,7 @@ public void buildExplodedModules() throws IOException {
"jdk/test/foo/resources/foo.properties");
createCmds(dir.resolve("bin"));
createLibs(dir.resolve("lib"));
createStaticLibs(dir.resolve("static-lib"));
createConfigs(dir.resolve("conf"));
}

Expand Down Expand Up @@ -301,12 +303,14 @@ public void testExtractDir() throws IOException {
Path cp = EXPLODED_DIR.resolve("foo").resolve("classes");
Path bp = EXPLODED_DIR.resolve("foo").resolve("bin");
Path lp = EXPLODED_DIR.resolve("foo").resolve("lib");
Path slp = EXPLODED_DIR.resolve("foo").resolve("static-lib");
Path cf = EXPLODED_DIR.resolve("foo").resolve("conf");

jmod("create",
"--conf", cf.toString(),
"--cmds", bp.toString(),
"--libs", lp.toString(),
"--static-libs", slp.toString(),
"--class-path", cp.toString(),
MODS_DIR.resolve("fooExtractDir.jmod").toString())
.assertSuccess();
Expand All @@ -328,8 +332,10 @@ public void testExtractDir() throws IOException {
p.resolve("classes/jdk/test/foo/Foo.class"));
assertSameContent(bp.resolve("first"),
p.resolve(CMDS_PREFIX).resolve("first"));
assertSameContent(lp.resolve("first.so"),
assertSameContent(lp.resolve("second.so"),
p.resolve(LIBS_PREFIX).resolve("second.so"));
assertSameContent(slp.resolve("third").resolve("third.a"),
p.resolve(STATIC_LIBS_PREFIX).resolve("third").resolve("third.a"));
assertSameContent(cf.resolve("second.cfg"),
p.resolve(CONFIGS_PREFIX).resolve("second.cfg"));
});
Expand Down Expand Up @@ -435,6 +441,32 @@ public void testLibs() throws IOException {
assertJmodContent(jmod, expectedFilenames);
}
});

assertMagic(jmod, 0);
}

@Test
public void testStaticLibs() throws IOException {
Path jmod = MODS_DIR.resolve("fooLibs.jmod");
FileUtils.deleteFileIfExistsWithRetry(jmod);
Path cp = EXPLODED_DIR.resolve("foo").resolve("classes");
Path slp = EXPLODED_DIR.resolve("foo").resolve("static-lib");

jmod("create",
"--static-libs=" + slp.toString(),
"--class-path", cp.toString(),
jmod.toString())
.assertSuccess()
.resultChecker(r -> {
try (Stream<String> s1 = findFiles(slp).map(p -> STATIC_LIBS_PREFIX + p);
Stream<String> s2 = findFiles(cp).map(p -> CLASSES_PREFIX + p)) {
Set<String> expectedFilenames = Stream.concat(s1,s2)
.collect(toSet());
assertJmodContent(jmod, expectedFilenames);
}
});

assertMagic(jmod, 1);
}

@Test
Expand All @@ -444,22 +476,26 @@ public void testAll() throws IOException {
Path cp = EXPLODED_DIR.resolve("foo").resolve("classes");
Path bp = EXPLODED_DIR.resolve("foo").resolve("bin");
Path lp = EXPLODED_DIR.resolve("foo").resolve("lib");
Path slp = EXPLODED_DIR.resolve("foo").resolve("static-lib");
Path cf = EXPLODED_DIR.resolve("foo").resolve("conf");

jmod("create",
"--conf", cf.toString(),
"--cmds=" + bp.toString(),
"--libs=" + lp.toString(),
"--static-libs=" + slp.toString(),
"--class-path", cp.toString(),
jmod.toString())
.assertSuccess()
.resultChecker(r -> {
try (Stream<String> s1 = findFiles(lp).map(p -> LIBS_PREFIX + p);
Stream<String> s2 = findFiles(cp).map(p -> CLASSES_PREFIX + p);
Stream<String> s3 = findFiles(bp).map(p -> CMDS_PREFIX + p);
Stream<String> s4 = findFiles(cf).map(p -> CONFIGS_PREFIX + p)) {
Set<String> expectedFilenames = Stream.concat(Stream.concat(s1,s2),
Stream.concat(s3, s4))
Stream<String> s4 = findFiles(cf).map(p -> CONFIGS_PREFIX + p);
Stream<String> s5 = findFiles(slp).map(p -> STATIC_LIBS_PREFIX + p)) {
Set<String> expectedFilenames = Stream.concat(Stream.concat(Stream.concat(s1,s2),
Stream.concat(s3, s4)),
s5)
.collect(toSet());
assertJmodContent(jmod, expectedFilenames);
}
Expand Down Expand Up @@ -920,6 +956,20 @@ static void assertSameContent(Path p1, Path p2) {
}
}

static void assertMagic(Path file, int minorVersion) throws IOException {
try (InputStream in = Files.newInputStream(file)) {
// validate the header
byte[] magic = in.readNBytes(4);
if (magic.length != 4) {
throw new IOException("Invalid JMOD file: " + file);
}
assertEquals(magic[0], 0x4A); // J
assertEquals(magic[1], 0x4D); // M
assertEquals(magic[2], 0x01);
assertEquals(magic[3], minorVersion);
}
}

static JmodResult jmod(String... args) {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
PrintStream ps = new PrintStream(baos);
Expand Down Expand Up @@ -953,6 +1003,12 @@ static void createLibs(Path dir) throws IOException {
createFiles(dir, files);
}

static void createStaticLibs(Path dir) throws IOException {
List<String> files = Arrays.asList(
"first.a", "second.a", "third" + File.separator + "third.a");
createFiles(dir, files);
}

static void createConfigs(Path dir) throws IOException {
List<String> files = Arrays.asList(
"first.cfg", "second.cfg", "third" + File.separator + "third.cfg");
Expand Down