Sourcemap improvements #23741

lvlte · 2025-02-24T22:01:31Z

Provide source map settings to emcc (resolves #23686, resolves #22189) :

for embedding the sources content in the source map : -gsource-map=inline.
for applying path prefix substitution : -sSOURCE_MAP_PREFIXES=["<old>=<new>"].

Update documentation accordingly.

Fix source file resolver :

Always fall back to the given filepath if prefix not provided or doesn't match.
Fix source content loading when no --load-prefix is given/needed.
Fix relative path in "sources" field when--prefix is given but doesn't match the given file.
Cache filepaths with no/unmatched prefix as well.
Resolve deterministic prefix when loading source content (related: Resolve synthetic compilation dir /emsdk/emscripten to obtain actual path for better source map #20779).
Don't emit relative paths for sources with a deterministic prefix.

Improve existing test for wasm-sourcemap.py :

Parameterize test_wasm_sourcemap() and do proper checks according to the combinations of options given.
Fix regex for checking the "mappings" field (was checking only the first character).

Add test for emcc covering the basic use cases where :

no option is given (users do path substitution as needed via their client or server configuration).
different prefixes are provided for source files and emscripten dependencies.
source content is embedded in the sourcemap.

- Always fall back to the given filepath if prefix not provided or doesn't match. - Fix source content loading when no `--load-prefix` is given/needed. - Fix relative path in "sources" field when`--prefix` is given but doesn't match the given file. - Cache filepaths with no/unmatched prefix as well.

…ist when present.

- Parameterize `test_wasm_sourcemap()` and do proper checks according to the combinations of options given. - Fix regex for checking the "mappings" field (was checking only the first character).

This allows to provide sourcemap settings to emcc via -s options. Each setting maps to an option used in tools/wasm-sourcemap.py : - `INLINE_SOURCES` => `--sources` - `SOURCE_MAP_PREFIXES` => `--prefix` - `INLINE_SOURCES_PREFIXES` => `--load-prefix`

This covers the basic use cases where : - no option is given (users do path substitution as needed via their client or server configuration) - different prefixes are provided for source files and emscripten dependencies - source content is embedded in the sourcemap

I realized while doing tests that we won't ever need to provide this option (along with `INLINE_SOURCES` for loading sources content in the sourcemap), since all sources are known by emcc and can be resolved properly, including paths with deterministic prefix.

sbc100

This looks awesome, thanks for working on it!

The only part that gives me some hesitation is that addition of two new settings. If possible we try to avoid adding new settings but this might be a case when they are needed.

I wonder if INLINE_SOURCES could be replaced with -gsource-map=inline?

I also wonder if this is really two different uses? How common do you thing the embedding of sources will be compared to the use of prefixes? What kind of use case fo you see for both of them? Could we split this PR into two for these two features perhaps?

(Also, thanks for fixing the typo, we can probably land that as its own cleanup PR right away).

lvlte · 2025-02-24T23:11:04Z

I wonder if INLINE_SOURCES could be replaced with -gsource-map=inline?

Sure, I chose the -s option because it was easier to implement. I will do that tomorrow.

I also wonder if this is really two different uses?

Yes, if we embed the sources content, we don't need to use path substitution, and conversely if we use prefixes it implies we don't have inline content.

How common do you thing the embedding of sources will be compared to the use of prefixes?

I tried both :

embedding is easier because it works out of the box now that the resolver is fixed.
using prefixes might be preferred for large projects but requires some tweaking, it works fine just like if we do path substitution via server or client configuration.

What kind of use case fo you see for both of them?

The tests might covers some combinations since the options are not mutually exclusive, but yes using both doesn't make sense.

lvlte · 2025-02-24T23:14:18Z

Could we split this PR into two for these two features perhaps?

I would prefer not honestly. Both features require the path resolver fix to work properly, which represents most of the code change (if we ignore the tests). The tests also share the same code for different options.

sbc100 · 2025-02-24T23:22:30Z

Could we split this PR into two for these two features perhaps?

I would prefer not honestly. Both features require the path resolver fix to work properly, which represents most of the code change (if we ignore the tests). The tests also share the same code for different options.

Fair enough. I think we can land this as one change then, once we have finished bikeshedding the settings naming.

BTW, gcc and clang already have -ffile-prefix-map=old=new.. i wonder if we could re-use that as a linker flag here instead of adding a new setting? Maybe too magical?

lvlte · 2025-02-25T18:54:07Z

I'm not sure about -ffile-prefix-map=old=new (to be honest my understanding of gcc/clang is very limited, and I'm not sure to understand what you mean by reusing it as a linker flag).

But I noticed we use that to replace the emscripten path with the synthetic path here :

emscripten/tools/system_libs.py

Lines 589 to 593 in 273f021

    
           source_dir = utils.path_from_root() 
        
           relative_source_dir = os.path.relpath(source_dir, self.build_dir) 
        
           cflags += [f'-ffile-prefix-map={source_dir}={DETERMINISITIC_PREFIX}', 
        
                      f'-ffile-prefix-map={relative_source_dir}={DETERMINISITIC_PREFIX}', 
        
                      f'-fdebug-compilation-dir={DETERMINISITIC_PREFIX}']

and, for example when SDL is required, I noticed most of the lib files that are cached won't have their path prefix replaced. If I provide mappings to emcc via -ffile-prefix-map, it won't work either for most of the lib files (cached ports and other lib files).

sbc100 · 2025-02-25T19:22:12Z

@dschuff @kripken WDYT?

kripken

Nice!

Please document -gsource-map=inline option in emcc --help, which is in site/source/docs/tools_reference/emcc.rst. May as well also mention that SOURCE_MAP_PREFIXES may be useful, there.

dschuff · 2025-02-25T23:45:39Z

Sorry I'm late to this party, I have a few questions. Mostly I'm concerned about making the source map behavior as close to or consistent with the DWARF behavior as possible, and as deterministic as possible (aside of course from the usual considerations already mentioned). I think -gsource-map=inline is fine. For flags, I'm wondering if it might it make sense to get the desired behavior using one of the DWARF flags (e.g. -ffile-prefix-map/-fdebug-prefix-map/-fdebug-compilation-dir). These change the output that is used to create the source map, but we could potentially even use them to also control the behavior of the source map generator as well.

sbc100 · 2025-02-26T00:14:44Z

Sorry I'm late to this party, I have a few questions. Mostly I'm concerned about making the source map behavior as close to or consistent with the DWARF behavior as possible, and as deterministic as possible (aside of course from the usual considerations already mentioned). I think -gsource-map=inline is fine. For flags, I'm wondering if it might it make sense to get the desired behavior using one of the DWARF flags (e.g. -ffile-prefix-map/-fdebug-prefix-map/-fdebug-compilation-dir). These change the output that is used to create the source map, but we could potentially even use them to also control the behavior of the source map generator as well.

I think that is what I was suggesting in #23741 (comment). i'm not sure about extending the meaning of those existing flags (which I think today are no link time, but only compile time flags). Maybe its a good idea? Maybe too magical?

sbc100 · 2025-02-26T00:15:59Z

@lvlte I'm curious what you think main use case for SOURCE_MAP_PREFIXES might be? How are you planning on using it, for example?

lvlte · 2025-02-26T18:01:28Z

The usage would be exactly the same as for the --prefix option in tools/wasm-sourcemap.py, one flag for any number of prefix replacements, for example :

emcc ... -sSOURCE_MAP_PREFIXES=["/emsdk/emscripten/=scheme://host/path/to/emscripten/", "=scheme://host/"]

How would users obtain the same output using -ffile-prefix-map/-fdebug-prefix-map/-fdebug-compilation-dir exactly ?

Speaking about this, anyone knows why most of the cached lib files don't have a deterministic prefix ?

src/settings.js

sbc100 · 2025-02-26T18:54:29Z

The usage would be exactly the same as for the --prefix option in tools/wasm-sourcemap.py,

But we don't currently use that flag do we? I guess I'm curious how folks have been using source maps without this, assuming its necessary? Or have source maps been broken since we switch to the using a deterministic prefix?

one flag for any number of prefix replacements, for example :

emcc ... -sSOURCE_MAP_PREFIXES=["/emsdk/emscripten/=scheme://host/path/to/emscripten/", "=scheme://host/"]

How would users obtain the same output using -ffile-prefix-map/-fdebug-prefix-map/-fdebug-compilation-dir exactly ?

I think the idea would be that you would write -ffile-prefix-map=/emsdk/emscripten/=scheme://host/path/to/emscripten at link time instead of -sSOURCE_MAP_PREFIXES=.. but it would have the same effect. And since -ffile-prefix-map= is already a known compiler flag we wouldn't be adding any new flag. However, emcc doesn't currently parse this flag (it just passes it directly to clang). Its also not clear if re-using this existing flag makes sense.

Speaking about this, anyone knows why most of the cached lib files don't have a deterministic prefix ?

They should all use the prefix I think. Can you give an example of one that does not using the prefix?

lvlte · 2025-02-26T19:33:17Z

I'm curious how folks have been using source maps without this, assuming its necessary?

Personally I was using path substitution in Chrome, but we can't do that with Firefox for example.

Or have source maps been broken since we switch to the using a deterministic `prefix?

Even before the switch, all sources outside the compilation dir would end up with some ../ in their paths, which the browser would then discard when fetching the file, ending in a 404. I guess users haven't reported this because they likely focus on their own source files during debugging.

Regarding -ffile-prefix-map, I'm still confused as what I tried doesn't seem to work.

They should all use the prefix I think. Can you give an example of one that does not using the prefix?

The simplest would be to take this example (or any example that relies on SDL I think), from that, I get 35% of the emscripten files (all cached files except one) without the prefix.

dschuff · 2025-02-26T23:50:36Z

Regarding SDL and such; maybe we should configure the ports builder to use the same flags as embuilder uses for the core libraries.

See emscripten-core#23741

sbc100 · 2025-02-26T23:59:52Z

Regarding SDL and such; maybe we should configure the ports builder to use the same flags as embuilder uses for the core libraries.

Good call: #23777

See #23741

lvlte added 9 commits February 23, 2025 17:44

Sourcemap field sourcesContent is optional but should always be a l…

2164df3

…ist when present.

[test] Improve sourcemap test

ec9d23a

- Parameterize `test_wasm_sourcemap()` and do proper checks according to the combinations of options given. - Fix regex for checking the "mappings" field (was checking only the first character).

Sourcemap: Resolve deterministic prefix when loading source content.

9a0914b

Sourcemap: Don't use relative path for files with deterministic prefix

82cfc12

Fix typo

20969b2

sbc100 reviewed Feb 24, 2025

View reviewed changes

lvlte added 2 commits February 24, 2025 23:21

Update settings reference

427ba6e

Merge branch 'main' into sourcemap_improvements

3834b19

lvlte added 2 commits February 25, 2025 19:38

Replace option -sINLINE_SOURCES with -gsource-map=inline

6707f68

Merge branch 'main' into sourcemap_improvements

a3c59f1

sbc100 requested a review from dschuff February 25, 2025 19:21

kripken reviewed Feb 25, 2025

View reviewed changes

sbc100 approved these changes Feb 25, 2025

View reviewed changes

lvlte added 2 commits February 26, 2025 19:32

Document -gsource-map=inline and -sSOURCE_MAP_PREFIXES

95058d5

Merge branch 'main' into sourcemap_improvements

511b12a

sbc100 reviewed Feb 26, 2025

View reviewed changes

src/settings.js Outdated Show resolved Hide resolved

Prefer/encourage the simpler list form in settings documentation

d2c0aae

lvlte force-pushed the sourcemap_improvements branch from 9fe508d to d2c0aae Compare February 26, 2025 20:49

sbc100 added a commit to sbc100/emscripten that referenced this pull request Feb 26, 2025

Make debug info in ports deterministic. NFC

de1a6cb

See emscripten-core#23741

sbc100 mentioned this pull request Feb 26, 2025

Make debug info in ports deterministic. NFC #23777

Merged

sbc100 added a commit that referenced this pull request Feb 27, 2025

Make debug info in ports deterministic. NFC (#23777)

4b76531

See #23741

Merge branch 'main' into sourcemap_improvements

02412cd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sourcemap improvements #23741

Sourcemap improvements #23741

lvlte commented Feb 24, 2025 •

edited

Loading

sbc100 left a comment

lvlte commented Feb 24, 2025

lvlte commented Feb 24, 2025

sbc100 commented Feb 24, 2025

lvlte commented Feb 25, 2025

sbc100 commented Feb 25, 2025

kripken left a comment

dschuff commented Feb 25, 2025

sbc100 commented Feb 26, 2025

sbc100 commented Feb 26, 2025

lvlte commented Feb 26, 2025

sbc100 commented Feb 26, 2025

lvlte commented Feb 26, 2025 •

edited

Loading

dschuff commented Feb 26, 2025

sbc100 commented Feb 26, 2025

Sourcemap improvements #23741

Are you sure you want to change the base?

Sourcemap improvements #23741

Conversation

lvlte commented Feb 24, 2025 • edited Loading

sbc100 left a comment

Choose a reason for hiding this comment

lvlte commented Feb 24, 2025

lvlte commented Feb 24, 2025

sbc100 commented Feb 24, 2025

lvlte commented Feb 25, 2025

sbc100 commented Feb 25, 2025

kripken left a comment

Choose a reason for hiding this comment

dschuff commented Feb 25, 2025

sbc100 commented Feb 26, 2025

sbc100 commented Feb 26, 2025

lvlte commented Feb 26, 2025

sbc100 commented Feb 26, 2025

lvlte commented Feb 26, 2025 • edited Loading

dschuff commented Feb 26, 2025

sbc100 commented Feb 26, 2025

lvlte commented Feb 24, 2025 •

edited

Loading

lvlte commented Feb 26, 2025 •

edited

Loading