Fix: IntrinsicError when a non-directory PATH entry is used with system_binary#23327
Fix: IntrinsicError when a non-directory PATH entry is used with system_binary#23327jbylund wants to merge 3 commits intopantsbuild:mainfrom
system_binary#23327Conversation
… PATH Treat ENOTDIR the same as ENOENT in `PosixFS::path_metadata`: return `Ok(None)` rather than propagating the error. This covers the case where a PATH element like `foo/bar` has a file at `foo`, which was crashing with `IntrinsicError: Not a directory (os error 20)`. Also guard the symlink-to-file case in `system_binaries.py`: check `os.path.isdir()` before constructing a `symlink/binary_name` lookup, and log a debug message when a symlink on PATH does not resolve to a directory. Fixes pantsbuild#21990
|
|
||
| if metadata.kind in (PathMetadataKind.DIRECTORY, PathMetadataKind.SYMLINK): | ||
| if metadata.kind == PathMetadataKind.DIRECTORY or ( | ||
| metadata.kind == PathMetadataKind.SYMLINK and os.path.isdir(metadata.path) |
There was a problem hiding this comment.
Calling filesystem APIs from Pants rule code is forbidden since it prevents the Pants engine from knowing about the filesystem accesses. If you need to check whether the symlink target is a directory, then aggregate those requests and call the path_metadata_request rule again just as is done with file_metadata_requests.
There was a problem hiding this comment.
A better solution would be to add a follow_symlinks option to PathMetadataRequest and then use that option here. This would leave the Pants engine in control of filesystem accesses.
The relevant code in the Rust layer is
pants/src/rust/fs/src/posixfs.rs
Line 248 in 7c08ed5
Instead of always calling tokio::fs::symlink_metadata, that code can switch on follow_symlinks and call either tokio::fs::symlink_metadata (if false) or tokio::fs::metadata (if true).
Closes #21990
Problem
Pants crashed with
IntrinsicError: Not a directory (os error 20)when asystem_binarytarget (or any binary lookup) included a non-directory entry inits search path. Two distinct shapes of the same underlying bug:
foo/baron PATH wherefoois a file. Callingpath_metadata_requeston
foo/barhitsENOTDIRin the OS. That error was not caught in the Rustlayer, so it surfaced as an uncaught
IntrinsicError.A symlink-to-file on PATH — the case from the issue report. The first
path_metadata_requestsucceeds and returnsPathMetadataKind::SYMLINK,but the Python code then tried to look up
symlink/binary_name, which alsohits
ENOTDIR.Fix
src/rust/fs/src/posixfs.rs—path_metadatanow accepts afollow_symlinksflag. When
true, it callstokio::fs::metadata(which follows symlinks) insteadof
tokio::fs::symlink_metadata, so a symlink-to-directory appears as aDirectoryrather than aSymlink. It also continues to treatENOTDIRas absent(
Ok(None)), fixing the file-component-on-PATH crash.src/rust/fs/src/lib.rs— TheVfstrait'spath_metadatasignature isupdated to accept
follow_symlinks: bool. The in-memoryDigestTrieimplementationaccepts but ignores the flag.
src/rust/engine/src/nodes/path_metadata.rs—PathMetadataNodenow carriesfollow_symlinksand passes it through to the VFS call.src/rust/engine/src/intrinsics/digests.rs— Thepath_metadata_requestintrinsic reads
follow_symlinksfrom the PythonPathMetadataRequestobject andpasses it to
PathMetadataNode.src/python/pants/engine/fs.py—PathMetadataRequestgains afollow_symlinks: bool = Falsefield.src/python/pants/core/util_rules/system_binaries.py— The initial lookup ofeach PATH entry uses
follow_symlinks=True. A symlink-to-directory is returned asPathMetadataKind::DIRECTORYand handled correctly; a symlink-to-file is returnedas
PathMetadataKind::FILEand skipped.src/python/pants/core/util_rules/system_binaries_test.py— Three regressiontests alongside the existing
test_find_binary_file_path:test_find_binary_symlink_to_dir_on_path— symlink on PATH pointing to a directory containing the binary is found correctlytest_find_binary_symlink_to_file_on_path— symlink on PATH pointing to a file is skipped gracefullytest_find_binary_file_component_on_path— path likenot_a_dir/subdirwherenot_a_diris a file is skipped gracefully