ci: verify source using OS-installed dependencies #2718

lidavidm · 2025-04-21T12:19:57Z

We've only really tested Conda so far. Validate non-Conda builds too.

This only tests Ubuntu/APT but eventually we should also test Debian/APT, macOS/Homebrew, Fedora/RPM, and AlmaLinux/RPM.

We've only really tested Conda so far. Validate non-Conda builds too.

paleolimbot · 2025-04-22T16:10:42Z

dev/release/verify-release-candidate.sh

+  local dotnet_download_thank_you_url=https://dotnet.microsoft.com/download/thank-you/dotnet-sdk-${dotnet_version}-${dotnet_platform}-x64-binaries
+  show_info "Getting .NET download URL from ${dotnet_download_thank_you_url}"


I appreciate the politeness exuded by dotnet's download URL 🙂

ci/scripts/verify_ubuntu.sh

paleolimbot · 2025-04-22T16:13:27Z

ci/scripts/verify_ubuntu.sh

+    # Install R
+    # https://cloud.r-project.org/bin/linux/ubuntu/
+
+    wget -qO- https://cloud.r-project.org/bin/linux/ubuntu/marutter_pubkey.asc | \
+        tee -a /etc/apt/trusted.gpg.d/cran_ubuntu_key.asc
+
+    add-apt-repository "deb https://cloud.r-project.org/bin/linux/ubuntu $(lsb_release -cs)-cran40/"


I believe you can also just apt-get install r-base:

https://github.com/apache/arrow-nanoarrow/blob/4241f5f07a642a615912f339a5c089c1fe597ed0/ci/docker/ubuntu.dockerfile#L23

paleolimbot · 2025-04-22T16:14:21Z

.github/workflows/nightly-verify.yml

+          OS: ${{ matrix.os }}
+          OS_VERSION: ${{ matrix.version }}
+        run: |
+          # Hmm, -e doesn't work?


Does this work now?

I couldn't get it to work - I'll just remove the comment but I'm not quite sure what's going on here

Does it mean that this step isn't failed when docker compose run is failed?

Oh - no, the step works. What I mean is that I expected docker compose run -e UBUNTU=22.04 ... to also work, but it seems that docker ignores the -e switch.

Ah, I understand.

env UBUNTU=22.04 ... is used in ${UBUNTU} in compose.yaml and ... -e UBUNTU=22.04 ... is used in a container created by docker compose run. In this case, we want to use ${UBUNTU} in compose.yaml. So we need to use env UBUNTU=22.04 ... not ... -e UBUNTU=22.04 ... here.

Example:

lidavidm@rondo ~/C/arrow-adbc (minor-nightly-docker)> docker compose run --rm -e UBUNTU=22.04 -it verify-all-ubuntu /bin/bash WARN[0000] The "UBUNTU" variable is not set. Defaulting to a blank string. WARN[0000] /home/lidavidm/Code/arrow-adbc/compose.yaml: the attribute `version` is obsolete, it will be ignored, please remove it to avoid potential confusion [+] Creating 1/1 ✔ Network arrow-adbc_default Created 0.0s unable to get image 'ubuntu:': Error response from daemon: invalid reference format

It's expected. Because we use ${UBUNTU} at https://github.com/apache/arrow-adbc/pull/2718/files#diff-facaded51b7d1c9656ae43b449b69aa398fee9129321a0001d97048c00c8cc1cR282 .
It's in compose.yaml. It's referred BEFORE docker compose run creates a container. docker compose run -e sets an environment variable for container.

Ah sorry, I didn't see your first comment somehow. I suppose I misunderstood how they work together then, or maybe it's archery that uses -e to set variables for the container?

Both of Archery and docker compose run use -e to set environment variables for the container. (archery docker run is a wrapper of docker compose run.)

FYI: We need to use UBUNTU=24.04 archery docker run ubuntu-cpp to use Ubuntu 24.04 for ubuntu-cpp service. We can't use -e for it.

Ok, then I just misunderstood how it works in the first place, sorry 🙁

kou · 2025-04-22T21:40:20Z

.github/workflows/nightly-verify.yml

+        run: |
+          # Hmm, -e doesn't work?
+          pushd arrow-adbc
+          env ${OS@U}=${OS_VERSION} docker compose run --rm verify-all-${OS}


Wow! I didn't know @U!

ci/scripts/verify_ubuntu.sh

Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>

lidavidm · 2025-04-23T06:34:15Z

Hmm, that's an odd failure. I guess we need to explicitly install protoc somewhere.

lidavidm · 2025-04-23T06:34:55Z

Or rather, explicitly install a version newer than 3.12 (in Ubuntu 22.04)

lidavidm · 2025-04-23T14:08:53Z

Ok, this is finally green...

kou

+1

dev/release/verify-release-candidate.sh

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

We've only really tested Conda so far. Validate non-Conda builds too. This only tests Ubuntu/APT but eventually we should also test Debian/APT, macOS/Homebrew, Fedora/RPM, and AlmaLinux/RPM. --------- Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>

lidavidm force-pushed the minor-nightly-docker branch 4 times, most recently from 3823d24 to 5cf889c Compare April 21, 2025 12:47

lidavidm changed the title ~~ci: also verify under Docker with native deps~~ ci: also verify under Docker with native deps Apr 21, 2025

lidavidm force-pushed the minor-nightly-docker branch 11 times, most recently from cb2870b to 804d839 Compare April 22, 2025 12:01

lidavidm changed the title ~~ci: also verify under Docker with native deps~~ ci: also verify source using OS-installed dependencies Apr 22, 2025

lidavidm changed the title ~~ci: also verify source using OS-installed dependencies~~ ci: verify source using OS-installed dependencies Apr 22, 2025

lidavidm marked this pull request as ready for review April 22, 2025 12:02

lidavidm requested review from kou and zeroshade as code owners April 22, 2025 12:02

github-actions bot modified the milestone: ADBC Libraries 18 Apr 22, 2025

ci: also verify under Docker with native deps

9f9c7d4

We've only really tested Conda so far. Validate non-Conda builds too.

lidavidm force-pushed the minor-nightly-docker branch from 804d839 to 9f9c7d4 Compare April 22, 2025 12:39

paleolimbot approved these changes Apr 22, 2025

View reviewed changes

kou reviewed Apr 22, 2025

View reviewed changes

Apply suggestions from code review

acdaeb0

Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>

simplify R install

9922855

lidavidm force-pushed the minor-nightly-docker branch from 683e84f to 9922855 Compare April 23, 2025 07:08

kou approved these changes Apr 24, 2025

View reviewed changes

dev/release/verify-release-candidate.sh Outdated Show resolved Hide resolved

Update dev/release/verify-release-candidate.sh

a7895d0

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

lidavidm merged commit a4f86c9 into apache:main Apr 24, 2025
76 of 77 checks passed

lidavidm deleted the minor-nightly-docker branch April 24, 2025 02:41

		local dotnet_download_thank_you_url=https://dotnet.microsoft.com/download/thank-you/dotnet-sdk-${dotnet_version}-${dotnet_platform}-x64-binaries
		show_info "Getting .NET download URL from ${dotnet_download_thank_you_url}"

ci: verify source using OS-installed dependencies #2718

ci: verify source using OS-installed dependencies #2718

Uh oh!

Conversation

lidavidm commented Apr 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lidavidm commented Apr 23, 2025

Uh oh!

lidavidm commented Apr 23, 2025

Uh oh!

lidavidm commented Apr 23, 2025

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lidavidm commented Apr 21, 2025 •

edited

Loading