Magnus Jacobsson [Mon, 12 Jul 2021 14:44:59 +0000 (16:44 +0200)]
add MSBuild project reference to gvc for neatogen
This is step 1 of 4 in replacing ortho with gvc as a link dependency
for neatogen. It ensures the correct build order under all
circumstances, also when neatogen is built stand-alone. Just a link
dependency is not enough which does not cause the dependency to
actually be built.
A project reference is a more modern way of creating a project
dependency than the centralized project dependencies in the solution
file. See https://stackoverflow.com/a/2317017/3122101.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 14:26:45 +0000 (16:26 +0200)]
add MSBuild project reference to gvc for dotgen
This is step 1 of 4 in replacing ortho with gvc as a link dependency
for dotgen. It ensures the correct build order under all
circumstances, also when dotgen is built stand-alone. Just a link
dependency is not enough which does not cause the link dependency to
actually be built.
A project reference is a more modern way of creating a project
dependency than the centralized project dependencies in the solution
file. See https://stackoverflow.com/a/2317017/3122101.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
force all ortho object files to be linked into gvc for CMake and MSBuild
This is what the autotools builds do.
Whithout this, the ortho object files ended up both in
libgvplugin_dot_layout.so and libgvplugin_neato_layout.so which caused
an ASan odr_violation.
This commit also adds MSVC an import/export declaration for orthoEdges
which otherwise would become unresolved.
It also changes gvc to use PRIVATE instead of PUBLIC linking to
ortho. The reason is detailed below.
The ASan error message was:
==235826==ERROR: AddressSanitizer: odr-violation (0x7ff55b6c72c0):
[1] size=4 'odb_flags' /home/magjac/graphviz/lib/ortho/ortho.c:44:5
[2] size=4 'odb_flags' /home/magjac/graphviz/lib/ortho/ortho.c:44:5
These globals were registered at these points:
[1]:
#0 0x7ff55b8c5938 in __asan_register_globals ../../../../src/libsanitizer/asan/asan_globals.cpp:341
#1 0x7ff55b2ce1a2 in _sub_I_00099_3 (/home/magjac/graphviz/build/plugin/dot_layout/libgvplugin_dot_layout.so.6+0x43b1a2)
#2 0x7ff55c28c96d (/lib64/ld-linux-x86-64.so.2+0x1196d)
[2]:
#0 0x7ff55b8c5938 in __asan_register_globals ../../../../src/libsanitizer/asan/asan_globals.cpp:341
#1 0x7ff55a8008e4 in _sub_I_00099_3 (/home/magjac/graphviz/build/plugin/neato_layout/libgvplugin_neato_layout.so.6+0x6d38e4)
#2 0x7ff55c28c96d (/lib64/ld-linux-x86-64.so.2+0x1196d)
==235826==HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global 'odb_flags' at /home/magjac/graphviz/lib/ortho/ortho.c:44:5
==235826==ABORTING
Here is a more detailed explanation of the problem and the fix:
1. The ortho library is not a library that we ship.
2. The ortho library is a static library.
3. The ortho object modules should be part of the gvc library. With
autotools they are, but before this fix they are not with CMake or
MSBuild.
4. A user program should load the ortho object modules from the gvc
libray. This works with autotools, but before this fix not with
CMake or MSBuild.
5. All plugins (and all other Graphviz programs and libraries) should
load the ortho object modules from the gvc library. This works with
autotools, but before this fix not with CMake or MSBuild.
6. Nothing should ever link directly to the ortho library except gvc.
7. Even before this fix, the gvc library had ortho as a PUBLIC
dependency, but since gvc itself doesn't refer to any ortho
symbols, no object modules from ortho gets included in the gvc
library itself unless we explicitly include them. Which is what the
autotools builds do and what the CMake and BSBuild builds do after
this fix. My interpretation is that without this, gvc acted as an
INTERFACE library for ortho, i.e., it could tell anything linking
to it where to find the object modules, but not provide them itself
(again, my interpretation. For more see
https://cmake.org/cmake/help/latest/command/add_library.html#interface-libraries).
8. The dot_layout and neato_layout plugins have the gvc library as a
public dependency, but they don't have any direct dependency on the
ortho library itself. Because of this, they find the ortho objects
modules that they need though the gvc library. Before this fix
however, since gvc did not contain the ortho objects, they had no
option but to link in the ortho object modules statically. This is
what caused the ODR violation when both plugins were loaded
simultaneously.
9. This fix also changes the gvc dependency on ortho from PUBLIC to
PRIVATE to avoid giving anything access to the ortho static library
through gvc.
Magnus Jacobsson [Mon, 12 Jul 2021 13:42:05 +0000 (15:42 +0200)]
add MSBuild project reference to ortho for gvc
This is a preparation for adding ortho as a link dependency to gvc in
an upcoming commit. It ensures the correct build order under all
circumstances, also when gvc is built stand-alone. A link dependency
is not enough which does not cause the link dependency to actually be
built.
A project reference is a more modern way of creating a project
dependency than the centralized project dependencies in the solution
file. See https://stackoverflow.com/a/2317017/3122101.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 05:43:34 +0000 (07:43 +0200)]
remove useless MSBuild link dependency on ortho for gvmap
gvmap only depends indirectly on ortho through neatogen which it
already has a link dependency on.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the neatogen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 14:56:13 +0000 (16:56 +0200)]
remove useless MSBuild project dependency on ortho for gvmap
gvmap only depends indirectly on ortho through neatogen which it
already has a project dependency on.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the neatogen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 05:37:19 +0000 (07:37 +0200)]
remove useless MSBuild project reference to ortho for dot
dot only depends indirectly on ortho through dotgen which it already
has a project reference to.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the dotgen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 14:54:05 +0000 (16:54 +0200)]
remove useless MSBuild project dependency on ortho for neato_layout plugin
The neato_layout plugin only depends indirectly on ortho through
neatogen which it already has a project reference to.
A project reference is a more modern way of creating a project
dependency than the centralized project dependencies in the solution
file. See https://stackoverflow.com/a/2317017/3122101.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the neatogen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 05:50:59 +0000 (07:50 +0200)]
remove useless MSBuild link dependency on ortho for neato_layout plugin
The neato_layout plugin only depends indirectly on ortho through
neatogen which it already has a link dependency on.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the neatogen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 05:49:57 +0000 (07:49 +0200)]
remove useless MSBuild project reference to ortho for dot_layout plugin
The dot_layout plugin only depends indirectly on ortho through dotgen
which it already has a project reference to.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the dotgen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
Magnus Jacobsson [Mon, 12 Jul 2021 03:17:43 +0000 (05:17 +0200)]
remove useless MSBuild link dependency on ortho for dot_layout plugin
The dot_layout plugin only depends indirectly on ortho through dotgen
which it already has a link dependency on.
This change is made to avoid confusion when upcoming commits in this
series change ortho to be part of the gvc shared library and adjusts
the dotgen library accordingly.
Towards https://gitlab.com/graphviz/graphviz/-/issues/2096.
remove unnecessary './' prefix on report.txt output in CI
Commit 8f7c1f4c2ef253bc1a763515b344d599e5c199ee removed any directory prefix
from this artifact, outputting it to the root directory of the Graphviz checkout
in CI. In this case, like metrics.txt, it does not need an explicit './' prefix.
gxl2gv: fix: recognize and handle HTML-like strings
When translating a Graphviz file through gv2gxl, HTML-like and non-HTML-like
strings would be translated identically, resulting in a loss of information.
This was fixed in the series merged in d44d19c86f0f469d1c76ce0d7630c8e0ac85e993.
However, to take this through to completion and make round-tripping such strings
possible, changes to gxl2gv were also required.
The present commit updates gxl2gv to recognize the new ‘kind’ GXL attribute that
gv2gxl uses to indicate a string originated as an HTML-like string. The output
of gxl2gv now correctly translates this back to an HTML-like string. Fixes #517.
Note that this fix is orthogonal to the behavior when using an HTML-like and
non-HTML-like string with the same content. It is still not possible to do this
without confusing the internal string interning dictionary (#2089).
gxl2gv: use an enum instead of #defines for attribute type
This introduces some stronger typing and makes it clearer that the attribute
type variables like Current_class can only ever be one of these values. Related
to #517.
An upcoming change makes the type of Current_class an enum, which introduces new
-Wswitch compiler warnings. To avoid this in advance, we give this switch a
default case that does nothing.
gxl2gv: push the ability to indicate an attribute is HTML-like outwards
Callers of setAttr can now indicate the attribute they are setting is an
HTML-like string. This is not yet taken advantage of, but this change is a step
towards #517.
gxl2gv: move common agxset call outside conditional in setEdgeAttr
Every path through the conditional block in this code ends in the same agxset
call. We can simplify this (and upcoming changes) by moving the common call
outside the block. Related to #517.
This file was upstreamed into Vim itself in Vim commit 2346a6378483c9871016f9fc821ec5cbea638f13. The version in Vim can now be
considered authoritative and we can cease maintaining a copy here in Graphviz.
Related to #1762.
The sfio library versioned its format structure, _sffmt_s. This allowed multiple
format structures of differing versions to exist in memory at once,¹ with sfio
able to distinguish between a structure created by itself vs a structure created
by a different version of sfio. In theory, this allows multiple versions of the
sfio library to be loaded at once.
In practice, Graphviz does not take advantage of any of this. In fact, it is not
possible to do this within Graphviz as SFIO_VERSION has not been bumped when
lib/sfio is changed. It still claims to be compatible with a version developed
in 2001.
This commit removes the versioning mechanism entirely, in order to both reduce
maintenance overhead as well as remove the runtime cost associated with managing
this data. The latter is trivial in terms of runtime, but it is also work that
has no purpose.
¹ This kind of design often also allows you to serialize a structure to disk or
network to later be read back in by a potentially differing version of the
library. In the case of sfio however, this use case was not possible because
the structure contains function pointers.
I believe these were used by Kiem-Phong Vo to remind themselves to expose these
definitions unconditionally in future. However, it seems they were never
followed up on. We remove them now to ease ongoing maintenance.
As of the previous commit, the Docker image on which Pylint checks are run in CI
has all Python module dependencies available, so import-error warnings are no
longer an issue. This change adds a build-failing more relaxed Pylint check to
the existing (non-build-failing) one, that only checks for conclusive errors. It
should no longer be possible to introduce broken Python code into Graphviz –
even when it is uncalled – without breaking CI.
remove MarkupSafe as a claimed dependency of doc/infosrc
Nothing in Graphviz uses MarkupSafe. Its last use was removed in 38e3b019bb1b73f273beec66ee2d79e814a97251, but it was incorrectly left behind as
a dependency. I only discovered this by trying to use
doc/infosrc/requirements.txt and finding that MarkupSafe 1.0 is uninstallable.
This is a configuration option to
SparseMatrix_from_coordinate_arrays_not_compacted that is never used. The
leading comment that is also removed in this commit seems to have been a
mistake. If refers to incorrect constants, repeats variables, and contains
numerous typographical errors.
change the always pass test to a simple Graphviz test
The test now uses the existing C API to render SVG from DOT source
code.
To support this, the commit also adds the necessary Graphviz libraries
and the dot_builtins to the common test library and the necessary
include directories to the macro creating the individual test
executables.
don't disable leak detection for ctest based tests
The current test does not trigger any memory leaks and we want to
ensure that we don't introduce any new leaks and fix existing leaks as
new tests encounter them.
We still want leak detection disabled when running pytest since those
tests still trigger many memory leaks.
This is accomplished by moving the disabling of the leak detection
into the test template and performing it after ctest has been
executed, but before pytest executes.
This commit also makes the temporary disabling of ODR violation
bacause of lib/ortho to be selectively for ctest only.
This adds a new test template that is based on the default test
template, but replaces the script section in order to be able to set
up and execute ctest without affecting tests on older OSes.
archive all shared libraries in Ubuntu 21.04 build for ctest
The Ubuntu 21.04 build is renamed and modified to be able to run
ctest, which uses the build products directly from where they reside
before installing them.
Adds archiving of all shared libraries produced directly by "cmake
--build". In the test stage, the test executables load them from those
directory locations since LD_LIBRARY_PATH is automatically set up to
contain them.
add optional ctest & catch2 based test infrastructure
This commit adds a CMakeLists.txt in the tests directory containing
the foundation for easily creating new test executables. It includes
creation of a shared library that the individual test executables can
be linked to, allowing them to be as small as possible.
The tests are optional and can be enabled with the CMake option
with_cxx_tests.
Also added is a first simple test case that always passes to show how
to create test cases.
fix incorrect reference counting of HTML-like strings in gvpack
When duplicating graph attributes, gvpack was calling agstrdup_html to construct
an HTML-like string and then agattr/agset to assign this to an attribute. It did
not anticipate that *both* of these increment the reference count for the string
that has been interned in the graph’s dictionary. The result was a reference
count of 2 for a new HTML-like string, despite only 1 reference to the string
existing.
This was not directly visible to users (hence no changelog entry for this
commit) and the extra reference counts are disposed of when the graph itself is
destroyed. However, this persistent off-by-one issue internally confused some
debugging and potentially retained memory longer than was necessary. The issue
was visible with the following patch applied:
projects / graphviz / log