Skip to content

Updating make/configure for hdrrwr lib includes#12909

Open
ezelkow1 wants to merge 2 commits intoapache:9.2.xfrom
ezelkow1:geo-92x
Open

Updating make/configure for hdrrwr lib includes#12909
ezelkow1 wants to merge 2 commits intoapache:9.2.xfrom
ezelkow1:geo-92x

Conversation

@ezelkow1
Copy link
Member

@ezelkow1 ezelkow1 commented Feb 23, 2026

Try to clean up ambiguities around geo library includes for header rewrite

May help resolve issues around #12862

@ezelkow1 ezelkow1 added this to the 9.2.13 milestone Feb 23, 2026
@ezelkow1 ezelkow1 requested a review from randall February 23, 2026 23:38
@ezelkow1 ezelkow1 self-assigned this Feb 23, 2026
@ezelkow1 ezelkow1 requested a review from bryancall as a code owner February 23, 2026 23:38
@ezelkow1 ezelkow1 added the header_rewrite header_rewrite plugin label Feb 23, 2026
@github-project-automation github-project-automation bot moved this from In progress to Ready to Merge in 9.2.x Branch and Release Feb 24, 2026
@bryancall bryancall requested a review from Copilot February 24, 2026 16:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the build configuration for the header_rewrite plugin to resolve ambiguities in geo library includes. It renames build conditionals from HAS_MAXMINDDB/HAS_GEOIP to USE_HRW_MAXMINDDB/USE_HRW_GEOIP and adds proper validation for the --with-hrw-geo-provider configuration option.

Changes:

  • Renamed build conditionals to use USE_HRW_* prefix instead of HAS_* for clarity
  • Added validation to fail early if a geo provider is specified but the library is not found
  • Improved error message to list valid geo provider options

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
plugins/header_rewrite/Makefile.inc Updated conditional compilation directives to use new USE_HRW_* variable names
configure.ac Added library availability checks, improved error messages, and defined AM_CONDITIONAL macros for the new variable names

Copy link
Contributor

@bryancall bryancall left a comment

Choose a reason for hiding this comment

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

Looks good. The rename from HAS_GEOIP/HAS_MAXMINDDB to USE_HRW_GEOIP/USE_HRW_MAXMINDDB correctly gates the Makefile conditionals on the user's provider selection rather than just library presence. The early-fail validation and improved error message are nice additions.

Minor note: the old AM_CONDITIONAL([HAS_GEOIP], ...) and AM_CONDITIONAL([HAS_MAXMINDDB], ...) definitions are still in configure.ac and are now unused, but that's not a blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

header_rewrite header_rewrite plugin

Projects

Status: Ready to Merge

Development

Successfully merging this pull request may close these issues.

4 participants