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

Extract aya-build for building eBPF crates #1101

Merged
merged 2 commits into from
Dec 1, 2024
Merged

Conversation

tamird
Copy link
Member

@tamird tamird commented Nov 28, 2024

Please ignore all but the last commit.


This change is Reviewable

Copy link

netlify bot commented Nov 28, 2024

Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 60eaa7d
🔍 Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/674b3f85600a4c00080ddf98
😎 Deploy Preview https://deploy-preview-1101--aya-rs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mergify mergify bot added aya-bpf This is about aya-bpf (kernel) test A PR that improves test cases or CI labels Nov 28, 2024
@tamird tamird force-pushed the aya-build branch 4 times, most recently from 9d8b361 to b90ee6f Compare November 28, 2024 20:29
/// prevent their use for the time being.
///
/// [bindeps]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html?highlight=feature#artifact-dependencies
pub fn build_ebpf(packages: impl IntoIterator<Item = Package>) {
Copy link
Member

@vadorovsky vadorovsky Nov 29, 2024

Choose a reason for hiding this comment

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

I would prefer returning an error instead of panicking (at least in places which actually can panic if the build fails).

I know that 99% of users will unwrap in build.rs anyways (that's fine and expected), but still, I would stick to the principle that libraries should return an error. There always might be that 1% of people who want to create some more sophisticated xtask and handle the error somehow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do. In the meantime can you review #1098 which is a prerequisite of this?

Copy link

mergify bot commented Nov 30, 2024

@tamird, this pull request is now in conflict and requires a rebase.

@vadorovsky
Copy link
Member

needs a rebase and a bless

Copy link
Member

@vadorovsky vadorovsky left a comment

Choose a reason for hiding this comment

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

The code looks good, thanks for adding the errors!

We'll use this in the template and book to avoid duplicating all the
code.
@vadorovsky vadorovsky merged commit b01bc3f into aya-rs:main Dec 1, 2024
27 checks passed
@tamird tamird deleted the aya-build branch December 1, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aya-bpf This is about aya-bpf (kernel) test A PR that improves test cases or CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants