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,
#1877, and #1909, while re-opening #1767 and #1789.
  * 4f283dd1c02a6a4999b53ad2fcbf2264a7074a8b
  * 85b09cf13179b0e5ab8bddb4857e3d2af0a39a31
  * 9409324489a69557229d3d6f505857b9af85a913
  * 2a9449a99b2a2146fce01fa1d9713e999ad3dd4e
  * 14be5169ef49faad0f30cd9d36cdd438e1739f77
  * b6ffeca3a4457efcffbc3fcdbcee683375f74d05

We will need to find a different solution to #1767.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a4dfe2afdb0b2050f5509453b71be6b77f9a555b..460e84b2f7a4bb851e0de3348ee64ff136a97b1f 100644 (file)
--- 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
@@ -72,6 +71,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - gml2gv doesn't handle some attributes correctly #1869
 - Add missing circo, fdp, neato, osage, patchwork, sfdp & twopi tools to Windows builds (copies of dot)
 - Add gv2gml tool to CMake (copy of gml2gv on Windows, symlink to gml2gv otherwise)
+- 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 6b7a4822c7e575ce09992929a54b761eddd04c7c..58682a25230dd7426c4708539abbc0948372b8a7 100644 (file)
--- 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 b3196451c000115fc9b56e2fbeb95f20d488100b..3bedc80b759721af608106bb0779234748297e71 100755 (executable)
--- 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 2c787196c7d453352f2805287600d6fdf3de9644..8f0573f7f1615c27e0ac7f1bc9193ccc665523d6 100644 (file)
--- 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():
     '''
@@ -560,7 +559,6 @@ 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.xfail(strict=True) # FIXME
 @pytest.mark.skipif(shutil.which('gvpr') is None, reason='gvpr not available')
 def test_1909():
     '''