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

ARROW-18429 [R]: Bump dev version following 10.0.1 patch release #14887

Merged
merged 9 commits into from
Dec 12, 2022

Conversation

thisisnic
Copy link
Member

No description provided.

@github-actions
Copy link

github-actions bot commented Dec 8, 2022

@github-actions
Copy link

github-actions bot commented Dec 8, 2022

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@thisisnic thisisnic marked this pull request as ready for review December 8, 2022 12:48
@thisisnic
Copy link
Member Author

I'm seeing build failures on the rhub/debian-gcc-devel:latest build, I think due to the dev Arrow R package there being incompatible with the 10.0.1 binary.

ARROW-17509 was merged on 10th November but was not released as part of 10.0.1 (released 22nd November). It touches both libarrow and the R package.

What do we need to do? I guess, ultimately, we should have added ARROW-17509 to 10.0.1, but we didn't catch it. Do we need request/suggest a 10.0.2 release, or do something else?

@paleolimbot What do you think? I think everything in this PR is necessary changes as I based them on this PR from after the 6.0.1 release.

@paleolimbot
Copy link
Member

The CI scripts might not be up for that level of version calculation...if they're passing with the existing 10.0.0.9000, let's keep it that way?

@paleolimbot
Copy link
Member

Hm....on a separate PR we get:

> checking CRAN incoming feasibility ... WARNING
Warning:   Maintainer: 'Neal Richardson <neal@ursalabs.org>'
  
  Insufficient package version (submitted: 10.0.0.9000, existing: 10.0.1)
  Version contains large components (10.0.0.9000)

...so we do need a way to figure that one out.

@paleolimbot
Copy link
Member

I looked through the failures and I can't spot what needs to happen...as far as I can tell, this PR should do it. I don't think we need a new CRAN release for anything...the CRAN checks are clean for all 10.0.1 versions ( https://cran.r-project.org/web/checks/check_results_arrow.html ). I think we need to bat signal @nealrichardson! Help!

@thisisnic
Copy link
Member Author

Just to clarify, @paleolimbot (apologies if I'm saying something you already know here), the reason that the CRAN checks are fine is that the code in the R package there is from the r-10.0.1 branch (in my fork) which is branched off the maint-10.0.1 branch here, at which point everything builds fine. What I'm saying is that the dev R package has updated C++ code in r/src (from ARROW-17509) so isn't compatible with the 10.0.1 binary, which is what that CI job is trying to build off and failing.

If I had a time machine I'd just go back and mention that we need ARROW-17509 in the 10.0.1 release, but alas, I do not have access to that technology ;) I don't think we should update the CI job to build from source instead either. So I'm out of ideas.

@paleolimbot
Copy link
Member

Oh I see...probably we just have to hard-code something in a development build script until 11.0.0 then?

@thisisnic
Copy link
Member Author

Oh I see...probably we just have to hard-code something in a development build script until 11.0.0 then?

That sounds like a reasonable workaround. Maybe if we build from the libarrow nightlies on those builds? Maybe we should do that anyway?

@paleolimbot
Copy link
Member

I can revisit this later, but I'm pretty lost on how to do any of that.

@thisisnic
Copy link
Member Author

I've spoken to @assignUser and have an idea of how to do it, I'll take a look tomorrow.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

I think that .9000 is needed for versions in master.

ci/scripts/PKGBUILD Outdated Show resolved Hide resolved
r/DESCRIPTION Outdated Show resolved Hide resolved
thisisnic and others added 2 commits December 9, 2022 12:16
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@thisisnic
Copy link
Member Author

@paleolimbot Ignore everything I said before, I was totally wrong on all that, it was just a version number thing after all.

@thisisnic
Copy link
Member Author

@kou All of the R CI jobs are passing now, but the release scripts still aren't happy - any idea what's going on with them?

@kou
Copy link
Member

kou commented Dec 9, 2022

r/pkgwon/assets/versions.json wasn't updated. The release scripts are green now.

BTW, how did you update these versions? Did you update them manually? If so, can we automate it? We have scripts in dev/release/ to automate our release process.

@thisisnic
Copy link
Member Author

Thanks @kou!

Yeah, I did it manually, but makes sense to do it automatically. I wasn't sure if it could be done manually as part of the existing release process, becauseas in the R release there are multiple steps which are independent of the rest of the release process (i.e. see the checklist for the R release), but an additional script to do it could make sense.

@nealrichardson Is this something which has been done manually in past releases, or is it specific to a minor version release? I saw a similar PR for 6.0.1 but not for major version releases.

@paleolimbot
Copy link
Member

I agree with Nic - a script could be nice for this; however, until Nic and I get a hang of the release process I think we may want to hold off on additional automation (lest we get it wrong!).

Can we merge this sooner rather than later? The version thing is keeping CI from passing on new R PRs.

@thisisnic
Copy link
Member Author

@paleolimbot Given CI is green here, if you're happy to approve, let's merge and follow up the other conversations later

@thisisnic thisisnic merged commit 398ce31 into apache:master Dec 12, 2022
@nealrichardson
Copy link
Member

@nealrichardson Is this something which has been done manually in past releases, or is it specific to a minor version release? I saw a similar PR for 6.0.1 but not for major version releases.

Yes, when there is a major release, the post-release scripts bump the versions for us. But patch releases don't, so we have to do it manually. None of the other languages require this because they use the next.0.0-SNAPSHOT for dev version, but R uses current.y.z.9000, so when z changes we have to do this for the R package.

@kou
Copy link
Member

kou commented Dec 12, 2022

Thanks for sharing the current work flow.

How about running dev/release/post-11-bump-versions.sh script in patch release too like the following?

dev/release/post-11-bump-versions.sh 10.0.1 11.0.0

I think that we can run dev/release/post-11-bump-versions.sh in patch release with the following changes:

diff --git a/dev/release/post-11-bump-versions.sh b/dev/release/post-11-bump-versions.sh
index ad4403f1b..00f99d726 100755
--- a/dev/release/post-11-bump-versions.sh
+++ b/dev/release/post-11-bump-versions.sh
@@ -41,6 +41,15 @@ version=$1
 next_version=$2
 next_version_snapshot="${next_version}-SNAPSHOT"
 
+case "${version}" in
+  *.0.0)
+    is_major_release=1
+    ;;
+  *)
+    is_major_release=0
+    ;;
+esac
+
 if [ ${BUMP_UPDATE_LOCAL_DEFAULT_BRANCH} -gt 0 ]; then
   echo "Updating local default branch"
   git fetch --all --prune --tags --force -j$(nproc)
@@ -68,8 +77,8 @@ if [ ${BUMP_DEB_PACKAGE_NAMES} -gt 0 ]; then
     cd $SOURCE_DIR/../tasks/linux-packages/apache-arrow
     for target in debian*/lib*${deb_lib_suffix}.install; do
       git mv \
-	${target} \
-	$(echo $target | sed -e "s/${deb_lib_suffix}/${next_deb_lib_suffix}/")
+        ${target} \
+        $(echo $target | sed -e "s/${deb_lib_suffix}/${next_deb_lib_suffix}/")
     done
     deb_lib_suffix_substitute_pattern="s/(lib(arrow|gandiva|parquet|plasma)[-a-z]*)${deb_lib_suffix}/\\1${next_deb_lib_suffix}/g"
     sed -i.bak -E -e "${deb_lib_suffix_substitute_pattern}" debian*/control*
@@ -107,7 +116,7 @@ if [ ${BUMP_PUSH} -gt 0 ]; then
   git push apache ${DEFAULT_BRANCH}
 fi
 
-if [ ${BUMP_TAG} -gt 0 ]; then
+if [ ${BUMP_TAG} -gt 0 -a ${is_major_release} -gt 0 ]; then
   dev_tag=apache-arrow-${next_version}.dev
   echo "Tagging ${dev_tag}"
   git tag ${dev_tag} ${DEFAULT_BRANCH}
diff --git a/dev/release/utils-prepare.sh b/dev/release/utils-prepare.sh
index 1e50a9295..20346e66a 100644
--- a/dev/release/utils-prepare.sh
+++ b/dev/release/utils-prepare.sh
@@ -166,22 +166,18 @@ update_versions() {
     parquet/writer_properties.go
   sed -i.bak -E -e \
     "s/const PkgVersion = \".*/const PkgVersion = \"${version}\"/" \
-    arrow/doc.go  
+    arrow/doc.go
 
   find . -name "*.bak" -exec rm {} \;
   git add .
-  popd  
+  popd
 
-  case "${base_version}" in
-    *.0.0)
-      pushd "${ARROW_DIR}"
-      ${PYTHON:-python3} "dev/release/utils-update-docs-versions.py" \
-                         . \
-                         "${base_version}" \
-                         "${next_version}"
-      git add docs/source/_static/versions.json
-      git add r/pkgdown/assets/versions.json
-      popd
-      ;;
-  esac
+  pushd "${ARROW_DIR}"
+  ${PYTHON:-python3} "dev/release/utils-update-docs-versions.py" \
+                     . \
+                     "${base_version}" \
+                     "${next_version}"
+  git add docs/source/_static/versions.json
+  git add r/pkgdown/assets/versions.json
+  popd
 }
diff --git a/dev/release/utils-update-docs-versions.py b/dev/release/utils-update-docs-versions.py
index af9d05ec1..3a5260435 100644
--- a/dev/release/utils-update-docs-versions.py
+++ b/dev/release/utils-update-docs-versions.py
@@ -27,30 +27,32 @@ next_version = sys.argv[3]
 main_versions_path = dir_path + "/docs/source/_static/versions.json"
 r_versions_path = dir_path + "/r/pkgdown/assets/versions.json"
 
-# Update main docs version script
-
-with open(main_versions_path) as json_file:
-    old_versions = json.load(json_file)
-
 split_version = version.split(".")
-split_next_version = next_version.split(".")
-dev_compatible_version = ".".join(split_next_version[:2])
-stable_compatible_version = ".".join(split_version[:2])
-previous_compatible_version = old_versions[1]["name"].split(" ")[0]
 
-# Create new versions
-new_versions = [
-    {"name": f"{dev_compatible_version} (dev)",
-     "version": "dev/"},
-    {"name": f"{stable_compatible_version} (stable)",
-     "version": ""},
-    {"name": previous_compatible_version,
-     "version": f"{previous_compatible_version}/"},
-    *old_versions[2:],
-]
-with open(main_versions_path, 'w') as json_file:
-    json.dump(new_versions, json_file, indent=4)
-    json_file.write("\n")
+if split_version[1:] == ["0", "0"]:
+    # Update main docs version script
+
+    with open(main_versions_path) as json_file:
+        old_versions = json.load(json_file)
+
+    split_next_version = next_version.split(".")
+    dev_compatible_version = ".".join(split_next_version[:2])
+    stable_compatible_version = ".".join(split_version[:2])
+    previous_compatible_version = old_versions[1]["name"].split(" ")[0]
+
+    # Create new versions
+    new_versions = [
+        {"name": f"{dev_compatible_version} (dev)",
+         "version": "dev/"},
+        {"name": f"{stable_compatible_version} (stable)",
+         "version": ""},
+        {"name": previous_compatible_version,
+         "version": f"{previous_compatible_version}/"},
+        *old_versions[2:],
+    ]
+    with open(main_versions_path, 'w') as json_file:
+        json.dump(new_versions, json_file, indent=4)
+        json_file.write("\n")
 
 # Update R package version script
 
@@ -62,12 +64,19 @@ release_r_version = version
 previous_r_name = old_r_versions[1]["name"].split(" ")[0]
 previous_r_version = ".".join(previous_r_name.split(".")[:2])
 
-new_r_versions = [
-    {"name": f"{dev_r_version} (dev)", "version": "dev/"},
-    {"name": f"{release_r_version} (release)", "version": ""},
-    {"name": previous_r_name, "version": f"{previous_r_version}/"},
-    *old_r_versions[2:],
-]
+if split_version[1:] == ["0", "0"]:
+    new_r_versions = [
+        {"name": f"{dev_r_version} (dev)", "version": "dev/"},
+        {"name": f"{release_r_version} (release)", "version": ""},
+        {"name": previous_r_name, "version": f"{previous_r_version}/"},
+        *old_r_versions[2:],
+    ]
+else:
+    new_r_versions = [
+        {"name": f"{dev_r_version} (dev)", "version": "dev/"},
+        {"name": f"{release_r_version} (release)", "version": ""},
+        *old_r_versions[2:],
+    ]
 with open(r_versions_path, 'w') as json_file:
     json.dump(new_r_versions, json_file, indent=4)
     json_file.write("\n")

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.

None yet

4 participants