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

Spurious rebuilds of subpackages #1113

Open
jbeich opened this issue Jan 25, 2024 · 14 comments
Open

Spurious rebuilds of subpackages #1113

jbeich opened this issue Jan 25, 2024 · 14 comments

Comments

@jbeich
Copy link
Contributor

jbeich commented Jan 25, 2024

Describe the bug

poudriere always obsoletes existing packages via new dependency when the requested dependency moved to a different subpackage.

How to reproduce

  1. Apply WIPs for imlib2 (split libjxl), libdecor (split dbus), libei (split basu)
  2. Build some consumers e.g., xnotify (imlib2) and wlroots (libdecor + libei)
  3. Notice poudriere always tries to rebuild dependencies

Alternatively,

  1. In any port add SUBPACKAGES=pciids + RUN_DEPENDS.pciids=pciids>0:misc/pciids + bump PORTREVISION
  2. Try building the port more than once
  3. Notice poudriere always finds new dependency

Expected behavior

Subpackages are built only once.

Screenshots

$ poudriere bulk -j 132amd64 x11/xnotify x11-toolkits/wlroots
[...]
[00:00:03] Checking packages for incremental rebuild needs
[00:00:04] Deleting imlib2-1.12.1_2,2.pkg: new dependency: graphics/libjxl
[00:00:04] Deleting libdecor-0.2.2_1.pkg: new dependency: devel/dbus
[00:00:04] Deleting libei-1.2.0_1.pkg: new dependency: devel/basu
[00:00:04] Deleting libei-basu-1.2.0_1.pkg: new dependency: devel/libevdev
[00:00:05] Deleting xwayland-devel-21.0.99.1.664_1.pkg: new dependency: x11/libei~basu
[00:00:06] Deleting wlroots-0.17.1.pkg: missing dependency: xwayland-devel-21.0.99.1.664_1
[00:00:06] Deleting xnotify-0.9.3_1.pkg: missing dependency: imlib2-1.12.1_2,2
[00:00:06] Deleting stale symlinks... done
[00:00:06] Deleting empty directories... done
[...]

Environment

@jbeich jbeich added the bug label Jan 25, 2024
@fluffykhv
Copy link
Member

Just another 2¢

Outdated revisions of subpackages are still kept in repo directory instead of purging

$> ls libdecor*
libdecor-0.2.2_2.pkg
libdecor-cairo-0.2.2_1.pkg
libdecor-cairo-0.2.2_2.pkg
libdecor-examples-0.2.2_2.pkg
libdecor-gtk3-0.2.2_1.pkg
libdecor-gtk3-0.2.2_2.pkg

@bdrewery
Copy link
Member

bdrewery commented Feb 5, 2024

x11-toolkits/libdecor hits this too.

I think ports should be reverted until Poudriere is handling dependencies and cleanup properly. There are not even tests in Poudriere to keep this feature working.

@bdrewery bdrewery self-assigned this Feb 5, 2024
@jbeich
Copy link
Contributor Author

jbeich commented Feb 6, 2024

I think ports should be reverted until Poudriere is handling dependencies and cleanup properly.

Doesn't look critical for a revert but I'm biased:

  • both alsa-plugins and libdecor changes provide great benefit to binary packages users and rarely used in headless context
  • /latest package set has a lot of churn (recently and in general), so existing subpackages have near zero impact on the package cluster build times
  • portmgr@ already put landing more subpackages on pause, so local poudriere build times shouldn't get any worse
  • unsupervised builds are unaffected (options are still there), maybe also portmaster
  • dogfooding is important to uncover (more) bugs in various tools and avoid regressions due to future changes in ports and poudriere

@0EVSG
Copy link

0EVSG commented Feb 6, 2024

@jbeich, I have to agree with @bdrewery here - this makes life as a ports maintainer really frustrating. Whatever I do now, alsa-plugins makes poudriere want to rebuild half of the KDE stack (qt*-webengine) and chromium (pipewire).

@bdrewery
Copy link
Member

bdrewery commented Feb 7, 2024

I am working on adding tests and fixing this issue this week.

@bdrewery
Copy link
Member

bdrewery commented Feb 8, 2024

Note to self: The problem is probably in delete_old_pkg where I left the # XXX: dep_subpkg comment during review in df663bf.

@rrbrussell
Copy link

Removing the package tree with poudriere pkgclean -A -j jail does not fix the rebuild problem.

@bdrewery
Copy link
Member

I wouldn't expect it to. It's not a package problem. It's a logic problem.

@bdrewery
Copy link
Member

I thought this would be a simple fix but it is looking like it will be just as complicated FLAVORS handling was.
The problem is that each port returns all subpackages in 1 go with DEPENDS_ALL vars that include the depends for each subpackage. But we need to fetch each dependency list individually to properly handle incremental checks; the incremental checks need to know exactly which depends each package needs to determine if they changed but the information is not fetched. This will require reworking the queue used and complicating the implementation to be more like the FLAVORS handling. And removing the use of the DEPENDS_ALL variants. @pizzamig FYI

@bdrewery
Copy link
Member

I have a test to cover this now in a local branch. bulk-build-inc-subpackage.sh

[00:00:36] (00:00:00) stdout: [00:00:02] [Dry Run] Deleting port-test-0.0.2.pkg: new dependency: sysutils/pstree
[00:00:36] (00:00:00) stdout: [00:00:02] [Dry Run] Deleting port-test-0.0.2.pkg: current deps:  sysutils/pstree
[00:00:36] (00:00:00) stdout: [00:00:02] [Dry Run] Deleting port-test-0.0.2.pkg: package deps:
[00:00:37] (00:00:00) stderr: Asserting that only '' are in the dep='' queue
[00:00:37] (00:00:00) stderr: => Asserting that nothing else is in the dep='' queue
[00:00:37] (00:00:00) stderr: => Items remaining:
[00:00:37] (00:00:00) stderr: ==> ports-mgmt/subpkgtest port-test-0.0.2 listed
[00:00:37] (00:00:00) stderr: =>> 31679> /root/git/poudriere6/test/bulk-build-inc-subpackage.sh:51:_assert_bulk_queue_and_stats:20:assert_queued:69 TEST: '0' == '1'
[00:00:37] (00:00:00) stderr: =>> 31679> /root/git/poudriere6/test/bulk-build-inc-subpackage.sh:51:_assert_bulk_queue_and_stats:20:assert_queued:69 FAIL: Queue should be empty$
[00:00:37] (00:00:00) stderr: >>   expected '0$'
[00:00:37] (00:00:00) stderr: >>   actual   '1$'

@bdrewery
Copy link
Member

I have a fix for this that I am testing locally. I'll push it out in the next few days. I need to add a few more test cases.

@bdrewery
Copy link
Member

@bapt @pizzamig Say a subpackage gets a new dependency. Should all of the subpackages be deleted to rebuild? The main package? Or only the subpackage? I assume that all subpackages would be produced if the port is built. So we should delete them all. Yes?

@bdrewery
Copy link
Member

There is so much missing logic in incremental handling for subpackages. I fix 1 case and identify more cases. This is not a small problem. Nearly every check in delete_old_pkg is wrong.

Leaving this issue open/broken is the best course until all of the proper test cases are added and addressed. Otherwise we risk packages not rebuilding when they are supposed to. I am very busy with work lately. I will keep poking along on this but I don't expect a full solution soon.

I still think a revert is the best course of action. This was put in without proper support in any tool. It's not enough for Poudriere to simply produce a package. It needs to know how to handle it in incremental, pkgclean, etc. #1127 (pkgclean support missing). I'm sure more will be found as I add more tests.

@pizzamig
Copy link
Contributor

poudriere cannot build a single subpackage. The pattern is 1 build -> N packages.
If a subpackage change, the whole port needs to be rebuild anyway.

The approach I followed was to queue the origin (AKA port) to be built. The *_ALL variables provides all the needed dependencies.
Adding more logic only to fix delete_old_pkg seems a bit overkill to me.
In the gather queue, I'm adding extra logic to fetch data about subpackages, to be used in delete_old_pkg
It's a simple version of having a special queue.
However, I'm not sure I'll be able to cover all cases. poudriere has the hypotheses 1 build <-> 1 pkg rooted in many places.

freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Feb 21, 2024
freebsd/poudriere#1113
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=276845

Requested by:	makc (kde), bdrewery (pkgmgr)
Suggested by:	pizzamig (portmgr)

This reverts commit 06dbf1d.
freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Feb 21, 2024
freebsd/poudriere#1113
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=276845

Requested by:	makc (kde), bdrewery (pkgmgr)
Suggested by:	pizzamig (portmgr)

This reverts commit bd94cb1.
This reverts commit bb176de.
This reverts commit 1a1e431.
freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Feb 21, 2024
freebsd/poudriere#1113
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=276845

Requested by:	makc (kde), bdrewery (pkgmgr)
Suggested by:	pizzamig (portmgr)

This reverts commit 4b4539c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants