Modernize the xdelta3 build#283
Merged
Merged
Conversation
The codebase compiled and passed its built-in tests, but the scaffolding had bitrotted. This restores a clean build on modern toolchains. - xdelta3.h used static_assert without including <assert.h>; moreover the <assert.h> macro only exists for C11+ while the build targets C99. Replace with a portable XD3_STATIC_ASSERT macro (_Static_assert under C11+, a negative-size-array typedef under C99). - Add a cross-platform CMake build (config.h.cmake -> config.h) that detects type sizes, liblzma, and unaligned-access support, and builds xdelta3, xdelta3decode, xdelta3regtest, and xdelta3checksum. `ctest` runs the built-in regression test. - Add a GitHub Actions CI workflow building on Linux and macOS, with and without liblzma. - configure.ac: AC_HELP_STRING -> AS_HELP_STRING (the former is removed in modern autoconf). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove testing/xdelta3-test.py and testing/xdelta3-regtest.py. Both
`import xdelta` (the SWIG Python binding that Makefile.am records as
"broken, removed from distribution") and rely on Python-2-only APIs
(print statements, os.popen3) and RCS tooling. They cannot run.
- Modernize the Go regression harness:
- Convert the pre-modules GOPATH layout (go/src/...) to a Go module
(go/go.mod, module github.com/jmacd/xdelta/xdelta3/go); the package
moves to go/xdelta and the driver to go/regtest.go.
- Replace hard-coded absolute binary/dataset paths with flags
(-xdelta3, -compare, -dataset, -seed, -min-offset, -max-offset); the
dataset comparison test runs only when -dataset and -compare are given.
- Use keyed struct literals and drop a `for k, _ := range`, so `go vet`
is clean.
- Add a CI job that builds xdelta3 and runs the Go harness (smoke + small
offset tests) plus `go vet`.
- Makefile.am: update GOLANG_SRCS paths and drop the removed Python files.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add a Building section and CI badge to the top-level README. - Document the CMake build, ctest, liblzma options, and the modernized Go regression harness in xdelta3/README.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CMake is now the supported build system, so retire the GNU Autotools scaffolding: - Delete configure.ac, Makefile.am, generate_build_files.sh, the m4/ macros, and run_release.sh (plus its testing/ wrapper). run_release.sh was an autotools-only release/cross-build driver. - Trim .gitignore to the CMake build directory and generated config.h, dropping the many autotools-generated artifacts. - Update stale references to configure.ac / Makefile.am in CMakeLists.txt, config.h.cmake, xdelta3.c, and README.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sweep of orphaned files left behind by the autotools and Python-binding removals: - Delete rcs_junk.cc (a 1800-line GLib RCS-corpus tool, unreferenced and uncompilable here), xdelta3.i (SWIG interface for the removed Python binding), plot.sh (gnuplot helper for the removed Python benchmark), testing/Makefile (delegated to the deleted autotools Makefile), and badcopy.c (debug fuzz helper driven only by the deleted Python regtest). - Drop the badcopy.c entry from xdelta3.c's file-manifest comment. - Revive linkxd3lib.c as the `xdelta3link` CMake target: it references every public API symbol, so building it verifies the library links and catches xdelta3.h signature drift. This caught a stale call -- xd3_set_appheader gained data/size parameters -- now fixed. It is a build/link check only (not run; it passes NULL arguments by design). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Delete xdelta3.vcxproj (a Visual Studio 2013 project) and the WiX installer definitions xdelta3.wxi / xdelta3.wxs. They are unmaintained and unreferenced by the sources. The portable _WIN32/_MSC_VER code paths in the sources are unaffected; a Windows build is best driven by CMake. Also correct README.md, which still advertised the removed Python API and the VC++/Cygwin builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validate the Windows build on GitHub's free windows-latest runners rather than relying on a hand-maintained Visual Studio project. - CI: add a windows-latest job (MSVC) building and running ctest with liblzma off and on; the lzma=on leg installs liblzma via the runner's preinstalled vcpkg. Force -A x64 (VS generators default to Win32). - CMakeLists: on WIN32, define XD3_WIN32=1 / XD3_POSIX=0 / XD3_STDIO=0 and disable EXTERNAL_COMPRESSION and SHELL_TESTS for the full-source targets, matching the configuration the removed VS project used. Without these the build defaults to the POSIX I/O path and pulls in <unistd.h>. - xdelta3-test.h: make the regression harness portable. It hardcoded "/tmp/..." scratch paths and POSIX getpid(), which fail on Windows; use a temp dir from TEMP/TMP/TMPDIR and a _getpid() shim under XD3_WIN32. The portability fixes were confirmed with a mingw-w64 cross-compile of the XD3_WIN32 path; runtime is validated by the Windows CI job. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The first Windows CI run failed building xdelta3regtest / xdelta3checksum: testing/test.h includes <unistd.h> and the harness uses the __typeof__ extension, neither of which MSVC supports (the old Visual Studio project never built this harness either). The xdelta3 CLI itself compiled fine. Gate those two targets on `if(NOT MSVC)`. The built-in `xdelta3 test` (ctest) and the xdelta3link API/link check still run on Windows, and all targets are unaffected on Linux/macOS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Clean up the warnings surfaced by the MSVC and Clang builds and add a lint gate so they cannot regress. Warning fixes: - Fix four real 64-bit bugs flagged as MSVC C4334: `1 << n` is a 32-bit shift but the result is stored in a 64-bit usize_t with n up to 63 (xdelta3-hash.h, xdelta3-second.h). Use `(usize_t)1 << n`. - Add explicit casts at the ~38 intentional narrowing conversions MSVC reports as C4244/C4267 (codec internals writing bytes/indices), across xdelta3.c and the djw/fgk/hash/blkcache/main/test headers. No behavior change; the truncations are now documented. - Replace the incompatible `(xd3_posix_func*) &read/&write` casts with thin wrapper functions, fixing -Wcast-function-type on GCC and Clang. - Define _CRT_SECURE_NO_WARNINGS on MSVC to silence the C4996 "unsafe" nags about standard functions like snprintf. Lint gate: - Add an XD3_WERROR option that turns on /WX (MSVC) and -Werror (Clang) for the core targets (xdelta3, xdelta3decode, xdelta3link). GCC is excluded: it emits many additional, largely false-positive diagnostics (implicit-fallthrough, format-truncation, stringop-overflow) that are out of scope. The C++ test harness and vendored cpp-btree are also excluded as they carry their own pre-existing warnings. - Enable XD3_WERROR=ON in the CMake and Windows CI jobs. Also bump actions/checkout from v4 to v5. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The first /WX run still failed on C4996: the define was only on targets that received XD3_PLATFORM_DEFS (so xdelta3decode missed it), and the POSIX-name deprecations (unlink, fileno) need _CRT_NONSTDC_NO_WARNINGS in addition to _CRT_SECURE_NO_WARNINGS. Define both for every target via the shared helper. All C4244/C4267/C4334 were already eliminated by the casts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
Adds cmake configuration, Linux/Windows/MacOS CI.