RFC: testsuite: add minimium version to dg-require-dot (PR analyzer/93293)
diff mbox series

Message ID 20200117220751.11230-1-dmalcolm@redhat.com
State New
Headers show
Series
  • RFC: testsuite: add minimium version to dg-require-dot (PR analyzer/93293)
Related show

Commit Message

David Malcolm Jan. 17, 2020, 10:07 p.m. UTC
I ran into difficulties with the Graphviz format changing from under
me during an upgrade, where the new version of "dot" would reject .dot
files generated by the analyzer.

The analyzer testsuite has a dot-output.c test that attempts to check
that the generated .dot can be handled by dot, with a dg-require-dot
gating the test.

Unfortunately older versions of dot can't handle the newer generated
output, leading to FAILs from the test.

(I'm also not convinced that newer graphviz versions can always
handle the output from the other passes)

I'd like to automatically verify that the dot output is actually
viewable.

The following patch attempts to add version-checking to dg-require-dot,
rejecting the test as unsupported if dot on the host is earlier than
version 2.40

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, FWIW.

But presumably I should do some kind of feature-testing rather than
version-testing.  I don't think I can embed the .dot input into a dg
directive.  Perhaps have dg-require-dot be passed a relative path to a
feature-test .dot file in the testsuite source directory, and if it can
handle that, then dg-require-dot returns 1 and allows the test to occur?

Thoughts?

gcc/testsuite/ChangeLog:
	PR analyzer/93293
	* gcc.dg/analyzer/dot-output.c (dg-require-dot): Add "2.40" to
	argument.
	* lib/target-supports-dg.exp (dg-require-dot): Add argument.
	* lib/target-supports.exp (parse_version_string): New.
	(check_dot_available): Add args and parse them for a minimum
	required version.  Attempt to parse the output of dot -V.
	Compare them.
---
 gcc/testsuite/gcc.dg/analyzer/dot-output.c |  2 +-
 gcc/testsuite/lib/target-supports-dg.exp   |  5 +-
 gcc/testsuite/lib/target-supports.exp      | 67 +++++++++++++++++++++-
 3 files changed, 68 insertions(+), 6 deletions(-)

Comments

Mike Stump Jan. 19, 2020, 1:58 a.m. UTC | #1
On Jan 17, 2020, at 2:07 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> I ran into difficulties with the Graphviz format changing from under
> me during an upgrade, where the new version of "dot" would reject .dot
> files generated by the analyzer.
> 
> Thoughts?

[ consider this a peanut gallery comment ] Another thought would be to bump the version required and just not worry about it.  The people that have, like and use dot, are likely small, so I'd like to think they can update it.  I don't have/use dot, so, I'm happy to defer to others that have, use, and play with dot.
Eric Gallager Jan. 23, 2020, 5:14 a.m. UTC | #2
On Sat, Jan 18, 2020 at 8:58 PM Mike Stump <mikestump@comcast.net> wrote:

> On Jan 17, 2020, at 2:07 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> >
> > I ran into difficulties with the Graphviz format changing from under
> > me during an upgrade, where the new version of "dot" would reject .dot
> > files generated by the analyzer.
> >
> > Thoughts?
>
> [ consider this a peanut gallery comment ] Another thought would be to
> bump the version required and just not worry about it.  The people that
> have, like and use dot, are likely small, so I'd like to think they can
> update it.  I don't have/use dot, so, I'm happy to defer to others that
> have, use, and play with dot.


Eh, dot is part of graphviz, which is a dependency of lots of other
software packages out there... I rarely ever invoke dot manually, but I see
it running all the time whenever it gets called by doxygen to autogenerate
a package's documentation, and, considering how many users doxygen has,
that's kind of a lot. I'm guessing dot's number of users is probably larger
than small, too.

Patch
diff mbox series

diff --git a/gcc/testsuite/gcc.dg/analyzer/dot-output.c b/gcc/testsuite/gcc.dg/analyzer/dot-output.c
index 25cb31f2d89..628bbda8d34 100644
--- a/gcc/testsuite/gcc.dg/analyzer/dot-output.c
+++ b/gcc/testsuite/gcc.dg/analyzer/dot-output.c
@@ -1,7 +1,7 @@ 
 /* Verify that the various .dot output files from the analyzer are readable
    by .dot.  */
 
-/* { dg-require-dot "" } */
+/* { dg-require-dot "2.40" } */
 /* { dg-additional-options "-fdump-analyzer-callgraph -fdump-analyzer-exploded-graph -fdump-analyzer-state-purge -fdump-analyzer-supergraph" } */
 
 #include <stdlib.h>
diff --git a/gcc/testsuite/lib/target-supports-dg.exp b/gcc/testsuite/lib/target-supports-dg.exp
index 2a21424b890..8bee84975cb 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -180,11 +180,12 @@  proc dg-require-iconv { args } {
     }
 }
 
-# If this host does not have "dot", skip this test.
+# If this host does not have "dot", with a minimum version
+# of at least REQ_MAJOR.REQ_MINOR, skip this test.
 
 proc dg-require-dot { args } {
     verbose "dg-require-dot" 2
-    if { ![ check_dot_available ] } {
+    if { ![ check_dot_available $args ] } {
 	upvar dg-do-what dg-do-what
         set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
     }
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 79166986c77..b527d30e8a7 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -514,17 +514,78 @@  proc check_gc_sections_available { } {
     }]
 }
 
-# Returns 1 if "dot" is supported on the host.
+# Attempt to parse STR as a major.minor pair.
+# Return in list form as a major.minor pair, or throw an exception
+
+proc parse_version_string { str } {
+    if { [regexp {([0-9]*)\.([0-9]*)} $str match major minor] } {
+	verbose "major: $major" 3
+	verbose "minor: $minor" 3
+	lappend result $major $minor
+	return $result
+    } else {
+	error "unable to parse version $str"
+    }
+}
 
-proc check_dot_available { } {
+# Returns 1 if "dot" is supported on the host, with a minimum version
+# of at least "REQ_MAJOR.REQ_MINOR"
+
+proc check_dot_available { args } {
     verbose "check_dot_available" 2
+    verbose "  args: $args" 3
+    set req_version_str [lindex [lindex $args 0] 1]
+    verbose "  req_version_str: $req_version_str" 3
+    if { [catch { set req_version [parse_version_string $req_version_str] } ] } {
+	verbose "unable to determine required dot version" 2
+	return 0
+    } else {
+	set req_major [lindex $req_version 0]
+	set req_minor [lindex $req_version 1]
+    }
+    verbose "  req_version: $req_version" 3
+    verbose "  req_major: $req_major" 3
+    verbose "  req_minor: $req_minor" 3
 
     set status [remote_exec host "dot" "-V"]
     verbose "  status: $status" 2
     if { [lindex $status 0] != 0 } {
 	return 0
     }
-    return 1
+    set dot_output [lindex $status 1]
+    verbose "  dot_output: $dot_output" 3
+
+    # Attempt to parse the dot version into major and minor components.
+    # Expect a string like "dot - graphviz version 2.40.1 (0)".
+    if { [catch { set version [parse_version_string $dot_output] } ] } {
+	verbose "unable to determine dot version" 2
+	return 0
+    } else {
+	set major [lindex $version 0]
+	set minor [lindex $version 1]
+    }
+    verbose "  version: $version" 3
+    verbose "  major: $major" 3
+    verbose "  minor: $minor" 3
+    
+    if { [expr $major > $req_major] } {
+	# Major release beats REQ_MAJOR: no need to check minor
+	verbose "dot major version ($major) exceeds minimum major version ($req_major)" 3
+	return 1
+    } elseif { [expr $major == $req_major] } {
+	if { [expr $minor >= $req_minor] } {
+	    verbose "dot minor version ($minor) exceeds minimum major version ($req_minor)" 3
+	    return 1
+	} else {
+	    # Minor release too low
+	    verbose "dot version ($major.$minor) is below minimum ($req_major.$req_minor)" 2
+	    return 0
+	}
+    } else {
+	# Major release too low
+	verbose "dot version ($major.$minor) is below minimum ($req_major.$req_minor)" 2
+	return 0
+    }
 }
 
 # Return 1 if according to target_info struct and explicit target list