freebsd / poudriere

Port/Package build and test system
https://github.com/freebsd/poudriere/wiki
BSD 2-Clause "Simplified" License
379 stars 161 forks source link

Spurious rebuilds of subpackages #1113

Open jbeich opened 5 months ago

jbeich commented 5 months ago

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

fluffykhv commented 5 months ago

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 commented 5 months ago

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.

jbeich commented 5 months ago

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:

0EVSG commented 5 months ago

@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 commented 5 months ago

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

bdrewery commented 4 months ago

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

rrbrussell commented 4 months ago

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

bdrewery commented 4 months ago

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

bdrewery commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

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.