Skip to content

wix: allow to skip more components #135255

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

Closed
wants to merge 1 commit into from
Closed

Conversation

klensy
Copy link
Contributor

@klensy klensy commented Jan 8, 2025

Initially this fixed try builds for dist-x86_64-msvc when i played with it #133033 (comment)

Looking at errors, it should probably fix issues with beta\stable bumps too: #133447 (comment) #135163 (comment)

This makes tools like clippy/rustfmt/(and miri, which is nightly only component)/etc optional for msi, so they can be turned on/off by dist.

@rustbot
Copy link
Collaborator

rustbot commented Jan 8, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 8, 2025
@klensy klensy mentioned this pull request Jan 8, 2025
built_tools: &HashSet<&'static str>,
) {
// envs for wix should be always defined, even if not used
// FIXME: is they affect ccache?
Copy link
Member

Choose a reason for hiding this comment

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

What is this referring to? I don't think we use (s?)ccache for Wix invocations?

It's also not clear to me what we mean by "should always be defined even if not used". Can you elaborate? What happens if we get that wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reference to sccache because this comment from pr with attempt to bump sccache, should be deleted.

As for "defined" - wix fails to build if it can't find all associated with components env vars. i.e. for rustfmt - which is optional, env var CFG_RUSTFMT should be always set (with 1 or 0 value), doesn't matter if it rustfmt component build or not. Hmm, need rephrase this.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2025
@LFS6502
Copy link
Contributor

LFS6502 commented Mar 1, 2025

@klensy
From wg-triage. Any updates on this PR?

@heiseish
Copy link
Contributor

I can take over the work to push it over finish line. Thanks @klensy for initial effort!

@Dylan-DPC
Copy link
Member

Closing this as it's superseded by #138606

@Dylan-DPC Dylan-DPC closed this Mar 19, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 24, 2025
…imulacrum

Fix missing rustfmt in msi installer - cont

## Context
- This PR extended and fixed rust-lang#131365, which was reverted in rust-lang#135253
- Initial effort from `@klensy` in rust-lang#135255 (at any points if you feel like picking this up again, let me know I'll close my PR! Just trying to push this through since it's my mistake in the original commits)
- Tested with both `beta` and `nightly` `rust.channel`

r? `@Mark-Simulacrum`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2025
Rollup merge of rust-lang#138606 - heiseish:131365-extended, r=Mark-Simulacrum

Fix missing rustfmt in msi installer - cont

## Context
- This PR extended and fixed rust-lang#131365, which was reverted in rust-lang#135253
- Initial effort from `@klensy` in rust-lang#135255 (at any points if you feel like picking this up again, let me know I'll close my PR! Just trying to push this through since it's my mistake in the original commits)
- Tested with both `beta` and `nightly` `rust.channel`

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants