shiznix / unity-gentoo

A Gentoo overlay to install the Unity desktop
70 stars 13 forks source link

Ehooks break version control #251

Closed shiznix closed 3 years ago

shiznix commented 3 years ago

Hi @c4pp4

Huge thanks for stepping up to version stabilise Hirsute release, very much appreciated!

Cleaning up the repo removing packages and moving packages into ehooks looks like a great step forward.

How do you propose we keep version control and do version bumps between releases as we normally do with ./version_check.sh ?

c4pp4 commented 3 years ago

@shiznix Hi, I'm glad to see you back online :), I hope everythink is OK.

Version control of ehooks is not ideal. There are packages without need of control and packages blocked via unity-portage.pmask to prevent update fail - we must manually check if there is a new version and if it's compatible with ehook patches. I'm trying to find a better solution to control it and make a script to get output similar to this: "there is a new version, patches control passed, package bumped (unity-portage.pmask updated)... or patches control failed...". Patches control within the script can be made via 'ebuild prepare' command. I'm doing version control and I think it's not a big problem for me so far.

Apart from that I'm trying to find a better solution in case of manually generated emerge command via 'ehooks -c'.

I think 21.04 should be Hirsute: https://github.com/shiznix/unity-gentoo/releases/tag/Groovy

shiznix commented 3 years ago

Eek you're right, was supposed to be 20.10 (stable tag release Groovy before dropping it), will apply a fix shortly.

Thanks for concern, yes it was a tough time but it's good to be back after a couple of months. As you may or may not know was involved in a significant car accident that has kept me offline.

The problem where ehooks breaks version control is a topic we've spoken about before.

Until there is a solution to maintain version control, ehooks cannot be implemented globally and were only allowed for some peripheral non-core packages where convenient (simple or small patchsets that often apply across versions to large packages already maintained in main portage tree. eg. Chromium, Firefox, Libreoffice etc).

AFAICT the mass migration of versioned ebuilds to unversioned ehook patches seems to have occurred to some 54 packages in commit 9913323f46bcf2c0cf0f170c29148d6638c2e6fb under 'Stabilise Hirsute and drop Groovy' on June 18.

We'll need to re-instate most of these packages as versioned ebuilds back into their previous directories as per the standard portage tree structure. I say 'most' as a blind revert wouldn't be acceptable now as it would blow away the good work done after this commit.

At a minimum we'll need to start bringing back the following directories with full ebuilds and files/ directory patches: gnome-base/ gnome-extra/ sys-apps/accountsservice x11-libs/

One positive is we might use the opportunity to clean and drop maintaining some packages all together :)

So in summary we totally need the version control ebuilds give us. Not only for knowing when/where to bump but also for the purpose of maintaining two different release streams (1x stable + 1x developing).

Performing manual cross checking of patchsets versions as you're currently doing seems like a tedious and time expensive exercise in self harm that was easily solved by having an automated ebuild apply the correct versioned tarball patch, when all we need do is maintain the version and ensure it builds/runs.

Thanks! :)

shiznix commented 3 years ago

On another note to reduce maintenance, steer away from hand rolling our own patches, this leads to exponential maintenance down the road. eg. Hirsute has Vala-0.48 (not 0.52) as a maximum version (see https://packages.ubuntu.com/search?suite=hirsute&keywords=libvala).

Specifying the correct VALA_MIN/MAX_API_VERSION in ubuntu-versionator.eclass steers around problems such as seen in issue #250 and eliminates the need for custom patching (and maintenance thereafter). If a higher or lower version is needed by a particular package, specifying it in the package's ebuild overrides the eclass (see media-gfx/shotwell and unity-lenses/unity-lens-music ebuilds).

Thanks again for all your efforts, really is appreciated! :)

c4pp4 commented 3 years ago

Give me a list of packages you want to bring back to ebuilds, it's easy to do it. The reason of changes to ehooks was that I find out that many ebuilds are just copies of old main tree ebuilds, not updated to newer ones and that we are just providing patches from debian archive files.

c4pp4 commented 3 years ago

I have forked overlay and I'm working on ehooks separately. Sorry for the problems.

shiznix commented 3 years ago

Give me a list of packages you want to bring back to ebuilds, it's easy to do it. The reason of changes to ehooks was that I find out that many ebuilds are just copies of old main tree ebuilds, not updated to newer ones and that we are just providing patches from debian archive files.

Thanks, we'll need all those packages that were located in the directories mentioned above prior to stabilising Hirsute: gnome-base/ gnome-extra/ sys-apps/accountsservice x11-libs/

Especially important we keep Gnome versions tied to their relevant Ubuntu release instead of hinging on the whim of a Gentoo maintainer which ehooks would do (available main tree Gnome versions can lag on updates or be dropped prematurely).

I have forked overlay and I'm working on ehooks separately. Sorry for the problems.

Don't feel sorry, this is only one tiny problem in comparison to all your other great contributions. Your intentions are good and I totally get the Pro's for including ehooks to remove maintaining ebuilds who's content can become outdated over several version bumps.

If it weren't for version control maintenance we might happily make everything an ehook however, I've had another Gentoo maintainer in IRC last week quietly question our use of ehooks as bending the portage overlay standard (not using standard ebuilds but using the overlay to insert patches into main tree ebuilds).

Which it probably does but in the back of mind raises alarm bells that if that line of thought takes flight with more maintainers it may lead to them implementing a change to kill ehooks.

So on that I'd be using them extra sparingly rather than risk being seen abusing it, rely on them wholesale and give them enough reason in their eyes to see ehooks killed off.

An example to illustrate their argument is looking at where ebuilds appear to be installed from when relying on ehooks: # qlist -IRv x11-libs/gtk+-3 x11-libs/gtk+-3.24.29::gentoo

Versus the large amount of changes made to the installed package that misleadingly appears to be from main Portage tree: # ls -l /var/lib/layman/unity-gentoo/profiles/ehooks/x11-libs/gtk+\:3/files/patches 0001-calendar-always-emit-day-selected-once.patch 0001-gtk-reftest-Force-icon-theme-to-Adwaita.patch 0001-gtkwindow-set-transparent-background-color.patch 016_no_offscreen_widgets_grabbing.patch 017_no_offscreen_device_grabbing.patch 060_ignore-random-icons.patch 073_treeview_almost_fixed.patch bzg_gtkcellrenderer_grabbing_modifier.patch cssshadowvalue-Apply-device-scale-to-the-offset-when.patch demos-examples-tests-Don-t-distribute-built-files.patch Disable-accessibility-dump-aka-a11ytests-test.patch gdk-Don-t-distribute-generated-files-in-tarballs.patch git_imcontext.patch git_wayland_fonts.patch gtk-Really-don-t-distribute-built-files.patch message-dialog-restore-traditional-look-on-unity.patch print-dialog-show-options-of-remote-dnssd-printers.patch reftest-known-fail.patch series ubuntu_gtk_custom_menu_items.patch uimanager-guard-against-nested-node-updates.patch unity-border-radius.patch unity-headerbar-maximized-mode.patch updateiconcache-Sort-list-of-entries.patch x11-dnd-Ignore-XErrors-from-the-COW.patch x-canonical-accel.patch

I can see it becoming an issue for them of security and bug tracking as the reason for either killing the ability to implement ehooks entirely or asking we remove them or be removed from layman's approved repo list.

Anyway, just my thoughts. Really hope your forking means we can still work together on this.

Cheers! :)

c4pp4 commented 3 years ago

Don't worry, I'm not going to quit unity-gentoo. :)

I know what you mean but:

bending the portage overlay standard (not using standard ebuilds but using the overlay to insert patches into main tree ebuilds).

Ehooks are based on wiki - chapter - "Enabling /etc/portage/patches for all ebuilds" from https://wiki.gentoo.org/wiki//etc/portage/patches#Enabling_.2Fetc.2Fportage.2Fpatches_for_all_ebuilds (original version of site with pre_srcprepare function is archived here: https://web.archive.org/web/20191226202345/https://wiki.gentoo.org/wiki//etc/portage/patches#Enabling.2Fetc.2Fportage.2Fpatches_for_all_ebuilds)

Versus the large amount of changes made to the installed package that misleadingly appears to be from main Portage tree

Gentoo is a system for users who know what they are doing. I think tt's sufficient for users to have informations in NOTES file, users can disable or change our ehooks and patches to their needs via EHOOK_PATH and we highlight ehooks changes like this:

 * gtk+-3.24.29.tar.xz BLAKE2B SHA512 size ;-) ...                                                                                                                                                   [ ok ]
 * checking ebuild checksums ;-) ...                                                                                                                                                                 [ ok ]
 * checking auxfile checksums ;-) ...                                                                                                                                                                [ ok ]
 * checking miscfile checksums ;-) ...                                                                                                                                                               [ ok ]
>>> Unpacking source...
>>> Unpacking gtk+-3.24.29.tar.xz to /var/tmp/portage/x11-libs/gtk+-3.24.29/work
>>> Source unpacked in /var/tmp/portage/x11-libs/gtk+-3.24.29/work

>>> Loading unity-gentoo ebuild hooks from /var/lib/layman/unity-gentoo/profiles/ehooks/x11-libs/gtk+:3 ...

 * Processing 01-pre_src_prepare.ehook ...
 * Applying gdk-Don-t-distribute-generated-files-in-tarballs.patch ...                                                                                                                               [ ok ]
 * Applying gtk-Really-don-t-distribute-built-files.patch ...                                                                                                                                        [ ok ]
 * Applying demos-examples-tests-Don-t-distribute-built-files.patch ...                                                                                                                              [ ok ]
 * Applying 016_no_offscreen_widgets_grabbing.patch ...                                                                                                                                              [ ok ]
 * Applying 017_no_offscreen_device_grabbing.patch ...                                                                                                                                               [ ok ]
 * Applying 060_ignore-random-icons.patch ...                                                                                                                                                        [ ok ]
 * Applying reftest-known-fail.patch ...                                                                                                                                                             [ ok ]
 * Applying Disable-accessibility-dump-aka-a11ytests-test.patch ...                                                                                                                                  [ ok ]
 * Applying 073_treeview_almost_fixed.patch ...                                                                                                                                                      [ ok ]
 * Applying bzg_gtkcellrenderer_grabbing_modifier.patch ...                                                                                                                                          [ ok ]
 * Applying ubuntu_gtk_custom_menu_items.patch ...                                                                                                                                                   [ ok ]
 * Applying print-dialog-show-options-of-remote-dnssd-printers.patch ...                                                                                                                             [ ok ]
 * Applying uimanager-guard-against-nested-node-updates.patch ...                                                                                                                                    [ ok ]
 * Applying x-canonical-accel.patch ...                                                                                                                                                              [ ok ]
 * Applying message-dialog-restore-traditional-look-on-unity.patch ...                                                                                                                               [ ok ]
 * Applying 0001-gtk-reftest-Force-icon-theme-to-Adwaita.patch ...                                                                                                                                   [ ok ]
 * Applying 0001-calendar-always-emit-day-selected-once.patch ...                                                                                                                                    [ ok ]
 * Applying 0001-gtkwindow-set-transparent-background-color.patch ...                                                                                                                                [ ok ]
 * Applying unity-border-radius.patch ...                                                                                                                                                            [ ok ]
 * Applying unity-headerbar-maximized-mode.patch ...                                                                                                                                                 [ ok ]
 * Applying gtk+-3.24.12-revert_gtkmenu_gtkwindow.diff ...                                                                                                                                           [ ok ]

>>> Done.

>>> Preparing source in /var/tmp/portage/x11-libs/gtk+-3.24.29/work/gtk+-3.24.29 ...
 * Applying gtk+-3.24.25-update-icon-cache.patch ...                                                                                                                                                 [ ok ]
 * Applying gtk+-3.22.20-libcloudproviders-automagic.patch ...
shiznix commented 3 years ago

No arguments there, /etc/portage/patches is a good defence.

I'd be willing to bet though since 2x devs have already raised the question on it's use (usually after someone strolls into #gentoo looking for support), they'd consider it's blind use at an overlay level as bordering unacceptable.

'blind use' would be an easy claim for them as /etc/portage/patches/... requires user intervention to manually place patches, and being able to see the overlay applying patches as you've shown above also requires user intervention by setting EMERGE_DEFAULT_OPTS="--quiet-build=n" in make.conf

Note I'm just playing devil's advocate here to perhaps cover off which way they might head.

I do think the replication of /etc/portage/patches in overlay and how ehooks has made that shine is extremely good to keep for certain packages (version control withstanding), I just fear it's overuse might spell it's end given it's use has been highlighted already.

For now I think we play it careful, keep using ehooks for those large final application type ebuilds with simple patchsets (chromium,thunderbird,firefox,libreoffice) and have everything else as normal visible ebuilds under standard structure.

Be interested to see what you can come up with on the version control side for ehooks with patch testing.

c4pp4 commented 3 years ago

Revert done... df0d1118e673214575969ce0c09f1e9ddf785283

I can change the color of the line (>>> Loading unity-gentoo ebuild hooks) to red and I can add some "press any key to continue" command or something like that with configurable flag to suppress such behavior if someone have a problem with it, to be safe.

shiznix commented 3 years ago

Thanks @c4pp4 :)

I can change the color of the line (>>> Loading unity-gentoo ebuild hooks) to red and I can add some "press any key to continue" command or something like that with configurable flag to suppress such behavior if someone have a problem with it, to be safe.

Sure thing, sounds like an improvement.

c4pp4 commented 3 years ago

Ehooks version control: 1) I've removed 'patches' directory from single ehooks, now it downloads debian archive file when applying ehook and the patches are applied from it, downloading is enabled via env FEATURES="network-sandbox-proxy -network-sandbox" set for a single package, it's checked via BLAKE2 checksum after download: e.g. rhythmbox:

ebuild_hook() {
    local \
        blake=bee02acb3ccb6f530b92c685365470f93caee9a37a7fca937232074dc68c0713e45d7a6c37c5d10485e00fa642e594e6d26f10fd30f8f7c82fe48cb06f82826b \
        uver=rhythmbox_3.4.4-1ubuntu4

    source "${EHOOK_PATH}"/templates/fetch_debian.fnc
    fetch_debian "${uver}" "${blake}"
}

Bumping version of debian archive if needed is very easy, it suffices to change uver variable and then launch ehook -c media-sound/rhythmbox inside dev repo, it automatically refreshes blake checksum (or ehook -c to refresh all ehooks of that kind).

2) unity-portage.pmask ehooks related entries are updated via ehooks -u, it automatically test patches before changing some entry (via ebuild ..... prepare), e.g.:

Looking for gentoo main portage updates... done!

 * Updates available:

 * Update from www-client/firefox-89.0.2 to www-client/firefox-90.0
 * Test command 'ebuild $(portageq get_repo_path / gentoo)/www-client/firefox/firefox-90.0.ebuild clean prepare'... [failed] and unity-portage.pmask entry... [not updated]

 * Update from x11-terms/gnome-terminal-3.40.1 to x11-terms/gnome-terminal-3.40.2
 * Test command 'ebuild $(portageq get_repo_path / gentoo)/x11-terms/gnome-terminal/gnome-terminal-3.40.2.ebuild clean prepare'... [passed] and 'unity-portage.pmask' entry... [updated]

3) unity-extra/ehooks package now generates emerge command for all ehooks changes via pkg_postinst and log it as ewarn. 'All ehooks changes' mean if we change something from ehooks tree (patches, ehook...) or if user change some use flag from unity-extra/ehooks package. We can bump unity-extra/ehooks package to immediately show some new tree changes.

It's usable now but I'm still testing it for sure.

c4pp4 commented 3 years ago

Ehooks warning:

>>> Emerging (1 of 1) media-gfx/gnome-screenshot-40.0-r1::gentoo
 * gnome-screenshot-40.0.tar.xz BLAKE2B SHA512 size ;-) ...                                                                                                                                          [ ok ]
>>> Unpacking source...
>>> Unpacking gnome-screenshot-40.0.tar.xz to /var/tmp/portage/media-gfx/gnome-screenshot-40.0-r1/work
>>> Source unpacked in /var/tmp/portage/media-gfx/gnome-screenshot-40.0-r1/work
 * 
 * Warning!
 * 
 * Patches from unity-gentoo-fork overlay will be applied to media-gfx/gnome-screenshot and some other packages from the Gentoo tree via ebuild hooks patching system.
 * For more details see README_ehooks.txt chapter I.
 * Set EHOOK_ACCEPT="yes" in make.conf to confirm you agree with it.
 * 
 * ERROR: media-gfx/gnome-screenshot-40.0-r1::gentoo failed (prepare phase):
 *   Acceptance needed
 * 
 * Call stack:
 *        ebuild.sh, line 127:  Called pre_src_prepare
 *   profile.bashrc, line 245:  Called __ehook_apply
 *      environment, line 383:  Called die
 * The specific snippet of code:
 *           die "Acceptance needed";
 * 
 * If you need support, post the output of `emerge --info '=media-gfx/gnome-screenshot-40.0-r1::gentoo'`,
 * the complete build log and the output of `emerge -pqv '=media-gfx/gnome-screenshot-40.0-r1::gentoo'`.
 * The complete build log is located at '/var/tmp/portage/media-gfx/gnome-screenshot-40.0-r1/temp/build.log'.
 * The ebuild environment file is located at '/var/tmp/portage/media-gfx/gnome-screenshot-40.0-r1/temp/environment'.
 * Working directory: '/var/tmp/portage/media-gfx/gnome-screenshot-40.0-r1/work/gnome-screenshot-40.0'
 * S: '/var/tmp/portage/media-gfx/gnome-screenshot-40.0-r1/work/gnome-screenshot-40.0'
 * Please submit ehook bug at 'https://github.com/c4pp4/unity-gentoo-fork/issues'.
 * The ehook log is located at '/var/tmp/portage/media-gfx/gnome-screenshot-40.0-r1/temp/ehook.log'.

>>> Failed to emerge media-gfx/gnome-screenshot-40.0-r1, Log file:

>>>  '/var/tmp/portage/media-gfx/gnome-screenshot-40.0-r1/temp/build.log'