diff --git a/.nextchanges/cli/setup-local-dbconnect-consolidate.md b/.nextchanges/cli/setup-local-dbconnect-consolidate.md new file mode 100644 index 00000000000..025970559c1 --- /dev/null +++ b/.nextchanges/cli/setup-local-dbconnect-consolidate.md @@ -0,0 +1 @@ +`databricks environments setup-local` now removes a `databricks-connect` pin from `[project].dependencies`, an optional-dependency extra, or a dependency group when its version range conflicts with the compute target's `databricks-connect` version, so `uv sync` no longer fails with an unsatisfiable resolution when a template ships a conflicting pin. A pin that co-resolves, carries no version, or is marker-gated is left untouched, and each removed pin is reported with the new `W_DBCONNECT_CONSOLIDATED` warning. Wildcard version pins such as `==15.1.*` are now also checked for conflicts with the environment's constraints. diff --git a/acceptance/localenv/constraints-only-existing/out.test.toml b/acceptance/localenv/constraints-only-existing/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/localenv/constraints-only-existing/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/localenv/constraints-only-existing/output.txt b/acceptance/localenv/constraints-only-existing/output.txt new file mode 100644 index 00000000000..b17f420e4e5 --- /dev/null +++ b/acceptance/localenv/constraints-only-existing/output.txt @@ -0,0 +1,59 @@ + +>>> [CLI] environments setup-local --serverless-version 4 --constraints-only --dry-run --output json +{ + "schemaVersion": 1, + "command": "environments setup-local", + "ok": true, + "mode": "constraints-only", + "dryRun": true, + "compute": { + "source": "serverless", + "serverlessVersion": "v4", + "envKey": "serverless/serverless-v4" + }, + "resolved": { + "pythonVersion": "3.12", + "artifactSource": "network" + }, + "greenfield": false, + "plan": { + "wouldWrite": "[TEST_TMP_DIR]/pyproject.toml", + "wouldBackup": "[TEST_TMP_DIR]/pyproject.toml.bak", + "wouldInstallPython": "3.12", + "diff": "--- pyproject.toml\n+++ pyproject.toml.new\n@@ -1,8 +1,15 @@\n [project]\n name = \"demo\"\n-requires-python = \"\u003e=3.10\"\n+requires-python = \"\u003e=3.12\"\n dependencies = [\"databricks-connect==15.1.*\"]\n \n [dependency-groups]\n dev = [\"databricks-connect~=16.0\"]\n docs = [\"databricks-connect==15.0.0\"]\n+\n+# managed by databricks environments setup-local — do not edit\n+[tool.uv]\n+constraint-dependencies = [\n+ \"pyarrow\u003c19\",\n+]\n+# end managed by databricks environments setup-local\n" + }, + "phases": [ + { + "phase": "preflight", + "status": "ok" + }, + { + "phase": "resolve", + "status": "ok" + }, + { + "phase": "fetch", + "status": "ok" + }, + { + "phase": "merge", + "status": "ok" + }, + { + "phase": "provision", + "status": "ok" + }, + { + "phase": "validate", + "status": "ok" + } + ], + "warnings": [ + { + "code": "W_REQUIRES_PYTHON_OVERRIDDEN", + "message": "requires-python \"\u003e=3.10\" is replaced by the environment's \"\u003e=3.12\"" + } + ], + "error": null, + "durationMs": [DURATION_MS] +} diff --git a/acceptance/localenv/constraints-only-existing/script b/acceptance/localenv/constraints-only-existing/script new file mode 100644 index 00000000000..0ce45c47589 --- /dev/null +++ b/acceptance/localenv/constraints-only-existing/script @@ -0,0 +1,16 @@ +# --constraints-only omits databricks-connect entirely: no managed pin is inserted and +# no stray is removed, so every databricks-connect requirement the user already had — in +# [project].dependencies, dev, and a sibling group — is left byte-for-byte untouched. Only +# requires-python and the [tool.uv] constraints are managed. The JSON diff shows this. +cat > pyproject.toml <<'PY' +[project] +name = "demo" +requires-python = ">=3.10" +dependencies = ["databricks-connect==15.1.*"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +docs = ["databricks-connect==15.0.0"] +PY + +trace $CLI environments setup-local --serverless-version 4 --constraints-only --dry-run --output json diff --git a/acceptance/localenv/constraints-only-existing/test.toml b/acceptance/localenv/constraints-only-existing/test.toml new file mode 100644 index 00000000000..902567bb130 --- /dev/null +++ b/acceptance/localenv/constraints-only-existing/test.toml @@ -0,0 +1,24 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +# The script writes pyproject.toml as the merge input; --dry-run leaves it unchanged. +Ignore = ["pyproject.toml"] + +[Env] +DATABRICKS_LOCALENV_CONSTRAINT_SOURCE_URL_TEST_OVERRIDE = "$DATABRICKS_HOST" + +[[Server]] +Pattern = "GET /serverless/serverless-v4/pyproject.toml" +Response.Body = ''' +[project] +requires-python = ">=3.12" + +[dependency-groups] +dev = ["databricks-connect~=17.2.0"] + +[tool.uv] +constraint-dependencies = ["pyarrow<19"] +''' + +[[Repls]] +Old = 'uv uv \S+(?: \([^)]+\))?' +New = 'uv [UV_VERSION]' diff --git a/acceptance/localenv/dbconnect-compatible-kept-check/out.test.toml b/acceptance/localenv/dbconnect-compatible-kept-check/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/localenv/dbconnect-compatible-kept-check/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/localenv/dbconnect-compatible-kept-check/output.txt b/acceptance/localenv/dbconnect-compatible-kept-check/output.txt new file mode 100644 index 00000000000..3f36af3bce1 --- /dev/null +++ b/acceptance/localenv/dbconnect-compatible-kept-check/output.txt @@ -0,0 +1,64 @@ + +>>> [CLI] environments setup-local --serverless-version 4 --dry-run --output json +{ + "schemaVersion": 1, + "command": "environments setup-local", + "ok": true, + "mode": "default", + "dryRun": true, + "compute": { + "source": "serverless", + "serverlessVersion": "v4", + "envKey": "serverless/serverless-v4" + }, + "resolved": { + "pythonVersion": "3.12", + "dbconnectVersion": "17.2.0", + "artifactSource": "network" + }, + "greenfield": false, + "plan": { + "wouldWrite": "[TEST_TMP_DIR]/pyproject.toml", + "wouldBackup": "[TEST_TMP_DIR]/pyproject.toml.bak", + "wouldInstallPython": "3.12", + "diff": "--- pyproject.toml\n+++ pyproject.toml.new\n@@ -1,6 +1,6 @@\n [project]\n name = \"demo\"\n-requires-python = \"\u003e=3.10\"\n+requires-python = \"\u003e=3.12\"\n dependencies = [\n \"databricks-connect\u003e=15\",\n \"databricks-connect ; python_version \u003c '3.13'\",\n@@ -10,5 +10,11 @@\n extra = [\"databricks-connect\"]\n \n [dependency-groups]\n-dev = [\"databricks-connect~=16.0\"]\n+dev = [\"databricks-connect~=17.2.0\"]\n test = [\"databricks-connect\u003e=15,\u003c20\"]\n+\n+# managed by databricks environments setup-local — do not edit\n+[tool.uv]\n+constraint-dependencies = [\n+]\n+# end managed by databricks environments setup-local\n" + }, + "phases": [ + { + "phase": "preflight", + "status": "ok" + }, + { + "phase": "resolve", + "status": "ok" + }, + { + "phase": "fetch", + "status": "ok" + }, + { + "phase": "merge", + "status": "ok" + }, + { + "phase": "provision", + "status": "ok" + }, + { + "phase": "validate", + "status": "ok" + } + ], + "warnings": [ + { + "code": "W_REQUIRES_PYTHON_OVERRIDDEN", + "message": "requires-python \"\u003e=3.10\" is replaced by the environment's \"\u003e=3.12\"" + }, + { + "code": "W_DBCONNECT_PIN_OVERRIDDEN", + "message": "databricks-connect \"databricks-connect~=16.0\" is replaced by the environment's \"databricks-connect~=17.2.0\"" + } + ], + "error": null, + "durationMs": [DURATION_MS] +} diff --git a/acceptance/localenv/dbconnect-compatible-kept-check/script b/acceptance/localenv/dbconnect-compatible-kept-check/script new file mode 100644 index 00000000000..0d4aa45fa95 --- /dev/null +++ b/acceptance/localenv/dbconnect-compatible-kept-check/script @@ -0,0 +1,24 @@ +# Only a databricks-connect pin that provably cannot co-resolve with the managed +# version (~=17.2.0) is removed. A pin that overlaps it (>=15), carries no version, or +# is marker-gated resolves fine, so it is left in place — the merge does not silently +# rewrite the user's declaration (including [project].dependencies wheel metadata) when +# nothing is broken. The dry-run diff shows only requires-python and the dev pin change, +# and no W_DBCONNECT_CONSOLIDATED warning is emitted. +cat > pyproject.toml <<'PY' +[project] +name = "demo" +requires-python = ">=3.10" +dependencies = [ + "databricks-connect>=15", + "databricks-connect ; python_version < '3.13'", +] + +[project.optional-dependencies] +extra = ["databricks-connect"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +test = ["databricks-connect>=15,<20"] +PY + +trace $CLI environments setup-local --serverless-version 4 --dry-run --output json diff --git a/acceptance/localenv/dbconnect-compatible-kept-check/test.toml b/acceptance/localenv/dbconnect-compatible-kept-check/test.toml new file mode 100644 index 00000000000..1a8bd79ac7f --- /dev/null +++ b/acceptance/localenv/dbconnect-compatible-kept-check/test.toml @@ -0,0 +1,24 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +# The script writes pyproject.toml as the merge input; --dry-run leaves it unchanged. +Ignore = ["pyproject.toml"] + +[Env] +DATABRICKS_LOCALENV_CONSTRAINT_SOURCE_URL_TEST_OVERRIDE = "$DATABRICKS_HOST" + +[[Server]] +Pattern = "GET /serverless/serverless-v4/pyproject.toml" +Response.Body = ''' +[project] +requires-python = ">=3.12" + +[dependency-groups] +dev = ["databricks-connect~=17.2.0"] + +[tool.uv] +constraint-dependencies = [] +''' + +[[Repls]] +Old = 'uv uv \S+(?: \([^)]+\))?' +New = 'uv [UV_VERSION]' diff --git a/acceptance/localenv/dbconnect-consolidate-check/out.test.toml b/acceptance/localenv/dbconnect-consolidate-check/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/localenv/dbconnect-consolidate-check/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/localenv/dbconnect-consolidate-check/output.txt b/acceptance/localenv/dbconnect-consolidate-check/output.txt new file mode 100644 index 00000000000..31cb766909f --- /dev/null +++ b/acceptance/localenv/dbconnect-consolidate-check/output.txt @@ -0,0 +1,76 @@ + +>>> [CLI] environments setup-local --serverless-version 4 --dry-run --output json +{ + "schemaVersion": 1, + "command": "environments setup-local", + "ok": true, + "mode": "default", + "dryRun": true, + "compute": { + "source": "serverless", + "serverlessVersion": "v4", + "envKey": "serverless/serverless-v4" + }, + "resolved": { + "pythonVersion": "3.12", + "dbconnectVersion": "17.2.0", + "artifactSource": "network" + }, + "greenfield": false, + "plan": { + "wouldWrite": "[TEST_TMP_DIR]/pyproject.toml", + "wouldBackup": "[TEST_TMP_DIR]/pyproject.toml.bak", + "wouldInstallPython": "3.12", + "diff": "--- pyproject.toml\n+++ pyproject.toml.new\n@@ -1,15 +1,20 @@\n [project]\n name = \"demo\"\n-requires-python = \"\u003e=3.10\"\n+requires-python = \"\u003e=3.12\"\n dependencies = [\n \"databricks-dlt\",\n- \"databricks-connect==15.1.*\",\n \"pytest\",\n ]\n \n [project.optional-dependencies]\n-extra = [\"databricks-connect==14.0.0\"]\n+extra = []\n \n [dependency-groups]\n-dev = [\"databricks-connect~=16.0\"]\n-docs = [\"databricks-connect==15.0.0\"]\n+dev = [\"databricks-connect~=17.2.0\"]\n+docs = []\n+\n+# managed by databricks environments setup-local — do not edit\n+[tool.uv]\n+constraint-dependencies = [\n+]\n+# end managed by databricks environments setup-local\n" + }, + "phases": [ + { + "phase": "preflight", + "status": "ok" + }, + { + "phase": "resolve", + "status": "ok" + }, + { + "phase": "fetch", + "status": "ok" + }, + { + "phase": "merge", + "status": "ok" + }, + { + "phase": "provision", + "status": "ok" + }, + { + "phase": "validate", + "status": "ok" + } + ], + "warnings": [ + { + "code": "W_REQUIRES_PYTHON_OVERRIDDEN", + "message": "requires-python \"\u003e=3.10\" is replaced by the environment's \"\u003e=3.12\"" + }, + { + "code": "W_DBCONNECT_PIN_OVERRIDDEN", + "message": "databricks-connect \"databricks-connect~=16.0\" is replaced by the environment's \"databricks-connect~=17.2.0\"" + }, + { + "code": "W_DBCONNECT_CONSOLIDATED", + "message": "databricks-connect \"databricks-connect==15.1.*\" in [project].dependencies conflicts with the environment's \"databricks-connect~=17.2.0\" and is removed; it is managed in \"dev\"" + }, + { + "code": "W_DBCONNECT_CONSOLIDATED", + "message": "databricks-connect \"databricks-connect==14.0.0\" in [project.optional-dependencies].extra conflicts with the environment's \"databricks-connect~=17.2.0\" and is removed; it is managed in \"dev\"" + }, + { + "code": "W_DBCONNECT_CONSOLIDATED", + "message": "databricks-connect \"databricks-connect==15.0.0\" in [dependency-groups].docs conflicts with the environment's \"databricks-connect~=17.2.0\" and is removed; it is managed in \"dev\"" + } + ], + "error": null, + "durationMs": [DURATION_MS] +} diff --git a/acceptance/localenv/dbconnect-consolidate-check/script b/acceptance/localenv/dbconnect-consolidate-check/script new file mode 100644 index 00000000000..b809942455c --- /dev/null +++ b/acceptance/localenv/dbconnect-consolidate-check/script @@ -0,0 +1,24 @@ +# Every stray databricks-connect pin here is disjoint from the managed version, so it +# would make uv unsatisfiable and is removed — from [project].dependencies, an +# optional-dependency extra, and another dependency group — leaving the single managed +# pin in [dependency-groups].dev. The JSON diff shows each stray removed, with one +# W_DBCONNECT_CONSOLIDATED warning per removed pin (in document order). +cat > pyproject.toml <<'PY' +[project] +name = "demo" +requires-python = ">=3.10" +dependencies = [ + "databricks-dlt", + "databricks-connect==15.1.*", + "pytest", +] + +[project.optional-dependencies] +extra = ["databricks-connect==14.0.0"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +docs = ["databricks-connect==15.0.0"] +PY + +trace $CLI environments setup-local --serverless-version 4 --dry-run --output json diff --git a/acceptance/localenv/dbconnect-consolidate-check/test.toml b/acceptance/localenv/dbconnect-consolidate-check/test.toml new file mode 100644 index 00000000000..1a8bd79ac7f --- /dev/null +++ b/acceptance/localenv/dbconnect-consolidate-check/test.toml @@ -0,0 +1,24 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +# The script writes pyproject.toml as the merge input; --dry-run leaves it unchanged. +Ignore = ["pyproject.toml"] + +[Env] +DATABRICKS_LOCALENV_CONSTRAINT_SOURCE_URL_TEST_OVERRIDE = "$DATABRICKS_HOST" + +[[Server]] +Pattern = "GET /serverless/serverless-v4/pyproject.toml" +Response.Body = ''' +[project] +requires-python = ">=3.12" + +[dependency-groups] +dev = ["databricks-connect~=17.2.0"] + +[tool.uv] +constraint-dependencies = [] +''' + +[[Repls]] +Old = 'uv uv \S+(?: \([^)]+\))?' +New = 'uv [UV_VERSION]' diff --git a/acceptance/localenv/merge-warnings-json/output.txt b/acceptance/localenv/merge-warnings-json/output.txt index 2f614e198e6..6005d4f186a 100644 --- a/acceptance/localenv/merge-warnings-json/output.txt +++ b/acceptance/localenv/merge-warnings-json/output.txt @@ -21,7 +21,7 @@ "wouldWrite": "[TEST_TMP_DIR]/pyproject.toml", "wouldBackup": "[TEST_TMP_DIR]/pyproject.toml.bak", "wouldInstallPython": "3.12", - "diff": "--- pyproject.toml\n+++ pyproject.toml.new\n@@ -1,9 +1,17 @@\n [project]\n name = \"demo\"\n-requires-python = \"\u003e=3.10\"\n+requires-python = \"\u003e=3.12\"\n dependencies = [\"pyarrow==21.0.0\"]\n \n [dependency-groups]\n-dev = [\"databricks-connect~=16.0.0\", {include-group = \"spark\"}]\n+dev = [\"databricks-connect~=17.2.0\", {include-group = \"spark\"}]\n spark = [\"databricks-connect==15.0.0\"]\n qa = [\"pandas==4.0.0\"]\n+\n+# managed by databricks environments setup-local — do not edit\n+[tool.uv]\n+constraint-dependencies = [\n+ \"pyarrow\u003c19\",\n+ \"pandas\u003c3\",\n+]\n+# end managed by databricks environments setup-local\n" + "diff": "--- pyproject.toml\n+++ pyproject.toml.new\n@@ -1,9 +1,17 @@\n [project]\n name = \"demo\"\n-requires-python = \"\u003e=3.10\"\n+requires-python = \"\u003e=3.12\"\n dependencies = [\"pyarrow==21.0.0\"]\n \n [dependency-groups]\n-dev = [\"databricks-connect~=16.0.0\", {include-group = \"spark\"}]\n-spark = [\"databricks-connect==15.0.0\"]\n+dev = [\"databricks-connect~=17.2.0\", {include-group = \"spark\"}]\n+spark = []\n qa = [\"pandas==4.0.0\"]\n+\n+# managed by databricks environments setup-local — do not edit\n+[tool.uv]\n+constraint-dependencies = [\n+ \"pyarrow\u003c19\",\n+ \"pandas\u003c3\",\n+]\n+# end managed by databricks environments setup-local\n" }, "phases": [ { @@ -59,8 +59,8 @@ "message": "databricks-connect \"databricks-connect~=16.0.0\" is replaced by the environment's \"databricks-connect~=17.2.0\"" }, { - "code": "W_DBCONNECT_PIN_DUPLICATED", - "message": "databricks-connect \"databricks-connect==15.0.0\" is not rewritten by the merge; the environment's \"databricks-connect~=17.2.0\" sits in \"dev\" alongside it, and no version satisfies both" + "code": "W_DBCONNECT_CONSOLIDATED", + "message": "databricks-connect \"databricks-connect==15.0.0\" in [dependency-groups].spark conflicts with the environment's \"databricks-connect~=17.2.0\" and is removed; it is managed in \"dev\"" }, { "code": "W_USER_CONSTRAINT_CONFLICT", diff --git a/acceptance/localenv/merge-warnings/output.txt b/acceptance/localenv/merge-warnings/output.txt index 89bf2437070..d6ddf728c83 100644 --- a/acceptance/localenv/merge-warnings/output.txt +++ b/acceptance/localenv/merge-warnings/output.txt @@ -2,7 +2,7 @@ >>> [CLI] environments setup-local --serverless-version 4 --dry-run warning: requires-python ">=3.10" is replaced by the environment's ">=3.12" warning: databricks-connect "databricks-connect~=16.0.0" is replaced by the environment's "databricks-connect~=17.2.0" -warning: databricks-connect "databricks-connect==15.0.0" is not rewritten by the merge; the environment's "databricks-connect~=17.2.0" sits in "dev" alongside it, and no version satisfies both +warning: databricks-connect "databricks-connect==15.0.0" in [dependency-groups].spark conflicts with the environment's "databricks-connect~=17.2.0" and is removed; it is managed in "dev" warning: dependency "pyarrow==21.0.0" conflicts with the environment constraint "pyarrow<19" warning: dependency "pandas==4.0.0" conflicts with the environment constraint "pandas<3" Plan: [TEST_TMP_DIR]/pyproject.toml diff --git a/acceptance/localenv/wildcard-constraint-conflict-check/out.test.toml b/acceptance/localenv/wildcard-constraint-conflict-check/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/localenv/wildcard-constraint-conflict-check/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/localenv/wildcard-constraint-conflict-check/output.txt b/acceptance/localenv/wildcard-constraint-conflict-check/output.txt new file mode 100644 index 00000000000..a45244f373c --- /dev/null +++ b/acceptance/localenv/wildcard-constraint-conflict-check/output.txt @@ -0,0 +1,8 @@ + +>>> [CLI] environments setup-local --serverless-version 4 --dry-run +warning: requires-python ">=3.10" is replaced by the environment's ">=3.12" +warning: dependency "pyarrow==18.*" conflicts with the environment constraint "pyarrow~=19.0" +Plan: [TEST_TMP_DIR]/pyproject.toml + changed region: requires-python + changed region: tool.uv.constraint-dependencies +Check complete. No files were modified. diff --git a/acceptance/localenv/wildcard-constraint-conflict-check/script b/acceptance/localenv/wildcard-constraint-conflict-check/script new file mode 100644 index 00000000000..d4c6ee6b1b2 --- /dev/null +++ b/acceptance/localenv/wildcard-constraint-conflict-check/script @@ -0,0 +1,14 @@ +# A PEP 440 prefix-match wildcard pin ("pyarrow==18.*") is modeled as the version range +# it admits, so a conflict with the environment's constraint-dependencies is reported +# rather than silently missed — the 18.* series lies entirely below the env's ~=19.0. +cat > pyproject.toml <<'PY' +[project] +name = "demo" +requires-python = ">=3.10" +dependencies = ["pyarrow==18.*"] + +[dependency-groups] +dev = ["databricks-connect~=17.2.0"] +PY + +trace $CLI environments setup-local --serverless-version 4 --dry-run diff --git a/acceptance/localenv/wildcard-constraint-conflict-check/test.toml b/acceptance/localenv/wildcard-constraint-conflict-check/test.toml new file mode 100644 index 00000000000..55086a71e61 --- /dev/null +++ b/acceptance/localenv/wildcard-constraint-conflict-check/test.toml @@ -0,0 +1,24 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +# The script writes pyproject.toml as the merge input; --dry-run leaves it unchanged. +Ignore = ["pyproject.toml"] + +[Env] +DATABRICKS_LOCALENV_CONSTRAINT_SOURCE_URL_TEST_OVERRIDE = "$DATABRICKS_HOST" + +[[Server]] +Pattern = "GET /serverless/serverless-v4/pyproject.toml" +Response.Body = ''' +[project] +requires-python = ">=3.12" + +[dependency-groups] +dev = ["databricks-connect~=17.2.0"] + +[tool.uv] +constraint-dependencies = ["pyarrow~=19.0"] +''' + +[[Repls]] +Old = 'uv uv \S+(?: \([^)]+\))?' +New = 'uv [UV_VERSION]' diff --git a/libs/localenv/merge.go b/libs/localenv/merge.go index b48b60535ae..795492ba876 100644 --- a/libs/localenv/merge.go +++ b/libs/localenv/merge.go @@ -1,6 +1,7 @@ package localenv import ( + "cmp" "errors" "fmt" "regexp" @@ -45,28 +46,46 @@ var ( requiresPythonRe = regexp.MustCompile(`^(\s*)requires-python\s*=`) ) -// replacedDBConnectPin returns the databricks-connect requirement that merging -// target would rewrite in place, or "" when the merge would instead insert the -// managed pin (leaving anything already there untouched). -// -// detectMergeWarnings uses this rather than re-deciding which pins the merge -// recognizes: the merge matches double-quoted elements within the array devKeyRe -// finds, and any second implementation of that rule drifts from it. A detector that -// disagrees reports an override for a pin the merge never touched, which is a false -// claim about the user's file rather than a missed warning. -func replacedDBConnectPin(target []byte, c Constraints) string { +// removedDBConnect names a databricks-connect requirement the consolidation pass +// deletes and the array it came from. The location is a human-readable label +// ("[project].dependencies", "[dependency-groups].test") used in the advisory +// message and to subtract the pin from the warning detector's resolution view. +type removedDBConnect struct { + location string + pin string +} + +// dbconnectPlan is what the merge does to databricks-connect, computed by running +// the merge on a clone so the warning detector never re-derives the merge's rules. +// replacedDevPin is the dev-group pin mergeDatabricksConnect rewrites in place ("" if +// it inserts the managed pin instead); removed is every stray pin the consolidation +// pass deletes. +type dbconnectPlan struct { + replacedDevPin string + removed []removedDBConnect +} + +// planDBConnect returns the databricks-connect edits merging target would make, or +// the zero plan in constraints-only mode (empty pin) where databricks-connect is left +// untouched. It mirrors MergeManaged's preprocessing (CRLF normalization, multi-line +// string bail) and runs both databricks-connect passes on a clone, in the same order +// as MergeManaged, so replacedDevPin and removed match what the real merge does. +func planDBConnect(target []byte, c Constraints) dbconnectPlan { if c.DatabricksConnect == "" { - return "" + return dbconnectPlan{} } // Mirror MergeManaged's own preprocessing so the same lines are inspected. lines := strings.Split(strings.ReplaceAll(string(target), "\r\n", "\n"), "\n") if containsMultilineString(lines) { - return "" + return dbconnectPlan{} } // mergeDatabricksConnect rewrites element lines in place, so hand it a copy: this - // probe must not disturb the caller's view of the pre-merge file. - _, replaced, _ := mergeDatabricksConnect(slices.Clone(lines), c.DatabricksConnect) - return replaced + // probe must not disturb the caller's view of the pre-merge file. The consolidation + // pass then runs on its output, so removed reflects the post-dev-merge state where + // the managed pin is already the dev group's first databricks-connect element. + merged, replaced, _ := mergeDatabricksConnect(slices.Clone(lines), c.DatabricksConnect) + _, removed, _ := removeStrayDatabricksConnect(merged, c.DatabricksConnect) + return dbconnectPlan{replacedDevPin: replaced, removed: removed} } // MergeManaged applies the three managed transforms to target, preserving every other @@ -113,7 +132,15 @@ func MergeManaged(target []byte, c Constraints) (merged []byte, regions []string } lines, _, dbcChanged := mergeDatabricksConnect(lines, c.DatabricksConnect) - if dbcChanged { + // In the install flow, after the managed pin lands in the dev group, remove any + // databricks-connect pin elsewhere that is disjoint from it — the pins that would + // otherwise make uv unsatisfiable. Compatible pins are left alone. Skipped in + // constraints-only mode (empty pin), where databricks-connect is left untouched. + strayChanged := false + if c.DatabricksConnect != "" { + lines, _, strayChanged = removeStrayDatabricksConnect(lines, c.DatabricksConnect) + } + if dbcChanged || strayChanged { regions = append(regions, regionDatabricksConnect) } @@ -397,6 +424,315 @@ func replaceDbconnectElement(code, elem string) (out, replaced string, ok bool) return code, "", false } +// arrayKeyRe matches a "key = [" array assignment, capturing the key. The "[" must +// be on the assignment line (TOML array syntax), so an inline table value ("= {") +// or a scalar is not matched. "." is allowed in the key so a dotted assignment +// (optional-dependencies.extra = [...] inside [project]) is captured whole — as the +// key "optional-dependencies.extra", which is intentionally not equal to "dependencies" +// and so is skipped, rather than mistaken for the [project].dependencies array. +var arrayKeyRe = regexp.MustCompile(`^\s*([A-Za-z0-9._-]+)\s*=\s*\[`) + +// arraySpan locates one "key = [ ... ]" array assignment: the key and the line +// indices its value opens and closes on. +type arraySpan struct { + key string + start int + last int +} + +// arrayAssignmentsIn returns each top-level "key = [ ... ]" array assignment in the +// table body spanning (header, end), in file order. Continuation lines of a +// multi-line array are skipped so a requirement string that happens to look like an +// assignment is not mistaken for one. +func arrayAssignmentsIn(lines []string, header, end int) []arraySpan { + var out []arraySpan + for i := header + 1; i < end; i++ { + m := arrayKeyRe.FindStringSubmatch(lines[i]) + if m == nil { + continue + } + last, _ := arrayLineSpan(lines, i, end) + out = append(out, arraySpan{key: m[1], start: i, last: last}) + i = last + } + return out +} + +// removeStrayDatabricksConnect deletes the databricks-connect requirements that +// provably cannot co-resolve with the managed env pin (envPin) — from +// [project].dependencies, every [project.optional-dependencies] extra, and every +// [dependency-groups] group. It reports each deleted pin with its location. +// +// Only a *disjoint* pin is removed (see dbconnectPinConflicts). That is the pin that +// actually makes uv unsatisfiable — the bug this exists to fix — and it is the whole +// justification for reaching into locations the merge otherwise leaves alone, +// including [project].dependencies, which the default template compiles into the +// built wheel's metadata: deleting a pin there is only warranted when the project +// would not resolve or build as-is. A pin that co-resolves (databricks-connect>=15 +// against ~=17.2.0), carries no version, is marker-gated, or already equals envPin is +// left untouched — there is nothing to fix, so the user's declaration stands. This +// deliberately overrides, for disjoint pins only, the "sibling groups are user-owned +// and left untouched" contract that scopes mergeDatabricksConnect (see devKeyRe). +// +// The removal is line-based and matches the same shapes the rewrite does: a +// double-quoted element under a bare-key array in the [project], [project.optional- +// dependencies], or [dependency-groups] tables. Rarer-but-valid spellings are out of +// its reach and left in place — a single-quoted pin, a pin under a quoted TOML key +// ("qa group" = [...]), and a pin in an inline-table or dotted sub-table form. These +// are not silent: detectMergeWarnings decodes the file fully, so a disjoint survivor +// is still surfaced as W_DBCONNECT_PIN_DUPLICATED. +func removeStrayDatabricksConnect(lines []string, envPin string) (out []string, removed []removedDBConnect, changed bool) { + type target struct { + span arraySpan + location string + } + var targets []target + + if h, e, ok := tableBounds(lines, "[project]"); ok { + for _, a := range arrayAssignmentsIn(lines, h, e) { + // Only [project].dependencies is a resolution requirement; other [project] + // arrays (keywords, classifiers, ...) never hold databricks-connect. + if a.key == "dependencies" { + targets = append(targets, target{a, "[project].dependencies"}) + } + } + } + if h, e, ok := tableBounds(lines, "[project.optional-dependencies]"); ok { + for _, a := range arrayAssignmentsIn(lines, h, e) { + targets = append(targets, target{a, "[project.optional-dependencies]." + a.key}) + } + } + if h, e, ok := tableBounds(lines, "[dependency-groups]"); ok { + for _, a := range arrayAssignmentsIn(lines, h, e) { + targets = append(targets, target{a, "[dependency-groups]." + a.key}) + } + } + if len(targets) == 0 { + return lines, nil, false + } + + // Rewrite spans in a single ascending pass so a line-count change from one edit + // does not shift the indices of the others. Spans in different tables never + // overlap, and arrayAssignmentsIn already returns each table's spans in order. + slices.SortFunc(targets, func(a, b target) int { return cmp.Compare(a.span.start, b.span.start) }) + + out = make([]string, 0, len(lines)) + prev := 0 + for _, t := range targets { + out = append(out, lines[prev:t.span.start]...) + newSpan, pins := removeDbconnectFromArraySpan(lines[t.span.start:t.span.last+1], envPin) + out = append(out, newSpan...) + for _, pin := range pins { + removed = append(removed, removedDBConnect{location: t.location, pin: pin}) + } + prev = t.span.last + 1 + } + out = append(out, lines[prev:]...) + + // out is fully built above; it is only returned when something was removed, so a + // no-op run returns the original slice unchanged. + if len(removed) == 0 { + return lines, nil, false + } + return out, removed, true +} + +// dbconnectPinConflicts reports whether a databricks-connect pin provably cannot +// co-resolve with the managed env pin — their version ranges are disjoint. A pin the +// range model cannot compare (no version, a marker-gated requirement, an unparseable +// or wildcard-only spelling) is not provably conflicting, so it is treated as +// compatible and left in place. envPin, written by the merge, always parses. +func dbconnectPinConflicts(pin, envPin string) bool { + _, pinSpec, pinOK := splitDepSpec(pin) + _, envSpec, envOK := splitDepSpec(envPin) + return pinOK && envOK && rangesDisjoint(pinSpec, envSpec) +} + +// removeDbconnectFromArraySpan removes the databricks-connect elements that conflict +// with envPin (see dbconnectPinConflicts) from a single array value spanning spanLines +// (single- or multi-line), returning the rewritten lines and the removed pins. A +// compatible databricks-connect element — including the managed dev pin, which equals +// envPin and so never conflicts with itself — is left in place. It operates on +// top-level array elements so a version range comma ("databricks-connect>=15,<16") or +// an inline table is not split, and rejoining survivors preserves each element's own +// leading whitespace, newline, and trailing comma. +func removeDbconnectFromArraySpan(spanLines []string, envPin string) (out, removed []string) { + block := strings.Join(spanLines, "\n") + prefix, body, suffix, ok := arrayParts(block) + if !ok { + return spanLines, nil + } + var kept []string + // carry holds the leading comment/blank lines of a removed element. splitTopLevelElements + // breaks on commas, so a trailing comment left on the *previous* element's line lands as + // a leading comment line on this element's token; dropping the whole token would delete + // that previous element's comment (MergeManaged preserves comments). Carry those lines to + // the next retained token so the comment stays on the line it belonged to. A comment that + // instead described the removed element is indistinguishable from that case, so it is kept + // too and may end up beside the following element — content is never lost, but a comment + // can be relocated; that is the accepted trade-off of the split-on-commas approach. + carry := "" + for _, elem := range splitTopLevelElements(body) { + elem = carry + elem + carry = "" + if pin, isDBC := dbconnectElementPin(elem); isDBC && dbconnectPinConflicts(pin, envPin) { + removed = append(removed, pin) + carry = leadingLinesBeforeElement(elem) + continue + } + kept = append(kept, elem) + } + if len(removed) == 0 { + return spanLines, nil + } + // A carry left over after the last token (the removed element was last) has no following + // token; emit it on its own line before the closing "]" so the comment survives and does + // not comment out the bracket. + if strings.TrimSpace(carry) != "" { + kept = append(kept, carry+"\n") + } + // Removing the last element of a multi-line array would pull the closing "]" up onto + // the previous element's line (that element's token carried no trailing newline) and + // drop the array's trailing comma. When the array was multi-line (body ends in a + // newline) and a real element still remains, restore the trailing comma and the + // newline before "]" so the bracket keeps its own line and the magic trailing comma + // survives. + joined := strings.Join(kept, ",") + if strings.HasSuffix(body, "\n") && len(kept) > 0 && !strings.HasSuffix(joined, "\n") { + if !strings.HasSuffix(joined, ",") { + joined += "," + } + joined += "\n" + } + return strings.Split(prefix+joined+suffix, "\n"), removed +} + +// leadingLinesBeforeElement returns the comment/blank lines that precede the value line +// in an array-element token — the trailing comment splitTopLevelElements moved here from +// the previous element's line. The value line and anything after it (the element's own +// trailing comment) is excluded, since that belongs to the element being removed. +func leadingLinesBeforeElement(elem string) string { + lines := strings.Split(elem, "\n") + for i, line := range lines { + if c := commentStart(line); c >= 0 { + line = line[:c] + } + if strings.TrimSpace(line) != "" { + return strings.Join(lines[:i], "\n") + } + } + return "" +} + +// arrayParts splits a joined "key = [ ... ]" block into the text up to and including +// the opening "[", the body between the brackets, and the text from the matching "]" +// onward. It scans across newlines, tracking quoted strings and per-line "#" comments +// so a bracket inside either is ignored. ok is false when no balanced array is found. +func arrayParts(block string) (prefix, body, suffix string, ok bool) { + depth := 0 + var quote byte + inComment := false + openIdx := -1 + for i := 0; i < len(block); i++ { + c := block[i] + switch { + case c == '\n': + inComment = false + case inComment: + // skip until newline + case quote != 0: + if c == '\\' && quote == '"' { + i++ + } else if c == quote { + quote = 0 + } + case c == '"' || c == '\'': + quote = c + case c == '#': + inComment = true + case c == '[': + depth++ + if depth == 1 { + openIdx = i + } + case c == ']': + depth-- + if depth == 0 && openIdx != -1 { + return block[:openIdx+1], block[openIdx+1 : i], block[i:], true + } + } + } + return "", "", "", false +} + +// splitTopLevelElements splits an array body into its element tokens, breaking only +// on commas outside quotes, outside nested []/{} , and outside "#" comments. Each +// token keeps its surrounding whitespace, newline, and any comment, so survivors +// reassemble byte-for-byte. The final segment (the whitespace before "]") is kept so +// a trailing comma is preserved on rejoin. +func splitTopLevelElements(body string) []string { + var elems []string + depth := 0 + var quote byte + inComment := false + start := 0 + for i := 0; i < len(body); i++ { + c := body[i] + switch { + case c == '\n': + inComment = false + case inComment: + // skip until newline + case quote != 0: + if c == '\\' && quote == '"' { + i++ + } else if c == quote { + quote = 0 + } + case c == '"' || c == '\'': + quote = c + case c == '#': + inComment = true + case c == '[' || c == '{': + depth++ + case c == ']' || c == '}': + depth-- + case c == ',' && depth == 0: + elems = append(elems, body[start:i]) + start = i + 1 + } + } + return append(elems, body[start:]) +} + +// dbconnectElementPin returns the databricks-connect requirement an array element +// token pins, if the token is a double-quoted string naming databricks-connect. +// Single-quoted and non-string tokens (e.g. a PEP 735 include-group table) are not +// matched, mirroring replaceDbconnectElement's double-quoted-only rule. +// +// The token is scanned line by line rather than as one string: splitTopLevelElements +// breaks only on commas, so the token for an element carries any inline comment left +// on the *previous* element's line as a leading comment line. Stripping the comment +// with commentStart over the whole token would treat that leading "#" as running to +// end-of-token and hide the quoted requirement on the next line, silently skipping a +// stray pin. commentStart is therefore applied per line, where it correctly stops at +// the line's own end. +func dbconnectElementPin(elem string) (pin string, ok bool) { + for line := range strings.SplitSeq(elem, "\n") { + if c := commentStart(line); c >= 0 { + line = line[:c] + } + line = strings.TrimSpace(line) + if len(line) >= 2 && line[0] == '"' && line[len(line)-1] == '"' { + inner := line[1 : len(line)-1] + if isDatabricksConnectDep(inner) { + return inner, true + } + } + } + return "", false +} + // ensureTrailingComma appends a "," after the last non-space code character of // line when it lacks one, preserving any trailing whitespace and inline comment. // A blank or comment-only line is returned unchanged. diff --git a/libs/localenv/merge_test.go b/libs/localenv/merge_test.go index a2ad23babbb..9e3ffbdb8f5 100644 --- a/libs/localenv/merge_test.go +++ b/libs/localenv/merge_test.go @@ -507,9 +507,11 @@ func TestBracketDepthDeltaIgnoresStringsAndComments(t *testing.T) { } } -func TestMergeDatabricksConnectOnlyTouchesDevGroup(t *testing.T) { - // A databricks-connect pin in a sibling group (docs) must be left alone; only - // the dev group's entry is managed. +func TestMergeConsolidatesDatabricksConnectFromSiblingGroup(t *testing.T) { + // databricks-connect is fully owned by setup-local in the install flow: a stray pin + // in a sibling group (docs) makes uv unsatisfiable, so it is removed and the managed + // pin lives only in dev. This deliberately overrides the "sibling groups untouched" + // rule that scopes mergeDatabricksConnect, but only for databricks-connect. in := []byte(`[project] requires-python = ">=3.10" @@ -522,12 +524,259 @@ dev = [ out, _, err := MergeManaged(in, testConstraints()) require.NoError(t, err) s := string(out) - // docs untouched; dev updated to the managed pin. - assert.Contains(t, s, `docs = ["databricks-connect~=14.3"]`) + // docs consolidated (its db-connect removed); dev updated to the managed pin. + assert.Contains(t, s, `docs = []`) + assert.NotContains(t, s, `databricks-connect~=14.3`) assert.Contains(t, s, `"databricks-connect~=17.2.0",`) requireValidTOML(t, out) } +func TestMergeConsolidatesDatabricksConnectFromProjectDeps(t *testing.T) { + // The reported bug: a template ships databricks-connect in [project].dependencies + // while the dev group carries a different pin, so uv cannot co-resolve them. The + // merge removes the [project].dependencies pin and manages db-connect only in dev. + in := []byte(`[project] +requires-python = ">=3.10" +dependencies = [ + "databricks-dlt", + "pytest", + "databricks-connect==15.1.*", +] + +[dependency-groups] +dev = [ + "databricks-connect~=16.0", +] +`) + out, regions, err := MergeManaged(in, testConstraints()) + require.NoError(t, err) + s := string(out) + assert.NotContains(t, s, "databricks-connect==15.1.*", "the stray project.dependencies pin is removed") + assert.Contains(t, s, `"databricks-dlt",`, "sibling dependencies are preserved") + assert.Contains(t, s, `"pytest",`) + assert.Contains(t, s, `"databricks-connect~=17.2.0",`, "dev carries the single managed pin") + assert.Contains(t, regions, regionDatabricksConnect) + requireValidTOML(t, out) + + // Idempotent: a second merge finds nothing to remove and produces identical bytes. + out2, _, err := MergeManaged(out, testConstraints()) + require.NoError(t, err) + assert.Equal(t, string(out), string(out2)) +} + +func TestMergeConsolidatesDatabricksConnectEmptiesSingleLineArray(t *testing.T) { + // Removing the only element of a single-line array leaves a valid empty array, + // not a dangling comma. + in := []byte(`[project] +requires-python = ">=3.10" + +[project.optional-dependencies] +spark = ["databricks-connect==15.0.0"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +`) + out, _, err := MergeManaged(in, testConstraints()) + require.NoError(t, err) + s := string(out) + assert.Contains(t, s, "spark = []", "the optional-dependency extra is emptied") + assert.NotContains(t, s, "databricks-connect==15.0.0") + requireValidTOML(t, out) +} + +func TestMergeConsolidatesLeavesOtherElementsInArray(t *testing.T) { + // Only the databricks-connect element is removed from a mixed single-line array; + // the surviving elements and the array structure stay intact. + in := []byte(`[project] +requires-python = ">=3.10" +dependencies = ["numpy", "databricks-connect==15.1.*", "pytest"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +`) + out, _, err := MergeManaged(in, testConstraints()) + require.NoError(t, err) + s := string(out) + assert.Contains(t, s, `dependencies = ["numpy", "pytest"]`) + assert.NotContains(t, s, "databricks-connect==15.1.*") + requireValidTOML(t, out) +} + +func TestMergeKeepsCompatibleDatabricksConnectPins(t *testing.T) { + // Only a databricks-connect pin that provably cannot co-resolve with the managed + // pin is removed. A pin that overlaps it, carries no version, or is marker-gated + // resolves fine, so it is left in place — the merge does not silently rewrite a + // user declaration (including [project].dependencies wheel metadata) that isn't + // broken. env pin here is ~=17.2.0. + in := []byte(`[project] +requires-python = ">=3.10" +dependencies = [ + "databricks-connect>=15", + "databricks-connect ; python_version < '3.13'", +] + +[project.optional-dependencies] +extra = ["databricks-connect"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +test = ["databricks-connect>=15,<20"] +`) + out, _, err := MergeManaged(in, testConstraints()) + require.NoError(t, err) + s := string(out) + assert.Contains(t, s, `"databricks-connect>=15",`, "an overlapping pin co-resolves and is kept") + assert.Contains(t, s, "python_version < '3.13'", "a marker-gated pin is not compared and is kept") + assert.Contains(t, s, `extra = ["databricks-connect"]`, "an unversioned pin is kept") + assert.Contains(t, s, `test = ["databricks-connect>=15,<20"]`, "an overlapping ranged pin in another group is kept") + assert.Contains(t, s, `"databricks-connect~=17.2.0"`, "the dev pin is still updated to the managed version") + requireValidTOML(t, out) +} + +func TestMergeKeepsEnvEqualPinInProjectDeps(t *testing.T) { + // A [project].dependencies pin that already equals the managed version is not + // disjoint from it, so it is left in place — the only declaration in the wheel + // metadata is never silently deleted, and no consolidation warning is emitted. + in := []byte(`[project] +requires-python = ">=3.10" +dependencies = ["databricks-connect~=17.2.0"] +`) + out, _, err := MergeManaged(in, testConstraints()) + require.NoError(t, err) + assert.Contains(t, string(out), `dependencies = ["databricks-connect~=17.2.0"]`, "the env-equal pin is kept") + requireValidTOML(t, out) +} + +func TestMergeKeepsClosingBracketOnItsOwnLine(t *testing.T) { + // Removing the last element of a multi-line array with no trailing comma on it must + // not pull the closing "]" up onto the previous element's line (a common uv-init shape). + in := []byte(`[project] +requires-python = ">=3.10" +dependencies = [ + "databricks-dlt", + "pytest", + "databricks-connect==15.1.*" +] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +`) + out, _, err := MergeManaged(in, testConstraints()) + require.NoError(t, err) + s := string(out) + assert.NotContains(t, s, "databricks-connect==15.1.*") + assert.Contains(t, s, "\n \"pytest\",\n]", "the closing bracket keeps its own line and the trailing comma survives") + assert.NotContains(t, s, `"pytest"]`, "the bracket is not pulled up") + requireValidTOML(t, out) +} + +func TestMergeConstraintsOnlyLeavesDatabricksConnectUntouched(t *testing.T) { + // In constraints-only mode (empty DatabricksConnect) databricks-connect is not + // managed at all: no pin is inserted and no stray is removed, anywhere. + in := []byte(`[project] +requires-python = ">=3.10" +dependencies = ["databricks-connect==15.1.*"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +docs = ["databricks-connect~=14.3"] +`) + c := Constraints{RequiresPython: "==3.12.*", ConstraintDeps: []string{"pydantic~=2.10.6"}} + out, regions, err := MergeManaged(in, c) + require.NoError(t, err) + s := string(out) + assert.Contains(t, s, "databricks-connect==15.1.*", "project.dependencies pin left untouched") + assert.Contains(t, s, "databricks-connect~=16.0", "dev pin left untouched") + assert.Contains(t, s, "databricks-connect~=14.3", "sibling group left untouched") + assert.NotContains(t, regions, regionDatabricksConnect) + requireValidTOML(t, out) +} + +func TestMergeConsolidatesStrayAfterCommentedElement(t *testing.T) { + // A stray databricks-connect element whose token carries a leading comment (from + // the previous element's trailing comment, or the opening "[" line) must still be + // removed. dbconnectElementPin scans the token per line so the leading comment does + // not hide the quoted requirement on the next line. + cases := map[string]string{ + "trailing comment on previous element": `[project] +requires-python = ">=3.10" +dependencies = [ + "pytest", # test runner + "databricks-connect==15.1.*", +] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +`, + "comment on the opening bracket line": `[project] +requires-python = ">=3.10" +dependencies = [ # runtime deps + "databricks-connect==15.1.*", + "pytest", +] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +`, + } + // A comment on a surviving element (or the opening line) must be preserved; the + // removal only drops the databricks-connect element, not the neighbouring comment. + wantComment := map[string]string{ + "trailing comment on previous element": "# test runner", + "comment on the opening bracket line": "# runtime deps", + } + for name, in := range cases { + out, _, err := MergeManaged([]byte(in), testConstraints()) + require.NoError(t, err, name) + s := string(out) + assert.NotContains(t, s, "databricks-connect==15.1.*", name) + assert.Contains(t, s, `"pytest",`, name) + assert.Contains(t, s, `"databricks-connect~=17.2.0"`, name) + assert.Contains(t, s, wantComment[name], name+": neighbouring comment must be preserved") + requireValidTOML(t, out) + } +} + +func TestMergeConsolidatesSecondDevPinAfterCommentedManagedPin(t *testing.T) { + // The dev-group keepFirst dedup must also see through a comment: the managed pin + // carries a trailing comment, and the second databricks-connect on the next line + // must still be removed rather than shadowed by that comment. + in := []byte(`[project] +requires-python = ">=3.10" + +[dependency-groups] +dev = [ + "databricks-connect~=16.0", # managed by setup-local + "databricks-connect==15.0.0", +] +`) + out, _, err := MergeManaged(in, testConstraints()) + require.NoError(t, err) + s := string(out) + assert.Contains(t, s, `"databricks-connect~=17.2.0",`, "the managed pin is kept and updated") + assert.NotContains(t, s, "databricks-connect==15.0.0", "the second pin is removed") + assert.Contains(t, s, "# managed by setup-local", "the managed pin's comment is preserved") + requireValidTOML(t, out) +} + +func TestMergeConsolidationSkipsIncludeGroupAndLiteralStrings(t *testing.T) { + // The removal matches double-quoted databricks-connect elements only, mirroring the + // rewrite: a PEP 735 include-group reference and a single-quoted pin are left alone. + in := []byte(`[project] +requires-python = ">=3.10" + +[dependency-groups] +dev = [{include-group = "spark"}] +spark = ['databricks-connect==15.0.0'] +`) + out, _, err := MergeManaged(in, testConstraints()) + require.NoError(t, err) + s := string(out) + assert.Contains(t, s, `{include-group = "spark"}`, "the include-group reference is preserved") + assert.Contains(t, s, `'databricks-connect==15.0.0'`, "the single-quoted pin is not removed") + requireValidTOML(t, out) +} + func TestMergeDatabricksConnectDoesNotClobberComment(t *testing.T) { // A databricks-connect token inside a trailing comment is user content and // must not be rewritten. diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index 8fe227620b9..46a5507a9fa 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -323,14 +323,14 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, if err != nil { return nil, greenfield, p.fail(PhaseMerge, false, NewError(ErrMerge, err, "merge managed regions failed")) } - // Surface merge-quality warnings (overridden or duplicated pins, conflicting - // user constraints) from the pre-merge file. Greenfield has nothing of the - // user's to override, so it is skipped. This runs for both dry-run and real - // runs so the --json consumer sees the same warnings either way. The pin the - // merge rewrote comes from the merge itself, so the warning can never claim a - // replacement that did not happen. + // Surface merge-quality warnings (overridden, consolidated, or duplicated pins, + // conflicting user constraints) from the pre-merge file. Greenfield has nothing of + // the user's to override, so it is skipped. This runs for both dry-run and real + // runs so the --json consumer sees the same warnings either way. The databricks-connect + // edits come from the merge itself (planDBConnect), so a warning can never claim a + // rewrite or removal that did not happen. p.res.Warnings = append(p.res.Warnings, - detectMergeWarnings(baseBytes, effective, replacedDBConnectPin(baseBytes, effective))...) + detectMergeWarnings(baseBytes, effective, planDBConnect(baseBytes, effective))...) } // Under --dry-run, build the plan (with a diff) for reporting. A real run does diff --git a/libs/localenv/result.go b/libs/localenv/result.go index 06fb588f9d3..1e704bcc55d 100644 --- a/libs/localenv/result.go +++ b/libs/localenv/result.go @@ -242,14 +242,21 @@ const ( // WarnDBConnectPinOverridden: the user's databricks-connect pin sits directly in // the dev group and is replaced by the managed value. WarnDBConnectPinOverridden = "W_DBCONNECT_PIN_OVERRIDDEN" + // WarnDBConnectConsolidated: a databricks-connect pin the user had outside the + // managed dev entry — in [project].dependencies, an optional-dependency extra, or + // another dependency group — was disjoint from the environment's version and is + // removed, so what remains resolves. Only conflicting pins are removed; a pin that + // co-resolves, carries no version, or is marker-gated is left in place. Emitted once + // per removed pin. Informational: the merge makes the project resolvable. + WarnDBConnectConsolidated = "W_DBCONNECT_CONSOLIDATED" // WarnDBConnectPinDuplicated: a databricks-connect pin of the user's is one the - // merge does not rewrite — reached through a PEP 735 include-group, a second pin - // in the dev array, or a group key that normalizes to "dev" without matching the - // literal key the merge edits — so the managed pin lands in the dev group - // alongside it. Unlike an override this leaves two pins for one package and uv - // cannot resolve it, a distinct and worse outcome that needs a manual fix, so it - // carries its own code. It can accompany an override and persists across re-runs - // for as long as the retained pin does. + // merge can neither rewrite nor remove — a spelling the line-based passes do not + // reach (a single-quoted element, a pin under a quoted TOML key, or an inline-table + // or dotted sub-table form) — so the managed pin lands in the dev group alongside + // it. Unlike an override this leaves two pins for one package and, where their + // ranges are disjoint, uv cannot resolve it — a distinct and worse outcome that + // needs a manual fix, so it carries its own code. It can accompany an override or a + // consolidation and persists across re-runs for as long as the survivor does. WarnDBConnectPinDuplicated = "W_DBCONNECT_PIN_DUPLICATED" // WarnUserConstraintConflict: a user dependency pins a package that the env's // constraint-dependencies also constrains, to a provably non-overlapping version diff --git a/libs/localenv/warnings.go b/libs/localenv/warnings.go index 3eb5821a2d4..d6737c31926 100644 --- a/libs/localenv/warnings.go +++ b/libs/localenv/warnings.go @@ -18,8 +18,9 @@ import ( var depSpecRe = regexp.MustCompile(`^([A-Za-z0-9._-]+)\s*(.*)$`) // singleClauseRe parses one version clause: an operator and a dotted numeric -// release. Pre/post/dev suffixes and wildcards are not modeled — a clause we -// cannot parse this simply is treated as "unknown" and never yields a conflict. +// release. A trailing "==X.Y.*" prefix wildcard is modeled by parseClause; pre/post/dev +// suffixes are not — a clause we cannot parse this simply is treated as "unknown" and +// never yields a conflict. var singleClauseRe = regexp.MustCompile(`^(>=|<=|==|~=|!=|<|>)?\s*([0-9]+(?:\.[0-9]+)*)`) // splitDepSpec returns the normalized package name and the version specifier @@ -160,7 +161,7 @@ func resolutionRequirements(p userPyprojectTOML) []string { // content) produce nothing — there is nothing of the user's to override. Warnings are // deterministic and ordered (requires-python, then databricks-connect, then // constraint conflicts in the order uv would encounter them) so goldens are stable. -func detectMergeWarnings(userPyproject []byte, c Constraints, replacedPin string) []Warning { +func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan) []Warning { if len(userPyproject) == 0 { return nil } @@ -183,51 +184,89 @@ func detectMergeWarnings(userPyproject []byte, c Constraints, replacedPin string }) } - // The user's databricks-connect pin differs from the env's. Only meaningful in - // default mode (c.DatabricksConnect is empty in constraints-only, where the dev - // group is left untouched). + // The requirements uv still resolves after the merge: everything minus the dev pin + // the merge rewrites (replacedDevPin) and the strays the consolidation pass removes. + // A conflict against an entry the merge discards describes a state that does not + // survive it, so excluding both keeps the conflict scan honest. + survivors := removeDBConnectPins(retainedRequirements(p, plan.replacedDevPin), plan.removed) + + // The user's databricks-connect pins differ from the env's. Only meaningful in + // default mode (c.DatabricksConnect is empty in constraints-only, where + // databricks-connect is left untouched). if c.DatabricksConnect != "" { - warnings = append(warnings, dbconnectWarnings(p, replacedPin, c.DatabricksConnect)...) + warnings = append(warnings, dbconnectWarnings(plan, survivors, c.DatabricksConnect)...) } - // The pin the merge replaces is excluded from the conflict scan: a conflict against - // an entry that is about to be discarded describes a state that does not survive - // the merge, and its own warning already covers it. Retained databricks-connect - // pins stay in scope — they are still part of the resolution. - warnings = append(warnings, constraintConflicts(retainedRequirements(p, replacedPin), c.ConstraintDeps)...) + warnings = append(warnings, constraintConflicts(survivors, c.ConstraintDeps)...) return warnings } -// dbconnectWarnings reports how the merge treats the user's databricks-connect -// pins, distinguishing two outcomes that need different user actions. +// removeDBConnectPins drops from reqs the requirements the consolidation pass deletes, +// one occurrence per removed pin (mirroring retainedRequirements' subtraction of the +// rewritten dev pin). The strays are removed by MergeManaged, so a warning about them +// as if they survived would describe a state the merge does not produce. +func removeDBConnectPins(reqs []string, removed []removedDBConnect) []string { + if len(removed) == 0 { + return reqs + } + counts := make(map[string]int, len(removed)) + for _, r := range removed { + counts[strings.TrimSpace(r.pin)]++ + } + out := make([]string, 0, len(reqs)) + for _, r := range reqs { + t := strings.TrimSpace(r) + if counts[t] > 0 { + counts[t]-- + continue + } + out = append(out, r) + } + return out +} + +// dbconnectWarnings reports how the merge treats the user's databricks-connect pins, +// distinguishing three outcomes that need different user actions. What the merge does +// is not re-derived here — it comes from plan (see planDBConnect): // -// Which pin the merge rewrites is not re-derived here — replacedPin comes from the -// merge itself (see replacedDBConnectPin). Every other databricks-connect -// requirement uv would resolve is left in place beside the managed pin: a pin in a -// group the merge does not edit, one reached through a PEP 735 include-group, or a -// second element in dev's own array. That leaves two pins for one package, and where -// their ranges do not intersect uv cannot resolve at all — a strictly worse outcome -// than an override, and one the user has to fix by hand. Reporting it as "is -// replaced" would state something factually untrue and hide the resolution failure -// behind a reassuring advisory. +// - replacedDevPin: the pin sitting directly in the dev group, rewritten in place to +// the env's version (WarnDBConnectPinOverridden). +// - removed: strays the consolidation pass deletes from [project].dependencies, an +// optional-dependency extra, or another group because their range is disjoint from +// the env pin — they would otherwise make uv unsatisfiable (WarnDBConnectConsolidated). +// - a databricks-connect pin the line-based merge cannot delete but uv still resolves +// — a spelling the passes do not reach (single-quoted element, quoted TOML key, +// inline-table or dotted sub-table form) — left beside the managed pin. Where its +// range is disjoint from the env's, uv cannot resolve at all +// (WarnDBConnectPinDuplicated), a distinct failure the user must fix by hand. // -// The two conditions are reported independently rather than as a first-match choice, -// because they can coexist and the duplicate outlives the override: once the env's -// pin is in dev, a re-run finds a matching direct pin, and stopping there would go -// silent on a retained pin that still makes the project unresolvable. -func dbconnectWarnings(p userPyprojectTOML, replacedPin, envPin string) []Warning { +// The conditions are reported independently rather than as a first-match choice, +// because a run can override the dev pin, consolidate strays, and still leave an +// unresolvable survivor all at once. +func dbconnectWarnings(plan dbconnectPlan, survivors []string, envPin string) []Warning { var warnings []Warning envPin = strings.TrimSpace(envPin) - if replacedPin != "" && strings.TrimSpace(replacedPin) != envPin { + if plan.replacedDevPin != "" && strings.TrimSpace(plan.replacedDevPin) != envPin { warnings = append(warnings, Warning{ Code: WarnDBConnectPinOverridden, - Message: fmt.Sprintf("databricks-connect %q is replaced by the environment's %q", strings.TrimSpace(replacedPin), envPin), + Message: fmt.Sprintf("databricks-connect %q is replaced by the environment's %q", strings.TrimSpace(plan.replacedDevPin), envPin), + }) + } + + // Every removed pin is disjoint from envPin (removeStrayDatabricksConnect only deletes + // those), so each is a real conflict the user should see — there is no equal-pin case + // to skip here. + for _, r := range plan.removed { + warnings = append(warnings, Warning{ + Code: WarnDBConnectConsolidated, + Message: fmt.Sprintf("databricks-connect %q in %s conflicts with the environment's %q and is removed; it is managed in %q", + strings.TrimSpace(r.pin), r.location, envPin, devGroup), }) } _, envSpec, envOK := splitDepSpec(envPin) - for _, pin := range dbconnectPins(retainedRequirements(p, replacedPin)) { + for _, pin := range dbconnectPins(survivors) { if pin == envPin { // An identical pin needs no reconciliation: uv sees one requirement twice. continue @@ -323,9 +362,12 @@ func constraintConflicts(userDeps, envConstraints []string) []Warning { } // clause is a parsed single version clause: an operator and a numeric release. +// wildcard marks a "==X.Y.*" prefix match, whose range is the segment held fixed +// (see clause.interval). type clause struct { - op string - rel []int + op string + rel []int + wildcard bool } // parseClause parses one "" clause. ok is false when the clause cannot @@ -339,15 +381,23 @@ func parseClause(spec string) (clause, bool) { if m == nil { return clause{}, false } - // Reject anything trailing the numeric release (wildcards, pre/post/dev tags): - // modeling those correctly is out of scope, and guessing risks a false conflict. - if strings.TrimSpace(spec[len(m[0]):]) != "" { - return clause{}, false - } op := m[1] if op == "" { op = "==" // a bare version in dependencies means an exact pin } + // A trailing ".*" is a PEP 440 prefix-match wildcard. It is only defined for "==" + // (and "!="); "==15.1.*" pins the 15.1 series. For any other operator ".*" is + // malformed, and "!=X.*" excludes a whole series — a non-contiguous set the + // interval model cannot represent — so both stay unknown. Any other trailing text + // (pre/post/dev tags) is out of scope and also refused, to avoid a false conflict. + wildcard := false + switch rest := strings.TrimSpace(spec[len(m[0]):]); { + case rest == "": + case rest == ".*" && op == "==": + wildcard = true + default: + return clause{}, false + } var rel []int for part := range strings.SplitSeq(m[2], ".") { n, err := strconv.Atoi(part) @@ -364,7 +414,7 @@ func parseClause(spec string) (clause, bool) { if op == "~=" && len(rel) < 2 { return clause{}, false } - return clause{op: op, rel: rel}, true + return clause{op: op, rel: rel, wildcard: wildcard}, true } // rangesDisjoint reports whether two version specifiers provably share no @@ -435,6 +485,13 @@ type interval struct { func (c clause) interval() (interval, bool) { switch c.op { case "==": + if c.wildcard { + // "==X.Y.*" admits the whole X.Y series: floor X.Y inclusive, exclusive + // ceiling incrementing the last held segment ("==15.1.*" -> [15.1, 15.2)). + ceil := slices.Clone(c.rel) + ceil[len(ceil)-1]++ + return interval{lo: c.rel, loIncl: true, hi: ceil}, true + } // A single point. compareRelease zero-pads, so this also covers the spellings // that denote the same release (3.12 and 3.12.0). return interval{lo: c.rel, loIncl: true, hi: c.rel, hiIncl: true}, true diff --git a/libs/localenv/warnings_test.go b/libs/localenv/warnings_test.go index 598c040e7a2..3fd6bb1e573 100644 --- a/libs/localenv/warnings_test.go +++ b/libs/localenv/warnings_test.go @@ -7,12 +7,12 @@ import ( "github.com/stretchr/testify/require" ) -// detectWarnings runs the detector the way mergePlan does, taking the replaced pin -// from the merge itself rather than a hand-supplied value. Tests must not assert -// against a different notion of what the merge rewrites than production uses — that -// divergence is the bug this wiring exists to prevent. +// detectWarnings runs the detector the way mergePlan does, taking the databricks-connect +// plan from the merge itself rather than hand-supplied values. Tests must not assert +// against a different notion of what the merge rewrites or removes than production uses — +// that divergence is the bug this wiring exists to prevent. func detectWarnings(userPyproject []byte, c Constraints) []Warning { - return detectMergeWarnings(userPyproject, c, replacedDBConnectPin(userPyproject, c)) + return detectMergeWarnings(userPyproject, c, planDBConnect(userPyproject, c)) } // codes extracts the warning codes in order for concise assertions. @@ -129,13 +129,12 @@ test = [] }, codes(detectWarnings(user, c))) } -func TestDetectMergeWarningsIncludeGroupIsDuplicatedNotOverridden(t *testing.T) { - // MergeManaged only rewrites a pin sitting in dev's own array. A pin reached - // through an include-group is left alone and the env's pin is inserted alongside - // it, so the merged file carries two pins for one package and uv cannot resolve. - // That is a different condition from an override and needs its own code — - // reporting "is replaced" here would state something untrue and hide a hard - // resolution failure behind a reassuring advisory. +func TestDetectMergeWarningsStrayPinsAreConsolidated(t *testing.T) { + // A databricks-connect pin in another group — reached through an include-group or + // held directly in a sibling group — is removed by the consolidation pass so the + // managed pin is the only one left. That is reported as consolidation, not as an + // override (nothing in dev was rewritten) and not as a duplicate (the pin does not + // survive the merge). c := Constraints{RequiresPython: "==3.12.*", DatabricksConnect: "databricks-connect==17.0.0"} indirect := []byte(`[project] @@ -146,7 +145,7 @@ dev = [{include-group = "spark"}] spark = ["databricks-connect==16.1.0"] `) got := detectWarnings(indirect, c) - assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(got)) + assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(got)) // The message must not claim a replacement that did not happen. assert.NotContains(t, got[0].Message, "is replaced") @@ -160,8 +159,7 @@ dev = ["databricks-connect==16.1.0"] assert.Equal(t, []string{WarnDBConnectPinOverridden}, codes(detectWarnings(direct, c))) // With pins both in dev and behind an include-group, the merge rewrites dev's and - // leaves the included one, so both conditions hold and both are reported. Only - // flagging the override would go silent on the pin that still breaks resolution. + // removes the included one, so both an override and a consolidation are reported. both := []byte(`[project] requires-python = "==3.12.*" @@ -169,18 +167,17 @@ requires-python = "==3.12.*" dev = ["databricks-connect==16.1.0", {include-group = "spark"}] spark = ["databricks-connect==15.0.0"] `) - assert.Equal(t, []string{WarnDBConnectPinOverridden, WarnDBConnectPinDuplicated}, + assert.Equal(t, []string{WarnDBConnectPinOverridden, WarnDBConnectConsolidated}, codes(detectWarnings(both, c))) - // The merge is idempotent, but the included pin it does not rewrite is not fixed - // by re-running: the duplicate warning must persist for as long as the two pins do, - // or the user loses the only signal about a project uv cannot resolve. + // Consolidation actually fixes the project: re-running on the merged file finds a + // single managed pin and nothing to warn about (the spark stray is gone). merged, _, err := MergeManaged(both, c) require.NoError(t, err) - assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(detectWarnings(merged, c))) + assert.Empty(t, detectWarnings(merged, c)) // An include-group cycle must terminate rather than recurse forever, and the pin - // behind it is still found. + // behind it is still removed. cyclic := []byte(`[project] requires-python = "==3.12.*" @@ -188,19 +185,22 @@ requires-python = "==3.12.*" dev = [{include-group = "a"}] a = [{include-group = "dev"}, "databricks-connect==16.1.0"] `) - assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(detectWarnings(cyclic, c))) + assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(detectWarnings(cyclic, c))) - // PEP 735 normalizes group names the same way PEP 503 normalizes package names. - renamed := []byte(`[project] + // A quoted TOML group key is outside the line-based removal's reach, so the pin + // survives beside the managed one and is reported as a duplicate — uv still cannot + // resolve it, so the user must not lose the signal. + quotedKey := []byte(`[project] requires-python = "==3.12.*" [dependency-groups] -dev = [{include-group = "My_Spark.Group"}] +dev = [{include-group = "my-spark-group"}] "my-spark-group" = ["databricks-connect==16.1.0"] `) - assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(detectWarnings(renamed, c))) + assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(detectWarnings(quotedKey, c))) - // An included pin that already matches the env needs no reconciliation. + // A stray pin that already matches the env is not disjoint from it, so the gate + // leaves it in place and there is nothing to warn about. matching := []byte(`[project] requires-python = "==3.12.*" @@ -364,11 +364,11 @@ Dev = ["numpy==9.9.9"] } } -func TestDetectMergeWarningsNonLiteralDevGroupIsNotAnOverride(t *testing.T) { +func TestDetectMergeWarningsNonLiteralDevGroupIsConsolidated(t *testing.T) { // MergeManaged finds the dev array with devKeyRe (`^\s*dev\s*=`), so a group spelled - // "Dev" is never rewritten: the merge adds its own dev key and the user's pin stays. - // Claiming an override here would describe a replacement that did not happen, and - // staying silent would hide a file uv rejects outright. + // "Dev" is never rewritten: the merge adds its own dev key. The consolidation pass + // then removes the stray pin from "Dev", so it is reported as consolidated, not as + // an override (nothing was replaced in place). c := Constraints{RequiresPython: "==3.12.*", DatabricksConnect: "databricks-connect==17.0.0"} user := []byte(`[project] requires-python = "==3.12.*" @@ -377,42 +377,40 @@ requires-python = "==3.12.*" Dev = ["databricks-connect==16.1.0"] `) got := detectWarnings(user, c) - assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(got)) + assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(got)) assert.NotContains(t, got[0].Message, "is replaced") merged, _, err := MergeManaged(user, c) require.NoError(t, err) - assert.Contains(t, string(merged), `"databricks-connect==16.1.0"`, "the user's pin is retained, not replaced") + assert.NotContains(t, string(merged), `databricks-connect==16.1.0`, "the stray pin is removed, not retained") } -func TestDBConnectOverrideFollowsWhatTheMergeRewrites(t *testing.T) { - // The override warning must be driven by the merge's own answer, not by a second - // implementation of "is this pin in the dev array". MergeManaged rewrites only - // double-quoted elements, so each spelling below is left in place beside the managed - // pin — reporting it as replaced would be a false claim about the user's file. +func TestDBConnectWarningsFollowWhatTheMergeDoes(t *testing.T) { + // The warnings must be driven by the merge's own answer, not by a second + // implementation of what it edits. The merge rewrites and removes only + // double-quoted elements the line-based passes reach, so a pin the passes cannot + // touch survives beside the managed pin and is a duplicate (uv cannot resolve), + // while a pin they can reach is consolidated away. c := Constraints{RequiresPython: "==3.12.*", DatabricksConnect: "databricks-connect==17.0.0"} - for name, body := range map[string]string{ - // A TOML literal string is a string, but not one replaceDbconnectElement matches. + + // Pins the line-based passes cannot reach survive the merge — a duplicate. + survives := map[string]string{ + // A TOML literal string is a string, but not one the double-quoted matcher sees. "single-quoted element": `[project] requires-python = "==3.12.*" [dependency-groups] dev = ['databricks-connect==16.1.0'] `, - // A dotted key defines the same table but no line matches devKeyRe. + // A dotted key defines the group outside a [dependency-groups] table header, so + // the line-based removal never scans it. "top-level dotted key": `dependency-groups.dev = ["databricks-connect==16.1.0"] [project] requires-python = "==3.12.*" `, - // devKeyRe is literal, so a normalization-equal key is a different array. - "capitalized group": `[project] -requires-python = "==3.12.*" - -[dependency-groups] -Dev = ["databricks-connect==16.1.0"] -`, - } { + } + for name, body := range survives { got := detectWarnings([]byte(body), c) assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(got), name) assert.NotContains(t, got[0].Message, "is replaced", name) @@ -423,20 +421,35 @@ Dev = ["databricks-connect==16.1.0"] assert.Contains(t, string(merged), "databricks-connect==16.1.0", name) assert.Contains(t, string(merged), "databricks-connect==17.0.0", name) } + + // A double-quoted pin in a normalization-equal group ("Dev") is reached by the + // removal pass, so it is consolidated away rather than left as a duplicate. + capitalized := []byte(`[project] +requires-python = "==3.12.*" + +[dependency-groups] +Dev = ["databricks-connect==16.1.0"] +`) + got := detectWarnings(capitalized, c) + assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(got)) + merged, _, err := MergeManaged(capitalized, c) + require.NoError(t, err) + assert.NotContains(t, string(merged), "databricks-connect==16.1.0") + assert.Contains(t, string(merged), "databricks-connect==17.0.0") } func TestDBConnectDuplicateOnlyWhenRangesCannotBothHold(t *testing.T) { - // Two pins for one package are only a problem when nothing satisfies both. uv - // resolves an overlapping pair without complaint, so there is nothing to reconcile - // and no warning to give. + // A pin the merge cannot remove (single-quoted, so the double-quoted-only passes do + // not reach it) survives beside the managed pin. Two pins for one package are only a + // problem when nothing satisfies both: uv resolves an overlapping pair without + // complaint, so there is nothing to reconcile and no warning to give. c := Constraints{RequiresPython: "==3.12.*", DatabricksConnect: "databricks-connect~=17.2.0"} overlapping := []byte(`[project] requires-python = "==3.12.*" [dependency-groups] -dev = [{include-group = "spark"}] -spark = ["databricks-connect>=16"] +dev = ['databricks-connect>=16'] `) assert.Empty(t, detectWarnings(overlapping, c), ">=16 and ~=17.2.0 both hold at 17.2.x") @@ -444,16 +457,16 @@ spark = ["databricks-connect>=16"] requires-python = "==3.12.*" [dependency-groups] -dev = [{include-group = "spark"}] -spark = ["databricks-connect==15.0.0"] +dev = ['databricks-connect==15.0.0'] `) assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(detectWarnings(disjoint, c)), "==15.0.0 is outside ~=17.2.0, so uv cannot resolve") } -func TestDBConnectWarningsSecondDirectPinIsRetained(t *testing.T) { - // MergeManaged replaces only the first databricks-connect element in the dev array, - // so a second one survives and leaves two pins uv cannot resolve. +func TestDBConnectWarningsSecondDirectPinIsConsolidated(t *testing.T) { + // MergeManaged rewrites only the first databricks-connect element in the dev array; + // the consolidation pass then removes any further one, so a second pin is reported + // as consolidated rather than left as a duplicate uv cannot resolve. c := Constraints{RequiresPython: "==3.12.*", DatabricksConnect: "databricks-connect==17.0.0"} user := []byte(`[project] requires-python = "==3.12.*" @@ -461,9 +474,70 @@ requires-python = "==3.12.*" [dependency-groups] dev = ["databricks-connect==17.0.0", "databricks-connect==16.0.0"] `) - // The first pin already matches the env, so there is nothing to override — but the - // second still has to be reconciled by hand. - assert.Equal(t, []string{WarnDBConnectPinDuplicated}, codes(detectWarnings(user, c))) + // The first pin already matches the env, so there is nothing to override; the second + // is removed and reported as consolidated. + assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(detectWarnings(user, c))) +} + +func TestConsolidatedWarningNamesLocationAndCoexistsWithOverride(t *testing.T) { + // A stray pin in [project].dependencies is consolidated, and the message names where + // it came from so the user can see what the merge edited. + c := Constraints{RequiresPython: "==3.12.*", DatabricksConnect: "databricks-connect~=17.2.0"} + projectDeps := []byte(`[project] +requires-python = "==3.12.*" +dependencies = ["databricks-connect==15.1.*"] +`) + got := detectWarnings(projectDeps, c) + require.Equal(t, []string{WarnDBConnectConsolidated}, codes(got)) + assert.Contains(t, got[0].Message, "[project].dependencies") + + // An override in dev and consolidations across an optional extra and a sibling group + // coexist, in a deterministic order (override first, then removals in resolution + // order: optional-dependencies before dependency-groups). + mixed := []byte(`[project] +requires-python = "==3.12.*" + +[project.optional-dependencies] +extra = ["databricks-connect==14.0.0"] + +[dependency-groups] +dev = ["databricks-connect==16.1.0"] +docs = ["databricks-connect==15.0.0"] +`) + want := codes(detectWarnings(mixed, c)) + assert.Equal(t, []string{WarnDBConnectPinOverridden, WarnDBConnectConsolidated, WarnDBConnectConsolidated}, want) + // The order must be stable across runs (group names come from a Go map). + for range 200 { + assert.Equal(t, want, codes(detectWarnings(mixed, c))) + } +} + +func TestConstraintsOnlyEmitsNoDBConnectWarnings(t *testing.T) { + // With an empty DatabricksConnect (constraints-only mode) databricks-connect is left + // untouched wherever it sits, so none of the db-connect warnings fire. + c := Constraints{RequiresPython: "==3.12.*", DatabricksConnect: ""} + user := []byte(`[project] +requires-python = "==3.12.*" +dependencies = ["databricks-connect==15.1.*"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +docs = ["databricks-connect==15.0.0"] +`) + assert.Empty(t, detectWarnings(user, c)) +} + +func TestUserConstraintConflictFiresForWildcardPin(t *testing.T) { + // A prefix-match wildcard pin ("==18.*") must be modeled so a conflict with the env's + // constraint-dependencies is reported rather than silently missed. + user := []byte(`[project] +requires-python = "==3.12.*" +dependencies = ["pyarrow==18.*"] +`) + c := Constraints{RequiresPython: "==3.12.*", ConstraintDeps: []string{"pyarrow~=19.0"}} + got := detectWarnings(user, c) + assert.Equal(t, []string{WarnUserConstraintConflict}, codes(got)) + assert.Contains(t, got[0].Message, "pyarrow") } func TestConstraintConflictsDuplicateEnvEntriesAreOrderIndependent(t *testing.T) { @@ -530,9 +604,18 @@ func TestRangesDisjoint(t *testing.T) { {">=2.0", "==2.5", false, "2.5 is above the floor — overlapping"}, {"!=2.0", "==2.0", false, "!= is not modeled — never a conflict"}, {">=2.0,<3.0", "==5.0", true, "5.0 is above the compound range's ceiling"}, - {"==2.*", "==2.0", false, "wildcards are unparsed — no conflict"}, {"", "~=21.0.0", false, "no user spec — nothing to compare"}, + // PEP 440 prefix-match wildcards ("==X.Y.*"). The clause admits the whole X.Y + // series: [X.Y, X.(Y+1)). Only "==" carries a wildcard; "!=X.*" and any other + // operator stay undecidable (parseClause refuses them), never a false conflict. + {"==15.1.*", "~=17.0", true, "the 15.1 series is entirely below [17.0, 18.0)"}, + {"==17.0.*", "~=17.0", false, "the 17.0 series sits inside [17.0, 18.0)"}, + {"==17.*", "~=17.2.0", false, "the 17.* series spans the ~=17.2.0 range"}, + {"==2.*", "==3.0", true, "the 2.* series ends at 3, which the exact 3.0 exceeds"}, + {"==2.*", "==2.0", false, "2.0 is inside the 2.* series [2, 3)"}, + {"!=2.*", "==2.0", false, "!=X.* excludes a whole series — undecidable, never a conflict"}, + // Opposite-direction bounds. This is the shape the published constraint // artifacts use (pyarrow<19, pandas<3), so failing to decide it would make the // warning miss the common case and under-report the metric silently.