Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ h3/test/results
/h3/test/sql/ci-*.sql
/libh3-*/
/h3-*.zip
/h3-*.tar.gz
*.BAK

# IDEs
Expand Down
12 changes: 12 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,15 @@ add_custom_target(pgxn
-o h3-${INSTALL_VERSION}.zip HEAD
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
)
add_custom_target(source_bundle
COMMAND ${CMAKE_COMMAND}
-DARCHIVE_OUTPUT=${CMAKE_CURRENT_SOURCE_DIR}/h3-${INSTALL_VERSION}.tar.gz
-DARCHIVE_PREFIX=h3-${INSTALL_VERSION}
-DH3_SOURCE_DIR=${h3_SOURCE_DIR}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Populate H3_SOURCE_DIR before invoking the bundle script

When running scripts/bundle, this expands to -DH3_SOURCE_DIR= because h3_SOURCE_DIR is populated by FetchContent_MakeAvailable(h3) inside cmake/h3/CMakeLists.txt; CMake variables set in a subdirectory do not propagate back to the parent. BundleSource.cmake treats an empty H3_SOURCE_DIR as fatal, so the new source_bundle target fails before producing the release tarball. Export the path from the subdirectory scope before passing it here.

Useful? React with 👍 / 👎.

-DMETA_JSON=${CMAKE_BINARY_DIR}/META.json
-DPROJECT_BINARY_DIR=${CMAKE_BINARY_DIR}
-DPROJECT_SOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR}
-P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/BundleSource.cmake
Comment thread
coderabbitai[bot] marked this conversation as resolved.
DEPENDS pgxn
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
)
34 changes: 34 additions & 0 deletions cmake/BundleSource.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
foreach(required_var ARCHIVE_OUTPUT ARCHIVE_PREFIX H3_SOURCE_DIR META_JSON PROJECT_BINARY_DIR PROJECT_SOURCE_DIR)
if(NOT DEFINED ${required_var} OR "${${required_var}}" STREQUAL "")
message(FATAL_ERROR "${required_var} is required")
endif()
endforeach()

set(bundle_root "${PROJECT_BINARY_DIR}/source-bundle")
set(staging_dir "${bundle_root}/${ARCHIVE_PREFIX}")
set(archive_tar "${PROJECT_BINARY_DIR}/${ARCHIVE_PREFIX}.git.tar")

file(REMOVE_RECURSE "${bundle_root}" "${archive_tar}" "${ARCHIVE_OUTPUT}")
file(MAKE_DIRECTORY "${bundle_root}")

execute_process(
COMMAND git archive --format tar --prefix=${ARCHIVE_PREFIX}/ -o "${archive_tar}" HEAD
WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}"
COMMAND_ERROR_IS_FATAL ANY
)
execute_process(
COMMAND "${CMAKE_COMMAND}" -E tar xf "${archive_tar}"
WORKING_DIRECTORY "${bundle_root}"
COMMAND_ERROR_IS_FATAL ANY
)

file(COPY "${META_JSON}" DESTINATION "${staging_dir}")
file(REMOVE_RECURSE "${staging_dir}/cmake/h3/upstream")
file(MAKE_DIRECTORY "${staging_dir}/cmake/h3/upstream")
file(COPY "${H3_SOURCE_DIR}/" DESTINATION "${staging_dir}/cmake/h3/upstream")

execute_process(
COMMAND "${CMAKE_COMMAND}" -E tar czf "${ARCHIVE_OUTPUT}" --format=gnutar "${ARCHIVE_PREFIX}"
WORKING_DIRECTORY "${bundle_root}"
COMMAND_ERROR_IS_FATAL ANY
)
28 changes: 23 additions & 5 deletions cmake/h3/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,30 @@ set(ENABLE_FORMAT OFF)
set(ENABLE_LINTING OFF)
set(ENABLE_DOCS OFF)

FetchContent_Declare(
h3
URL https://github.com/uber/h3/archive/refs/tags/v${H3_CORE_VERSION}.tar.gz
URL_HASH SHA256=${H3_CORE_SHA256}
)
set(H3_BUNDLED_SOURCE_DIR "${CMAKE_CURRENT_LIST_DIR}/upstream")
if(EXISTS "${H3_BUNDLED_SOURCE_DIR}/CMakeLists.txt")
file(READ "${H3_BUNDLED_SOURCE_DIR}/VERSION" H3_BUNDLED_VERSION)
string(STRIP "${H3_BUNDLED_VERSION}" H3_BUNDLED_VERSION)
if(NOT H3_BUNDLED_VERSION VERSION_EQUAL H3_CORE_VERSION)
message(FATAL_ERROR
"Bundled H3 source version ${H3_BUNDLED_VERSION} does not match "
"configured H3 core version ${H3_CORE_VERSION}"
)
endif()

FetchContent_Declare(
h3
SOURCE_DIR "${H3_BUNDLED_SOURCE_DIR}"
)
else()
FetchContent_Declare(
h3
URL https://github.com/uber/h3/archive/refs/tags/v${H3_CORE_VERSION}.tar.gz
URL_HASH SHA256=${H3_CORE_SHA256}
)
endif()
FetchContent_MakeAvailable(h3)
set(h3_SOURCE_DIR "${h3_SOURCE_DIR}" CACHE PATH "Vendored H3 source directory" FORCE)

set_target_properties(h3 PROPERTIES
INTERPROCEDURAL_OPTIMIZATION TRUE
Expand Down
15 changes: 14 additions & 1 deletion scripts/bundle
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
#!/usr/bin/env bash

set -euo pipefail

die() {
echo "bundle: $*" >&2
exit 1
}

require_clean_tracked_tree() {
git diff --quiet || die "tracked worktree changes are present"
git diff --cached --quiet || die "staged changes are present"
}

require_clean_tracked_tree
Comment on lines +10 to +15

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Also guard against untracked files before cleaning.

This only checks tracked/staged edits, but the next git clean -xdf will still delete untracked/ignored files after the script reports the tree as acceptable. That is an easy way to lose local packaging inputs or release artifacts. Either fail on untracked content here as well, or make the destructive clean step explicitly opt-in.

Suggested change
 require_clean_tracked_tree() {
   git diff --quiet || die "tracked worktree changes are present"
   git diff --cached --quiet || die "staged changes are present"
+  git status --porcelain --untracked-files=all | grep -q '^\?\?' &&
+    die "untracked files are present"
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
require_clean_tracked_tree() {
git diff --quiet || die "tracked worktree changes are present"
git diff --cached --quiet || die "staged changes are present"
}
require_clean_tracked_tree
require_clean_tracked_tree() {
git diff --quiet || die "tracked worktree changes are present"
git diff --cached --quiet || die "staged changes are present"
git status --porcelain --untracked-files=all | grep -q '^\?\?' &&
die "untracked files are present"
}
require_clean_tracked_tree
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/bundle` around lines 10 - 15, The cleanup gate in
require_clean_tracked_tree only checks tracked and staged changes, so it can
still proceed when untracked files exist and are about to be removed by the
later git clean -xdf step. Update require_clean_tracked_tree in scripts/bundle
to also detect untracked content (or otherwise require explicit opt-in before
destructive cleanup) so the script fails before deleting local inputs or
artifacts.

git clean -xdf
cmake -B build -DCMAKE_BUILD_TYPE=Release
cmake --build build --target=pgxn
cmake --build build --target=source_bundle
2 changes: 1 addition & 1 deletion scripts/release
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ main() {
echo " - review and commit the release-${version} changes"
echo " - push and merge the release branch"
echo " - have the release manager publish GitHub release v${version} from CHANGELOG.md"
echo " - run scripts/bundle and upload to PGXN"
echo " - run scripts/bundle, upload h3-${version}.zip to PGXN, and attach h3-${version}.tar.gz to the GitHub release"
echo " - run scripts/postrelease after the release is merged"
}

Expand Down
Loading