More work on MSI installer #1234
Conversation
|
A potential issue with the current approach is that the logic to determine the install location (which is being extracted into a separate crate as per #1205) is now duplicated here in the MSI (using MSI actions instead of calling Rust code). It could be delegated to Rust code in a custom action, but in general we should aim to use custom actions as sparingly as possible where MSI provides equivalent functionality natively. I do understand that this makes maintenance harder, so this is something that needs to be discussed, not only w.r.t. this PR, but future work on the MSI in general. |
|
Thanks for this! Sorry I'm pretty out-of-the-loop with respect to the MSI installer. Mind filling in the details for me a bit? My impression was that the MSI installer was mostly for a better installation experience on Windows (e.g. the first time you use it) and then I believe for uninstallation as well (it shows up in the "Uninstall Programs" standard thing on Windows). Is that right? If so, is the thinking here that we'll transition to the MSI installer by default for everyone that already exists to ensure that there's a uniform uninstallation experience? |
Yes. Some discussion (with a few issues still needing to be figured out) happened in #253.
At least that would be my thinking, yes. So at some point, we would enable a transition path from the non-MSI
As you say, the goal is a better (un-)installation experience on Windows. MSI provides this, but is probably not the only way to achieve it (a custom GUI which registers the appropriate registry entries for the system-wide "Installed Programs" dialog would also work). MSI also has the advantage that it runs within a system service, so it won't suffer from the problems around deleting files that are in-use by Windows that lead to this monster. The "problem" with MSI is that it's a database format which is meant to be used mainly to install files and settings. So what I did (after discussing it with @brson), is to turn rustup into a DLL and use that from the MSI in the form of "custom actions": "Custom actions" are exported DLL functions which are called while the MSI proceeds, so we can theoretically call all the existing rustup installation logic (written in Rust) from the MSI. However, the "right"/idiomatic way to write MSIs is to have as few custom actions as possible, and use them only for very specific, small tasks, because writing correct custom actions (that e.g. also work on rollback) is hard (this is why I disabled rollback completely for this MSI). MSI built-in actions are already quite powerful and definitely more robust than what we can come up with. So most of the installation logic can be written in MSI/WIX itself, e.g. adding the rustup directory to the path in a robust way is trivial (which theoretically lets us get rid of a lot of Windows-specific code in the rustup self-install code). But duplicating the logic in WIX is a maintenance burden of course ... One thing which is currently being done by Rust code, but should (IMHO) be done by MSI built-in actions instead is the "hard-linking" of the installed binaries. There is no built-in action for hard-linking in MSI, only for duplicating files, but the problem is that hard-linking either requires admin privileges or only works on NTFS (it's currently implemented in that way ... did anyone ever stumple upon that?). However, considering that the Rust toolchain needs plenty of disk space anyway, I wonder if it's really not a better way to just duplicate the files (brson was against that when I brought it up before). Lastly, I want to add a personal thing: I'm probably the only one right now working on this, and I don't know if there is anyone else familiar with MSI/WIX in the Rust community. The problem is that unfortunately I won't be able to contribute much more in the future (besides the PRs that I have opened already) because I have other obligations. |
|
Another thing: The largest piece that is still missing after this PR is the actual setup of the initial toolchain (including the GUI that lets you select which toolchain(s) you want). |
|
Ok thanks for the explanation! And no worries if you've got other obligations now, thanks regardless for all your work up to this point! This seems like a totally OK PR to land in the meantime as it's mostly just touching the installer. Could the |
|
Sure. I've now removed the commit concerning |
|
@bors: r+ |
|
📌 Commit 1a55ea0 has been approved by |
More work on MSI installer Actually implements a large part of the MSI logic (previously this was mostly stubs). With these changes, the MSI installer will install rustup to the right location, so running the installer without setting `CARGO_HOME` will overwrite the existing installation! A test script is included that installs rustup into a different location by setting `CARGO_HOME` (and `RUSTUP_HOME`). This PR also enables building/testing the MSI version on AppVeyor CI (but the tests for install and self update are disabled, because they are not compatible with the MSI version, and no MSI specific tests have been added yet).
|
💥 Test timed out |
|
@bors: retry
…On Mon, Aug 28, 2017 at 9:04 PM, bors ***@***.***> wrote:
💥 Test timed out
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1234 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAD95N5TYir7KDZF8TxqEs4dQdriBMXLks5sc3G8gaJpZM4O2B6F>
.
|
More work on MSI installer Actually implements a large part of the MSI logic (previously this was mostly stubs). With these changes, the MSI installer will install rustup to the right location, so running the installer without setting `CARGO_HOME` will overwrite the existing installation! A test script is included that installs rustup into a different location by setting `CARGO_HOME` (and `RUSTUP_HOME`). This PR also enables building/testing the MSI version on AppVeyor CI (but the tests for install and self update are disabled, because they are not compatible with the MSI version, and no MSI specific tests have been added yet).
|
@bors: retry |
More work on MSI installer Actually implements a large part of the MSI logic (previously this was mostly stubs). With these changes, the MSI installer will install rustup to the right location, so running the installer without setting `CARGO_HOME` will overwrite the existing installation! A test script is included that installs rustup into a different location by setting `CARGO_HOME` (and `RUSTUP_HOME`). This PR also enables building/testing the MSI version on AppVeyor CI (but the tests for install and self update are disabled, because they are not compatible with the MSI version, and no MSI specific tests have been added yet).
|
☀️ Test successful - status-appveyor, status-travis |
Actually implements a large part of the MSI logic (previously this was mostly stubs).
With these changes, the MSI installer will install rustup to the right location, so running the installer without setting
CARGO_HOMEwill overwrite the existing installation!A test script is included that installs rustup into a different location by setting
CARGO_HOME(andRUSTUP_HOME).This PR also enables building/testing the MSI version on AppVeyor CI (but the tests for install and self update are disabled, because they are not compatible with the MSI version, and no MSI specific tests have been added yet).