linfa icon indicating copy to clipboard operation
linfa copied to clipboard

CI testing on Windows missing

Open relf opened this issue 2 years ago • 4 comments

Trying to compile linfa master, I stumbled on the following:

   > cargo test --all --release
   [...]
   Compiling mnist v0.5.0
   Compiling time v0.2.26
error[E0433]: failed to resolve: could not find `unix` in `os`
  --> D:\rlafage\.cargo\registry\src\github.com-1ecc6299db9ec823\mnist-0.5.0\src\download.rs:88:34
   |
88 |                     use std::os::unix::fs::MetadataExt;
   |                                  ^^^^ could not find `unix` in `os`

error[E0599]: no method named `size` found for struct `Metadata` in the current scope
  --> D:\rlafage\.cargo\registry\src\github.com-1ecc6299db9ec823\mnist-0.5.0\src\download.rs:99:49
   |
99 | ...                   current_size = meta.size() as usize;
   |                                           ^^^^ method not found in `Metadata`

Some errors have detailed explanations: E0433, E0599.
For more information about an error, try `rustc --explain E0433`.
error: could not compile `mnist` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: build failed

Actually, it is a mnist 0.5 bug. I will open an issue on mnist repo. The issue for linfa is the lack of automated tests on Windows to catch this.

relf avatar Mar 31 '22 09:03 relf

I can bump the mnist version but I don't have experience with Windows in GitHub Actions

YuhanLiin avatar Apr 01 '22 01:04 YuhanLiin

There's not a new version to bump to yet; I just created a branch that should have the fix, but Remi will be testing it locally before merging it will be merged upstream into mnist main. Once that happens, it can get pushed onto crates.io (it shouldn't be a breaking change, so will just be a patch version bump). I'm not sure what that timeline will look like; it doesn't look like the owner of the mnist crate has been very active on Github recently, so it may be at least a few days before everything resolves itself.

It could make sense to add Windows to the Github Actions CI, but honestly I know very few developers, and even fewer ML developers, that use Windows as their primary OS, so I'm not sure how strong the prioritization should be, versus just trying to fix bugs as they're reported by users like in this instance.

quietlychris avatar Apr 01 '22 01:04 quietlychris

I understand that it has not a very high priority. Such problem may not arise that often.... It has not so far. At the moment, ML in Rust is not that common and on Windows... Well I feel like a weirdo for sure 🤣 . Anyway I will try to submit a PR to implement CI tests for Windows.

relf avatar Apr 01 '22 07:04 relf

Okay, a PR with the fix for the mnist crate has been submitted upstream, just waiting on a response from the maintainer

quietlychris avatar Apr 06 '22 00:04 quietlychris