Skip to content

cmdline: allow overriding -jsD directives (#26576)#26639

Open
subhaushsingh wants to merge 2 commits intoemscripten-core:mainfrom
subhaushsingh:fix-jsd-override-26576
Open

cmdline: allow overriding -jsD directives (#26576)#26639
subhaushsingh wants to merge 2 commits intoemscripten-core:mainfrom
subhaushsingh:fix-jsd-override-26576

Conversation

@subhaushsingh
Copy link
Copy Markdown
Contributor

@subhaushsingh subhaushsingh commented Apr 7, 2026

The Problem

  • Currently, passing -jsDFOO=1 -jsDFOO=2 triggers an error: cannot change built-in settings values with a -jsD directive.
  • This happens because the first flag adds FOO to the internal settings dictionary. When the second flag appears, the compiler mistakenly thinks a protected system setting (like WASM) is being overwritten.

The Fix

  • Added user_js_defines: A set in tools/cmdline.py to track which keys were introduced by the user during the current run.
  • Smart Validation: The error now only triggers if a key is a built-in and hasn't already been defined by the user via -jsD in this session.

Verification

  • Regression Test: Added a case to test/test_jslib.py that specifically tests duplicate/overridden -jsD flags.
  • Safety Check: Confirmed that attempting to override actual system settings (e.g., -jsDWASM=0) still correctly fails as intended.

@subhaushsingh subhaushsingh force-pushed the fix-jsd-override-26576 branch from aff073f to 02426d8 Compare April 7, 2026 14:42
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Thanks!

lgtm % test comments

self.cflags += ['--js-library', test_file('jslib/test_jslib_custom_settings.js'), '-jsDCUSTOM_JS_OPTION=1']
self.do_run_in_out_file_test('jslib/test_jslib_custom_settings.c')

self.run_process([EMCC, test_file('hello_world.c'), '-jsDCUSTOM_JS_OPTION=1', '-jsDCUSTOM_JS_OPTION=2'])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you replace these two lines with:

self.do_runf('jslib/test_jslib_custom_settings.c', '1\n')

# verify that the settings can be specified more then once, and that the last one wins.
self.do_runf('jslib/test_jslib_custom_settings.c', '2\n', cflags=['-jsDCUSTOM_JS_OPTION=2'])

Then you can also delete the test/jslib/test_jslib_custom_settings.out file

@subhaushsingh subhaushsingh force-pushed the fix-jsd-override-26576 branch from 73088d2 to 9855548 Compare April 8, 2026 04:41
@subhaushsingh
Copy link
Copy Markdown
Contributor Author

@sbc100 It looks like the Codesize Checks are failing due to a pre-existing stale baseline on main (specifically test_codesize_cxx_lto.json showing a +206 byte drift). This seems unrelated to my -jsD logic. Could you please run the rebaseline-tests workflow on main to unblock the CI? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants