fix: assign anonymous IDs to named entities

Instead of using string pointers as IDs, we now assign the IDs for named
entities exactly the same way we assign them for anonymous identities. This
works because we first check the internal map before creating any new ID, so
processing the same name twice will result in the same ID as before.

This fixes #1767. Now clusters within a graph are consistently processed in the
order in which they appear in the input file, rather than an order dependent on
pointers given out by the allocator.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 30d9920bc039afb40d52bcd9e19f8d6ece1a091f..4257dac46e56c8a1639675504147c0f558c4dbcf 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@ 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
 
 ## [2.44.1] - 2020-06-29
 

diff --git a/lib/cgraph/id.c b/lib/cgraph/id.c
index 14804d2c4c1a3add050500956e60d9dcb83152fd..29ed46075bb6c199247c96fd500d4f2be77e0b86 100644 (file)
--- a/lib/cgraph/id.c
+++ b/lib/cgraph/id.c
@@ -24,22 +24,16 @@ 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);
-    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;
-    }
+    NOTUSED(str);
+    NOTUSED(createflag);
+
+    *id = ctr;
+    ctr += 2;
+
     return TRUE;
 }
 
@@ -54,9 +48,9 @@ static long idalloc(void *state, int objtype, IDTYPE request)
 
 static void idfree(void *state, int objtype, IDTYPE id)
 {
+    NOTUSED(state);
     NOTUSED(objtype);
-    if (id % 2 == 0)
-       agstrfree((Agraph_t *) state, (char *) id);
+    NOTUSED(id);
 }
 
 static char *idprint(void *state, int objtype, IDTYPE id)

diff --git a/rtest/1767.c b/rtest/1767.c
new file mode 100644 (file)
index 0000000..c981030
--- /dev/null
+++ b/rtest/1767.c
@@ -0,0 +1,48 @@
+/* test case for repeated use of Pango plugin (see
+ * test_regression.py:test_1767())
+ */
+
+#include <assert.h>
+#include <graphviz/cgraph.h>
+#include <graphviz/gvc.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static void load(const char *filename) {
+  Agraph_t *g;
+  GVC_t *gvc;
+  FILE *file;
+  int r;
+  Agraph_t *sg;
+
+  gvc = gvContext();
+  assert(gvc != NULL);
+
+  file = fopen(filename, "r");
+  assert(file != NULL);
+
+  g = agread(file, 0);
+  printf("Loaded graph:%s\n", agnameof(g));
+
+  r = gvLayout(gvc, g, "dot");
+  assert(r == 0);
+
+  for (sg = agfstsubg(g); sg != NULL; sg = agnxtsubg(sg)) {
+    printf("%s contains %d nodes\n", agnameof(sg), (int)agnnodes(sg));
+  }
+
+  gvFreeLayout(gvc, g);
+  agclose(g);
+  fclose(file);
+  gvFreeContext(gvc);
+}
+
+int main(int argc, char **argv) {
+
+  assert(argc == 2);
+
+  load(argv[1]);
+  load(argv[1]);
+
+  return 0;
+}

diff --git a/rtest/1767.dot b/rtest/1767.dot
new file mode 100644 (file)
index 0000000..bcc030b
--- /dev/null
+++ b/rtest/1767.dot
@@ -0,0 +1,24 @@
+digraph clusters {
+    subgraph cluster_0 {
+        a -> b -> c -> d -> e
+    }
+
+    subgraph cluster_1 {
+        a -> f -> c
+    }
+    
+    subgraph cluster_2 {
+        rank = same
+        p1
+        p2
+        p3->f
+    }
+    
+    subgraph cluster_3 {
+        rank = same
+        S1->a
+        S2->f
+        S3->p1
+    }
+}
+

diff --git a/rtest/test_regression.py b/rtest/test_regression.py
index 4759972a08231dfaf1cedf3da1a49e60eda77d5e..95805a6f00a7d8730a2b3f4f0c27983102d14057 100644 (file)
--- a/rtest/test_regression.py
+++ b/rtest/test_regression.py
@@ -1,8 +1,11 @@
+import atexit
 import pytest
 import platform
+import shutil
 import subprocess
 import os
 import re
+import tempfile
 
 # The terminology used in rtest.sh is a little inconsistent. At the
 # end it reports the total number of tests, the number of "failures"
@@ -190,3 +193,50 @@ def test_1594():
 
     assert 'line 3:' in stderr, \
       'GVPR did not identify correct line of syntax error'
+
+def test_1767():
+    '''
+    using the Pango plugin multiple times should produce consistent results
+    https://gitlab.com/graphviz/graphviz/-/issues/1767
+    '''
+
+    # FIXME: some of the Windows CI builds don't install libcgraph that this
+    # test depends on. Others fail the execution of the compiled binary for
+    # unknown reasons.
+    if platform.system() == 'Windows':
+      pytest.skip('test disabled on Windows')
+
+    # find co-located test source
+    c_src = os.path.abspath(os.path.join(os.path.dirname(__file__), '1767.c'))
+    assert os.path.exists(c_src), 'missing test case'
+
+    # create some scratch space to work in
+    tmp = tempfile.mkdtemp()
+    atexit.register(shutil.rmtree, tmp)
+
+    # compile our test code
+    exe = os.path.join(tmp, 'a.exe')
+    if platform.system() == 'Windows':
+        subprocess.check_call(['cl', c_src, '-Fe:', exe, '-nologo', '-link',
+          'cgraph.lib', 'gvc.lib'])
+    else:
+        cc = os.environ.get('CC', 'cc')
+        subprocess.check_call([cc, c_src, '-o', exe, '-lcgraph', '-lgvc'])
+
+    # find our co-located dot input
+    dot = os.path.abspath(os.path.join(os.path.dirname(__file__), '1767.dot'))
+    assert os.path.exists(dot), 'missing test case'
+
+    # 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'