Repository navigation
Conversation
010c1ae to
f456adc
Compare
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8839 |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f456adc3e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94a90ef802
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1c8248246
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd0c132ece
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccacd4b908
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
cknitt
changed the title
765184e to
a5af682
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5af682777
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Unwatching one path can drop an overlapping watch with inotify, so only macOS, where FSEvents restarts its stream on every change, updates watches incrementally. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BC5F5QPeQh5cHBjGBnvJ2T Signed-off-by: Christoph Knittel <ck@cca.io>
Outside macOS, every path is registered again after the stale ones are unwatched, which restores watches an unwatch dropped without a gap for the others. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BC5F5QPeQh5cHBjGBnvJ2T Signed-off-by: Christoph Knittel <ck@cca.io>
a5af682 to
e3ada4d
Compare
cknitt
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3ada4d2cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let _ = watcher.unwatch(&watch.path); | ||
| } | ||
| if !REGISTER_ONLY_NEW_WATCHES { | ||
| return register_watches(watcher, next); |
There was a problem hiding this comment.
On Linux, when next contains a recursive parent and a non-recursive child or alias of a directory it covers, replaying every entry can overwrite notify 5.2's single inotify record for that directory with is_recursive = false if the non-recursive entry is registered last. Because compute_watch_paths returns fresh HashMap iteration order, a full rebuild can downgrade a previously recursive watch; directories subsequently created below that point are not added to inotify, so edits inside them are missed. Register non-recursive targets before recursive targets, or otherwise preserve the strongest mode for shared directory identities.
Useful? React with 👍 / 👎.
| for watch in stale { | ||
| let _ = watcher.unwatch(&watch.path); | ||
| } | ||
| if !REGISTER_ONLY_NEW_WATCHES { | ||
| return register_watches(watcher, next); |
There was a problem hiding this comment.
When a stale recursive watch overlaps a retained child, unwatch removes the child's shared inotify descriptor and the child is restored only later by register_watches. An edit during that interval occurs after initialize_build has already scanned the sources and produces no event, leaving its output stale. Although the prior gap report was answered by retaining unchanged entries, the final stale-only loop still drops these particular retained entries indirectly; rescan after registration or otherwise reconcile changes made during this gap.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
rescript watchregisters each source folder as its own watch, and source folders may overlap. Since #8730, a full rebuild (after arescript.jsonchange, but also when a source file is added, removed or renamed) unwatches the paths that went away and watches only the new ones. With inotify on Linux that loses watches: notify 5 keeps one watch per directory, shared by every registered path that reaches it, so unwatching one path can drop another that is still configured. For example, with sources[{"dir": "src", "subdirs": true}, "src/retained"]changed to["src/retained"], unwatching the recursivesrcalso droppedsrc/retained, and edits there were no longer picked up (#8825). The same happens for other spellings of a folder (.., symlinks) and for directory symlinks inside a recursive source folder.update_watchesnow still unwatches only the stale watches (paths no longer configured, or whose directory was replaced), and then depends on the platform:watch/unwatchcall restarts the event stream, and events during a restart are lost; that is why Fix watcher missing watch.lock removal during full rebuilds #8730 made updates incremental. FSEvents watches are independent paths, so unwatching one never drops another. As before, only new paths are registered, so a full rebuild that keeps the same paths doesn't restart the stream.The doc comment of
update_watchesexplains these differences.New test:
keeps_watching_after_an_overlapping_watch_is_removed(Linux only) registers watches with a real notify watcher, updates them, and checks that a write in a folder that is still configured is reported. It covers a full rebuild that keeps the same paths, a folder kept below a removed recursive folder (the case from #8825), a folder removed below a kept recursive one, a folder that stops being recursive, other spellings of a folder (.., a symlink), and a recursive folder reaching a separately watched one through a directory symlink. Registering only the new paths on Linux, as master does, fails the #8825 case.Fixes #8825
🤖 Generated with Claude Code
https://claude.ai/code/session_01BC5F5QPeQh5cHBjGBnvJ2T