VPSC: uniformly treat objects as structs that need to be exposed to C
The keywords "class" and "struct" are synonyms in C++, except for changing the
default visibility of declarations within the object. By using struct for
anything that needs to be declared in the C API, we can avoid some #ifdef mess
in this header.
Like the change to add_tree_edge(), this also fixes some resource leakage that
occurred in this error path. Now the tree list in rank2 is correctly cleaned up
on error. Related to #1801.
This also fixes some resource leaks that occurred in this path. The longjmp()
would jump over some intermediate allocations that had been made, whereas we now
return back through the callers, cleaning up resources as we go. Related to
#1801.
It looks to me as if cdc5efaf40d94b1020b650d654f94ebf0a856c62 attempted to
consolidate Qt5 support but actually erroneously re-enabled Qt4 qmake support.
This re-enabled qmake doesn't work because it produces a Makefile designed to
work with Qt4, while the CFLAGS and LDFLAGS set by PKG_CHECK_MODULES are all
based on Qt5 library names.
The outcome of this is that building on a Linux machine with libqt5 and
libqt4-dev installed but *not* libqt5-dev (a legal configuration) causes
Graphviz compilation to fail when processing GVEdit #includes it cannot find.
The fix here is to simply remove support for Qt4's qmake because Qt4 is
deprecated [0] and force the build system to find Qt5's qmake. Closes #1862.
make_label() internally strdups its input, so this extra allocation was simply
being lost. This was observable using an ASan-instrumented build and the command
`dot -Tsvg -o /dev/null ./rtest/share/alf.gv`:
Direct leak of 121 byte(s) in 12 object(s) allocated from:
#0 0x7fd2541bf810 in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
#1 0x7fd25404a955 in parse_reclbl /tmp/tmp.bXYbsH93VJ/graphviz/lib/common/shapes.c:3314
#2 0x7fd25404a5eb in parse_reclbl /tmp/tmp.bXYbsH93VJ/graphviz/lib/common/shapes.c:3292
#3 0x7fd25404ca5f in record_init /tmp/tmp.bXYbsH93VJ/graphviz/lib/common/shapes.c:3556
#4 0x7fd25405966a in common_init_node /tmp/tmp.bXYbsH93VJ/graphviz/lib/common/utils.c:658
#5 0x7fd24fc78a01 in dot_init_node /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:40
#6 0x7fd24fc79522 in dot_init_node_edge /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:81
#7 0x7fd24fc7ad61 in dotLayout /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:295
#8 0x7fd24fc7c4b9 in doDot /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:450
#9 0x7fd24fc7ca94 in dot_layout /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:496
#10 0x7fd253f7673d in gvLayoutJobs /tmp/tmp.bXYbsH93VJ/graphviz/lib/gvc/gvlayout.c:85
#11 0x55a9961b3960 in main /tmp/tmp.bXYbsH93VJ/graphviz/cmd/dot/dot.c:132
#12 0x7fd253d2309a in __libc_start_main ../csu/libc-start.c:308
Direct leak of 118 byte(s) in 19 object(s) allocated from:
#0 0x7fd2541bf810 in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x3a810)
#1 0x7fd25404a955 in parse_reclbl /tmp/tmp.bXYbsH93VJ/graphviz/lib/common/shapes.c:3314
#2 0x7fd25404ca5f in record_init /tmp/tmp.bXYbsH93VJ/graphviz/lib/common/shapes.c:3556
#3 0x7fd25405966a in common_init_node /tmp/tmp.bXYbsH93VJ/graphviz/lib/common/utils.c:658
#4 0x7fd24fc78a01 in dot_init_node /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:40
#5 0x7fd24fc79522 in dot_init_node_edge /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:81
#6 0x7fd24fc7ad61 in dotLayout /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:295
#7 0x7fd24fc7c4b9 in doDot /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:450
#8 0x7fd24fc7ca94 in dot_layout /tmp/tmp.bXYbsH93VJ/graphviz/lib/dotgen/dotinit.c:496
#9 0x7fd253f7673d in gvLayoutJobs /tmp/tmp.bXYbsH93VJ/graphviz/lib/gvc/gvlayout.c:85
#10 0x55a9961b3960 in main /tmp/tmp.bXYbsH93VJ/graphviz/cmd/dot/dot.c:132
#11 0x7fd253d2309a in __libc_start_main ../csu/libc-start.c:308
Magnus Jacobsson [Sun, 15 Nov 2020 12:31:28 +0000 (13:31 +0100)]
Remove commented out collection = 'stable'
Leaving the "unused" collection as a comment was originally intended
to ensure that no mistake is made when the collection is changed, but
since there's an assertion guarding for this, this is unnecessary and
just gives less clean commit diffs.
Rob Hart [Wed, 16 Dec 2020 21:50:19 +0000 (13:50 -0800)]
Check for empty strings in tp and hp.
Edit by Matthew Fernandez: Squashed a fixup commit into this one, adjusted the
(now prior) test case to expect to pass, and added a changelog entry. For
reference, the bug being fixed was introduced in 31bab037c9bfde3bd18e06b5ab878c09de265ccf.
Rob Hart [Sat, 19 Dec 2020 22:14:22 +0000 (14:14 -0800)]
Add regression test
Edit by Matthew Fernandez: moved this commit ahead of the one that fixes the bug
and marked the test case as expected to fail. This allows us to more accurately
confirm the bug exists and its fix works as intended, while still leaving the
commit series bisectable.
This reverts commits 429718cb387092b5bf81850ac97d18ca34149055 but with a
slightly different variant of the macro hack that avoids subsequent sed
replacement steps. I had several false starts at this fix, so let's detail what
did not work:
1. We cannot implement an always-no-answering isatty() locally or redirect
isatty() to another function, because Flex calls isatty(fileno(f)) and our
"FILE" from SFIO does not provide this. On operating systems where fileno()
references an offset in the FILE struct (e.g. macOS) this causes a
segfault. So we need to suppress the entire argument to isatty(), not just
the call to it.
2. Versions of Flex prior to 2.5.39 emit a spurious extern declaration [0]. If
we reverted 429718cb387092b5bf81850ac97d18ca34149055 as-is, we would have
also had to revert 3b00c1fc6b949cc9744d075e0d2b2ed4b1c46763. However, even
this would not be enough because we now support a CMake build, so we would
have to introduce a corresponding find-and-replace step in
lib/cgraph/CMakeLists.txt. Rather than do this, I came up with a different
macro replacement that still makes isatty() calls evaluate to 0 but does
not require other find-and-replace steps.
3. Although Flex 2.5.39 was released almost a decade ago, we cannot easily
drop support for prior versions of Flex. The latest version of Apple XCode
ships Flex 2.5.35 and is unlikely to upgrade any time soon. This means most
macOS users have Flex 2.5.35 without realizing it. Compounding the problem,
the files generated by Flex #include headers that ship with Flex, so even
macOS users who have a newer version of Flex via Homebrew/Macports may
accidentally have the Flex 2.5.35 headers in their include path. The path
of least irritation to our downstream macOS users appear to be retaining
Flex 2.5.35 support.
Fixes #1910. The underlying problem here still exists: with `never-interactive`
set in the lib/cgraph/scan.l lexer, we are somehow not resetting internal state
in between lexer invocations. I suspect this will return as an issue in future
and we will have to more thoroughly debug this.
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).
fix: require exactly \d+.\d+.\d+ for a release version
This regex was incorrectly checking for anything *starting* with this pattern.
As a result, versions like "2.44.2~dev.20210109.1932" would pass this check.
Related to #1892.
fix: handle --version not being passed to deployment script
b443b70a828a34c79c71e41af9cb2eab7b950a14 incorrectly did not anticipate neither
the --force nor --version flags being passed to the deployment script.
Unfortunately I failed to notice this because I was testing using --force and
there was no easy way to test the final work flow until merging to master.
There is also no easy way to test this commit before being merged. I have run
pylint, but after that just need to cross my fingers.
rewrite deployment CI task to create releases on Gitlab
Prior to this commit, the deployment CI task runs on a private runner and
uploads release artifacts to www2.graphviz.org. The SSH steps in the deployment
task are currently failing due to connectivity issues. The current maintainers
are unable to fix this as we do not have enough knowledge of how this work flow
was intended to function.
Apart from these concerns, Gitlab have recently introduced support for so-called
“generic packages” [0] to host binary artifacts directly on Gitlab (or its
underlying CDN) as well as the ability to programmatically create releases [1]
on the releases page of a project. The decision was made to simplify the
Graphviz infrastructure and host future releases (both stable and development)
on Gitlab.
This change rewrites the deployment task to (1) run on the Gitlab shared runner
using a release-cli Docker image, (2) upload package artifacts to Graphviz'
generic package, and (3) create a release from this for stable version numbers.
To comprehend the role the generic package container is playing here, note that
generic package version numbers are synthesized from commit SHA and are not
intended to be exposed to users. Users can browse the generic package page [2],
but are only ever expected to be interested in the latest version available
here. In contrast, version numbers on the release page [3] are intended to
correspond to Graphviz stable version numbers. ci/deploy.py handles mapping a
release on this page to its related (synthesized) generic package version.