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

Compiletest panics on custom lldb version #84131

Closed
Manishearth opened this issue Apr 12, 2021 · 4 comments · Fixed by #84132
Closed

Compiletest panics on custom lldb version #84131

Manishearth opened this issue Apr 12, 2021 · 4 comments · Fixed by #84132

Comments

@Manishearth
Copy link
Member

My system has a custom version of lldb on it, and the lldb version string does not match the expected format. I'm not sure if I can share the exact version string but the first line is somehting like lldb version foo5-bar (revision somehash)

Check compiletest suite=ui mode=ui (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: Empty }', src/tools/compiletest/src/main.rs:978:56
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I feel like I should be able to run the uitests without needing the precise lldb version. I tried to override this by passing --lldb-version to ./x.py but it doesn't seem to accept that parameter.

@Mark-Simulacrum
Copy link
Member

Hm, I think we recently landed a PR relaxing some of this parsing a bit. But it may be that we should relax it more; is the version you expect to pass extractable from the --version results?

I don't think x.py should accept it directly as an option, but perhaps we can add it to config.toml.

@Manishearth
Copy link
Member Author

@Mark-Simulacrum No, there is no version number, just a hash. I have a patch that fixes this acceptably for my use case (makes the function return None), unsure if that's the best behavior here.

@Mark-Simulacrum
Copy link
Member

Hm, making it return none presumably means we just skip the lldb tests? It might be better to move the validation later, such that --exclude src/test/debuginfo-lldb or something like it works to skip this check as well.

We don't really have a principled story about ignoring vs. erroring, but I'm trying to be more cautious over time about silent acceptance.

@Manishearth
Copy link
Member Author

Hm, making it return none presumably means we just skip the lldb tests? It might be better to move the validation later, such that --exclude src/test/debuginfo-lldb or something like it works to skip this check as well.

That would also work for me.

That function does return None in a couple other cases already though.

@bors bors closed this as completed in 78e0f2f Apr 27, 2021
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 a pull request may close this issue.

2 participants