Resolve prefix injection #11666

kenodegard · 2022-07-28T03:02:26Z

Description

Depends on #11649
Resolves #11174

When running conda run, users discovered that the wrong executable is invoked. This was especially noticeable on Windows. We first attempted to address this issue in #11257 but found that the source of the prefix injection occurred at the shell integration level so trying to do the path deduplication in the Python code would've been ripe for buggy behavior.

We narrowed it down to being the prefix injection that occurred in the following locations:

conda/conda/shell/etc/profile.d/conda.sh

Lines 4 to 30 in 8c835dd

    
           __add_sys_prefix_to_path() { 
        
               # In dev-mode CONDA_EXE is python.exe and on Windows 
        
               # it is in a different relative location to condabin. 
        
               if [ -n "${_CE_CONDA}" ] && [ -n "${WINDIR+x}" ]; then 
        
                   SYSP=$(\dirname "${CONDA_EXE}") 
        
               else 
        
                   SYSP=$(\dirname "${CONDA_EXE}") 
        
                   SYSP=$(\dirname "${SYSP}") 
        
               fi 
        
               if [ -n "${WINDIR+x}" ]; then 
        
                   PATH="${SYSP}/bin:${PATH}" 
        
                   PATH="${SYSP}/Scripts:${PATH}" 
        
                   PATH="${SYSP}/Library/bin:${PATH}" 
        
                   PATH="${SYSP}/Library/usr/bin:${PATH}" 
        
                   PATH="${SYSP}/Library/mingw-w64/bin:${PATH}" 
        
                   PATH="${SYSP}:${PATH}" 
        
               else 
        
                   PATH="${SYSP}/bin:${PATH}" 
        
               fi 
        
               \export PATH 
        
           } 
        
           __conda_exe() ( 
        
               __add_sys_prefix_to_path 
        
               "$CONDA_EXE" $_CE_M $_CE_CONDA "$@" 
        
           )

conda/conda/shell/etc/profile.d/conda.csh

Lines 29 to 30 in 8c835dd

    
           set conda_tmp_path="$PATH" 
        
           setenv PATH "`dirname ${_CONDA_EXE}`:$PATH"

conda/conda/shell/condabin/conda.bat

Line 25 in 8c835dd

    
           @SET PATH=!_sysp!;!_sysp!\Library\mingw-w64\bin;!_sysp!\Library\usr\bin;!_sysp!\Library\bin;!_sysp!\Scripts;!_sysp!\bin;%PATH%

conda/conda/shell/condabin/_conda_activate.bat

Line 23 in 8c835dd

    
           @SET PATH=!_sysp!;!_sysp!\Library\mingw-w64\bin;!_sysp!\Library\usr\bin;!_sysp!\Library\bin;!_sysp!\Scripts;!_sysp!\bin;%PATH%

conda/conda/shell/condabin/Conda.psm1

Lines 46 to 73 in 8c835dd

    
           <# 
        
               .SYNOPSIS 
        
                   Adds the entries of sys.prefix to PATH and returns the old PATH. 
        
               .EXAMPLE 
        
                   $OldPath = Add-Sys-Prefix-To-Path 
        
           #> 
        
           function Add-Sys-Prefix-To-Path() { 
        
               $OldPath = $Env:PATH; 
        
               if ($Env:_CE_CONDA -eq '' -And $Env:OS -eq 'Windows_NT') { 
        
                   # Windows has a different layout for the python exe than other platforms. 
        
                   $sysp = Split-Path $Env:CONDA_EXE -Parent; 
        
               } else { 
        
                   $sysp = Split-Path $Env:CONDA_EXE -Parent; 
        
                   $sysp = Split-Path $sysp -Parent; 
        
               } 
        
               if ($Env:OS -eq 'Windows_NT') { 
        
                   $Env:PATH = $sysp + ';' + 
        
                               $sysp + '\Library\mingw-w64\bin;' + 
        
                               $sysp + '\Library\usr\bin;' + 
        
                               $sysp + '\Library\bin;' + 
        
                               $sysp + '\Scripts;' + 
        
                               $sysp + '\bin;' + $Env:PATH; 
        
               } else { 
        
                   $Env:PATH = $sysp + '/bin:' + $Env:PATH; 
        
               } 
        
               return $OldPath; 
        
           }

After digging through conda.activate and the git history, it appears that the prefix injection code was first introduced in conda 4.4 (April 2017), @kalefranz sums it up well in comments that have unfortunately not withstood the test of time:

conda/conda/activate.py

Lines 219 to 225 in c6e8d6a

    
           def _get_starting_path_list(self): 
        
               path = os.environ['PATH'] 
        
               if on_win: 
        
                   # on Windows, the python interpreter prepends sys.prefix\Library\bin on startup WTF 
        
                   return path.split(os.pathsep)[1:] 
        
               else: 
        
                   return path.split(os.pathsep)

Subsequent changes indicate that this injection on Python startup was actually a Python 3.6 feedstock patch which appears to have been long since removed (AnacondaRecipes/python-feedstock@ef17e17).

In conda 4.6.9 (March 2019) the Python package patching was instead replaced (ca77c9c) with the shell interface prefix injection that we see today.

Originally all of this prefix injection was unique to Windows but eventually in conda 4.6.13 (April 2019) it was mirrored over to *nix systems (8ec72dd) which is where we find ourselves now, still wondering WTF.

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
~~Add / update outdated documentation?~~

jaimergp · 2022-08-01T08:18:59Z

I think it has to do with how DLL loading works on Windows. DLLs have to be on PATH, and the Library path is a conda artifact that is not usually part of the standard Python path. I think all of this helps Windows users running Python without having to activate the environment, but at this point I am just guessing.

kenodegard · 2022-08-01T15:12:44Z

@jaimergp probably but I assume something has improved in this regard since the patch has been removed and since other versions of the Python package don't appear to have this patch

jaimergp · 2022-08-01T15:24:56Z

I think it was removed in 3.9.something and 3.10 (at least in conda-forge).

kenodegard · 2022-08-01T15:31:21Z

I see that CF's Python 3.6 still has the patch, the other versions do not

jezdez

Comments not withstanding, let's merge this as-is and create a follow-up issue to fix the Windows CI setup and/or add support for the tests that are currently not working in the CI setup. It's not ideal, but as long as the manual tests worked, that's better than nothing.

I'd be interested to hear what the VS Code people think abut this, maybe they could try the Canary build of this?

Nice sleuthing @kenodegard!! 🥳

conda/activate.py

jezdez · 2022-08-12T10:17:57Z

conda/shell/etc/profile.d/conda.csh

            if( $conda_tmp_status != 0 ) exit ${conda_tmp_status}
            eval "${ask_conda}"
            rehash
            breaksw
        default:
            $_CONDA_EXE $argv[1-]
-            setenv PATH "$conda_tmp_path"
+            # setenv PATH "$conda_tmp_path"


@kenodegard Didn't you want to clean these things up before merge?

oops missed this one

kenodegard requested a review from a team as a code owner July 28, 2022 03:02

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jul 28, 2022

travishathaway added the source::anaconda created by members of Anaconda, Inc. label Jul 28, 2022

kenodegard force-pushed the prefix-injection branch 5 times, most recently from 227ec1d to a6d66e3 Compare August 4, 2022 19:57

jezdez previously approved these changes Aug 8, 2022

View reviewed changes

conda/activate.py Outdated Show resolved Hide resolved

kenodegard dismissed jezdez’s stale review via 23705cc August 9, 2022 17:42

kenodegard force-pushed the prefix-injection branch from a6d66e3 to 23705cc Compare August 9, 2022 17:42

kenodegard added 4 commits August 9, 2022 14:46

Remove prefix injection

6bfb20a

Add tests

49b1efe

Remove comments

f7210dc

Install updated conda scripts before using

9903534

kenodegard force-pushed the prefix-injection branch from b5e5f77 to 9903534 Compare August 9, 2022 19:46

kenodegard added 4 commits August 9, 2022 19:55

Use POSIX source

f898c9a

Use more primitive types

99fe38b

Add more tests

1f7214b

Annoying fix

f29a46a

jezdez approved these changes Aug 10, 2022

View reviewed changes

kenodegard merged commit c6ead1b into conda:main Aug 10, 2022

kenodegard deleted the prefix-injection branch August 10, 2022 19:02

kenodegard mentioned this pull request Aug 11, 2022

Debugging borked builds #11720

Merged

3 tasks

jezdez reviewed Aug 12, 2022

View reviewed changes

kenodegard mentioned this pull request Aug 12, 2022

Remove CSH prefix injection #11722

Merged

3 tasks

kenodegard mentioned this pull request Aug 16, 2022

SPIKE: Investigate python -m usage conda/conda-build#4561

Closed

isuruf mentioned this pull request Nov 5, 2022

CondaSSLError: OpenSSL appears to be unavailable on this machine. #11795

Closed

mbargull mentioned this pull request Dec 9, 2022

Conda update failing on git bash (libmamba-solver issue) #12161

Closed

kenodegard mentioned this pull request Jan 12, 2023

Removing prefix injection breaks DLL loading if the Python DLL patch is missing #12240

Open

github-actions bot added the locked [bot] locked due to inactivity label Aug 13, 2023

github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Resolve prefix injection #11666

Resolve prefix injection #11666

Uh oh!

kenodegard commented Jul 28, 2022 •

edited

Loading

Uh oh!

jaimergp commented Aug 1, 2022

Uh oh!

kenodegard commented Aug 1, 2022

Uh oh!

jaimergp commented Aug 1, 2022

Uh oh!

kenodegard commented Aug 1, 2022

Uh oh!

jezdez left a comment

Uh oh!

Uh oh!

jezdez Aug 12, 2022

Uh oh!

kenodegard Aug 12, 2022

Uh oh!

kenodegard Aug 12, 2022

Uh oh!

Uh oh!

	__add_sys_prefix_to_path() {
	# In dev-mode CONDA_EXE is python.exe and on Windows
	# it is in a different relative location to condabin.
	if [ -n "${_CE_CONDA}" ] && [ -n "${WINDIR+x}" ]; then
	SYSP=$(\dirname "${CONDA_EXE}")
	else
	SYSP=$(\dirname "${CONDA_EXE}")
	SYSP=$(\dirname "${SYSP}")
	fi

	if [ -n "${WINDIR+x}" ]; then
	PATH="${SYSP}/bin:${PATH}"
	PATH="${SYSP}/Scripts:${PATH}"
	PATH="${SYSP}/Library/bin:${PATH}"
	PATH="${SYSP}/Library/usr/bin:${PATH}"
	PATH="${SYSP}/Library/mingw-w64/bin:${PATH}"
	PATH="${SYSP}:${PATH}"
	else
	PATH="${SYSP}/bin:${PATH}"
	fi
	\export PATH
	}

	__conda_exe() (
	__add_sys_prefix_to_path
	"$CONDA_EXE" $_CE_M $_CE_CONDA "$@"
	)

	set conda_tmp_path="$PATH"
	setenv PATH "`dirname ${_CONDA_EXE}`:$PATH"

	<#
	.SYNOPSIS
	Adds the entries of sys.prefix to PATH and returns the old PATH.

	.EXAMPLE
	$OldPath = Add-Sys-Prefix-To-Path
	#>
	function Add-Sys-Prefix-To-Path() {
	$OldPath = $Env:PATH;
	if ($Env:_CE_CONDA -eq '' -And $Env:OS -eq 'Windows_NT') {
	# Windows has a different layout for the python exe than other platforms.
	$sysp = Split-Path $Env:CONDA_EXE -Parent;
	} else {
	$sysp = Split-Path $Env:CONDA_EXE -Parent;
	$sysp = Split-Path $sysp -Parent;
	}
	if ($Env:OS -eq 'Windows_NT') {
	$Env:PATH = $sysp + ';' +
	$sysp + '\Library\mingw-w64\bin;' +
	$sysp + '\Library\usr\bin;' +
	$sysp + '\Library\bin;' +
	$sysp + '\Scripts;' +
	$sysp + '\bin;' + $Env:PATH;
	} else {
	$Env:PATH = $sysp + '/bin:' + $Env:PATH;
	}
	return $OldPath;
	}

	def _get_starting_path_list(self):
	path = os.environ['PATH']
	if on_win:
	# on Windows, the python interpreter prepends sys.prefix\Library\bin on startup WTF
	return path.split(os.pathsep)[1:]
	else:
	return path.split(os.pathsep)

Resolve prefix injection #11666

Resolve prefix injection #11666

Uh oh!

Conversation

kenodegard commented Jul 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist - did you ...

Uh oh!

jaimergp commented Aug 1, 2022

Uh oh!

kenodegard commented Aug 1, 2022

Uh oh!

jaimergp commented Aug 1, 2022

Uh oh!

kenodegard commented Aug 1, 2022

Uh oh!

jezdez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jezdez Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

kenodegard Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

kenodegard Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kenodegard commented Jul 28, 2022 •

edited

Loading