gts_surface_foreach_edge: fix mismatch of calling convention in callback
The compiler said about this code:
delaunay.c: In function ‘edgeStats’:
delaunay.c:245:34: warning: cast between incompatible function types from
‘void (*)(GtsSegment *, estats *)’ {aka ‘void (*)(struct _GtsSegment *,
struct <anonymous> *)’} to ‘gint (*)(void *, void *)’ {aka
‘int (*)(void *, void *)’} [-Wcast-function-type]
gts_surface_foreach_edge (s, (GtsFunc) cnt_edge, sp);
^
This is not quite as bad as the previous instance of this fixed. However, it
seems to have been relying on a coincident return value. E.g. on x86-64 this
code relies on 0 ending up in `RAX` at the end of the callback to indicate
iteration should continue, despite `cnt_edge` having no declared return value.
delaunay_remove_holes: fix mismatch of calling convention in callback
The compiler said about this code:
delaunay.c: In function ‘delaunay_remove_holes’:
delaunay.c:45:9: warning: cast between incompatible function types from
‘gboolean (*)(GtsTriangle *)’ {aka ‘int (*)(struct _GtsTriangle *)’}
to ‘gint (*)(void *, void *)’ {aka ‘int (*)(void *, void *)’}
[-Wcast-function-type]
(GtsFunc) triangle_is_hole, NULL);
This warning is not spurious. In particular, the mismatch in the number of
arguments passed to the callback means the callback function has a different
calling convention than expected by the code calling it. The result of this can
be stack corruption or incorrect interpretation of function arguments.
In practice, all major native calling conventions use registers for both
function types, so this is somewhat benign. However, this likely caused problems
on stack-based environments like WASM and JITs.
Each Graphviz version has a notion of “collection,” which traditionally could be
the values “stable” or “development.” The semantically versioned releases that
are published to the website are considered stable. The inter-release packages
that the deploy.py script still uploads to the Gitlab package repository are
considered development. gen_version.py knows how to determine the collection of
a version during build.
Additionally .gitlab-ci.yml knew of another collection value, “experimental,” of
which gen_version.py was unaware. It would manually tweak the collection to this
for certain non-mainstream builds.
deploy.py knows nothing of this notion of collections.
The combined effect of the above has led to some negative outcomes:
1. Published releases include a mixture of files labelled “stable” and files
labelled “experimental.” It is not clear to the general public what either
of these labels mean or how they differ.
2. In recent times, “development” versions are no more or less stable than
“stable” versions. The terminology here unfortunately has become
inaccurate.
3. The directory hierarchy for published releases has included an extra level
for collection. If deployment worked as intended, this would be redundant
as every release artifact would be “stable.” However, as discussed above,
the deployment script actually publishes “experimental” artifacts as well.
To reduce confusion and simplify build and packaging going forwards, this change
removes the notion of collection. The generated COLLECTION file is no longer
produced and users should have no need to be aware of this concept.
gen_version.py still internally knows the idea of collections to distinguish
published release version numbers from inter-release version numbers. But this
is no longer exposed to the rest of the build system and packaging.
The notion of “experimental” as a collection value is completely removed.
standardize on '/usr/bin/env bash' shebang for CI scripts
This is slightly more portable (e.g. on FreeBSD Bash does not live at
/bin/bash). The previous shebang was fine on all the operating systems we
currently support, but this adds robustness as we expand CI coverage.
Commit d64b75d2704bd3ed47b3f74c0686e19ca74e65fc explicitly enabled Lefty in
several places in preparation for it becoming disabled by default. However, it
accidentally missed the out-of-source build.
While it is probably not a good idea to have a GVC config that is 100000 bytes,
I see no reason it should be arbitrarily rejected. In future, it might be a good
idea to make `gvconfig_plugin_install_from_config` accept a file pointer instead
of a string and read/parse the config file incrementally. If this were done,
then there would probably be no issue at all with large configs.
gvconfig.c: realign alternative for 'glob' with POSIX
If the `glob` system function¹ is available, it is used in this code. If not (on
Windows), an alternative static function is provided. The POSIX spec mandates
only three members of the `glob_t` struct, `gl_pathc`, `gl_pathv`, and
`gl_offs`. This commit realigns the members with their types in the POSIX spec.
This squashes some -Wsign-compare warnings on POSIX platforms.
fix: exclude cmd/smyrna when Smyrna is disabled during build
Similar to the situation just fixed with Lefty, when Smyrna was disabled
(`--with-smyrna=no` passed to the build system), several Smyrna related
artifacts would still be built and installed. We now exclude the entire Smyrna
subdirectory, which should be less error prone.
fix: exclude cmd/lefty when Lefty is disabled during build
When Lefty was disabled (`--enable-lefty` not passed to the build system),
several Lefty related artifacts would still be built and installed. E.g. the man
page. We now exclude the entire Lefty subdirectory, which should be less error
prone.
enable persistent Chocolatey cache in Windows CI jobs
The intention of this change is to accelerate Windows CI jobs, currently one of
the limiting factors on our CI throughput. This change takes a conservative
approach, giving each relevant job its own cache. In theory these jobs could all
share a cache, but the Gitlab docs¹ imply this is only safe if only a single job
has a `push` policy. To do this effectively, we would probably have to create a
new preliminary CI job that only built the cache. Then all other jobs would
become `pull` consumers of this cache. Perhaps an improvement to make in future.
do not create packaging hierarchy directory for architecture
The packaging directories were created based on various dimensions of a matrix,
with architecture at the lowest level. On Linux and macOS, Graphviz is only
packaged for a single architecture (x86-64). On Windows, Graphviz is only
packaged for two architectures (x86 and x86-64). However, the architecture is
also present in the suffix of every packaged file. E.g.
So there is no ambiguity if we drop the architecture directory. This seems to be
a case of YAGNI¹ where the system was setup to accommodate multiple
architectures but this has not been needed. We remove it now to (1) reduce
complexity and (2) have less verbose release filenames.²
Closes #2149.
¹ https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it
² Example of a current verbose release filename,
stable_windows_10_cmake_Release_Win32_graphviz-install-2.49.3-win32.exe
Upcoming changes will add references to functions that are in libgvc. Note that
no changes are required to the MSBuild build system because it already links
Smyrna against libgvc.
gvconfig_libdir: fix const-correctness of char pointers in macOS code
Commit 284df35236e258a49b86fad6f0d062bf0c2fa840 changed the source pointer `tmp`
used here to be `const`. A side effect of this was introducing a warning
breaking the CMake build on macOS:
lib/gvc/gvconfig.c:329:10: error: initializing 'char *' with an expression of
type 'const char *' discards qualifiers
[-Werror,-Wincompatible-pointer-types-discards-qualifiers]
char* s = tmp-1;
^ ~~~~~
This commit was merged via !2231 whose source branch did not have permission to
use the macOS shared runners, so this issue was not caught in pre-merge CI.
projects / graphviz / log