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

Supported targets might not need an exact target triple check #15

Closed
hug-dev opened this issue Mar 29, 2021 · 3 comments · Fixed by #18
Closed

Supported targets might not need an exact target triple check #15

hug-dev opened this issue Mar 29, 2021 · 3 comments · Fixed by #18
Assignees
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers

Comments

@hug-dev
Copy link
Member

hug-dev commented Mar 29, 2021

When we select the bindings that we want, we only check target_arch and target_os. If those two parameters are the only things determining the value of the bindings, then we can simplify this check in build.rs:

        let supported_platforms = vec![
            "x86_64-unknown-linux-gnu".to_string(),
            "aarch64-unknown-linux-gnu".to_string(),
            "armv7-unknown-linux-gnueabi".to_string(),
            "armv7-unknown-linux-gnueabihf".to_string(),
            "arm-unknown-linux-gnueabi".to_string(),
        ];
        let target = std::env::var("TARGET").unwrap();

        // check if target is in the list of supported ones or panic with nice message
        if !supported_platforms.contains(&target) {
            panic!("Compilation target ({}) is not part of the supported targets ({:?}). Please compile with the \"generate-bindings\" feature or add support for your platform :)", target, supported_platforms);
        }

by parsing the target triple as <arch><sub>-<vendor>-<sys>-<abi> (see here) and only checking for the tuple (arch, os).

That would allow much more crates to be used with the generated bindings.

edit: exact same issue on the tss-esapi crate.

@hug-dev hug-dev added bug Something isn't working enhancement New feature or request good first issue Good for newcomers labels Mar 29, 2021
@anta5010
Copy link

Please notice that ABI might be omitted. For example, Yocto poky sets target as aarch64-poky-linux, x86_64-poky-linux, etc.

@hug-dev
Copy link
Member Author

hug-dev commented Apr 1, 2021

Related, see:

I am thinking, should we remove that check altogether? I think linking will fail without a harder to decipher message but maybe we can do something about that. That would solve this.

@ionut-arm
Copy link
Member

It will fail with a large list of errors that won't be helpful at all - I've already seen it happen on the TSS crate (because of the feature = armv7 instead of arm mistake) :) I think the proposed solution from the top comment is best - checking for (arch, os) tuple instead. We're not going to have an unbounded list of supported triples anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants