ci: verify source using OS-installed dependencies#2718
Conversation
lidavidm
force-pushed
the
minor-nightly-docker
branch
4 times, most recently
from
3823d24 to
5cf889c
Compare
lidavidm
changed the title
lidavidm
force-pushed
the
minor-nightly-docker
branch
11 times, most recently
from
cb2870b to
804d839
Compare
lidavidm
changed the title
lidavidm
changed the title
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
| 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}" |
There was a problem hiding this comment.
I appreciate the politeness exuded by dotnet's download URL 🙂
| # 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/" |
There was a problem hiding this comment.
I believe you can also just apt-get install r-base:
| OS: ${{ matrix.os }} | ||
| OS_VERSION: ${{ matrix.version }} | ||
| run: | | ||
| # Hmm, -e doesn't work? |
There was a problem hiding this comment.
I couldn't get it to work - I'll just remove the comment but I'm not quite sure what's going on here
There was a problem hiding this comment.
Does it mean that this step isn't failed when docker compose run is failed?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ok, then I just misunderstood how it works in the first place, sorry 🙁
| run: | | ||
| # Hmm, -e doesn't work? | ||
| pushd arrow-adbc | ||
| env ${OS@U}=${OS_VERSION} docker compose run --rm verify-all-${OS} |
Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Co-authored-by: Dewey Dunnington <dewey@dunnington.ca>
|
Hmm, that's an odd failure. I guess we need to explicitly install protoc somewhere. |
|
Or rather, explicitly install a version newer than 3.12 (in Ubuntu 22.04) |
lidavidm
force-pushed
the
minor-nightly-docker
branch
from
683e84f to
9922855
Compare
|
Ok, this is finally green... |
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>
3 participants
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.