Do not export GOROOT for Go versions >= 1.9

[mmlb]

Description:

Do not export GOROOT. This has not been necessary since Go 1.9 at least (although clunky) but definitely not since Go 1.10. This is cargo culting code that is more than 2 years out of date and runs into issues when multiple go versions are used in an action run.

Related issue:

#107

Check list:

• Mark if documentation changes are required.
• Mark if tests were added or updated to cover the changes.

[mmlb]

Opened this up as a draft to get CI running as I'm a super n00b at ts/js. I wanted to test the current behavior (adding GOROOT) before removing it and updating the test.

[mmlb]

The actual fix commit I'll add afterwards is:

commit 607cfc72940f41ed709d01f36a75387ed7d150dd
Author: Manuel Mendez <[email protected]>
Date:   Tue Dec 14 13:24:18 2021 -0500

    Do not export GOROOT
    
    This has not been necessary since [Go 1.9](https://go.dev/doc/go1.9#goroot) at least (although clunky) but definitely not since [Go 1.10](https://go.dev/doc/go1.10#goroot).
    This is cargo culting code that is more than 2 years out of date and runs into issues when multiple go versions are used in an action run.
    
    Signed-off-by: Manuel Mendez <[email protected]>

diff --git a/__tests__/setup-go.test.ts b/__tests__/setup-go.test.ts
index a001839..9368804 100644
--- a/__tests__/setup-go.test.ts
+++ b/__tests__/setup-go.test.ts
@@ -243,7 +243,7 @@ describe('setup-go', () => {
     });
 
     await main.run();
-    expect(vars).toBe({'GOROOT': 'foo'});
+    expect(vars).toBe({});
   });
 
   it('finds a version of go already in the cache', async () => {
diff --git a/src/main.ts b/src/main.ts
index 2d90b2f..29ae2b6 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -26,7 +26,6 @@ export async function run() {
 
       const installDir = await installer.getGo(versionSpec, stable, auth);
 
-      core.exportVariable('GOROOT', installDir);
       core.addPath(path.join(installDir, 'bin'));
       core.info('Added go to the path');

[mmlb]

Gentle ping, @spark

[mmlb]

Pinging @bryanmacfarlane and @joshmgross because it seems the spark I pinged isn't the same spark as in reviewers.

[mmlb]

Hi @joshmgross I've updated to address a merge conflict. It'd be really nice if someone could take a look here.

[joshmgross]

👋 @mmlb could you take a look at the failing CI?

It looks like we need to update dist.jswith npm run build and there's a formatting issue

[mmlb]

Hey @joshmgross yep will fix. I remember now that I expected ci to fail for this reason, I couldn't figure out how to get tests running locally.

[mmlb]

@joshmgross updated!

[mmlb]

Thanks for the review to both of you. I'm not a ts/js dev so had no clue how to run/test this and was letting CI do, but using the npm run build as a hint I figured out npm run test :D.

[joshmgross]

It looks like CI is failing for validating setup-go with versions < 1.9 that still need GOROOT - https://github.com/actions/setup-go/runs/5500146663?check_suite_focus=true

It seems like we should still export GOROOT if go-version is < 1.9.
@mmlb can you made those changes? If you're not comfortable enough with JS/TS, I'd be happy to help out.

[mmlb]

It looks like CI is failing for validating setup-go with versions < 1.9 that still need GOROOT - actions/setup-go/runs/5500146663?check_suite_focus=true

It seems like we should still export GOROOT if go-version is < 1.9. @mmlb can you made those changes? If you're not comfortable enough with JS/TS, I'd be happy to help out.

I'd appreciate if you took over on that change.

Side question: go1.8 is > 5years old, whats the policy for setup-go dropping versions? Is it "don't, unless really really necessary"?

[joshmgross]

Side question: go1.8 is > 5years old, whats the policy for setup-go dropping versions? Is it "don't, unless really really necessary"?

I don't know if we have an explicit policy at this point - we'd need more data to understand what versions users need in their workflows.

[mmlb]

thanks for taking this up, just one small nit.

[mmlb]

lgtm, for whatever thats worth :D

[brcrista]

Thanks @mmlb !