search

emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.37k stars 3.25k forks source link

Disallow EMTEST_SAVE_DIR, which has not worked in mode 2 #22178

Closed kripken closed 1 day ago

kripken commented 2 days ago

At some point mode 2 there broke. To avoid confusion, remove the env var entirely, and provide a clear error message pointing people to the proper options.

kripken commented 2 days ago

(I think mode 2 broke because configure() is called twice, and we read and wrote the env var for internal reasons, so we trampled the state. To avoid that, this PR makes us not use the env var as internal state at all.)

aheejin commented 2 days ago

Do we have any comment on what these values (0/1/2) for EMTEST_SAVE_DIR mean? I just empirically used to use --save-dir --no-clean because it worked, but I'm not sure what these values mean. Perhaps we can add some comments on its definition? (If this is a user-facing option, adding an entry in the doc would be better, but I guess it's mostly used by us?)

kripken commented 2 days ago

Those values are not user-facing after this PR, and maybe I should refactor the internals to avoid them..? I wasn't sure how to rename them though in a way that is consistent with the ones around them.

But, they just mean 1 = save-dir, 2 = no-clean (which also implies save-dir, so note that you don't need both).