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

Serpent patch policy for packages #489

Open
ermo opened this issue Dec 29, 2024 · 0 comments
Open

Serpent patch policy for packages #489

ermo opened this issue Dec 29, 2024 · 0 comments
Labels
type: documentation Improvements or additions to documentation.

Comments

@ermo
Copy link
Member

ermo commented Dec 29, 2024

Ikey Doherty
Also random one here
Reilly your PRs tend to carry an absolute fuckload of patches
It's incredibly difficult to review or justify
As in, I have nfc what they're supposed to be fixing and no record of having encountered the issues
Reilly Brogan
I try to include comments where the patch is applied or in the patch itself
Ikey Doherty
Right but I mean it seems like every single package you update gets a load of patches
Which is going to make updating them a bitch
I just want a way to track what's what
As we're meant to be staying close to upstream
I also want to ensure that whatever patches are in are directly relevant to serpent
And not being carried across from equivalent solus packaging?
Tldr anyone should be able to jump on any package in our repo trivially so we need a good way to track what's opinionated, build related, stateless, downstream or backported
Maybe dep3 tagging..?
ermo
https://dep-team.pages.debian.net/deps/dep3/ <- this dep3 tagging...?
Ikey Doherty
Aye
It's quite easy to just delete patches that don't apply anymore and not realise they're essential
Likewise it's easy to keep carrying patches indefinitely not realising they're maintaining a silent regression
Reilly Brogan

ermo
https://dep-team.pages.debian.net/deps/dep3/ <- this dep3 tagging...?

This looks great
I'll look through it soon and try to apply it to my patches. I do try to make sure that what the patches do and are for is documented but clearly that can always be better
(...)
ermo
If I could add one small comment that is close to my heart: IME, it is "nicer" for end users if packagers use well-commented default configs where the configs can yield the desired behaviour.

Hiding things away in patches (which could just as well have been done in configs) precludes people from discovering the system defaults and why they are like that in the first place.
Stateless patches ought, IMVHO, only enable us to ship good default configs.
I realise that this paradigm can lead to slightly higher maintenance costs, because it forces packagers to periodically check if config options are still relevant, whereas patches will fail vociferously if they no longer apply.
From a "make how the system works discoverable" perspective, this is a price I am personally willing to pay, if it means that more users get interested in changing defaults, which might eventually lead them to become packagers (so a strategy that supports the onboarding workflow).
But, again, personal perspective.
(could of course make the default config changes a big patch, which will then fail when the defaults change)
Ikey Doherty
Re packaging changes a preference I do strongly hold..
No butchery of source in the recipe
As it requires way more mental processing
Ie the sed hacks we sometimes resort on
Prefer a well formatted and explained patch where possible
I'm guilty of hacks fwiw just explaining my preferences ^^
(...)
ermo
As I see it, we have two goals:

  • Discoverability for end users
    • Prefer default config file changes over hidden patch changes for running system visibility
  • Maintainability for packagers
    • Prefer patches, but only absolutely necessary ones
    • Use dep3 format to annotate patches
    • Patch config files as well for easier discovery of upstream config changes via build failure

Joey Riches
i do think sed is okay in some circumstances ffiw
i.e. sed lib64 lib cmakelists.txt
ermo
Ikey Doherty: We need a scanner that looks for "forbidden by policy" contents in build phases methinks.
Stuff like sed / awk / %forbidden_in_production_macros
like, say %break_*
ermo

Joey Riches
i.e. sed lib64 lib cmakelists.txt

Either you allow sed, or you don't. The problem is discoverability, just like it is with patches; as in: "sed invocations can silently do nothing."
Reilly Brogan
Well, rust has this sed -e 's|@@LTO@@|thin-local|g' %(pkgdir)/config.toml.in > config.toml.pgo
Which should definitely be allowed
ermo
In that case, we have to resort to warnings for sed.
since we are not ever going to be able to assign actual meaning to sed invocations in a machine-parsing context unless we use GenAI to check it.
Joey Riches
on my side it's a laziness problem as well e.g. i find myself manually removing the git patch header to turn it into a raw diff then mangling the patch manually
rather than clone source, apply rebase

See serpent-os/tools#124 for a proposal on how to improve the patch-the-source UX/workflow to be more convenient.

@ermo ermo added the type: documentation Improvements or additions to documentation. label Dec 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation Improvements or additions to documentation.
Projects
None yet
Development

No branches or pull requests

1 participant