-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix case insensitive upload (#15245) #26369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 29 commits
83790a7
11c9609
8303655
2841fba
69b327c
9a4f36f
1d1e6f0
519a05d
ab1dd58
b95f524
4ca1484
c7ae52d
dbafd47
b7a4d21
97524da
e95950f
399e6a9
f22817a
7a8f50d
5185579
bb86f51
aa8aa1d
e33e8dd
fddd729
1d715f0
22d44f8
0e0133c
57da4cd
e705c0e
1a65d77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| FS.createDataFile('/', "file.txt", "foo"); | ||
| var errCode = 0; | ||
| try { | ||
| FS.createDataFile('/', "fILe.txt", "foo2"); | ||
| } | ||
| catch (e) | ||
| { | ||
| errCode = e.errno; | ||
| } | ||
| var fileContents = FS.readFile("/file.txt"); | ||
| out('file.txt: ' + fileContents); | ||
| var ret = FS.analyzePath('/file.txt'); | ||
| out('file.txt collison: ' + ret.object.name_next); | ||
| out('errorCode: ' + errCode); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /* | ||
| * Copyright 2026 The Emscripten Authors. All rights reserved. | ||
| * Emscripten is available under two separate licenses, the MIT license and the | ||
| * University of Illinois/NCSA Open Source License. Both these licenses can be | ||
| * found in the LICENSE file. | ||
| */ | ||
|
|
||
| #include <assert.h> | ||
| #include <stdio.h> | ||
|
|
||
| #include <emscripten.h> | ||
|
|
||
| void loadScript() { | ||
| printf("load2"); | ||
| FILE* file = fopen("file1.txt", "r"); | ||
|
|
||
| if (!file) { | ||
| assert(false); | ||
| } | ||
|
|
||
| while (!feof(file)) { | ||
| char c = fgetc(file); | ||
| if (c != EOF) { | ||
| putchar(c); | ||
| } | ||
| } | ||
| fclose(file); | ||
| exit(0); | ||
| } | ||
|
|
||
| void scriptLoadFail() { | ||
| printf("failed to load data_files.js\n"); | ||
| assert(false); | ||
| } | ||
|
|
||
| int main() { | ||
| emscripten_async_load_script("data_files.js", loadScript, scriptLoadFail); | ||
| return 99; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13838,9 +13838,24 @@ def test_recursive_cache_lock(self): | |
| self.assertContained('AssertionError: attempt to lock the cache while a parent process is holding the lock', err) | ||
|
|
||
| @also_with_wasmfs | ||
| @crossplatform | ||
| def test_fs_icase(self): | ||
| # c++20 for ends_with(). | ||
| self.do_other_test('test_fs_icase.cpp', cflags=['-sCASE_INSENSITIVE_FS', '-std=c++20']) | ||
| create_file('file1.txt', 'one') | ||
| create_file('fILe1.txt', 'two') | ||
| # `--from-emcc` needed here otherwise the output defines `var Module =` which will shadow the | ||
| # global `Module`. | ||
| self.run_process([FILE_PACKAGER, 'test.data', '--preload', 'file1.txt', 'fILe1.txt', '--from-emcc', '--js-output=data_files.js']) | ||
| self.do_runf('other/test_crash_icase.c', cflags=['-sFORCE_FILESYSTEM', '-sCASE_INSENSITIVE_FS']) | ||
|
|
||
| @crossplatform | ||
| def test_collision_icase(self): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about calling this
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like this test is specifically testing testing the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is testing the createDataFile API (specifically the collisions in case insensitive file systems) - in that case, what name would you prefer: |
||
| self.set_setting('DEFAULT_LIBRARY_FUNCS_TO_INCLUDE', ['$FS']) | ||
| self.set_setting('INCLUDE_FULL_LIBRARY', 1) | ||
| self.set_setting('CASE_INSENSITIVE_FS', 1) | ||
| self.add_pre_run(read_file(test_file('other/test_collision_icase.js'))) | ||
| self.do_runf('hello_world.c', 'file.txt: 102,111,111\nfile.txt collison: undefined\nerrorCode: 20') | ||
|
|
||
| @crossplatform | ||
| @with_all_fs | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -683,8 +683,12 @@ def generate_preload_js(data_target, data_files, metadata): | |
| } catch (e) { | ||
| err(`Preloading file ${name} failed`, e); | ||
| }\n''' | ||
| create_data = '''// canOwn this data in the filesystem, it is a slice into the heap that will never change | ||
| Module['FS_createDataFile'](name, null, data, true, true, true); | ||
| create_data = '''try { | ||
| // canOwn this data in the filesystem, it is a slice into the heap that will never change | ||
| Module['FS_createDataFile'](name, null, data, true, true, true); | ||
| } catch(e) { | ||
| err(`Preloading file ${name} failed`, e); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there some reason why its important to swallow this error here? Is it needed for this change?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a case (see #26327) where this error is not caught, causing the old code to hang. Is there a better place to catch it? |
||
| } | ||
| Module['removeRunDependency'](`fp ${name}`);''' | ||
|
|
||
| finish_handler = create_preloaded if options.use_preload_plugins else create_data | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So does this test print "one" or "two"? I'm guess its prints one because the attempt to write the second file will fail with 'err(
Preloading file ${name} failed, e);'Please add either 'one\n' or 'two\n' as the second argument to do_runf here.
Also, can we make this
test_crash_icase.cinto is separate test totest_fs_icase.Also, how about calling it
test_fs_icase_file_packager?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually prints "two". Other suggestions implemented