poudriere icon indicating copy to clipboard operation
poudriere copied to clipboard

Spurious rebuilds of subpackages

Open jbeich opened this issue 1 year ago • 14 comments

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

  • Host OS: 15.0-CURRENT amd64
  • Jail OS: any
  • Poudriere Version: e00503d846dc
  • Ports branch and revision: main@f1f2963f809f

jbeich avatar Jan 25 '24 01:01 jbeich

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

fluffykhv avatar Jan 31 '24 19:01 fluffykhv

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 avatar Feb 05 '24 21:02 bdrewery

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

jbeich avatar Feb 06 '24 01:02 jbeich

@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).

0EVSG avatar Feb 06 '24 03:02 0EVSG

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

bdrewery avatar Feb 07 '24 02:02 bdrewery

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

bdrewery avatar Feb 08 '24 03:02 bdrewery

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

rrbrussell avatar Feb 17 '24 17:02 rrbrussell

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

bdrewery avatar Feb 17 '24 17:02 bdrewery

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 avatar Feb 18 '24 21:02 bdrewery

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 avatar Feb 19 '24 00:02 bdrewery

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 avatar Feb 19 '24 00:02 bdrewery

@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 avatar Feb 19 '24 01:02 bdrewery

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.

bdrewery avatar Feb 19 '24 02:02 bdrewery

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.

pizzamig avatar Feb 19 '24 13:02 pizzamig