GH-49719: [C++] Rename vendored date header guards#49778
Conversation
Prefix all header guards in the vendored date library with ARROW_VENDORED_ to avoid collisions when a project also uses the real date library. Update update.sh to apply the same rewrite automatically on future re-vendoring.
There was a problem hiding this comment.
Pull request overview
This PR updates Arrow’s vendored HowardHinnant/date headers to avoid preprocessor header-guard collisions when downstream projects also include the upstream date library.
Changes:
- Prefixed the vendored date library header guards with
ARROW_VENDORED_indate.h,tz.h,tz_private.h, andios.h. - Updated
update.shto automatically apply the same header-guard rewrite on future re-vendoring.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
cpp/src/arrow/vendored/datetime/update.sh |
Adds a sed rewrite step to prefix specific upstream header guards with ARROW_VENDORED_ during vendoring. |
cpp/src/arrow/vendored/datetime/date.h |
Renames DATE_H guard to ARROW_VENDORED_DATE_H. |
cpp/src/arrow/vendored/datetime/tz.h |
Renames TZ_H guard to ARROW_VENDORED_TZ_H. |
cpp/src/arrow/vendored/datetime/tz_private.h |
Renames TZ_PRIVATE_H guard to ARROW_VENDORED_TZ_PRIVATE_H. |
cpp/src/arrow/vendored/datetime/ios.h |
Renames ios_hpp guard to ARROW_VENDORED_ios_hpp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kou
left a comment
kou
left a comment
There was a problem hiding this comment.
You used this sed command line to update the existing header files, right?
|
@kou i fixed it, can you take a look again? |
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 36c8c9a. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
Arrow ships with a vendored date library that places all definitions in the
arrow_vendorednamespace but keeps the original header guards, leading to problems when used in a project that also consumes the real date library.What changes are included in this PR?
Prefix all header guards with
ARROW_VENDORED_. Updateupdate.shto apply the same rewrite automatically on future re-vendoring.date.hDATE_H→ARROW_VENDORED_DATE_Htz.hTZ_H→ARROW_VENDORED_TZ_Htz_private.hTZ_PRIVATE_H→ARROW_VENDORED_TZ_PRIVATE_Hios.hios_hpp→ARROW_VENDORED_ios_hppupdate.shAre these changes tested?
Built Arrow C++ locally. No build breakage.
Are there any user-facing changes?
No API changes. Fixes a header collision for projects that also use the real date library.