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

Run platform bootstrap first in bootstrap #31840

Merged
merged 1 commit into from Apr 29, 2024
Merged

Run platform bootstrap first in bootstrap #31840

merged 1 commit into from Apr 29, 2024

Conversation

sagudev
Copy link
Member

@sagudev sagudev commented Mar 23, 2024

So the user do not need to install llvm via choco manually. This should solve error: linker lld-link.exe not found error on windows.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because they just adjust the bootstrap scripts.

Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

This will cause a problem on unsupported Linux distributions, as it prevents the installation of crown and taplo.

@sagudev
Copy link
Member Author

sagudev commented Mar 23, 2024

This will cause a problem on unsupported Linux distributions, as it prevents the installation of crown and taplo.

We only need to do this on windows.

@sagudev sagudev requested a review from mrobinson March 23, 2024 15:23
@mrobinson
Copy link
Member

Hrm. Thinking out loud here, maybe the order shouldn't be so important when running code in ./mach bootstrap. What do you think about emitting a warning on Linux, but then a "Press enter to continue" prompt so that it isn't lost in the other output?

@sagudev
Copy link
Member Author

sagudev commented Mar 24, 2024

Hrm. Thinking out loud here, maybe the order shouldn't be so important when running code in ./mach bootstrap. What do you think about emitting a warning on Linux, but then a "Press enter to continue" prompt so that it isn't lost in the other output?

Warning for unsupported Linux distributions, right?

@mrobinson
Copy link
Member

Warning for unsupported Linux distributions, right?

Yep, exactly. Perhaps this should just warn and allow continuing, rather than simply exiting and skipping the installation of taplo and crown.

@sagudev sagudev force-pushed the sagudev-patch-1 branch 2 times, most recently from 178a2f6 to e8c0a61 Compare April 1, 2024 05:58
@jschwe
Copy link
Contributor

jschwe commented Apr 4, 2024

Yep, exactly. Perhaps this should just warn and allow continuing, rather than simply exiting and skipping the installation of taplo and crown.

It would be nice if that also happened on windows. For example, I was quite happy that the choco step currently happens late in the bootstrap. I don't have choco installed, but also don't need choco to install anything, since I only care about cross-compiling.

@mrobinson mrobinson added this pull request to the merge queue Apr 29, 2024
@mrobinson
Copy link
Member

@jschwe I've opened #32176 for the usecase you mention.

Merged via the queue into main with commit 6ca3bb4 Apr 29, 2024
10 checks passed
@mrobinson mrobinson deleted the sagudev-patch-1 branch April 29, 2024 14:39
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.

None yet

3 participants