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

Reorganize into 4 metapackage levels #113

Merged
merged 49 commits into from
Jul 26, 2023
Merged

Conversation

13r0ck
Copy link
Contributor

@13r0ck 13r0ck commented Apr 4, 2023

Allows for more ease when building pop-os containers and/or server images.
Also has the added benefit of being able to drop some ubuntu dependencies.

Requires: pop-os/default-settings#169

@13r0ck 13r0ck force-pushed the metapackage-reorg branch 2 times, most recently from 6e1ced6 to 735354f Compare April 4, 2023 18:30
13r0ck added a commit to pop-os/default-settings that referenced this pull request Apr 5, 2023
@13r0ck 13r0ck force-pushed the metapackage-reorg branch 6 times, most recently from 1cd68f0 to 09ccc53 Compare April 6, 2023 02:53
@13r0ck 13r0ck marked this pull request as ready for review April 6, 2023 03:10
@13r0ck 13r0ck requested review from a team April 6, 2023 03:10
@13r0ck
Copy link
Contributor Author

13r0ck commented Apr 6, 2023

Yes I did shamelessly add neovim, but only as a suggested application, so it wont install on existing installs, just new clean installs
Edit: not anymore

@n3m0-22 n3m0-22 self-assigned this Apr 6, 2023
@13r0ck
Copy link
Contributor Author

13r0ck commented Apr 6, 2023

  1. ubuntu-advantage-tools is not autoremoved by this. Though it should be removed in OS upgrades (maybe refresh installs too?). We could conflict the package and remove it, but that might be too heavy handed?

  2. As well I split pipewire dependencies into a separate metapackage pop-pipewire. It is still a dependency, so it really doesn't do anything, but the thinking was to start the conversation of, moving that to recommends would let someone install pulseaudio on pop os if they really wanted to. We may not want to allow that? Or maybe there is another group of packages that we may want to make optional in the same way?

  3. Our pop-container-runtime is pretty big. It's the same size as the ubuntu container + our PPAs and the pop-keyring package. Any ideas how we could shink that down? If we have a fork of the apt package we could make adduser a recommended package. That would then let us remove quite a few packages from the container. Other ideas?

n3m0-22
n3m0-22 previously approved these changes Apr 6, 2023
Copy link
Contributor

@n3m0-22 n3m0-22 left a comment

Choose a reason for hiding this comment

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

I have seen no regression testing this. Checking with apt show pop-container-runtime or apt show pop-desktop before and after this change clearly shows the difference.

I have tested on clean, refresh, and active installs of 22.04. Depends and Recommends are matching the changes. Looks good to me.

debian/control Outdated Show resolved Hide resolved
@jacobgkau jacobgkau dismissed their stale review April 6, 2023 23:29

Typo was addressed

@n3m0-22 n3m0-22 self-requested a review April 7, 2023 00:47
@jacobgkau
Copy link
Member

jacobgkau commented Apr 7, 2023

Just curious about a couple of things:

  • As we now have a pop-de-gnome and there's a pop-community-de-kde being acknowledged here, do we plan to have a pop-de-cosmic in the future? Currently, cosmic-session pulls in everything needed for COSMIC. I suppose a separate metapackage to add the apps on top isn't necessarily a bad thing, just wanted to know if there's a plan for the naming going forward.
  • I was concerned about pop-de-gnome being auto-replaced with pop-community-de-kde in situations where it would have previously refused to be removed. @n3m0-22 tested that and found that it's not happening, which is good (although it could still be a concern going forward once the KDE package is actually named to match what's being depended on here.) It looks like we have pop-de-gnome being marked as essential... it seems like the "or" dependency of pop-desktop on pop-de-gnome | pop-community-de-kde isn't really an "or" if pop-de-gnome is essential and can't be removed for that reason. Am I missing something with how this is supposed to work in the future?

@13r0ck
Copy link
Contributor Author

13r0ck commented Apr 7, 2023

As we now have a pop-de-gnome ...

That was my thinking. I'm open to name change suggestions

I was concerned about pop-de-gnome ...

I will remove the kde reference from this PR before this is released, as the timing isn't going to line up for that to make sense here.

Thinking about it though, "or" doesn't really work there. A user may want more than one de installed. So the "or" should probably be on the display manager, then a virtual package of at least one DE (gnome/cosmic/kde).
"or" also has the unfortunate side effect with display managers where it causes the display to go black while one is removed and the other is installed...
I will revisit that implementation.

debian/control Outdated Show resolved Hide resolved
@13r0ck 13r0ck marked this pull request as draft April 19, 2023 17:27
@13r0ck
Copy link
Contributor Author

13r0ck commented Apr 25, 2023

Does it make sense for policykit-desktop-privleges to be in the DE packages?

@13r0ck
Copy link
Contributor Author

13r0ck commented Jun 15, 2023

I also don't think that using ubuntu for testing #107 is quite the way to go. For my testing I created a ISO using the new staging branch PR, where it installed pop-server and pop-default-settings (with STAGING_BRANCHES=metapackage-reorg). Launched that ISO, then installed pop-desktop

@13r0ck
Copy link
Contributor Author

13r0ck commented Jun 15, 2023

please also check that an iso created with pop-os/iso#321 works as intended before this is approved. And please take your time and be thorough testing this. I really don't want to cause packaging conflicts because of this 😄

@thomas-zimmerman
Copy link

With this reorganization, can we bring hplip and libfuse2 back a base dependencies that are installed by default.

  • hplip for basic HP printer support without needing to install packages
  • libfuse2 is needed for AppImage support

@jacobgkau
Copy link
Member

It might be best to get the transition dealt with before we go adding new dependencies, but I think libfuse2 as a depends and hplip as at least a recommends would be reasonable to consider.

@jacobgkau
Copy link
Member

The upstream ubuntu-standard depends on hdparm. On a System76 machine, hdparm is still installed after adding this branch and running sudo apt autoremove --purge because system76-driver depends on pm-utils, and pm-utils recommends hdparm. However, in a virtual machine without system76-driver installed, hdparm is not depended on or recommended by anything, so it's autoremoved.

This may be a problem because system76-power (which is installed via pop-session -> gnome-shell-extension-system76-power) calls hdparm in one of its disk functions. However, looking at pop-os/system76-power#261, it seems like system76-power may not actually use that function anymore.

@mmstick Do you think we should make system76-power depend on hdparm, should we add it to one of the metapackages here, or is it okay for that not to be present on some systems?

@mmstick
Copy link
Member

mmstick commented Jul 7, 2023

It's fine if hdparm is removed since the code was never called. I've created a PR to remove it.

@jacobgkau
Copy link
Member

One of the other packages that is currently autoremovable after installation is friendly-recovery. This package doesn't seem to be relevant on EFI installations because we don't include that type of recovery option in the systemd-boot menu (instead using our Recovery partition setup.) However, the package is relevant on legacy BIOS installations because recovery mode is available via the GRUB menu. I'm adding it back to pop-container-interactive to correspond to ubuntu-standard.

jacobgkau
jacobgkau previously approved these changes Jul 11, 2023
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 update installs fine with the Pop!_Shop or a regular sudo apt upgrade.

  • This appears as an update to the packages pop-desktop, ubuntu-minimal, and ubuntu-standard. Installing the updates also installs the new packages pop-container-interactive, pop-container-runtime, pop-de-gnome, pop-pipewire, and pop-server.

  • After the updates are installed, running sudo apt autoremove --purge will remove the old dependencies hdparm, plymouth-theme-ubuntu-text, ubuntu-release-upgrader-core, and update-manager-core.

    • The latter two packages are currently coming from our pop-upgrade repository, but simply provide pointer messages to use pop-upgrade instead, so having them removed is fine. We would want to consider adding them back as depends or recommends to the pop-upgrade package if we find people get confused about why the do-release-upgrade command doesn't exist, although installing ubuntu-release-upgrader-core back as suggested by command-not-found will still result in getting our modified version with the pop-upgrade message.
    • On previous tests, I thought I saw ubuntu-minimal and ubuntu-standard also get autoremoved, but right now, I'm seeing them stay since they're marked as manually installed. That's not a problem, since they don't do anything as empty transitional packages and can be removed manually.
  • Installing this update does pull in any recommends that were previously removed and are now changing between packages-- e.g., if someone manually removed Firefox (currently a recommends of pop-desktop), it will now be installed again as a recommends of pop-de-gnome. Those packages can be removed manually again after the upgrade.

  • Regarding Switch pop-desktop from Essential to Protected to prevent auto-install #107, it's working as expected. Removing the pop-desktop package still requires passing the --allow-remove-essential option. On a system without a GUI running pop-server, if the main Pop!_OS repo is disabled (so the essential version of pop-desktop isn't available), then running sudo apt full-upgrade does not attempt to install pop-desktop again.

  • I'm very confident this will not cause any breakage, and I think we should go ahead and move forward with it. I'm still not 100% sure the pop-de-gnome package is semantically better than including those dependencies in pop-session, but things can still be further moved around later as we get closer to releasing COSMIC Epoch and we know how we're going to want to package that/what we'll want to be included in Pop!_OS.

    • I was questioning whether firefox remain part of pop-desktop instead of being moved to pop-de-gnome, but it does make sense that e.g. if we had a community KDE option, it could have Falkon instead, a future non-COSMIC GNOME option could have GNOME Web instead, etc.

mmstick
mmstick previously approved these changes Jul 11, 2023
@mmstick mmstick dismissed stale reviews from jacobgkau and themself via 501f7e5 July 11, 2023 14:40
jacobgkau
jacobgkau previously approved these changes Jul 11, 2023
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.

Ok, this now pulls in libfuse2 in addition to the other mentioned packages. Still installs with a normal apt upgrade, no full-upgrade required. Everything else is behaving the same as previously described.

debian/control Outdated Show resolved Hide resolved
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.

command-not-found is now able to be removed, in addition to the previous reviews' notes.

@mmstick mmstick merged commit 191b519 into master_jammy Jul 26, 2023
@mmstick mmstick deleted the metapackage-reorg branch July 26, 2023 15:33
jacobgkau pushed a commit to pop-os/default-settings that referenced this pull request Jul 26, 2023
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.

7 participants