Unify FSAccessor and SourceAccessor
#9269
Conversation
Instead stat() now returns std::nullopt to denote that the file doesn't exist.
The narOffset field only applies to NAR accessors. The fileSize field may be too expensive to compute for certain accessors (e.g. libgit).
github-actions
bot
added
new-cli
Ericson2314
changed the title
FSAccessor and SourceAccessor
| @@ -1 +1 @@ | |||
| error: getting status of '/pwd/lang/fnord': No such file or directory | |||
| error: path '/pwd/lang/fnord' does not exist | |||
There was a problem hiding this comment.
This isn't a sign we lost a potentially useful perror() via SysError is it?
There was a problem hiding this comment.
No, it's because SourceAccessor::lstat() now handles the file-not-found case. SourceAccessor::maybeLstat() returns std::nullopt if the file doesn't exist, but throws a SysError for everything else.
|
@infinisil might you know what is going on with this Nixpkgs test that is failing? We should probably create a new |
|
Looks like the error message when calling
to
And that's being tested in Nixpkgs. Doesn't seem like a serious problem, there's no requirement for Nix to stay backwards compatibly with error messages. I guess it would be best to not check Nix-specific error messages in the Nixpkgs tests. |
|
Thanks, yeah we should update the Nixpkgs test to not over-test. (In this specific case I think we might want the |
|
Done in NixOS/nixpkgs#264860 |
There was a problem hiding this comment.
Thanks again for doing this, @edolstra. This really is a massive improvement to me, and I very much appreciate both it being done and not having to do it myself. Happy to approve it now regardless of how it might change before it's merged.
Co-authored-by: John Ericson <git@JohnEricson.me>
In NixOS/nix#9269 the error messages change which would've broken this test.
In NixOS/nix#9269 the error messages change which would've broken this test. (cherry picked from commit add2546)
Flake lock file updates:
• Updated input 'nixpkgs':
'github:NixOS/nixpkgs/31ed632c692e6a36cfc18083b88ece892f863ed4' (2023-09-21)
→ 'github:NixOS/nixpkgs/9eb24edd6a0027fed010ccfe300a9734d029983c' (2023-11-01)
Co-authored-by: John Ericson <git@JohnEricson.me>
|
This broke the test suite on macOS. |
In NixOS/nix#9269 the error messages change which would've broken this test.
|
The change also broke user-facing API. It broke |
|
Note the C++ API was never supposed to be stable, at least between releases. But yes, |
Motivation
This removes
FSAccessor, moving all its functionality intoSourceAccessor.Context
Priorities
Add 👍 to pull requests you find important.