Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++] Make git-dependent preprocessor definitions internal #41783

Closed
pitrou opened this issue May 22, 2024 · 1 comment
Closed

[C++] Make git-dependent preprocessor definitions internal #41783

pitrou opened this issue May 22, 2024 · 1 comment

Comments

@pitrou
Copy link
Member

pitrou commented May 22, 2024

Describe the enhancement requested

The ARROW_GIT_ID and ARROW_GIT_DESCRIPTION preprocessor variables are currently exposed in arrow/util/config.h, and included from arrow/config.h. This means that any file indirectly including these headers will have to be recompiled if the git information changes - something which happens quite frequently during development.

Using ccache with properly tuned configuration can work around the issue, but does not fully remove overhead. It also requires users to think about the best ccache configuration.

By making those two variable privates, we should fix the problem entirely.

Component(s)

C++

kou pushed a commit that referenced this issue May 22, 2024
### Rationale for this change

Exposing the ARROW_GIT_ID and ARROW_GIT_DESCRIPTION preprocessor variables in our public headers tends to make incremental builds less efficient, since those values change very often during development.

Also, these values don't need to be preprocessor variables since they're just strings: you can't write useful `#if` directives with them. Instead, they can be inspected using `GetBuildInfo()`.

### Are these changes tested?

By existing builds and tests.

### Are there any user-facing changes?

Use cases depending on these preprocessor variables, which is unlikely, may break. They can be fixed by calling `arrow::GetBuildInfo()` instead.

* GitHub Issue: #41783

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 17.0.0 milestone May 22, 2024
@kou
Copy link
Member

kou commented May 22, 2024

Issue resolved by pull request 41781
#41781

@kou kou closed this as completed May 22, 2024
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
…#41781)

### Rationale for this change

Exposing the ARROW_GIT_ID and ARROW_GIT_DESCRIPTION preprocessor variables in our public headers tends to make incremental builds less efficient, since those values change very often during development.

Also, these values don't need to be preprocessor variables since they're just strings: you can't write useful `#if` directives with them. Instead, they can be inspected using `GetBuildInfo()`.

### Are these changes tested?

By existing builds and tests.

### Are there any user-facing changes?

Use cases depending on these preprocessor variables, which is unlikely, may break. They can be fixed by calling `arrow::GetBuildInfo()` instead.

* GitHub Issue: apache#41783

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants