Skip to content

Install patched LLDB on vscode extension activation#1637

Merged
wenyongh merged 5 commits intobytecodealliance:mainfrom
cimacmillan:cmmacmil/vscode
Dec 1, 2022
Merged

Install patched LLDB on vscode extension activation#1637
wenyongh merged 5 commits intobytecodealliance:mainfrom
cimacmillan:cmmacmil/vscode

Conversation

@cimacmillan
Copy link
Copy Markdown
Contributor

@cimacmillan cimacmillan commented Oct 25, 2022

Summary

Download and install the WAMR patched LLDB binary on vscode extension activation

This allows the user to download the packaged .vsix file, where the activation script should handle determining what LLDB binary they should use, and install it in the correct location.

@loganek
Copy link
Copy Markdown
Contributor

loganek commented Oct 25, 2022

I wonder if we actually need the non-bundled version at all? Are there any benefits of having that?
@wenyongh @lum1n0us

@NingW101
Copy link
Copy Markdown
Contributor

@loganek @cimacmillan Hey guys, followings are current status and some enhancement need to be developed for wamr-ide with the release.

Necessary Resource

  • Docker images: wasm-toolchain:release-version & wasm-debug-server:release-version
  • LLDB under VSCode-Extension/resource/debug (for source debugging)

Current Status:

  • docker images are built with release-version-number tag, can be directly pulled with docker command.
  • The extension version of wamr-ide has set as the release-version-number(stored in package.json config file)

Enhancement to be developed:

  • wamr-ide will auto check and pull the docker images if the images are not ready in activation stage.
    • wamr-ide reads the release-version-number firstly,
    • execute the docker pull command with the release-version-number tag
  • wamr-ide will auto install the lldb and construct the resource\debug folder
  • The docker-image:tag of commands to create docker containers is hard code right now, please check here , it should be changed with the release-version-number
    • read release-version-number firstly
    • invoke the script with the release-version-number argument.

so back to this PR, LLDB will be auto installed in vscode-extension activation stage according to the above.

@cimacmillan
Copy link
Copy Markdown
Contributor Author

  • wamr-ide will auto install the lldb and construct the resource\debug folder

@NingW101 This approach makes sense, and is a bit more flexible. In the linux case, it would allow the user to specify which of the ubuntu LLDB builds (or determine via the OS currently running).

Is it something that's already being worked on? If not, I can revise this PR.

@NingW101
Copy link
Copy Markdown
Contributor

Is it something that's already being worked on?

The modifications are mainly in vscode-extension part, and the development has not started yet.

If not, I can revise this PR.

That would be nice, thank you very much! I can help to file an issue and draft the basic logic so that we can have more discussion there

@cimacmillan cimacmillan changed the title Release vscode extension with bundled LLDB Install LLDB on vscode extension activation Nov 2, 2022
@cimacmillan cimacmillan changed the title Install LLDB on vscode extension activation Install patched LLDB on vscode extension activation Nov 2, 2022
@cimacmillan
Copy link
Copy Markdown
Contributor Author

wamr-ide will auto install the lldb and construct the resource\debug folder

@NingW101 Sorry for the delay, I've revised the PR to address this aspect.

Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/lldbUtilities.ts
Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/directoryUtilities.ts Outdated
Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/directoryUtilities.ts Outdated
Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/directoryUtilities.ts Outdated
Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/lldbUtilities.ts Outdated
Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/lldbUtilities.ts Outdated
Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/lldbUtilities.ts Outdated
Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/lldbUtilities.ts Outdated
Copy link
Copy Markdown
Contributor

@NingW101 NingW101 left a comment

Choose a reason for hiding this comment

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

One more thing after my debugging for this patch, I think maybe we should add enhancement for starting debugging process. Currently we only check the lldb resource and give prompt during the activation stage, but what if the lldb is not downloaded or installed normally or users just skip it? We should check lldb and prompt it again if necessary after users clicking Debug button, otherwise, debugging process wont work well. How do you think?

Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/lldbUtilities.ts Outdated
Comment thread test-tools/wamr-ide/VSCode-Extension/src/utilities/lldbUtilities.ts Outdated
@cimacmillan
Copy link
Copy Markdown
Contributor Author

@NingW101 This is a good suggestion, thanks. I'll revise the PR.

One more thing after my debugging for this patch, I think maybe we should add enhancement for starting debugging process. Currently we only check the lldb resource and give prompt during the activation stage, but what if the lldb is not downloaded or installed normally or users just skip it? We should check lldb and prompt it again if necessary after users clicking Debug button, otherwise, debugging process wont work well. How do you think?

@cimacmillan cimacmillan requested a review from NingW101 November 11, 2022 10:02
@wenyongh wenyongh changed the base branch from dev/binary_release to main November 14, 2022 08:22
@wenyongh
Copy link
Copy Markdown
Collaborator

@cimacmillan I changed the destination branch to main since the changes of dev/binary_release has been merged into main and dev/binary_release is to be removed. And since this PR supposes that lldb binary has been compiled and released, and gets lldb binary from the released link, so my understanding is that it doesn't work currently, and we should not merge this PR until we are to release a new version with lldb binary released, right?

And there are several conflicts after changing the destination branch, could you help resolve them? Thanks.

@wenyongh wenyongh changed the base branch from main to dev/binary_release November 14, 2022 08:34
Copy link
Copy Markdown
Contributor

@NingW101 NingW101 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@loganek loganek left a comment

Choose a reason for hiding this comment

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

Thanks for making the change!

@wenyongh wenyongh changed the base branch from dev/binary_release to main November 14, 2022 09:24
@cimacmillan
Copy link
Copy Markdown
Contributor Author

@cimacmillan I changed the destination branch to main since the changes of dev/binary_release has been merged into main and dev/binary_release is to be removed. And since this PR supposes that lldb binary has been compiled and released, and gets lldb binary from the released link, so my understanding is that it doesn't work currently, and we should not merge this PR until we are to release a new version with lldb binary released, right?

And there are several conflicts after changing the destination branch, could you help resolve them? Thanks.

@wenyongh Thanks, I've rebased my changes on top of main. The binary release job will build patched LLDB versions and this vscode extension in parallel. If a user installs the extension from the wamr-ide artefact in the release, there will be compiled LLDB available and it should setup correctly. I think we should merge this before the next release so that latest wamr-ide artefact includes these changes.

There isn't an official WAMR release which contains patched LLDB yet, so it's only a consideration for internal testing. Personally, I have been running the binary release job from a personal fork, which has been a good method.

@wenyongh
Copy link
Copy Markdown
Collaborator

@cimacmillan I changed the destination branch to main since the changes of dev/binary_release has been merged into main and dev/binary_release is to be removed. And since this PR supposes that lldb binary has been compiled and released, and gets lldb binary from the released link, so my understanding is that it doesn't work currently, and we should not merge this PR until we are to release a new version with lldb binary released, right?
And there are several conflicts after changing the destination branch, could you help resolve them? Thanks.

@wenyongh Thanks, I've rebased my changes on top of main. The binary release job will build patched LLDB versions and this vscode extension in parallel. If a user installs the extension from the wamr-ide artefact in the release, there will be compiled LLDB available and it should setup correctly. I think we should merge this before the next release so that latest wamr-ide artefact includes these changes.

There isn't an official WAMR release which contains patched LLDB yet, so it's only a consideration for internal testing. Personally, I have been running the binary release job from a personal fork, which has been a good method.

@cimacmillan OK, let's merge the PR before we release a new version.

@loganek
Copy link
Copy Markdown
Contributor

loganek commented Nov 21, 2022

@wenyongh @cimacmillan are there any further changes required before we merge that? Looks like all the comments have been resolved and there are no conflicts with the main branch.

@wenyongh
Copy link
Copy Markdown
Collaborator

@wenyongh @cimacmillan are there any further changes required before we merge that? Looks like all the comments have been resolved and there are no conflicts with the main branch.

@loganek No more comments from me, and yes, all the comments have been resolved, but since the PR changes the behavior of wamr-ide, which requires to download lldb binary from WAMR binary release, and we haven't built and released the binaries yet, so I didn't merged it. I prepare to merge it until we want to release a new version with binaries built, so it can work after we release the version, not sure whether it is OK you? Thanks.

@loganek
Copy link
Copy Markdown
Contributor

loganek commented Nov 21, 2022

sounds good, thanks

@cimacmillan
Copy link
Copy Markdown
Contributor Author

@wenyongh When is the next release planned?

I think it would be safe to merge this. The missing wamr lldb binary release is only a consideration for internal testing. To avoid an error around the missing LLDB binary, you can include a pre-built LLDB in the debug folder of the VS code extension for testing. If LLDB is present in this folder, then it won't attempt to download anything.

@wenyongh
Copy link
Copy Markdown
Collaborator

wenyongh commented Dec 1, 2022

@wenyongh When is the next release planned?

I think it would be safe to merge this. The missing wamr lldb binary release is only a consideration for internal testing. To avoid an error around the missing LLDB binary, you can include a pre-built LLDB in the debug folder of the VS code extension for testing. If LLDB is present in this folder, then it won't attempt to download anything.

OK, let's merge it first. BTW, we may upload another PR to include the pre-built LLDB in the debug folder of the VS code extension, and do not merge it now. If developer encounters issue, we can let him refer to the PR.

And yes, we are planning to release the next version, is there any PR from you to merge recently?

@wenyongh wenyongh merged commit 6eaf779 into bytecodealliance:main Dec 1, 2022
NingW101 pushed a commit to NingW101/wasm-micro-runtime that referenced this pull request Dec 1, 2022
…#1637)

Download and install the WAMR patched LLDB binary on vscode extension activation.

This allows the user to download the packaged .vsix file, where the activation script
should handle determining what LLDB binary they should use, and install it in the
correct location.
wenyongh pushed a commit that referenced this pull request Dec 6, 2022
Download and install the WAMR patched LLDB binary on vscode extension activation.

This allows the user to download the packaged .vsix file, where the activation script
should handle determining what LLDB binary they should use, and install it in the
correct location.
wenyongh pushed a commit that referenced this pull request Dec 19, 2022
Download and install the WAMR patched LLDB binary on vscode extension activation.

This allows the user to download the packaged .vsix file, where the activation script
should handle determining what LLDB binary they should use, and install it in the
correct location.
wenyongh pushed a commit that referenced this pull request Dec 19, 2022
Download and install the WAMR patched LLDB binary on vscode extension activation.

This allows the user to download the packaged .vsix file, where the activation script
should handle determining what LLDB binary they should use, and install it in the
correct location.
vickiegpt pushed a commit to vickiegpt/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…#1637)

Download and install the WAMR patched LLDB binary on vscode extension activation.

This allows the user to download the packaged .vsix file, where the activation script
should handle determining what LLDB binary they should use, and install it in the
correct location.
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.

4 participants