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

DM version detection broken when cross compiling #801

Open
flxo opened this issue Nov 10, 2022 · 8 comments
Open

DM version detection broken when cross compiling #801

flxo opened this issue Nov 10, 2022 · 8 comments
Assignees
Labels

Comments

@flxo
Copy link
Collaborator

flxo commented Nov 10, 2022

The build script uses device-mapper-sys for the dm version detection. The dependency is declared here.
What happens, is that the TARGET is set to the host target - the machine that is used for building. The result is that bindgen in the device-mapper-sys build script uses the hosts dm-ioctl.h header in instead of the one (probably) in the cross toolchain in use. If the headers differ a wrong version is detected. In my case accidently 441 is assumed but the later build fails because a DM_GET_TARGET_VERSION_CMD is missing because during the later cross build TARGET is correct.

I verified with a rustc:warning in the device-mapper-sys build script:

// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

use std::{env::var, path::PathBuf};

use bindgen::Builder;

fn main() {
    println!("cargo:warning={:?}", std::env::var("TARGET"));
    let bindings = Builder::default()
        .header("dm-ioctl.h")
        .derive_debug(true)
        .derive_default(true)
        .generate()
        .expect("Could not generate bindings");

    let mut bindings_path = PathBuf::from(var("OUT_DIR").unwrap());
    bindings_path.push("bindings.rs");
    bindings
        .write_to_file(&bindings_path)
        .expect("Could not write bindings to file");
}

Then a random cross compile reveals:

# cargo check --target aarch64-linux-android    
    Checking bitflags v1.3.2
    Checking cfg-if v1.0.0
    Checking lazy_static v1.4.0
   Compiling libc v0.2.137
   Compiling serde v1.0.147
   Compiling semver v1.0.14
   Compiling memoffset v0.6.5
   Compiling devicemapper-sys v0.1.3 (/home/felix/devicemapper-rs/devicemapper-rs-sys)
warning: Ok("x86_64-unknown-linux-gnu")
   Compiling devicemapper v0.32.2 (/home/felix/devicemapper-rs)
The following warnings were emitted during compilation:

warning: Ok("aarch64-linux-android")
...

A solution must tell the build script in the dependency (device-mapper-sys) that TARGET is something different. This is the classic "build script is not cross compiled problem".

@flxo flxo added the bug label Nov 10, 2022
@flxo
Copy link
Collaborator Author

flxo commented Nov 10, 2022

I didn't find a way to tell build devicemapper-rs-sys crate the correct target tripple.
Here the simple hack: hide the detection behind a feature.

What do you think?

@mulkieran mulkieran added this to To do in 2022November via automation Nov 10, 2022
@mulkieran
Copy link
Member

@flxo Is it reasonable to assume that the correct dm-ioctl.h contents is somehow provided by the cross-toolchain and that our build script is just not looking in the correct place? Or does it make sense to allow specifying an alternative location for the dm-ioctl.h file during the build process?

@flxo
Copy link
Collaborator Author

flxo commented Nov 11, 2022

@mulkieran The cross toolchain is not used when the bindings are generated the first time (for version detection) and the correct header is not available.
A cross compile is something like:

build devicemapper-rs for *target*
  build other dependencies for *target*
  build devicemapper-rs build.rs for *host* (for the version detection)
     build devicemapper-sys-rs build.rs for *host* (build dependency - generates bindings for host - *not* for target)
  run devicemapper-rs build script on *host* to configure features
  build devicemapper-sys-rs for *target*
    generate target bindings (because bindgen is aware of TARGET).

@mulkieran mulkieran removed this from To do in 2022November Nov 21, 2022
@mulkieran mulkieran added this to To do in 2022December via automation Nov 21, 2022
@mulkieran mulkieran removed this from To do in 2022December Dec 26, 2022
@mulkieran mulkieran added this to To do in 2023January via automation Dec 26, 2022
@jbaublitz
Copy link
Member

@flxo After looking into this a little bit, would

.clang_arg(format!("target={}", env!("TARGET")).as_str())

work in bindgen?

@mulkieran mulkieran moved this from To do to In progress (long term) in 2023January Jan 20, 2023
@flxo
Copy link
Collaborator Author

flxo commented Jan 23, 2023

@flxo After looking into this a little bit, would

.clang_arg(format!("target={}", env!("TARGET")).as_str())

work in bindgen?

I don't think so because the host toolchain is used and not the one for the target. This approach would require to setup the full cross compile foo in the build script - e.g search for clang etc. The problem is that we need the target headers.

@jbaublitz
Copy link
Member

I see two options.

The first I'm less sure about. I know a little bit less about cross compilation than other domains of cargo so I'm not sure how realistic this is, but I could file an upstream bug to have cargo respect the cross compile target in its build-dependencies. The piece I'm not sure about is I would expect the build-dependencies to have to run on the host so I could see this potentially rendering the build-dependencies unrunable. However, the TARGET in devicemapper-rs is set to the cross compile target so I don't see why devicemapper-rs-sys might not be able to do the same.

The second option I see is generating the low level bindings in devicemapper-rs as well just for version detection and removing devicemapper-rs-sys as a build dependency. Because the target is set appropriately in devicemapper-rs's build script, I think this should work.

What are your thoughts?

@mulkieran mulkieran removed this from In progress (long term) in 2023January Jan 30, 2023
@mulkieran mulkieran added this to To do in 2023February via automation Jan 30, 2023
@mulkieran mulkieran moved this from To do to In progress (long term) in 2023February Jan 30, 2023
@flxo
Copy link
Collaborator Author

flxo commented Jan 31, 2023

I see two options.

The first I'm less sure about. I know a little bit less about cross compilation than other domains of cargo so I'm not sure how realistic this is, but I could file an upstream bug to have cargo respect the cross compile target in its build-dependencies. The piece I'm not sure about is I would expect the build-dependencies to have to run on the host so I could see this potentially rendering the build-dependencies unrunable. However, the TARGET in devicemapper-rs is set to the cross compile target so I don't see why devicemapper-rs-sys might not be able to do the same.

Think it's not only about setting TARGET. For getting the desired correct generated content you need the headers for the target - means the target toolchain including options, sysroot etc...

The second option I see is generating the low level bindings in devicemapper-rs as well just for version detection and removing devicemapper-rs-sys as a build dependency. Because the target is set appropriately in devicemapper-rs's build script, I think this should work.

Hm. That should be a workaround.
You still have the problem, that devicemapper-sys-rs build script uses the headers from the host toolchain and not from the target. This is probably not a problem because the host toolchains are normally newer than the cross target ones and the headers are backward compatible. In my case I had a feature enabled by the build script and the later cross build failed because the cross toolchain doesn't know about some defines etc..
Until it's possible to pass all the cross setup down to build scripts of dependencies this is broken.

But isn't the problem we're discussing here a general one for every -sys crate that uses bindgen in a build script in cross compile?

@jbaublitz
Copy link
Member

@flxo I think this is a more general problem. I've created a tracking issue for all of our -sys libraries (linked above) where you can provide your feedback.

I feel like I'm still misunderstanding a little bit what exactly you want. Are you saying you want to cross compile without having all of the target headers installed? Are you saying you have all of the target headers installed but in the current state of things, devicemapper-rs is using the host headers? I think I need a little bit more concrete information about what your desired state is.

I may need to read up a little bit more on cross compilation in Rust because I'm not as familiar with this domain as other parts of the toolchain.

@mulkieran mulkieran removed this from In progress (long term) in 2023February Mar 13, 2023
@mulkieran mulkieran added this to To do in 2023March via automation Mar 13, 2023
@mulkieran mulkieran moved this from To do to In progress (long term) in 2023March Mar 13, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023March Apr 2, 2023
@mulkieran mulkieran added this to To do in 2023April via automation Apr 2, 2023
@mulkieran mulkieran moved this from To do to In progress (long term) in 2023April Apr 2, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023April May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants