Skip to content

Conversation

@marcoesters
Copy link
Contributor

Description

Convert simple script and directory subcommands into NSIS code to further decouple the python dependency from EXE installers (#549). Scripts are already executed directly via cmd.exe for pre-install scripts and mkdirs is a simple directory creation command.

Note that this means that post-install and pre-uninstall script failures will cause the installation/uninstallation process to fail now. I think this is good behavior as it has masked issues in the past (see #942 and #1020).

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?

@github-project-automation github-project-automation bot moved this to 🆕 New in 🔎 Review Sep 18, 2025
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Sep 18, 2025
@marcoesters marcoesters marked this pull request as ready for review September 19, 2025 00:17
@marcoesters marcoesters requested a review from a team as a code owner September 19, 2025 00:17
@lrandersson
Copy link
Contributor

@marcoesters Is this something that is awaiting review? Let me know and I can review it.

@marcoesters
Copy link
Contributor Author

@marcoesters Is this something that is awaiting review? Let me know and I can review it.

You are free to leave comments, but in the end, I need approval from a constructor maintainer.

call un.AbortRetryNSExecWait
${EndIf}
!insertmacro AbortRetryNSExecWaitLibNsisCmd "rmpath"
!insertmacro AbortRetryNSExecWaitLibNsisCmd "rmreg"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this no longer needed?

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 was actually never needed. The uninstallation via conda-standalone already performs the same function during the installation process. I included that line back then by mistake.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, thanks!

jaimergp
jaimergp previously approved these changes Oct 2, 2025
Copy link
Contributor

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

Nice cleanups! Thanks!

@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Approved in 🔎 Review Oct 2, 2025
@marcoesters
Copy link
Contributor Author

I found a way to remove the rmreg command with uninstallers that don't use conda-standalone. I think this should be part of this PR.

@marcoesters marcoesters requested a review from jaimergp October 3, 2025 16:41
@marcoesters
Copy link
Contributor Author

${EndIf}
!insertmacro AbortRetryNSExecWaitLibNsisCmd "rmpath"
{%- if has_conda %}
${If} ${FileExists} "$INSTDIR\.nonadmin"
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional:
In the Function un.onInit we also have this logic:

    ${If} ${FileExists} "$INSTDIR\.nonadmin"
        SetShellVarContext Current
    ${Else}
        SetShellVarContext All
    ${EndIf}

If you prefer, you can create a new variable (maybe un.IsUserInstall?) and assign that variable within that function, to reduce the number of if .nonadmin exists.

I have no strong opinion though, I also like the simplicity with simply using $R0 as in your commit.

# that DOSKEY (called by conda_hook.bat) is not a valid command.
# While the operation still succeeds, this error is confusing.
# Calling via cmd.exe fixes that.
push '"$CMD_EXE" /D /C "$INSTDIR\condabin\conda.bat" init cmd.exe --reverse --$R0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
push '"$CMD_EXE" /D /C "$INSTDIR\condabin\conda.bat" init cmd.exe --reverse --$R0'
push '"$CMD_EXE" /D /C "$INSTDIR\condabin\conda.bat" init --reverse --$R0'

Should we revert for both cmd and powershell (which is achieved by passing no positional arguments)?

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 powershell function does not perform path validation, so it will also revert profiles that point to a different installation. The cmd.exe function, on the other hand, only reverts the autorun entry that belongs to the installation.

@marcoesters marcoesters merged commit 74f6056 into conda:main Oct 14, 2025
20 checks passed
@github-project-automation github-project-automation bot moved this from ✅ Approved to 🏁 Done in 🔎 Review Oct 14, 2025
@marcoesters marcoesters deleted the port-nsispy-cmds-nsis branch October 14, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed [bot] added once the contributor has signed the CLA

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants