Map remote go module cache to local module cache #3079
Conversation
|
Thanks for the PR @fnmunhoz @quoctruong, Can you verify the fix here? |
|
LGTM |
|
|
||
| if (gopath && indexGoModCache > 0) { | ||
| return path.join( | ||
| gopath, |
There was a problem hiding this comment.
gopath can potentially have multiple paths with the path separator specific to the OS platform. In that case, we can't use it in path.join directly this way.
There was a problem hiding this comment.
Does /pkg/mod apply to the first path in the gopath? Is there a rule around this?
Also, we need to ensure the right path separators are being used like we did in #3108
cc @quoctruong
There was a problem hiding this comment.
Does /pkg/mod apply to the first path in the gopath? Is there a rule around this?
Yes. I'm not sure where this is specified, but you can see goimports relying on this fact here.
There was a problem hiding this comment.
Thanks @stamblerre
@fnmunhoz, @quoctruong I have pushed a commit to use the first go path, can either of you test this one more time?
There was a problem hiding this comment.
@ramya-rao-a sure I'll try to do it today and let you know. Thanks.
There was a problem hiding this comment.
Hey @ramya-rao-a I just double checked the extension after the change you made and it works fine for me, I was able to debug all files, including libraries inside go mod cache.
Thanks!
|
|
|
@fnmunhoz Thanks for your first PR contribution to this project. Can you please sign the CLA as requested in the above comment? |
| pathToConvert | ||
| .substr(indexGoModCache) | ||
| .split(this.remotePathSeparator) | ||
| .join(this.localPathSeparator) |
There was a problem hiding this comment.
I just thought of one problem with this logic. Even if you are replacing the local path separator by the remote path separator, path.join will normalize the path according to the platform. This means that you probably have to do the replacing part AFTER joining the path?
There was a problem hiding this comment.
We will have to do similar changes like you did to toDebuggerPath here as well. I was hoping to do that outside of this PR and have this PR focus only on the module cache part. Because the change has to applied to the goroot workaround and to the return statement a few lines below
|
Hey @ramya-rao-a sorry for the really long delay. I've signed the CLA. Not sure if this PR is still valid based on the comments above, but feel free to go ahead with it if that's the case. Thanks! |
|
Thanks @fnmunhoz! |
|
Yay! Thank you @ramya-rao-a |
|
The latest update 0.14.2 has this change, thanks @fnmunhoz! |
|
Good to know! Thanks @ramya-rao-a 😃 |
This is similar to another scenario: #1179
When using the remote debug feature, the paths to remote go module cache might not match the paths to local go module cache.
The idea here is to check if the remote path includes the go module cache sub string, for example:
/go/pkg/mod/github.com/account/packageIn this case, it would be a match for the go module cache substring:
/pkg/mod/And in this case, the path would be replaced by:
/Users/someuser/go/pkg/mod/github.com/account/packageConsidering a local gopath like this:
$GOPATH="/Users/someuser/go".By doing that, vscode-go can find all source files during debug, including cached modules.
I've made the minimal required changes to make it work for the scenario that I was facing, but I'm happy to make improvements based on feedback.
If I missed any guidelines, please let me know.
Also, I'm curious to know if this is the right approach to deal with this, or there is a better alternative.