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

feat: reduce package scope and remove redundancies #544

Closed
wants to merge 3 commits into from

Conversation

RoyalOughtness
Copy link
Contributor

To be lazy consensus voted

@RoyalOughtness RoyalOughtness requested a review from castrojo as a code owner April 6, 2024 20:09
packages.json Outdated Show resolved Hide resolved
Copy link
Contributor

@fiftydinar fiftydinar left a comment

Choose a reason for hiding this comment

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

I suggested to remove nvtop too, since Mission Center exists, which uses this as a backend for GPU information.

"google-noto-sans-balinese-fonts",
"google-noto-sans-cjk-fonts",
"google-noto-sans-javanese-fonts",
"google-noto-sans-sundanese-fonts",
"grub2-tools-extra",
"heif-pixbuf-loader",
"htop",
Copy link
Contributor

Choose a reason for hiding this comment

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

This was added over a year ago ( #82 ) as an opinionated, lightweight helpful utility. The only concern I've ever seen about this is that it comes with a desktop file which shows up in DE app lists and some people seem to find that offensive.

I'm not inclined to remove it.

Copy link
Contributor

@fiftydinar fiftydinar Apr 7, 2024

Choose a reason for hiding this comment

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

The only concern I've ever seen about this is that it comes with a desktop file which shows up in DE app lists and some people seem to find that offensive.

This one can be easy solved with:
echo 'Hidden=true' >> /usr/share/applications/htop.desktop

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, I am fine with hiding the desktop icon if the group wishes to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, I am fine with hiding the desktop icon if the group wishes to do that.

I would also do that for nvtop too, if majority agrees.

Copy link
Member

Choose a reason for hiding this comment

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

I'm -1 for mangling .desktop files in main - I agree it's annoying, which is why we do it downstream. 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Also agreed, though, I'd suggest that be a distinct PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bsherman ultimately this PR is about giving main a well-defined scope, which it currently lacks. Without which, it is not clear at all what belongs here, upstream, or downstream.

If these packages are universally useful, they should be added in fedora comps.

Copy link
Contributor

Choose a reason for hiding this comment

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

@qoijjj I just addressed this in top-level comment.

@@ -42,7 +38,6 @@
"solaar-udev",
"symlinks",
"tcpdump",
"tmux",
Copy link
Contributor

Choose a reason for hiding this comment

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

This was added nearly a year ago ( #190 ) due to multiple contributors/users who believed it to be a useful, lightweight tool which is valuable on the host main images and who realized it was being added on several downstream images.

I'm not inclined to remove it.

Copy link
Member

Choose a reason for hiding this comment

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

Second this. Similar to vim this is a popular tool for folks and adds almost no size to the image to include it.

packages.json Show resolved Hide resolved
@@ -4,20 +4,16 @@
"all": [
"alsa-firmware",
"android-udev-rules",
"apr",
"apr-util",
Copy link
Contributor

Choose a reason for hiding this comment

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

APR was added specifically to support installations of Davinci Resolve ( #91 ) and was a nice example of a user providing a clean request for package support or alternative documentations.

There's no harm to keeping this, but removing it is reversing a past decision to save... 246k on a download?

I'm not sure I would support adding this today, but I'm not really in favor of removing support without a strong reason.

Copy link
Contributor

@fiftydinar fiftydinar Apr 7, 2024

Choose a reason for hiding this comment

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

Unofficial Davinci Resolve flatpak is in progress:
https://github.com/pobthebuilder/resolve-flatpak

davincibox also exists:
https://github.com/zelikos/davincibox

Copy link
Contributor Author

Choose a reason for hiding this comment

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

save... 246k on a download?

Image size is not the purpose of this PR 🙂

@bsherman
Copy link
Contributor

bsherman commented Apr 7, 2024

I probably could have just given a single comment, but I was initially trying to take each package removal on its own and give feedback.

Over time as contributors we've tightened the scope of main, as a result we became much more strict about what packages were ADDED to the main images. We also did remove some packages which were previously added, but usually removals of previously added packages were reserved for packages which caused some issue downstream.

Example, we removed kmods from the main images (see #369 ) because they prevented users and dowstream images from using alternative kernels. That was a a conflict which arguably reduced the main images' ability to support hardware (an in scope goal), but it was important for the main images to be useful as a common base.

So, despite tightened scope, which, if applied today may prevent adding some of packages which are currently included, if those pacakges don't cause downstream problems, our position up to now has been to be cautious about removals.

@fiftydinar
Copy link
Contributor

fiftydinar commented Apr 7, 2024

I think it's good to sort-out what should be removed indefinitely & what should be proposed to integrate into Fedora directly, so we can remove that package later, without making any regressions to the user (if Fedora agrees to the proposal).

@RoyalOughtness
Copy link
Contributor Author

RoyalOughtness commented Apr 7, 2024

So, despite tightened scope, which, if applied today may prevent adding some of packages which are currently included, if those pacakges don't cause downstream problems, our position up to now has been to be cautious about removals.

@bsherman I understand that, but it's confusing to contributors and users alike when there are say two packages, one included in ublue main and one excluded from it, yet both are out of scope on paper. It means that the true scope is functionally an everchanging and subjective matter, as opposed to an open and objective matter. While having a subjective and changing scope is one way to do things, it is fairly limiting.

Ideally this repo would have a clear line in the sand of what belongs in it, vs what should go upstream and what should go downstream. This line should be documented in the repo, enforced, and made clear to users and contributors, so that no one needs to ask whether something is in scope or not. It should be clear from the documentation and the existing changeset. As of today, this is not possible.

This was referenced Apr 7, 2024
@xynydev
Copy link
Member

xynydev commented Apr 7, 2024

I think it would be great if package.json allowed comments in the case that potentially out-of-scope packages are kept due to continuity. It could explain the reasoning behind keeping those packages and rejecting new ones. Sadly I don't think there's a comment-compatible parser command line config available by default on Fedora.

But anyway. The most important discussion here boils down to QoL package scope. Do we...?

  • only include codecs and hardware enablement
    • so no packages that Fedora could ship
    • and what does this mean wrt ujust
  • include codecs, hardware enablement, and packages / tweaks that...
    • need to be installed on the image or layered
    • are generally useful, expected, or required for a functional configuration
    • potentially stuff like adw-gtk3
  • include codecs, hardware enablement, and packages /tweaks that...
    • users might reasonably expect to be there
    • don't take too much disk space
    • fix problems without creating them
    • potentially stuff like vim and htop
  • include codecs, hardware enablement, and packages / tweaks that...
    • improve user experience in specific use cases
    • might be opionionated by nature
    • are things we find useful and suggest for users
    • are maybe things that Fedora should be doing upstream, but isn't
    • potentially stuff like distrobox and apr

Of course, this is an imperfect spectrum, and a lot of these things can overlap. But, as our goal is to refine the scope, I want to make sure we that think of the general scope 'rules' and their implications first, before getting too deep into how changes to the actual package set should be rolled out.

@RoyalOughtness
Copy link
Contributor Author

RoyalOughtness commented Apr 7, 2024

@xynydev

I want to make sure we that think of the general scope 'rules' and their implications first, before getting too deep into how changes to the actual package set should be rolled out.

@castrojo already documented the scope here: https://universal-blue.discourse.group/t/universal-blue-project-governance/51

Focus on things that Fedora CANNOT ship:
Hardware enablement.
Non-free codecs aka. “a bunch of that RPMFusion stuff”.
Built in Nvidia drivers with multiple version support.
We should be brutal about making things out of scope. 

About the comments though I agree with you. The purpose of this PR isn't to just tear out right away everything that isn't in scope. This was supposed to be the first of several, including only the least controversial removals

I agree that adding comments to the out of scope packages or something to that effect could help identify what needs to be removed long-term to bring this repo more in line with the "line in the sand" scope.

and what does this mean wrt ujust

ujust along with ujust/config provides hardware enablement functionality like for optimus laptops. It should fall under hardware enablement for that reason

are generally useful, expected, or required for a functional configuration

This should be upstream

users might reasonably expect to be there

upstream

don't take too much disk space

way too broad, this would mean installing every package under 20MB 😄

fix problems without creating them

Since packages added upstream cannot be removed downstream at this time, every out of scope package that's added adds a small downstream user inconvenience

potentially stuff like vim and htop

They should be added upstream if they have universal utility

might be opionionated by nature
are things we find useful and suggest for users
are maybe things that Fedora should be doing upstream, but isn't

these would belong downstream

@castrojo
Copy link
Member

castrojo commented Apr 7, 2024

It could explain the reasoning behind keeping those packages and rejecting new ones.

Most of the packages have associated issues that have been filed in the repo with a justification, etc.

@fiftydinar
Copy link
Contributor

@qoijjj What if upstream doesn't accept the package that belongs to them in your view? Should main image keep that package, or not ship it at all?

@RoyalOughtness
Copy link
Contributor Author

@fiftydinar If a package has universal utility and can be shipped by Fedora (i.e. no non-free stuff) then I would think we keep it here until upstream makes a decision and leave it in place with a link to the upstream issue if they say "no".

But each out of scope package should have a discussion with upstream linked to it so we can keep track of why/why not upstream is/isn't pulling in packages that are in scope upstream.

@RoyalOughtness
Copy link
Contributor Author

also @xynydev one other note is that json sadly doesn't support comments 🥲

@xynydev
Copy link
Member

xynydev commented Apr 8, 2024

ujust along with ujust/config provides hardware enablement functionality like for optimus laptops. It should fall under hardware enablement for that reason

I counted 27 non-duplicate upstream ujust recipes on my system. ~20 of them I would not count as hardware enablement, ex.:

  • bios # Boot into this device's BIOS/UEFI screen
  • distrobox-assemble CONTAINER="prompt" ACTION="create" FILE="/etc/distrobox/distrobox.ini" # Create distroboxes from a defined manifest
  • distrobox-new IMAGE="prompt" NAME="prompt" HOMEDIR="" # Create a new custom distrobox
  • install-brew # Install Homebrew | https://brew.sh
  • install-obs-studio-portable # Install obs-studio-portable from wimpysworld, which bundles an extensive collection of 3rd party plugins
  • logs-last-boot # Show all messages from last boot

Obviously, many of these are useful, but I don't know how they fit into this scope:

Focus on things that Fedora CANNOT ship:
Hardware enablement.
Non-free codecs aka. “a bunch of that RPMFusion stuff”.
Built in Nvidia drivers with multiple version support.

We should be brutal about making things out of scope.


And yes, I know my late-night thought dump was an "imperfect spectrum" as I said. I wouldn't nitpick every single line separately, but rather try to see what I'm trying to say with it. (or ask for clarification if that is not clear)

@fiftydinar
Copy link
Contributor

fiftydinar commented Apr 8, 2024

@xynydev
I think that ujusts is software enablement. It allows you to do useful things, without having to search what each command does. It's like simplified Yast, but for Fedora & in TUI.

It's most useful for easy troubleshooting imo.

I think that ujust is different in this regard, compared to classic bling packages, which affect downstream.
Ujust modifications by Universal Blue can be easily removed if desired, it doesn't harm downstream as out-of-scope packages do.

But it is indeed useful to know what precisely should go to ujust & what shouldn't.
So far, all scripts are relatively good supported & I find them great.

@RoyalOughtness
Copy link
Contributor Author

closing out old PRs

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.

6 participants