🐛 fix(activation): restore PKG_CONFIG_PATH that was not set before - #3233
🐛 fix(activation): restore PKG_CONFIG_PATH that was not set before#3233darrenhuai wants to merge 2 commits into
Conversation
Activation prepends the environment's `lib/pkgconfig` to `PKG_CONFIG_PATH` and saves the previous value so deactivation can put it back. When the variable was not set to begin with - which is the normal case, since most systems leave it alone - there is no previous value to put back, and bash, fish and PowerShell all read that as "nothing to restore" and leave the environment's directory in place. `PKG_CONFIG_PATH` goes from unset to permanently pointing into a virtual environment that is no longer active, so later builds keep resolving `.pc` files out of it. bash and fish save the empty string and then test it with `-n`, which is false for both "unset before" and "nothing saved". They now distinguish the two: the variable is unset again when it had not been set, and restored when it had. PowerShell only created its `_OLD_PKG_CONFIG_PATH` when the variable existed, so deactivation could not tell the two cases apart either; it now always records one, using `$null` for "was not set", which the existing restore already turns back into a removal. The same lines built the new value by pasting the old one on unconditionally, so batch, fish, nushell and PowerShell ended up with a trailing separator and an empty entry in the path when there was nothing to prepend to. Each now only appends a separator when there is something after it. csh and the Python activator already handled both cases and are unchanged. Round-trip tests for bash, batch, fish and PowerShell activate with `PKG_CONFIG_PATH` unset and check the value both while active and after deactivating.
|
The one red job, It stalled at 98%, in The four new tests all ran in that same job before the stall: 1.1s and 1.2s for the two that run on Windows, the other two skipped there as intended. The same commit is green on 3.10, 3.11, 3.12 and 3.14 on the same Windows image, which is not what a real regression from this change would look like. A re-run should clear it — I cannot trigger one from a fork. Happy to push a change if you would rather see it go green on a fresh run. |
Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)
tox -e fix)docs/changelogfolderActivation prepends the environment's
lib/pkgconfigtoPKG_CONFIG_PATHand saves the previous value so deactivation can put it back. When the variable was not set to begin with — the normal case, since most systems leave it alone — there is no previous value, and bash, fish and PowerShell all read that as "nothing to restore" and leave the environment's directory in place:So
PKG_CONFIG_PATHgoes from unset to permanently pointing into a virtual environment that is no longer active, and later builds keep resolving.pcfiles out of it.bash and fish save the empty string and then test it with
-n, which is false both for "was unset" and for "nothing saved". They now tell the two apart: the variable is unset again when it had not been set, and restored when it had. PowerShell only created_OLD_PKG_CONFIG_PATHwhen the variable already existed, so its deactivation could not distinguish the cases either; it now always records one, using$nullfor "was not set", which the existing restore already turns back into a removal.The same lines built the new value by pasting the old one on unconditionally, so batch, fish, nushell and PowerShell left a trailing separator — an empty entry in the path — when there was nothing to prepend to:
Each now only writes a separator when something follows it.
cshandactivate_this.pyalready handled both cases and are unchanged — the cshif ($?PKG_CONFIG_PATH) ... else ...and its|| unsetenv PKG_CONFIG_PATHare what the others are being brought in line with.Four round-trip tests (bash, batch, fish, PowerShell) activate with
PKG_CONFIG_PATHunset and check the value both while active and after deactivating, so they cover the missing restore and the trailing separator together.I could reproduce and re-check bash, batch and PowerShell directly — bash also under
set -o nounset, since this touches the lines from #3044 — and reverting each change individually puts the corresponding failure back. fish and nushell I could not run locally, so those two rest on review and CI.One thing I left alone, in case it is worth a separate issue:
activate.batdoes not calldeactivatefirst the way the bash and PowerShell scripts do, so re-activating stacksPKG_CONFIG_PATH— activate A, then B, then deactivate, and A'slib/pkgconfigstays behind. That is a different cause from the above and this PR neither fixes nor worsens it.