Skip to content

Extend the build directive syntax with cargo::#12201

Merged
bors merged 11 commits intorust-lang:masterfrom
0xPoe:rustin-patch-build-script
Dec 24, 2023
Merged

Extend the build directive syntax with cargo::#12201
bors merged 11 commits intorust-lang:masterfrom
0xPoe:rustin-patch-build-script

Conversation

@0xPoe
Copy link
Member

@0xPoe 0xPoe commented May 30, 2023

What does this PR try to resolve?

close #11461

Extend the build directive syntax with cargo:: and update tests and docs.

How should we test and review this PR?

Check out the unit tests.

@rustbot
Copy link
Collaborator

rustbot commented May 30, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-build-scripts Area: build.rs scripts S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. A-documenting-cargo-itself Area: Cargo's documentation labels May 30, 2023
@0xPoe 0xPoe marked this pull request as draft May 30, 2023 00:43
Copy link
Member Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

A lot of code and docs need to update. I will update it later.

@0xPoe 0xPoe force-pushed the rustin-patch-build-script branch 5 times, most recently from 4ea02bb to 9ad93ad Compare May 31, 2023 01:29
@0xPoe 0xPoe changed the title Extend the build directive syntax with cargo:: WIP: Extend the build directive syntax with cargo:: May 31, 2023
@0xPoe 0xPoe force-pushed the rustin-patch-build-script branch 4 times, most recently from b8adc79 to 1d703a0 Compare June 2, 2023 01:41
Copy link
Member Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

  • update other error messages
  • update other docs
  • add more tests

@0xPoe 0xPoe force-pushed the rustin-patch-build-script branch 6 times, most recently from 83ae1a8 to eabea47 Compare June 4, 2023 03:53
@0xPoe 0xPoe force-pushed the rustin-patch-build-script branch 2 times, most recently from b8fd7b3 to 8f917ce Compare June 11, 2023 07:31
0xPoe added 11 commits December 23, 2023 13:17
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@epage
Copy link
Contributor

epage commented Dec 24, 2023

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 24, 2023

📌 Commit dec9e8c has been approved by epage

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 24, 2023

⌛ Testing commit dec9e8c with merge d45969a...

@bors
Copy link
Contributor

bors commented Dec 24, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing d45969a to master...

@0xPoe
Copy link
Member Author

0xPoe commented Dec 24, 2023

Thanks for your review! 💚 💙 💜 💛 ❤️

@weihanglo
Copy link
Member

weihanglo commented Dec 24, 2023

Just thought of target.<triple>.<links>. The TOML table accepts arbitrary keys as metadata keys. Should we also extend the format?

_ => {
let val = value.string(key)?.0;
output.metadata.push((key.clone(), val.to_string()));
}

@0xPoe
Copy link
Member Author

0xPoe commented Dec 25, 2023

Should we also extend the format?

Do you mean we should add a prefix to those metadata keys? For instance: metadata-KEY = VALUE.

It seems that the purpose of this is just to override rustc configurations. Therefore, I'm unsure if we need to support it here.

anyhow::bail!("`{}` is not supported in build script overrides", key);

@weihanglo
Copy link
Member

I mean, it is definitely a breaking change if we want to extend more keys to override from build scripts. Actually #10274 may break stuff if people already used key like rustc-link-arg-tests as a metadata key (which is rare).

Huh… maybe it's too hard and not worthy of a transition.

@epage
Copy link
Contributor

epage commented Dec 27, 2023

I created #13211 to track this

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

Labels

A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-documenting-cargo-itself Area: Cargo's documentation A-rebuild-detection Area: rebuild detection and fingerprinting disposition-merge FCP with intent to merge finished-final-comment-period FCP complete relnotes Release-note worthy S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot extend build directives within cargo

7 participants