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.

Sign up for GitHub

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

Jump to bottom

CI: Test LLDB pretty-printers on Windows #7399

Merged
merged 2 commits into from
Oct 6, 2021
Merged

CI: Test LLDB pretty-printers on Windows #7399

merged 2 commits into from
Oct 6, 2021

Conversation

artemmukhin
Copy link
Member

@artemmukhin artemmukhin commented Jun 24, 2021 •
edited
Loading

This PR enables testing of LLDB pretty-printers on Windows (with bundled MSVC LLDB) on CI.

Besides CI, runPrettyPrintersTests task can be executed on Windows locally to check the pretty-printers. But keep in mind that mklink command used in runPrettyPrintersTests task may require Administrator privileges on Windows 10 (in contrast to Windows Server used by GitHub Actions). To deal with this, consider enabling Developer Mode.

changelog: Test LLDB pretty-printers on Windows

@artemmukhin artemmukhin added the internal Pull requests about internal improvements/fixes that don't affect users directly label Jun 24, 2021
@artemmukhin artemmukhin mentioned this pull request Jun 24, 2021
Merged
@artemmukhin artemmukhin force-pushed the ortem/test-lldb-pretty-printers-windows branch from b3a364e to e5592c5 Compare June 25, 2021 11:23
Undin
Undin reviewed Jul 5, 2021
View reviewed changes
build.gradle.kts Outdated Show resolved Hide resolved
pretty_printers_tests/Settings_linux.toml Outdated Show resolved Hide resolved
build.gradle.kts Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
pretty_printers_tests/tests/hash_map.rs
Comment on lines -8 to +10
// lldbg-check:[...]xs = size=4 { [0] = { 0 = 1 1 = 10 } [1] = { 0 = 2 1 = 20 } [2] = { 0 = 3 1 = 30 } [3] = { 0 = 4 1 = 40 } }
// lldbr-check:[...]xs = size=4 { [0] = { 0 = 1 1 = 10 } [1] = { 0 = 2 1 = 20 } [2] = { 0 = 3 1 = 30 } [3] = { 0 = 4 1 = 40 } }
// lldb-command:print ys
// lldbg-check:[...]ys = size=4 { [0] = 1 [1] = 2 [2] = 3 [3] = 4 }
// lldbr-check:[...]ys = size=4 { [0] = 1 [1] = 2 [2] = 3 [3] = 4 }
Copy link
Member

Choose a reason for hiding this comment

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

Do I understand correctly that these changes skip hash_map test for Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and now also for macOS (since we use LLDB from Xcode distribution)

@artemmukhin artemmukhin mentioned this pull request Jul 30, 2021
Open
19 tasks
@Undin
Copy link
Member

Undin commented Aug 29, 2021

@ortem ping

@artemmukhin artemmukhin force-pushed the ortem/test-lldb-pretty-printers-windows branch 7 times, most recently from 5e00cf7 to a327f46 Compare September 2, 2021 12:32
@artemmukhin
Copy link
Member Author

@Undin I realized that native_rust flag is actually redundant for GDB. Any GDB since version 7.12 provides native Rust support (see rust-lang/rust@dce4600#diff-07ab4bd8a21ba170bf63118c2d786444dd5b845b8da832a2e74f7ec86189ec24R486), and CLion is not even compatible with versions older than 7.8.

Previously, native_rust = false was always used, along with gdbg-check and lldbg-check commands. It was very confusing: actually, we use CLion's LLDB with native Rust support on Linux, but vanilla LLDB on macOS. So I organized the tests in a better way:

  • With GDB, only gdb-check commands are used (so native_rust is always true implicitly)
  • With LLDB, as following:
    • On Linux, native_rust = true and lldbr-check commands are used
    • On macOS, native_rust = false and lldbg-check commands are used (because we are still unable to use lldb Python module from CLion distribution, in progress)
    • On Windows, native_rust = false and lldbg-check commands are used (because MSVC LLDB does not natively support Rust, in progress)

Undin
Undin approved these changes Sep 30, 2021
View reviewed changes
Copy link
Member

@Undin Undin left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot added a commit that referenced this pull request Sep 30, 2021
@bors @artemmukhin
Merge #7399
1c33204 
7399: CI: Test LLDB pretty-printers on Windows r=Undin a=ortem

This PR enables testing of LLDB pretty-printers on Windows (with bundled MSVC LLDB) on CI.

Besides CI, `runPrettyPrintersTests` task can be executed on Windows locally to check the pretty-printers. But keep in mind that `mklink` command used in `runPrettyPrintersTests` task may require Administrator privileges on Windows 10 (in contrast to Windows Server used by GitHub Actions). To deal with this, consider enabling [Developer Mode](https://docs.microsoft.com/en-us/windows/apps/get-started/enable-your-device-for-development).

changelog: Test LLDB pretty-printers on Windows


Co-authored-by: Artem Mukhin <ortem00@gmail.com>
@bors
Copy link
Contributor

bors bot commented Sep 30, 2021

Build failed:

@Undin
Copy link
Member

Undin commented Oct 6, 2021

@ortem Could you check why pretty printer tests fail, please?

@artemmukhin
T: Split debugger settings for pretty-printers tests
cb80522 
* Each `Settings_%os%.toml` file can now contain separate sections to configure tests for GDB and LLDB
* `native_rust` flag is no longer used for GDB because any GDB since version 7.12 provides native Rust support
@artemmukhin artemmukhin force-pushed the ortem/test-lldb-pretty-printers-windows branch from a327f46 to 039c5e0 Compare October 6, 2021 11:45
@artemmukhin
CI: Test LLDB pretty-printers on Windows
369d158 
Some checks are temporary disabled for MSVC LLDB.
Pretty-printers for pointer types of string slices does not work since Rust 1.55 because of the changes in debug info generation introduced in rust-lang/rust#85269. Since 1.55, rustc generates `ptr_const$<...>` and `ptr_mut$<...>` type names instead of `const str *` and `mut str *` when targeting MSVC. So pretty-printer should be updated and the corresponding `lldbg-check`s should be added
@artemmukhin artemmukhin force-pushed the ortem/test-lldb-pretty-printers-windows branch from 039c5e0 to 369d158 Compare October 6, 2021 12:26
@artemmukhin artemmukhin mentioned this pull request Oct 6, 2021
Closed
@artemmukhin
Copy link
Member Author

@Undin Yes, they fail because of the changes in debug info generation introduced in rust-lang/rust#85269. I'd just disable these checks for now and merge the PR. Pretty-printers for MSVC can be updated later in another PR. I've opened an issue for that: #7935

Undin
Undin approved these changes Oct 6, 2021
View reviewed changes
Copy link
Member

@Undin Undin left a comment

Choose a reason for hiding this comment

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

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 6, 2021

Build succeeded:

@bors bors bot merged commit 9220950 into master Oct 6, 2021
@bors bors bot deleted the ortem/test-lldb-pretty-printers-windows branch October 6, 2021 18:41
@github-actions github-actions bot added this to the v158 milestone Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Undin Undin Undin approved these changes

Assignees

@Undin Undin

Labels
internal Pull requests about internal improvements/fixes that don't affect users directly
Projects
None yet
Milestone
v158
Development

Successfully merging this pull request may close these issues.
2 participants
@artemmukhin @Undin