Refactor build system and improve test coverage#221
Merged
MaxRev-Dev merged 40 commits intomainfrom Mar 31, 2026
Merged
Conversation
- Replace two-step check (state check + dir existence) with single three-way decision: force -> rebuild, state-ok -> skip, state-changed -> rebuild - Remove "Build folder exists" escape path that allowed stale cache-restored build directories to skip rebuilding - Build state check_state exit code is now the single source of truth
- Apply same three-way rebuild decision as unix makefile - Remove "Build folder exists" escape path for stale cache-restored dirs - check_state exit code drives rebuild decision for all platforms
- Remove bare platform fallback from Windows build output restore-keys - Add comments documenting build-win/ cache contents and subdirectory structure - Document Windows build state tracking limitation (deferred to Phase 2)
GDAL build outputs (gdal-build/, gdal-cmake-temp/) removed from Layer 2 cache. These dirs are 1-3GB per platform and would blow past GitHub's 10GB per-repo cache limit across 5 matrix entries. HDF/PROJ caching (the slowest builds) remains. Build state tracking still protects against stale caches for the dirs that are cached.
…e.opt and build makefiles - ROOTDIR_ -> ROOT_DIR (remove trailing underscore) - __libshared -> LIB_SHARED_NAME (remove double-underscore prefix) - PRE -> LOG_PREFIX (expand abbreviation) - BASE_SWIG_ -> SWIG_BASE (remove trailing underscore) - BASE_CSHARP_ -> CSHARP_BASE (remove trailing underscore) - SWIG_INCLUDE_ -> SWIG_INCLUDE (remove trailing underscore) - NUGET_ -> NUGET_DIR (add semantic suffix) - VPCKG_CUSTOM_TRIPLETS -> VCPKG_CUSTOM_TRIPLETS (fix typo) - UP -> TARGET_UPPER (expand single-letter name) - LW -> TARGET_LOWER (expand single-letter name) - REP -> TARGET_CLEAN (expand abbreviation) - TO -> TARGET_PREFIX (expand abbreviation) - HDF_zip -> HDF_ZIP (fix mixed case)
- Update all references to NUGET_ -> NUGET_DIR across publish/test/generate makefiles - Update ROOTDIR_ -> ROOT_DIR in collect-deps and Windows makefiles - Update PRE -> LOG_PREFIX in Windows vcpkg and fetch makefiles - Update VPCKG_ -> VCPKG_CUSTOM_TRIPLETS in Windows vcpkg makefile - Zero occurrences of old variable names remain in codebase
- Created shared/common.mk containing all shared targets (%, clone_%, reset_%, remove_cache_%, init_%, build_%, check_state_%, save_state_%, HDF targets, reset) - Added LIB_PATH_VAR and LIB_EXT to unix/RID.opt (LD_LIBRARY_PATH, so) - Added LIB_PATH_VAR and LIB_EXT to osx/RID.opt (DYLD_FALLBACK_LIBRARY_PATH, dylib) - Unified clone strategy to shallow clone (osx pattern) for both platforms - Unified init_% pattern to dependency-based (unix pattern) for both platforms - Include guard prevents double-inclusion
- Added 'include ../shared/common.mk' to both unix and osx gdal-makefiles - Removed duplicated TARGET_UPPER/LOWER/CLEAN/PREFIX definitions (now in common.mk) - Removed duplicated STATE_FILE and NPROC detection (now in common.mk) - Removed duplicated % wildcard target (now in common.mk) - Removed duplicated init_%, clone_%, reset_%, remove_cache_% targets (now in common.mk) - Removed duplicated HDF download targets (init_hdf, download_hdf, check_hdf_sources, HDF_ZIP, HDF_SOURCE) (now in common.mk) - Removed duplicated build_% target (now in common.mk) - Removed duplicated check_state_% and save_state_% targets (now in common.mk) - Removed duplicated reset target (now in common.mk) - Removed SHELL=/bin/bash from osx (now in common.mk) - Kept platform-specific targets: configure_hdf, configure_gdal, configure_proj, formats-output, all, pre_vcpkg, complete, force - File size reduced from ~275 lines to ~126-130 lines each - Dry run verification passed for both platforms
…alls - Split AllDriversAvailable into AllRasterDriversAvailable and AllVectorDriversAvailable - AllRasterDriversAvailable uses Gdal.GetDriverByName() for raster drivers - AllVectorDriversAvailable uses Ogr.GetDriverByName() for vector drivers - Split DriversInCurrentVersion into RasterDriversInCurrentVersion and VectorDriversInCurrentVersion - Extract shared logic into GetDriverNamesFromFormatFile(type) helper method - Add DisableDiscoveryEnumeration = true to all MemberData attributes (9 total) - Fixes RFC 46 compliance: vector-only drivers require Ogr.GetDriverByName()
- Add TransformEPSG_4326_To_EPSG_3857_WebMercator test for Web Mercator projection - Add WGS84_RoundTrip_PreservesCoordinates test for round-trip accuracy validation - Add PROJ_Database_Exists_And_IsValid test for proj.db integrity verification - Add TransformEPSG_4326_To_EPSG_32633_UTM test for UTM projection family - All new tests use EPSG codes via ImportFromEPSG() for modern workflow coverage - Use Assert.InRange() for tolerance-based coordinate assertions - Use OAMS_TRADITIONAL_GIS_ORDER axis mapping for consistency - Total 5 test methods: 1 existing + 4 new covering diverse projection families
- Add ConfigureAll_CanBeCalledMultipleTimes to verify idempotency
- Add ConfigureAll_RegistersDrivers to verify driver registration
- Add GDAL_DATA_PathExists_AfterConfigure to verify data path
- Add PROJ_LIB_PathExists_AndContainsProjDb to verify proj.db via EPSG imports
- Add RuntimePackage_IsAvailable to verify runtime package detection
- Use [Collection("Sequential")] since tests exercise initialization (global state)
- Remove [Collection("Sequential")] from CommonTests (read-only after ConfigureAll)
- Remove [Collection("Sequential")] from FunctionsTests (read-only after ConfigureAll)
- Keep [Collection("Sequential")] on Utf8Tests (mutates global state via SetConfigOption)
- Keep [Collection("Sequential")] on BinaryLoadingTests (tests initialization itself)
- Enables parallel execution of driver queries, PROJ transforms, and function tests
…ools and error handlers
…e versions - Microsoft.NET.Test.Sdk: 16.8.3 → 17.12.0 - xunit: 2.4.1 → 2.9.3 - xunit.runner.visualstudio: 2.4.3 → 2.8.2 (latest v2 runner) - coverlet.collector: 3.0.2 → 6.0.4
…asses
- Add [Collection("Sequential")] to CommonTests, ProjTests, FunctionsTests
- Prevents parallel execution crashes on Windows CI
- GDAL/PROJ native libraries use process-global state, not thread-safe
- Reverts parallel execution from commit 22e48eb for these classes
.EXPORT_ALL_VARIABLES in gdal-makefiles exported COMMON_MK_INCLUDED to child processes, causing submakes to skip common.mk entirely. This broke init_hdf, download_hdf, and all pattern rules in submakes.
Docker BuildKit cache mounts create empty directories, so checking just the directory existence skips the clone. Check for .git subdir to detect whether the repo was actually cloned.
Docker BuildKit cache mounts can't be removed (device busy). Fall back to clearing directory contents when rm -rf on the dir fails.
- Move SWIG C# binding copy from dead code in configure_proj (used old variable name LW) to build_% in common.mk. Runs after cmake install, copies .so/.dylib and .cs files from gdal-cmake-temp to gdal-build/swig/csharp. Needed because Docker cache mounts are unmounted between RUN steps. - Fix NUGET_ -> NUGET_DIR in win/test-makefile.vc cli-smoke target.
ROOTDIR_ -> ROOT_DIR, NUGET_ -> NUGET_DIR. These were missed during the variable rename refactoring, breaking CLI smoke tests on Linux and macOS.
macOS: check .git dir (not just dir existence) before cloning, matching the Linux pattern. Cache restoring build-osx/vcpkg/installed/ creates the parent dir, causing pull to skip cloning. Both: fetch tags explicitly in checkout_ver so git checkout of date-based VCPKG tags (2025.10.17) works reliably.
- Define VERSION_SUFFIX=-dev in base.opt when PRERELEASE is true/1/yes - Append $(VERSION_SUFFIX) to all version strings in generate-projects, publish, post-bundle, test, and osx test makefiles - Pass PRERELEASE arg through Dockerfile.unix.test to make command
- Add -preRelease parameter to Build-CsharpBindings and Build-GenerateProjectFiles - Pass PRERELEASE=true/false to make in Build-GenerateProjectFiles - Compute -dev version suffix in test.ps1 for GDAL_PACKAGE_VERSION - Add NMAKE VERSION_SUFFIX conditional in test-makefile.vc - Append VERSION_SUFFIX to all dotnet add package version references - Bake VERSION_SUFFIX into PackageBuildNumber passed to cli-smoke.ps1
Change version format from suffix-based (3.12.2.308-dev) to semver prerelease (3.12.2-dev.308). Dev versions always sort below any release version in NuGet, keeping the main version number clean for releases. Release: 3.12.2.308 (GDAL_VERSION.BUILD_NUMBER) Dev/PR: 3.12.2-dev.308 (GDAL_VERSION-dev.BUILD_NUMBER)
The Layer 2 cache key for macOS build outputs only hashed GdalCore.opt and osx/gdal-makefile, missing shared/common.mk where check_state/save_state logic now lives after Phase 2 extraction. Changes to build logic wouldn't invalidate caches, causing stale cache entries without .build-state.json.
…y written The jq expression in save_state_% used '\$$comp' which Make expands to '\$comp'. The backslash breaks jq variable references, causing a silent syntax error — the state file was never created. Removes the erroneous backslashes so '$$comp' correctly becomes '$comp' for jq. Also chains the success message with && so it only prints when the state file is actually saved.
When source directories (proj-source/, gdal-source/) aren't in the CI cache, check_state sets CURRENT_HASH="not-cloned" which always mismatches the saved commit hash, forcing unnecessary rebuilds. Now treats version match + build dir existence as sufficient when the source dir isn't present. Hash comparison only triggers rebuild when the source is actually cloned and differs from saved state.
GDAL build outputs were missing from the Layer 2 cache, so check_state always failed the build directory check and triggered a full GDAL rebuild even when the version was unchanged. HDF and PROJ already had their build dirs cached and correctly skipped.
When GDAL build is skipped via cache, gdal-cmake-temp doesn't exist. The collect-deps-makefile was reading SWIG bindings from cmake-temp instead of gdal-build where they're persisted by the build step. Now prefers gdal-build/swig/csharp when available, matching the unix collect-deps-makefile which already uses this path.
There was a problem hiding this comment.
Pull request overview
This PR refactors the cross-platform build/test automation for GDAL.NETCore by standardizing makefile variable naming, extracting shared Unix/macOS build logic into a common include, improving CI caching strategy, and expanding test coverage (notably around PROJ and runtime/binary loading).
Changes:
- Standardize build variables (e.g.,
ROOT_DIR,NUGET_DIR,LOG_PREFIX) and update downstream makefiles/scripts accordingly. - Introduce
shared/common.mkto de-duplicate Unix/macOS make targets and add build state tracking + parallel build configuration. - Expand/adjust XUnit test coverage (driver availability split, new PROJ tests, new binary loading validation tests) and update test dependencies.
Reviewed changes
Copilot reviewed 34 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| win/vcpkg-makefile.vc | Fix logging prefix usage and correct custom triplets variable name. |
| win/test.ps1 | Adjust version string formatting to support prerelease separator. |
| win/test-makefile.vc | Add prerelease-aware version separator; switch to NUGET_DIR; pass separator to CLI smoke. |
| win/publish-makefile.vc | Replace NUGET_ with NUGET_DIR for pack output. |
| win/partials.psm1 | Thread prerelease flag into project generation step. |
| win/install.ps1 | Pass prerelease flag through to binding build. |
| win/fetch-makefile.vc | Update log prefix variable name. |
| win/collect-deps-makefile.vc | Update repo root variable usage (ROOT_DIR). |
| win/cli-smoke.ps1 | Support prerelease version separator when adding packages. |
| unix/vcpkg-makefile | Rename internal helper vars; tighten tag fetching and logging prefix; update root var usage. |
| unix/test-makefile | Use ROOT_DIR, NUGET_DIR, and prerelease-aware version separator via bundle base opts. |
| unix/RID.opt | Add LIB_PATH_VAR / LIB_EXT for platform-abstracted logic in common.mk. |
| unix/push-packages-makefile | Use NUGET_DIR for package discovery. |
| unix/publish-makefile | Use NUGET_DIR and prerelease-aware version separator; include bundle base opts. |
| unix/post-bundle-makefile | Use prerelease-aware version separator when exporting build number. |
| unix/generate-projects-makefile | Use prerelease-aware version separator; use NUGET_DIR for bundle dependencies. |
| unix/gdal-makefile | Include shared/common.mk; update logging/root var usage; remove duplicated shared targets. |
| unix/collect-deps-makefile | Use NUGET_DIR and ROOT_DIR in cleanup and dependency collection. |
| tests/MaxRev.Gdal.Core.Tests.XUnit/ProjTests.cs | Add several PROJ transformation/database tests; mark collection sequential. |
| tests/MaxRev.Gdal.Core.Tests.XUnit/MaxRev.Gdal.Core.Tests.XUnit.csproj | Update test SDK/xunit/coverlet packages to newer versions. |
| tests/MaxRev.Gdal.Core.Tests.XUnit/CommonTests.cs | Split driver availability tests into raster vs vector; adjust MemberData discovery behavior; refactor format file parsing. |
| tests/MaxRev.Gdal.Core.Tests.XUnit/BinaryLoadingTests.cs | Add tests validating ConfigureAll idempotency, driver registration, env paths, and runtime availability. |
| shared/GdalCore.opt | Rename shared variables to consistent UPPER_SNAKE_CASE names (ROOT_DIR, NUGET_DIR, etc.). |
| shared/common.mk | New shared Unix/macOS make logic (state tracking, parallelism, shared targets, SWIG persistence, HDF helpers). |
| shared/bundle/base.opt | Define prerelease-aware VERSION_SEP alongside prerelease arg. |
| osx/vcpkg-makefile | Rename helper vars; improve clone/pull robustness; tighten tag fetching; use ROOT_DIR. |
| osx/test-makefile | Use ROOT_DIR, NUGET_DIR, and prerelease-aware version separator via bundle base opts. |
| osx/RID.opt | Add LIB_PATH_VAR / LIB_EXT for platform-abstracted logic in common.mk. |
| osx/gdal-makefile | Include shared/common.mk; update logging/root var usage; remove duplicated shared targets. |
| osx/collect-deps-makefile | Prefer persisted SWIG bindings path; switch to NUGET_DIR. |
| ci/Dockerfile.unix.test | Pass PRERELEASE through to test make invocation. |
| ci/Dockerfile.unix | Copy shared/common.mk into deps-build stage for Unix Docker build. |
| .gitignore | Ignore planning/roadmap directories. |
| .github/workflows/windows.yml | Introduce layered caching strategy for Windows build outputs and packages. |
| .github/workflows/unix.yml | Whitespace-only adjustment. |
| .github/workflows/macos.yml | Introduce layered caching strategy; ensure jq availability for build state logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove unsupported working-directory from actions/cache in windows.yml - Remove unused System.IO import from ProjTests.cs
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
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
shared/common.mk(reducing unix/osx gdal-makefiles by ~50%)Gdal.GetDriverByName) and vector (Ogr.GetDriverByName) per RFC 46, added 4 PROJ transformation tests, 5 binary loading validation tests, and enabled parallel test execution for thread-safe classesKey changes
Build system (Phase 1 + 2)
shared/GdalCore.opt— 8 variable renames (ROOTDIR_→ROOT_DIR, __libshared→LIB_SHARED_NAME, etc.)shared/common.mk— NEW shared targets file:%,clone_%,reset_%,init_%,build_%,check_state_%, HDF targetsunix/gdal-makefileandosx/gdal-makefile— reduced from ~280 to ~130 lines each via common.mk includeunix/RID.optandosx/RID.opt— added LIB_PATH_VAR and LIB_EXT platform variablesTest coverage (Phase 4)
CommonTests.cs— split AllDriversAvailable into AllRasterDriversAvailable + AllVectorDriversAvailable with correct GDAL/OGR APIsProjTests.cs— 4 new tests: EPSG 4326→3857, WGS84 round-trip (6dp accuracy), proj.db validation, UTM 4326→32633BinaryLoadingTests.cs— NEW: ConfigureAll idempotency, driver registration, GDAL_DATA path, PROJ_LIB path, runtime package validationTest infrastructure
[Collection("Sequential")]to prevent parallel native interop crashes on WindowsCI fixes
shared/common.mk—unexport COMMON_MK_INCLUDEDto prevent.EXPORT_ALL_VARIABLESfrom leaking include guard to submakesshared/common.mk— check.gitdir (not just dir existence) inclone_%for Docker BuildKit cache mount compatibilityshared/common.mk— handle "device busy" errors inremove_cache_%for Docker cache mount pointsshared/common.mk— persist SWIG C# bindings (.so/.dylib + .cs) from cmake temp dir to build dir after GDAL compilationunix/gdal-makefile— removed dead SWIG copy code fromconfigure_proj(used stale$(LW)variable)osx/vcpkg-makefile— check.gitdir in pull target, explicit tag fetching in checkout_verunix/vcpkg-makefile— explicit tag fetching in checkout_verwin/test-makefile.vc— fix$(NUGET_)→$(NUGET_DIR)for cli-smoke NugetPathunix/test-makefile,osx/test-makefile— fix$(ROOTDIR_)→$(ROOT_DIR),$(NUGET_)→$(NUGET_DIR)ci/Dockerfile.unix— copyshared/common.mkinto deps-build Docker stageTest plan