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-27839: [R] Fetch latest nightly binary for arrow R dev versions. #38236

Merged
merged 20 commits into from Oct 25, 2023

Conversation

assignUser
Copy link
Member

@assignUser assignUser commented Oct 12, 2023

Rationale for this change

We currently need to manually download the latest nightly build as the ".9000' dev version never matches any nightly builds.

What changes are included in this PR?

Check the nightly repo html listing for the latest version matching the local major version.
Refactor nixlibs.R and integrate the functionality of winlibs.R into it to simplify maintenance and prepare for the future of in-line windows libarrow builds.

These changes are build on #38195 so once that is merged I will rebase this.

Are these changes tested?

Crossbow

@assignUser
Copy link
Member Author

@github-actions crossbow submit -g r

@github-actions
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@github-actions

This comment was marked as outdated.

@assignUser
Copy link
Member Author

assignUser commented Oct 12, 2023

@github-actions crossbow submit r-binary-packages

@github-actions
Copy link

Revision: 183f365

Submitted crossbow builds: ursacomputing/crossbow @ actions-007951fa57

Task Status
r-binary-packages Github Actions

@github-actions
Copy link

Revision: dd2a5f3

Submitted crossbow builds: ursacomputing/crossbow @ actions-fefb754618

Task Status
r-binary-packages Github Actions

r/tools/nixlibs.R Outdated Show resolved Hide resolved
r/tools/nixlibs.R Outdated Show resolved Hide resolved
r/tools/nixlibs.R Outdated Show resolved Hide resolved
paleolimbot added a commit that referenced this pull request Oct 13, 2023
### Rationale for this change

Several PRs over the last few months have update the build system to be more friendly for developers. During this process it has also come to light that we haven't supported the Windows development setup documented here since R 4.1 (released in spring 2021). I had to remove Windows from the test-r-devdocs job because the approach used there was not compatible with the `setup-r@ v2` action, and the job was failing with the `@ v1` action.

### What changes are included in this PR?

- Updated the sections on using pre-built static libraries and bundled builds
- Removed the Windows section regarding the bundled build. This section would need rewriting to support the last two minor releases of R but in the meantime I think it is mostly confusing.

### Are these changes tested?

They are documentation changes. They are also slightly optimisitc: we can fix problems with the developer setup incrementally between releases, but it's more difficult to update our documentation. This PR documents the intended behaviour after #38236 .

### Are there any user-facing changes?

No.
* Closes: #37945

Lead-authored-by: Dewey Dunnington <dewey@voltrondata.com>
Co-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Co-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
@assignUser
Copy link
Member Author

@paleolimbot ah sorry I got sidetracked and was fiddling around on windows^^ I'll update this today, would gladly accept some help with adding tests for the new functionality!

@assignUser assignUser marked this pull request as ready for review October 24, 2023 02:21
@assignUser
Copy link
Member Author

assignUser commented Oct 24, 2023

With the release over I think it makes sense to merge this an refine in follow up PRs (e.g. #38430, #38429) so we can all benefit from the new functionality (and find out the kinks that need ironing out^^).

Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

Pending the rename of del perhaps followed by a crossbow submit -g r...I'll take on #38430 after this is merged. Thanks!

r/tools/nixlibs.R Outdated Show resolved Hide resolved
Comment on lines +51 to +53
matching_major <- versions[versions$X1 == description_version[1, 1], ]
latest <- matching_major[which.max(matching_major$X4), ]
package_version(paste0(latest, collapse = "."))
Copy link
Member

Choose a reason for hiding this comment

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

Lets punt this to #38430

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels Oct 24, 2023
@assignUser
Copy link
Member Author

@github-actions crossbow submit r-binary-packages

@github-actions
Copy link

Revision: d219ba4

Submitted crossbow builds: ursacomputing/crossbow @ actions-c07979cf0d

Task Status
r-binary-packages Github Actions

@assignUser assignUser merged commit 2a4e0db into apache:main Oct 25, 2023
10 checks passed
@assignUser assignUser removed the awaiting merge Awaiting merge label Oct 25, 2023
JerAguilon pushed a commit to JerAguilon/arrow that referenced this pull request Oct 25, 2023
…ons. (apache#38236)

### Rationale for this change

We currently need to manually download the latest nightly build as the ".9000' dev version never matches any nightly builds.

### What changes are included in this PR?

Check the nightly  repo html listing for the latest version matching the local major version.
Refactor  nixlibs.R and integrate the functionality of winlibs.R  into it to simplify maintenance and prepare for the future of in-line windows libarrow builds.

These changes are build on apache#38195 so once that is merged I will rebase this.

### Are these changes tested?

Crossbow
* Closes: apache#27839

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
@conbench-apache-arrow
Copy link

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

There were no benchmark performance regressions. 🎉

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

paleolimbot added a commit that referenced this pull request Nov 10, 2023
…38534)

### Rationale for this change

A few rough edges exist after #38236 including:

- When zero or 1 nightly with the matching major version exist, detection of the latest nightly might fail
- At least one CI job is pulling nightlies instead of using the version from the current commit

### What changes are included in this PR?

- Clean up `find_latest_nightly()` + add test
- Ensure all CI jobs are not using `find_latest_nightly()`

### Are these changes tested?

Yes (test added)

### Are there any user-facing changes?

No
* Closes: #38430

Lead-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
thisisnic pushed a commit to thisisnic/arrow that referenced this pull request Nov 13, 2023
…ons. (apache#38236)

### Rationale for this change

We currently need to manually download the latest nightly build as the ".9000' dev version never matches any nightly builds.

### What changes are included in this PR?

Check the nightly  repo html listing for the latest version matching the local major version.
Refactor  nixlibs.R and integrate the functionality of winlibs.R  into it to simplify maintenance and prepare for the future of in-line windows libarrow builds.

These changes are build on apache#38195 so once that is merged I will rebase this.

### Are these changes tested?

Crossbow
* Closes: apache#27839

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
### Rationale for this change

Several PRs over the last few months have update the build system to be more friendly for developers. During this process it has also come to light that we haven't supported the Windows development setup documented here since R 4.1 (released in spring 2021). I had to remove Windows from the test-r-devdocs job because the approach used there was not compatible with the `setup-r@ v2` action, and the job was failing with the `@ v1` action.

### What changes are included in this PR?

- Updated the sections on using pre-built static libraries and bundled builds
- Removed the Windows section regarding the bundled build. This section would need rewriting to support the last two minor releases of R but in the meantime I think it is mostly confusing.

### Are these changes tested?

They are documentation changes. They are also slightly optimisitc: we can fix problems with the developer setup incrementally between releases, but it's more difficult to update our documentation. This PR documents the intended behaviour after apache#38236 .

### Are there any user-facing changes?

No.
* Closes: apache#37945

Lead-authored-by: Dewey Dunnington <dewey@voltrondata.com>
Co-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Co-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…he#38303)

### Rationale for this change

`expr` was printing the number of matching chars which showed up as noise in the log (which we want to avoid as much as possible to avoid any false positive checks)
See apache#38236 (comment) for @ jonkeane's investigation.

### What changes are included in this PR?

Replace use of expr with test.

### Are these changes tested?
Crossbow

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Jonathan Keane <jkeane@gmail.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…ons. (apache#38236)

### Rationale for this change

We currently need to manually download the latest nightly build as the ".9000' dev version never matches any nightly builds.

### What changes are included in this PR?

Check the nightly  repo html listing for the latest version matching the local major version.
Refactor  nixlibs.R and integrate the functionality of winlibs.R  into it to simplify maintenance and prepare for the future of in-line windows libarrow builds.

These changes are build on apache#38195 so once that is merged I will rebase this.

### Are these changes tested?

Crossbow
* Closes: apache#27839

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…tor (apache#38534)

### Rationale for this change

A few rough edges exist after apache#38236 including:

- When zero or 1 nightly with the matching major version exist, detection of the latest nightly might fail
- At least one CI job is pulling nightlies instead of using the version from the current commit

### What changes are included in this PR?

- Clean up `find_latest_nightly()` + add test
- Ensure all CI jobs are not using `find_latest_nightly()`

### Are these changes tested?

Yes (test added)

### Are there any user-facing changes?

No
* Closes: apache#38430

Lead-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
raulcd pushed a commit that referenced this pull request Nov 28, 2023
…38236)

### Rationale for this change

We currently need to manually download the latest nightly build as the ".9000' dev version never matches any nightly builds.

### What changes are included in this PR?

Check the nightly  repo html listing for the latest version matching the local major version.
Refactor  nixlibs.R and integrate the functionality of winlibs.R  into it to simplify maintenance and prepare for the future of in-line windows libarrow builds.

These changes are build on #38195 so once that is merged I will rebase this.

### Are these changes tested?

Crossbow
* Closes: #27839

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
raulcd pushed a commit that referenced this pull request Nov 28, 2023
…38534)

### Rationale for this change

A few rough edges exist after #38236 including:

- When zero or 1 nightly with the matching major version exist, detection of the latest nightly might fail
- At least one CI job is pulling nightlies instead of using the version from the current commit

### What changes are included in this PR?

- Clean up `find_latest_nightly()` + add test
- Ensure all CI jobs are not using `find_latest_nightly()`

### Are these changes tested?

Yes (test added)

### Are there any user-facing changes?

No
* Closes: #38430

Lead-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
assignUser pushed a commit to assignUser/arrow that referenced this pull request Dec 1, 2023
…tor (apache#38534)

### Rationale for this change

A few rough edges exist after apache#38236 including:

- When zero or 1 nightly with the matching major version exist, detection of the latest nightly might fail
- At least one CI job is pulling nightlies instead of using the version from the current commit

### What changes are included in this PR?

- Clean up `find_latest_nightly()` + add test
- Ensure all CI jobs are not using `find_latest_nightly()`

### Are these changes tested?

Yes (test added)

### Are there any user-facing changes?

No
* Closes: apache#38430

Lead-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
### Rationale for this change

Several PRs over the last few months have update the build system to be more friendly for developers. During this process it has also come to light that we haven't supported the Windows development setup documented here since R 4.1 (released in spring 2021). I had to remove Windows from the test-r-devdocs job because the approach used there was not compatible with the `setup-r@ v2` action, and the job was failing with the `@ v1` action.

### What changes are included in this PR?

- Updated the sections on using pre-built static libraries and bundled builds
- Removed the Windows section regarding the bundled build. This section would need rewriting to support the last two minor releases of R but in the meantime I think it is mostly confusing.

### Are these changes tested?

They are documentation changes. They are also slightly optimisitc: we can fix problems with the developer setup incrementally between releases, but it's more difficult to update our documentation. This PR documents the intended behaviour after apache#38236 .

### Are there any user-facing changes?

No.
* Closes: apache#37945

Lead-authored-by: Dewey Dunnington <dewey@voltrondata.com>
Co-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Co-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…he#38303)

### Rationale for this change

`expr` was printing the number of matching chars which showed up as noise in the log (which we want to avoid as much as possible to avoid any false positive checks)
See apache#38236 (comment) for @ jonkeane's investigation.

### What changes are included in this PR?

Replace use of expr with test.

### Are these changes tested?
Crossbow

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Jonathan Keane <jkeane@gmail.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…ons. (apache#38236)

### Rationale for this change

We currently need to manually download the latest nightly build as the ".9000' dev version never matches any nightly builds.

### What changes are included in this PR?

Check the nightly  repo html listing for the latest version matching the local major version.
Refactor  nixlibs.R and integrate the functionality of winlibs.R  into it to simplify maintenance and prepare for the future of in-line windows libarrow builds.

These changes are build on apache#38195 so once that is merged I will rebase this.

### Are these changes tested?

Crossbow
* Closes: apache#27839

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…tor (apache#38534)

### Rationale for this change

A few rough edges exist after apache#38236 including:

- When zero or 1 nightly with the matching major version exist, detection of the latest nightly might fail
- At least one CI job is pulling nightlies instead of using the version from the current commit

### What changes are included in this PR?

- Clean up `find_latest_nightly()` + add test
- Ensure all CI jobs are not using `find_latest_nightly()`

### Are these changes tested?

Yes (test added)

### Are there any user-facing changes?

No
* Closes: apache#38430

Lead-authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
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.

[R] [Packaging] Developing + {LIBARROW_BINARY=true}
3 participants