Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 72 additions & 22 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -811,36 +811,86 @@ jobs:
- 1433:1433
steps:
- uses: actions/checkout@v4
- run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev alien libaio1 gnupg2 curl odbcinst1debian2 libodbc1 odbcinst # libmysqlclient-dev mysql-client odbc-postgresql
# - name: Setup MySQL ODBC connector
# run: |
# wget https://dev.mysql.com/get/Downloads/Connector-ODBC/8.2/mysql-connector-odbc_8.2.0-1ubuntu22.04_amd64.deb
# wget https://dev.mysql.com/get/Downloads/MySQL-8.2/mysql-community-client-plugins_8.2.0-1ubuntu22.04_amd64.deb
# sudo dpkg -i mysql-community-client-plugins_8.2.0-1ubuntu22.04_amd64.deb mysql-connector-odbc_8.2.0-1ubuntu22.04_amd64.deb

- name: Free disk space

- name: Pre-check runner disk & cleanup
run: |
sudo df
sudo apt autopurge
sudo apt clean
sudo rm -rf /usr/share/dotnet
sudo npm cache clean --force
sudo docker system prune -a --force
sudo df
- name: Setup Oracle ODBC connector
echo "Disk usage before cleanup:"
df -h .
free -h
sudo rm -f /var/lib/dpkg/lock-frontend /var/cache/apt/archives/lock || true
sudo apt-get -y autoremove --purge || true
sudo apt-get -y clean || true
sudo rm -rf /usr/share/dotnet || true
sudo npm cache clean --force || true
sudo docker system prune -a --force || true
echo "Disk usage after cleanup:"
df -h .

- name: Install basic system dependencies (robust)
env:
DEBIAN_FRONTEND: noninteractive
run: |
set -euxo pipefail
sudo rm -f /var/lib/dpkg/lock-frontend /var/cache/apt/archives/lock || true
sudo apt-get update -o Acquire::Retries=3
for i in 1 2 3; do
sudo apt-get install -y --no-install-recommends \
-o Dpkg::Options::="--force-confdef" \
-o Dpkg::Options::="--force-confold" \
libssl-dev unixodbc-dev alien libaio1 gnupg2 curl \
odbcinst1debian2 libodbc1 odbcinst || {
echo "apt-get install attempt $i failed; retrying in 10s..."
sleep 10
continue
}
break
done
Comment on lines +836 to +847
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The retry logic has a subtle bug: if the last attempt (attempt 3) fails, the script will exit with a non-zero status due to set -e, but the loop will have already completed all iterations. This means the retry won't actually fail the step - it will silently succeed.

The issue is that after the loop completes all 3 iterations without success, there's no explicit check to verify if the installation actually succeeded. Consider adding a verification after the loop:

for i in 1 2 3; do
  sudo apt-get install -y --no-install-recommends \
    -o Dpkg::Options::="--force-confdef" \
    -o Dpkg::Options::="--force-confold" \
    libssl-dev unixodbc-dev alien libaio1 gnupg2 curl \
    odbcinst1debian2 libodbc1 odbcinst && break || {
    echo "apt-get install attempt $i failed; retrying in 10s..."
    if [ "$i" -eq 3 ]; then
      echo "All 3 attempts failed"
      exit 1
    fi
    sleep 10
  }
done
Suggested change
for i in 1 2 3; do
sudo apt-get install -y --no-install-recommends \
-o Dpkg::Options::="--force-confdef" \
-o Dpkg::Options::="--force-confold" \
libssl-dev unixodbc-dev alien libaio1 gnupg2 curl \
odbcinst1debian2 libodbc1 odbcinst || {
echo "apt-get install attempt $i failed; retrying in 10s..."
sleep 10
continue
}
break
done
success=0
for i in 1 2 3; do
sudo apt-get install -y --no-install-recommends \
-o Dpkg::Options::="--force-confdef" \
-o Dpkg::Options::="--force-confold" \
libssl-dev unixodbc-dev alien libaio1 gnupg2 curl \
odbcinst1debian2 libodbc1 odbcinst && { success=1; break; } || {
echo "apt-get install attempt $i failed; retrying in 10s..."
sleep 10
continue
}
done
if [ "$success" -ne 1 ]; then
echo "apt-get install failed after 3 attempts"
exit 1
fi

Copilot uses AI. Check for mistakes.

- name: Setup Oracle ODBC connector (.deb) and fix deps
env:
DEBIAN_FRONTEND: noninteractive
run: |
set -euxo pipefail
wget https://www.devart.com/odbc/oracle/devartodbcoracle_amd64.deb
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The wget command lacks retry logic and timeout configuration, which could fail due to transient network issues. Given the PR's goal of hardening against transient failures, consider adding retry and timeout options:

wget --tries=3 --timeout=30 https://www.devart.com/odbc/oracle/devartodbcoracle_amd64.deb

This aligns with the retry philosophy applied to apt-get operations throughout the PR.

Suggested change
wget https://www.devart.com/odbc/oracle/devartodbcoracle_amd64.deb
wget --tries=3 --timeout=30 https://www.devart.com/odbc/oracle/devartodbcoracle_amd64.deb

Copilot uses AI. Check for mistakes.
sudo dpkg -i devartodbcoracle_amd64.deb
- run: ls -l /etc/apt/sources.list.d/
- name: Setup SQL Server ODBC connector
sudo dpkg -i devartodbcoracle_amd64.deb || true
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The Oracle ODBC dpkg command should also have explicit configuration options to avoid interactive prompts, similar to the Microsoft packages step. While || true allows it to fail, adding dpkg options would make the non-interactive intent clearer and match the hardening approach:

sudo DEBIAN_FRONTEND=noninteractive dpkg -i --force-confdef --force-confold devartodbcoracle_amd64.deb || true

This ensures dpkg won't prompt for configuration file conflicts.

Suggested change
sudo dpkg -i devartodbcoracle_amd64.deb || true
sudo dpkg -i --force-confdef --force-confold devartodbcoracle_amd64.deb || true

Copilot uses AI. Check for mistakes.
sudo apt-get update -o Acquire::Retries=3
sudo apt-get install -y -f
dpkg -l | grep -Ei 'devart|odbc|oracle' || true

- name: Setup Microsoft packages & msodbcsql18
env:
DEBIAN_FRONTEND: noninteractive
run: |
set -euxo pipefail
curl -sSL -O https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.deb
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The curl command lacks retry logic and timeout configuration. Given the PR's goal of hardening against transient failures, consider adding retry options:

curl --retry 3 --retry-delay 5 --max-time 60 -sSL -O https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.deb

This aligns with the retry philosophy applied to apt-get operations throughout the PR.

Suggested change
curl -sSL -O https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.deb
curl --retry 3 --retry-delay 5 --max-time 60 -sSL -O https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.deb

Copilot uses AI. Check for mistakes.
sudo dpkg -i packages-microsoft-prod.deb
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

Missing DEBIAN_FRONTEND=noninteractive environment variable on the dpkg command. While the env is set at the step level, being explicit when calling dpkg can prevent potential interactive prompts if the environment isn't properly inherited.

Consider adding it explicitly to the dpkg command:

sudo DEBIAN_FRONTEND=noninteractive dpkg -i packages-microsoft-prod.deb

This is especially important since this step had the -o Dpkg::Options:: flags for confdef/confold in the system dependencies step but those options are not applicable to dpkg command directly (they're apt-get options).

Suggested change
sudo dpkg -i packages-microsoft-prod.deb
sudo DEBIAN_FRONTEND=noninteractive dpkg -i packages-microsoft-prod.deb

Copilot uses AI. Check for mistakes.
rm packages-microsoft-prod.deb
sudo apt-get update
sudo apt-get update -o Acquire::Retries=3
sudo ACCEPT_EULA=Y apt-get install -y msodbcsql18
ls /opt/microsoft/msodbcsql18/include
- run: ls -l /etc/apt/sources.list.d/

- name: Capture apt/dpkg/system diagnostics on failure
if: failure()
run: |
set -euxo pipefail
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

Using set -euxo pipefail with a diagnostics step that should tolerate failures is problematic. The -e flag will cause the script to exit on the first command that returns a non-zero exit status, even though all commands have || true to prevent failures. While the || true will prevent individual command failures from stopping the script, the set -e is unnecessary here since this is a diagnostic step that should never fail.

Consider using set -uxo pipefail instead (without -e) for this diagnostic step, or remove set entirely since all commands already have || true fallbacks.

Suggested change
set -euxo pipefail
set -uxo pipefail

Copilot uses AI. Check for mistakes.
echo "===== APT / DPKG DIAGNOSTICS ====="
sudo tail -n +1 /var/log/apt/term.log || true
sudo tail -n +1 /var/log/apt/history.log || true
sudo tail -n +1 /var/log/dpkg.log || true
df -h || true
free -h || true
uname -a || true
dmesg --level=err,warn | tail -n 200 || true
mkdir -p $GITHUB_WORKSPACE/ci-apt-logs
sudo cp /var/log/apt/* $GITHUB_WORKSPACE/ci-apt-logs/ 2>/dev/null || true
sudo cp /var/log/dpkg.log $GITHUB_WORKSPACE/ci-apt-logs/ 2>/dev/null || true
ls -l $GITHUB_WORKSPACE/ci-apt-logs || true

- name: Upload apt/dpkg logs
if: always()
uses: actions/upload-artifact@v4
with:
name: apt-dpkg-logs
path: ci-apt-logs
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The artifact upload step uses a hardcoded path ci-apt-logs instead of $GITHUB_WORKSPACE/ci-apt-logs which is used in the diagnostic capture step. While this may work if the working directory is $GITHUB_WORKSPACE, it's inconsistent and could fail if the working directory changes.

For consistency and clarity, use the full path:

path: ${{ github.workspace }}/ci-apt-logs

or ensure the path matches what's created in line 883.

Suggested change
path: ci-apt-logs
path: ${{ github.workspace }}/ci-apt-logs

Copilot uses AI. Check for mistakes.
- run: >-
cmake -S. -Bcmake-build -GNinja -DPOCO_MINIMAL_BUILD=ON -DENABLE_DATA_ODBC=ON -DENABLE_TESTS=ON
- run: cmake --build cmake-build --target all --parallel 4
Expand Down
Loading