-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Windows paths in canonPath
and absPath
#9767
Conversation
245aaf6
to
f4b069f
Compare
🎉 All dependencies have been resolved ! |
f4b069f
to
a45023a
Compare
a45023a
to
1428ec2
Compare
A few meetings ago @edolstra had asked whether it would be better to go straight to using (This PR more straightforwardly preserves behavior by reusing code, and increasing test coverage. We can afterwards try |
I'm pretty wary about adding a lot of complexity to support Windows, if we haven't even decided whether we want that, what that would look like, and whether there is sufficient interest to justify the complexity. (Having no access to Windows development machines, I really don't want to maintain code with a lot of WIN32 #ifdefs.) It might be better to develop a Windows proof-of-concept on a separate branch for the time being. |
There are now some years-old separate branches in https://github.com/nix-windows/nix. I did write up that document. I am trying to minimize the CPP by dropping features to start, focusing on the unit tests, etc. and using more We can try asking the Nix community some more, but while I expect people to be passively interested I wouldn't expect much up-front commitment. The Windows and Unix industries are fairly disjoint, and plenty of people are happily avoiding Windows (certainly myself included for any downstream project I have full control over.) For me, this is more an expanding into new areas thing / growing the Nix community in new ways, than making something existing community members will immediate jump on.
I do all the development cross compiling from Linux, and will be creating a derivation to run the unit tests with Wine before merging something more significant. I would be fine simply not testing native Windows builds for the imminent future (a long trial mode) so there is no build/test failure that can only be reproduced on them. |
1428ec2
to
c92b577
Compare
In the meantime, #9881 is the first part which should be worthwhile with or without Windows. |
@edolstra Discourse is not letting me reply to myself another time, so if you could comment in https://discourse.nixos.org/t/nix-on-windows/1113 saying you're trying to gauge user interest, that would be appreciated. |
#ifdef _WIN32 | ||
# define FS_SEP "\\" | ||
# define FS_ROOT "C:" FS_SEP // Need a mounted one, C drive is likely | ||
#else | ||
# define FS_SEP "/" | ||
# define FS_ROOT FS_SEP | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we get this from NativePath
and save a #ifdef
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, though then I couldn't rely on "easy" literal concatenation. Also note that FS_ROOT
we still cannot get from it because on Windows it is an arbitrary choice of drive just for testing purposes.
c92b577
to
8378bd5
Compare
We agreed I would try to get rid of |
Discussed in meeting today
// split path, trying to make the native prefix as short as possible, and CanonPath suffix as long as possible
std::fs::path -> (std::fs::path, CanonPath)
// feed native prefix to NativeInputAccessor to create SourcePath
(std::fs::path, CanonPath) -> SourcePath
SourcePath rootPath(CanonPath path);
SourcePath rootPath(std::fs:path path);
SourcePath rootPath(std::fs:path path, cwd ...) Agreed |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-02-05-nix-team-meeting-minutes-121/39340/1 |
e8096e9
to
3e06333
Compare
This prepares the code to also support Windows paths in the next commit.
24d6be7
to
2d21bf5
Compare
canonPath
and absPath
without messing up CanonPath
canonPath
and absPath
`canonPath` and `absPath` work on native paths, and so should switch between supporting Unix paths and Windows paths accordingly. The templating is because `CanonPath`, which shares the implementation, should always be Unix style. It is the pure "nix-native" path type for virtual file operations --- it is part of Nix's "business logic", and should not vary with the host OS accordingly.
2d21bf5
to
319ec6f
Compare
Thanks! |
Motivation
canonPath
andabsPath
work on native paths, and so should switch between supporting Unix paths and Windows paths accordingly.The templating is because
CanonPath
, which shares the implementation, should always be Unix style. It is the pure "nix-native" path type for virtual file operations --- it is part of Nix's "business logic", and should not vary with the host OS accordingly.Context
Depends on #9759Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.