Skip to content

use cargo rustc for macro expansion so -Zbuild-std can be supported#5214

Open
skrap wants to merge 4 commits into
rust-lang:mainfrom
skrap:bugfix/allow-zbuild-std-macro-expansion
Open

use cargo rustc for macro expansion so -Zbuild-std can be supported#5214
skrap wants to merge 4 commits into
rust-lang:mainfrom
skrap:bugfix/allow-zbuild-std-macro-expansion

Conversation

@skrap

@skrap skrap commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Description

Address #5204 by invoking macro expansion through cargo rustc instead of rustc directly. This allows the use of the -Zbuild-std flag, enabling targets without a prebuilt std (e.g. tier 3 targets) to build the libc tests.

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot rustbot added ctest Issues relating to the ctest crate S-waiting-on-review labels Jun 24, 2026
@skrap skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch 4 times, most recently from 0689c20 to b5da051 Compare June 24, 2026 18:34
Comment thread ctest/src/macro_expansion.rs Outdated
@tgross35

Copy link
Copy Markdown
Contributor

Cc @mbyx for ctest

@tgross35 tgross35 added the stable-unneeded This PR is applied to main but already exists on libc-0.2 label Jun 24, 2026
@skrap skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from b5da051 to 1100563 Compare June 24, 2026 19:18
Comment thread ctest/src/macro_expansion.rs
Comment thread ctest/src/macro_expansion.rs Outdated
Comment thread ctest/src/macro_expansion.rs Outdated
Comment thread ctest/src/macro_expansion.rs Outdated
Comment thread ctest/src/macro_expansion.rs Outdated
Comment thread ctest/src/macro_expansion.rs
@rustbot

rustbot commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@skrap skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from bef315b to af836b6 Compare July 1, 2026 03:16
@rustbot

This comment has been minimized.

@skrap

skrap commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot review

I think I have addressed the feedback, though in exploring the changes I sometimes made some different choices. I've left comments in the conversations where appropriate.

Comment thread ctest/src/generator.rs Outdated
Comment thread ctest/src/generator.rs Outdated
Comment thread ctest/src/generator.rs
Comment thread ctest/src/generator.rs Outdated
Comment thread ctest/src/macro_expansion.rs Outdated
Comment thread ctest/src/macro_expansion.rs
Comment thread ctest/src/macro_expansion.rs Outdated
Comment thread ctest/Cargo.toml Outdated
Comment thread ctest/src/macro_expansion.rs Outdated
@skrap skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from 3294988 to 9dd5dd1 Compare July 1, 2026 13:12
@rustbot

This comment has been minimized.

@skrap skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch 2 times, most recently from ba0cdaa to f752fb9 Compare July 1, 2026 15:46
@rustbot

This comment has been minimized.

@skrap

skrap commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot review

Further review feedback. I'm a bit shy about adding more API than necessary for the stated purpose of the PR. If we have a clear use case for the API proposed, I think that should be proposed in a separate PR.

Comment thread ctest/src/generator.rs
Comment thread ctest/src/generator.rs Outdated
Comment thread ctest/src/macro_expansion.rs Outdated

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just updating the status here.

Also, regarding the general concern about API surface area: don't worry about it :) as long as it's somewhat sane, for ctest I tend to err on the side of adding small things we might need while related code is already being modified, rather than leaving a gap that might need ctest-specific changes in the future. There might be somebody who appreciates it even if libc doesn't have an immediate use.

View changes since this review

@skrap skrap force-pushed the bugfix/allow-zbuild-std-macro-expansion branch from f752fb9 to c9e1f8a Compare July 6, 2026 17:32
@rustbot

rustbot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@skrap

skrap commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot review

OK, I have gone with your suggestions on the remaining items. Please have a look. I will squash the commits into one before merging.

@tgross35 tgross35 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Last pass, noticed a few small things then happy to merge

View changes since this review

Comment on lines +58 to +61
// In order to avoid warnings causing failures here when the
// environment's RUSTFLAGS contains -Dwarnings (or similar),
// we will clear RUSTFLAGS.
cmd.env_remove("RUSTFLAGS");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Change to .env_clear() so Cargo doesn't pick up more flags that are meant for the host.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am guessing this will fail because it'll do away with things like PATH, which might be necessary for cargo to call the correct helpers, but I will do as you ask in the interest of moving this forward.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair point, feel free to leave it for now if you haven't already tested and I'll poke around some more afterwards

Comment thread ctest/src/generator.rs
}

/// Configures extra arguments to be passed to cargo during macro expansion.
/// This can be used, for example, to pass `-Zbuild-std`` if required.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// This can be used, for example, to pass `-Zbuild-std`` if required.
/// This can be used, for example, to pass `-Zbuild-std` if required.

Mismatched backtick

Comment thread ctest/src/generator.rs

/// Configures extra arguments to be passed to cargo during macro expansion.
/// This can be used, for example, to pass `-Zbuild-std`` if required.
pub fn macro_expansion_cargo_args_mut(&mut self) -> &mut Vec<String> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub fn macro_expansion_cargo_args_mut(&mut self) -> &mut Vec<String> {
pub fn expansion_cargo_args_mut(&mut self) -> &mut Vec<String> {

Simpler name, there's only one expansion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ctest Issues relating to the ctest crate S-waiting-on-author stable-unneeded This PR is applied to main but already exists on libc-0.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants