search

Vertispan / j2clmavenplugin

Maven plugin to launch new J2CL compilation
https://vertispan.github.io/j2clmavenplugin/
Apache License 2.0
53 stars 27 forks source link

Save closure compiler logs in output.log file #99

Open xamde opened 2 years ago

xamde commented 2 years ago

I have a multi-module reactor build which fails (that can happen) and I see a warning message in the shell when running mavenm but not in console.log. Also, we should put the exit code of closure into the output.log.

0 error and 0 warnings seems to be an overly optimistic self-assessment of closure compiler :-)

The output.log in question contains only the lines

[INFO]  0 error(s), 0 warning(s)

[DEBUG] --js_module_root
(1000 of js paths follow)

In the console I get

[INFO] com.example.foo:bar-client:1.0-SNAPSHOT/optimized_js: 0 error(s), 0 warning(s)

java.lang.IllegalStateException: Duplicate module paths after resolving: [/C-/Users/xamde/.

(1000s of paths, some indicating a x 2, which is helpful to nail down the error)

, /jsinterop/base/jsinterop.js, /jsinterop/base/Js.java.js]
        at com.google.common.base.Preconditions.checkState(Preconditions.java:589)
        at com.google.javascript.jscomp.deps.ModuleLoader.resolvePaths(ModuleLoader.java:273)
        at com.google.javascript.jscomp.deps.ModuleLoader.<init>(ModuleLoader.java:149)
        at com.google.javascript.jscomp.deps.ModuleLoader.<init>(ModuleLoader.java:48)
        at com.google.javascript.jscomp.deps.ModuleLoader$Builder.build(ModuleLoader.java:139)
        at com.google.javascript.jscomp.Compiler.initializeModuleLoader(Compiler.java:1814)
        at com.google.javascript.jscomp.Compiler.parseInputs(Compiler.java:1858)
        at com.google.javascript.jscomp.Compiler.parseForCompilationInternal(Compiler.java:1127)
        at com.google.javascript.jscomp.Compiler.lambda$parseForCompilation$12(Compiler.java:1110)
        at com.google.javascript.jscomp.CompilerExecutor.lambda$runInCompilerThread$0(CompilerExecutor.java:101)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
        at java.base/java.lang.Thread.run(Thread.java:832)
[ERROR] Exception executing task com.example.foo:bar-client:1.0-SNAPSHOT/optimized_js
java.lang.IllegalStateException: Closure Compiler failed, check log for details
    at com.vertispan.j2cl.build.provided.ClosureTask$2.execute (ClosureTask.java:148)
    at com.vertispan.j2cl.build.TaskScheduler$2.executeTask (TaskScheduler.java:172)
    at com.vertispan.j2cl.build.TaskScheduler$2.lambda$onReady$0 (TaskScheduler.java:211)
    at java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:515)
    at java.util.concurrent.FutureTask.run (FutureTask.java:264)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run (ScheduledThreadPoolExecutor.java:304)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1130)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:630)
    at java.lang.Thread.run (Thread.java:832)
java.lang.RuntimeException
        at com.vertispan.j2cl.build.DiskCache.markFailed(DiskCache.java:443)
        at com.vertispan.j2cl.build.DiskCache$CacheResult.markFailure(DiskCache.java:53)
        at com.vertispan.j2cl.build.TaskScheduler$2.executeTask(TaskScheduler.java:181)
        at com.vertispan.j2cl.build.TaskScheduler$2.lambda$onReady$0(TaskScheduler.java:211)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
        at java.base/java.lang.Thread.run(Thread.java:832)

The closure compiler takes at construction time a PrintStream for err and out. These streams need to be redirected to both the console and the log file on disk. This requires extending the DiskCache to supply a PrintStream. I had a local hacked version of doing this, but struggled to put it niceley into the layered architecture of maven plugin > build services > disk cache

niloc132 commented 2 years ago

Just hit the very same error in https://github.com/Vertispan/j2clmavenplugin/pull/10. The streams should be avoided where possible (all errors/warnings and other build feedback are handled via our own impl of closure's ErrorManager now).

I was trying to avoid replacing the PrintStreams (as you can't tell if it is an error or a warning, etc) and thought it was sufficient to just supply the ErrorManager, but I see now that isn't the case. Either we need to implement that, or inline away more and more of AbstractCommandLineRunner, possibly only interacting with the Compiler directly. It is inevitable, but I'd hoped to get away with it a bit longer...

niloc132 commented 2 years ago

Might be worth offering a patch upstream to thin out that error message and only show the x 2 items - the others seem to just be noise?

xamde commented 2 years ago

Although the closure logs about all JS files are very verbose, I would still love the ability to see them as a developer. They let me spot patterns of which resources were seen and which not. In my local hacky spike I used a regex to say: "Found {n} duplicated resources on path, one of them is {sample}. See logs at (path) for details."

Am Fr., 21. Jan. 2022 um 05:10 Uhr schrieb Colin Alworth < @.***>:

Might be worth offering a patch upstream to thin out that error message and only show the x 2 items - the others seem to just be noise?

— Reply to this email directly, view it on GitHub https://github.com/Vertispan/j2clmavenplugin/issues/99#issuecomment-1018155706, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHCQBDGJXDWS23YCYWMSM3UXDLZDANCNFSM5MLYOC3Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

-- @.*** | http://Xam.de

niloc132 commented 2 years ago

If you are interested in offering a patch to upstream closure-compiler, try a patch where this error is modified to not return the entire multiset, but only the elements which have more than one entry:

https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/deps/ModuleLoader.java#L263-L276

If they aren't interested for some reason in accepting that, we can consider adding it to our list of cherry-picked fixes on top of closure-compiler.

xamde commented 2 years ago

I actually do like having the entire multiset. I just propose to additionally add a one-line auto-generated summary. Maybe I can try to propose that as a patch. Thanks for the link.

Am Fr., 21. Jan. 2022 um 15:41 Uhr schrieb Colin Alworth < @.***>:

If you are interested in offering a patch to upstream closure-compiler, try a patch where this error is modified to not return the entire multiset, but only the elements which have more than one entry:

https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/deps/ModuleLoader.java#L263-L276

If they aren't interested for some reason in accepting that, we can consider adding it to our list of cherry-picked fixes on top of closure-compiler.

— Reply to this email directly, view it on GitHub https://github.com/Vertispan/j2clmavenplugin/issues/99#issuecomment-1018565085, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHCQBGWU2HKNRQCN5DL4T3UXFV75ANCNFSM5MLYOC3Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

-- @.*** | http://Xam.de

niloc132 commented 2 years ago

Sorry, I decided for the moment at least to log these errors in a single block, but only list the conflicts, so that there are at least line breaks between the duplicate files. It turns out we can't always detect duplicates, so I want to warn as clearly as possible when we can.

https://github.com/Vertispan/j2clmavenplugin/pull/109

I will plan on adding additional debug logging around logging all files across what closure would have logged in the multi-set, as well as addressing the original issue in this bug report.

niloc132 commented 12 months ago

I believe this may be fixed with some of the other logging changes, but need to verify. Should also consider a flag to enable a "debugDir" - note that this flag is not available from the command line, can only be set by manipulating the CompilerOptions object.