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: add chain debug command #3221

Merged
merged 32 commits into from
Dec 9, 2022
Merged

feat: add chain debug command #3221

merged 32 commits into from
Dec 9, 2022

Conversation

jeronimoalbi
Copy link
Member

Resolves #2992

@jeronimoalbi jeronimoalbi added the type:feat To implement new feature. label Dec 2, 2022
@jeronimoalbi jeronimoalbi self-assigned this Dec 2, 2022
To keep the command simple it only starts the debug shell or server
favouring the build and init commands to init/reset blockchain app data
or to build the debug binary.

Both build and init commands have now a `--debug` flag to build a binary
for debugging.
Bubbletea UI is used when the debug server is run (--server flag) while
a standard Cobra command is used when running the debug console. The
debug command without `--server` runs outside Bubbletea because the
debugger needs control over stdout.
@jeronimoalbi jeronimoalbi marked this pull request as ready for review December 7, 2022 10:51
@jeronimoalbi
Copy link
Member Author

Running the debugger in macOS requires the command line developer tools to be installed. I'm going to add a warning for that case but it's also possible to ask the user and trigger the developer tools installation from the CLI. This functionality could be documented instead which would keep the CLI code simpler.

In any case we need a new section in the documentation to explain how to start debugging using the CLI or other clients like VSCode.

@Pantani
Copy link
Collaborator

Pantani commented Dec 7, 2022

Running the debugger in macOS requires the command line developer tools to be installed. I'm going to add a warning for that case but it's also possible to ask the user and trigger the developer tools installation from the CLI. This functionality could be documented instead which would keep the CLI code simpler.

In any case we need a new section in the documentation to explain how to start debugging using the CLI or other clients like VSCode.

I think it's ok because we already need the developer tools for Go and homebrew. But it's nice we can document this and also the steps to debug

ignite/cmd/chain.go Outdated Show resolved Hide resolved
ignite/cmd/chain_debug.go Outdated Show resolved Hide resolved
tbruyelle
tbruyelle previously approved these changes Dec 8, 2022
Copy link
Contributor

@tbruyelle tbruyelle left a comment

Choose a reason for hiding this comment

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

Awesome feature!

aljo242
aljo242 previously approved these changes Dec 8, 2022
tbruyelle
tbruyelle previously approved these changes Dec 9, 2022
@codecov
Copy link

codecov bot commented Dec 9, 2022

Codecov Report

Merging #3221 (912f062) into main (aec01ac) will increase coverage by 0.01%.
The diff coverage is 18.67%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3221      +/-   ##
==========================================
+ Coverage   20.05%   20.06%   +0.01%     
==========================================
  Files         383      386       +3     
  Lines       30487    30822     +335     
==========================================
+ Hits         6114     6185      +71     
- Misses      23784    24049     +265     
+ Partials      589      588       -1     
Impacted Files Coverage Δ
ignite/cmd/chain.go 0.00% <0.00%> (ø)
ignite/cmd/chain_build.go 0.00% <0.00%> (ø)
ignite/cmd/chain_debug.go 0.00% <0.00%> (ø)
ignite/cmd/chain_init.go 0.00% <0.00%> (ø)
ignite/cmd/model/chain_serve.go 73.21% <ø> (ø)
ignite/pkg/debugger/server.go 0.00% <0.00%> (ø)
ignite/pkg/gocmd/gocmd.go 22.50% <ø> (ø)
ignite/services/chain/build.go 0.00% <0.00%> (ø)
ignite/services/chain/serve.go 0.00% <0.00%> (ø)
...nite/services/network/networkchain/networkchain.go 0.00% <0.00%> (ø)
... and 5 more

@aljo242 aljo242 merged commit ce76c25 into main Dec 9, 2022
@aljo242 aljo242 deleted the feat/chain-debug-command branch December 9, 2022 15:49
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* feat: add support to compile chain app without optimizations

* feat: add `debugger` package

* chore: change `cliui` package to allow multiple `Session.End()` calls

* feat: add `chain debug` command to launch a debugging session

* feat: add debug server support to `chain debug` command

This allows running a debug server without launching a debug client
which is useful to debug using other clients like gdlv or VSCode.

* chore: consistency changes

* chore: add missing handling for config flag

* chore: fix GRPC address

* feat: add bubbletea UI to `chain debug` when `--server` flag is present

* chore: remove some `chain debug` flags in favour of default values

* refactor: simplify debug command by removing build functionality

To keep the command simple it only starts the debug shell or server
favouring the build and init commands to init/reset blockchain app data
or to build the debug binary.

Both build and init commands have now a `--debug` flag to build a binary
for debugging.

* chore: change `ignite serve` to build a debugging binary

* docs: add long description for `ignite debug` command

* test: add unit tests for `chain debug` server UI views

* fix: change error output to be consisten between UI & non UI runs

Bubbletea UI is used when the debug server is run (--server flag) while
a standard Cobra command is used when running the debug console. The
debug command without `--server` runs outside Bubbletea because the
debugger needs control over stdout.

* refactor: use a single `AddCommand` call to register commands

Co-authored-by: Danilo Pantani <[email protected]>

* chore: remove redundant `AddCommand` calls

* chore: change chain debug command variable name to match flag name

Co-authored-by: Alex Johnson <[email protected]>

* chore: move disconnect channel init to debug options initialization

Co-authored-by: Thomas Bruyelle <[email protected]>

* docs: add docstrings to public model methods

Co-authored-by: Danilo Pantani <[email protected]>
Co-authored-by: Alex Johnson <[email protected]>
Co-authored-by: Thomas Bruyelle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feat To implement new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add support for delve debugger
4 participants