-
Notifications
You must be signed in to change notification settings - Fork 4.2k
ARROW-18345: [R] Create a CRAN-specific packaging checklist that lives in the R package directory #14678
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-18345: [R] Create a CRAN-specific packaging checklist that lives in the R package directory #14678
Changes from 19 commits
75c7890
f9aff01
cd73564
62d4a33
9386274
8ae4993
14f7af2
a58be68
6c23b74
62a65f0
a2a4ac1
f982caa
d302439
5e4fd7a
282e78b
780ec18
ab41e71
dca9b36
36bd3e5
97b350e
afe23c2
567ee14
4e9c0b3
ce003a9
148b471
9cce455
d2edb6e
528541f
1b9b843
1ec8b91
3f8d2b5
a981190
6fd9d40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,3 +30,4 @@ STYLE.md | |
| ^cheatsheet$ | ||
| ^revdep$ | ||
| ^vignettes$ | ||
| ^PACKAGING\.md$ | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,128 @@ | ||||||
|
|
||||||
| <!--- | ||||||
| Licensed to the Apache Software Foundation (ASF) under one | ||||||
| or more contributor license agreements. See the NOTICE file | ||||||
| distributed with this work for additional information | ||||||
| regarding copyright ownership. The ASF licenses this file | ||||||
| to you under the Apache License, Version 2.0 (the | ||||||
| "License"); you may not use this file except in compliance | ||||||
| with the License. You may obtain a copy of the License at | ||||||
|
|
||||||
| http://www.apache.org/licenses/LICENSE-2.0 | ||||||
|
|
||||||
| Unless required by applicable law or agreed to in writing, | ||||||
| software distributed under the License is distributed on an | ||||||
| "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||||
| KIND, either express or implied. See the License for the | ||||||
| specific language governing permissions and limitations | ||||||
| under the License. | ||||||
| --> | ||||||
|
|
||||||
| # Packaging checklist for CRAN release | ||||||
|
|
||||||
| For a high-level overview of the release process see the | ||||||
| [Apache Arrow Release Management Guide](https://arrow.apache.org/docs/developers/release.html#post-release-tasks). | ||||||
|
|
||||||
| Before the release candidate is cut: | ||||||
|
|
||||||
| - [ ] [Create a GitHub issue](https://github.com/apache/arrow/issues/new/) | ||||||
| entitled `[R] CRAN packaging checklist for version X.X.X` | ||||||
| and copy this checklist to the issue. | ||||||
| - [ ] Evaluate the status of any failing | ||||||
| [nightly tests and nightly packaging builds](https://lists.apache.org/list.html?builds@arrow.apache.org). These checks | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Link to the crossbow dashboard instead? |
||||||
| replicate most of the checks that CRAN runs, so we need them all to be passing | ||||||
| or to understand that the failures may (though won't necessarily) result in a rejection from CRAN. | ||||||
| - [ ] Check [current CRAN check results](https://cran.rstudio.org/web/checks/check_results_arrow.html) | ||||||
|
thisisnic marked this conversation as resolved.
Outdated
paleolimbot marked this conversation as resolved.
Outdated
|
||||||
| - [ ] Ensure the contents of the README is accurate | ||||||
|
paleolimbot marked this conversation as resolved.
Outdated
|
||||||
| - [ ] Run `urlchecker::url_check()` on the R directory at the release candidate | ||||||
| commit. Ignore any errors with badges as they will be removed in the CRAN release branch. | ||||||
| - [ ] [Polish NEWS](https://style.tidyverse.org/news.html#news-release) but do **not** update version numbers (this is done automatically later). | ||||||
| - [ ] For major releases, prepare tweet thread highlighting new features | ||||||
|
|
||||||
| Wait for the release candidate to be cut: | ||||||
|
|
||||||
| - [ ] Release candidate! | ||||||
|
|
||||||
| Make pull requests into the [autobrew](https://github.com/autobrew) and | ||||||
| [rtools-packages](https://github.com/r-windows/rtools-packages) repositories | ||||||
| used by the configure script on MacOS and Windows. These pull requests will | ||||||
| use the release candidate as the source. | ||||||
|
|
||||||
| - [ ] Pull request to modify | ||||||
| [the apache-arrow autobrew formula]( https://github.com/autobrew/homebrew-core/blob/high-sierra/Formula/apache-arrow.rb) | ||||||
| to update the release version, SHA256 checksum of the release source file, and any changes to dependencies and build steps that have changed in the | ||||||
|
paleolimbot marked this conversation as resolved.
Outdated
|
||||||
| [copy of the formula we have of that formula in the Arrow repo](https://github.com/apache/arrow/blob/master/dev/tasks/homebrew-formulae/autobrew/apache-arrow.rb) | ||||||
| - [ ] Pull request to modify | ||||||
| [the apache-arrow-static autobrew formula]( https://github.com/autobrew/homebrew-core/blob/master/Formula/apache-arrow-static.rb) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get a 404 for this URL
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| to update the version, SHA, and any changes to dependencies and build steps that have changed in the | ||||||
| [copy of the formula we have of that formula in the Arrow repo](https://github.com/apache/arrow/blob/master/dev/tasks/homebrew-formulae/autobrew/apache-arrow-static.rb) | ||||||
| - [ ] Pull request to modify the | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be good to note that there often are no changes in this script, and that's ok. |
||||||
| [autobrew script](https://github.com/autobrew/scripts/blob/master/apache-arrow) | ||||||
| to include any additions made to | ||||||
| [r/tools/autobrew](https://github.com/apache/arrow/blob/master/r/tools/autobrew). | ||||||
| - [ ] Pull request to modify the | ||||||
| [PKGBUILD script](https://github.com/r-windows/rtools-packages/blob/master/mingw-w64-arrow/PKGBUILD) | ||||||
|
paleolimbot marked this conversation as resolved.
Outdated
|
||||||
| to reflect changes in | ||||||
| [ci/PKGBUILD](https://github.com/apache/arrow/blob/master/ci/scripts/PKGBUILD), | ||||||
| uncommenting the line that says "uncomment to test the rc". | ||||||
|
|
||||||
| Prepare and check the .tar.gz that will be released to CRAN. | ||||||
|
|
||||||
| - [ ] `git fetch upstream && git checkout release-X.X.X-rcXX && git clean -f -d` | ||||||
| - [ ] Unset the `ARROW_HOME` environment variable and then run `make build` (copies Arrow C++ into tools/cpp, prunes some unnecessary components, and runs `R CMD build`) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why unset
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I tried it earlier, when I ran
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Used" as in used to build against in order to build the vignettes? Generally that will be useful because it will save you time, as long as your local build is up to date. But that doesn't affect what cpp source gets copied into the R package.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Neal has a good point...the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried it again and had to unset it as the version of Arrow I have in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added some clarification here that I hope is helpful...the big picture idea is that whatever libarrow the R package configure script picks up needs to be able to install the rc version. For example, when I ran the reverse dependency checks I rebuilt Arrow C++ from your crossbow PR branch and installed it to |
||||||
| - [ ] `devtools::check_built("arrow_X.X.X.tar.gz")` locally | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're doing |
||||||
| - [ ] Run reverse dependency checks | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a bit up in the air. There is a crossbow job that will do hit; however, it times out if you try to run it on a PR. If you run it locally via I think I just petered out trying to fit that in a bullet point. Any suggestions?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have removed the crossbow job because it would have been a bunch of work to fix the timeout. But I was wondering why revdepcheck was taking SO LONG (even if you take the 2 core gha machines into account). And locally it will still timeout for a different reason ({targets} timesout with multiple workers??). But I would be fine with reworking the job to use your gist or even the build in function I did not know exists: https://www.r-bloggers.com/2019/04/checking-reverse-dependencies-the-tiny-way/ current docs: https://stat.ethz.ch/R-manual/R-patched/library/tools/html/check_packages_in_dir.html
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doing it locally is a pain as I'm having to install a whole load of system libraries I don't need, to be able to run the gist; please can we rework the crossbow job?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah, that gist is way better on MacOS because the binary packages for MacOS are self-sufficient.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't we do this later, after we've picked the additional commits across, just in case some weird thing (or a cheeky last minute addition that we've added to the CRAN release) has an impact?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverse dependency checks typically result in cheeky last minute additions of their own. For the last release I ran them early (to find problems) and again after the PRs that fixed those problems had been merged.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this will be a nightly/crossbow job soon I think we can remove it anyway.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought we tried before and it was not feasible to do revdep checks in crossbow
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the current approach, no, but @assignUser had some ideas and I think we can make it work. |
||||||
|
|
||||||
| Wait for the official release... | ||||||
|
|
||||||
| - [ ] Release vote passed! | ||||||
| - [ ] Create a CRAN-release branch from the official release commit | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would do this at RC time--the commit doesn't change just because the vote passes. That way, all of those checks you're describing below you can do while the vote is open--that's your manual release verification.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense (and rebase if the release vote doesn't pass?) |
||||||
| - [ ] Pick any commits that were made to master since the release commit that | ||||||
| were needed to fix CRAN-related submission issues (e.g., fixed URLs, fixes | ||||||
| to ) | ||||||
| - [ ] Remove badges from README.md | ||||||
| - [ ] Run `urlchecker::url_check()` on the R directory | ||||||
| - [ ] Bump the Version in DESCRIPTION to X.X.X | ||||||
|
nealrichardson marked this conversation as resolved.
Outdated
paleolimbot marked this conversation as resolved.
Outdated
|
||||||
| - [ ] Create a PR entitled `WIP: [R] Verify CRAN release-10.0.1-rc0`. Add | ||||||
| a comment `@github-actions crossbow submit --group r` to run all R crossbow | ||||||
| jobs against the CRAN-specific release branch. | ||||||
|
Comment on lines
+93
to
+95
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is great, much more clear, thanks!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, wait, what branches are we comparing on the PR? Does it even matter? I made an arbitrary one which branched off the release candidate and made a whitespace change. Should we recommend that here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For crossbow I think we just need any PR to exist as long as "from" is correct. I think a whitespace change is fine for now but we should find a workflow that is less of a hack in the future. I don't have a good answer right now though! |
||||||
| - [ ] Regenerate arrow_X.X.X.tar.gz (i.e., `make build`) | ||||||
|
|
||||||
| Update autobrew and r-windows PRs to use the *release* instead of the | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would note that lately Jeroen has been merging these PRs before the vote closes so that we can test. So this may require new PRs. Also you can make all of the version bumps in the autobrew formulae in the initial PR now, no need to follow up here. It uses the RC URL as "mirror", so before the vote passes, the release URL will 404 and it falls back to the RC. After, it uses the official release. So it's only the rtools-packages one that needs switching.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll let @thisisnic update the language here since they're going through that process now. I imagine we at least need to check those scripts at this stage in case there was a new release candidate since the initial PR. |
||||||
| *release candidate*; ensure linux binary packages are available: | ||||||
|
|
||||||
| - [ ] PR into autobrew/homebrew-core (apache-arrow autobrew formula) | ||||||
| - [ ] PR into autobrew/homebrew-core (apache-arrow-static autobrew formula) | ||||||
| - [ ] PR into autobrew/scripts | ||||||
| - [ ] PR into r-windows/rtools-packages | ||||||
| - [ ] Ensure linux binaries are available in the artifactory: | ||||||
| https://apache.jfrog.io/ui/repos/tree/General/arrow/r | ||||||
|
|
||||||
| Check binary Arrow C++ distributions specific to the R package: | ||||||
|
|
||||||
| - [ ] Upload the .tar.gz to [win-builder](https://win-builder.r-project.org/upload.aspx) | ||||||
| and confirm that the check is clean | ||||||
|
paleolimbot marked this conversation as resolved.
Outdated
|
||||||
| - [ ] Upload the .tar.gz to [MacBuilder](https://mac.r-project.org/macbuilder/submit.html) | ||||||
| and confirm that the check is clean | ||||||
| - [ ] Check `install.packages("arrow_X.X.X.tar.gz")` on Ubuntu and ensure that the | ||||||
| hosted binaries are used | ||||||
| - [ ] `devtools::check("arrow_X.X.X.tar.gz")` locally one more time (for luck) | ||||||
|
paleolimbot marked this conversation as resolved.
Outdated
|
||||||
|
|
||||||
| Submit! | ||||||
|
|
||||||
| - [ ] Upload arrow_X.X.X.tar.gz to the | ||||||
| [CRAN submit page](https://xmpalantir.wu.ac.at/cransubmit/) | ||||||
| - [ ] Confirm the submission email | ||||||
|
|
||||||
| Wait for CRAN... | ||||||
|
|
||||||
| - [ ] Accepted! | ||||||
| - [ ] Tag the tip of the CRAN-specific release branch | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tag how? This isn't something we've done before
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose we don't have to (as long as there is a way to get back to the source at the time of the CRAN release in case we need to patch something?)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I've created a branch on my fork called r-10.0.1, but we should probably do something more robust.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @paleolimbot Any thoughts on this?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think for now that is OK. For 11.0.0, perhaps we can file an issue or ask on the mailing list what the appropriate branch name would be in the main Arrow repo? There is not currently any equivalent branch on the main repo for the packaging of a component but I imagine this has come up before (patches applied prior to packaging a component). |
||||||
| - [ ] `usethis::use_dev_version()` | ||||||
|
paleolimbot marked this conversation as resolved.
Outdated
|
||||||
| - [ ] Add a new line to the matrix in the [backwards compatability job](https://github.com/apache/arrow/blob/master/dev/tasks/r/github.linux.arrow.version.back.compat.yml) | ||||||
|
thisisnic marked this conversation as resolved.
paleolimbot marked this conversation as resolved.
|
||||||
| - [ ] Update the packaging checklist template to reflect any new realities of the | ||||||
| packaging process. | ||||||
| - [ ] Wait for CRAN-hosted binaries on the | ||||||
| [CRAN package page](https://cran.r-project.org/package=arrow) to reflect the | ||||||
| new version | ||||||
| - [ ] Tweet! | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where would I find this official URL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they are embedded in the hombrew files (there are links to the homebrew scripts in the checklist). I think there will be a number of specific additions after we get through this release and I wonder if we could use the release issue itself as a place to collect additions that would be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each of these scripts has the official and rc URLs in them, and it's a matter of updating the versions and (un)commenting to enable one or the other.