WasmFS: Fix FS.cwd() usage without FORCE_FILESYSTEM (#19616)

The linking of FS.cwd is pretty awkward, unfortunately, but without adding too much more complexity this PR makes it work in another case, when FORCE_FILESYSTEM is not set. I am hoping this is the end of the complexity here. See more details in the big comment. The enabled test is an example of a case that failed before. This PR also allows us to remove the ugly INCLUDE_FULL_LIBRARY hack in this code.
emscripten-core · Jun 14, 2023 · 3b0eb96 · 3b0eb96
1 parent 7660103
commit 3b0eb96
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 9 deletions.
diff --git a/src/library_wasmfs.js b/src/library_wasmfs.js
@@ -30,7 +30,7 @@ FS.createPreloadedFile = FS_createPreloadedFile;
     '$FS_getMode',
     // For FS.readFile
     '$UTF8ArrayToString',
-#if FORCE_FILESYSTEM || INCLUDE_FULL_LIBRARY // see comment below on FORCE
+#if FORCE_FILESYSTEM
     '$FS_modeStringToFlags',
     'malloc',
     'free',
@@ -114,16 +114,23 @@ FS.createPreloadedFile = FS_createPreloadedFile;
       return ret;
     },
 
-#if FORCE_FILESYSTEM || INCLUDE_FULL_LIBRARY // FORCE_FILESYSTEM makes us
-                                             // include all JS library code. We
-                                             // must also do so if
-                                             // INCLUDE_FULL_LIBRARY as other
-                                             // places will refer to FS.cwd()
-                                             // in that mode, and so we need
-                                             // to include that.
+    // FS.cwd is in the awkward position of being used from various JS
+    // libraries through PATH_FS. FORCE_FILESYSTEM may not have been set while
+    // using those libraries, which means we cannot put this method in the
+    // ifdef for that setting just below. Instead, what we can use to tell if we
+    // need this method is whether the compiled get_cwd() method is present, as
+    // we include that both when FORCE_FILESYSTEM *and* when PATH_FS is in use
+    // (see the special PATH_FS deps logic for WasmFS).
+    //
+    // While this may seem odd, it also makes sense: we include this JS method
+    // exactly when the wasm method it wants to call is present.
+#if hasExportedSymbol('_wasmfs_get_cwd')
+    cwd: () => UTF8ToString(__wasmfs_get_cwd()),
+#endif
+
+#if FORCE_FILESYSTEM
     // Full JS API support
 
-    cwd: () => UTF8ToString(__wasmfs_get_cwd()),
     mkdir: (path, mode) => withStackSave(() => {
       mode = mode !== undefined ? mode : 511 /* 0777 */;
       var buffer = stringToUTF8OnStack(path);

diff --git a/test/test_browser.py b/test/test_browser.py
@@ -2549,6 +2549,7 @@ def test_worker_api_with_pthread_compilation_fails(self):
     stderr = self.expect_fail([EMCC, 'hello.o', '-o', 'a.js', '-g', '--closure=1', '-pthread', '-sBUILD_AS_WORKER'])
     self.assertContained("pthreads + BUILD_AS_WORKER require separate modes that don't work together, see https://github.com/emscripten-core/emscripten/issues/8854", stderr)
 
+  @also_with_wasmfs
   def test_emscripten_async_wget2(self):
     self.btest_exit('test_emscripten_async_wget2.cpp')