Skip to content

Conversation

@hoodmane
Copy link
Collaborator

Decrement nlinks before doing continue linkloop; due to a .. at a file system loop. We're not actually traversing a symlink here.

Caught by the Python test suite.

Decrement nlinks before doing `continue linkloop;` due to a .. at a
file system loop. We're not actually traversing a symlink here.
Copy link
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!

@hoodmane
Copy link
Collaborator Author

Okay updated it.

@hoodmane
Copy link
Collaborator Author

Now let's see if I can convince rebaseline_tests to work correctly. I reran ./emsdk install tot and then tools/maint/rebaseline_tests.py but the size diffs looked way too big for me, so I tried ./emcc --clear-cache and am running again. I get a couple of closure tests tests failing with roughly this error:

test_codesize_minimal_O1 (test_other.other) ... FAIL
Running codesize test: mem_no_argv.c: ['-O3', '-sSTANDALONE_WASM'] True False
building:ERROR: Closure compiler run failed:

building:ERROR: /tmp/emtest_y5rraz1l/emscripten_temp_2bt05snf/a.out.jso4.js:43:262: ERROR - [JSC_UNDEFINED_VARIABLE] variable assert is undeclared
  43| if(typeof module!="undefined"){module["exports"]=Module}quit_=(status,toThrow)=>{process.exitCode=status;throw toThrow}}else if(ENVIRONMENT_IS_SHELL){readBinary=f=>{if(typeof readbuffer=="function"){return new Uint8Array(readbuffer(f))}let data=read(f,"binary");assert(typeof data=="object");return data};readAsync=async f=>readBinary(f);globalThis.clearTimeout??=id=>{};// spidermonkey lacks setTimeout but we use it above in readAsync.
                                                                                                                                                                                                                                                                            ^^^^^^

1 error(s), 0 warning(s)

@hoodmane
Copy link
Collaborator Author

The jssizes have all gone up by ~600 bytes and the gzsizes by ~1300 bytes. Seems too big for this change.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 17, 2025

If you want I can try to generate the codesize change for this PR?

@hoodmane
Copy link
Collaborator Author

That would be helpful. It would be good to eventually figure out how to generate these myself. Maybe we could run the rebasline_tests.py in a docker image like @kleisauke suggested.

This is an automatic change generated by tools/maint/rebaseline_tests.py.

The following (20) test expectation files were updated by
running the tests with `--rebaseline`:

```
test/other/codesize/test_codesize_cxx_ctors1.gzsize updated
test/other/codesize/test_codesize_cxx_ctors1.jssize updated
test/other/codesize/test_codesize_cxx_ctors2.gzsize updated
test/other/codesize/test_codesize_cxx_ctors2.jssize updated
test/other/codesize/test_codesize_cxx_except.gzsize updated
test/other/codesize/test_codesize_cxx_except.jssize updated
test/other/codesize/test_codesize_cxx_except_wasm.gzsize updated
test/other/codesize/test_codesize_cxx_except_wasm.jssize updated
test/other/codesize/test_codesize_cxx_except_wasm_legacy.gzsize updated
test/other/codesize/test_codesize_cxx_except_wasm_legacy.jssize updated
test/other/codesize/test_codesize_cxx_lto.gzsize updated
test/other/codesize/test_codesize_cxx_lto.jssize updated
test/other/codesize/test_codesize_cxx_mangle.gzsize updated
test/other/codesize/test_codesize_cxx_mangle.jssize updated
test/other/codesize/test_codesize_cxx_noexcept.gzsize updated
test/other/codesize/test_codesize_cxx_noexcept.jssize updated
test/other/codesize/test_codesize_files_js_fs.gzsize updated
test/other/codesize/test_codesize_files_js_fs.jssize updated
test/other/codesize/test_codesize_hello_dylink.gzsize updated
test/other/codesize/test_codesize_hello_dylink.jssize updated
```
@sbc100
Copy link
Collaborator

sbc100 commented Jun 17, 2025

Done

@hoodmane hoodmane closed this Jun 18, 2025
@hoodmane hoodmane reopened this Jun 18, 2025
@hoodmane
Copy link
Collaborator Author

All the tests passed but something went wrong with the status report for test-wasm-js and GitHub shows it as in progress while circle ci shows it as passed. I guess I can push an empty commit and retrigger.

@sbc100 sbc100 merged commit cc1de1a into emscripten-core:main Jun 18, 2025
30 of 32 checks passed
@hoodmane hoodmane deleted the no-eloop-due-to-dotdot branch June 19, 2025 14:34
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