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

GH-41426: [R][CI] Install CRAN style openssl on gh runners. #41629

Merged
merged 6 commits into from
May 21, 2024

Conversation

assignUser
Copy link
Member

@assignUser assignUser commented May 12, 2024

Rationale for this change

See issue.

What changes are included in this PR?

Enforce usage of binary and install of cran openssl version on intel and arm macos.

Are these changes tested?

Crossbow

Copy link

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

@assignUser

This comment was marked as outdated.

@github-actions github-actions bot added the awaiting review Awaiting review label May 12, 2024

This comment was marked as outdated.

This comment was marked as outdated.

@assignUser

This comment was marked as outdated.

This comment was marked as outdated.

@assignUser
Copy link
Member Author

assignUser commented May 13, 2024

Partial success? It get's the right binary but still uses the brew openssl include? https://github.com/ursacomputing/crossbow/actions/runs/9056112849/job/24878643760#step:16:66

configure seems to ignore the set Envvar?

Edit: it's not configure, we don't pass the envvar on to cmake which then looks for a brew prefix.

This comment was marked as outdated.

@assignUser
Copy link
Member Author

@github-actions crossbow submit r-binary-packages

Copy link

Revision: f3cf9f8

Submitted crossbow builds: ursacomputing/crossbow @ actions-5ce67b9abe

Task Status
r-binary-packages GitHub Actions

@assignUser assignUser marked this pull request as ready for review May 14, 2024 02:28
@assignUser assignUser requested a review from jonkeane May 14, 2024 02:28
@assignUser
Copy link
Member Author

assignUser commented May 14, 2024

That worked, we never actually passed the envvar to cmake but the result was as expected because cmake also looks for a brew prefix. Guess it's good that cran doesn't have openssl brew? ^^ https://github.com/ursacomputing/crossbow/actions/runs/9072462442/job/24928500464#step:16:75

@assignUser assignUser merged commit f0678ec into apache:main May 21, 2024
12 checks passed
@assignUser assignUser removed the awaiting review Awaiting review label May 21, 2024
assignUser added a commit that referenced this pull request May 21, 2024
### Rationale for this change

See issue.

### What changes are included in this PR?

Enforce usage of binary and install of cran openssl version on intel and arm macos.

### Are these changes tested?
Crossbow
* GitHub Issue: #41426

Authored-by: Jacob Wujciak-Jens <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
Copy link

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

There were no benchmark performance regressions. 🎉

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

vibhatha pushed a commit to vibhatha/arrow that referenced this pull request May 25, 2024
…ache#41629)

### Rationale for this change

See issue.

### What changes are included in this PR?

Enforce usage of binary and install of cran openssl version on intel and arm macos.

### Are these changes tested?
Crossbow
* GitHub Issue: apache#41426

Authored-by: Jacob Wujciak-Jens <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant