Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

[EngSys] Detect compiled dev requirements deterministically instead of regex-matching [tool.cibuildwheel]

Aperta
  • Qualcosa non va?
#48,372 0 commenti 1 reazione 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@danieljurek ci sta già lavorando.

Dal 30/7/2026.

Valutazione

Questa issue non è ancora stata valutata.

Descrizione

EngSys

Summary

PR #48371 added a Check for compiled dev requirements step to eng/pipelines/templates/jobs/live.tests.yml (line 191). It gates a set of cibuildwheel configuration steps on whether the service under test contains a package that needs them.

The gate works, but it is a heuristic: it recursively globs sdk/<ServiceDirectory> for pyproject.toml files and regex-matches ^\[tool\.cibuildwheel\].

$found += Get-ChildItem -Path $root -Filter pyproject.toml -Recurse -ErrorAction SilentlyContinue `
  | Where-Object { (Get-Content $_.FullName -Raw) -match '(?m)^\[tool\.cibuildwheel\]' }
The problem

[tool.cibuildwheel] is not what actually triggers cibuildwheel. create_package branches on setup_parsed.ext_modules — see ci_tools/build.py:243 and :292. It never reads the [tool.cibuildwheel] section.

Worse, for the one package this currently matters for, the file being grepped isn't where the truth lives. azure-storage-extensions has no [tool.setuptools.ext-modules] in its pyproject.toml at all; ParsedSetup falls back to parsing the adjacent setup.py (parse_functions.py:713-722). The regex happens to match because that package also declares cibuildwheel config — the two are correlated today, not causally linked.

I scanned every package under sdk/ and confirmed the sets are currently identical:

has ext_modules        : ['sdk/storage/azure-storage-extensions']
has [tool.cibuildwheel]: ['sdk/storage/azure-storage-extensions']
ext_modules but NO cibuildwheel section (missed by current check): []

So there is no active bug — this is latent. A package that grows an extension without adding a [tool.cibuildwheel] section (relying on cibuildwheel defaults) would silently fail to be detected, and live tests for that service would start failing at dev-requirement install exactly the way storage did — with a root cause that is considerably harder to spot the second time.

Secondary issue: the gate is scoped too broadly

The check answers "does this service directory contain a compiled package?" The condition that actually matters is "does any package under test in this job have a relative dev requirement that resolves to a package with ext_modules?" — which is what build_whl_for_req actually evaluates at runtime. A job targeting a single storage package that has no such dev requirement still gets the variables set.

Proposed solutions

Option A — Reuse package properties / ENABLE_EXTENSION_BUILD (not viable)

The natural instinct is to reuse what the build pipeline does. steps/resolve-build-platforms.yml sets ENABLE_EXTENSION_BUILD by reading the PackageInfo JSON folder.

Two independent blockers:

  1. The folder doesn't exist in this job. live.tests.yml calls resolve-package-targeting.yml without PackagePropertiesFolder, which defaults to ''. No PackageInfo folder is produced anywhere in the live test chain. jobs/ci.tests.yml does pass it; jobs/live.tests.yml does not.

  2. The schema has no field for this anyway. eng/scripts/get_package_properties.py emits only name, version, is_new_sdk, folder, and dependent_packages. There is nothing about extensions or compilation.

That is precisely why resolve-build-platforms.yml resorts to:

if ($packageProperties -contains "azure-storage-extensions") {

Adopting this pattern would trade a regex for a hardcoded package name — less deterministic, not more. Worth noting the build pipeline has the same latent problem, just expressed differently.

A variant would be to add an ext_modules / is_compiled field to the package properties schema and plumb PackagePropertiesFolder through live.tests.yml. That's a larger change touching cross-language tooling (Package-Properties.ps1 consumers), and it makes the live test job depend on a save-package-properties step it doesn't currently run.

Option B1 — Query ParsedSetup directly (minimal change)

Keep the current shape and scope; just fix the predicate. Ask the same parser create_package uses:

from ci_tools.parsing import ParsedSetup
ParsedSetup.from_path(package_dir).ext_modules

Walk the package directories under the service directory and check .ext_modules instead of regex-matching the file.

Pros: roughly a one-line semantic change; removes the proxy entirely; no parameter plumbing.

Cons: still service-directory-scoped rather than target-scoped; introduces a Python dependency into a step that is currently pure PowerShell (see the ordering constraint below).

Option B2 — Mirror build_whl_for_req exactly (most faithful)

Replicate what actually happens at runtime. For each package in $(TargetingString), read its dev_requirements.txt, filter with is_relative_install_path, and check .ext_modules on the resolved target:

for pkg in discover_targeted_packages(targeting_string, os.path.join("sdk", service_dir)):
    for req in open(os.path.join(pkg, "dev_requirements.txt")):
        if is_relative_install_path(req.strip(), pkg):
            if ParsedSetup.from_path(os.path.abspath(os.path.join(pkg, req.strip()))).ext_modules:
                # cibuildwheel will run

This is a line-for-line mirror of build_whl_for_req — the function that invokes cibuildwheel. It uses the same ParsedSetup, the same is_relative_install_path, and the same dev-requirements traversal, so it cannot drift from the behavior it is predicting.

I prototyped this locally against the real repo:

Input Result
azure-storage-blob / storage true — found azure-storage-extensions, 1 ext module
azure-keyvault-keys / keyvault false

It also tightens the gate from service directory to packages actually under test, addressing the secondary issue above.

Pros: correct predicate, correct scope, structurally incapable of diverging from runtime behavior.

Cons: most code; same ordering constraint as B1; ~1s per package parse cost (ParsedSetup execs setup.py).

Ordering constraint (applies to both B1 and B2)

ci_tools is not importable where the step currently sits. In live.tests.yml the new steps run before the build-test.yml template, and everything needed arrives inside it:

What Where
Interpreter selection steps/build-test.yml → use-python-version.yml
Virtualenv steps/build-test.yml → use-venv.yml
azure-sdk-tools install steps/build-test.yml → Prep Environment (pip install -r eng/ci_tools.txt)

build-test.yml exposes a BeforeTestSteps parameter that runs after Prep Environment and before Run Tests — exactly the window required. All four cibuildwheel steps would move into that hook.

There is a latent bug in the way: live.tests.yml declares BeforeTestSteps as a parameter (line 20) but never forwards it to build-test.yml. Live-test callers that set it have their steps silently dropped today. jobs/ci.tests.yml forwards correctly. stages/archetype-sdk-tests.yml does pass the parameter into live.tests.yml, so this is reachable from real callers — stages/cosmos-sdk-client.yml is an existing consumer of BeforeTestSteps.

Wiring it through is ~3 lines and fixes that bug as a side effect. It should arguably be fixed regardless of what happens to this issue.

Option C — Narrow what needs gating at all

Orthogonal simplification, combinable with any of the above.

The CIBW_* environment variables are inert unless cibuildwheel actually runs. Setting CIBW_ARCHS, CIBW_SKIP, CIBW_TEST_SKIP, and CIBW_ENVIRONMENT_PASS_LINUX in a job that never invokes cibuildwheel has no effect whatsoever.

So the Configure cibuildwheel for dev requirement builds step could be made unconditional at zero risk. Only the two Windows NuGet steps genuinely need gating, because those cost wall-clock time and can fail.

That shrinks the surface the detection logic has to protect from four steps to two, and correspondingly reduces the cost of the detection being wrong: a false negative would then only mean a Windows job fetching CPython from a blocked endpoint (loud, obvious) rather than a silent misconfiguration.

Recommendation

[!NOTE]
The following recommendation was produced by an LLM agent (GitHub Copilot CLI) during the investigation that led to PR #48371. It reflects analysis of the code paths cited above and locally prototyped verification, but has not been reviewed by a human engineer. Treat it as a starting point rather than a decision.

Option B2 (mirror build_whl_for_req) via BeforeTestSteps, combined with Option C.

Rationale:

  1. C first, independently — make the Configure cibuildwheel step unconditional. It is free, reduces the gated surface to the two Windows NuGet steps, and lowers the blast radius of any detection mistake.
  2. Fix the BeforeTestSteps forwarding bug in live.tests.yml regardless. It's a genuine defect with a ~3-line fix, and it unblocks the ordering constraint.
  3. Then B2, because it is the only option that cannot drift: it calls the same functions the runtime calls. B1 is a reasonable fallback if the parameter plumbing is unwanted — it is strictly better than the current regex for roughly one added line, and keeps every step where it is today.

Option A should be ruled out unless someone is separately motivated to add a compiled/extension field to the package properties schema, in which case resolve-build-platforms.yml's hardcoded azure-storage-extensions string would be worth fixing at the same time.

Priority: low. There is no active bug — the heuristic and the truth agree on every package in the repo today. This is about preventing a confusing failure mode later, and the cost of that failure mode is a service's live tests breaking with a non-obvious root cause.

Context
  • PR: #48371
  • Original failing build: 6634674 (22 of 24 jobs failed at dev-requirement install)
  • Validation build: 6636078
Lingua principale
Python
Stelle
5.6k
Fork
3.4k
Merge medio
1g 18h
PR unite (30g)
214

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Azure/azure-sdk-for-python

Tutte le issue di Azure/azure-sdk-for-python

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.