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

Add wrappers for gdb and lldb when invoked from bazel #508

Closed
wants to merge 1 commit into from

Conversation

ohodson
Copy link
Contributor

@ohodson ohodson commented Apr 4, 2023

Usage:

$ bazel run --run_under $(realpath tools/bazel-gdb-wrapper.sh) //src/workerd/server:workerd --
serve $(realpath /samples/helloworld_esm/config.capnp)

@ohodson ohodson requested review from byule and penalosa April 4, 2023 12:48
Usage:

$ bazel run --run_under $(realpath tools/bazel-gdb-wrapper.sh) //src/workerd/server:workerd -- \
    serve $(realpath /samples/helloworld_esm/config.capnp)
@jp4a50
Copy link
Collaborator

jp4a50 commented Apr 27, 2023

Is this perhaps worth a mention in the README, perhaps under "Running workerd"?

@jp4a50
Copy link
Collaborator

jp4a50 commented Apr 27, 2023

Is this perhaps worth a mention in the README, perhaps under "Running workerd"?

While we're at it it might be worth encouraging users to set up a bazel --disk_cache to avoid these commands overwriting previous build outputs (and vice versa) due to the extra build options.

#
# Example command-line invocation:
#
# bazel run -c dbg --spawn_strategy=local --features=oso_prefix_is_pwd --run_under \
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment suggests to me that the features flag alone is sufficient and we should be able to at least do away with the local spawn_strategy?

Copy link
Contributor Author

@ohodson ohodson Apr 27, 2023

Choose a reason for hiding this comment

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

Interesting, I cannot tell you how much time I wasted with this OSX only issue, it does indeed appear redundant.

@ohodson
Copy link
Contributor Author

ohodson commented Apr 27, 2023

Is this perhaps worth a mention in the README, perhaps under "Running workerd"?

While we're at it it might be worth encouraging users to set up a bazel --disk_cache to avoid these commands overwriting previous build outputs (and vice versa) due to the extra build options.

Perhaps better in a separate CL and/or separate tips.md under docs/. My workflow uses --remote_cache without --disk_cache. Remote cache is painless and fast enough in practice (https://github.com/buchgr/bazel-remote). I recall Bazel's disk cache being in need of regular maintenance to keep the size in check (a cron-job or a helper-script).

@ohodson
Copy link
Contributor Author

ohodson commented Apr 27, 2023

I've decided to drop this. The changes are fairly trivial and actually don't work for debugging tests, bazel runs tests with a wrapper around your wrapper if you invoke (on an OS-X box):

 bazel run -c dbg --spawn_strategy=local --features=oso_prefix_is_pwd --run_under /Volumes/Source/workerd/tools/bazel-lldb-wrapper.sh //src/workerd/jsg:promise-test

Bazel runs:

 bash external/bazel_tools/tools/test/test-setup.sh /Volumes/Source/workerd/tools/bazel-lldb-wrapper.sh src/workerd/jsg/promise-test

And the bazel test-setup.sh redirects all stdio streams so you can't enter commands into the debugger.

For binaries, there is no equivalent wrapper on the bazel side so debugging binaries work. The main binary here is workerd and we have support for debugging that from vscode which is much more convenient than gdb / lldb at the command-line.

@ohodson ohodson closed this Apr 27, 2023
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