Extend shared OneBranch pipeline to build mssql-odbc native driver

[gargsaumya]

Description

This pull request adds support for optionally building and injecting the native mssql-odbc driver into Python wheels as part of the CI pipeline, across Windows, Linux (glibc + musl), and macOS (x64 and ARM64) platforms. The implementation introduces new pipeline parameters, conditional build steps, and a new build template for macOS, as well as updates to existing templates to control artifact publishing.

Key changes:

Pipeline parameterization and documentation

• Added a new buildOdbcNative boolean parameter to the main pipeline YAMLs (.pipeline/OneBranch/NonOfficialPythonWheelsPublish.yml, .pipeline/OneBranch/stages.yml) to enable or disable building and injecting the native ODBC driver. Also updated documentation in .pipeline/OneBranch/stages.yml to describe the new workflow. [1] [2] [3] [4]

Conditional ODBC native driver build and injection

• Introduced conditional build and injection steps for the native ODBC driver for Windows (x64, ARM64), Linux (x64, ARM64, glibc + musl), and macOS (x64, ARM64) in .pipeline/OneBranch/stages.yml, ensuring the driver is only built and injected when buildOdbcNative is true. [1] [2] [3] [4] [5]

New and updated build templates

• Added a new template .pipeline/templates/build-odbc-macos-template.yml for cross-compiling the mssql-odbc driver for macOS x64 and ARM64, producing separate binaries for each architecture.
• Updated existing ODBC build templates (build-odbc-template.yml, build-odbc-alpine-template.yml, build-odbc-glibc228-template.yml) to support a new publishArtifacts parameter, allowing the caller to control whether the template publishes its own build artifacts or defers to the main pipeline. [1] [2] [3] [4] [5] [6]

Miscellaneous

• Improved version extraction from Cargo.toml in PowerShell to be more robust by matching only the [package] section.

Related Issues

ADO work item: https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/47791

Checklist

• cargo bfmt passes
• cargo bclippy passes
• cargo btest passes
• New/changed functionality has tests
• Public API changes are documented

[Copilot]

Pull request overview

Extends the shared OneBranch wheel pipeline to optionally build and package native ODBC drivers.

Changes:

• Adds conditional Windows and Linux ODBC builds.
• Combines ODBC outputs with existing job artifacts.
• Creates a separate native-driver NuGet package.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
.pipeline/templates/build-odbc-windows-template.yml	Adds native Windows driver build and staging.
.pipeline/templates/build-odbc-template.yml	Makes glibc artifact publishing optional.
.pipeline/templates/build-odbc-alpine-template.yml	Makes musl artifact publishing optional.
.pipeline/templates/build-odbc-glibc228-template.yml	Makes glibc-2.28 artifact publishing optional.
.pipeline/OneBranch/stages.yml	Integrates builds, artifact collection, and NuGet packaging.
.pipeline/OneBranch/NonOfficialPythonWheelsPublish.yml	Exposes the optional native ODBC build parameter.

Suppressed comments (3)

.pipeline/OneBranch/stages.yml:241

• Like the preceding variant, the Alpine container leaves odbc-drop root-owned. Running this cleanup as the host agent can fail on the contents and prevent the glibc-2.28 build; that next container already removes the drop as root, so delete this host-side task.

      - script: rm -rf $(Build.SourcesDirectory)/odbc-drop
        displayName: 'Clear odbc-drop before next ODBC variant'

.pipeline/OneBranch/stages.yml:313

• The ARM64 glibc container creates this mounted drop as root, while this task runs as the host agent. The repository's container build script already documents and handles this ownership boundary by cleaning on entry as root, so this host cleanup can fail with permission denied. Remove it and let the following Alpine container perform the cleanup.

        - script: rm -rf $(Build.SourcesDirectory)/odbc-drop
          displayName: 'Clear odbc-drop before next ODBC variant'

.pipeline/OneBranch/stages.yml:326

• This second ARM64 host cleanup has the same ownership failure: the Alpine container produced a root-owned mounted tree, so the agent may be unable to remove its contents. Delete this task because the glibc-2.28 container cleans odbc-drop as root before building.

        - script: rm -rf $(Build.SourcesDirectory)/odbc-drop
          displayName: 'Clear odbc-drop before next ODBC variant'

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Balanced

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 100%

🎯 Overall Coverage 93.4%

📦 Project: mssql-tds + mssql-odbc + mssql-py-core ℹ️ Note: diff coverage is reported, not enforced.

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

---

🔗 Quick Links

View Azure DevOps Build · Coverage Report

[Copilot]

🔵 Needs a closer look

The multi-platform build and packaging paths require successful end-to-end OneBranch validation before approval.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[ttk (Theekshna)]

Unattended review (never reviewed before). This is a pipeline/infra-only change -- no Rust source touched (.pipeline/** YAML + mssql-odbc/scripts/finalize-artifact.sh), so cargo bfmt/bclippy/btest are trivially unaffected; the unchecked checklist boxes for those don't indicate a real gap here.

Changes verified:

• publishArtifacts toggle added identically to all four build-odbc-*-template.yml templates (-alpine-, -glibc228-, the Linux base, and the new Windows one), each defaulting to true -- confirmed this preserves validation-stages.yml's existing per-template-publish behavior unchanged, exactly as the PR description claims.
• New build-odbc-windows-template.yml follows the same shape as the Linux ones (driver-only, CI-non-PR-gated, publishArtifacts toggle); CARGO_TARGET_DIR usage matches the existing $(Build.SourcesDirectory)-as-target-dir convention already used throughout validation-stages.yml, not something newly invented here.
• New build-odbc-macos-template.yml builds one arch slice per invocation via cargo build --target, explicitly not lipo-fusing -- finalize-artifact.sh's new optional target-triple argument (${TARGET_TRIPLE:+$TARGET_TRIPLE/}) is correct bash parameter expansion for inserting the triple into cargo's standard target/<triple>/<profile>/ layout, and the existing native-build call path (empty triple) is unchanged.
• stages.yml's buildAllTargets gating (ARM64/macOS entries only added to the platform map when true) matches the pre-existing job-level conditionals already gating those jobs -- not a new invariant, just followed correctly.
• Package versioning (Schedule -> -nightly.<date>, official Manual -> bare version, everything else -> -dev.<date>.<buildId>) is a reasonable, unambiguous scheme; nuspec generation and NuGet pack step look standard.

Test sufficiency: infra changes like this can't be unit-tested in the usual sense; the PR's own strategy -- land behind a default-false buildOdbcNative flag, validate end-to-end on the Non-Official pipeline first, then port identically to Official -- is the appropriate substitute, and I confirmed the actual GH-Non-Official Python Wheels Publish run on this PR is exercising the new ODBC build lanes now (Windows x64 leg passed at review time, others in flight). No CI failures found via gh pr checks.

msodbcsql parity: N/A for the pipeline mechanics; the one factual claim referencing msodbcsql (shipping separate per-arch macOS dylibs rather than a universal binary) matches its publicly documented release layout -- not something I could or needed to check against the internal source tree, which is source only and has no packaging scripts.

Divergences documented: N/A -- this doesn't touch driver behavior.

PR description currency: matches the diff; linked to an ADO work item (AB#47791... actually a direct work-item URL, not the AB# shorthand, but same target). Checklist boxes are unchecked but not misleading for a non-Rust change as noted above.

Verbose slop: none found -- comments are functional and specific (e.g., explaining why macOS doesn't lipo-fuse, why publishArtifacts exists, why the nested build/ layout matters for the Publish stage's extraction step).

Category	Count
Blocking	0
Suggestion	0
Nit	0

[Saurabh Singh (saurabh500)]

gargsaumya why a different nuget package? Can't we inject mssqlodbc into the current python wheels which are ultimately nuget packed? That's anyhow the ultimate goal. The same wheels get published to PyPi

[David Engel (David-Engel)]

Solid structure overall. The publishArtifacts parameterization is backward compatible (defaults to true, so the existing validation-stages.yml callers are unaffected), the optional target-triple arg on finalize-artifact.sh is compatible with all six existing callers, and the build/<driver> drop layout is consistent across Windows/Linux/macOS and matches what the Publish job extracts. A few things I'd like resolved before this merges.

Blocking-ish

1. None of the new code is exercised by any check. PR-triggered runs get buildOdbcNative: false (queue-time parameters can't be set by PR triggers), and every new step is separately gated on ne(Build.Reason, 'PullRequest'). The green GH-Non-Official Python Wheels Publish check proves only that the YAML parses — not that a single ODBC binary builds, stages, or packs. Could you queue a manual run with buildOdbcNative=true and link it here?

2. The Linux CopyFiles steps copy the entire CMake e2e build tree. See the inline comment on stages.yml.

3. This is also an SDL exposure change. NonOfficialPythonWheelsPublish.yml sets globalSdl.binskim.scanOutputDirectoryOnly: true. Today the output directory holds only .whl zips, which BinSkim skips. This PR puts raw mssqlodbc.dll / .so / .dylib — plus, per #2, a pile of unhardened gtest binaries and .o files — directly into the scanned directory. Rust cdylibs don't carry /guard:cf or Spectre mitigations, and the pipeline's own scanOutputDirectoryOnly comment already acknowledges that for build scripts. I'd expect BinSkim findings on the Windows DLL at minimum. The manual run in #1 is what proves this one way or the other.

Should fix

4. Linux builds the whole C++ e2e suite just to extract a .so. containerized-odbc-build.sh runs apt-get install cmake unixodbc-dev, then CMake + FetchContent(googletest) + a full gtest build — six times per run (3 tracks x 2 arches) — on a pipeline already at ~48 min. The Windows and macOS templates you added are correctly driver-only. A --driver-only path through build_e2e.sh (or a small driver-only entry script) would make the three platforms consistent and cut a lot of time.

5. build-odbc-windows-template.yml hardcodes the cargo output path. See inline.

6. The macOS comments are factually wrong. See inline.

7. ODBC binaries are never signed. Wheels get signWindowsWheels: ${{ parameters.isOfficial }}; the ODBC DLL/dylib/so go into the nupkg unsigned. That's fine for the non-official pipeline, but stages.yml is shared with OfficialPythonWheelsBuild.yml — someone flipping buildOdbcNative on there later would ship unsigned binaries in an official package with no signal that anything is wrong. Worth either an explicit comment documenting the gap or a guard that fails when isOfficial && buildOdbcNative until signing is wired up.

Nits

• macOS drop isn't cleaned between passes. The x64 and ARM64 invocations of build-odbc-macos-template.yml share $(Build.SourcesDirectory)/odbc-drop. Safe today (one file, overwritten), but the Linux scripts deliberately rm -rf the drop dir for exactly this reason. A one-line clean in the staging step makes the ARM64 pass provably free of x64 leftovers.
• mssql-odbc/README.md documents finalize-artifact.sh usage but not the new optional target-triple argument.
• The ODBC nuspec uses the full $(Build.SourceVersion); the wheels nuspec just above uses the short SHA. Minor inconsistency.
• The isOfficial branch in the ODBC version switch is unreachable today (only the non-official pipeline passes buildOdbcNative). Fine as future-proofing — just noting it isn't covered.
• Nightly never produces the ODBC package. Since buildOdbcNative defaults to false and is queue-time only, the scheduled run won't build it, so consumers only ever get -dev.<date>.<buildid> versions from manual runs — the Schedule -> -nightly.<date> branch is dead code in practice. Is that intended, or should the nightly opt in?

[Shiwani Gupta (shiwanigupta0809)]

Read the full diff plus the head state of stages.yml, the three Linux ODBC templates and their container entry scripts, finalize-artifact.sh, the Publish job's artifact map, and both official pipelines. Read the existing threads first — not re-raising the package name, the root-owned drop cleanup, the ARM64 map gating, the version derivation, the Windows ARM64 target, the contents: '**' weight, the SDL/BinSkim exposure, the e2e-suite build cost, the hardcoded cargo path, the macOS host comments, signing, or the nits. Those cover the diff well.

One thing I did not see raised.

Blocking: none today. Suggestions: 1.

Suggestion

The ten new Collect ODBC … CopyFiles@2 tasks carry no condition:, while every step in the templates they follow is gated on ne(Build.Reason, 'PullRequest'). That asymmetry turns into a hard failure on every PR run the moment buildOdbcNative is on by default — and the manual run being requested in the review above cannot detect it.

The templates gate their real work:

condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))

That appears on the build step, the stage-into-drop step, and the publish step in all four build-odbc-*-template.yml files. The collect steps in stages.yml do not:

- task: CopyFiles@2
  displayName: 'Collect ODBC native driver (Windows x64)'
  inputs:
    sourceFolder: '$(Build.SourcesDirectory)\odbc-drop'
    targetFolder: '$(ob_outputDirectory)\odbc_windows_x64'
    contents: '**'

I checked all of them rather than sampling: 10 Collect ODBC … tasks (Windows x64 / ARM64, Linux x64 ×3, Linux ARM64 ×3, macOS ×2), zero with a condition:.

On a PR-reason run with the block enabled, the template steps skip, odbc-drop is never created, and CopyFiles@2 fails on a non-existent sourceFolder — so the job fails after the wheels have already built. NonOfficialPythonWheelsPublish.yml carries pr: - main, so this pipeline does run on PRs into main; it is only the default: false on the queue-time parameter that keeps the block compiled out today.

Two reasons this is worth fixing now rather than when it fires:

• It is on the path this PR is already headed down. The nit above about the nightly never producing an ODBC package has the same remedy — make buildOdbcNative default on, or set it in the pipeline rather than at queue time. Either change lands directly on this.
• The requested validation cannot catch it. A manual queue with buildOdbcNative=true runs with Build.Reason = Manual, which takes the happy path; so does Schedule. PullRequest is the one reason that breaks, and it is the one reason a queue-time parameter can never be set for. A green manual run would be read as "the feature works", and it would be right about everything except this.

Adding the same condition to each collect task is the smaller change and keeps the two halves of each pair reading alike:

- task: CopyFiles@2
  displayName: 'Collect ODBC native driver (Windows x64)'
  condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
  inputs: …

Worth a moment's thought on whether the runtime ne(Build.Reason, 'PullRequest') inside the templates is still the right mechanism for this caller at all, given the whole block is already excluded at compile time by ${{ if eq(parameters.buildOdbcNative, true) }}. The condition has to stay for validation-stages.yml, which invokes the same templates unconditionally — so duplicating it on the collect steps is the fix that does not disturb the other caller.

[Shiwani Gupta (shiwanigupta0809)]

Reviewed — see #458 (comment)

[Saurabh Singh (saurabh500)]

Why a different nuget package? Can't we inject mssqlodbc into the current python wheels which are ultimately nuget packed? That's anyhow the ultimate goal. The same wheels get published to PyPi.

If we package the odbc binary in the current wheels, then that takes us closer to publishing a pypi package, which is already available for consumption as is by python. Else we will need to consolidate later.

Putting a request change till we can close out on this topic.

[gargsaumya]

Why a different nuget package? Can't we inject mssqlodbc into the current python wheels which are ultimately nuget packed? That's anyhow the ultimate goal. The same wheels get published to PyPi.

If we package the odbc binary in the current wheels, then that takes us closer to publishing a pypi package, which is already available for consumption as is by python. Else we will need to consolidate later.

Putting a request change till we can close out on this topic.

Hey Saurabh Singh (@saurabh500), I want to make sure we’re aligned after today's discussion before I restructure the pipeline work.

For mssql-python, the original plan was to keep the existing setup and add the new Rust ODBC piece on top. mssql-rs would ship raw platform binaries, and mssql-python would package those into proper wheels and publish them to PyPI, similar to how it already handles the msodbcsql18 ODBC driver.

Based on today’s discussion, our understanding of the updated shape is:

• mssqlodbc will be built into the same wheel as mssql_py_core - one PyO3 extension + one driver binary - across the ~35-wheel matrix (5 Python versions × 7 platforms). These will be published as a new mssql-python-rs package, replacing the earlier plan of mssql-python-rust-odbc package and the mssql-odbc-native / mssql-py-core-wheels NuGet split.
• import mssql_py_core remains unchanged, so there’s no downstream impact to the existing import path.
• Since the two components now build together, mssql_py_core will also use the same manylinux_2_28 container as mssqlodbc. This addresses the current gap where pip install mssql-python cannot install on RHEL 8.
• The trade-offs are that the mssqlodbc binary will be duplicated across Python-version wheels for each platform (storage cost only), and driver-only fixes will require a full mssql-python-rs rebuild rather than being shipped independently.

Please let me know if this matches your understanding of the direction. If so, I’ll proceed with restructuring the pipeline accordingly.

Also, one open question I want to resolve before starting the pipeline work again is who will own the release of mssql-python-rs - Python or Rust?

• If Python owns the release: we can reuse the existing release pipeline and PyPI credentials that mssql-python already uses for mssql-python-odbc. This would essentially be an additional artifact in the existing flow, with minimal new publishing setup.
• If Rust owns the release: mssql-rs does not currently publish to PyPI, so this would be a net-new capability for its pipeline. I’d need to engage ESRP to determine whether the existing mssql-python PyPI credential setup can be reused, or whether mssql-rs would require separately provisioned credentials.
  So, can you confirm which release ownership we’re going with? If Rust owns it, I’ll start the ESRP discussion.

cc: David Engel (@David-Engel), Sumit Sarabhai (@sumitmsft)

[Saurabh Singh (saurabh500)]

gargsaumya

Since the two components now build together, mssql_py_core will also use the same manylinux_2_28 container as mssqlodbc. This addresses the current gap where pip install mssql-python cannot install on RHEL 8.

For the nuget packing, we wont build mssql_py_core in manylinux_2_28. It will build in 2_34 and will ship odbc meant for 2_34.

The Nuget packaging is not meant to address the RHEL compat.
When we ship to PyPi then we will build and ship for 2_28 and 2_34 and will package the 2_28 compat odbc in 2_28 compat wheel.

About the size explosion, there is a ways of mitigating it mentioned below, but that happens during the PyPi publish with a tweak to the python versions that we build for.
About ESRP, I believe the same credentials (the AKV cert I think) are shared across all the drivers. The intention is to keep the ESRP publish in mssql-rs. We should simply setup a pipeline in mssql-rs to publish a dummy package. If that fails then we can use mssql-python as the fallback option.

To recap:
Iterative change

1. Add the odbc 2.34 compatible binaries to the existing Wheels for mssql_py_core which are nuget packed and downloaded by mssql-python. Do not use the 2_28 manylinux.

---

Refactor and start the PyPi prep

• Change the name of mssql-py-core Package to mssql-python-rs (it would be preferred to change the module name as well and make corresponding changes in mssql-python to consume this).
• Parallel, publish a dummy mssql-python-rs package to start the 7 day cool off period. This should preferably happen from the mssql-rs repo.

---

---

• Now we will build the mssql-python-rs PyPi package containing mssql-python-rs (earlier mssql-py-core) and odbc driver for 2_28 and 2_34 with the lowest python version supported instead of one wheel per Python version. One per python version is an overkill.
• Publish the Pypi package to Azure internal feed and see if Python can consume it and can support RHEL8 along with the existing tests it runs.
• Publish to PyPi.

---

[Saurabh Singh (saurabh500)]

Changes have been acknowledged so changing the review to Comment, in case this needs to be approved by other members.

Resetting my review to take another pass at the latest changes.

[gargsaumya]

All inline nits are addressed in 1363411.

[David Engel (David-Engel)]

Automated review — generated by GitHub Copilot on behalf of David Engel (@David-Engel). This is not an approval and does not satisfy the human review requirement. Findings may be incomplete or wrong; push back on anything that looks off.

Re-reviewed at 1363411d. The earlier round (package name, root-owned drop cleanup, ARM64 map gating, version derivation, Windows ARM64 target, contents: '**' weight, missing collect conditions, SDL/BinSkim exposure, e2e build cost, hardcoded cargo path, macOS host comments, signing, nits) is already covered — not re-raising any of it. The wheel-injection restructure (85b5ee4f) is the part no prior review has seen, so that is where I focused.

Verified good, so you don't have to re-check it:

• I ran scripts/inject-odbc-into-wheels.py locally against synthetic wheels carrying all seven platform tags this pipeline actually produces (win_amd64, win_arm64, manylinux_2_34_x86_64, manylinux_2_34_aarch64, musllinux_1_2_x86_64, musllinux_1_2_aarch64, macosx_15_0_universal2 — the last one taken from build-python-wheels-template.yml:116). Every tag maps to the right driver path, universal2 correctly gets both slices, and the regenerated RECORD verifies clean: every hash and size matches, no entry is missing, RECORD's own line is hash-less and last, and directory entries are excluded from RECORD while surviving as real directory entries in the archive.
• buildAllTargets gating is consistent in both directions: the ARM64/macOS ODBC blocks sit inside the same ${{ if eq(parameters.buildAllTargets, true) }} as the jobs they extend, and the $map in the staging step guards the same eight paths behind $buildAllTargets.
• Dropping the host-side rm -rf odbc-drop steps is safe. containerized-odbc-build.sh:29 and scripts/dockerentry/alpine-odbc-build.sh:30 both rm -rf "$DROP_DIR" as root on entry, so the glibc → musl sequence in a shared job can't leak a stale .so into the musl collect.
• The collect tasks now all carry condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest')).

Required checks are pending, none failing.

Suggestions

1. The injected glibc driver isn't built against the ABI floor the wheel advertises. .pipeline/OneBranch/stages.yml:224 builds it via build-odbc-template.yml, whose default image is ghcr.io/microsoft/mssql-rs/build/ubuntu:22.04 — containers/Dockerfile.Ubuntu.Build:1 is ubuntu:22.04, glibc 2.35. The wheel it gets injected into is built in manylinux_2_34_x86_64 (containers/Dockerfile.PythonBuild.manylinux.x64:10) and tagged manylinux_2_34_*. Because injection happens in the Publish stage, after the wheel is built and after any auditwheel repair, nothing re-validates the archive: a GLIBC_2.35 reference in mssqlodbc.so ships inside a wheel that pip will happily install on a glibc-2.34 host, and the failure surfaces as a dlopen error at first connect rather than at install. This also cuts against the shape agreed in the thread above ("It will build in 2_34 and will ship odbc meant for 2_34"). Building the glibc driver in the same manylinux_2_34 image is the clean fix; a cheap interim guard is a readelf --version-info/objdump -T assertion in the collect step that fails if the max required GLIBC_ version exceeds 2.34. musl is fine — Alpine 3.18 is musl 1.2.x and musllinux_1_2 carries no symbol-version contract.

2. The libs/ layout is written against an upstream resolver that isn't public yet. The docstring in scripts/inject-odbc-into-wheels.py cites GetDriverPathForProviderCpp in ddbc_bindings.cpp and roots the tree at the mssql_py_core package directory. I pulled microsoft/mssql-python@main: there is no GetDriverPathForProviderCpp, no mssql-odbc provider, and GetOdbcLibsBaseDir() (ddbc_bindings.cpp:1003) resolves the base from the mssql_python_odbc package, not mssql_py_core — this repo's own .pipeline/scripts/swap-mssql-python-odbc-driver.sh:8-14 documents that same rooting. The per-platform shape you chose does match what's there (libs/linux/<glibc|musl>/<arch>/lib/, libs/macos/<arch>/lib/, libs/windows/<x64|arm64>/, with arm64/x86_64 and Windows normalized to x64), so this isn't a layout error — but the base directory differs from anything that exists today, and neither repo has a test that would catch a drift. Worth naming the upstream PR/commit in the docstring so the next reader can tell "unlanded contract" from "typo", and ideally pointing the cross-repo mssql-python suite on mssql-odbc driver job at an injected wheel once that lands.

Nits

• scripts/inject-odbc-into-wheels.py:750 — zipfile.writestr(name, data) with a str name discards the source ZipInfo, so the rewritten wheel gets 0o600 on every file (including mssql_py_core's own PyO3 extension) plus a fresh mtime. I confirmed this locally: 0644/0755 in, 0600 out, on all seven wheels. It's benign under pip, which recreates files through open() and only re-applies an exec bit for entries that had one, but anyone extracting the wheel directly gets owner-only files, and the wheels stop being byte-reproducible. Carrying the original ZipInfo through for existing entries and minting one only for the additions costs a couple of lines.
• scripts/inject-odbc-into-wheels.py:706 — the wheels directory is filled by contents: '**/*.whl' with flattenFolders: true across all downloaded artifacts, and any wheel that isn't recognized is a hard SystemExit. That's the right default for today's matrix, but it means one stray .whl in any future artifact fails the Publish job after everything has already built. Fine as-is if intentional — just flagging that the strictness is load-bearing on nothing else ever dropping a wheel.

Category	Count
Blocking	0
Suggestion	2
Nit	2

[David Engel (David-Engel)]

Automated review — generated by GitHub Copilot on behalf of David Engel (@David-Engel). This is not an approval and does not satisfy the human review requirement. Findings may be incomplete or wrong; push back on anything that looks off.

Re-reviewed at e05eba7b. Everything from the earlier rounds (package/nuspec shape, drop cleanup, buildAllTargets gating, Windows ARM64 target, hardcoded cargo path, macOS host comments, signing gap, RECORD regeneration and wheel-entry permissions, the glibc ABI-floor question, the unlanded libs/ resolver contract) is unchanged or already covered — not re-raising any of it. The two new commits are what I focused on: bbcd8351 moving injection out of the Publish job into each Build job, and e05eba7b remapping the bare linux_<arch> tag.

Moving injection into the Build jobs is the right call — it removes the Pipeline.Workspace path-string coupling to OneBranch's artifact naming, kills the fragile $map/$buildAllTargets duplication, and makes the Official Build artifacts self-contained. It also incidentally resolves the earlier SDL concern, since raw binaries no longer land in the scanned output directory (see suggestion 2 for the flip side).

Verified good, so you don't have to re-check it: I exercised drivers_for_platform_tag locally against all twelve tags this matrix could plausibly produce. linux_x86_64 / linux_aarch64 / manylinux_2_34_* map to glibc, musllinux_1_2_* maps to musl (the elif ordering is correct — musllinux contains linux and is checked first), win_amd64 / win_arm64 map correctly, macosx_*_universal2 returns both slices, and macosx_15_0_arm64 / linux_armv7l / any return [] and would be caught by the SystemExit. The macOS and Windows staging steps are symmetric and correct, and the Linux sudo chown before injection is necessary and correctly placed (the wheels are written as root through the container bind mount, and the script writes a sibling .tmp then replace()s).

Required checks are pending; none failing.

Blocking

1. The Linux ARM64 job never stages the musl driver. Compare stages.yml:242-247 (x64) with stages.yml:316-329 (ARM64): x64 has a Stage musl driver (Linux x64) step between the Alpine template and the injection step, ARM64 does not. build-odbc-alpine-template.yml runs for ARM64 and leaves odbc-drop/build/mssqlodbc.so, but nothing copies it to drivers/linux/musl/arm64/lib/mssqlodbc.so, so that path never exists when the injection script runs. inject-odbc-into-wheels.py:114-118 treats a missing staged driver as a hard SystemExit, so the moment any wheel in that job maps to _MUSL_ARM64 the ARM64 build job fails after everything has already built. If no wheel maps to it (see finding 2), the ARM64 musl driver you just spent a containerized cross-build producing is silently discarded. Either way this looks like an editing slip rather than an intentional asymmetry.

2. glibc and musl wheels may not be distinguishable by platform tag, which would make the musl mapping dead and the injected driver wrong. e05eba7b establishes empirically that auditwheel = "skip" leaves the manylinux-container wheels tagged linux_x86_64 rather than manylinux_2_34_x86_64. That same [tool.maturin] auditwheel = "skip" (mssql-py-core/pyproject.toml:22) governs the musllinux container build too — scripts/build-python-wheels-in-container.sh:78-82 invokes plain maturin build with no --compatibility in both cases. In maturin, the musllinux_x_y tag comes out of the auditwheel policy analysis; with that skipped the platform tag falls back to PlatformTag::Linux, which renders as linux regardless of libc. If that holds here, three things follow:

• The Linux x64 job builds glibc wheels (stages.yml:215-219) and musl wheels (stages.yml:220-224) into the same $(ob_outputDirectory)/wheels directory. Identical tags mean identical filenames, so the Alpine build silently overwrites the manylinux wheels. Same for ARM64.
• The musllinux branch in drivers_for_platform_tag and both Stage musl driver steps are dead code, and whatever wheel survives gets the glibc .so injected — which will not dlopen on Alpine.
• docs/release-management.md:284-285 documents musllinux_1_2_*.whl filenames that would not be what the pipeline actually emits.

The collision itself predates this PR, but this PR's correctness rests entirely on the tag distinction, so it needs settling here. Cheap way to confirm: in the last non-PR run, compare the ls -lh "$(ob_outputDirectory)/wheels" output at the end of the manylinux step against the one at the end of the musllinux step in the Linux x64 job — if the wheel count doesn't roughly double, they are colliding. If it does collide, passing an explicit --compatibility manylinux_2_34 / --compatibility musllinux_1_2 (or building each libc into its own --out subdirectory) fixes the filenames, the tags, and this mapping in one go.

Suggestions

1. On the Official path, uninjected wheel copies survive in the artifact and can win the Publish-stage flatten. build-python-wheels-template.yml:199 moves the freshly built wheels into $(ob_outputDirectory)\unsigned-wheels, and the cleanup at line 297/347 removes wheels, signed-wheels, wheel-verify, and wheel-unpacked — but never unsigned-wheels. Those originals stay inside ob_outputDirectory and get published in the Build artifact alongside the signed, now-injected ones. The Publish job then collects with contents: '**/*.whl' and flattenFolders: true (stages.yml:425-431) into a single directory, and the stale copies have the same filenames — so which one lands in the nupkg depends on CopyFiles traversal order. Previously the injection happened after that flatten, so it applied to whichever copy won; now it happens before, so a stale copy silently ships without the driver (and unsigned). bbcd8351's message calls out unblocking the Official path specifically, and this is the one hazard unique to it. Removing $unsignedDir in the same cleanup at line 347 is the small fix.

2. The native driver is now invisible to both artifact consumers and BinSkim. With publishArtifacts: false on all four ODBC templates and the CopyFiles collect steps deleted, no raw mssqlodbc.dll / .so / .dylib — and no PDB — is published anywhere in this pipeline; the binaries exist only inside .whl zips. That also means globalSdl.binskim.scanOutputDirectoryOnly: true (NonOfficialPythonWheelsPublish.yml:64-66) will skip them, since BinSkim doesn't descend into zip archives. That resolves the SDL noise I flagged in the earlier round, but it flips it into the opposite problem for the Official path: a Rust cdylib ships in a released package having never been scanned, and there's no artifact to hand to anyone debugging a crash. Worth an explicit decision and a comment, even if the answer is "acceptable for now".

Nits

• stages.yml:251 and :324 — sudo chown -R $(whoami):$(whoami) works only because Azure Pipelines leaves the unrecognized $(whoami) macro untouched for bash to expand. It matches the existing pattern at build-python-wheels-template.yml:63, so it's consistent, but any pipeline variable ever named whoami would silently rewrite it. sudo chown -R "$USER" ... sidesteps the macro-syntax collision without changing behavior.
• Staging root — drivers/ is created under $(Build.SourcesDirectory) (the repo checkout) rather than $(Build.StagingDirectory) or $(Agent.TempDirectory). It's the same tree the ODBC container builds bind-mount, and on Linux it's also CARGO_TARGET_DIR. Harmless today, but a staging directory is the more natural home for scratch output.

Category	Count
Blocking	2
Suggestion	2
Nit	2

[David Engel (David-Engel)]

Automated review — generated by GitHub Copilot on behalf of David Engel (@David-Engel). This is not an approval and does not satisfy the human review requirement. Findings may be incomplete or wrong; push back on anything that looks off.

Re-reviewed at b7d8a989. The only new commit since the last round is b7d8a989, which adds the missing Stage musl driver (Linux ARM64) step — that closes blocking finding 1 from the e05eba7b review. Nothing else in the diff changed, so I am not re-arguing anything already raised; the list below is just what is still outstanding, plus one new supporting detail on the tag question.

Verification

• Confirmed the new step is the exact mirror of the x64 one and that the path it writes (drivers/linux/musl/arm64/lib/mssqlodbc.so) is byte-for-byte what the script expects: I imported scripts/inject-odbc-into-wheels.py and ran drivers_for_platform_tag over the twelve tags this matrix can produce — musllinux_1_2_aarch64 → linux/musl/arm64/lib/mssqlodbc.so, matching stages.yml:321-325. All eight staged paths in stages.yml now have a mapping, and every mapping has a stager.
• Re-read containerized-odbc-build.sh:29 and dockerentry/alpine-odbc-build.sh:30: both rm -rf only $DROP_DIR and mssql-odbc/tests/e2e/build, so the sequential glibc → musl passes cannot wipe the accumulating drivers/ tree in the same job. The staging order (glibc stage → alpine build → musl stage → inject) is correct on both Linux jobs.
• finalize-artifact.ps1 emits only the resolved path on stdout, so $driver = & ... in build-odbc-windows-template.yml:48 binds a single string. The macOS template's rm -rf odbc-drop on entry (build-odbc-macos-template.yml:50) makes the two per-arch passes independent.
• gh pr checks: nothing failing; several ADO legs still pending.

Blocking

1. Still open: glibc and musl wheels may share the same platform tag, which would make the musl mapping dead and the injected driver wrong. Unchanged from the e05eba7b round and, as far as I can tell, unanswered — and this PR's correctness rests entirely on the tag distinction, so it needs settling here rather than deferred.

One thing I did not say last time that makes this harder to notice than it looks: the musllinux leg's own success guard cannot detect it. build-python-wheels-template.yml:153 runs find "$(ob_outputDirectory)/wheels" -name '*.whl' | wc -l against the shared wheels directory that the manylinux leg already populated, so WHEEL_COUNT -eq 0 stays false even if the musl build produced nothing new or overwrote the glibc wheels one-for-one. The manylinux guard at line 66 has the same shape. So a collision would sail through both checks, and the first visible symptom would be an Alpine user getting a glibc .so at dlopen time.

Cheapest confirmation is still reading the ls -lh "$(ob_outputDirectory)/wheels" output at the end of the manylinux step versus the end of the musllinux step in the last non-PR Linux x64 run — if the count does not roughly double, they are colliding. If it does, an explicit --compatibility manylinux_2_34 / --compatibility musllinux_1_2 on the two maturin build invocations (or a separate --out per libc) fixes the filenames, the tags, and the mapping together.

Suggestions

Both carried forward unchanged from e05eba7b; restating in one line each rather than re-arguing.

1. Uninjected wheel copies survive on the Official path. build-python-wheels-template.yml:199 moves originals to unsigned-wheels and the cleanup at line 347 never removes it, so stale same-named wheels ride along in the Build artifact and can win the Publish-stage flattenFolders collect (stages.yml:431-437). Adding $unsignedDir to the line 347 Remove-Item is the fix.
2. The native driver is now invisible to both artifact consumers and BinSkim. With publishArtifacts: false everywhere and the collect steps gone, no raw mssqlodbc.dll/.so/.dylib or PDB is published, and scanOutputDirectoryOnly: true will not descend into the .whl zips. Worth an explicit decision and a comment even if the answer is "acceptable for now".

Nits

1. scripts/inject-odbc-into-wheels.py:150 — writestr(name, data) with a str name drops the source ZipInfo, so every rewritten entry comes out 0o600 with a fresh mtime. Benign under pip, but it costs byte-reproducibility.
2. stages.yml:251 / :324 — sudo chown -R $(whoami):$(whoami) only works because ADO leaves the unrecognized macro for bash; "$USER" sidesteps the collision. And drivers/ under $(Build.SourcesDirectory) (which is also CARGO_TARGET_DIR on Linux) would sit more naturally in $(Build.StagingDirectory).

Also still unanswered from the bot's first pass: the Windows ARM64 leg builds with whatever .pipeline/scripts/InstallRustup.ps1 installs, and that script unconditionally fetches the x86_64-pc-windows-msvc rustup-init with no ARM64 branch. Worth a reply either way — if the pre-installed toolchain on RUST-WINSRV-ARM makes it moot, saying so closes the thread.

Category	Count
Blocking	1
Suggestion	2
Nit	2

[ttk (Theekshna)]

Unattended run, independent pass at �7d8a989. This PR is under heavy active review (Copilot-assisted rounds posted on behalf of David-Engel) — I read all three prior rounds plus saurabh500's and did not re-verify what they already verified; I focused on what's still open and on ground nobody had covered yet.

Severity	Count
Blocking	0 (new)
Suggestion	1 (new)
Nit	0 (new)

Not re-filing, but independently corroborated as still real and unresolved on this head: Blocking finding 1 from the �05eba7b/�7d8a989 rounds — glibc and musl wheels may share the same linux_ platform tag. I checked this against maturin's own documented behavior rather than taking the claim on faith: with �uditwheel = "skip" (confirmed at mssql-py-core/pyproject.toml:22), maturin skips its libc-detection pass entirely and falls back to the generic linux tag regardless of the container's libc — it does not distinguish manylinux from musllinux the way it would with auditwheel enabled. scripts/build-python-wheels-in-container.sh:78 still has no --compatibility flag or per-libc --out directory, so nothing in this diff or its dependencies closes the gap. This remains genuinely Blocking and unanswered.

New Suggestion — no automated test coverage for the new injection logic. scripts/inject-odbc-into-wheels.py adds drivers_for_platform_tag, platform_tag_of, and _record_line (lines 55, 87, 96) — pure functions with no I/O, straightforward to unit test with pytest (already a dev dependency per mssql-py-core/pyproject.toml:25). Correctness here has so far been established only by ad hoc manual runs pasted into review comments (all twelve platform tags, RECORD hash verification, etc.) — real verification, but not CI-enforced, so a future edit to the tag-mapping table or the RECORD-regeneration logic has nothing to catch a regression before it silently ships a wrong driver into a wheel. A small ests/test_inject_odbc.py covering the tag→path table and one _record_line/RECORD round-trip would close this cheaply.

Other checks: msodbcsql parity — N/A, pipeline/build infra only, no ODBC protocol or driver-behavior code touched. Divergence docs — N/A, same reason. PR description currency — matches the diff; checklist boxes are unchecked without a note, but (as I found in my prior pass on this PR) that's not misleading for a non-Rust change. AI slop — none found; the script's docstrings and pipeline comments explain non-obvious why (why macOS doesn't lipo-fuse, why the libs/ tree roots where it does), not restatement.

[David Engel (David-Engel)]

Nice work — the layering is clean: each Build job now produces its own driver and injects it into the wheels it just built, which avoids the fan-in/fan-out artifact plumbing a Publish-stage injection would have needed. The publishArtifacts opt-out on the existing ODBC templates is a tidy way to reuse them without double-publishing, and the finalize-artifact.* reuse (instead of hardcoding cargo layout) is the right call.

I ran scripts/inject-odbc-into-wheels.py locally against a synthetic maturin-shaped wheel to check the tag mapping and the repack. Tag mapping is correct for every tag this matrix produces (linux_x86_64, linux_aarch64, musllinux_1_2_*, win_amd64, win_arm64, macosx_15_0_universal2), platform_tag_of handles the optional build-tag field, and re-running the script is idempotent. The repack does have one side effect worth fixing, below.

Suggestions

1. inject-odbc-into-wheels.py repack drops file modes from every entry in the wheel

zf.writestr(name, data) with a str arcname makes zipfile synthesize a fresh ZipInfo, and CPython hardcodes external_attr = 0o600 << 16 in that path. Because the script rewrites every entry that way — not just the ones it adds — the whole wheel is re-emitted at 0600 with fresh timestamps.

Reproduced locally on a wheel whose native module was stored at 0755:

mssql_py_core/mssql_py_core.abi3.so           mode=0o600  was=0o755
mssql_py_core/__init__.py                     mode=0o600  was=0o644
mssql_py_core/libs/.../mssqlodbc.so           mode=0o600  (new)

Practical impact is limited for pip install — pip's zip_item_is_executable() will now return False, so files are extracted at the default 0644 rather than 0755, and dlopen doesn't require the exec bit on Linux/macOS. But the wheel content no longer matches what maturin produced, and anything that consumes the archive directly (wheel unpack, container image builds, vendoring scripts, unzip as a different user) gets a 0600 file. Cheap to preserve:

with zipfile.ZipFile(wheel, "r") as zf:
    names = zf.namelist()
    infos = {i.filename: i for i in zf.infolist()}
    entries = {name: zf.read(name) for name in names}
...
with zipfile.ZipFile(tmp, "w", zipfile.ZIP_DEFLATED) as zf:
    for name, data in entries.items():
        info = infos.get(name)
        if info is None:
            info = zipfile.ZipInfo(name, date_time=(1980, 1, 1, 0, 0, 0))
            info.external_attr = 0o755 << 16  # injected shared library
        info.compress_type = zipfile.ZIP_DEFLATED
        zf.writestr(info, data)

2. Nothing validates that the injected Linux driver is loadable on the platform the wheel claims

Two independent sources of drift here:

• glibc floor. The glibc driver is built in ghcr.io/microsoft/mssql-rs/build/ubuntu:22.04 (glibc 2.35), but the wheels it is injected into come out of the manylinux_2_34_* images. The header comment says "2_34 only", so a 2.35-linked .so landing in a 2.34-baseline wheel is already half a notch off, and a future bump of the Ubuntu build image would widen it silently.
• System OpenSSL. mssql-odbc pulls reqwest/native-tls and mssql-tds takes a non-vendored openssl dependency, so the Linux .so carries NEEDED entries on the build image's libssl/libcrypto. The wheel bundles neither, and it has no way to declare them.

I'd add a cheap gate right after the "Stage … driver" steps so a toolchain or base-image change can't produce a wheel that only fails at dlopen time on a customer machine — e.g. dump the max GLIBC_ version and the NEEDED list and fail if they exceed the intended floor:

objdump -T "$DRIVER" | grep -o 'GLIBC_[0-9.]*' | sort -Vu | tail -1
readelf -d "$DRIVER" | grep NEEDED

(otool -L for the macOS equivalent.)

3. buildOdbcNative should be gated on buildPythonWheels

stages.yml is a shared template, and the injection blocks are conditioned only on buildOdbcNative. With buildOdbcNative: true, buildPythonWheels: false the injection step still runs and fails in a confusing way: $(ob_outputDirectory)/wheels never gets created, and on Windows bare python is only on PATH as a side effect of the UsePythonVersion@0 tasks inside build-python-wheels-template.yml. The NonOfficialPythonWheelsPublish caller hardcodes buildPythonWheels: true so it isn't reachable today, but the guard is one expression:

- ${{ if and(eq(parameters.buildOdbcNative, true), eq(parameters.buildPythonWheels, true)) }}:

Alternatively make the Windows step invoke a resolved interpreter rather than bare python.

Nits

$(whoami) collides with ADO macro syntax (stages.yml, both Linux injection steps). sudo chown -R $(whoami):$(whoami) … only works because no pipeline variable named whoami exists, so the agent leaves the text intact and bash performs the substitution. build-python-wheels-template.yml already does this, so it's consistent with the existing code — but sudo chown -R "$USER:$USER" says the same thing without depending on that coincidence.

macOS deployment target isn't pinned for the driver. The wheel build exports MACOSX_DEPLOYMENT_TARGET=15.0, but build-odbc-macos-template.yml doesn't, so the two binaries shipped in the same wheel can end up with different minimum-OS floors depending on the runner image. Worth exporting the same value in the driver build.

Compressed platform tag sets map to a single libc. drivers_for_platform_tag("musllinux_1_2_x86_64.manylinux_2_34_x86_64") returns only the musl driver (verified locally). The current matrix never emits a compressed tag set, so this is theoretical — but if one ever appears, the wheel silently ships the wrong libc build instead of failing. Splitting the tag on . and mapping each sub-tag (or rejecting compressed sets outright) keeps the failure loud.

No test coverage, and PR validation can't exercise any of this. Every new step carries ne(variables['Build.Reason'], 'PullRequest'), so the green checks on this PR only prove the YAML expands. drivers_for_platform_tag / platform_tag_of are pure functions and would be trivial to cover with a small pytest alongside the script. Before merging, I'd also do a manual buildOdbcNative: true run of NonOfficialPythonWheelsPublish and unzip one wheel per platform to confirm the mssql_py_core/libs/... layout matches what the mssql-python resolver expects — particularly the arm64 vs aarch64 spelling on Linux, which this PR standardizes on arm64 and which I can't verify from this repo.

Verified

• (?ms)^\[package\].*?^version\s*=\s*"([^"]+)" resolves to 0.1.10 against the current mssql-py-core/Cargo.toml — good fix over the unanchored version.
• ${TARGET_TRIPLE:+$TARGET_TRIPLE/} in finalize-artifact.sh preserves the existing native-build path exactly when the second argument is absent, so the build_e2e.sh caller is unaffected.
• The macOS template's rm -rf odbc-drop before staging correctly prevents the ARM64 pass from inheriting the x64 dylib; the Linux path gets the same protection for free via containerized-odbc-build.sh, which already cleans $DROP_DIR as root.
• Injection steps are ordered before each job's PublishPipelineArtifact@1, so the published drop contains the injected wheels.
• All four changed/added pipeline files parse as valid YAML.

[gargsaumya]

Pushed three commits (8da8b7f, 23f2b7f, d6f108f) and resolved the open findings from the review summaries. Evidence below for the two Blocking items — both empirically do not reproduce.

Blocking: glibc/musl wheels sharing a platform tag — not reproduced

Inspected the Linux wheels in the published injected mssql-py-core-wheels package. They do not collide:

bare linux_*  (glibc, auditwheel=skip):  10   linux_x86_64 / linux_aarch64
musllinux_1_2_*:                          10   musllinux_1_2_x86_64 / musllinux_1_2_aarch64
manylinux_*:                               0

Both …-cp312-cp312-linux_x86_64.whl and …-cp312-cp312-musllinux_1_2_x86_64.whl exist as separate files in the same package — if they shared a tag they'd share a filename and there would be one, not both. auditwheel = "skip" only suppresses the manylinux promotion/repair of the glibc wheel (leaving it bare linux_*); maturin still derives musllinux_1_2 for the musl wheel from the musl-linked container interpreter, no --compatibility flag required. So the musl/… mapping is live, and an Alpine host resolves the musllinux_1_2 wheel (pip prefers musl tags on musl) and gets the musl driver — never a glibc .so.

This is now locked in CI-checkable form by scripts/test_inject_odbc.py (d6f108f), including test_musl_and_glibc_do_not_collide and the full tag→path table.

Separately valid but out of scope here: the glibc wheels being bare linux_* (not manylinux_*) means they aren't PyPI-installable — that's the deferred PyPI phase (2_28 + 2_34 manylinux tagging), not this NuGet-consumed path.

Blocking: Windows ARM64 packaging an x64 DLL — not reproduced

Read the PE machine field of both binaries in the published win_arm64 wheel:

mssqlodbc.dll                       0xAA64  ARM64
mssql_py_core.cp311-win_arm64.pyd   0xAA64  ARM64

Both are native ARM64. The driver builds on the dedicated native RUST-WINSRV-ARM pool using the same no---target native pattern as the established win_arm64 wheel build (build-python-wheels-template.yml) — which is exactly why the .pyd in the same wheel is also ARM64. macOS uses --target only because it cross-builds both slices on one Intel host; Windows has per-arch native pools. No change needed.

Nit worth a follow-up: InstallRustup.ps1 unconditionally fetches the x86_64-pc-windows-msvc rustup-init — harmless in practice (the ARM64 pool still yields an ARM64 toolchain, per the above) but arch-blind; a host-aware branch would remove the emulation and the confusion.

Suggestion: un-injected wheel copies on the Official path — fixed

23f2b7f adds unsigned-wheels to the cleanup in the repack/verify step, so the pre-signing (un-injected, unsigned) copies no longer survive in the Build artifact where they could win the Publish collect.

Suggestion: no automated test coverage for the injection logic — added

d6f108f adds scripts/test_inject_odbc.py (21 tests, pure stdlib, no DB/extension): the tag→path table for all matrix tags, unknown-tag rejection, platform_tag_of, _record_line, and a full inject round-trip verifying the driver lands at the right arcname with a correct RECORD hash. Not yet wired into a CI leg — happy to add a pytest scripts/test_inject_odbc.py step if you'd prefer it gated rather than just present.

Suggestion: driver invisible to BinSkim

Acceptable as-is: the driver is ESRP malware-scanned and Authenticode-signed at its source in build-odbc-*-template.yml before injection, so nothing unscanned ships. It isn't separately BinSkim'd inside the .whl and there's no standalone driver/PDB artifact on this path — a deliberate trade-off for the single-wheel model; can add a raw-driver artifact later if PDB-level debugging needs it.

Also folded in: 8da8b7f enables buildOdbcNative on the Official build so the driver is injected (and ESRP-signed via isOfficial) on the official path too.

[David Engel (David-Engel)]

Automated review — generated by GitHub Copilot. This is not an approval and does not satisfy the human review requirement. Findings may be incomplete or wrong; push back on anything that looks off.

Reviewed at d6f108f4 against merge base 7b4cbc4b. I read the prior rounds (saurabh500, Theekshna, and the three Copilot-assisted rounds) before writing, and I am deliberately not re-filing anything they already raised.

I independently reproduced the two things worth reproducing and reached the same conclusions as the last round, so I'm recording them as verified rather than as new findings:

• drivers_for_platform_tag is correct for every tag this matrix actually emits (linux_x86_64, linux_aarch64, musllinux_1_2_*, win_amd64, win_arm64, macosx_15_0_universal2 — the macOS tag is pinned by the wheel tags --platform-tag macosx_15_0_universal2 retag step in build-python-wheels-template.yml). Injection is idempotent, and the regenerated RECORD is spec-shaped (hashed lines for every entry, hash-less line for RECORD itself).
• The repack drops file modes on every entry (0o755 → 0o600 on the maturin extension module). Already raised in the d6f108f4 round — not re-filing, just confirming it still reproduces on this head.
• Signing order is sound: ESRP signs the driver in build-odbc-windows-template.yml before injection, and the wheel-level signing pass only Authenticode-signs the binaries inside the wheel (no container signature to invalidate), so injecting after the repack does not break anything.
• Remove-Item ... $unsignedDir in build-python-wheels-template.yml is a real fix, not cosmetic: CopyFiles@2 in the Publish job uses flattenFolders: true with the default overWrite: false, so a duplicate un-injected .whl filename in the same artifact could have won the collect.

Suggestions

1. The new pytest suite never runs anywhere

scripts/test_inject_odbc.py is good coverage of exactly the right surface, but nothing collects it. Every pytest entry point in the repo is rooted elsewhere: mssql-py-core/pytest.ini + pytest tests/ ... from mssql-py-core (test-mssql-python-template.yml:95, test-longhaul-template.yml:127, test-mssql-python-macos-template.yml:116), and run-mssql-python-odbc-tests.sh drives the cross-repo mssql-python suite. There is no GitHub Actions workflow that runs pytest at the repo root either (rg pytest .github/workflows is empty), and the coverage report on this PR scopes to mssql-tds + mssql-odbc + mssql-py-core, so scripts/ isn't measured.

Net effect: the tag→driver table can regress and CI stays green. Worth one explicit step wherever Python is already installed, e.g. in the job that builds the wheels:

- script: python -m pytest scripts/test_inject_odbc.py -q
  displayName: 'Unit test ODBC wheel injection script'

That also makes the suite run on PR builds, which matters here because every pipeline step this PR adds carries ne(variables['Build.Reason'], 'PullRequest') — the green checks on this PR only prove the YAML expands, they exercise none of the new logic. A collected pytest is the one piece of this change PR validation can actually cover.

(This also closes the "no test coverage" item from the previous round, which predates d6f108f4.)

Required validation

mssql-rs Pull request validation is red on d6f108f4 — Build mssql-python Build mssql-python macOS (cross-repo) failed (build 172312). Every other leg passed. Worth confirming whether that job is flaky or whether the finalize-artifact.sh signature change (the only file in this PR that PR validation actually executes) is involved, before marking ready for review. The ${TARGET_TRIPLE:+$TARGET_TRIPLE/} expansion is a no-op when the second argument is absent, so the existing build_e2e.sh caller should be unaffected — but the failing job is the macOS one, so it's worth a look rather than an assumption.

Nits

None new. The PR description still has all five checklist boxes unchecked; worth ticking or striking them (the cargo ones are N/A for a pipeline-only change) before this goes to human review.

[David Engel (David-Engel)]

This suite is never collected by CI. Every pytest entry point in the repo is rooted at mssql-py-core (pytest.ini + pytest tests/ in test-mssql-python-template.yml, test-longhaul-template.yml, test-mssql-python-macos-template.yml) or drives the cross-repo mssql-python suite via run-mssql-python-odbc-tests.sh. No GitHub Actions workflow runs pytest at the repo root, and the coverage report scopes to mssql-tds + mssql-odbc + mssql-py-core, so scripts/ isn't measured either.

So the tag->driver map can regress with CI green. Add one step wherever Python is already on the box, e.g.:

- script: python -m pytest scripts/test_inject_odbc.py -q
  displayName: 'Unit test ODBC wheel injection script'

Worth putting it somewhere that runs on PR builds: every pipeline step this PR adds is gated on ne(variables['Build.Reason'], 'PullRequest'), so this suite is the only part of the change PR validation can actually exercise.

[Saurabh Singh (saurabh500)]

The cross-platform driver integration is directionally sound, but the Linux artifact currently violates the wheel's advertised compatibility: a driver built on Ubuntu 22.04/glibc 2.35 is injected into a manylinux_2_34 wheel. Please build the driver in the matching wheel environment before packaging it. I also recommend letting maturin include the staged driver rather than maintaining a custom wheel rewriter. The existing macOS cross-repository validation is currently failing and should be green on the final commit.

Replaced at reviewer request with a non-blocking comment review.

[Saurabh Singh (saurabh500)]

The cross-platform driver integration is directionally sound, but please consider aligning the Linux driver build environment with the wheel's advertised compatibility and using maturin's native include mechanism instead of a custom wheel rewriter. The existing macOS cross-repository validation is currently failing and should be green on the final commit.

[Saurabh Singh (saurabh500)]

This does not produce the glibc 2.34 binary described at the top of this file. build-odbc-template.yml defaults to ghcr.io/microsoft/mssql-rs/build/ubuntu:22.04, so the driver is built against Ubuntu's glibc 2.35 and then injected into wheels built in the manylinux_2_34 image. That makes the wheel's advertised compatibility weaker than its contents.

Please build the driver in the same manylinux_2_34_{x86_64,aarch64}_rust environment as its wheel (and similarly keep the musl build with the musllinux wheel) before packaging it. Also verify the resulting wheel's required GLIBC/OpenSSL symbol versions rather than only its filename tag.

[Saurabh Singh (saurabh500)]

Can we avoid maintaining a custom wheel rewriter here? Maturin supports wheel-only includes and generates RECORD itself. Build and stage the platform driver before maturin build, place it under mssql-py-core/mssql_py_core/libs/..., and configure:

[tool.maturin]
include = [
  { path = "mssql_py_core/libs/**/*", format = "wheel" },
]

Each lane should clean and stage only its platform payload before invoking its wheel build; the macOS lane can stage both thin dylibs before the universal2 build. This removes the post-build injection steps, this script, and its separate tests while preserving ZIP metadata and standard wheel generation behavior. Please retain an installed-wheel smoke test that resolves and loads the packaged driver through the real mssql-python path.

[Saurabh Singh (saurabh500)]

buildOdbcNative is independent of buildPythonWheels, but every ODBC branch later invokes the injector against $(ob_outputDirectory)/wheels. With buildOdbcNative: true and buildPythonWheels: false, the ODBC build succeeds and injection then fails because no wheel directory exists. Please reject that parameter combination explicitly or gate the ODBC packaging path on both parameters.