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

fix: Forbid breaking the system #1

Merged
merged 1 commit into from Oct 27, 2021
Merged

fix: Forbid breaking the system #1

merged 1 commit into from Oct 27, 2021

Conversation

mmstick
Copy link
Member

@mmstick mmstick commented Oct 27, 2021

Unless the system has a certain file in place

Unless the system has a file at `/etc/apt/break-my-system`
@mmstick mmstick requested review from a team October 27, 2021 13:42
Copy link
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

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

This works for exiting when the prompt would have been shown:

Screenshot from 2021-10-27 09-56-17

After touching the config file, the prompt is shown again:

Screenshot from 2021-10-27 09-56-27

With the config file present, the prompt still fails when typed incorrectly and succeeds when typed correctly. Tested on Hirsute and Impish.

@jackpot51 jackpot51 merged commit 03e0f3a into master Oct 27, 2021
@jackpot51 jackpot51 deleted the break branch October 27, 2021 16:17
@oliwarner
Copy link

The thing that stands out most, is how lost that warning message is in the HUGE list of packages. It's not the 90s so can't we (eg) tput colors to detect support and then paint a bright red banner around this so people get the point that they're probably doing something wrong?

@jacobgkau
Copy link
Member

jacobgkau commented Nov 9, 2021

can't we (eg) tput colors to detect support and then paint a bright red banner around this

We're aiming to make further improvements over in https://github.com/pop-os/shop so users don't have to fall back to the terminal in the first place, but I don't see any reason (from a UX standpoint) why we wouldn't accept something like this if you'd like to open a PR for it. Upstream apt is located at https://salsa.debian.org/apt-team/apt if you'd like to work on it there instead/in addition.

I'm not sure how much a red color would help beyond the already strongly-worded messaging, though. Some people will still proceed without understanding the consequences, which is why we no longer show the prompt by default.

@shivangswain
Copy link

shivangswain commented Nov 10, 2021

I'm not sure how much a red color would help beyond the already strongly-worded messaging, though.

The strongly worded message will atleast be identifiable/discernible through the wall of text if it is coloured another colour. Maybe the colour could also make the warning not seem trivial. Given apt already colours trivial things like the progress bar, this will only improve the experience.

@mmstick
Copy link
Member Author

mmstick commented Nov 10, 2021

I really don't think we should risk the possibility that they'll still continue to ignore it or think they can handle the consequences. I'm sure someone out there would go through with it.

@therealpxc
Copy link

therealpxc commented Nov 10, 2021

Would you guys mind adding something about this, or maybe a link to the forked APT repo on your Differences from Ubuntu page?

I remember I once rendered a Pop_OS system non-booting because I installed a different kernel, then assumed update-grub would be sufficient to plug it into the bootloader (since grub2-common was still present for some reason). Discovering that Pop is using systemd-boot in that fashion was a frustrating surprise I'd have been spared by consulting that handy article. :)

I might similarly be a little perplexed to discover only through experimentation that Pop_OS is doing things like marking desktop meta-packages as essential instead of just tools like dpkg and coreutils or whatever, and I might be very confused to ‘learn’ that the ability to remove essential packages had been removed from APT. But if all such Pop-specific changes are documented in one place, it would make Pop_OS feel more safe/familiar and predictable to me and other people who might be experienced with Debian and Ubuntu.

2¢ about the message text and what's a sufficiently severe warning: I think the kind of extra gate you've added here, which doesn't require patching or replacing APT to get around it, but also doesn't invite the user to override it by telling them right there in the prompt what they can do next to override it, is a great compromise.

@mmstick
Copy link
Member Author

mmstick commented Nov 10, 2021

I would hope that this is a change that Ubuntu and Debian alike would be willing to accept upstream, so it doesn't have to be a difference.

@RivenSkaye
Copy link

RivenSkaye commented Nov 10, 2021

I would hope that this is a change that Ubuntu and Debian alike would be willing to accept upstream, so it doesn't have to be a difference.

I sure hope not. It's fine if you decide to take a hand-holding approach for users that clearly have a good reason not to use your GUI, but users that feel confident enough to use an OS where a CLI is actually supposed to be used should not be limited in their freedom to make impactful choices because some inexperienced Pop users aren't able to heed a warning and feel the need to go in against very clear warnings.

Edit:
That said, I do think making the warning about potentially breaking changes a separate red box listing the impactful packages is a good idea that upstream could use as well

@mmstick
Copy link
Member Author

mmstick commented Nov 10, 2021

There is still a way to override it. And this is the behavior that every other package manager has. So I don't think it's the wrong thing to do.

@RivenSkaye
Copy link

Opened up my Arch WSL install and it seems to allow removing base just fine, as well as systemd.
Want me to try any other package managers to show this is, in fact, not default behavior for other package managers?
image
image

@mmstick
Copy link
Member Author

mmstick commented Nov 10, 2021

Sure, but we're talking about Arch. A system that essentially has nothing but a super minimal image on a terminal on first boot.

@RivenSkaye
Copy link

Does that mean it's any less of a Linux distro? Or that Pacman is not a package manager?
By all means do tell me how all package managers forbid users from making breaking changes to the system.

@mmstick
Copy link
Member Author

mmstick commented Nov 10, 2021

This is just cherry picking to make a point that I'm sure you know I just don't agree with. RPM distributions like Fedora don't allow this. If it works for them, it should be good enough for Debian.

@RivenSkaye
Copy link

Would uninstalling systemd on OpenSUSE suffice then? That's RPM based
image
image

It's not about me just not agreeing, this is about limiting the freedom that Linux was built upon for the sake of fixing a problem that wouldn't even exist if users were competent enough to actually read the warnings presented to them.

@mmstick
Copy link
Member Author

mmstick commented Nov 10, 2021

It's not limiting your freedom if you can override it, is it? This is just about making it more difficult for a newcomer (or even someone just not paying attention) accidentally wrecking their system and having a bad impression of the Linux desktop. I don't think we should be promoting that kind of experience.

@RivenSkaye
Copy link

Except this does limit user freedom because the only way to get it working is an undocumented file that needs to exist somewhere, without which users can't even go and set up a different DE! And if you'd keep this contained to just Pop, I wouldn't even mind. But no, there is now a suggestion to try and merge this hackjob fix upstream, making it affect other distros too.

All because someone on YouTube tried to install Steam and that caused conflicts and breakage.
The GUI env properly handled it by not allowing the Steam install, forcing usage of tooling that new users would call "advanced", at which point you should expect them to take note of program output and warnings.
And because a distro branded as "beginner friendly" breaks when users ignore several clear signs that something's wrong, it now has to force this undocumented 5 minute patchjob on users of other distros too, even though those users are generally much more aware of what they're doing.

@jackpot51
Copy link
Member

Working on upstreaming in https://salsa.debian.org/apt-team/apt/-/merge_requests/196

@raspi
Copy link

raspi commented Nov 10, 2021

Maybe it should be command line parameter such as --i-really-know-what-i-am-doing?

File exist solution might cause problems if disk is having some issues (corruption, mounted ro, ..) and you might be in recovery mode and you might not be able to create or delete said file.

@mmstick
Copy link
Member Author

mmstick commented Nov 10, 2021

At this point it's up to Debian's apt maintainer to make the choice. Hopefully we can do something good here for everyone.

@jackpot51
Copy link
Member

The recommendations from the Debian maintainer seem like cleaner changes to achieve the same effect.

@Happy86
Copy link

Happy86 commented Nov 10, 2021

Just to understand: Weird dependencies with steam are bricking the system.

Instead of trying to fix the dependencies (that basically uninstall the desktop env) this tries to make it even harder to break the system?

Or did I miss something? If yes can someone please point me to the issue fixing the dependencies on the steam package? :-)

It just sounds like: Instead of trying to fix the actual problem/issue only the symptom is being addressed.

Also while making it harder to brick your system is generally a good idea - not telling the user upfront how to do it anyway if you know what you are doing also seems like an exceptionally bad idea. :-(

Edit: Removed diff as the Debian Maintainer made similar recommendations.

@jacobgkau
Copy link
Member

It just sounds like: Instead of trying to fix the actual problem/issue only the symptom is being addressed.

That is not the case.

Or did I miss something? If yes can someone please point me to the issue fixing the dependencies on the steam package? :-)

There was never an issue with the Steam package. The issue was with the 32-bit binaries for some dependencies of Steam not being built properly for a short period of time. From my understanding, when Steam went to install the 32-bit binaries while they were not available from the Pop!_OS PPA, it fell back to the versions Ubuntu had available, which also (correctly) tried to install the matching Ubuntu versions of the 64-bit binaries. This caused system components that wanted the Pop!_OS versions to get removed, including pop-desktop. The first "fix" was getting a build server to successfully build those 32-bit binaries again; I'm not the one who did that, but I don't think it would have involved a GitHub issue page as it wasn't an issue with the code.

Even after the relevant builds were fixed, users could still run into the issue if they attempted to install Steam without installing system updates first. The second "fix" for this was releasing an updated iso to remove the "install updates" step for new installations, which was done a week ago.

The third "fix" in case a similar issue occurs again in the future is to ensure that users know to try installing updates if an application fails to install. The Pop!_Shop is being updated to add a more user-friendly message and button to install updates: see pop-os/shop#296 and pop-os/shop#302.

QA procedures have been adjusted to increase the reliability of Steam going forward. Humans already checked Steam manually before every mesa update; humans now also check Steam manually before every systemd update. Steam is also tested before new isos are released (although in this case, I don't believe this would have been an issue in the "bad" iso until the updates related to the failed i386 builds were published later.) Additionally, work is being done on automated testing via openQA to catch issues faster than humans, especially when issues may be influenced by packages coming from upstream Ubuntu that do not go through Pop!_OS QA.

This apt patch was a low-hanging fix intended as just one line of defense against future breakages. I hope this alleviates your concerns about the actual problem not being addressed.

@Happy86
Copy link

Happy86 commented Nov 11, 2021

Thank you very much for the explanation. :-)

This apt patch was a low-hanging fix intended as just one line of defense against future breakages. I hope this alleviates your concerns about the actual problem not being addressed.

Yes it does.

@mazunki
Copy link

mazunki commented Nov 11, 2021

I really have no reason to comment here, as I don't really use any of the distributions in place, but regardless: I disagree with Debian and Ubuntu having this no-break-policy by default, yet I do understand some people willing to have it.

My proposal would be to have a configuration file for apt at /etc/apt/apt.conf, with a line saying allow-break-my-system = true as a good compromise. This way a distribution or image maintainer could simply default it to whatever they want, and if you install a distribution which comes bundled with a DE already, it might be fair to change it in said ISO image to false.

Arbitrarily just having a file to set a configuration setting seems arbitrary, and I'm not entirely certain if it follows any particular standard.

@josephcsible
Copy link

Where is it documented that creating /etc/apt/break-my-system is how you override this? If someone got that error, and knew what they were doing so correctly wanted to proceed, but didn't already know about this PR, how could they ever figure that out?

@zorbathut
Copy link

Where is it documented that creating /etc/apt/break-my-system is how you override this? If someone got that error, and knew what they were doing so correctly wanted to proceed, but didn't already know about this PR, how could they ever figure that out?

I appreciate that you solved this problem yourself :)

For other people, note that as of this writing, the top hit on Google is a StackExchange post, posted almost exactly when you made this comment, made by "Joseph Sible-Reinstate Monica" which I'm just going to assume is the same person as josephcsible.

This honestly feels like the right solution; anyone unable to do a Google search for the exact error message really should not be bypassing the error message, but if you do that search, there's instructions right there (along with a rightfully terrifying filename.)

@josephcsible
Copy link

Yes, that was me. But I think this information should be available in our first-party documentation somewhere instead of people having to rely on third-party sites for it.

@J-Moravec
Copy link

Some suggestions:

  1. Hide the list of packages that are no longer required. It is a huge wall of text that takes away/distracts from the core problem. Set some reasonable limit past which the packages won't show and prompt it with "and 86 others.". Document this with: "To show all packages removed this way, type sudo apt remove xxx --show-all

  2. add info where one can list essential packages. Such as sudo apt list --essentials. Inform the user about this option with: To show essential packages, type: sudo apt list --essentials.

  3. Also add a message about the documentation of the force option. I don't know what might be the correct place, but point to some info about essential packages (i.e., what that means, how to enable the force option how to set package as essential etc.)

@tuxayo
Copy link

tuxayo commented Nov 14, 2021

Additionally, work is being done on automated testing via openQA to catch issues faster than humans

Automated GUI testing is a very good move. Thanks for your work!

@tuxayo
Copy link

tuxayo commented Nov 14, 2021

@RivenSkaye

fixing a problem that wouldn't even exist if users were competent enough to actually read the warnings presented to them.

That stuff (removing the UI with apt) even happened to two friends of mine that are using Linux since years. One for more than 10 and has done Debian packaging.
Anyway, many Linux beginners even on user-friendly distros will find at some point advice that tells to use apt. So such situations will happen and the first time we should expect those to not read the 100+ log lines. Even the scary part in the end. Actually the "Yes, do as I say!" isn't as explicit as the unread message about removing essential packages. On Windows there are these scary smart screen warnings when running exes (so a lot of the time when installing software) and on the web the invalid HTTPS certificate error. We are all trained to mechanically ignore some warnings. So the first time we see one, we don't know if it's expected or not.

So it's good to try to have more foolproof software.
There might be better ways that still let it easy to uninstall a desktop environment though. Is it though? What is the use case that would be hindered by the proposed fix here? I read about removing DEs but I'm not sure it's actually the case.
It seems that having package conflicts suggesting to removing a lot of important packages is a thing that apt does and not most other popular package managers. They still manage to not get in the way when removing a DE.

@binEpilo
Copy link

I have got a question about this topic. I tried the solution of putting the break-my-system file in /etc/apt, but somehow I still do not get the option to type "Yes, do as I say" when trying to remove essential packages. Is there a new way of circumventing the linus-proof?

@jacobgkau
Copy link
Member

jacobgkau commented Feb 16, 2023

I have got a question about this topic. I tried the solution of putting the break-my-system file in /etc/apt, but somehow I still do not get the option to type "Yes, do as I say" when trying to remove essential packages. Is there a new way of circumventing the linus-proof?

Yes, this quick-and-dirty fix was replaced with a solution in upstream apt: #4

The upstream merge requests were:

The new way to get around the safety is by passing the --allow-remove-essential option.

(There is no longer a "do as I say" prompt, as that was shown to be ineffective. Passing this option will simply show a warning and give you the usual Y/n prompt.)

@jacobgkau
Copy link
Member

Also, note that we currently ship Ubuntu's apt package in 22.04, so this GitHub repository does not reflect anything happening in Pop!_OS 22.04 right now. The only branches here end in _impish, _focal, and _bionic (for 21.10, 20.04, and 18.04).

@binEpilo
Copy link

Thanks that worked :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet