From 250e4886d85a7e131da76f181b3fa7be02d1a76d Mon Sep 17 00:00:00 2001
From: Jan-Marek Glogowski <glogow@fbihome.de>
Date: Sun, 12 Dec 2021 22:13:37 +0100
Subject: [PATCH] Fail module-deps.pl on more errors

* Fail on the error code returned from make
* Collect dependency errors in the tree and fail and print them

The "perldoc -f open" has an example in the "Opening a filehandle
into a command" section, which uses waitpid in the parent. Since
we parse the pipe output, there seems to be no need for waitpid,
because we end when the pipe is closed by the writer.
waitpid always returns -1 at this point and since there aren't
any zombie processes, it seems to be fine to ignore.
Additionally strace shows, clone, execve and waitpid calls, so
the explicit exit in the example seems bogus, if the process
is actually overwritten.
And the pipe is just open in the parent process anyway.

Change-Id: I2e77aa33a0eaa5d6d3e06904bb0af53f4b66ef91
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126789
Tested-by: Jenkins
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>
---
 bin/module-deps.pl | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/bin/module-deps.pl b/bin/module-deps.pl
index abec124e4abb..b3efc72f4d36 100755
--- a/bin/module-deps.pl
+++ b/bin/module-deps.pl
@@ -29,6 +29,7 @@ sub read_deps()
     my $invalid_tolerance = 100;
     my $line_count = 0;
     my %deps;
+    my $child_pid = 0;
     if (defined $to_file)
     {
         open($to, ">$to_file") or die "can not open file for writing $to_file";
@@ -36,14 +37,15 @@ sub read_deps()
     if (defined $from_file) {
         open ($p, $from_file) || die "can't read deps from cache file: $!";
     } else {
-        open ($p, "ENABLE_PRINT_DEPS=1 $gnumake -qrf $makefile_build|") || die "can't launch make: $!";
+        $child_pid = open ($p, "-|", "ENABLE_PRINT_DEPS=1 $gnumake -qrf $makefile_build") // die "couldn't launch make: $!";
+        exit if (!$child_pid);
     }
     $|=1;
     print STDERR "reading deps ";
     while (<$p>) {
         my $line = $_;
         $line_count++;
-        print STDERR '.' if ($line_count % 10 == 0);
+        print STDERR '.' if (!$verbose && $line_count % 10 == 0);
         logit($line);
         print $to $line if defined $to_file;
         chomp ($line);
@@ -68,6 +70,14 @@ sub read_deps()
         }
     }
     close ($p);
+    if ($child_pid) {
+        my $err = $? >> 8;
+        # make query mode returns 0 or 1, depending on the build status
+        if ($err != 0 && $err != 1) {
+            print STDERR " error\n" if (!$verbose);
+            die("Errorcode $err from make - aborting!");
+        }
+    }
     print STDERR " done\n";
 
     return \%deps;
@@ -189,8 +199,13 @@ sub optimize_tree($)
 {
     my $tree = shift;
     prune_redundant_deps($tree);
+    my @errors;
     for my $name (sort keys %{$tree}) {
         my $result = $tree->{$name};
+        if (!defined($result->{target})) {
+            push @errors, "missing target for dependency '$name'!";
+            next;
+        }
         logit("minimising deps for $result->{target}\n");
         my @newdeps;
         for my $dep (@{$result->{deps}}) {
@@ -210,6 +225,10 @@ sub optimize_tree($)
         # re-write the shrunk set to accelerate things
         $result->{deps} = \@newdeps;
     }
+    if (scalar @errors > 0) {
+        print STDERR join("\n", @errors) . "\n";
+        die("Missing targets for dependencies - aborting!");
+    }
     return $tree;
 }
 
@@ -337,13 +356,22 @@ END
             'shape=box,style=filled,color="#CCCCCC"' .
             "];" . join(';', @merged_names) . "\n";
 
+   my @errors;
    for my $name (sort keys %{$tree}) {
        my $result = $tree->{$name};
+       if (!defined($result->{target})) {
+           push @errors, "Missing target for dependency '$name'!";
+           next;
+       }
        logit("minimising deps for $result->{target}\n");
        for my $dep (@{$result->{deps}}) {
            print $to "$name -> $dep;\n" ;
        }
     }
+    if (scalar @errors > 0) {
+        print STDERR join("\n", @errors) . "\n";
+        die("Missing targets for dependencies - aborting!");
+    }
     print $to "}\n";
 }