Buildbot now fails to build wine dependencies

[Rainer Müller]

On 2017-07-04 07:11, Mojca Miklavec wrote:
> One question: does a clean installation of "gtk3 +quartz" still work
> now? It depends on both glib2 and cairo, so I would expect problems or
> at least some weird non-determinism.

I see no problem with that. Only the explicitly specified +quartz is
passed down to dependencies. On a clean prefix this would install:
 glib2 +quartz
 cairo +quartz +x11
 gtk3 +quartz

The problem only occurs when you explicitly ask for both +x11 and
+quartz on a dependent of glib2 and glib2 was not yet installed.

> On 4 July 2017 at 00:36, Rainer Müller <raimue at macports.org> wrote:
>> On 2017-07-03 22:26, Mojca Miklavec wrote:
>>> The problem is that we loop through all dependencies and install and
>>> *uninstall* each individual one of them. This is both inefficient and
>>> leads to problems like this one.
>>>
>>> The problem should be solved if we skip deactivation of dependencies.
>>
>> We have to do this as we want to build all ports cleanly. Otherwise we
>> would not be able to detect missing dependencies.
> 
> Ports - yes. But why are we being so picky about dependencies? On the
> old buildbot we would only ever run "port install foo". OK, now we
> would also upload individual dependencies to the server in case they
> were not built before (which they usually should be).

In order to make use of the failcache, we need to build dependencies
individually, so we can check whether they failed before.

The ideal solution would have been to schedule an individual build job
for each dependency. However, as we noticed builds just cannot be
scheduled dynamically with buildbot...

> We should at least provide an option to avoid such behaviour on Travis
> to make the builds with already limited time frame for the build, more
> efficient.

+1

Sounds reasonable for the quick check on Travis, where we mostly care
about the changed port and not about dependencies.

> (a) Do the installation recursively. We now list all dependencies, but
> then install some dependencies (among them A), deactivate them, and
> install another dependency (say B which depends on A). Variants for A
> might then be wrong. We should keep the dependency list from the first
> step and before installing B, install A with original variants and all
> other B's dependencies. (Sorry, my sentence is probably super
> confusing. What I mean is that we should explicitly install all of
> cairo's dependencies before installing cairo, including "glib2 +x11".)

I still don't understand why we pass *default* variants to dependencies
at all. As I see it, the problem would go away if we would not request
default variants explicitly...

@Clemens,
as you originally added this to mpbb/tools/dependencies.tcl, do you
still remember the reason?

I am attaching a patch to only print active variants that are not
enabled by default (or disabled default variants). To me the result
looks like what we want to have:

$ tools/dependencies.tcl wine | grep -E 'glib2|cairo'
glib2 +universal
cairo +universal
$ tools/dependencies.tcl gtk3 +quartz | grep -E 'glib2|cairo'
glib2 +quartz
cairo

I did not test this patch with a buildbot setup, though.

> (b) When building cairo, we should check whether the main port was
> built with any variants. If the main port was built with +quartz (that
> is: once we start supporting building "gtk3 +quartz"), then keep
> +quartz when installing cairo. If the main port was not built with
> +quartz, only list non-default variants. We can still know which
> variants were built with cairo, so we can either put those variants in
> the failcache list. Or well ... as it turned out a failed "cairo +x11
> +quartz" is not exactly the same as a failed "cairo" with x11 and
> cairo being explicit, so two failcaches would be fine.

Introducing special cases for +x11 and +quartz adds even more complexity
and I would not want to bless variants in any special way.

Rainer
-------------- next part --------------
From 34c0f3430ffdbd9421703d9db5c831b1067420eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rainer=20M=C3=BCller?= <raimue at macports.org>
Date: Tue, 4 Jul 2017 16:12:12 +0200
Subject: [PATCH] Do not request default variants explictly

This avoids the problem that explicitly requested variants are passed
down to dependencies, where they can cause conflicts.

This may lead to cache misses in the failcache, as we can no longer
differentiate between a build with an explicitly requested variant and
the default variant. These cases should be rare, so they are
neglectible.

See: https://lists.macports.org/pipermail/macports-dev/2017-June/035978.html
---
 tools/dependencies.tcl | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/dependencies.tcl b/tools/dependencies.tcl
index 12a0aec..7af0444 100755
--- a/tools/dependencies.tcl
+++ b/tools/dependencies.tcl
@@ -102,7 +102,7 @@ proc printdependency {ditem} {
     # Given the active_variants of the current dependency calculation and the
     # default variants, calculate the required string.
     set default_variants {}
-    if {[array size variants] > 0 && [info exists depinfo(vinfo)]} {
+    if {[info exists depinfo(vinfo)]} {
         foreach {vname vattrs} $depinfo(vinfo) {
             foreach {key val} $vattrs {
                 if {$key eq "is_default" && $val eq "+"} {
@@ -116,13 +116,11 @@ proc printdependency {ditem} {
     set variantstring ""
     array set active_variants $depinfo(active_variants)
 
-    set relevant_variants [lsort -unique [concat [array names active_variants] $default_variants]]
-    foreach variant $relevant_variants {
-        if {[info exists active_variants($variant)]} {
+    # output all non-default variants and all explictly disabled default variants
+    foreach variant [array names active_variants] {
+        if {($active_variants($variant) eq "+" && [lsearch -exact $default_variants $variant] == -1)
+                || ($active_variants($variant) eq "-" && [lsearch -exact $default_variants $variant] >= 0)} {
             append variantstring "$active_variants($variant)$variant"
-        } else {
-            # the only case where this situation can occur is a default variant that was explicitly disabled
-            append variantstring "-$variant"
         }
     }
 
-- 
2.13.2