The macOS tasks were, by default, blocking on the portable source generation.
However these tasks do not use the portable source or any prior stage’s
artifacts. Now that we have moved to shared macOS runners without caching, the
macOS CI tasks are the limiting factor on CI runtime. Cutting this dependency
should meaningfully impact this.
fix: propagate VERSION from macOS build tasks to macOS CI test tasks
The macOS test tasks in CI were picking up the VERSION from the portable source
task, despite their build tasks using a different process to derive VERSION.
This change ensures they get the VERSION file their build task created.
fix computation of minimum graph rank on large graphs
This code was using MAXSHORT as the maximum (unrealistically large) rank. That
is, start out with this value and assume all nodes will rank below this so we
can progressively step down. This was likely correct when the code was first
written. However, 97cb5fffdcaa97eadb76a9b82a026d084a6a94d9 altered the type used
for storing ranks from short to int to support larger graphs. That is, nodes in
a graph are now anticipated to have ranks in excess of 32767 (on e.g. x86). This
change did not update these algorithms to compensate.
The effect of this is that any graph whose nodes were all ranked >32767 would be
incorrectly judged to have a minimum rank of 32767. The present change fixes
this by realigning this to the limit of the new ranking type.
Note that this removes the last use MAXSHORT in the code base. Unfortunately we
cannot easily remove its definition without breaking API because the containing
header lib/common/arith.h is shipped.
use range-based for loops to clean up some manual iteration
Note that in once instance this modifies a loop that was skipping the first
element to not skip it. But this is a no-op the compiler should see through.
store Graphics as managed pointers in Visio::Render instead of raw pointers
This lets us clean up some manual handling of these and stop worrying about
memory management. Note that Visio::Render::ClearGraphicsAndTexts touched in
this commit is effectively a hand written destructor for this class. Presumably
it was implemented in a time before destructors were standardized. We can likely
move the other collections affected here to using smart pointers too and remove
this function entirely eventually.
take a reference instead of a pointer in Visio::Render::PrintEdgeShape
Similar to the previous commits, this function assumes its input pointer is
non-null, so we may as well use the more convenient C++ syntax for this. This
will also ease some upcoming changes to convert the underlying raw pointers to
smart pointers.
take a reference instead of a pointer in Visio::Render::PrintInnerShape
Similar to the previous commit, this function assumes its input pointer is
non-null, so we may as well use the more convenient C++ syntax for this. This
will also ease some upcoming changes to convert the underlying raw pointers to
smart pointers.
take a reference instead of a pointer in Visio::Render::PrintOuterShape
This function assumes its input pointer is non-null, so we may as well use the
more convenient C++ syntax for this. This will also ease some upcoming changes
to convert the underlying raw pointers to smart pointers.
Using a const qualifier on the element type of an STL container is usually an
anti-pattern. In this case, doubly so, because Visio::Render does not treat
elements of such a vector as const.
fix: do not recognize "&;" as an XML escape sequence
xml_isentity was incorrectly recognizing "&;" as an escape sequence. Despite
vague wording in the standard, it seems fairly clear that &<name>; is only
intended to be a valid escape sequence when <name> is non-empty. Fixes #797.
Note that unfortunately due to #1868, we need to fix this bug four times in
copy-pasted versions of the same function.
include stddef.h in some files that are using NULL
These were relying on picking up NULL through one of their other includes. We
are about to remove some internal definitions of NULL that may affect this, so
adding explicit stddef.h includes guards against these upcoming changes.
use a bitfield instead of shifting and masking for reference count in refstr.c
This performs the same operations, but asks the compiler to generate the
required shifts and masks instead of relying on the error prone process of doing
them manually.
fix: put HTML bit at the top of the count in reference counted strings
lib/cgraph/refstr.c implements reference-counted strings with the structure
refstr_t. Since the initial commit of cgraph, d7767d4b5c511afa89b46798eea4e2f915930844, HTML_BIT has been (IMHO incorrectly)
set based on unsigned int instead of unsigned long. Presumably this was not an
issue because most machines were 32-bit x86 at the time, where
sizeof(unsigned int) == sizeof(unsigned long).
In the transition to x86-64 machines, this setup became awkward. The reference
count is 8 bytes wide, but HTML_BIT is set to be bit 31. I.e. the bit indicating
that a string is HTML is now in the middle of the reference count. A string with
≥ 2³¹ references, something that is possible on an x86-64 machine, now
accidentally sets the HTML bit. Note that this ecosystem transition also made
the comment about HTML_BIT incorrect, /* msbit of unsigned long */.
d1244c8001e8c681def4c0ff25a91136845c2a75 did a tree-wide replacement of unsigned
long with uint64_t, which further compounded the situation. Now the HTML bit is
in the incorrect position on both 32-bit and 64-bit x86.
The present change fixes this by moving the HTML bit into bit 63 of the refcnt
field. Fixes #1984.
It is questionable whether this even needs to be a bitfield, and not simply a
uint64_t and a bool, but I think we should leave this space optimization in
place for now.
The macOS CI closed beta program Graphviz has been participating in is winding
down. As of 2021-03-24, we have been requested to migrate to a shared macOS
runner as the program transitions into an open beta [0]. The cut over date for
this is scheduled for 2021-04-07. This change causes our CI setup to now run
macOS jobs on this shared runner instead of the reserved macOS VM Graphviz had
during the closed beta.
Note that this change also migrates these CI jobs from macOS 10.15.6 (Catalina)
to macOS 11.2 (Big Sur). With this, the XCode version is expected to move
forwards as well, though I do not know the precise version numbers.
These tasks were passing despite missing steps to install dependencies because
the current private beta macOS runner Graphviz uses is stateful. Moving to the
stateless shared macOS runner for the open beta exposes that testing fails
because the runtime dependencies of Graphviz are missing.