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

build: update ci job to use cargo-cross #90

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

fujiapple852
Copy link
Contributor

@fujiapple852 fujiapple852 commented Oct 20, 2023

WIP PR to support cargo cross (for netbsd). See #87

cc @0323pin @Byron

@fujiapple852
Copy link
Contributor Author

Fails to link for netbsd with:

/usr/local/lib/gcc/x86_64-unknown-netbsd/9.4.0/../../../../x86_64-unknown-netbsd/bin/ld: cannot find -lexecinfo

I'm not familiar with libexecinfo but seems to be needed here.

@fujiapple852 fujiapple852 force-pushed the build-add-cargo-cross-ci branch from 762cc8c to 05ee139 Compare October 20, 2023 10:22
@fujiapple852
Copy link
Contributor Author

Seemed to be this: cross-rs/cross#1345

Pushed a fix

@fujiapple852 fujiapple852 force-pushed the build-add-cargo-cross-ci branch from 05ee139 to f885a91 Compare October 20, 2023 11:09
@Byron Byron marked this pull request as draft October 20, 2023 11:14
@fujiapple852 fujiapple852 force-pushed the build-add-cargo-cross-ci branch 2 times, most recently from 33d46f2 to c335921 Compare October 20, 2023 11:26
@fujiapple852
Copy link
Contributor Author

This works when I change from cargo test to cargo build so I think there is a genuine issue with the tests on netbsd (I see there are several platform specific tests, I haven't dug into the logic much).

I'd suggest trying to reproduce the test failure on a netbsd box and fixing up whatever is wrong. Failing that we can update the pr to have a cargo test for all platforms and a cargo build for netbsd.

Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for getting NetBSD testing started!

Maybe for now, like you suggested, you could parameterize the test portion in cargo test so it can be build with NetBSD and test with everything else.

That way we wouldn't loose any test-coverage, validate builds in NetBSD while allowing tests to be enabled by contribution (CC @0323pin maybe) later.

@fujiapple852
Copy link
Contributor Author

I ran the tests locally in my netbsd vm (9.3) and they pass. So the issue here seem to be some combination of cargo cross and and netbsd and cargo test.

@fujiapple852 fujiapple852 force-pushed the build-add-cargo-cross-ci branch from c335921 to da47b3f Compare October 20, 2023 12:19
@fujiapple852 fujiapple852 force-pushed the build-add-cargo-cross-ci branch from da47b3f to be43b09 Compare October 20, 2023 12:19
@fujiapple852 fujiapple852 marked this pull request as ready for review October 20, 2023 12:24
@fujiapple852 fujiapple852 requested a review from Byron October 20, 2023 12:25
@fujiapple852 fujiapple852 changed the title build: update ci job to use cargo-cross (WIP) build: update ci job to use cargo-cross Oct 20, 2023
Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your help, it's much appreciated!

@@ -51,3 +51,14 @@ windows = { version = "0.44.0", features = [
"Win32_UI_Shell_PropertiesSystem",
] }
scopeguard = "1.2.0"

# workaround for https://github.com/cross-rs/cross/issues/1345
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we can keep an eye on this issue to eventually remove the workaround.

@Byron Byron merged commit 695af32 into Byron:master Oct 20, 2023
4 checks passed
@0323pin
Copy link
Contributor

0323pin commented Oct 20, 2023

🚀 ❤️

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

Successfully merging this pull request may close these issues.

3 participants