Skip to content

GH-48885: [C++] Add missing curl dependency of Arrow::arrow_static CMake target#48891

Merged
kou merged 1 commit intoapache:mainfrom
kou:cpp-curl
Jan 23, 2026
Merged

GH-48885: [C++] Add missing curl dependency of Arrow::arrow_static CMake target#48891
kou merged 1 commit intoapache:mainfrom
kou:cpp-curl

Conversation

@kou
Copy link
Copy Markdown
Member

@kou kou commented Jan 19, 2026

Rationale for this change

We changed macro(build_google_cloud_cpp_storage) to function(...) in GH-48333 . So find_curl() doesn't change ARROW_SYSTEM_DEPENDENCIES in parent scope. (function() creates a new scope.)

What changes are included in this PR?

Move find_curl() to the top-level from in function(build_google_cloud_cpp_storage).

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #48885 has been automatically assigned in GitHub to PR creator.

@kou kou added the CI: Extra: Package: Linux Run extra Linux Packages CI label Jan 19, 2026
Copy link
Copy Markdown
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

CI failures are unrelated and should be fixed by:

Let's wait and rebase to have a clean CI.

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jan 19, 2026
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jan 20, 2026
@kou kou force-pushed the cpp-curl branch 5 times, most recently from 6f73a88 to b2de466 Compare January 21, 2026 06:27
@kou
Copy link
Copy Markdown
Member Author

kou commented Jan 22, 2026

Rebased. This is ready. CI failures are fixed.

Copy link
Copy Markdown
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

LGTM.
@kou if I understand correctly the changes on the verify-xxx.sh scripts are so this will be surfaced on the Linux packaging jobs so we can prevent for these type of things happening again before releasing?

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Jan 22, 2026
@kou
Copy link
Copy Markdown
Member Author

kou commented Jan 23, 2026

Correct. We can detect similar problems by new checks in verify-XXX.sh.

@kou kou merged commit 9489a66 into apache:main Jan 23, 2026
74 of 75 checks passed
@kou kou removed the awaiting merge Awaiting merge label Jan 23, 2026
@kou kou deleted the cpp-curl branch January 23, 2026 02:51
@conbench-apache-arrow
Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 9489a66.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 31 possible false positives for unstable benchmarks that are known to sometimes produce them.

raulcd pushed a commit that referenced this pull request Feb 3, 2026
…CMake target (#48891)

### Rationale for this change

We changed `macro(build_google_cloud_cpp_storage)` to `function(...)` in GH-48333 . So `find_curl()` doesn't change `ARROW_SYSTEM_DEPENDENCIES` in parent scope. (`function()` creates a new scope.)

### What changes are included in this PR?

Move `find_curl()` to the top-level from in `function(build_google_cloud_cpp_storage)`.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* GitHub Issue: #48885

Authored-by: Sutou Kouhei <kou@clear-code.com>
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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants