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

xtask: Allow tasks & the kernel to specify default features. #1830

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

flihp
Copy link
Contributor

@flihp flihp commented Jul 16, 2024

tasks & kernels that want to disable default features can do so using the no-default-features boolean in the task / kernel config sections of the relevant app.toml files.

this resolves #1829

@flihp
Copy link
Contributor Author

flihp commented Jul 16, 2024

I'm sure there was a reason for disabling this so this may be ill advised but I want default features for the rng task in #1820. I've built a few images for the RoT and basic tire kicking was encouraging. The output from the build (task & kernel elfs) are identical except for the idle task. I haven't dug deep into why but if anyone is interested in this feature or thinks this is a bad idea I'd appreciate the feedback good or bad.

@flihp flihp force-pushed the default-features branch from e28609d to bb63797 Compare July 22, 2024 14:34
@flihp flihp mentioned this pull request Jul 23, 2024
@flihp flihp force-pushed the default-features branch 3 times, most recently from f18db42 to dc75180 Compare August 19, 2024 14:38
@flihp
Copy link
Contributor Author

flihp commented Aug 19, 2024

As a way to convince myself that enabling default-features doesn't have any unintended side effects the first commit in this PR removes all use of default-features from existing tasks. These were no-ops since we were passing --no-default-features to rustc anyway. The second commit here enables the use of default-features in tasks and because we removed all use of these in the previous commit the build output is identical both before and after dc75180 is applied. I have tested and confirmed that this is the case for every app.toml in the source tree.

@cbiffle
Copy link
Collaborator

cbiffle commented Aug 19, 2024

Historical context:

So, in the early days, we used outside-of-xtask builds in two situations:

  1. Doing single-task iteration (in the days before xtask build) using cargo check
  2. In editors using RLS, back when RLS was a thing

We used default features to fake a subset of the build system features to achieve this, which is part of what you're finding in the old default features lists.

Nowadays we've got the better rust-analyzer - build-system integration, and it's probably safe to stop doing this hack. So I'm generally positive about this change in concept, though I haven't finished reading the implementation yet.

xtask explicitly disables default features in our top level crates
(tasks & the kernel) by passing the `--no-default-features` flag to
`rustc`. Defining default features in these `Cargo.toml`s has no effect.
tasks & kernels that want to disable default features can do so using
the `no-default-features` boolean in the task / kernel config sections
of the relevant `app.toml` files.
@flihp flihp merged commit 92548d4 into oxidecomputer:master Sep 9, 2024
106 checks passed
@flihp flihp deleted the default-features branch September 9, 2024 20:59
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.

support default features at the task / kernel level
2 participants