Skip to content
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

Install utility does not compile on Windows #1174

Closed
danieldulaney opened this issue Mar 29, 2018 · 5 comments · Fixed by #1449
Closed

Install utility does not compile on Windows #1174

danieldulaney opened this issue Mar 29, 2018 · 5 comments · Fixed by #1449

Comments

@danieldulaney
Copy link

The install utility fails to build on Windows (nightly-x86_64-pc-windows-gnu 2018-03-27).

D:\git\coreutils>cargo build --no-default-features --features install
   Compiling install v0.0.1 (file:///D:/git/coreutils/src/install)
error[E0433]: failed to resolve. Use of undeclared type or module `mode`
  --> src\install\mode.rs:14:9
   |
14 |         mode::parse_numeric(0, mode_string)
   |         ^^^^ Use of undeclared type or module `mode`

error[E0433]: failed to resolve. Use of undeclared type or module `mode`
  --> src\install\mode.rs:16:9
   |
16 |         mode::parse_symbolic(0, mode_string, considering_dir)
   |         ^^^^ Use of undeclared type or module `mode`

warning: unused import: `std::fs`
 --> src\install\mode.rs:4:5
  |
4 | use std::fs;
  |     ^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

error: aborting due to 2 previous errors

It looks like the error is because install/mode.rs doesn't use uucore::mode on Windows, but doesn't have an alternate implementation of parse for Windows to use.

#[cfg(not(windows))]
use uucore::mode;
/// Takes a user-supplied string and tries to parse to u16 mode bitmask.
pub fn parse(mode_string: &str, considering_dir: bool) -> Result<u32, String> {
let numbers: &[char] = &['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
// Passing 000 as the existing permissions seems to mirror GNU behaviour.
if mode_string.contains(numbers) {
mode::parse_numeric(0, mode_string)
} else {
mode::parse_symbolic(0, mode_string, considering_dir)
}
}

Does it make sense to break up parse into Windows and non-Windows variants the same way chmod is broken up? Or maybe something has to change at the caller to make sure Windows code doesn't rely on parse?

@Arcterus
Copy link
Collaborator

How are you trying to build? At the moment, the proper way to build on Windows is cargo build --no-default-features --features generic (this should probably be added to the README until we can enable features depending on the target).

I'm not sure how modes should work on Windows honestly, as Windows permissions are different from Unix permissions. I suppose if you want install to compile on Windows and you are willing to use it without being able to change file permissions, going the chmod route would be fine. Ideally though we'd figure out an analogue for Windows.

@danieldulaney
Copy link
Author

I built it with that and it totally worked! Thanks! Would it be possible to add those instructions to README? I think it would be useful for people trying to use the project on Windows.

On the bigger picture, Windows does have some vaguely analogous features. It looks like the SetSecurityInfo function does lots of the same things, but it could be very misleading to just slap a chmod-style interface on top. There's lots of opportunities for things to get lost in translation, and pretty much anyone who wants to manage Windows file permissions at that granularity is probably using some other tool.

I don't see any of the authorization functions in the winapi crate, so they might not be available yet. It looks like chmod and other things that set file permissions are just no-ops on Windows, so it would be consistent at least for install to work the same way.

@Arcterus
Copy link
Collaborator

Arcterus commented Apr 2, 2018

No problem. :)

I actually didn't realize they weren't in the README, so I'll add them later (unless someone does it before me).

I'm thinking that we'll just make them no-ops (or issue an error at runtime or something) unless we can figure out a clear mapping between the Unix-style permissions and Windows ones. We might want to take a look at how existing ports of the GNU coreutils to Windows handle this issue (by seeing what Windows permissions are changed after running things like chmod g+x file and such), if they even handle the issue at all.

@danieldulaney
Copy link
Author

This looks like a really tricky issue.

Cygwin (the only coreutils port I've ever used) has a bunch of documentation on how they implemented their system. They spend a lot of time on windows user-uid mapping, but then under the "File permissions" heading they go through some of the complexities with chmod. It looks like they do make a best-effort attempt at making it all work, but I'm not sure if it makes sense to put that level of effort into implementing it here. Doing Cygwin-style shimming for any more of glibc on Windows would be a major project.

I think it makes sense for it to be a no-op with a runtime error, at least for now. The same probably applies to some of the other crates that don't build on Windows (chmod, chown, chroot, groups?, hostid?).

@Arcterus
Copy link
Collaborator

Arcterus commented Apr 11, 2018

Yeah, I'm inclined to just make them no-ops at the moment. Some of the ones that don't build on Windows right now actually can be relatively easily made to support Windows but need someone to look through the Windows docs and write some platform-specific code. Of course, in some cases, the code needed to properly support Windows is non-trivial, despite seemingly like the necessary code should be relatively simple (e.g. look at this).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants