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

delete and recreate go.mod instead of running go mod tidy #2948

Merged
merged 2 commits into from
Dec 23, 2019
Merged

delete and recreate go.mod instead of running go mod tidy #2948

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b5bd55
delete and recreate go.mod instead of running go mod tidy
stamblerre Dec 16, 2019
0910988
only write file when modules are on
stamblerre Dec 23, 2019
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
15 changes: 7 additions & 8 deletions src/goInstallTools.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,6 @@ export function installTools(missing: Tool[], goVersion: GoVersion): Promise<voi
// Install tools in a temporary directory, to avoid altering go.mod files.
const toolsTmpDir = fs.mkdtempSync(getTempFilePath('go-tools-'));

// Write a temporary go.mod file.
const tmpGoModFile = path.join(toolsTmpDir, 'go.mod');
fs.writeFileSync(tmpGoModFile, 'module tools');

return missing.reduce((res: Promise<string[]>, tool: Tool) => {
// Disable modules for tools which are installed with the "..." wildcard.
// TODO: ... will be supported in Go 1.13, so enable these tools to use modules then.
Expand All @@ -148,16 +144,19 @@ export function installTools(missing: Tool[], goVersion: GoVersion): Promise<voi
envForTools['GO111MODULE'] = 'on';
}

// Write a temporary go.mod file. This shouldn't affect anything if we've explicitly disabled modules.
const tmpGoModFile = path.join(toolsTmpDir, 'go.mod');
fs.writeFileSync(tmpGoModFile, 'module tools');
Copy link
Contributor

Choose a reason for hiding this comment

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

With this, we are creating a new go.mod file for each tool being installed right?
In that case, should we move it inside the else block of the if (modulesOffForTool) check a few lines above?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, did you check if fs.writeFileSync fails gracefully if the file already exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't matter if we write the file even if modules are off, but I moved the check in case. fs.writeFileSync replaces the file if it already exists.


return res.then(sofar => new Promise<string[]>((resolve, reject) => {
const opts = {
env: envForTools,
cwd: toolsTmpDir,
};
const callback = (err: Error, stdout: string, stderr: string) => {
// Make sure to run `go mod tidy` between tool installations.
// This avoids us having to create a fresh go.mod file for each tool.
if (!modulesOffForTool) {
cp.execFileSync(goRuntimePath, ['mod', 'tidy'], opts);
// Make sure to delete the temporary go.mod file, if it exists.
if (fs.existsSync(tmpGoModFile)) {
fs.unlinkSync(tmpGoModFile);
}
if (err) {
outputChannel.appendLine('Installing ' + getImportPath(tool, goVersion) + ' FAILED');
Expand Down