Skip to content

Conversation

kleisauke
Copy link
Collaborator

Similar to #10669 but then for NODERAWFS.

src = path_from_root('tests', 'fs', 'test_mmap.c')
out = path_from_root('tests', 'fs', 'test_mmap.out')
self.emcc_args = orig_compiler_opts + ['-D' + fs]
if fs == 'NODEFS':
self.emcc_args += ['-lnodefs.js']
if fs == 'NODERAWFS':
self.emcc_args += ['-sNODERAWFS=1']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does -lnoderawfs.js work? (like the line two lines above?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That works too, but in that case also -lnodefs.js needs to be linked.

(Perhaps we could just synonym -sNODERAWFS=1 with -lnoderawfs.js? See for e.g. kleisauke@b3607b9)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I included -lnodefs.js and -lnoderawfs.js with commit kleisauke@244e478, but perhaps this (i.e. ['-sNODERAWFS=1'] -> ['-lnodefs.js', '-lnoderawfs.js']) should be done throughout the whole test suite? Let me know if you want a separate PR for this.

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.

lgtm with the errno change.

+ add a test case that covers this.

(this commit fixes also a TypeError in $syscallMunmap,
which was hit by this test case)
@kleisauke kleisauke force-pushed the mmap-msync-noderawfs branch from 6ac5c09 to e9ceca1 Compare January 6, 2021 16:19
@kleisauke
Copy link
Collaborator Author

Rebased on top of the master branch to resolve merge conflict.

@sbc100 sbc100 merged commit 0320c04 into emscripten-core:master Jan 7, 2021
@kleisauke kleisauke deleted the mmap-msync-noderawfs branch January 7, 2021 12:07
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