Skip to content

Remove custom python commands from EXE installers #1089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Remove custom python commands from EXE installers #1089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
31e99e3
Remove python from base environment where possible
marcoesters Oct 6, 2025
4b71bb6
Fix NSIS template
marcoesters Oct 6, 2025
6234d6c
Add news file
marcoesters Oct 16, 2025
65f4c94
Only exit with error if base needs python
marcoesters Oct 16, 2025
9eed660
Remove Python logic from NSIS if Python is not in the base environment
marcoesters Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions constructor/fcp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,9 @@ def _solve_precs(
conda_exe="conda.exe",
extra_env=False,
input_dir="",
base_needs_python=True,
):
# Add python to specs, since all installers need a python interpreter. In the future we'll
# probably want to add conda too.
# JRG: This only applies to the `base` environment; `extra_envs` are exempt
if not extra_env:
if not extra_env and base_needs_python:
specs = (*specs, "python")
if environment:
logger.debug("specs: <from existing environment '%s'>", environment)
Expand Down Expand Up @@ -312,7 +310,7 @@ def _solve_precs(
if python_prec:
precs.remove(python_prec)
precs.insert(0, python_prec)
elif not extra_env:
elif not extra_env and base_needs_python:
# the base environment must always have python; this has been addressed
# at the beginning of _main() but we can still get here through the
# environment_file option
Expand Down Expand Up @@ -392,6 +390,7 @@ def _main(
extra_envs=None,
check_path_spaces=True,
input_dir="",
base_needs_python=True,
):
precs = _solve_precs(
name,
Expand All @@ -408,6 +407,7 @@ def _main(
verbose=verbose,
conda_exe=conda_exe,
input_dir=input_dir,
base_needs_python=base_needs_python,
)
extra_envs = extra_envs or {}
conda_in_base: PackageCacheRecord = next((prec for prec in precs if prec.name == "conda"), None)
Expand Down Expand Up @@ -496,6 +496,7 @@ def main(info, verbose=True, dry_run=False, conda_exe="conda.exe"):
transmute_file_type = info.get("transmute_file_type", "")
extra_envs = info.get("extra_envs", {})
check_path_spaces = info.get("check_path_spaces", True)
base_needs_python = info.get("_base_needs_python", True)

if not channel_urls and not channels_remap and not (environment or environment_file):
sys.exit("Error: at least one entry in 'channels' or 'channels_remap' is required")
Expand Down Expand Up @@ -548,6 +549,7 @@ def main(info, verbose=True, dry_run=False, conda_exe="conda.exe"):
extra_envs,
check_path_spaces,
input_dir,
base_needs_python,
)

info["_all_pkg_records"] = pkg_records # full PackageRecord objects
Expand Down
27 changes: 26 additions & 1 deletion constructor/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,26 @@
import json
import logging
import os
import subprocess
import sys
from os.path import abspath, expanduser, isdir, join
from pathlib import Path
from textwrap import dedent
from typing import TYPE_CHECKING

from . import __version__
from .build_outputs import process_build_outputs
from .conda_interface import SUPPORTED_PLATFORMS, cc_platform
from .conda_interface import SUPPORTED_PLATFORMS, MatchSpec, cc_platform
from .conda_interface import VersionOrder as Version
from .construct import SCHEMA_PATH, ns_platform
from .construct import parse as construct_parse
from .construct import verify as construct_verify
from .fcp import main as fcp_main
from .utils import StandaloneExe, check_version, identify_conda_exe, normalize_path, yield_lines

if TYPE_CHECKING:
from typing import Any

DEFAULT_CACHE_DIR = os.getenv("CONSTRUCTOR_CACHE", "~/.conda/constructor")

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -76,6 +81,24 @@ def get_output_filename(info):
)


def _base_needs_python(info: dict[str, Any]) -> bool:
if sys.platform == "win32" and info.get("_conda_exe_type") == StandaloneExe.CONDA:
conda_exe = info["_conda_exe"]
results = subprocess.run(
[conda_exe, "constructor", "windows", "--help"],
capture_output=True,
check=False,
)
if results.returncode == 2:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, could we add a comment here explaining what returncode 2 means?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

return True
specs = {MatchSpec(spec) for spec in info.get("specs", ())}
# mamba 2 and newer does not need Python. However, without running the solver,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should assume we'd get mamba 2.x here. We can add an additional post-solve check to make sure we didn't use mamba 1.x.

# we won't know for sure which version of mamba is requested
if sys.platform == "linux" and "mamba" in {spec.name for spec in specs}:
return True
return False


def main_build(
dir_path,
output_dir=".",
Expand Down Expand Up @@ -275,6 +298,8 @@ def is_conda_meta_frozen(path_str: str) -> bool:
"enable_currentUserHome": "true",
}

info["_base_needs_python"] = _base_needs_python(info)

info["installer_type"] = itypes[0]
fcp_main(info, verbose=verbose, dry_run=dry_run, conda_exe=conda_exe)
if dry_run:
Expand Down
94 changes: 85 additions & 9 deletions constructor/nsis/main.nsi.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ var /global StdOutHandleSet
!define ARCH {{ arch }}
!define PLATFORM {{ installer_platform }}
!define CONSTRUCTOR_VERSION {{ constructor_version }}
{%- if has_python %}
!define PY_VER {{ pyver_components[:2] | join(".") }}
!define PYVERSION_JUSTDIGITS {{ pyver_components | join("") }}
!define PYVERSION {{ pyver_components | join(".") }}
!define PYVERSION_MAJOR {{ pyver_components[0] }}
{%- endif %}
!define DEFAULT_PREFIX {{ default_prefix }}
!define DEFAULT_PREFIX_DOMAIN_USER {{ default_prefix_domain_user }}
!define DEFAULT_PREFIX_ALL_USERS {{ default_prefix_all_users }}
Expand Down Expand Up @@ -298,7 +300,9 @@ FunctionEnd
/InstallationType=AllUsers [default: JustMe]$\n\
/AddToPath=[0|1] [default: 0]$\n\
/KeepPkgCache=[0|1] [default: {{ 1 if keep_pkgs else 0 }}]$\n\
{%- if has_python %}
/RegisterPython=[0|1] [default: AllUsers: 1, JustMe: 0]$\n\
{%- endif %}
/NoRegistry=[0|1] [default: AllUsers: 0, JustMe: 0]$\n\
/NoScripts=[0|1] [default: 0]$\n\
/NoShortcuts=[0|1] [default: 0]$\n\
Expand All @@ -317,8 +321,13 @@ FunctionEnd
Install for all users, but don't add to PATH env var:$\n\
> $EXEFILE /InstallationType=AllUsers$\n\
$\n\
{%- if has_python %}
Install for just me, add to PATH and register as system Python:$\n\
> $EXEFILE /RegisterPython=1 /AddToPath=1$\n\
{%- else %}
Install for just me and add to PATH:$\n\
> $EXEFILE /AddToPath=1$\n\
{%- endif %}
Comment on lines +327 to +330
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this option depend on having Python or not? I'd say the are independent:

Suggested change
{%- else %}
Install for just me and add to PATH:$\n\
> $EXEFILE /AddToPath=1$\n\
{%- endif %}
{%- endif %}
Install for just me and add to PATH:$\n\
> $EXEFILE /AddToPath=1$\n\

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't depend on Python, no, but I figured I'd not add an extra line to the output since it already has to be broken apart.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it already has to be broken apart.

I don't get this part. Are they mutually exclusive or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not mutually exclusive, I just didn't want the message to be overly long with examples. I was referring to the comment a few lines above:

        # There seems to be a limit to how many chars per ${Print} we can pass.
        # The message will get truncated silently, no errors.
        # That's why we split the help message in two calls.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case we can split the help message in one more call. I'd rather leave the examples there.

$\n\
Install for just me, with no registry modification (for CI):$\n\
> $EXEFILE /NoRegistry=1$\n\
Expand All @@ -343,6 +352,7 @@ FunctionEnd
${EndIf}

ClearErrors
{%- if has_python %}
${GetOptions} $ARGV "/RegisterPython=" $ARGV_RegisterPython
${IfNot} ${Errors}
${If} $ARGV_RegisterPython = "1"
Expand All @@ -351,6 +361,7 @@ FunctionEnd
StrCpy $Ana_RegisterSystemPython_State ${BST_UNCHECKED}
${EndIf}
${EndIf}
{%- endif %}

ClearErrors
${GetOptions} $ARGV "/KeepPkgCache=" $ARGV_KeepPkgCache
Expand Down Expand Up @@ -1136,6 +1147,7 @@ Function OnDirectoryLeave
UnicodePathTest::UnicodePathTest $INSTDIR
Pop $R1

{%- if has_python %}
# Python 3 can be installed in a CP_ACP path until MKL is Unicode capable.
# (mkl_rt.dll calls LoadLibraryA() to load mkl_intel_thread.dll)
# Python 2 can only be installed to an ASCII path.
Expand All @@ -1153,6 +1165,7 @@ Function OnDirectoryLeave
abort

valid_path:
{%- endif %}

Push $R1
${IsWritable} $INSTDIR $R1
Expand Down Expand Up @@ -1235,6 +1248,36 @@ FunctionEnd
!insertmacro AbortRetryNSExecWaitMacro ""
!insertmacro AbortRetryNSExecWaitMacro "un."

{% if not base_needs_python %}
!macro AddRemovePath add_remove un
{%- if initialize_conda == 'condabin' %}
push '"$INSTDIR\_conda.exe" constructor windows path --${add_remove}=user --prefix "$INSTDIR\condabin"'
push 'Failed to add condabin to PATH'
push 'WithLog'
call ${un}AbortRetryNSExecWait
{%- else %}
StrCpy $R0 "Library\mingw-w64\bin Library\usr\bin Library\bin Scripts"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we didn't need to hardcode this here. Wouldn't it be more adequate to have this logic in conda-standalone too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has to be hard-coded somewhere. I made the choice to have an abstract feature in conda-standalone and let the installer implementation decide which paths to add. Do you think it's better to have conda-standalone do the special-casing?

Copy link
Contributor

@jaimergp jaimergp Oct 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that these directories are defined by convention (although we're trying to standardize it here when building packages, so if at some point we decide to put binaries somewhere else in Windows (e.g. removing the Library/ bit), this hardcoded list might be too far removed from lists defined in other places like conda or other CLIs.

A future addition might be an arm64 implementation of mingw, which would need to be added here.

That said, we can address that in a future refactor. For now let's link to the CEP draft.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little concerned to leave this to a future refactor. If this gets moved into conda-standalone, we have to do yet another version check and adjust the template. I think I would rather implement this in conda-standalone then, which is probably much better at architecture detection than NSIS.

StrCpy $R1 0
loop_add_remove_path:
# Add INSTDIR first
${If} $R1 == 0
StrCpy $R2 "$INSTDIR"
${Else}
${WordFind} $R0 " " "E+$R1" $R2
IfErrors endloop_add_remove_path
StrCpy $R2 "$INSTDIR\$R2"
${EndIf}
push '"$INSTDIR\_conda.exe" constructor windows path --${add_remove}=user --prefix "$R2"'
push 'Failed to add $R2 to PATH'
push 'WithLog'
call ${un}AbortRetryNSExecWait
IntOp $R1 $R1 + 1
goto loop_add_remove_path
endloop_add_remove_path:
{% endif %}
!macroend
{% endif %}

# Installer sections
Section "Install"
${LogSet} on
Expand All @@ -1244,9 +1287,12 @@ Section "Install"

!insertmacro FindCmdExe

SetOutPath "$INSTDIR"
{% if base_needs_python %}
SetOutPath "$INSTDIR\Lib"
File "{{ NSIS_DIR }}\_nsis.py"
File "{{ NSIS_DIR }}\_system_path.py"
{% endif %}

# Resolve INSTDIR so that paths and registry keys do not contain '..' or similar strings.
# $0 is empty if the directory doesn't exist, but the File commands should have created it already.
Expand Down Expand Up @@ -1448,20 +1494,32 @@ Section "Install"
${EndIf}

{% if initialize_conda %}
${If} $Ana_AddToPath_State = ${BST_CHECKED}
${If} ${FileExists} "$INSTDIR\.nonadmin"
${If} $Ana_AddToPath_State = ${BST_CHECKED}
{%- if base_needs_python %}
{%- if initialize_conda == 'condabin' %}
${Print} "Adding $INSTDIR\condabin to PATH..."
push '"$INSTDIR\pythonw.exe" -E -s "$INSTDIR\Lib\_nsis.py" addcondabinpath'
${Print} "Adding $INSTDIR\condabin to PATH..."
push '"$INSTDIR\pythonw.exe" -E -s "$INSTDIR\Lib\_nsis.py" addcondabinpath'
{%- else %}
${Print} "Adding $INSTDIR\Scripts & Library\bin to PATH..."
push '"$INSTDIR\pythonw.exe" -E -s "$INSTDIR\Lib\_nsis.py" addpath ${PYVERSION} ${NAME} ${VERSION} ${ARCH}'
${Print} "Adding $INSTDIR\Scripts & Library\bin to PATH..."
push '"$INSTDIR\pythonw.exe" -E -s "$INSTDIR\Lib\_nsis.py" addpath ${PYVERSION} ${NAME} ${VERSION} ${ARCH}'
{%- endif %}
push 'Failed to add {{ NAME }} to PATH'
push 'WithLog'
call AbortRetryNSExecWait
push 'Failed to add {{ NAME }} to PATH'
push 'WithLog'
call AbortRetryNSExecWait
Comment on lines +1499 to +1509
Copy link
Contributor

@jaimergp jaimergp Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we implement this twice? Can't we just depend on the new NSIS macro?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting that I move the if base_needs_python logic into the macro instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can understand, the AddRemovePath macro already implements the same behavior as '"$INSTDIR\pythonw.exe" -E -s "$INSTDIR\Lib\_nsis.py" addcondabinpath | addpath', so I'm wondering whether we need to keep both. If the macro is sufficient, then we don't need the Python-based logic, do we?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macro implements the same behavior using conda-standalone. However, that requires the conda constructor windows subcommand to be available, which is currently only the case for the canary builds. The python commands must stay available for now to ensure backwards compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, your comment makes me think that it's better to move that logic into the macro, too. I will go ahead and do that - it should clear this up a little more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. Let's leave a comment to deprecate that part once we bump the minimum conda requirement.

{%- else %}
{%- if initialize_conda == 'condabin' %}
${Print} "Adding $INSTDIR\condabin to PATH..."
{%- else %}
${Print} "Adding $INSTDIR\Scripts & Library\bin to PATH..."
{%- endif %}
!insertmacro AddRemovePath "prepend" ""
{%- endif %}
${EndIf}
${EndIf}
{%- endif %}

{%- if has_python %}
# Create registry entries saying this is the system Python
# (for this version)
!define PYREG "Software\Python\PythonCore\${PY_VER}"
Expand All @@ -1479,6 +1537,7 @@ Section "Install"
WriteRegStr SHCTX "${PYREG}\PythonPath" \
"" "$INSTDIR\Lib;$INSTDIR\DLLs"
${EndIf}
{%- endif %}

${If} $ARGV_NoRegistry == "0"
# Registry uninstall info
Expand Down Expand Up @@ -1532,6 +1591,7 @@ Section "Install"
${Print} "Done!"
SectionEnd

{% if base_needs_python %}
!macro AbortRetryNSExecWaitLibNsisCmd cmd
SetDetailsPrint both
${Print} "Running ${cmd} scripts..."
Expand All @@ -1546,6 +1606,7 @@ SectionEnd
call un.AbortRetryNSExecWait
SetDetailsPrint both
!macroend
{% endif %}

Section "Uninstall"
${LogSet} on
Expand Down Expand Up @@ -1617,7 +1678,13 @@ Section "Uninstall"
push 'WithLog'
call un.AbortRetryNSExecWait
${EndIf}
{% if base_needs_python %}
!insertmacro AbortRetryNSExecWaitLibNsisCmd "rmpath"
{% else %}
${If} ${FileExists} "$INSTDIR\.nonadmin"
!insertmacro AddRemovePath "remove" "un."
${EndIf}
{% endif %}

# Parse arguments
StrCpy $R0 ""
Expand Down Expand Up @@ -1672,7 +1739,11 @@ Section "Uninstall"
push 'WithLog'
call un.AbortRetryNSExecWait
${EndIf}
!insertmacro AbortRetryNSExecWaitLibNsisCmd "rmpath"
${If} ${FileExists} "$INSTDIR\.nonadmin"
StrCpy $R0 "user"
${Else}
StrCpy $R0 "system"
${EndIf}
{%- if has_conda %}
${If} ${FileExists} "$INSTDIR\.nonadmin"
StrCpy $R0 "user"
Expand All @@ -1688,6 +1759,11 @@ Section "Uninstall"
push 'WithLog'
call un.AbortRetryNSExecWait
{%- endif %}
{% if base_needs_python %}
!insertmacro AbortRetryNSExecWaitLibNsisCmd "rmpath"
{% else %}
!insertmacro AddRemovePath "remove" "un."
{% endif %}

${Print} "Removing files and folders..."
nsExec::Exec 'cmd.exe /D /C RMDIR /Q /S "$INSTDIR"'
Expand Down
5 changes: 0 additions & 5 deletions constructor/osx/run_installation.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,6 @@ find "$PREFIX/pkgs" -type d -empty -exec rmdir {} \; 2>/dev/null || :
{{ condarc }}
{%- endfor %}

if ! "$PREFIX/bin/python" -V; then
echo "ERROR running Python"
exit 1
fi

# This is not needed for the default install to ~, but if the user changes the
# install location, the permissions will default to root unless this is done.
chown -R "${USER}" "$PREFIX"
Expand Down
Loading
Loading