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

[Release] R libarrow binaries have the wrong version number #32627

Closed
asfimport opened this issue Aug 9, 2022 · 10 comments
Closed

[Release] R libarrow binaries have the wrong version number #32627

asfimport opened this issue Aug 9, 2022 · 10 comments
Assignees
Milestone

Comments

@asfimport
Copy link
Collaborator

The libarrow binaries that are uploaded during the release process have the wrong version number. This is an issue with the submit binaries script/r-binary-packages job. The arrow version should be picked up by the job even if not passed explicitly as a custom param.

Reporter: Jacob Wujciak / @assignUser
Assignee: Kouhei Sutou / @kou

PRs and other links:

Note: This issue was originally created as ARROW-17353. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

Jacob Wujciak / @assignUser:
@kou this needs to be changed in the ruby scripts right?

@asfimport
Copy link
Collaborator Author

Kouhei Sutou / @kou:
Could you show us a sample wrong URL and correct URL for it?

https://apache.jfrog.io/ui/native/arrow/r/9.0.0/libarrow/bin/ubuntu-22.04/

We should remove ".20220729"' from ".asc" and ".sha512", right?

@asfimport
Copy link
Collaborator Author

Kouhei Sutou / @kou:
Ah, did you rename arrow-9.0.0.20220729.zip to arrow-9.0.0.zip manually?

@asfimport
Copy link
Collaborator Author

Kouhei Sutou / @kou:
https://github.com/ursacomputing/crossbow/releases/tag/release-9.0.0-rc2-0-github-r-binary-packages

It seems that built packages for 9.0.0-rc2 already have ".20220729". Is it intentional? I think that we should use "9.0.0" not "9.0.0.20220719" here.

@asfimport
Copy link
Collaborator Author

Neal Richardson / @nealrichardson:
I renamed them manually after the 9.0.0 release happened, yes.

@asfimport
Copy link
Collaborator Author

Jacob Wujciak / @assignUser:
Yes all of the files should be named with only the release version and not with the added date component as you said. While it might be possible to change the r-binary-packages job to use a different version (it is possible via param but this was intended for R only patch releases and such) I think it is safer to integrate the checking/renaming into the actual ruby release script.

@asfimport
Copy link
Collaborator Author

Kouhei Sutou / @kou:
We can check names of uploaded artifacts by Crossbow: https://github.com/apache/arrow/blob/master/dev/tasks/tasks.yml#L958-L970

Can we use \{no_rc_version\} or something instead of [0-9\.]+ there?

@asfimport
Copy link
Collaborator Author

Jacob Wujciak / @assignUser:
No, as the r nightly versions don't conform to the X.Y.Z.devXXX format so it would only work for the release script but not for the nightly build upload. But that regex works for a normal X.Y.Z format so adding the param -p custom_version=10.0.0 to the crossbow submit call should work?

@asfimport
Copy link
Collaborator Author

Kouhei Sutou / @kou:
How about defining a version for nightly and use it in dev/tasks/ instead of replacing version in dev/tasks/r/github.packages.yml something like the following?


diff --git a/dev/archery/archery/crossbow/core.py b/dev/archery/archery/crossbow/core.py
index 560a9f1e19..bacc751a22 100644
--- a/dev/archery/archery/crossbow/core.py
+++ b/dev/archery/archery/crossbow/core.py
@@ -770,6 +770,14 @@ class Target(Serializable):
         # '10.0.0-SNAPSHOT'
         self.no_rc_snapshot_version = re.sub(
             r'\.(dev\d+)$', '-SNAPSHOT', self.no_rc_version)
+        # Substitute dev version with today
+        #
+        # Example:
+        #
+        # '10.0.0.dev235' ->
+        # '10.0.0.20221002'
+        self.no_rc_today_version = re.sub(
+            r'\.(dev\d+)\Z', date.today().strftime('.%Y%m%d'), self.no_rc_version)
 
     @classmethod
     def from_repo(cls, repo, head=None, branch=None, remote=None, version=None,
diff --git a/dev/tasks/macros.jinja b/dev/tasks/macros.jinja
index f68aa15d54..1b676b5ccf 100644
--- a/dev/tasks/macros.jinja
+++ b/dev/tasks/macros.jinja
@@ -269,7 +269,7 @@ on:
       rm -f apache-arrow*.rb.bak
 {% endmacro %}
 
-{%- macro github_change_r_pkg_version(is_fork, version = '\\2.\'\"$(date +%Y%m%d)\"\'' ) -%}
+{%- macro github_change_r_pkg_version(is_fork, version) -%}
   - name: Modify version
     shell: bash
     run: |
diff --git a/dev/tasks/r/github.packages.yml b/dev/tasks/r/github.packages.yml
index b5b9548d59..ac721b4f60 100644
--- a/dev/tasks/r/github.packages.yml
+++ b/dev/tasks/r/github.packages.yml
@@ -17,10 +17,6 @@
 
 {% import 'macros.jinja' as macros with context %}
 
-# This allows us to set a custom version via param:
-# crossbow submit --param custom_version=8.5.3 r-binary-packages
-# if the param is unset defaults to the usual Ymd naming scheme
-{% set package_version = custom_version|replace("Unset", "\\2.\'\"$(date +%Y%m%d)\"\'") %}
 {% set is_fork = macros.is_fork %}
 
 {{ macros.github_header() }}
@@ -35,7 +31,7 @@ jobs:
       pkg_version: {{ '${{ steps.save-version.outputs.pkg_version }}' }}
     steps:
       {{ macros.github_checkout_arrow()|indent }}
-      {{ macros.github_change_r_pkg_version(is_fork, package_version)|indent }}
+      {{ macros.github_change_r_pkg_version(is_fork, arrow.no_rc_today_version)|indent }}
       - name: Save Version
         id: save-version
         shell: bash
diff --git a/dev/tasks/tasks.yml b/dev/tasks/tasks.yml
index 2cd6c259ec..ccd3eeeea8 100644
--- a/dev/tasks/tasks.yml
+++ b/dev/tasks/tasks.yml
@@ -953,19 +953,19 @@ tasks:
     params:
       custom_version: Unset
     artifacts:
-      - r-lib__libarrow__bin__windows__arrow-[0-9\.]+\.zip
+      - r-lib__libarrow__bin__windows__arrow-{no_rc_today_version}\.zip
       # The centos job is currently disabled due to the
       # change to C++ 17
-      #- r-lib__libarrow__bin__centos-7__arrow-[0-9\.]+\.zip
-      - r-lib__libarrow__bin__ubuntu-18.04__arrow-[0-9\.]+\.zip
-      - r-lib__libarrow__bin__ubuntu-22.04__arrow-[0-9\.]+\.zip
-      - r-pkg__bin__windows__contrib__4.1__arrow_[0-9\.]+\.zip
-      - r-pkg__bin__windows__contrib__4.2__arrow_[0-9\.]+\.zip
-      - r-pkg__bin__macosx__contrib__4.1__arrow_[0-9\.]+\.tgz
-      - r-pkg__bin__macosx__contrib__4.2__arrow_[0-9\.]+\.tgz
-      - r-pkg__bin__macosx__big-sur-arm64__contrib__4.1__arrow_[0-9\.]+\.tgz
-      - r-pkg__bin__macosx__big-sur-arm64__contrib__4.2__arrow_[0-9\.]+\.tgz
-      - r-pkg__src__contrib__arrow_[0-9\.]+\.tar\.gz
+      #- r-lib__libarrow__bin__centos-7__arrow-{no_rc_today_version}\.zip
+      - r-lib__libarrow__bin__ubuntu-18.04__arrow-{no_rc_today_version}\.zip
+      - r-lib__libarrow__bin__ubuntu-22.04__arrow-{no_rc_today_version}\.zip
+      - r-pkg__bin__windows__contrib__4.1__arrow_{no_rc_today_version}\.zip
+      - r-pkg__bin__windows__contrib__4.2__arrow_{no_rc_today_version}\.zip
+      - r-pkg__bin__macosx__contrib__4.1__arrow_{no_rc_today_version}\.tgz
+      - r-pkg__bin__macosx__contrib__4.2__arrow_{no_rc_today_version}\.tgz
+      - r-pkg__bin__macosx__big-sur-arm64__contrib__4.1__arrow_{no_rc_today_version}\.tgz
+      - r-pkg__bin__macosx__big-sur-arm64__contrib__4.2__arrow_{no_rc_today_version}\.tgz
+      - r-pkg__src__contrib__arrow_{no_rc_today_version}\.tar\.gz
 
 
   ########################### Release verification ############################

This will generate "r-pkg_*.10.0.0.zip" not "r-pkg_*10.0.0.202210XX.zip" for "10.0.0-rc1" because "10.0.0-rc1" doesn't have ".devXXX".

@asfimport
Copy link
Collaborator Author

Kouhei Sutou / @kou:
Issue resolved by pull request 14396
#14396

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