From eb3c31a0e9e0823a642ff4e1357c368c70c44362 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Wed, 18 Nov 2020 19:10:15 -0800 Subject: [PATCH 1/8] add a test case for #1877 --- rtest/test_regression.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/rtest/test_regression.py b/rtest/test_regression.py index 62c9f211f1..5af321237b 100644 --- a/rtest/test_regression.py +++ b/rtest/test_regression.py @@ -433,6 +433,23 @@ def test_1865(): # fdp should not crash when processing this file subprocess.check_call(['fdp', '-o', os.devnull, input]) +@pytest.mark.xfail(strict=True) # FIXME +@pytest.mark.skipif(shutil.which('fdp') is None, reason='fdp not available') +def test_1877(): + ''' + fdp should not fail an assertion when processing cluster edges + https://gitlab.com/graphviz/graphviz/-/issues/1877 + ''' + + # simple input with a cluster edge + input = 'graph {subgraph cluster_a {}; cluster_a -- b}' + + # fdp should be able to process this + p = subprocess.Popen(['fdp', '-o', os.devnull], stdin=subprocess.PIPE, + universal_newlines=True) + p.communicate(input) + assert p.returncode == 0 + # root directory of this checkout ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..')) -- GitLab From 4067b80f8d78e13a4f0f0885325c1e088d3a86e2 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Wed, 18 Nov 2020 19:16:40 -0800 Subject: [PATCH 2/8] add a test case for #1876 --- rtest/test_regression.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/rtest/test_regression.py b/rtest/test_regression.py index 5af321237b..8a4f21c101 100644 --- a/rtest/test_regression.py +++ b/rtest/test_regression.py @@ -433,6 +433,27 @@ def test_1865(): # fdp should not crash when processing this file subprocess.check_call(['fdp', '-o', os.devnull, input]) +@pytest.mark.xfail(strict=True) # FIXME +@pytest.mark.skipif(shutil.which('fdp') is None, reason='fdp not available') +def test_1876(): + ''' + fdp should not rename nodes with internal names + https://gitlab.com/graphviz/graphviz/-/issues/1876 + ''' + + # a trivial graph to provoke this issue + input = 'graph { a }' + + # process this with fdp + p = subprocess.Popen(['fdp'], stdin=subprocess.PIPE, stdout=subprocess.PIPE, + universal_newlines=True) + output, _ = p.communicate(input) + + assert p.returncode == 0, 'fdp failed to process trivial graph' + + # we should not see any internal names like '%3' + assert '%' not in output, 'internal name in fdp output' + @pytest.mark.xfail(strict=True) # FIXME @pytest.mark.skipif(shutil.which('fdp') is None, reason='fdp not available') def test_1877(): -- GitLab From 14210f81d6a96509b5b3b25ef30792962aca2392 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sun, 29 Nov 2020 13:21:00 -0800 Subject: [PATCH 3/8] revert changes to add non-local names to internal map Merge Request !1489 made a change to which names were stored in the internal map. Following this, non-local names (user-provided ones; not starting with '%') were stored in the internal map as well as local names. This inadvertently broke some fdp and circo assumptions (#1876, #1877, !1676). This change reverts the main pieces of the following commits, resolving #1876 and #1877, while re-opening #1767 and #1789. * 4f283dd1c02a6a4999b53ad2fcbf2264a7074a8b * 85b09cf13179b0e5ab8bddb4857e3d2af0a39a31 * 9409324489a69557229d3d6f505857b9af85a913 * 2a9449a99b2a2146fce01fa1d9713e999ad3dd4e * 14be5169ef49faad0f30cd9d36cdd438e1739f77 * b6ffeca3a4457efcffbc3fcdbcee683375f74d05 We will need to find a different solution to #1767. --- CHANGELOG.md | 3 ++- lib/cgraph/id.c | 46 +++++++++++++++++++++++----------------- rtest/rtest.py | 11 ++++++++++ rtest/test_regression.py | 23 ++++++++++---------- 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db845ee95a..3330bef89e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Windows MSBuild executables have the wrong version #1745 - Cast Overflow at pango_textlayout #1314 - x11 back end segfaults if display is unavailable #1776 -- Repeated file read gives different results with libcgraph #1767 - typo in cmd/gvpr/lib/clustg #1781 - Segfault in dot #1783 - Incorrect 'Arrow type "s" unknown' error #1444 @@ -69,6 +68,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - fdp crash #1865 - Graphviz always crash with this simple dot file #167 - Seg fault in dot #1771 +- Regression: fdp generates internal names in the output #1876 +- Regression: fdp assertion error on cluster in edge #1877 ## [2.44.1] - 2020-06-29 diff --git a/lib/cgraph/id.c b/lib/cgraph/id.c index 6b7a4822c7..58682a2523 100644 --- a/lib/cgraph/id.c +++ b/lib/cgraph/id.c @@ -26,16 +26,22 @@ static void *idopen(Agraph_t * g, Agdisc_t* disc) static long idmap(void *state, int objtype, char *str, IDTYPE *id, int createflag) { + char *s; static IDTYPE ctr = 1; - NOTUSED(state); NOTUSED(objtype); - NOTUSED(str); - NOTUSED(createflag); - - *id = ctr; - ++ctr; - + if (str) { + Agraph_t *g; + g = state; + if (createflag) + s = agstrdup(g, str); + else + s = agstrbind(g, str); + *id = (IDTYPE) s; + } else { + *id = ctr; + ctr += 2; + } return TRUE; } @@ -50,17 +56,19 @@ static long idalloc(void *state, int objtype, IDTYPE request) static void idfree(void *state, int objtype, IDTYPE id) { - NOTUSED(state); NOTUSED(objtype); - NOTUSED(id); + if (id % 2 == 0) + agstrfree(state, (char *) id); } static char *idprint(void *state, int objtype, IDTYPE id) { NOTUSED(state); NOTUSED(objtype); - NOTUSED(id); - return NILstr; + if (id % 2 == 0) + return (char *) id; + else + return NULL; } static void idclose(void *state) @@ -92,6 +100,13 @@ int agmapnametoid(Agraph_t * g, int objtype, char *str, { int rv; + if (str && str[0] != LOCALNAMEPREFIX) { + rv = (int) AGDISC(g, id)->map(AGCLOS(g, id), objtype, str, result, + createflag); + if (rv) + return rv; + } + /* either an internal ID, or disc. can't map strings */ if (str) { rv = aginternalmaplookup(g, objtype, str, result); @@ -100,15 +115,6 @@ int agmapnametoid(Agraph_t * g, int objtype, char *str, } else rv = 0; - if (str && (str[0] != LOCALNAMEPREFIX)) { - rv = (int) AGDISC(g, id)->map(AGCLOS(g, id), objtype, str, result, - createflag); - if (rv) { - aginternalmapinsert(g, objtype, str, *result); - return rv; - } - } - if (createflag) { /* get a new anonymous ID, and store in the internal map */ rv = (int) AGDISC(g, id)->map(AGCLOS(g, id), objtype, NILstr, result, diff --git a/rtest/rtest.py b/rtest/rtest.py index b3196451c0..3bedc80b75 100755 --- a/rtest/rtest.py +++ b/rtest/rtest.py @@ -102,6 +102,17 @@ def doDiff(OUTFILE, OUTDIR, REFDIR, testname, subtest_index, fmt): FILE1 = os.path.join(OUTDIR, OUTFILE) FILE2 = os.path.join(REFDIR, OUTFILE) F = fmt.split(':')[0] + # FIXME: Remove when https://gitlab.com/graphviz/graphviz/-/issues/1789 is + # fixed + if platform.system() == 'Windows' and \ + F in ['ps', 'gv'] and \ + testname in ['clusters', 'compound', 'rootlabel']: + print('Warning: Skipping {0} output comparison for test {1}:{2} : format ' + '{3} because the order of clusters in gv or ps output is not ' + 'stable on Windows (#1789)' + .format(F, testname, subtest_index, fmt), + file=sys.stderr) + return if F in ['ps', 'ps2']: with open(TMPFILE1, mode='w') as fd: subprocess.check_call( diff --git a/rtest/test_regression.py b/rtest/test_regression.py index 8a4f21c101..3a0a459c88 100644 --- a/rtest/test_regression.py +++ b/rtest/test_regression.py @@ -348,16 +348,17 @@ def test_1767(): # run the test stdout = subprocess.check_output([exe, dot], universal_newlines=True) - assert stdout == 'Loaded graph:clusters\n' \ - 'cluster_0 contains 5 nodes\n' \ - 'cluster_1 contains 1 nodes\n' \ - 'cluster_2 contains 3 nodes\n' \ - 'cluster_3 contains 3 nodes\n' \ - 'Loaded graph:clusters\n' \ - 'cluster_0 contains 5 nodes\n' \ - 'cluster_1 contains 1 nodes\n' \ - 'cluster_2 contains 3 nodes\n' \ - 'cluster_3 contains 3 nodes\n' + # FIXME: uncomment this when #1767 is fixed + # assert stdout == 'Loaded graph:clusters\n' \ + # 'cluster_0 contains 5 nodes\n' \ + # 'cluster_1 contains 1 nodes\n' \ + # 'cluster_2 contains 3 nodes\n' \ + # 'cluster_3 contains 3 nodes\n' \ + # 'Loaded graph:clusters\n' \ + # 'cluster_0 contains 5 nodes\n' \ + # 'cluster_1 contains 1 nodes\n' \ + # 'cluster_2 contains 3 nodes\n' \ + # 'cluster_3 contains 3 nodes\n' def test_1783(): ''' @@ -433,7 +434,6 @@ def test_1865(): # fdp should not crash when processing this file subprocess.check_call(['fdp', '-o', os.devnull, input]) -@pytest.mark.xfail(strict=True) # FIXME @pytest.mark.skipif(shutil.which('fdp') is None, reason='fdp not available') def test_1876(): ''' @@ -454,7 +454,6 @@ def test_1876(): # we should not see any internal names like '%3' assert '%' not in output, 'internal name in fdp output' -@pytest.mark.xfail(strict=True) # FIXME @pytest.mark.skipif(shutil.which('fdp') is None, reason='fdp not available') def test_1877(): ''' -- GitLab From 3b2f1c8a382dd7e122aa9d7d17f89e8e90c8b106 Mon Sep 17 00:00:00 2001 From: Rob Hart Date: Wed, 16 Dec 2020 13:50:19 -0800 Subject: [PATCH 4/8] Check for empty strings in tp and hp. --- lib/common/labels.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/common/labels.c b/lib/common/labels.c index 61aca92ed3..ef332b4b41 100644 --- a/lib/common/labels.c +++ b/lib/common/labels.c @@ -322,11 +322,11 @@ static char *strdup_and_subst_obj0 (char *str, void *obj, int escBackslash) t_str = agnameof(agtail(((edge_t *)obj))); pt = ED_tail_port((edge_t *)obj); if ((tp_str = pt.name)) - has_tp = TRUE; + has_tp = (tp_str != '\0'); h_str = agnameof(aghead(((edge_t *)obj))); pt = ED_head_port((edge_t *)obj); if ((hp_str = pt.name)) - has_hp = TRUE; + has_hp = (hp_str != '\0'); tl = ED_label((edge_t *)obj); if (tl) { l_str = tl->text; -- GitLab From 8ffb447ef87de1970fd5a5d05b67d780035162ff Mon Sep 17 00:00:00 2001 From: Rob Hart Date: Thu, 17 Dec 2020 10:29:30 -0800 Subject: [PATCH 5/8] Correct empty string test. --- lib/common/labels.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/common/labels.c b/lib/common/labels.c index ef332b4b41..216bc9b282 100644 --- a/lib/common/labels.c +++ b/lib/common/labels.c @@ -322,11 +322,11 @@ static char *strdup_and_subst_obj0 (char *str, void *obj, int escBackslash) t_str = agnameof(agtail(((edge_t *)obj))); pt = ED_tail_port((edge_t *)obj); if ((tp_str = pt.name)) - has_tp = (tp_str != '\0'); + has_tp = (*tp_str != '\0'); h_str = agnameof(aghead(((edge_t *)obj))); pt = ED_head_port((edge_t *)obj); if ((hp_str = pt.name)) - has_hp = (hp_str != '\0'); + has_hp = (*hp_str != '\0'); tl = ED_label((edge_t *)obj); if (tl) { l_str = tl->text; -- GitLab From 570ecaccb6548b431dd6dcaef86422bf95b14fa7 Mon Sep 17 00:00:00 2001 From: Rob Hart Date: Thu, 17 Dec 2020 10:55:17 -0800 Subject: [PATCH 6/8] Add simple twopi svg regression test --- rtest/linux.x86/svgtitle_twopi.svg | 31 ++++++++++++++++++++++++++++++ rtest/tests.txt | 4 ++++ 2 files changed, 35 insertions(+) create mode 100644 rtest/linux.x86/svgtitle_twopi.svg diff --git a/rtest/linux.x86/svgtitle_twopi.svg b/rtest/linux.x86/svgtitle_twopi.svg new file mode 100644 index 0000000000..c50a9d2b08 --- /dev/null +++ b/rtest/linux.x86/svgtitle_twopi.svg @@ -0,0 +1,31 @@ + + + + + + + +this is a graph + + +%3 + +a + + + +%4 + +b + + + +%4->%4 + + + + + diff --git a/rtest/tests.txt b/rtest/tests.txt index 3aac85351d..35ddc427cb 100644 --- a/rtest/tests.txt +++ b/rtest/tests.txt @@ -351,3 +351,7 @@ sides = dot ps +svgtitle +a.gv +twopi svg + -- GitLab From 0044d98202dd688a249da9fc84b32c97c8ab4dd5 Mon Sep 17 00:00:00 2001 From: Rob Hart Date: Sat, 19 Dec 2020 14:14:22 -0800 Subject: [PATCH 7/8] Add regression test --- rtest/test_regression.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/rtest/test_regression.py b/rtest/test_regression.py index e80242e599..3fa04403e3 100644 --- a/rtest/test_regression.py +++ b/rtest/test_regression.py @@ -558,3 +558,21 @@ def test_1869(variant: int): assert 'style=dashed' in output, 'style=dashed not found in DOT output' assert 'penwidth=2' in output, 'penwidth=2 not found in DOT output' + +@pytest.mark.skipif(shutil.which('twopi') is None, reason='twopi not available') +def test_1907(): + ''' + SVG edges should have title elements that match their names + https://gitlab.com/graphviz/graphviz/-/issues/1907 + ''' + + # a trivial graph to provoke this issue + input = 'digraph { A -> B -> C }' + + # generate an SVG from this input with twopi + p = subprocess.Popen(['twopi', '-Tsvg'], stdin=subprocess.PIPE, stdout=subprocess.PIPE, + universal_newlines=True) + output, _ = p.communicate(input) + + assert 'A->B' in output, \ + 'element title not found in SVG' -- GitLab From a941e5378586f1fc60f5b55c345cd27828b33634 Mon Sep 17 00:00:00 2001 From: Rob Hart Date: Sun, 20 Dec 2020 04:47:28 -0800 Subject: [PATCH 8/8] Revert "Add simple twopi svg regression test" This is now covered by a case in tesy_regression.py This reverts commit 570ecaccb6548b431dd6dcaef86422bf95b14fa7. --- rtest/linux.x86/svgtitle_twopi.svg | 31 ------------------------------ rtest/tests.txt | 4 ---- 2 files changed, 35 deletions(-) delete mode 100644 rtest/linux.x86/svgtitle_twopi.svg diff --git a/rtest/linux.x86/svgtitle_twopi.svg b/rtest/linux.x86/svgtitle_twopi.svg deleted file mode 100644 index c50a9d2b08..0000000000 --- a/rtest/linux.x86/svgtitle_twopi.svg +++ /dev/null @@ -1,31 +0,0 @@ - - - - - - - -this is a graph - - -%3 - -a - - - -%4 - -b - - - -%4->%4 - - - - - diff --git a/rtest/tests.txt b/rtest/tests.txt index 35ddc427cb..3aac85351d 100644 --- a/rtest/tests.txt +++ b/rtest/tests.txt @@ -351,7 +351,3 @@ sides = dot ps -svgtitle -a.gv -twopi svg - -- GitLab