apache / arrow

Apache Arrow is the universal columnar format and multi-language toolbox for fast data interchange and in-memory analytics
https://arrow.apache.org/
Apache License 2.0
14.65k stars 3.55k forks source link

GH-44703: [CI][MATLAB][Packaging] Update MATLAB CI and `crossbow` packaging workflows to build against MATLAB `R2024b` #44704

Closed kevingurney closed 1 week ago

kevingurney commented 1 week ago

Rationale for this change

MATLAB R2024b is now available for use with the matlab-actions/setup-matlab GitHub Action.

We should update the matlab.yml CI workflow, as well as the crossbow packaging workflows for the MATLAB MLTBX files to build against R2024b.

What changes are included in this PR?

  1. Updated the .github/workflows/matlab.yml CI workflow file to build the MATLAB Interface against MATLAB R2024b.
  2. Updated the dev/tasks/matlab/github.yml crossbow packaging workflow to build the MATLAB MLTBX files against MATLAB R2024b.
  3. Bumped mathworks/libmexclass version to commit cac7c3630a086bd5ba41413af44c833cef189c09 to work around mathworks/libmexclass#92

Are these changes tested?

Yes.

  1. CI workflow successfully passed on all platforms in mathworks/arrow.
  2. Crossbow job: https://github.com/ursacomputing/crossbow/actions/runs/11805816426.

Are there any user-facing changes?

Yes.

  1. All changes to the MATLAB interface will now be built against R2024b.
  2. The MATLAB MLTBX release artifacts will now be built against R2024b.

Notes

  1. Thank you @sgilmore10 for your help with this pull request!
    • GitHub Issue: #44703
kevingurney commented 1 week ago

@github-actions crossbow submit matlab

github-actions[bot] commented 1 week ago

Revision: 5fca60d9f05433268f60ad8a1c5bf9f3241adc94

Submitted crossbow builds: ursacomputing/crossbow @ actions-11d1a5204d

Task Status
matlab GitHub Actions
kevingurney commented 1 week ago

Thanks, @assignUser! I'll make the required change.

kevingurney commented 1 week ago

@github-actions crossbow submit matlab

github-actions[bot] commented 1 week ago

Revision: c9c36222a21f43187b8740192ba4a740afab9400

Submitted crossbow builds: ursacomputing/crossbow @ actions-6ab43281e8

Task Status
matlab GitHub Actions
kevingurney commented 1 week ago

@assignUser - we updated the tasks.yml file to expect the MLTBX name to look like matlab-arrow.X.Y.Z.mltbx and the crossbow job is now passing.

Just to clarify - this means the name of the MLTBX file will never include an RC version or a .dev[x][y] string in the name. We are assuming this is fine, but let us know if there are any concerns with this naming scheme.

Thank you!

kou commented 1 week ago

If we use [0-9]+.[0-9]+.[0-9]+, we can't detect wrong version number. For example, 20.0.0 is also accepted when 19.0.0 is expected.

How about adding a new variable for MAJOR.MINOR.PATCH only version?

diff --git a/dev/archery/archery/crossbow/core.py b/dev/archery/archery/crossbow/core.py
index 12571c0ff6..ea6ba31fb8 100644
--- a/dev/archery/archery/crossbow/core.py
+++ b/dev/archery/archery/crossbow/core.py
@@ -803,6 +803,11 @@ class Target(Serializable):
         self.r_version = r_version
         self.no_rc_version = re.sub(r'-rc\d+\Z', '', version)
         self.no_rc_r_version = re.sub(r'-rc\d+\Z', '', r_version)
+        # Example:
+        #
+        #   '19.0.0.dev66' ->
+        #   '19.0.0'
+        self.no_rc_no_dev_version = (r'\.dev\d+\Z', '', self.no_rc_version)
         # Semantic Versioning 1.0.0: https://semver.org/spec/v1.0.0.html
         #
         # > A pre-release version number MAY be denoted by appending an
@@ -1195,6 +1200,7 @@ class Job(Serializable):
         versions = {
             'version': target.version,
             'no_rc_version': target.no_rc_version,
+            'no_rc_no_dev_version': target.no_rc_version,
             'no_rc_semver_version': target.no_rc_semver_version,
             'no_rc_snapshot_version': target.no_rc_snapshot_version,
             'r_version': target.r_version,
diff --git a/dev/tasks/tasks.yml b/dev/tasks/tasks.yml
index 31d260bbfd..12dcfc1edd 100644
--- a/dev/tasks/tasks.yml
+++ b/dev/tasks/tasks.yml
@@ -693,7 +693,7 @@ tasks:
     ci: github
     template: matlab/github.yml
     artifacts:
-      - matlab-arrow-{no_rc_version}.mltbx
+      - matlab-arrow-{no_rc_no_dev_version}.mltbx

   ############################## Arrow JAR's ##################################
kevingurney commented 1 week ago

@kou - thank you. That's an excellent point and a helpful suggestion. I'll make this change.

kevingurney commented 1 week ago

@github-actions crossbow submit matlab

github-actions[bot] commented 1 week ago
'no_rc_no_dev_version'
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/11842416634
kevingurney commented 1 week ago

@kou - I've added no_rc_no_dev_version, but when running @github-actions crossbow submit matlab it looks like the version of archery being used is the version from main rather than this branch and therefore can't seem to find the property no_rc_no_dev_version.

Is there any way I can qualify the changes to crossbow using the bot on this PR? If not, would it make sense to open a separate PR to just to make the changes to crossbow and then follow up with a separate change for updating the MATLAB version and expected release artifact naming scheme to use no_rc_no_dev_version?

kevingurney commented 1 week ago

I just noticed that @raulcd ran into the exact same issue when qualifying his changes for no_rc_snapshot_version here: https://github.com/apache/arrow/pull/14135#issuecomment-1247932393. It looks like he had to run crossbow from his own fork to pick up the changes.

kevingurney commented 1 week ago

I tried setting up my own fork to test the crossbow changes following @raulcd's approach, but I am encountering the following issue when I try to use the crossbow bot:

Wrong oauth personal access token
The Archery job run can be found at: https://github.com/kevingurney/arrow/actions/runs/11845389640

If possible, could someone point me towards the steps required to configure a working OAuth access token for the crossbow bot? I've searched through the documentation, but haven't been able to find anything specific on this.

Thanks!

assignUser commented 1 week ago

I started the job locally: https://github.com/ursacomputing/crossbow/actions/runs/11860294408/job/33055221197

assignUser commented 1 week ago

@kevingurney it failed, for a new reason though: https://github.com/ursacomputing/crossbow/actions/runs/11860294408/job/33055563790#step:7:19

kevingurney commented 1 week ago

@kou - thanks for your review!

@assignUser - thanks for running the qualification locally! I'm sorry for the delay, I was caught up with some some other work. I was planning to try Kou's suggested approach of running the bot after merging these changes in but didn't get to that quickly enough. I appreciate your help!

I'll take a look at the new failure - sorry about that!

kevingurney commented 1 week ago

@assignUser - I think the failure was due to a syntax error in packageMatlabInterface.m. It looks like when I split the error message into two lines for the check to see if toolboxVersion is empty, I accidentally forgot the closing parenthesis.

Very sorry for the inconvenience!

I believe the packaging script should be fixed now.

assignUser commented 1 week ago

but didn't get to that quickly enough

Oh no worries, happy to help!

The new job: https://github.com/ursacomputing/crossbow/actions/runs/11862238463/job/33061195956

kevingurney commented 1 week ago

Thank you @assignUser! Looks like everything is passing now.

Unless anyone has any flags, I can go ahead and merge this.

kevingurney commented 1 week ago

+1

kevingurney commented 1 week ago

When I went to use the Python script for merging PRs (formerly merge_pr.py - now arrow_merge_pr.py), I noticed that the infrastructure for merging PRs has changed since I used it last.

I'll need to spend a bit of additional time configuring my MathWorks development machine before merging this in with the script. Unfortunately, I'll need to wait until next week to work on this.

If another Committer or PMC Member wanted to merge this in instead - that would work, as well.

My sincere apologies for the delay in getting this merged!

conbench-apache-arrow[bot] commented 1 week ago

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

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

kevingurney commented 1 week ago

Thanks, @kou!