Buildbot doesn't upload archives made distributable later

[ryandesign (Ryan Carsten Schmidt)]

Suppose a port is not distributable, because its license is not specified, but it builds successfully on the buildbot workers.

Later, a commit indicates the port's license and it is now considered distributable.

In portwatcher, mpbb analyzes the commit, sees that the port it modifies is already installed, and does not schedule a portbuilder task for that port, so the binary, though it is now distributable, is never uploaded to the rsync server.

This is a consequence of fixing #52765.

One solution could be the following pseudocode:

if the port is distributable and the archive is on the packages server then
    nothing to do
else if the port is not distributable but the archive path exists on the buildworker's disk then
    nothing to do
else
    build the port
fi

This logic would be in two places: mpbb-install-dependencies and mpbb-install-port.

[ryandesign (Ryan Carsten Schmidt)]

Here is an example of the problem occurring:

​https://build.macports.org/builders/ports-10.7_x86_64_legacy-watcher/builds/4522

This was triggered by [d7ffec330a3d4d0d2ed13d7cc8260fa63d02ec46/macports-ports] which made asymptote distributable, but mpbb list-subports said "Excluding 'asymptote' because it has already been built."

[jmroot (Joshua Root)]

All that needs to be done to fix this is revert [62834a4212c5b1c59df22221d896cb2c4aab3b44/mpbb]. It wasn't part of my initial PR for this reason but I added it on request.

[mojca (Mojca Miklavec)]

I prefer Ryan's approach. We currently have

if [[ -f $("${option_prefix}/bin/port-tclsh" "${thisdir}/tools/archive-path.tcl" "${port}") ]]; then
    exclude=1
    exclude_reasons+=("it has already been built")
fi

I would change it into (pseudocode):

if [[ -f $("${option_prefix}/bin/port-tclsh" "${thisdir}/tools/archive-path.tcl" "${port}") ]]; then
    if ![["port is distributable" and "port doesn't exist on the packages server"]]; then
        exclude=1
        exclude_reasons+=("it has already been built")
    fi
fi

The code for the two functions is in mpbb-gather-archives:

• if ! curl -fIsL "${option_archive_site}/${archive_port}/${archive_basename}" > /dev/null;
• if "${tclsh}" "${option_jobs_dir}/port_binary_distributable.tcl" -v "${archive_port}"; then

It would be nice to create a standalone shell function to check whether a port is distributable to avoid some code duplication.

But we'll have another problem: install-dependencies and install-port would currently skip installing anything in case the port exists already. And gather-archives will only ever upload active ports. Personally I would remove all those checks from install-dependencies and install-port. I would let the list-subport routine do the job of the gatekeeper and build (activate) everything once on the builder.

While at this: we probably still have another tiny problem when wine's dependencies get updated and don't get built as universal. But I wouldn't want to activate all ports for the sake of that use case, I would try to solve that in a different way.

A quick and dirty hack would be to extend the if [when (not) to build the port] and always build ports with non-default architectures (for example i386 or universal on x86_64). In that case the buildbot would always build wine if somebody triggers that from time to time.

[jmroot (Joshua Root)]

Then we'd be running port_binary_distributable.tcl twice, which can take quite a while.

[mojca (Mojca Miklavec)]

I added some more comments above.

If you are really worried about efficiency of that script (but curl probably doesn't come for free either), then:

if [[ -f $("${option_prefix}/bin/port-tclsh" "${thisdir}/tools/archive-path.tcl" "${port}") ]]; then
    if [["port exists on the packages server"]]; then
        exclude=1
        exclude_reasons+=("it has already been built")
    else
        if ![["port is distributable"]]; then
            exclude=1
            exclude_reasons+=("it has already been built")
        fi
    fi
fi

Note that your suggestion of just letting everything start a job on the builder would be waaaaay more expensive than that.

[jmroot (Joshua Root)]

Would it? Have you measured?

[mojca (Mojca Miklavec)]

Let's assume that we want to rebuild 10000 ports that have all already been successfully built and uploaded before. (For example after changing the maintainers' addresses or after removing the Id line.)

If I understand your suggestion correctly, you would start 10000 individual jobs on the port builder. While all those jobs would be relatively inexpensive (but again: I suspect that with current logic we would not upload non-active ports, so we would have to change/optimize something anyway), there would still be quite some overhead involved in starting up all those jobs. And I cannot imagine the job to be done any faster than a few seconds. I'm unable to see the old logs to check that (presumably Ryan deleted the old logs already), but some ​random inexensive port (that was in fact activated) took half a minute.

No, I didn't measure it, but maybe I don't understand what your proposal is exactly. I cannot understand how starting an individual job on the builder could be any faster or more efficient than doing the check beforehand and avoid all the overhead.

Plus we would be dealing with lots of "useless build logs" (many successful jobs on the port builder that don't do anything at all) that would make the overview of what's happening on the buildbot much less useful.

[jmroot (Joshua Root)]

OK, let's test then.

[jmroot (Joshua Root)]

In 72e1814/mpbb:

Don't exclude already built ports

Testing for #53548

[jmroot (Joshua Root)]

OK, so about 15 seconds overhead per build. That seems like more than it should be. In any case it seems like a good argument for not doing a separate build per (sub)port...

But for the purposes of this problem that is indeed usually less than the checking cost.

[jmroot (Joshua Root)]

In b187a11/mpbb:

Refine exclusion logic

Don't exclude ports that have already been built if they have not
already been uploaded and are distributable.

Fixes: #53548

[mojca (Mojca Miklavec)]

The code doesn't work properly. See ​this:

/opt/bblocal/var/buildworker/ports/build/mpbb/mpbb-list-subports: line 48: option_archive_site: unbound variable

[ryandesign (Ryan Carsten Schmidt)]

[5c7ba12259a066a90a3e2ca2e4b09c9bac4aeb7b/mpbb] fixed this.