Initial FTS support #522
Merged
Initial FTS support #522
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kateinoigakukun
force-pushed
the
yt/fts
branch
8 times, most recently
from
a5caf7b to
f9a396b
Compare
Add FTS implementation derived from musl-fts with a few modifications. The compiled fts.o is included in the libc.a archive, and the fts.h header is installed in the sysroot (`include/fts.h`). * fts/musl-fts: Add a copy of the musl-fts sources with modifications. * fts/patches: A set of patches to apply to the musl-fts sources. * fts/update-musl-fts.sh: A script to update the musl-fts sources with the patches applied. * fts/config.h: A configuration header included by the musl-fts sources. * test/smoke: Add a test suite for wasi-libc specific features that libc-test does not cover.
sbc100
reviewed
Aug 1, 2024
There was a problem hiding this comment.
Thanks for working on this.
I assume if/when the upstream patch lands we can replace most of this with a git submodule? (i.e. remove the update script and the patch system?).
I wonder if its worth waiting a few days to see if upstream will simply accept the patch?
|
Given that the latest release was 8 years ago and the main maintainer has no public activity last 3 years, it will take a long time to land the patch in the upstream. |
Fair point :) |
yamt
reviewed
Aug 26, 2024
|
|
||
| +#if !HAVE_FCHDIR | ||
| + /* If we don't have fchdir, pretend that FTS_NOCHDIR is always set. */ | ||
| + options |= FTS_NOCHDIR; |
There was a problem hiding this comment.
doesn't this introduce the behavior changes visible to fts-using application?
i guess it's safer to make fts_open fail unless FTS_NOCHDIR is given.
There was a problem hiding this comment.
Yes, setting NOCHDIR by default indeed introduces changes in how cwd changes while traversing.
However, changing cwd is an internal optimization in fts, and the expected cwd during traversing is not defined in manual pages of most of fts impls. So I don't think most of user applications can depend on cwd while traversing if FTS_NOCHDIR is not specified.
Thus applications without FTS_NOCHDIR specified should work even if not changing cwd and we don't need to ban fts_open calls without FTS_NOCHDIR.
sunfishcode
approved these changes
Aug 29, 2024
|
@sunfishcode @sbc100 Can we merge this into the main branch as we haven't had any license concerns for the past 3 weeks? |
|
Merged! |
kateinoigakukun
added a commit
to kateinoigakukun/wasi-libc
that referenced
this pull request
Oct 12, 2024
WebAssembly#522 did not include the necessary changes to the Makefile for libc_so build.
abrown
pushed a commit
that referenced
this pull request
Oct 16, 2024
#522 did not include the necessary changes to the Makefile for libc_so build. Additionally, updated CI to check `libc_so` build too to avoid future breakage.
abrown
added a commit
to abrown/wasi-libc
that referenced
this pull request
Nov 21, 2024
This change represents a rather large re-design in how `wasi-libc` builds and runs its tests. Initially, WebAssembly#346 retrieved the `libc-test` repository and built a subset of those tests to give us some amount of test coverage. Later, because there was no way to add custom C tests, WebAssembly#522 added a `smoke` directory which allowed this. But (a) each of these test suites was built and run separately and (b) it was unclear how to add more tests flexibly--some tests should only run on `*p2` targets or `*-threads` targets, e.g. This change reworks all of this so that all tests are built the same way, in the same place. For downloaded tests like those from `libc-test`, I chose to add "stub tests" that `#include` the original version. This not only keeps all enabled tests in one place, it also allows us to add "directives," C comments that the `Makefile` uses to filter out tests for certain targets or add special compile, link or run flags. These rudimentary scripts, along with other Bash logic I moved out of the Makefile now live in the `scripts` directory. Finally, all of this is explained more clearly in an updated `README.md`. The hope with documenting this a bit better is that it would be easier for drive-by contributors to be able to either dump in new C tests for regressions they may find or enable more libc-tests. As of my current count, we only enable 40/75 of libc-test's functional tests, 0/228 math tests, 0/69 regression tests, and 0/79 API tests. Though many of these may not apply to WASI programs, it would be nice to explore how many more of these tests can be enabled to increase wasi-libc's test coverage. This change should explain how to do that and, with directives, make it possible to condition how the tests compile and run.
abrown
added a commit
that referenced
this pull request
Dec 9, 2024
This change represents a rather large re-design in how `wasi-libc` builds and runs its tests. Initially, #346 retrieved the `libc-test` repository and built a subset of those tests to give us some amount of test coverage. Later, because there was no way to add custom C tests, #522 added a `smoke` directory which allowed this. But (a) each of these test suites was built and run separately and (b) it was unclear how to add more tests flexibly--some tests should only run on `*p2` targets or `*-threads` targets, e.g. This change reworks all of this so that all tests are built the same way, in the same place. For downloaded tests like those from `libc-test`, I chose to add "stub tests" that `#include` the original version. This not only keeps all enabled tests in one place, it also allows us to add "directives," C comments that the `Makefile` uses to filter out tests for certain targets or add special compile, link or run flags. These rudimentary scripts, along with other Bash logic I moved out of the Makefile now live in the `scripts` directory. Finally, all of this is explained more clearly in an updated `README.md`. The hope with documenting this a bit better is that it would be easier for drive-by contributors to be able to either dump in new C tests for regressions they may find or enable more libc-tests. As of my current count, we only enable 40/75 of libc-test's functional tests, 0/228 math tests, 0/69 regression tests, and 0/79 API tests. Though many of these may not apply to WASI programs, it would be nice to explore how many more of these tests can be enabled to increase wasi-libc's test coverage. This change should explain how to do that and, with directives, make it possible to condition how the tests compile and run.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #520
Add FTS implementation derived from musl-fts with a few modifications. The compiled fts.o is included in the libc.a archive, and the fts.h header is installed in the sysroot (
include/fts.h).