Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Map remote go module cache to local module cache #3079

Merged
merged 2 commits into from
Apr 22, 2020
Merged

Map remote go module cache to local module cache #3079

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec42b49
Convert remote Go module cache paths to local Go module cache paths
fnmunhoz Feb 28, 2020
8a5a54e
Use the first gopath
ramya-rao-a Mar 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/debugAdapter/goDebug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,14 +846,29 @@ class GoDebugSession extends LoggingDebugSession {
return this.convertDebuggerPathToClient(pathToConvert);
}

// Fix for https://github.com/Microsoft/vscode-go/issues/1178
// When the pathToConvert is under GOROOT, replace the remote GOROOT with local GOROOT
// When the pathToConvert is under GOROOT or Go module cache, replace path appropriately
if (!pathToConvert.startsWith(this.delve.remotePath)) {
// Fix for https://github.com/Microsoft/vscode-go/issues/1178
const index = pathToConvert.indexOf(`${this.remotePathSeparator}src${this.remotePathSeparator}`);
const goroot = process.env['GOROOT'];
if (goroot && index > 0) {
return path.join(goroot, pathToConvert.substr(index));
}

const indexGoModCache = pathToConvert.indexOf(
`${this.remotePathSeparator}pkg${this.remotePathSeparator}mod${this.remotePathSeparator}`
);
const gopath = (process.env['GOPATH'] || '').split(path.delimiter)[0];

if (gopath && indexGoModCache > 0) {
return path.join(
gopath,
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @stamblerre

@fnmunhoz, @quoctruong I have pushed a commit to use the first go path, can either of you test this one more time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramya-rao-a sure I'll try to do it today and let you know. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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!

pathToConvert
.substr(indexGoModCache)
.split(this.remotePathSeparator)
.join(this.localPathSeparator)
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds good.

);
}
}
return pathToConvert
.replace(this.delve.remotePath, this.delve.program)
Expand Down