Skip to content
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.

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

mambaforge: Fix installer script #10457

Closed
wants to merge 1 commit into from

Conversation

chortbauer
Copy link
Contributor

As the 22.11.1-2 version of the mambaforge installer only installs into an empty directory, the installer is moved to the $env:TEMP directory befor installation. If the temporary path for the installer already exists, an error is thrown. After installation the installer is deleted as before.

Closes #10420

As the 22.11.1-2 version of the mambaforge installer only installs into an empty directory, the installer is moved to the $env:TEMP directory befor installation. If the temporary path for the installer already exists, an error is thrown. After installation the installer is deleted as before.
Copy link
Contributor

@c-ortbauer c-ortbauer left a comment

Choose a reason for hiding this comment

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

Full disclosure: I am the author of this PR.
I made this PR at home with my private account and tested it on Windows 11.
Now I have tested it on my work PC running Windows 10.

@rasa rasa changed the title update install script, for mambaforge 22.11.1-2 mambaforge: Fix installer script Feb 8, 2023
@jaimergp
Copy link

jaimergp commented Feb 9, 2023

NSIS (what constructor uses to produce installers on Windows, hence Mambaforge) does not change the exit code if the installation fails (i.e. it always throws 0). Would it be worth it to add post-install check? For example, testing for the existence of python.exe.

@elovelan
Copy link

@jaimergp, I'll happily write that, but hopefully, we can get this merged to unblock it!

@elovelan
Copy link

elovelan commented Feb 10, 2023

@jaimergp, the NSIS docs indicate that NSIS should be able to return a failure exit code. I'm happy to dig into this more if there's a chance that updating constructor might resolve it. I realize that may be a risky proposition however if any software is relying on it always having a return code of 0, so I'm also happy to continue down the path of your original suggestion.

@jaimergp
Copy link

I'd happily review PRs in constructor, yes! Maybe we do have to throw some SetErrorLevel calls here and there. I am not sure how broad 2 - Installation aborted by script is either, but we'll get there. Thanks!

And yea, my suggestion shouldn't block this. We might have more than one issue (see recent comment) anyway.

@elovelan
Copy link

/verify

@github-actions
Copy link
Contributor

Your changes do not pass checks.

mambaforge

  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate
  • Autoupdate Hash Extraction

Copy link
Contributor

@jasongodev jasongodev left a comment

Choose a reason for hiding this comment

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

Reminding myself to look at this again soon.

Copy link
Contributor

@jasongodev jasongodev left a comment

Choose a reason for hiding this comment

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

Please use:

"checkver": "github"

@niheaven niheaven closed this Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: mambaforge install fails after update, executable not found
6 participants