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

Add cross-compilation to CI #131

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Add cross-compilation to CI #131

merged 2 commits into from
Oct 18, 2024

Conversation

tamird
Copy link
Member

@tamird tamird commented Oct 17, 2024

Updates the README to use cargo target config instead of RUSTFLAGS to
avoid setting the linker for ebpf in cargo-in-cargo.

@tamird tamird force-pushed the cross-compile-mac-os branch 29 times, most recently from bae69c2 to bf179e9 Compare October 18, 2024 01:12
@tamird tamird force-pushed the cross-compile-mac-os branch 17 times, most recently from c939c72 to f4d8bdb Compare October 18, 2024 17:17
@tamird tamird force-pushed the cross-compile-mac-os branch 4 times, most recently from e85a8d7 to e4bf9b8 Compare October 18, 2024 17:44
Updates the README to use cargo target config instead of RUSTFLAGS to
avoid setting the linker for ebpf in cargo-in-cargo.
@tamird tamird force-pushed the cross-compile-mac-os branch from e4bf9b8 to b336e09 Compare October 18, 2024 17:50
@tamird tamird enabled auto-merge (rebase) October 18, 2024 17:54
Copy link
Member

@dave-tucker dave-tucker left a comment

Choose a reason for hiding this comment

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

LGTM. A couple of questions that could do answering, but nothing that should block this merging.

@@ -3,86 +3,111 @@
set -eux

TEMPLATE_DIR=$1
if [ -z "${TEMPLATE_DIR}" ]; then echo "template dir required"; exit 1; fi
if [ -z "${TEMPLATE_DIR}" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

What tool did you use to reformat the shell script?
Perhaps this is something we might consider adding to CI later to ensure changes are consistently formatted

Copy link
Member Author

Choose a reason for hiding this comment

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

I just used vscode and didn't investigate what it used under the hood.

@@ -205,8 +205,9 @@ async fn main() -> anyhow::Result<()> {
program.attach("{{tracepoint_name}}")?;
{%- endcase %}

let ctrl_c = signal::ctrl_c();
Copy link
Member

Choose a reason for hiding this comment

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

Why has this changed? I'm assuming it has something to do with the expect script and how we issue Ctrl+C in the tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure it matters, but I realized while debugging things that there was a subtle race here - we were logging "waiting for ..." before actually setting up the signal handler.

@tamird tamird merged commit 27e7867 into main Oct 18, 2024
22 checks passed
@tamird tamird deleted the cross-compile-mac-os branch October 18, 2024 21:33
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.

2 participants