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

x/build/cmd/release: silent uninstall of stable Go by release candidate #16305

Closed
pi opened this issue Jul 8, 2016 · 36 comments
Closed

x/build/cmd/release: silent uninstall of stable Go by release candidate #16305

pi opened this issue Jul 8, 2016 · 36 comments

Comments

@pi
Copy link

@pi pi commented Jul 8, 2016

I specified c:\go17 directory for RC's installation and Go 1.6.2 from c:\go was removed without confirmation. Please add uninstallation confirmation during release candidate cycle.

@pi pi changed the title Windows: silent uninstal of stable Go by release candidate Windows: silent uninstall of stable Go by release candidate Jul 8, 2016
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 8, 2016

@bradfitz bradfitz changed the title Windows: silent uninstall of stable Go by release candidate x/build/cmd/release: silent uninstall of stable Go by release candidate Jul 8, 2016
@bradfitz bradfitz added this to the Go1.7Maybe milestone Jul 8, 2016
@adg adg added the help wanted label Jul 8, 2016
@adg
Copy link
Contributor

@adg adg commented Jul 8, 2016

We really need help from someone who know how the MSI builder stuff works.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 9, 2016

I specified c:\go17 directory for RC's installation

Go installer assumes (see GOROOT_FINAL) that you will install Go into C:\Go directory. If you install RC into c:\go17, then all programs you build with RC will have references to files under C:\Go. So your stack traces will be wrong, your debugger will be confused.

If you want to install Go anywhere but c:\go you should just build from source.

Alex

@pi pi closed this Jul 9, 2016
@pi pi reopened this Jul 9, 2016
@pi
Copy link
Author

@pi pi commented Jul 9, 2016

@alexbrainman I can't build yet. TestDirWindows and TestEnvOverride fails in net/http/cgi test. Looking into it.

@pi
Copy link
Author

@pi pi commented Jul 9, 2016

That was perl. Now GDB dumps core in TestGdbPython and TestGdbBacktrace. Anyway issue was about installer, not about building from sources on windows.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 9, 2016

Anyway issue was about installer, not about building from sources on windows.

I understand. But IMHO the installer is for users how are happy with single version of Go installed into c:\go. If you want something different you should build from source.

Installer adds your %GOROOT%\bin directory to the %PATH%. If you use installer to install 2 different versions of Go into 2 different directories, the installer will add both ...\bin directories to the PATH. How are you going to run the Go version you want?

TestDirWindows and TestEnvOverride fails in net/http/cgi test.

Feel free to create new issue for it. Tests shouldn't fail just because you don't have PERL installed.

Now GDB dumps core in TestGdbPython and TestGdbBacktrace.

Same.

You don't have to run all tests to "build" your Go installation from source. Just running make.bat should do.

Alex

@pi
Copy link
Author

@pi pi commented Jul 9, 2016

Feel free to create new issue for it. Tests shouldn't fail just because you don't have PERL installed.

I don't want to bother the devs with secondary platform's build problems. I have ubuntu vm for experiments.

@broady
Copy link
Member

@broady broady commented Jul 10, 2016

I don't want to bother the devs with secondary platform's build problems.

Are you implying that Windows is a secondary platform? That's definitely not accurate. Windows is a first-class port.

@pi
Copy link
Author

@pi pi commented Jul 10, 2016

Secondary building platform, not port. Primary platform - where Go is developed in the first place. For example, problem with perl was totally my fault - there was perl.bat on my PATH, but no perl installed. Not worth filling an issue.

@pbennett
Copy link

@pbennett pbennett commented Jul 13, 2016

I can try to take a look at it or failing that, pass it on to someone in my company who knows WiX extremely well.

@adg
Copy link
Contributor

@adg adg commented Jul 13, 2016

@alexbrainman I am pretty sure the installer sets GOROOT, so setting an install path to C:\Go17 should work fine.

@pbennett
Copy link

@pbennett pbennett commented Jul 13, 2016

It seems there's multiple things being discussed here. So is the issue still simply that no confirmation dialog is displayed? That's it right?

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 13, 2016

@pbennett, I think this about us wanting them to be able to be co-installed (Go 1.6 stable and Go 1.7rc*), with different GOROOTs.

I think we need to use a different Product UUID in the wxl XML manifest thing.

@chadmiles
Copy link

@chadmiles chadmiles commented Jul 13, 2016

Most companies these days release 'major upgrades' for their product (eg: an new MSI that upgrades a previous MSI installation). When running the upgrade it will silently remove the previous version of the product behind-the-scenes before installing the new product (or it could install the new product first and remove anything that's orphaned from the previous version at the very end of the upgrade).

https://msdn.microsoft.com/en-us/library/windows/desktop/aa369786%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396

I downloaded 1.6.2 and 1.7 and confirmed the UpgradeCode UUIDs are the same so any upgrade will remove the previous version from the system.

To @bradfitz, the ProductCodes have changed between 1.6.2 and 1.7 (also another factor in developing an install as a major upgrade to a previous product release). The ProductCode changes, the ProductVersion changes, and the UpgradeCode stays the same.

There's ways to hack this, but you'll break any future upgrade path possibilites. Not something I'd advise.

@pbennett
Copy link

@pbennett pbennett commented Jul 13, 2016

@bradfitz Does go really support co-installs? The OSX install forces a single install, so it at least, implies only one is allowed. Is GOROOT just set implicitly from the go executables run path if not set explicitly?
Welcome @chadmiles ! (He's one of the install guys where I work - so we're in good hands now for WiX).

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 14, 2016

I am pretty sure the installer sets GOROOT, so setting an install path to C:\Go17 should work fine.

I am not sure about GOROOT.

But I was concerned about what happens to the PATH. The installer is updating global user PATH variable to include %GOROOT%\bin directory in it. For example, if you tell installer to install your Go into c:\go, you PATH will have:

set PATH=....;c:\go\bin

Do you suggest we install different version of Go into, for example, c:\go17 and add c:\go17\bin to the global PATH? Like:

set PATH=....;c:\go\bin;c:\go17\bin

But then Windows will find go.exe in c:\go\bin, not in c:\go17\bin. How are you going to control which version you want to use?

Same for GOROOT.

Also do not forget that GOROOT_FINAL is set to "c:\go" for a reason here https://github.com/golang/build/blob/master/cmd/release/release.go#L315. All binary *.a files under %GOROOT%\pkg contain full path to the package source files. *.a files will be copied by installer to the user disk as is. When user links these into her executable it will still contain c:\go... regardless of where these are on the disk. So your stack traces and debugging file paths will be different to reality (unless you install Go into c:\go).

Alex

@Egojard
Copy link

@Egojard Egojard commented Jul 22, 2016

This is something we actually struggle quite a bit with ourselves. We do have to support various versions of our software, not all using the same go version, and basically had to "vendor" go itself, making go a versioned dependency of our projects and wrapping all uses under tools which deal with setting the various environment variables as needed. This goes a bit beyond the initial scope of the discussion, but being able to actually attach projects to a go version (and then map this to an actual go installation) is something I could see a usage for.

@broady
Copy link
Member

@broady broady commented Aug 1, 2016

@pbennett @chadmiles we're having another unrelated issue related to wix that has some urgency - could you e-mail me at cbro@golang.org, please?

@rsc
Copy link
Contributor

@rsc rsc commented Nov 2, 2016

@broady what's the status here?

Also, @alexbrainman, I think we have fixed the toolchain so that the right paths end up in the final binaries even for alternate install locations. What you say was true for a long time but I think (at least hope) it's no longer true.

@broady
Copy link
Member

@broady broady commented Nov 2, 2016

Sorry about dropping this. I made contact with someone at Google who works on Windows software and has worked with wix in the past. I'll follow up with him to figure out what we should be doing here.

@broady
Copy link
Member

@broady broady commented Nov 2, 2016

MajorUpgrade looks promising.

It sounds like we should consider changing the Product GUID for each major version. But there's also some separate GUID for upgrades that appears to be used to associate packages.

RemoveExistingProducts will uninstall those associated packages, but it's unclear whether we can prompt the user to say Yes/No to it.

I've requested a Windows machine, will provide more updates once it arrives.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Nov 3, 2016

I think we have fixed the toolchain so that the right paths end up in the final binaries even for alternate install locations. What you say was true for a long time but I think (at least hope) it's no longer true.

You are right, I was wrong. Final binaries refer to installed location, not c:\go.

Alex

@chadmiles
Copy link

@chadmiles chadmiles commented Nov 3, 2016

If you can provide a dialog with a checkbox that is tied to a secure PROPERTY (so it can be passed from the InstallUISequence to InstallExecuteSequence), I believe you can override the stock conditioning of the "RemoveExistingProducts" CustomAction like below:

<InstallExecuteSequence>
  <RemoveExistingProducts Before="InstallInitialize" Overridable="yes"><![CDATA[USER_CHOSE_YES_IN_DIALOG_CHECKBOX_TO_REP=1]]></RemoveExistingProducts>
</InstallExecuteSequence>
@pi pi assigned broady Nov 7, 2016
@bradfitz bradfitz modified the milestones: Go1.9, Go1.8Maybe Dec 15, 2016
@gopherbot gopherbot added the Builders label Mar 21, 2017
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 14, 2017

/cc @johnsonj, if this is up your alley.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 14, 2017

@broady, this just came up again (dup bug #20679).

Here's a proposal: we stop releasing Windows MSIs for betas. Those users can use the zip files or go get golang.org/x/build/version/go1.9beta1. That also simplifies your code signing step.

@dlsniper
Copy link
Contributor

@dlsniper dlsniper commented Jun 14, 2017

@bradfitz but the same problem will then happen for stable as well. I really don't like that I should compile the or download an archive. On Windows / OS X it should be really just what users are used to do, download the MSI, install them and that's it.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 14, 2017

@dlsniper, and that's why we have this bug open.

But nobody knows how to fix it, so one mitigation is that we don't make the problem worse by dangling buggy MSIs at beta users.

@dlsniper
Copy link
Contributor

@dlsniper dlsniper commented Jun 14, 2017

I see, I make no promise but I'll try and have a look to see if I can work something out. The MSI stuff is open as well, yes?

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 14, 2017

See golang.org/x/build/cmd/release for how the MSI is built.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 28, 2017

Anybody have time for this for Go 1.9?

@johnsonj
Copy link
Contributor

@johnsonj johnsonj commented Jun 29, 2017

There's a clear path to disabling the uninstall (as @chadmiles pointed out) but if you install 2 Go's (C:\go17, C:\go19) what should happen when you type go.exe in a new cmd window? Is it enough to prepend the latest install to the PATH?

I propose we prompt the user before uninstalling (example) and if they say no, we exit the installer. No user confusion on why things are uninstalled, no official management of multiple versions. If that sounds good I can do it for Go1.9.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 29, 2017

Sounds like an improvement over the current situation.

@johnsonj johnsonj assigned johnsonj and unassigned broady Jun 29, 2017
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jun 29, 2017

Is it enough to prepend the latest install to the PATH?

I wouldn't like that done to my PATH. Especially, if I did not clearly understand what you 're doing.

I think that people, who are interested in beta versions, are clever enough to use zip file instead of MSI. Maybe we should push install from zip file more. It is really easy to install Go from zip file - just PATH needs to be changed.

I propose we prompt the user before uninstalling (example) and if they say no, we exit the installer.

SGTM

Alex

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 11, 2017

CL https://golang.org/cl/48010 mentions this issue.

@golang golang locked and limited conversation to collaborators Jul 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.