pop-os / installer

Installer for Pop!_OS & other Linux-based OSes
https://system76.com/pop
GNU General Public License v3.0
26 stars 16 forks source link

Alongside OS & Refresh Install Options #157

Closed mmstick closed 5 years ago

mmstick commented 6 years ago

Requires https://github.com/pop-os/distinst/pull/132

Refresh Install

This takes Cassidy's work with the DecryptionDialog at https://github.com/elementary/installer/pull/333, and integrates it with distinst, as well as a new RefreshView to list detected refresh options and to make them happen. This will require a few changes in distinst to expose some previously-hidden APIs, and to fix some issues with installing (updating the recovery partition fails due to running out of space).

Screenshots

screenshot from 2018-11-08 11-30-22

screenshot from 2018-11-08 16-31-08

Testing

Perform a clean install with and without encryption, and then do the following:

For the UI, these can be tested:

Alongside OS Install

The work had already been done in distinst, so it was just a matter of exposing it in the UI. By sharing UI code with the Refresh install view, both views have the same option selection view, but the alongside choice will also feature a slider for determining how much a partition should be shrunk to make room for the new install.

Screenshots

screenshot from 2018-11-08 17-09-50

Sector Slider

Testing

jackpot51 commented 6 years ago

@mmstick Please fix the build issues

mmstick commented 6 years ago

@jackpot51 It should build once https://github.com/pop-os/distinst/pull/132 is merged, since it requires some FFI and script changes.

WatchMkr commented 6 years ago

Why would a user want to keep the backup install? What would they do with it?

mmstick commented 6 years ago

@WatchMkr They may:

WatchMkr commented 6 years ago

Few things:

Let's move the "Applications installed on the system...." copy to the previous page "Refresh Install" description. To shorten it up we can use "Reinstall while keeping user accounts and files. Applications will need to be reinstalled manually." This way we can remove the extra step if there's only one possible target.

Since the drive is unlocked in the third screenshot, should the unlock dialog be removed?

Regarding prompting to keep the old install, I don't like this option for a few reasons. -- As a user, the affirmative action is to choose the green button; however, I have no utility to restore to this install. I don't know why I should choose yes or no. My instinct is to keep the backup just-in-case but that's likely unnecessary and a poor default choice. -- It breaks with the application UI (fixable) -- I don't know how much space it will take. (fixable) -- We're treating it somewhat like a snapshot backup but not considering other methods and technologies that might do this better. I'd prefer to consider snapshots and backups outside of the installer before thinking through how it affects the installer.

mmstick commented 6 years ago

@WatchMkr

Since the drive is unlocked in the third screenshot, should the unlock dialog be removed?

Yes, I'm working on the means to do that.

I do like the option to keep it, myself, but I'll remove it for now.

mmstick commented 6 years ago

I think this is ready for some testing. This will require to be tested alongside https://github.com/pop-os/distinst/pull/132, which will need basically every part of the installer to be tested again.

As soon as this is merged, I'll be adding the multiboot install option to the UI. An upgrade option could also be added to the UI somehow, since the installer can now compare the name and version of the OS to install with the name and version of the OS being refreshed.

mmstick commented 6 years ago

Found an issue with secondary groups. Will have that fixed soon.

mmstick commented 6 years ago

The "Install Alongside OS" option is ready now. It will list up to two options per block device: installing to the largest free region that meets minimum requiements, and shrinking the partition with the largest unused space.

brs17 commented 5 years ago

The first thing I noticed with this pr is the need to scroll to see all of the installer options. I think that makes the installer feel heavier and is probably not great UX.

screenshot from 2018-11-07 19-53-15

brs17 commented 5 years ago

The first thing I tried to do was do a "Refresh Install". The install I had on the disk was a custom partitioned drive (efi, /swap, /, /home, /tmp). After selecting this option, the installer immediately failed:

screenshot from 2018-11-07 19-53-48

I then selected to try installing again. This time when selecting "Refresh Install", I got the next page.

Until I noticed that the button to proceed was enabled. See the following screenshot: screenshot from 2018-11-07 19-54-28

It seems as though something is missing here (as it differs from the screenshot you provided).

Proceeding here, this fails as well screenshot from 2018-11-07 19-54-46

brs17 commented 5 years ago

With the previous having failed, I then attempted to "Install alongside OS". screenshot from 2018-11-07 19-59-17

This seems to be quite text heavy and potentially confusing in what is going to happen exactly.

Once selecting one of the options, the selection itself looks a little odd, with the orange highlight so far to the left of the actual option (since the button is that wide).

Also I am not really sure here what partitions those options are. (which one is partition 4?)

screenshot from 2018-11-07 20-02-53

My confusion continues on the next page:

screenshot from 2018-11-07 20-03-53

I think this could be shown better to explain what is happening to both partitions.

Moving the slider all the way to the left, I have a non-zero value. Why is that? Is MiB the best unit to use here with >9000MiB being the value I am looking at? With custom partitioning we just use gparted for all of the partitioning work, why not continue that here (and benefit from the visual contexts it provides)?

mmstick commented 5 years ago

Seems the recent distinst changes have broken the options. Probably the PARTUUID change.

Moving the slider all the way to the left, I have a non-zero value.

The minimum value for the slider is the minimum required size for an install. There has to be enough room for all the partitions that distinst will create in the free space of that shrunk partition.

With custom partitioning we just use gparted for all of the partitioning work, why not continue that here (and benefit from the visual contexts it provides)?

Main reason is that they don't need to. The installer will automate that process for them, whether it's choosing to install to unpartitioned space on the disk, or shrinking an existing partition to make room for the new install.

WatchMkr commented 5 years ago

Try or Install page UX Changes:

Change the page title from "Try or Install" to "Install" Change "Try Demo Mode" to a button in the ancillary action location at the bottom left of the window.

Copy change: Install Alongside another Operating System Add Pop!_OS next to one or more existing OS installations.

Alignment: Please align the Refresh Install option title and description with the other options.

I'll take a peak at the refresh and install alongside OS options next.

brs17 commented 5 years ago

Looking at build: 9aaae3c after doing a full disk encrypted install:

@WatchMkr, when you suggested moving Try Demo Mode to the bottom left, did you mean the following way or making it a button like the Next and Back button?

screenshot from 2018-11-07 22-41-00

WatchMkr commented 5 years ago

A button like the Back and Next buttons.

brs17 commented 5 years ago

When selecting to Unlock Encrypted Storage a popup appears:

screenshot from 2018-11-07 22-47-26

The only other time a popup appears is when gparted opens. Do we want a popup here? I could follow either argument.

I did notice, one has to press ESC twice to close the popup which seems odd.

Unlock page: screenshot from 2018-11-07 22-48-39

Wrong password page: screenshot from 2018-11-07 22-48-44

Thoughts @WatchMkr @jackpot51 ?

brs17 commented 5 years ago

The minimum value for the slider is the minimum required size for an install. There has to be enough room for all the partitions that distinst will create in the free space of that shrunk partition.

With custom partitioning we just use gparted for all of the partitioning work, why not continue that here (and benefit from the visual contexts it provides)?

I guess I kinda felt like a lot of this was presented without as good context as I would have liked.

WatchMkr commented 5 years ago

I think it's correct to use a pop-up for encryption unlock. If there is only one possible partition to unlock, skip selection and go directly to the second window. The copy for the second window should be:

"Unlock partition /dev/$partname" along with the Password and Device Name entry boxes. Is Device Name already filled in?

brs17 commented 5 years ago

Device name is already filled in.

I also saw that when typing in an incorrect password, once I pressed back, the incorrect password was still in the field, I imagine that should be cleared as well.

brs17 commented 5 years ago

I don't know why I didn't think about it sooner but I should probably use Peek or something and just record myself going through the process.

evertiro commented 5 years ago

Looking at build: 9aaae3c after doing a full disk encrypted install:

@WatchMkr, when you suggested moving Try Demo Mode to the bottom left, did you mean the following way or making it a button like the Next and Back button?

screenshot from 2018-11-07 22-41-00

Having Install, Refresh, Custom, Alongside, Demo... it gets bulky fast. Is there any good reason we can't do what Ubuntu did and have the first dialog simply be Try/Install, with the installation options being presented only after the user has indicated they want to install?

That would allow us to have a bit of a clear divide between "try" and "install", while still keeping the initial install screen from feeling overly cluttered.

cassidyjames commented 5 years ago

@evertiro you can check the full design spec at https://github.com/elementary/installer/wiki/Try-or-Install. But essentially the reasoning is that adding another step before we can begin installing just slows the user down. We should be able to offer a few sane options here without it getting too cluttered up.

WatchMkr commented 5 years ago

@evertiro we'd like to continue the user down the most likely and preferred path. Installing being the primary purpose and using demo mode or the live session being ancillary.

WatchMkr commented 5 years ago

The Install Alongside feature needs some design direction. There was work done already but I can't find it. @cassidyjames do you have the original work on that? I don't see it on the wiki.

evertiro commented 5 years ago

Oh I 100% agree that things should be streamlined and as simple as possible for users. Unfortunately, as the screenshot posted by @brs17 demonstrates, that particular screen is far from simple... It's downright cluttered! I'm not saying that going with a completely separate window is the right solution, only that there needs to be some sort of division between "try" and "install" for the users who aren't already familiar with Linux.

brs17 commented 5 years ago

I understand your concern, let's take a look once the "try" button becomes a button like the "next" button and reaccess once we see screenshots.

EDIT: I think moving the Try button is what we need to clean it up...note not all the options appear all the time (they are dependent on what is already on the system)

evertiro commented 5 years ago

EDIT: I think moving the Try button is what we need to clean it up...note not all the options appear all the time (they are dependent on what is already on the system)

This may be true. I'd like to see what you're envisioning for the Try button though.

cassidyjames commented 5 years ago

@WatchMkr there wasn't much set in stone. I was actually working on this today in the elementary Slack. The original (i.e. really old) mockup looked like this:

mockup

I'm working on something (VERY in-progress) that looks like this:

image

cassidyjames commented 5 years ago

@WatchMkr or more Pop! styled:

image

I have a PR open mostly for a prototype UI at https://github.com/elementary/installer/pull/365

WatchMkr commented 5 years ago

That looks great. What if there are more than one OS’s installed?

cassidyjames commented 5 years ago

@WatchMkr the intent was that that's a different use case; a single other OS is probably common, even for a less advanced user, so we handle that case here. But having multiple OSes likely signals that the user is more comfortable with setting up their installs how they want, so Custom should still be sufficient.

The different scenarios are listed out at https://github.com/elementary/installer/wiki/Try-or-Install

WatchMkr commented 5 years ago

Sounds good.

mmstick commented 5 years ago

@WatchMkr That should work regardless of how many operating systems are installed, though the word would change depending on what is being shrunk. Possible install locations are:

mmstick commented 5 years ago

I've fixed the refresh installs. PR coming soon for distinst.

mmstick commented 5 years ago

This is what the install view looks like now, with all the options in place.

screenshot from 2018-11-08 11-30-22

evertiro commented 5 years ago

This is what the install view looks like now, with all the options in place.

screenshot from 2018-11-08 11-30-22

That looks a TON better! Though personally, I think the demo button is a bit understated.

brs17 commented 5 years ago

@cassidyjames: @WatchMkr, @mmstick, and I just sat down to discuss these new additions to the installer, we like the designs you've put up thus far (and vids we've seen in Slack).

@mmstick and @cassidyjames might be worth being in touch as you two make progress so we aren't reinventing the wheel :stuck_out_tongue:

mmstick commented 5 years ago

WIP changes to the views

screenshot from 2018-11-08 16-31-08 screenshot from 2018-11-08 17-09-50

mmstick commented 5 years ago

@brs17 This is ready for testing, though you'll have to compile this from source with the refresh-fixes branch of distinst installed.

brs17 commented 5 years ago

@mmstick why does it need to be compiled from source?

mmstick commented 5 years ago

@brs17 Because this won't build without the refresh-fixes version of distinst.

brs17 commented 5 years ago

Well then let's get https://github.com/pop-os/distinst/pull/165 in there as well.

mmstick commented 5 years ago

Now with OS logos:

screenshot from 2018-11-14 14-11-51

Once we get an icon for Pop in our icon theme, that can be displayed as well[0].

[0] https://github.com/pop-os/icon-theme/issues/39

mmstick commented 5 years ago

screenshot from 2018-11-14 15-26-20

brs17 commented 5 years ago

Testing

Template


Intel/AMD


NVIDIA


Testing "Refresh" Install

brs17 commented 5 years ago

~- [ ] In 18.04 the "refresh" icon is not very visible: EDIT: Fixed for 18.10 here with https://github.com/pop-os/gtk-theme/pull/248. Still an issue in 18.04.~

brs17 commented 5 years ago
mmstick commented 5 years ago

@brs17

On the "Resize OS" page, the left side of the installer ("Resize OS" text as well as Zoe image) is smaller than the left side content is on other pages.

Fixed by removing the elementary margins (48) from the content grid.

The password field presented when decrypting storage does not put focus on the password entry field (does not focus on any text box).

Fixed the focus for the select and unlock buttons, as well as their sensitivities. Also added some constraints to the PV name entry so that users won't be able to input invalid characters.

When looking at options available with Ubuntu already installed, when selecting "Alongside" or "Custom", the padding around the "Try Demo Mode" button changes (increases). Padding should be consistent.

Had the same issue as the partitioning view. That's fixed now.

Just tried an "Alongside" install of Pop!_OS 18.04 with Pop!_OS 18.04 and got this failure: installer2.log

This only happens with 18.04? For whatever reason, fsck failed with "Operation not permitted".

Installer logs for when refresh install failed of 18.10 on dualboot of 18.04 and 18.10: installer.log

Haven't been able to replicate so far. Will keep testing. I've added more context to the error messages in that region of distinst so that future logs will have a bit more detail on where this happened, exactly.

The minimum size of the new install was 14.3GB (0 bytes free)

Do you think 5GB of overhead for a new install would be enough?

I would think it might be more clear/easier to recognize an install from /dev/sdx rather than /dev/mapper/data-root. Is there anything that could be done here?

It's possible to resolve a LV to a VG, then to a PV, and then back to the original physical block device. I could add that behavior here.

When successfully doing an "Alongside OS" install, two efi boot partitions are created and the user must switch OSes from the bios boot menu. Do we want this to be the case, or should the two operating systems share a boot parititon?

Supporting this just involves copying the contents of the other EFI partitions to the new EFI partition, if it is safe to do so. I did experiment with installing to an existing EFI partition. Would need to add a check to determine whether it's possible to re-use an existing EFI partition (free space + overhead).

We might also want to increase the default size of the EFI partition to 1G if we're supporting multiple operating systems + older kernels on the same EFI partition as Pop. Pop by itself uses about 50% of the 500M EFI partition, which leaves enough room for Windows (100M), but would not be enough for a dual boot of Pop or another Linux OS.

mmstick commented 5 years ago

The help dialog should no longer open multiple instances now. GParted will also no longer open more than once if the modify button is clicked multiple times in advance.