Allow both ZIP/JAR and directory output for GwtIncompatibleStripperCommandLineRunner (like J2clCommandLineRunner)

[tbroyer]

Is your feature request related to a problem? Please describe. Currently, GwtIncompatibleStripperCommandLineRunner unconditionally outputs into a ZIP file. This makes it impractical to javac those sources afterwards in non-Bazel environments as they have to be unzipped first.

Describe the solution you'd like Discriminate on the option value to allow either a ZIP or a directory, like J2clCommandLineRunner does.

Describe alternatives you've considered Alternatively, provide a public API that takes the source as a String or Reader and returns the output as a String or write to a Writer, processing one file at a time (e.g. make GwtIncompatibleStripper and its static String processFile(String fileContent) public)

Additional context For incremental processing (only processing those files that have changed), this would allow just giving the same output directory every time, and the tool would then overwrite those files that it was asked to process (e.g. call it with -d out src/p/A.java src/p/B.java, it will output out/p/A.java and out/p/B/java; then as src/p/A.java is changed by the developer, the tooling could call it again with -d out src/p/A.java this time –without src/p/B.java– and it would overwrite out/p/A.java and leave out/p/B.java untouched; it would of course be the tooling –Gradle, Maven, whatever– responsibility to delete out/p/X.java in case src/p/X.java is deleted by the developer)

This works great for GwtIncompatibleStripper because it processes files in isolation by design, at the AST level without the need to resolve type dependencies.

[tbroyer]

It's actually more complicated than that: because GwtIncompatibleStripper (currently) doesn't really look at the content of the file, it outputs it with the exact same path as received in argument, which means if you call it with src/p/A.java it'll output a src/p/A.java in the ZIP file, and if you call it with the absolute path to the same file, say /projects/my-project/src/p/A.java, it'll output a projects/my-project/src/p/A.java in the ZIP file.

So the alternative of exposing an API processing one file at a time would be more workable. Currently, not only the ZIP has to be unpacked, but the entry paths also need to be translated (e.g. from src/p/A.java to out/p/A.java)

The j2cl-maven-plugin currently uses GwtIncompatibleStripper.preprocessFiles(), passing a list of FileInfo, which encodes both the path to access the file and the relative output path; using the vertispan/j2cl fork of J2CL that exposes that method.

[gkdn]

Sorry I was out for vacation.

It is ok to support directory output similar to J2clCommandLineRunner. We would like to remove the zip completely anyway (including zipping and unzipping).

I understood the original request but didn't understand follow up comment.

If you pass src/p/A.java -d foo, it should output to foo/src/p/A.java. If you pass /projects/java/p/A.java -d foo, it should output to foo/java/p/A.java. (since 'java' would be considered a root) If you pass /projects/src/p/A.java -d foo, it should output to foo/projects/src/p/A.java.

That doesn't work for you?

[tbroyer]

If you pass /projects/java/p/A.java -d foo, it should output to foo/java/p/A.java. (since 'java' would be considered a root)

Are you sure about that rule? That might be the intended behavior, but not the actual one.

$ jar tf ../../google/j2cl/bazel-bin/jre/java/jre_j2cl_stripped-src.jar |head
bazel-out/
bazel-out/k8-fastbuild/
bazel-out/k8-fastbuild/bin/
bazel-out/k8-fastbuild/bin/jre/
bazel-out/k8-fastbuild/bin/jre/java/
bazel-out/k8-fastbuild/bin/jre/java/external/
bazel-out/k8-fastbuild/bin/jre/java/external/org_gwtproject_gwt/
bazel-out/k8-fastbuild/bin/jre/java/external/org_gwtproject_gwt/user/
bazel-out/k8-fastbuild/bin/jre/java/external/org_gwtproject_gwt/user/super/
bazel-out/k8-fastbuild/bin/jre/java/external/org_gwtproject_gwt/user/super/com/
$ jar tf ../../google/j2cl/bazel-bin/junit/emul/java/junit_emul_j2cl_stripped-src.jar |head
junit/
junit/emul/
junit/emul/java/
junit/emul/java/junit/
junit/emul/java/junit/framework/
junit/emul/java/junit/framework/Assert.java
junit/emul/java/junit/framework/AssertionFailedError.java
junit/emul/java/junit/framework/ComparisonCompactor.java
junit/emul/java/junit/framework/ComparisonFailure.java
junit/emul/java/junit/framework/Protectable.java

That logic is in FrontendUtils.getJavaPath, which is used in FrontendUtils.getAllSources to populate a FileInfo's targetPath: https://github.com/google/j2cl/blob/69a295d3afa5011fcb6c595f807f72700a8c7df8/transpiler/java/com/google/j2cl/common/FrontendUtils.java#L102 but GwtIncompatibleStripper uses the originalPath: https://github.com/google/j2cl/blob/69a295d3afa5011fcb6c595f807f72700a8c7df8/tools/java/com/google/j2cl/tools/gwtincompatible/GwtIncompatibleStripper.java#L83-L85

If you pass /projects/src/p/A.java -d foo, it should output to foo/projects/src/p/A.java.

That doesn't work for you?

Gradle doesn't guarantee the working directory (see note in https://docs.gradle.org/6.5/userguide/working_with_files.html#sec:single_file_paths), which forces us to pass absolute file paths. That puts us in that last case (or the previous one if GwtIncompatibleStripper switches from originalPath to targetPath), which then is (possibly) non-relocatable, so post-processing would still be required to copy files to paths that are independent from where the project is checked out on disk.

I'm currently doing just that when unzipping (my current code probably doesn't work on Windows, but that should be easily fixable)

[gkdn]

That might be the intended behavior, but not the actual one.

Yes that's the intended behavior; I don't remember any reason not to follow that convention so we can probably change that behavior.

We can also make the tool run via stdin/stdout which is probably the simplest contract for such a tool. WDYT?

[tbroyer]

We can also make the tool run via stdin/stdout which is probably the simplest contract for such a tool. WDYT?

If you mean System.in/System.out, that's unfortunately not threadsafe and I'm not sure how well/easy it would be to integrate (without always forking a new process). But that's close to my proposal of having an API processing one file at a time by taking streams as arguments (or simply exposing the existing GwtIncompatbleStripper.processFile(String))

public static void processFile(Reader in, Writer out) throws IOException { // <- could be Readable and Appendable too, or InputStream/OutputStream
   String fileContent = CharStreams.toString(in);
   String processedFileContent = processFile(fileContent); // <- that's the existing processFile
   out.write(processedFileContent);
}

[gkdn]

Does fixing If you pass /projects/java/p/A.java -d foo, it should output to foo/java/p/A.java work for your case?

[gkdn]

Following sounds good to me:

public static String strip(String content) throws IOException {}