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

Subpackages use indeterminate queue #1121

Open bdrewery opened 4 months ago

bdrewery commented 4 months ago

https://github.com/freebsd/poudriere/commit/270784e04a8efd0e83178ab19cd0a2e876fc3905

@@ -6988,7 +7040,7 @@ gather_port_vars_process_depqueue() {
        [ $# -eq 1 ] || eargs gather_port_vars_process_depqueue originspec
        local originspec="$1"
        local origin pkgname deps dep_origin
-       local dep_originspec dep_flavor queue rdep
+       local dep_originspec dep_flavor dep_subpkg queue rdep
        local fd_devnull

        msg_debug "gather_port_vars_process_depqueue (${COLOR_PORT}${originspec}${COLOR_RESET})"
@@ -7006,13 +7058,13 @@ gather_port_vars_process_depqueue() {
                fd_devnull=5
        fi

-       originspec_decode "${originspec}" origin ''
        for dep_originspec in ${deps}; do
-               originspec_decode "${dep_originspec}" dep_origin dep_flavor
+               originspec_decode "${dep_originspec}" dep_origin dep_flavor dep_subpkg
                # First queue the default origin into the gatherqueue if
                # needed.  For the -a case we're guaranteed to already
                # have done this via the category Makefiles.
-               if [ ${ALL} -eq 0 ]; then
+               # if it's a subpackage process it later
+               if [ ${ALL} -eq 0 ] && [ -z "${dep_subpkg}" ]; then
                        if [ -n "${dep_flavor}" ]; then
                                queue=mqueue
                                rdep="metadata ${dep_flavor} ${originspec}"
@@ -7027,6 +7079,12 @@ gather_port_vars_process_depqueue() {
                            "${rdep}"
                fi

+               if [ -z "${dep_flavor}" ] && [ -n "${dep_subpkg}" ]; then
+                       msg_debug "Want to enqueue ${COLOR_PORT}${dep_originspec}${COLOR_RESET} rdep=${COLOR_PORT}${origin}${COLOR_RESET} into ${queue}"
+                       gather_port_vars_process_depqueue_enqueue \
+                           "${originspec}" "${dep_originspec}" "${queue}" \
+                           "${originspec}"
+               fi
                # Add FLAVOR dependencies into the flavorqueue.
                if [ -n "${dep_flavor}" ]; then
                        # For the -a case we can skip the flavorqueue since

$queue here is not set when there is a $dep_subpkg value. The queues have very special different handling. It matters which queue an item ends up in.

bdrewery commented 4 months ago

@pizzamig

pizzamig commented 4 months ago

I tried to understand poudriere internals. but I have to admit that I hit my limits. The bug is the result of the copy&paste, but I guess it's gqueue. But, the subpackage should not be built, only the origin should be built once. I have a couple to test ports I've used to develope the feature you can re-use link

bdrewery commented 4 months ago

Sorry there's no design doc! The queue is definitely a weird mess that came out of FLAVORS support. Thanks for the overlay. I'll bring those in and add some tests for subpackages. Thanks for your work on this.

pizzamig commented 4 months ago

Let me know if you need other explanations on how I tried to add subpackage support to poudriere. In general, the approach should be:

I've found problematic to manage subpackage's information to manage functions like delete_old_pkg Maybe subpackages can be added to the meta queue, as only metadata are needed for delete_old_pkg, but I'm not 100% sure on how the mqueue are held. Adding a subpackage queue seems overkill to me, but maybe a cleaner approach.

bdrewery commented 4 months ago

I'm pretty sure this is dead code. I put an err 99 dead in there and nothing triggers it. There's no point in doing much here without a full test suite to cover the cases.

bdrewery commented 4 months ago

To support that it is dead code, this function handles queued items. Subpackages don't get queued. Only the main port does.