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 support for Delve debugger #384

Merged
merged 7 commits into from
Mar 22, 2017
Merged

Conversation

amrfaissal
Copy link
Collaborator

This adds support for debugging tool Delve. The command dlv starts a debugging REPL that allows the user to debug his application using Delve tool.
image

This adds support for debugging tool Delve. The command dlv starts a
debugging REPL that allows the user to debug his application using Delve
tool. Resolves beego#365.
@sergeylanzman
Copy link
Collaborator

@amrfaissal How I can check it?

@astaxie
Copy link
Member

astaxie commented Mar 20, 2017

cmd/commands/dlv/dlv.go:98:2: should merge variable declaration with assignment on next line (S1021)

@amrfaissal
Copy link
Collaborator Author

To learn more this is the documentation: https://github.com/derekparker/delve/tree/master/Documentation
I found also a video on Youtube: https://www.youtube.com/watch?v=zgLjVD5ZSOc

I will label this PR as work-in-progress as I am planning to do some improvements.

@astaxie
Copy link
Member

astaxie commented Mar 21, 2017

@amrfaissal @sergeylanzman when this PR is done, I would like to release a new version: 1.8.1

@amrfaissal
Copy link
Collaborator Author

I think this PR is ready for review/merge. Let me know what you think.

@astaxie
Copy link
Member

astaxie commented Mar 21, 2017

AFAIK the dlv has different installation for different os. I see dlv dependence in vendor. Does the user still need to install dlv to use this command?

@amrfaissal
Copy link
Collaborator Author

This is the purpose of my improvements: No need to install dlv command line. Delve can be used either using the binary or the API. In the last commits, I am using the API by starting the server and client terminal. The server used in bee supports MultiClient mode so the user can use bee dlv to use it as client and open another terminal and start a different client using: dlv connect [addr]:[port].


//
// Create and start the debugger server
//
Copy link
Member

Choose a reason for hiding this comment

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

you can remove these empty comments


//
// Start the Delve client REPL
//
Copy link
Member

Choose a reason for hiding this comment

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

you can remove these empty comments

utils/utils.go Outdated

// GoBuild runs the "go build" command on the specified package
func GoBuild(debugname, pkg string) error {
buildFlags := "-ldflags='-linkmode internal'"
Copy link
Member

Choose a reason for hiding this comment

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

could you add the comments here why need to use -ldflags='-linkmode internal' is this for dlv only? As this is the util package, so maybe other package will use it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes you're right. Other packages might use different ldflags, but here I am just making sure Go's internal linker is used. I will move this function to dlv.go.

func runDelve(addr, debugname string) int {
beeLogger.Log.Info("Starting Delve Debugger...")

err := utils.GoBuild(debugname, packageName)
Copy link
Member

Choose a reason for hiding this comment

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

from your screenshot you don't need the packageName for it. So bee dlv would start to work. But it looks you pass this to the go build -o. If it's not set then the output would be empty. I think you need to add a deafult value for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No debugname is passed to go build -o then we specify what package to build. If no package is supplied go build builds the current directory. So no need to add a default value.

@astaxie
Copy link
Member

astaxie commented Mar 21, 2017

The server used in bee supports MultiClient mode so the user can use bee dlv to use it as client and open another terminal and start a different client using: dlv connect [addr]:[port].

Thanks for the detail introduction. So we need to update our readme and docs

@amrfaissal
Copy link
Collaborator Author

I'll update the README as well before the release of 1.8.1.

@amrfaissal amrfaissal merged commit 8c0742c into beego:develop Mar 22, 2017
Baihhh pushed a commit to Baihhh/bee that referenced this pull request Jul 5, 2023
…g button (beego#384)

Add loading and countdown status to the verification code sending button

Revert "Add bcrypt encrypted password type"

This reverts commit ae995b5805de2caa02edd92cfb48bbcfb4dfb166.

fix:indentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants