remove advanced target suppression for RxSpencer in CMake builds
This section suppressed detailed messages about where (or not) RxSpencer was
found. Given the majority of CMake builds are performed by either CI or advanced
users, it did not make sense to suppress this information that is very useful
for debugging failures.
fix case warnings for RxSpencer during CMake builds
This fixes the following CI warning:
-- Found RXSPENCER: ...rxspencer.lib
CMake Warning (dev) at ...FindPackageHandleStandardArgs.cmake:272 (message):
The package name passed to `find_package_handle_standard_args` (RXSPENCER)
does not match the name of the calling package (RxSpencer). This can lead
to problems in calling code that expects `find_package` result variables
(e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/FindRxSpencer.cmake:5 (find_package_handle_standard_args)
CMakeLists.txt:90 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
use Bison's api.prefix and Flex's prefix instead of name mangling in cgraph
Related to !1523. Bison's api.prefix was introduced in 2.6, but switched to
using this bracing syntax in 3.0. Quoting from the Bison 3.0 release notes:
** Types of values for %define variables
Bison used to make no difference between '%define foo bar' and '%define
foo "bar"'. The former is now called a 'keyword value', and the latter a
'string value'. A third kind was added: 'code values', such as '%define
foo {bar}'.
Keyword variables are used for fixed value sets, e.g.,
%define lr.type lalr
Code variables are used for value in the target language, e.g.,
%define api.value.type {struct semantic_type}
String variables are used remaining cases, e.g. file names.
The Flex option prefix was introduced in Flex 2.4.1 (November 1993) and then
stabilized around 2.5.2 (April 1995). It provides similar functionality:
changes the default ‘yy’ prefix used by flex for all globally-visible variable
and function names to instead be ‘PREFIX’. For example, ‘--prefix=foo’ changes
the name of yytext to footext. It also changes the name of the default output
file from lex.yy.c to lex.foo.c.
It would have been nicer to split the lexer and parser changes into separate
commits. However, the MSBuild build does no name mangling here (see
lib/cgraph/cgraph.vcxproj), so changing one of these immediately broke the other
one there. On the positive side, this commit has the effect of realigning some
MSBuild outputs with the Autotools build, as the cgraph lexer and parser are now
aag-prefixed there as well.
drop compatibility with Yacc, Bison < 3 in lib/cgraph/grammar.y
This will allow us to use api.prefix with brace syntax in an upcoming commit.
Bison 3.0 was released in July 2013 [0], so this seems an acceptable base line
requirement from now on.
suppress warnings from using Bison extensions in lib/cgraph/grammar.y
We don't currently use any, but are about to introduce one. For some more back
story on this, we quote the Bison 3.0 release notes:
*** Use of YACC='bison -y'
TL;DR: With Autoconf <= 2.69, pass -Wno-yacc to (AM_)YFLAGS if you use
Bison extensions.
Traditional Yacc generates 'y.tab.c' whatever the name of the input file.
Therefore Makefiles written for Yacc expect 'y.tab.c' (and possibly
'y.tab.h' and 'y.outout') to be generated from 'foo.y'.
To this end, for ages, AC_PROG_YACC, Autoconf's macro to look for an
implementation of Yacc, was using Bison as 'bison -y'. While it does
ensure compatible output file names, it also enables warnings for
incompatibilities with POSIX Yacc. In other words, 'bison -y' triggers
warnings for Bison extensions.
Autoconf 2.70+ fixes this incompatibility by using YACC='bison -o y.tab.c'
(which also generates 'y.tab.h' and 'y.output' when needed).
Alternatively, disable Yacc warnings by passing '-Wno-yacc' to your Yacc
flags (YFLAGS, or AM_YFLAGS with Automake).
use abs() instead of ABS() for taking the absolute value of ints
Similar to b88ebe0548dd412ea415d6cfc410d91117270277 and the previous commit,
this replaces uses of the macro ABS with standard library functionality for
readability and maintainability.
This does the equivalent of the previous commit, to now use pkg-config to
discover Cairo on non-Windows platforms and to suppress the CMake warning:
CMake Warning (dev) at ...FindPackageHandleStandardArgs.cmake:272 (message):
The package name passed to `find_package_handle_standard_args` (CAIRO) does
not match the name of the calling package (Cairo). This can lead to
problems in calling code that expects `find_package` result variables
(e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/FindCairo.cmake:12 (find_package_handle_standard_args)
CMakeLists.txt:81 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
Compiling plugin/pango depends on the discovery of PangoCairo and its dependent
libraries which is handled by cmake/FindPangoCairo.cmake. This file was assuming
all libraries were available at default system paths, which is not true on e.g.
macOS when you install libraries via Homebrew or Macports. We now simply ask
pkg-config to find all necessary information for us (unless we are on Windows).
This also incidentally cleans up the following warning issued by CMake in CI:
CMake Warning (dev) at C:/…/FindPackageHandleStandardArgs.cmake:272 (message):
The package name passed to `find_package_handle_standard_args` (PANGOCAIRO)
does not match the name of the calling package (PangoCairo). This can lead
to problems in calling code that expects `find_package` result variables
(e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/FindPangoCairo.cmake:20 (find_package_handle_standard_args)
CMakeLists.txt:85 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
use fabs() instead of ABS() for taking the absolute value of a double
The fabs() function is available via math.h, even prior to C89 so there's no
reason not to use it. Avoiding the macro eliminates some mental overhead for the
reader. Also fabs has some more lenient semantics with respect to denormal
inputs, so this change results in slightly more efficient code.
Magnus Jacobsson [Sat, 22 Aug 2020 08:27:06 +0000 (10:27 +0200)]
Fix platform name in Windows CI artifacts directory
By renaming $Env:platform to $Env:project_platform. This avoids a
clash with the environment set up with vcvarsall.bat which changes
$Env:platform to the *architecture* which is 'x86' or 'x64' while we
use 'Win32' or 'x64' as the *project-level* platform.
From
https://docs.microsoft.com/en-us/visualstudio/ide/how-to-configure-projects-to-target-platforms?view=vs-2019:
"Note
The Win32 platform name is used for C++ projects, and it means
x86. Visual Studio considers both project-level platforms and
solution-level platforms, and the project platforms come from the
language-specific project systems. C++ projects use Win32 and x64, but
the solution platforms use x86 and x64. When you choose x86 as the
solution configuration, Visual Studio selects the Win32 platform for
C++ projects."
Magnus Jacobsson [Sun, 23 Aug 2020 08:25:50 +0000 (10:25 +0200)]
Make parsing of vcvarsall.bat output more robust
Only consider variable names with alpanumeric characters, underscore
and parentheses. This avoids most problems when parsing output
containing multi-line variable values with arbitrary text.
In GitLab CI, one of the environment variables is CI_COMMIT_MESSAGE. In the commit immediately following this commit there is an URL containing an equal sign that caused the error below:
Set-Content : An object at the specified path
env:\https://docs.microsoft.com/en-us/visualstudio/ide/how-to-configure-projects-to-target-platforms?view does not
exist, or has been filtered by the -Include or -Exclude parameter.
At line:3 char:5
+ Set-Content "env:\$($matches[1])" $matches[2];
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : ObjectNotFound: (System.String[]:String[]) [Set-Content], Exception
+ FullyQualifiedErrorId : ItemNotFound,Microsoft.PowerShell.Commands.SetContentCommand
[ 69%] Building C object lib/cgraph/CMakeFiles/cgraph.dir/grammar.c.o
[ 70%] Building C object lib/cgraph/CMakeFiles/cgraph.dir/scan.c.o
/Users/north/src/graphviz/build/lib/cgraph/scan.c:1706:12: error: expected
identifier or '('
extern int isatty (int );
^
scan.l:44:19: note: expanded from macro 'isatty'
#define isatty(x) 0
^
1 error generated.
make[2]: *** [lib/cgraph/CMakeFiles/cgraph.dir/scan.c.o] Error 1
make[1]: *** [lib/cgraph/CMakeFiles/cgraph.dir/all] Error 2
make: *** [all] Error 2
which conveniently explained why this find-and-replace had existed in the first
place. Rather than reverting this, the present change uses a more principled way
of instructing Flex not to call isatty().
Commits 02d2cacf8730f1e4347afd72b159c91fafed2182 and 80a9dc3a3d7a1bff49db3f8e40fc03e91ca2261e accidentally introduced two new
compiler warnings to the build. Following this change, the CMake build returns
to warning-free on Linux with GCC 8.3.0. The Autotools build still sprays
warnings, but I think it's worth taking proactive steps like this to never
introduce new warnings.
Though this change looks like it has a semantic effect, it does not. The base
member of these structs is the first member, used as an inheritance-style
pattern.
use a dynamic buffer when expanding text replacements
This change is intended to guard against bugs like that fixed in fbefeb31989130c48b965cc9a2f0ad0cac07015c. The buffer used to construct the
result string now dynamically expands so we do not need to manually calculate
its extent in advance. Related to #1676.
Instead of retrieving the contents of a (possibly dynamically allocated) buffer
and then allocating new space for a copy, we can simply take ownership of the
original buffer. This saves an extra unnecessary allocation.
Magnus Jacobsson [Fri, 14 Aug 2020 12:55:30 +0000 (14:55 +0200)]
Make the Windows pipeline stop on PowerShell cmdlet failure
Note that this doesn't affect failures from .exe programs (but that is
already handled by the GitLab Shared Windows Runner). See
https://stackoverflow.com/questions/9948517/how-to-stop-a-powershell-script-on-the-first-error.
fix incorrect allocation computation in strdup_and_subst_obj0
The logic in this function does two passes through computing the construction of
a string, the first to determine how many bytes will be written and the second
to write those bytes. Unfortunately the logic in these two passes did not match,
and some cases would cause the first pass to underestimate how many bytes would
be written by the second pass. As a result, the second pass would overrun the
extent of allocated memory and begin writing out of bounds.
As far as I can tell, this bug has existed since at least 2010. I think this
code should be refactored in future to use a dynamically expanding string buffer
(e.g. agxbuf) instead of relying on this fragile pairing of logic.
The test case added in this commit was something found by Google Autofuzz
project that triggered a crash in Graphviz due to reading a NULL pointer. After
fixing the initial crash (the previous commit), this out of bounds write was
observable using ASan. This commit fixes #1676.
fix: avoid accessing min/max rep on a NULL cluster
When entering compile_samerank, the parent cluster could be NULL, which was then
incorrectly dereferenced in the two cases affected in this change. This issue
was found by Google Autofuzz project. Related to #1676.
Magnus Jacobsson [Fri, 14 Aug 2020 10:04:08 +0000 (12:04 +0200)]
Fix publishing of Windows build artifcats
The COLLECTION was missing since we no longer get it from the
portable_source stage because the Windows builds start directly and do
not wait for the portable_source stage. Therefor we generate it in the
Windows builds directly instead.
Magnus Jacobsson [Fri, 14 Aug 2020 09:53:22 +0000 (11:53 +0200)]
Change to all-numeric version entry format and add collection
The version is now specified with a new collection variable which
shall be "stable" or "development" and an all-numeric version. The
"~dev" (and the .<commiter date>) suffix will be added by the script.
The distance between endb and next was being stored in an int variable. While
this should not exceed the bounds of an int during normal usage, it's risky to
store a pointer difference in a narrower type this way.
projects / graphviz / log