- Notifications
You must be signed in to change notification settings - Fork14.1k
Bootstrap: Don't get output iflldb --version errors#127112
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
rustbot commentedJun 29, 2024
Kobzol commentedJun 29, 2024
I don't understand how this fixes the crash. It seems just to change the returned error? The original issue showed a crash on line |
ChrisDenton commentedJun 29, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sorry, I didn't explain that very well. The code is a bit spaghetti but the And |
ChrisDenton commentedJun 29, 2024
I've pushed a new commit that hopefully makes the code a bit clearer. |
Kobzol commentedJun 29, 2024
Aah, I understand now, thanks! Yeah, this command invocation code is very messy. I will nuke it soon (probably in the next couple of days) anyway with my bootstrap command refactoring PRs. But it doesn't hurt to fix the issue for now, it will help me understand the intent of the original code better. Thanks! @bors rollup r+ |
bors commentedJun 29, 2024
ChrisDenton commentedJun 29, 2024
That'll be great! |
…llaumeGomezRollup of 7 pull requestsSuccessful merges: -rust-lang#126805 (Migrate `pdb-alt-path`, `mismatching-target-triples` and `mingw-export-call-convention` `run-make` tests to rmake) -rust-lang#126995 (Migrate `pretty-print-with-dep-file`, `pretty-print-to-file` and `libtest-padding` `run-make` tests to rmake) -rust-lang#127041 (Migrate `run-make/override-aliased-flags` to `rmake.rs`) -rust-lang#127072 (docs: say "includes" instead of "does include") -rust-lang#127073 (Remove unnecessary SeqCst in `impl fmt::Pointer for AtomicPtr`) -rust-lang#127112 (Bootstrap: Don't get output if `lldb --version` errors) -rust-lang#127116 (Migrate `run-make/return-non-c-like-enum` to `rmake.rs`)Failed merges: -rust-lang#127050 (Make mtime of reproducible tarballs dependent on git commit)r? `@ghost``@rustbot` modify labels: rollup
Rollup merge ofrust-lang#127112 - ChrisDenton:lldb, r=KobzolBootstrap: Don't get output if `lldb --version` errorsfixesrust-lang#126892`Command` can error in two ways: the OS can fail to run the binary at all or else the binary can return an error exit code. Unfortunately the distinction between the two is not clear cut. The OS may succeed in starting the binary but it may still error before `main` (e.g. if a necessary library fails to load) and this will be reported via the exit code.Fortunately this case is simpler. We can assume that `lldb --version` will only ever error if there's a startup issue of some kind. so both kinds of errors are caused by the OS. Thus it's safe for us to treat them equally for the sake of this specific check.
fixes#126892
Commandcan error in two ways: the OS can fail to run the binary at all or else the binary can return an error exit code. Unfortunately the distinction between the two is not clear cut. The OS may succeed in starting the binary but it may still error beforemain(e.g. if a necessary library fails to load) and this will be reported via the exit code.Fortunately this case is simpler. We can assume that
lldb --versionwill only ever error if there's a startup issue of some kind. so both kinds of errors are caused by the OS. Thus it's safe for us to treat them equally for the sake of this specific check.