closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

--create_source_map throws exception on windows when using input source maps and chunks

Open DerGernTod opened this issue 3 years ago • 16 comments

Closure throws an exception when I try to build source maps for chunks: java.io.FileNotFoundException: .\dist\closure\common.js.map (The system cannot find the path specified) It seems like there's a race condition issue if the source map of a depending module is built before the dependency. It seems to work properly if I run the build without --create_source_map, and then run it again with --create_source_map.

Reproducer is available here: https://github.com/DerGernTod/ts-esbuild-closure-reproducer/blob/main/index.js

The used flags: --chunk common:2 --chunk_wrapper common:(function(){%s})() --source_map_input dist/esbuild/chunk-7THFJKJQ.js|dist/esbuild/chunk-7THFJKJQ.js.map --js dist/esbuild/chunk-7THFJKJQ.js --source_map_input dist/esbuild/filecommon.js|dist/esbuild/filecommon.js.map --js dist/esbuild/filecommon.js --chunk filea:1:common --chunk_wrapper filea:(function(){%s})() --source_map_input dist/esbuild/filea.js|dist/esbuild/filea.js.map --js dist/esbuild/filea.js --chunk fileb:1:common --chunk_wrapper fileb:(function(){%s})() --source_map_input dist/esbuild/fileb.js|dist/esbuild/fileb.js.map --js dist/esbuild/fileb.js --language_in ECMASCRIPT_2017 --language_out ECMASCRIPT_2017 --chunk_output_path_prefix ./dist/closure/ --rename_prefix_namespace myNamespace --compilation_level ADVANCED_OPTIMIZATIONS --create_source_map %outname%.map

I also tried --source_map_include_content, but that doesn't seem to work either (even if you build it once without source maps to prevent the error mentioned above, and then build it with source maps). The source is not embedded at all.

DerGernTod avatar Jun 29 '21 09:06 DerGernTod

Assigning to myself to investigate further

lauraharker avatar Jul 01 '21 20:07 lauraharker

I'm having trouble reproducing the race condition. Here's what I've tried:

$ git clone https://github.com/DerGernTod/ts-esbuild-closure-reproducer.git
$ cd ts-esbuild-closure-reproducer/
$ node --version
v16.4.1
$ npm install
$ npm start

> [email protected] start
> tsc && node index.js

done 0
$ ls dist/closure
common.js  common.js.map  filea.js  filea.js.map  fileb.js  fileb.js.map

@DerGernTod could you confirm that following the above steps triggers an exception for you?

lauraharker avatar Jul 01 '21 23:07 lauraharker

I have tried it out and can confirm that I can reproduce the issue. One thing to note: I am using node "v14.16.0" and it throws the FileNotFoundException. Just to be sure I tried to update to node v16.4.0 and the issue is still there. Unfortunately I can not update to v16.4.1, because there is an issue that prevents the update: https://github.com/nodejs/node/issues/39224

Also doesn't seem to happen randomly, I tried it 10 times for each version and the Exception happened every time.

Edit: Since the Exception is coming from Java, I also checked the closure compiler version: v20210601 And my java version: openjdk version "1.8.0_282"

I have tried the following version as well: openjdk version "16.0.1" 2021-04-20 OpenJDK Runtime Environment (build 16.0.1+9-24) OpenJDK 64-Bit Server VM (build 16.0.1+9-24, mixed mode, sharing)

Took it from https://jdk.java.net/16/ and added it as JAVA_HOME environment variable and verified it is used by calling java -version.

kamtschatka avatar Jul 02 '21 06:07 kamtschatka

Thanks for adding more information @kamtschatka.

Looks like you're on Windows since you're affected by https://github.com/nodejs/node/issues/39224, so I wonder if the bug is specific to Window. I was trying to reproduce on Linux.

@concavelenz could I pass this to you to investigate now that you've taken over triage duty?

lauraharker avatar Jul 08 '21 22:07 lauraharker

I just tested with openjdk 11 and 16 on wsl and it worked fine, so it seems really to be a problem that only appears on windows.

kamtschatka avatar Jul 09 '21 17:07 kamtschatka

i just ran a few more test with windows/linux and there's definitely differences in the produced source maps. on windows, it seems that --source_map_include_content is completely ignored, while on linux, --source_map_location_mapping doesn't do anything. can be reproduced with the reproducer repo above. the source maps created on windows and linux respectively look completely different (while the linux one seems to work properly though, at least with included content as far as i can tell)

DerGernTod avatar Jul 15 '21 12:07 DerGernTod

Some times back, I have implemented exactly what you are doing, that is, using input source maps and chunks on Windows. It was done as a part of https://github.com/theseanl/tscc.

A difference of my case is that, I am feeding JS files to the compiler via stdin, via --json_streams=BOTH flag. In the JSON format that the compiler accepts, there is a path field you have to specify, so that closure compiler can know what content to refer to when it is given with a file path.

While I am implementing it, I have also observed that Closure Compiler cannot resolve paths in input sourcemaps when the above path fields are Windows-style paths. Then I changed any Windows-style paths in path value to a unix-style path, which is a lossless conversion I guess. And it seemed that it enables closure compiler to correctly resolve files in sourcemaps.

From compiler flags in the original post, I guess there is no more place to fiddle with paths, maybe json_streams mode processes some paths differently and may help getting correct sourcemaps.

theseanl avatar Jul 19 '21 08:07 theseanl

Since the Google team that forms the core support for closure-compiler do not use Windows at all, reproducing this problem in order to debug it is not something we can reasonably spend time on.

Would someone from the Open Source community be willing to debug this?

brad4d avatar Jul 26 '21 22:07 brad4d

@brad4d do you have a hint of where one could start fixing this? in the meantime i worked with WSL, which was fine so far, but now we're switching to pnpm and for some reasons it causes issues in combination with WSL on my machine (works elsewhere). so i'm back at having to build source maps on windows with closure compiler :D

DerGernTod avatar Jun 22 '22 14:06 DerGernTod

I'm not sure where to start looking either.

Since we non-Windows-users cannot reproduce the problem, perhaps you could post here the exception stack trace?

brad4d avatar Jun 22 '22 23:06 brad4d

here's the stack:

java -jar C:\workspaces\repos\reproducers\ts-esbuild-closure-reproducer\node_modules\google-closure-compiler-java\compiler.jar --chunk common:2 --chunk_wrapper common:(function(){%s})() --source_map_input dist/esbuild/chunk-7THFJKJQ.js|dist/esbuild/chunk-7THFJKJQ.js.map --js dist/esbuild/chunk-7THFJKJQ.js --source_map_input dist/esbuild/filecommon.js|dist/esbuild/filecommon.js.map --js dist/esbuild/filecommon.js --chunk filea:1:common --chunk_wrapper filea:(function(){%s})() --source_map_input dist/esbuild/filea.js|dist/esbuild/filea.js.map --js dist/esbuild/filea.js --chunk fileb:1:common --chunk_wrapper fileb:(function(){%s})() --source_map_input dist/esbuild/fileb.js|dist/esbuild/fileb.js.map --js dist/esbuild/fileb.js --language_in ECMASCRIPT_2017 --language_out ECMASCRIPT_2017 --chunk_output_path_prefix ./dist/closure/ --rename_prefix_namespace myNamespace --assume_function_wrapper --compilation_level ADVANCED_OPTIMIZATIONS --create_source_map %outname%.map

java.io.FileNotFoundException: .\dist\closure\common.js.map (The system cannot find the path specified)
        at java.base/java.io.FileOutputStream.open0(Native Method)
        at java.base/java.io.FileOutputStream.open(FileOutputStream.java:298)
        at java.base/java.io.FileOutputStream.<init>(FileOutputStream.java:237)
        at java.base/java.io.FileOutputStream.<init>(FileOutputStream.java:126)
        at com.google.javascript.jscomp.AbstractCommandLineRunner.filenameToOutputStream(AbstractCommandLineRunner.java:1782)
        at com.google.javascript.jscomp.AbstractCommandLineRunner.fileNameToOutputWriter2(AbstractCommandLineRunner.java:1773)
        at com.google.javascript.jscomp.AbstractCommandLineRunner.outputModuleBinaryAndSourceMaps(AbstractCommandLineRunner.java:1564)
        at com.google.javascript.jscomp.AbstractCommandLineRunner.processResults(AbstractCommandLineRunner.java:1418)
        at com.google.javascript.jscomp.AbstractCommandLineRunner.doRun(AbstractCommandLineRunner.java:1273)
        at com.google.javascript.jscomp.AbstractCommandLineRunner.run(AbstractCommandLineRunner.java:533)
        at com.google.javascript.jscomp.CommandLineRunner.main(CommandLineRunner.java:2150)

the file is definitely not there, the whole dist/closure directory hasn't been written at the time the error comes up. if i create the directory beforehands, maps are generated, but they still ignore --source_map_include_content. i also trid it with

"--source_map_include_content",
"--source_map_location_mapping", "source\\|..\\..\\source\\",

for a quick overview, this is how e.g. the filecommon.ts file looks like:

function fileCommonFunction() {
    exportedFunction();
}

export function exportedFunction() {
    console.log("i'm exported");
}

fileCommonFunction();

this is what closure makes of it:

(function(){'use strict';myNamespace.a=function(){console.log("i'm exported")};myNamespace.a();})()

this is what the source map looks like on windows (if i create the dist/closure directory first, if not, the error above happens):

{
"version":3,
"file":"./dist/closure/common.js",
"lineCount":1,
"mappings":"A,yBAIMA,WAAAA,CAAAA,CAAAA,CAAAA,QAAAA,EAA0BA,CAC5BC,OAAQC,CAAAA,GAARF,CAAYA,cAAZA,CAD4BA,CAH5BA,YAAAA,CAAAA,CAAA,E;",
"sources":["..\\..\\source\\filecommon.ts"],
"names":["exportedFunction","console","log"]
}

and this is what it looks like on linux (note that i switched the location mapping slashes):

{
"version":3,
"file":"./dist/closure/common.js",
"lineCount":1,
"mappings":"A,yBAIMA,WAAAA,CAAAA,CAAAA,CAAAA,QAAAA,EAA0BA,CAC5BC,OAAQC,CAAAA,GAARF,CAAYA,cAAZA,CAD4BA,CAH5BA,YAAAA,CAAAA,CAAA,E;",
"sources":["../../source/filecommon.ts"],
"sourcesContent":["function fileCommonFunction() {\r\n    exportedFunction();\r\n}\r\n\r\nexport function exportedFunction() {\r\n    console.log(\"i'm exported\");\r\n}\r\n\r\nfileCommonFunction();"],
"names":["exportedFunction","console","log"]
}

DerGernTod avatar Jun 23 '22 08:06 DerGernTod

re: the directory not existing

I would not expect closure-compiler to create a directory in which to put an output file. Throwing an exception is a very poor way to handle the directory not being there, but it's still a case of passing bad command line options if the compiler is being told to write a file into a non-existent directory.

It seems as if you can resolve that problem by just creating the directory before executing the compiler.

Re: missing "sourcesContent"

Have you tried using the --apply_input_source_maps to closure-compiler? I think you probably need that.

Can you confirm that the source maps generated by tsc contain sourcesContent? When using input source maps, closure-compiler copies the sourcesContent from them rather than using source code it is compiling. This is generally what you want, since you'd like to point back to the original TS, rather than the intermediate JS code it produced for compilation.

You might also try asking tsc to generate inline sourcemaps inside the JS file it outputs instead of separate files. Then ask closure-compiler to read those with --parse_inline_source_maps.

That's what we typically do.

brad4d avatar Jun 23 '22 16:06 brad4d

alright, so i tried a few more things you suggested today.

i inlined all source maps (so no lookup for .map files needed anymore), got rid of the "--source_map_input" for each file and added "--apply_input_source_maps" and "--parse_inline_source_maps" (i found no documentation about that... missing? https://github.com/google/closure-compiler-js/blob/master/README.md and https://github.com/google/closure-compiler/wiki/Flags-and-Options). unfortunately with no avail. the "sourcesContent" is still missing on windows.

i can confirm that it's available after both tsc and esbuild. example esbuild source map (base64 decoded from the inline map):

{
  "version": 3,
  "sources": ["../../source/filecommon.ts"],
  "sourcesContent": ["function fileCommonFunction() {\r\n    exportedFunction();\r\n}\r\n\r\nexport function exportedFunction() {\r\n    console.log(\"i'm exported\");\r\n}\r\n\r\nfileCommonFunction();"],
  "mappings": ";AAAA,8BAA2B;AACvB;;AAGE,4BAA0B;AAC5B,UAAQ,IAAI;;AAGhB;",
  "names": []
}

so the question remains: why does closure compiler not include the "sourcesContent" on windows? now it doesn't even make sense that it's a path problem since the sources are inlined

DerGernTod avatar Aug 03 '22 13:08 DerGernTod

@brad4d i spent quite some time yesterday and today checking out the closure compiler repo and building it myself. didn't manage to get it running within an ide but system out debugging helped, i found the issue. not sure why sourcesContent are related to that, but i found why the exception happens on windows and not on linux. within AbstractCommandLineRunner.java, in maybeCreateDirsForPath, the given directory from --chunk_output_path_prefix ./dist/closure/ is not created on windows, because File.separatorChar is actually \. so if the user passes / on windows, the exception happens. if the path is manually generated beforehands, the exception doesn't happen but sourcesContent property is missing anyways. i didn't dig into why this is related, but if i resolve the maybeCreateDirsForPath issue using \ instead of / when passing --chunk_output_path_prefix, the sourcesContent is there.

so in the end i'd say it's a user error. but due to the fact that you can interchange / and \ on powershell on windows, and for all other flags (such as --js), i think it's very hard to diagnose for a user to find the problem why their source maps don't work on windows.

DerGernTod avatar Aug 04 '22 08:08 DerGernTod

I'm submitting a change to unhide the two command line flags I mentioned. It turns out they defaulted to "on" anyway, but there's really no reason to hide them.

brad4d avatar Aug 04 '22 18:08 brad4d

Well done finding the problem and submitting the PR. I've done an initial review over there.

brad4d avatar Aug 04 '22 20:08 brad4d