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

feat(dap): support dynamically compiled executable #64

Merged

Conversation

bas-ie
Copy link
Contributor

@bas-ie bas-ie commented Nov 18, 2023

This:

  • configures dyanmic library paths by default (with the ability to disable)
  • loads Rust type information in lldb-vscode by default (with the ability to disable)
  • looks for and uses lldb-dap if present (on newer installations, this will replace lldb-vscode) ... which now that I've rebased, I see you already did!
  • corrects a bug with source map configuration (I think this was not being set in the final configuration?)
  • removes newlines that were interfering with rustc_sysroot path (because they came from reading stdout)
  • fixes a pattern matching issue that seemed to be preventing the commit hash being detected correctly

Sorry for cramming so many little bits and pieces in at once, this is basically just a "fix some of the things that were blockers for me with lldb-vscode". My main use-case was to get Bevy engine debugging working, which this does.

I'm submitting it as a WIP so you can have a quick scan and tell me if you agree with the approach, as I don't write a great deal of Lua and there are probably some wildly non-idiomatic things you can point out! I've tried to follow the overall approach of what was there before though.

I'll look at adding some tests and tidying up tomorrow.

@bas-ie bas-ie force-pushed the feat/support-dynamic-libs branch from 9edccf1 to 43f32b0 Compare November 18, 2023 06:45
lua/rustaceanvim/dap.lua Outdated Show resolved Hide resolved
@bas-ie bas-ie force-pushed the feat/support-dynamic-libs branch 2 times, most recently from d7d05db to a58b7c7 Compare November 18, 2023 07:55
@mrcjkb
Copy link
Owner

mrcjkb commented Nov 18, 2023

Thanks. This looks great!
I'll give it a closer look and test drive it on NixOS over the weekend.

@bas-ie
Copy link
Contributor Author

bas-ie commented Nov 18, 2023

I noticed an issue with sourcemaps crop up too, I haven't got that quite right yet:

Running initCommands:
(lldb) command script import "/home/basie/.local/share/rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/etc/lldb_lookup.py"
(lldb) command script import "/home/basie/.local/share/rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/etc/lldb_lookup.py"
(lldb) type synthetic add -l lldb_lookup.synthetic_lookup -x ".*" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)String$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^&(mut )?str$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^&(mut )?\\[.+\\]$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::ffi::([a-z_]+::)+)OsString$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Vec<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)VecDeque<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)BTreeSet<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)BTreeMap<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::collections::([a-z_]+::)+)HashMap<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::collections::([a-z_]+::)+)HashSet<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Rc<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Arc<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)Cell<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)Ref<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)RefMut<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)RefCell<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^core::num::([a-z_]+::)*NonZero.+$" --category Rust
(lldb) type category enable Rust
(lldb) type synthetic add -l lldb_lookup.synthetic_lookup -x ".*" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)String$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^&(mut )?str$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^&(mut )?\\[.+\\]$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::ffi::([a-z_]+::)+)OsString$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Vec<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)VecDeque<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)BTreeSet<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)BTreeMap<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::collections::([a-z_]+::)+)HashMap<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::collections::([a-z_]+::)+)HashSet<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Rc<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Arc<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)Cell<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)Ref<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)RefMut<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)RefCell<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^core::num::([a-z_]+::)*NonZero.+$" --category Rust
(lldb) type category enable Rust
source must be be an array of two-element arrays, each containing a source and replacement path string.
Hello, world!

The initCommands look fine, but I'm formatting sourcemaps wrong somehow.

I wonder if I can find an easy-ish way of running the nix tests. Would they run in a VM?

@bas-ie
Copy link
Contributor Author

bas-ie commented Nov 18, 2023

Oh, I see... I can

sudo nix flake check -L \
  --extra-experimental-features nix-command \
  --extra-experimental-features flakes

and get at least the results of what would happen in GHA.

lua/rustaceanvim/config/internal.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/config/internal.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/config/init.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/config/internal.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/config/internal.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/dap.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/dap.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/dap.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/dap.lua Outdated Show resolved Hide resolved
lua/rustaceanvim/shell.lua Show resolved Hide resolved
@mrcjkb
Copy link
Owner

mrcjkb commented Nov 18, 2023

I noticed an issue with sourcemaps crop up too, I haven't got that quite right yet:

Running initCommands:
(lldb) command script import "/home/basie/.local/share/rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/etc/lldb_lookup.py"
(lldb) command script import "/home/basie/.local/share/rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/etc/lldb_lookup.py"
(lldb) type synthetic add -l lldb_lookup.synthetic_lookup -x ".*" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)String$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^&(mut )?str$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^&(mut )?\\[.+\\]$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::ffi::([a-z_]+::)+)OsString$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Vec<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)VecDeque<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)BTreeSet<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)BTreeMap<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::collections::([a-z_]+::)+)HashMap<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::collections::([a-z_]+::)+)HashSet<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Rc<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Arc<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)Cell<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)Ref<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)RefMut<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)RefCell<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^core::num::([a-z_]+::)*NonZero.+$" --category Rust
(lldb) type category enable Rust
(lldb) type synthetic add -l lldb_lookup.synthetic_lookup -x ".*" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)String$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^&(mut )?str$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^&(mut )?\\[.+\\]$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::ffi::([a-z_]+::)+)OsString$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Vec<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)VecDeque<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)BTreeSet<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)BTreeMap<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::collections::([a-z_]+::)+)HashMap<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(std::collections::([a-z_]+::)+)HashSet<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Rc<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(alloc::([a-z_]+::)+)Arc<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)Cell<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)Ref<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)RefMut<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^(core::([a-z_]+::)+)RefCell<.+>$" --category Rust
(lldb) type summary add -F lldb_lookup.summary_lookup  -e -x -h "^core::num::([a-z_]+::)*NonZero.+$" --category Rust
(lldb) type category enable Rust
source must be be an array of two-element arrays, each containing a source and replacement path string.
Hello, world!

The initCommands look fine, but I'm formatting sourcemaps wrong somehow.

I wonder if I can find an easy-ish way of running the nix tests. Would they run in a VM?

🤔 This bit:

source must be be an array of two-element arrays, each containing a source and replacement path string.

... makes me think I may have got the structure of sourceMap wrong. I guess it's supposed to be a list of tuples, not a map. I'll look into this in a separate PR.

@mrcjkb
Copy link
Owner

mrcjkb commented Nov 19, 2023

I've just pushed a fix for the sourceMap (see #68).
It seems to work for me. Sorry for causing a merge conflict 😦

@mrcjkb
Copy link
Owner

mrcjkb commented Nov 19, 2023

Oh, I see... I can

sudo nix flake check -L \
  --extra-experimental-features nix-command \
  --extra-experimental-features flakes

and get at least the results of what would happen in GHA.

You can also add those flags to your nix.conf 😄

@bas-ie
Copy link
Contributor Author

bas-ie commented Nov 19, 2023

Thanks for the tips, will revise in next day or so.

- adds add_dynamic_library_paths
- adds load_rust_types
@bas-ie bas-ie force-pushed the feat/support-dynamic-libs branch 2 times, most recently from 1c93809 to 3107caf Compare November 25, 2023 22:44
@bas-ie bas-ie marked this pull request as ready for review November 25, 2023 22:48
@bas-ie
Copy link
Contributor Author

bas-ie commented Nov 25, 2023

@mrcjkb Hello! Finally managed to circle back to this. I think I've addressed the outstanding issues. Wonder if it's worth getting it merged to save on conflicts, and I'll address testing in a separate PR? But up to you obviously.

@bas-ie
Copy link
Contributor Author

bas-ie commented Nov 25, 2023

Oh, and could you enable checks on this PR please? Thx!

@bas-ie
Copy link
Contributor Author

bas-ie commented Nov 25, 2023

Actually, looking at the current tests I might need a tiny bit of guidance on what to test so I don't end up wasting your time 🙂

@mrcjkb mrcjkb enabled auto-merge (squash) November 26, 2023 00:10
@bas-ie bas-ie requested a review from mrcjkb November 26, 2023 01:22
@mrcjkb
Copy link
Owner

mrcjkb commented Nov 26, 2023

Awesome 🚀

I don't have time today, but I should be able to look at this tomorrow.
Regarding tests: I still have to figure out how to get integration tests that depend on external tools, like rust-analyzer, working.
So for now, I think the manual tests are fine. I will think about integration tests myself when I have some more capacity for it.

@mrcjkb mrcjkb disabled auto-merge November 27, 2023 07:45
Copy link
Owner

@mrcjkb mrcjkb left a comment

Choose a reason for hiding this comment

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

I just tested this manually with both lldb and codelldb.

  • With lldb, everything seems to work fine.
  • With codelldb, I get: Error on launch: Could not parse launch configuration: invalid type: sequence, expected a map

Note: I have force-pushed a rebase to your branch. As the fixes are relatively small, I will see if I have time to fix them myself 😄

lua/rustaceanvim/dap.lua Outdated Show resolved Hide resolved
'DKLD_LIBRARY_PATH=' .. rustc_target_path .. sep .. target_path .. sep .. '$DYLD_LIBRARY_PATH'
)
else
table.insert(
Copy link
Owner

Choose a reason for hiding this comment

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

issue: As with init_commands previously, environment will grow indefinitely. We can prevent this in the same way, by using a table with the workspace root as the key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shoot, yep ty will fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh nvm, I see you're on it :) I'll keep looking at the launch.json thing.

@mrcjkb mrcjkb changed the base branch from master to support-dynamic-libs-fixes November 27, 2023 09:11
@mrcjkb
Copy link
Owner

mrcjkb commented Nov 27, 2023

Merging into another branch where I can implement the fixes for codelldb and environment.

Thanks for the great contribution @richchurcher!

@mrcjkb mrcjkb merged commit 9f27450 into mrcjkb:support-dynamic-libs-fixes Nov 27, 2023
4 of 5 checks passed
@mrcjkb
Copy link
Owner

mrcjkb commented Nov 27, 2023

Continuing in #72

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.

2 participants