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

Windows isn’t supported #2

Closed
chris-morgan opened this issue Oct 20, 2016 · 9 comments
Closed

Windows isn’t supported #2

chris-morgan opened this issue Oct 20, 2016 · 9 comments
Assignees

Comments

@chris-morgan
Copy link

Firstly, it’s depending on std::os::unix which isn’t available on Windows:

error[E0432]: unresolved import `std::os::unix::fs::PermissionsExt`
 --> src/build.rs:6:5
  |
6 | use std::os::unix::fs::PermissionsExt;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `unix` in `std::os`

error: no method named `mode` found for type `std::fs::Permissions` in the current scope
   --> src/build.rs:173:30
    |
173 |     let current_mode = perms.mode();
    |                              ^^^^

error: no method named `set_mode` found for type `std::fs::Permissions` in the current scope
   --> src/build.rs:174:11
    |
174 |     perms.set_mode(0o100 | current_mode);
    |           ^^^^^^^^

error: aborting due to 2 previous errors

Setting +x isn’t relevant on Windows, so that can just be stubbed out on Windows:

diff --git a/src/build.rs b/src/build.rs
index 81eb58d..c239317 100644
--- a/src/build.rs
+++ b/src/build.rs
@@ -3,6 +3,7 @@ use std::ffi::OsString;
 use std::fs::File;
 use std::fs;
 use std::env;
+#[cfg(unix)]
 use std::os::unix::fs::PermissionsExt;
 use std::io::{Write,BufReader};
 use std::io::BufRead;
@@ -167,12 +168,21 @@ fn main() {
         println!("[build] Patching complete");
     }

-    // Set executable bits on the file
+    // Set executable bits on the file if appropriate (it's not on Windows)

-    let mut perms = fs::metadata(&detect_path).ok().expect("metadata missing").permissions();
-    let current_mode = perms.mode();
-    perms.set_mode(0o100 | current_mode);
-    fs::set_permissions(&detect_path, perms).ok().expect("permissions could not be set");
+    #[cfg(unix)]
+    fn set_exec(detect_path: &Path) {
+        let mut perms = fs::metadata(&detect_path).ok().expect("metadata missing").permissions();
+        let current_mode = perms.mode();
+        perms.set_mode(0o100 | current_mode);
+        fs::set_permissions(&detect_path, perms).ok().expect("permissions could not be set");
+    }
+
+    #[cfg(not(unix))]
+    fn set_exec(_detect_path: &Path) {
+    }
+
+    set_exec(&detect_path);

     // Build LevelDB
     build_leveldb(have_snappy);

But then the dependence on make crops up:

error: failed to run custom build command for `leveldb-sys v2.0.0 (file:///C:/code/leveldb-sys)`
process didn't exit successfully: `C:\code\leveldb-sys\target\debug\build\leveldb-sys-12748421a86593ec\build-script-build` (exit code: 101)
--- stdout
[build] Started
[build] Patching the `build_detect_platform` template
[build] Patching complete
[leveldb] Cleaning

--- stderr
thread 'main' panicked at 'clean failed', ../src/libcore\option.rs:705
note: Run with `RUST_BACKTRACE=1` for a backtrace.

In short, more work is needed to make it run smoothly on Windows.

I think the gcc crate is all about this sort of thing?

[Ported from https://github.com/andrew-d/leveldb-rs/issues/9 when I didn’t realise this had been split.]

@chris-morgan
Copy link
Author

Oh, apparently LevelDB doesn’t currently support Windows. That’s really odd. I thought I heard mention of it being used in things like Chrome which definitely run on Windows.

@skade
Copy link
Owner

skade commented Oct 25, 2016

As far as I heard, Google does not use LevelDB much anymore anyways.

RocksDB and similar seem to be more popular.

On 20 Oct 2016, at 18:44, Chris Morgan notifications@github.com wrote:

Oh, apparently LevelDB doesn’t currently support Windows. That’s really odd. I thought I heard mention of it being used in things like Chrome which definitely run on Windows.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@adumbidiot
Copy link

I believe that leveldb has added support for Windows now, so would it be possible to reopen this issue?

@skade
Copy link
Owner

skade commented Apr 1, 2020

@adumbidiot patches definitely welcome!

Reference here: https://github.com/google/leveldb#building-for-windows

@skade skade reopened this Apr 1, 2020
@adumbidiot
Copy link

Merging #14 is important since Windows support is only on newer versions of leveldb. However, it seems like the build system changed from gmake to cmake, which that pr seems to address. The pr also seems to remove all unix-specific functionality, so I think that pr should be sufficient for Windows support.

@skade
Copy link
Owner

skade commented Apr 2, 2020

@adumbidiot can I rely on you for windows testing? Then I'd do that.

@adumbidiot
Copy link

adumbidiot commented Apr 3, 2020

Sure. I just tested #14, and it seems to fail on windows when snappy support is enabled. The build script fails to find the compiled snappy lib as the windows linker doesn't understand the specified flag.

I believe this could be fixed by adding the windows linker flag to the LDFLAGS var, changing https://github.com/timberio/leveldb-sys/blob/update-leveldb/src/build.rs#L43 to something like format!("-L{dir} -libpath:{dir}", dir = snappy_prefix.join("lib").display()), though that seems kinda hacky.

@skade
Copy link
Owner

skade commented Apr 9, 2020

Hm, thanks. I have a windows machine available, I may check that.

Thanks for having a thorough look at that !

@skade skade self-assigned this Apr 9, 2020
@skade
Copy link
Owner

skade commented Sep 20, 2020

Fixed by #18

@skade skade closed this as completed Sep 20, 2020
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

No branches or pull requests

3 participants