axodotdev / cargo-dist

📦 shippable application packaging
https://axodotdev.github.io/cargo-dist/
Apache License 2.0
1.47k stars 66 forks source link

Arm cross build succeeds but fails at trivial step 'unable to run linkage report for this type of binary' #1388

Closed CramBL closed 3 weeks ago

CramBL commented 1 month ago

Hi thanks for this awesome tool, unfortunately my app doesn't make much sense to distribute without support for arm and armv7. I already distribute prebuilt binaries for a bunch of platforms but I am trying to switch to cargo-dist.

Using a bunch of "advanced" cargo-dist features I managed to hack a workflow together that builds for these targets (same as I always supported):

targets = [
    "aarch64-apple-darwin",
    "aarch64-unknown-linux-musl",
    "aarch64-pc-windows-msvc",
    "arm-unknown-linux-musleabihf",
    "armv7-unknown-linux-musleabihf",
    "x86_64-apple-darwin",
    "x86_64-unknown-linux-gnu",
    "x86_64-unknown-linux-musl",
    "x86_64-pc-windows-msvc",
]

workflow here: https://github.com/CramBL/fidelityfetch/actions/runs/10637940239

But arm and armv7 jobs fail after building with:

× unable to run linkage report for this type of binary; file was: Some("fife")

From reading issues such as https://github.com/axodotdev/cargo-dist/issues/74 and https://github.com/axodotdev/cargo-dist/issues/1378 I understand that you do not support these two targets (yet?) but it is blocking me from using cargo-dist for many many projects, and since I can actually get it to build by using your provided configuration "hacks" I suppose I should be able to bypass this final(?) issue of the linkage report failing?

ashleygwilliams commented 1 month ago

hey @CramBL - thank you so much for sharing the link to the repo and the detail. my understanding was that in 0.22.0 we made the linkage error non-fatal and so i am surprised to see this come up. give me a quick sec to see what exactly is going on.

Re support for those platforms.. in general, we support the platforms you can find runners for but our sysdep feature is currently artificially reducing that support because it's a bit more pedantic about triples than it should be. we consider that a bug and will be fixing it for an upcoming release. you can, as the linked issue shows, get around it for now by using the custom build setup feature, but we agree it's not the ideal solution :)

i saw that you also linked the cross-compilation feature, we'll be working on that in the next month as we prepare for 1.0 so that will also likely shake out a lot of the issues you are facing with those two target triples, though my suspicion is that if you do not require true crossing for those we should be able to get you sorted before that ships.

ashleygwilliams commented 1 month ago

yeah so reading through this, the fact that you are failing fully on the linkage check is a bug and it also very suspicious that it's printing out the debug format of the Option. i'll take a look into this, but this is definitely not intended behavior.

CramBL commented 1 month ago

Thanks for the swift response! I am not in a hurry to get this working, I'm just happy to hear that it is something you are planning to address in the near future.

I value cross-compilation highly because building for many targets on the ubiquitous x86 Ubuntu simplifies things, and cross-compiling for arm speeds up iteration/deployment a lot when developing for embedded Linux platforms.

ashleygwilliams commented 1 month ago

glad to know your time constraints! i am the only one in the office atm because of the holiday here. your usecase is one we definitely want to support and so i want to make sure these early bumps don't turn you off. gimme some time to dig in, i should be able to get you over this linkage check hump by early next week!

andrewdavidmackenzie commented 1 month ago

I'm facing link problems also (#1378 ) and investigating the error message I get:

" = note: /usr/bin/ld: /home/runner/work/pigg/pigg/target/aarch64-unknown-linux-gnu/dist/deps/piggui-50dd4cb48bc0e8bd.8zkhcavse9r40lavybm85km7r.rcgu.o: Relocations in generic ELF (EM: 183)"

I see posts like this saying

"This is invoking the wrong linker (system linker), and most likely also invoking the wrong compiler to compile first.c."

mistydemeo commented 3 weeks ago

The linkage check issue should be fixed by #1390.

As for the comment about ld, improved cross-compilation support will allow that to work better: https://github.com/axodotdev/cargo-dist/issues/1378#issuecomment-2332697441

I'm going to close this since I believe the linkage issue is resolved, and the fix will be in an upcoming release.

andrewdavidmackenzie commented 2 weeks ago

I have fixed my other build and linker problems, and now face only this issue in one of my targets for RPi.

It's only failing when compiling for armv7 (armv7-unknown-linux-gnueabihf) and not aarch64 (aarch64-unknown-linux-gnu)

Failing GH Action release build: https://github.com/andrewdavidmackenzie/pigg/actions/runs/10923873673/job/30321572872

Could that be related to the "unusual" triple for that target, as you mention triple parsing issues above?

In case it helps, I specify linkers in .cargo/config.toml thus:

[target.aarch64-unknown-linux-gnu]
linker = "aarch64-linux-gnu-gcc"

[target.armv7-unknown-linux-gnueabihf]
linker = "arm-linux-gnueabihf-gcc"

and I install the required linkers via:

Cargo.toml :

...
github-build-setup = "install-arm-linkers.yml"
...

install-arm-linkers.yml:

- name: Install armv7 and aarch64 Linkers
  if: runner.os == 'Linux'
  run: |
    sudo apt install gcc-aarch64-linux-gnu
    sudo apt install gcc-arm-linux-gnueabihf
andrewdavidmackenzie commented 2 weeks ago

I suspect this code:

/// Get the linkage for a single binary
fn try_determine_linkage(path: &Utf8PathBuf, target: &str) -> DistResult<Linkage> {
    let libraries = match target {
        // Can be run on any OS
        "i686-apple-darwin" | "x86_64-apple-darwin" | "aarch64-apple-darwin" => do_otool(path)?,
        "i686-unknown-linux-gnu"
        | "x86_64-unknown-linux-gnu"
        | "aarch64-unknown-linux-gnu"
        | "i686-unknown-linux-musl"
        | "x86_64-unknown-linux-musl"
        | "aarch64-unknown-linux-musl" => {
            // Currently can only be run on Linux
            if std::env::consts::OS != "linux" {
                return Err(DistError::LinkageCheckInvalidOS {
                    host: std::env::consts::OS.to_owned(),
                    target: target.to_owned(),
                });
            }
            do_ldd(path)?
        }
        // Can be run on any OS
        "i686-pc-windows-msvc"
        | "x86_64-pc-windows-msvc"
        | "aarch64-pc-windows-msvc"
        | "i686-pc-windows-gnu"
        | "x86_64-pc-windows-gnu"
        | "aarch64-pc-windows-gnu" => do_pe(path)?,
        _ => return Err(DistError::LinkageCheckUnsupportedBinary),   /// <-------- this is the error we see in output
    };
andrewdavidmackenzie commented 2 weeks ago

Would changing:

        | "x86_64-unknown-linux-gnu"
        | "aarch64-unknown-linux-gnu"

to

        | "x86_64-unknown-linux-gnu"
        | "armv7-unknown-linux-gnueabihf"
        | "aarch64-unknown-linux-gnu"

potentially fix it?

I would do a PR, but I'm not sure how to run it and test that fixes the issue.

andrewdavidmackenzie commented 2 weeks ago

@ashleygwilliams @mistydemeo any possibility of re-opening this issue and considering a fast/cirgical fix to add more targets as I mention above?

Gankra commented 2 weeks ago

@andrewdavidmackenzie it looks like you're running 0.22.0 and 0.22.1 actually added some fixes to make this kind of "I don't know how to linkage-check this" non-fatal, so it's possible just updating cargo-dist will solve it?

andrewdavidmackenzie commented 2 weeks ago

I noticed a .1 just now, so will try that later.

andrewdavidmackenzie commented 2 weeks ago

That worked! Finally, a clean release build. Thanks for your help.