Fix: pyvenv.cfg corruption and wrong Python distribution selection when multiple Python versions are installed (3.13 after 3.12) - #1698
Draft
NeuralFault wants to merge 5 commits into
Conversation
…/writer - Remove dependency on Salaros.Configuration.ConfigParser for pyvenv.cfg serialization in both PyVenvRunner and UvVenvRunner SetPyvenvCfg methods - Adds PyVenvConfigHelper.WritePyVenvCfg that reads, updates, and writes the key=value lines directly without section-header round-tripping - Fixes silent failure where ConfigParser.SetValue would not update the existing "home" key in a sectionless INI file, while successfully adding new keys (base-prefix, base-exec-prefix, base-executable), producing a corrupt config with mixed Python distribution paths - Preserve all non-path keys (include-system-site-packages, version, executable, command, etc.) in their original line order - Append missing path keys if the venv was created by an older version that did not write them
…re installed
- Fix fallback directory scanner in UvManager.InstallPythonVersionAsync using Contains("3.12") which also matched "3.13.12" directory names, causing the wrong Python distribution to be selected when UV listing failed and the newer 3.13 installation had a more recent creation timestamp
- Switched to strict version prefix matching ("3.12.") with an EndsWith fallback for edge cases like "pypy-3.12" naming
- Fix installedOnly parameter in ListAvailablePythonsAsync being ignored, causing uninstalled Python entries with null Path to reach the PyInstallation constructor and throw ArgumentException, which aborted the entire UV discovery loop via the catch-all in GetAllInstallationsAsync
- Wire PyVenvConfigHelper.WritePyVenvCfg into PyVenvRunner and UvVenvRunner SetPyvenvCfg, replacing the Salaros.Configuration.ConfigParser round-trip that silently failed to update the existing "home" key
- Remove unused Salaros.Configuration using directives from both runner files
- Parse each line into key and value by splitting on '=', then compare the key with ordinal case-insensitive Equals rather than StartsWith - Preserve lines with no '=' delimiter as-is - Eliminates ordering dependency between key checks. Each key is now matched exactly and independently, so reordering the checks or adding a new key like "base" cannot silently swallow "base-prefix" or "base-executable" through prefix collision
… UvManager - When installedOnly is false the preceding Where clause allows e.Path to be null, making the null-forgiving operator (!) semantically incorrect and misleading - Replace with a conditional that uses Path.GetDirectoryName only when e.Path is non-null, falling back to string.Empty otherwise
NeuralFault
changed the title
Contributor
Author
|
@mohnjiles @ionite34 can also remove the salaros reference in the package.prop and csproj files. The NuGet package still gets pulled during |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Installing forge-neo (requires Python 3.13.12) alongside an existing WebUI package installation (using Python 3.12.10) silently corrupts that pre-existing package's
pyvenv.cfg, causingerror no: 2at launch. Manually correcting the file has no effect as it is rewritten incorrectly on every subsequent launch.Steps to reproduce:
Data/Assets/Python/cpython-3.12.10-...and a venv is created with a correctpyvenv.cfgData/Assets/Python/cpython-3.13.12-...and its own venv is created correctlypyvenv.cfgnow hasbase-prefix,base-exec-prefix, andbase-executablepointing to the 3.13.12 distribution, whilehomeremains the original 3.12.10 pathRoot cause (three compounding bugs)
Bug A: Fallback directory scanner matches wrong version (
UvManager.cs):When UV's
python listfails and the fallback scanner runs,Contains("3.12")matches bothcpython-3.12.10-...andcpython-3.13.12-...(the substring "3.12" appears in "3.13.12").Results are ordered by
CreationTimeUtcdescending, so the more recently installed 3.13.12 directory is selected as the "discovered" 3.12.10 installation.Bug B:
installedOnlyparameter is dead code (UvManager.cs):ListAvailablePythonsAsync(installedOnly: true)never filters to installed-only entries.Uninstalled Python entries with
Path = nullproduce an emptyInstallPath, which throwsArgumentExceptioninPyInstallation's constructor, aborting the entire UV discovery loop via the catch-all inGetAllInstallationsAsync. This pushes the system into Bug A's fallback path.Bug C: ConfigParser silently fails on pre-existing
homekey(
PyVenvRunner.cs/UvVenvRunner.cs):The
SetPyvenvCfgmethod prepends[top]to make the sectionlesspyvenv.cfgparseable bySalaros.Configuration.ConfigParser, then callsSetValue("top", "home", ...). The ConfigParser silently refuses to update the existinghomekey while successfully adding the new keys (base-prefix,base-exec-prefix,base-executable), producing the mixed-path config withhomeat 3.12 and the other three at 3.13.Changes
StabilityMatrix.Core/Python/PyVenvConfigHelper.cs(new file)Replaces the
ConfigParserroundtrip with a direct line-by-line key=value reader/writer.Extracts the key before
=on each line, compares with exactEquals, updates matching keys, and appends missing ones.Preserves all non-path keys in their original order. This eliminates the fragile
[top]section-header hack, the silent key-update failure, and the dependency on a third-party INI parser for a format that is not INI.Why a new helper instead of fixing ConfigParser:
pyvenv.cfgis a simplekey = valueformat with no sections, no quoting, and no escaping. A section-based INI parser adds indirection without adding value.[top]→ parse →SetValue→ToString()→ strip-[top]roundtrip has three fragility points: the section injection, the key update semantics on a sectionless file, and the section removal viaReplace.StabilityMatrix.Core/Python/PyVenvRunner.csSetPyvenvCfgreduced from a 15-lineConfigParserroundtrip to a 3-line call toPyVenvConfigHelper.WritePyVenvCfg. Removedusing Salaros.Configuration.StabilityMatrix.Core/Python/UvVenvRunner.csIdentical change to
PyVenvRunner.cs. Removedusing Salaros.Configuration.StabilityMatrix.Core/Python/UvManager.csListAvailablePythonsAsync: TheinstalledOnlyparameter now actually filters whentrue, entries withPath == nullare excluded. Explicit null check one.Pathin theSelectprojection rather than a null-forgiving operator.InstallPythonVersionAsyncfallback scanner:Contains("3.12")replaced withContains("3.12.")plus an EndsWith("-3.12")` fallback for PyPy-style directory names. The trailing dot prevents substring collision with higher versions.