fix: embed bundled dxflib directly into libgerbv#369
Closed
rampageservices wants to merge 1 commit intogerbv:developfrom
Closed
fix: embed bundled dxflib directly into libgerbv#369rampageservices wants to merge 1 commit intogerbv:developfrom
rampageservices wants to merge 1 commit intogerbv:developfrom
Conversation
This was referenced Mar 6, 2026
Collaborator
|
That math fix isn't working for me yet, don't merge it yet please. |
9c5c7d4 to
c791b47
Compare
When system dxflib is absent, the bundled copy was built as a separate STATIC library and linked via the compilation-flags INTERFACE target. This works for the gerbv executable, but libgerbv.a never embeds the dxflib object files — they appear as undefined symbols, breaking downstream consumers like pcb2gcode. Replace the separate dxflib_bundled STATIC library with a DXFLIB_BUNDLED_SOURCES list that gets compiled directly into the gerbv library target via target_sources(). This ensures the dxflib symbols are part of libgerbv.a itself. The bundled sources now inherit -Wall from compilation-flags, which surfaces GCC false-positive -Wuninitialized warnings in DL_HatchEdgeData. Suppress these per-file since we don't maintain the dxflib code (tracked in gerbv#372). Also define _USE_MATH_DEFINES on MSVC via compilation-flags so that M_PI/M_1_PI (used by DEG2RAD/RAD2DEG in gerbv.h) are available under strict ANSI conformance. A header-level define doesn't work because several source files include <math.h> before gerbv.h. Fixes gerbv#366 Co-Authored-By: eyal0 <109809+eyal0@users.noreply.github.com>
c791b47 to
811529a
Compare
Contributor
Author
|
Closing in favor of #374 — spe-ciellt is restructuring the thirdparty CMake layout (#373) and it makes sense to land that first. The bug fixes from this PR (dxflib symbol embedding, warning suppression, MSVC |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When system
dxflibis absent, the bundled copy is built as a separateSTATIClibrary (dxflib_bundled) and linked via thecompilation-flagsINTERFACE target. The gerbv executable links fine, butlibgerbv.anever embeds the dxflib object files — they show up as*UND*, breaking downstream consumers like pcb2gcode.This PR:
dxflib_bundledSTATIC library with aDXFLIB_BUNDLED_SOURCESvariablegerbvlibrary target viatarget_sources(), so symbols are embedded inlibgerbv.a-Wno-uninitializedon the bundled dxflib files (GCC false positive onDL_HatchEdgeDatadefault construction, tracked in Bundled dxflib: DL_HatchEdgeData triggers -Wuninitialized with -Werror #372)_USE_MATH_DEFINESon MSVC viacompilation-flagssoM_PI/M_1_PI(used byDEG2RAD/RAD2DEG) are available under strict ANSI conformance — a header-level define ingerbv.hdoesn't work becausedraw.c,draw-gdk.c, andexport-isel-drill.cinclude<math.h>beforegerbv.hFixes #366
Approach
@eyal0's PR #368 identified the same root cause and proposed an
OBJECTlibrary approach. This PR takes a simpler path: compiling the bundled.cppfiles directly intolibgerbvviatarget_sources(), avoiding the extra CMake target entirely.Test plan
cmake --buildwith no system dxflib → bundled sources compile into libgerbvnm libgerbv.a | grep dl_dxf→ symbols are defined (T), not undefined (U)cmake --buildwith system dxflib installed → pkg-config path still worksCMAKE_COMPILE_WARNING_AS_ERROR ON(no dxflib warnings)M_PIresolves via the compile definition