Repository navigation
feat(libkernel_sys): implement lchflags - #9442
Gruzzberry wants to merge 1 commit into
Conversation
3e90ee8 to
fb9a9eb
Compare
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
fb9a9eb to
e5d6ec9
Compare
|
Updated: the link check now uses |
jotitadev
left a comment
jotitadev
left a comment
There was a problem hiding this comment.
Linux run of the current change, since the description lists only Windows ("Not tested: Linux").
Machine: Linux x86_64, i7-8700 (6 cores / 12 threads), Ubuntu 26.04, GCC 15.2, Release, BUILD_TESTING=ON. Head e5d6ec946 merged onto main a6dcea927 (the merge touches Stdio.cpp, libkernel_sys/Export.Discovery.cpp, libkernel_sys/Export.cpp and GuestKernelSys.cpp).
- Build ok.
guest_kernel_syspasses with the change, including the newlchflagscases (regular file,EFAULT,ENOENT,ENOTDIR,EOPNOTSUPP,EPERM, the dangling and working link cases and theUF_IMMUTABLEthrow). - Restoring the three implementation files to
mainand keeping the test abortsguest_kernel_sys:
terminate called after throwing an instance of 'std::runtime_error'
what(): lchflags_nid_postfix not implemented
so the regression reproduces on Linux as well and the test covers the change. - The export moves from the discovery stub (
Export.Discovery.cpp) to a real entry inExport.cpp; the patched/unpatchedlibkernel_syslibraries build and the NID/convention tests pass, so the NID patching stays intact.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the lchflags implementation and its source/test context. The cited FreeBSD syscall uses AT_SYMLINK_NOFOLLOW; the helper uses guest lstat so a dangling final-component link is recognized on MinGW, while chflags retains follow behavior. The existing unsupported-flag results are preserved. The author reports Windows tests and an independent reviewer reports Linux build/regression results; hosted eligibility, conventions, labels and report pass, while product/relinker jobs were skipped. I did not run local tests.
The current overlap results need author coordination before merge:
- #9423 changes the
PathErrorcontract and thechflagscode inStdio.cpp; this PR refactors that samechflagsimplementation and calls the oldPathErrorsignature. The branches must be reconciled so checked guest-path copying and the new no-follow helper both remain. - #9103 inserts
chown/lchown/fchownatexports inlibkernel_sys/Export.cppat the samefstatat/unlinkinsertion area where this PR addslchflags. - #6044 renames
GuestKernelSys.cpptotests/process/GuestKernelSys.cpp, while this PR adds the regression cases to the old path. Coordinate so the final registered test target includes the new coverage after the move.
Please agree the order with these authors, rebase/compose the changes, and record Depends on for any prerequisite. The present none entries do not document these direct overlaps. The scan also listed #9040, #9097 and #6525 at nearby test lines; I manually compared their hunks and found their helper/call insertions separate from the lchflags additions, so I am not requesting changes for those.
e5d6ec9 to
62a5ad2
Compare
|
Rebased onto Overlaps, now listed in the description under Overlaps: I asked on #9423 to land it first and will then rebase this PR so |
|
Holding this as a draft, following the direction on #5239: the open queue is above 700 and no named title needs this change on current |
62a5ad2 to
6ed9569
Compare
|
Rebased onto |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The implementation and reported Windows/Linux checks look well-supported. One documentation follow-up: please update the existing chflags debt entry in docs/dev/debt/libkernel.md to include lchflags and its no-follow behavior, including that this behavior and the console filesystem/flag handling have not been measured on a PS5. The entry currently documents the same shared flag limitations for chflags/fchflags, but does not name the newly implemented API or its symlink-specific path behavior. No blocking code issue found in this review.
6ed9569 to
f120c81
Compare
|
@MarcZX10 thanks, added in f120c81: a |
github-actions
Bot
added
needs rebase
lchflags is chflags without following a symbolic link in the last path component, as FreeBSD 11.4's sys_lchflags calls kern_chflagsat with AT_SYMLINK_NOFOLLOW. chflags now goes through a shared helper that takes the follow choice; a link in the last component is accepted as the target when not following, and the flag checks are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f120c81 to
47ea711
Compare
|
Follow-up in 47ea711: the |
github-actions
Bot
added
docs
and removed
needs rebase
|
A PR overlap check shows #10625 also edits the declaration block in |
What
lchflagswas a throwing stub in libkernel_sys. FreeBSD 11.4 implements it aschflagswithout following a symbolic link in the last component:sys_lchflagscallskern_chflagsatwithAT_SYMLINK_NOFOLLOW, andkern_chflagsatonly switchesnameifromFOLLOWtoNOFOLLOW.chflagsinStdio.cppnow goes throughChangeFileFlags_nid_no_patch, which takes the same follow choice. When not following, a symbolic link in the last component is the target, so a dangling link is accepted instead of failing the lookup. The link is recognised withFile::FillLinkStat, the guestlstat, because under MinGWstd::filesystem::symlink_statusreports a Windows symbolic link as the file or directory it names. A trailing slash still follows, as in FreeBSD. The flag checks are the existing ones, unchanged: unknown bitsEOPNOTSUPP,SF_SNAPSHOTEPERM, any other non-zero flag throws because the host keeps no BSD file flags.chflagsitself behaves as before.Tested
Windows 11 (10.0.26200), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release,
-DBUILD_TESTING=ON, onmaind6eafa5 (rebased), Developer Mode on so the link cases run.guest_kernel_sys: the newlchflagschecks (a regular file,EFAULT,ENOENTfor empty and missing paths,ENOTDIRthrough a file,EOPNOTSUPP,EPERM, a dangling link and a working link with flags 0,EOPNOTSUPPandEPERMon the link, the throw forUF_IMMUTABLE) abort onmainwithlchflags_nid_postfix not implementedand pass with this PR.guest_filesystem(the existingchflagsandfchflagschecks) andguest_file_timestamps: pass.maind6eafa5 and onmainwith feat(libkernel_sys): implement lchflags #9442, fix(libkernel): fail chflags through a dangling Windows symbolic link with ENOENT #9469, fix(libc): follow a Windows symbolic link in access #9474, fix(libkernel): follow a Windows symbolic link in chmod #9487 and fix(libkernel): fail open with O_CREAT and O_EXCL on a Windows symbolic link #9496 merged together, same machine: 1890 tests each, identical results (1869 passed, 18 skipped; on both, the 3agc_driver_sample_order*tests fail andagc_driver_image_array_window_past_layer_limitis not built, as its 64 KiBalignasis rejected by this MinGW toolchain).python tools/check_conventions.py --base upstream/main: passes.guest_kernel_syspasses, and fails withlchflags_nid_postfix not implementedwithout the change); see the review.Overlaps
PathErrorto copy the guest path and editschflags, which this PR moves intoChangeFileFlags_nid_no_patch. Proposed order: fix(kernel): safely copy guest path arguments #9423 first; this PR is then rebased so the helper takes the copied path for bothchflagsandlchflags. If this PR lands first, fix(kernel): safely copy guest path arguments #9423 moves its onePathErrorcall into the helper. Asked on fix(kernel): safely copy guest path arguments #9423.lchmod) removes the stub next tolchflagsinExport.Discovery.cpp, and feat(libkernel_sys): implement sceKernelFlock over flock #9097 and feat(libkernel_sys): implement chown, lchown, fchownat and linkat #9103 add code next to these lines inGuestKernelSys.cppandlibkernel_sys/Export.cpp: text-only, whichever lands second rebases.GuestKernelSys.cpptotests/process/; if it lands first, the new cases move with the file in the rebase.tools/pr_overlap.py --branchon the rebased head lists no other direct overlap; of the 22 pull requests it could not compare, none changeschflags,LookupErrororlchflags.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); no silent stubs added.mdfiles or images added to the repositoryPathErrorandchflags; this PR goes after it and is then rebased soChangeFileFlags_nid_no_patchkeeps the copied path). Text-only neighbours feat(libkernel_sys): implement lchmod #9200, feat(libkernel_sys): implement sceKernelFlock over flock #9097 and feat(libkernel_sys): implement chown, lchown, fchownat and linkat #9103: this PR goes second and rebases.AI-assisted (Claude Code).
🤖 Generated with Claude Code