Skip to content

build(cmake): fail fast with clear diagnostic when .venv has no pip - #14784

Merged
DonLakeFlyer merged 1 commit into
mavlink:masterfrom
DonLakeFlyer:venv-pip-diagnostic
Aug 8, 2026
Merged

build(cmake): fail fast with clear diagnostic when .venv has no pip#14784
DonLakeFlyer merged 1 commit into
mavlink:masterfrom
DonLakeFlyer:venv-pip-diagnostic

Conversation

@DonLakeFlyer

Copy link
Copy Markdown
Contributor

Description

A hand-rolled uv venv (the default, without --seed) creates a pip-less .venv. QGC reuses an existing .venv as-is, so configure later fails deep inside the mavlink CPM build with an opaque Python pip not found, pip is required error that gives no hint the problem is the local venv.

This adds a read-only check in PythonVenv.cmake at both venv-use sites: if the venv interpreter has no pip, configure fails immediately with an actionable message (python -m ensurepip --upgrade, or delete .venv and reconfigure so QGC recreates it with pip seeded via uv venv --seed).

Replaces #14767, which bootstrapped pip automatically via ensurepip. A diagnostic was preferred over auto-mutation: CMake shouldn't silently install packages into a venv the user manages by hand, and ensurepip itself can be unavailable (e.g. Debian without python3-venv), which would turn the auto-fix into a different opaque failure. Credit to @bytesByHarsh for identifying the problem.

Testing

Verified with cmake -P against both shapes:

  • venv created without pip → configure fails with the new actionable message
  • normal QGC-seeded .venv → check passes silently

Type of Change

  • CI/Build changes

By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).

This comment was marked as resolved.

A hand-rolled 'uv venv' (without --seed) creates a pip-less venv. QGC
reuses it and configure later dies inside the mavlink CPM build with an
opaque 'Python pip not found' error. Detect the missing pip up front and
fail with instructions instead.

Replaces mavlink#14767, which auto-bootstrapped pip via ensurepip; a read-only
diagnostic avoids CMake silently mutating a user-managed venv.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

cmake/modules/PythonVenv.cmake:27

  • The recreation fallback is not portable: rm -rf does not work in Windows cmd (or as written in Windows PowerShell), and both relative paths refer to the caller's working directory rather than CMAKE_SOURCE_DIR in an out-of-tree configure. Since this diagnostic runs on every platform, use platform-neutral deletion wording and print the source-qualified setup command.
                        "  rm -rf .venv && python tools/setup/install_python.py scripts")

@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 32.72%. Comparing base (f29efd3) to head (b33e157).
⚠️ Report is 251 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14784      +/-   ##
==========================================
+ Coverage   25.47%   32.72%   +7.25%     
==========================================
  Files         769      784      +15     
  Lines       65912    67602    +1690     
  Branches    30495    31311     +816     
==========================================
+ Hits        16788    22121    +5333     
+ Misses      37285    30595    -6690     
- Partials    11839    14886    +3047     
Flag Coverage Δ
unittests 32.72% <ø> (+7.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 477 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4edcc7...b33e157. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Build Results

Platform Status

Platform Status Details
Linux Passed View
Windows Passed View
MacOS Passed View
Android Passed View

All builds passed.

Pre-commit

Check Status Details
pre-commit Failed (non-blocking) View

Pre-commit hooks: 0 passed, 0 failed, 0 skipped.

Test Results

linux-coverage-integration: 37 passed, 0 skipped
linux-coverage-unit: 119 passed, 0 skipped
linux-sanitizers-integration: 20 passed, 0 skipped
linux-sanitizers-unit: 119 passed, 0 skipped
Total: 295 passed, 0 skipped

Code Coverage

Coverage: 69.5%

No baseline available for comparison

Artifact Sizes

Artifact Size
QGroundControl 221.79 MB
QGroundControl-aarch64 179.35 MB
QGroundControl-installer-AMD64 137.85 MB
QGroundControl-installer-AMD64-ARM64 80.37 MB
QGroundControl-installer-ARM64 109.13 MB
QGroundControl-linux 87.04 MB
QGroundControl-mac 87.04 MB
QGroundControl-x86_64 191.59 MB
No baseline available for comparison

Updated: 2026-08-08 18:39:51 UTC • Commit: b33e157 • Triggered by: Android

@DonLakeFlyer
DonLakeFlyer merged commit 492cfe5 into mavlink:master Aug 8, 2026
43 checks passed
@DonLakeFlyer
DonLakeFlyer deleted the venv-pip-diagnostic branch August 8, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants