You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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\].
[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:
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.
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.
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).
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:
forpkgindiscover_targeted_packages(targeting_string, os.path.join("sdk", service_dir)):
forreqinopen(os.path.join(pkg, "dev_requirements.txt")):
ifis_relative_install_path(req.strip(), pkg):
ifParsedSetup.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:
build-test.yml exposes a BeforeTestSteps parameter that runs afterPrep Environment and beforeRun 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:
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.
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.
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.
Summary
PR #48371 added a
Check for compiled dev requirementsstep toeng/pipelines/templates/jobs/live.tests.yml(line 191). It gates a set ofcibuildwheelconfiguration 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>forpyproject.tomlfiles and regex-matches^\[tool\.cibuildwheel\].The problem
[tool.cibuildwheel]is not what actually triggers cibuildwheel.create_packagebranches onsetup_parsed.ext_modules— seeci_tools/build.py:243and: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-extensionshas no[tool.setuptools.ext-modules]in itspyproject.tomlat all;ParsedSetupfalls back to parsing the adjacentsetup.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: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 whatbuild_whl_for_reqactually 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.ymlsetsENABLE_EXTENSION_BUILDby reading thePackageInfoJSON folder.Two independent blockers:
The folder doesn't exist in this job.
live.tests.ymlcallsresolve-package-targeting.ymlwithoutPackagePropertiesFolder, which defaults to''. NoPackageInfofolder is produced anywhere in the live test chain.jobs/ci.tests.ymldoes pass it;jobs/live.tests.ymldoes not.The schema has no field for this anyway.
eng/scripts/get_package_properties.pyemits onlyname,version,is_new_sdk, folder, anddependent_packages. There is nothing about extensions or compilation.That is precisely why
resolve-build-platforms.ymlresorts to: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_compiledfield to the package properties schema and plumbPackagePropertiesFolderthroughlive.tests.yml. That's a larger change touching cross-language tooling (Package-Properties.ps1consumers), and it makes the live test job depend on asave-package-propertiesstep it doesn't currently run.Option B1 — Query
ParsedSetupdirectly (minimal change)Keep the current shape and scope; just fix the predicate. Ask the same parser
create_packageuses:Walk the package directories under the service directory and check
.ext_modulesinstead 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_reqexactly (most faithful)Replicate what actually happens at runtime. For each package in
$(TargetingString), read itsdev_requirements.txt, filter withis_relative_install_path, and check.ext_moduleson the resolved target:This is a line-for-line mirror of
build_whl_for_req— the function that invokes cibuildwheel. It uses the sameParsedSetup, the sameis_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:
azure-storage-blob/storagetrue— foundazure-storage-extensions, 1 ext moduleazure-keyvault-keys/keyvaultfalseIt 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 (
ParsedSetupexecssetup.py).Ordering constraint (applies to both B1 and B2)
ci_toolsis not importable where the step currently sits. Inlive.tests.ymlthe new steps run before thebuild-test.ymltemplate, and everything needed arrives inside it:steps/build-test.yml→use-python-version.ymlsteps/build-test.yml→use-venv.ymlazure-sdk-toolsinstallsteps/build-test.yml→Prep Environment(pip install -r eng/ci_tools.txt)build-test.ymlexposes aBeforeTestStepsparameter that runs afterPrep Environmentand beforeRun Tests— exactly the window required. All four cibuildwheel steps would move into that hook.There is a latent bug in the way:
live.tests.ymldeclaresBeforeTestStepsas a parameter (line 20) but never forwards it tobuild-test.yml. Live-test callers that set it have their steps silently dropped today.jobs/ci.tests.ymlforwards correctly.stages/archetype-sdk-tests.ymldoes pass the parameter intolive.tests.yml, so this is reachable from real callers —stages/cosmos-sdk-client.ymlis an existing consumer ofBeforeTestSteps.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. SettingCIBW_ARCHS,CIBW_SKIP,CIBW_TEST_SKIP, andCIBW_ENVIRONMENT_PASS_LINUXin a job that never invokes cibuildwheel has no effect whatsoever.So the
Configure cibuildwheel for dev requirement buildsstep 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) viaBeforeTestSteps, combined with Option C.Rationale:
Configure cibuildwheelstep unconditional. It is free, reduces the gated surface to the two Windows NuGet steps, and lowers the blast radius of any detection mistake.BeforeTestStepsforwarding bug inlive.tests.ymlregardless. It's a genuine defect with a ~3-line fix, and it unblocks the ordering constraint.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 hardcodedazure-storage-extensionsstring 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