Ship mssql-odbc driver artifacts with product filenames - #397
Conversation
There was a problem hiding this comment.
Pull request overview
Renames the Rust ODBC driver artifact basename to mssqlodbc across builds, runtime reporting, tests, documentation, and CI.
Changes:
- Renames platform-specific shared libraries and linker metadata.
- Updates e2e and pipeline artifact paths.
- Updates
SQL_DRIVER_NAME, tracing filters, and documentation.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/dockerentry/odbc-e2e-rhel.sh |
Updates staged library documentation. |
scripts/dockerentry/odbc-e2e-deb.sh |
Updates staged library documentation. |
scripts/dockerentry/odbc-e2e-azlinux3.sh |
Updates staged library documentation. |
scripts/dockerentry/odbc-e2e-alpine.sh |
Updates staged library documentation. |
scripts/dockerentry/alpine-odbc-build.sh |
Updates build artifact documentation. |
mssql-odbc/tests/e2e/run_e2e.sh |
Resolves renamed Unix artifacts and tracing target. |
mssql-odbc/tests/e2e/run_e2e.ps1 |
Resolves the renamed Windows DLL. |
mssql-odbc/tests/e2e/README.md |
Documents renamed e2e artifacts. |
mssql-odbc/tests/e2e/build_e2e.sh |
Stages renamed Unix artifacts. |
mssql-odbc/src/api/get_info.rs |
Reports renamed platform driver files. |
mssql-odbc/src/api/exports.rs |
Updates shared-library documentation. |
mssql-odbc/README.md |
Documents new artifact names and metadata. |
mssql-odbc/plan.md |
Updates planned binary outputs. |
mssql-odbc/Cargo.toml |
Renames the library target. |
mssql-odbc/build.rs |
Renames SONAME and install name metadata. |
.pipeline/templates/test-mssql-python-odbc-template.yml |
Uses the renamed swap-test artifact. |
.pipeline/scripts/swap-mssql-python-odbc-driver.sh |
Updates replacement-driver guidance. |
.pipeline/scripts/containerized-odbc-swap-build.sh |
Stages the renamed swap artifact. |
.pipeline/scripts/containerized-odbc-build.sh |
Updates build artifact documentation. |
.pipeline/scripts/containerized-odbc-build-glibc228.sh |
Updates glibc build documentation. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
🔗 Quick Links |
David Engel (David-Engel)
left a comment
There was a problem hiding this comment.
Reviewed at 8df33793. I checked the PR out locally, traced every consumer of the old filenames (.pipeline, scripts/dockerentry, tests/e2e, build.rs, get_info.rs), and read the prior threads before writing this.
Summary
Renames the shipped ODBC driver to mssql-odbc.{dll,so,dylib} by keeping Cargo's internal lib target and adding finalize-artifact.{sh,ps1} that copy Cargo's output to the product name before staging or registration. SONAME, install name, SQL_DRIVER_NAME, and docs all follow. No stragglers left behind — the remaining msodbcsql18 references are all the reference driver, which is correct. ADO validation is green, including the cross-repo mssql-python swap suite that actually loads the renamed .so, which is the strongest signal here. Copilot's earlier finding about the vacuous driver_name_writes_wide_string test is genuinely fixed: the test now asserts against hardcoded literals, so reverting the rename breaks it.
No blocking findings. Four suggestions inline, plus notes below.
Notes (no action needed)
- The failing
coverage-reportGitHub check is not this PR. The same workflow failed or was cancelled on two other branches in the same window, and this PR's earlier run succeeded and posted 100% diff / 92.5% overall. cpleaves both filenames intarget/, socargo llvm-covpicks up the copy as an extra object file. That is already the steady state (Cargo hardlinks the cdylib into bothdeps/and the profile root) and the CI coverage numbers held, so no concern.- Dropping the
libprefix on Linux is unconventional but reads as deliberate: unixODBC loads by absolute path fromodbcinst.ini, and it removes the old SONAME collision with a genuinely loadedlibmsodbcsql-18.4.so.1.1.
One process nit: the cargo btest checklist box is unchecked on a non-draft PR. The Validation section shows a scoped cargo nextest run -p mssql-odbc, which is a reasonable scope for this diff — either run the workspace suite or note that ADO covers it, so the checklist stops disagreeing with itself.
|
|
||
| [lib] | ||
| name = "msodbcsql18" | ||
| name = "mssqlodbc" |
There was a problem hiding this comment.
The lib target name is also the tracing target, so this silently changes the log filter from msodbcsql18=debug to mssqlodbc=debug — and EnvFilter ignores unknown targets, so anyone still passing the old value just gets no driver logs rather than an error.
name = "mssql_odbc" is equally valid (only hyphens are rejected), gives the target mssql_odbc — matching the mssql_tds convention used in the docs one line over in the README — and the finalizers just copy libmssql_odbc.so -> mssql-odbc.so instead.
Either way, this env-var change is user-visible and isn't called out in the PR description. Worth a line there.
| if [ "$VERBOSE" -eq 1 ]; then | ||
| export MSSQL_TDS_TRACE="${MSSQL_TDS_TRACE:-true}" | ||
| export MSSQL_TDS_TRACE_LEVEL="${MSSQL_TDS_TRACE_LEVEL:-warn,msodbcsql18=debug}" | ||
| export MSSQL_TDS_TRACE_LEVEL="${MSSQL_TDS_TRACE_LEVEL:-warn,mssqlodbc=debug}" |
There was a problem hiding this comment.
I am wondering where mssqlodbc comes from? And why did this not have to change mssql-odbc
There was a problem hiding this comment.
I'm wondering if we should just get rid of the dash in mssql-odbc and make the "official" name (library, package, file, etc) "mssqlodbc" to avoid various infra/tool objections to dash or underscore in the name.
There was a problem hiding this comment.
Saurabh Singh (@saurabh500) mssqlodbc comes from Cargo’s private [lib] target name. Cargo target names cannot contain a hyphen, and Cargo derives the intermediate library filename and tracing target from that name. The finalization step then converts the Cargo artifact to the product name, so consumers and pipelines receive mssql-odbc.dll, mssql-odbc.so, or mssql-odbc.dylib.
There was a problem hiding this comment.
Cargo’s restriction only applies to its private target, the finalization step can still publish the hyphenated product filenames.
Saurabh Singh (@saurabh500) , David Engel (@David-Engel) Would you prefer the official name to remain mssql-odbc, or should we standardize the library, package, files, and tracing target on mssqlodbc?
I have kept the current naming (mssql-odbc) unchanged for now.
There was a problem hiding this comment.
I would like everything to be called mssql-odbc :)
If this causes scope creep for the current PR, then do a followup PR, that would be your discretion.
There was a problem hiding this comment.
Ahh I didn't read David Engel (@David-Engel)'s comment. You have a point.
There was a problem hiding this comment.
gargsaumya, Had a quick sync with David. It is going to be mssqlodbc
No dashes/underscore confusion.
Apologies for the to and fro. But Naming is a hard problem :)
There was a problem hiding this comment.
To clarify, this is a relatively "big" change for us, internally, but should avoid future confusion for everyone else. Top level name of "mssql-odbc" becomes just "mssqlodbc", which propagates to library/target/project/binary names. We can consider renaming the mssql-odbc project folder, too. Like I said, potentially disruptive for us but sets us up for a less confusing future.
FYI - ttk (@Theekshna), agree?
There was a problem hiding this comment.
FWIW +1 to uniformity, including the project (create) name.
Description
Ship the Rust ODBC driver with the exact product basename
mssql-odbcacross all supported platforms, architectures, build profiles, and Linux flavors.The shipped artifacts are:
mssql-odbc.dllmssql-odbc.somssql-odbc.dylibCargo retains the valid internal library target name
mssqlodbc. Platform finalization scripts copy Cargo's canonical output to the product filename before e2e registration or artifact publication. Linux SONAME, macOS install name, andSQL_DRIVER_NAMEreport the shipped filename.Microsoft
msodbcsql18package and DLL references used by parity tests remain unchanged.Related 问题
https://sqlclientdrivers.visualstudio.com/DefaultCollection/mssql-rs/_workitems/edit/47676
Validation
cargo build --release -p mssql-odbc --frozenmssql-odbc.dllartifactscargo nextest run -p mssql-odbc --frozen(873 passed, 1 skipped)mssql-odbc.soChecklist
cargo bfmtpassescargo bclippypassescargo btestpasses