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

catch install-time errors on windows #553

Merged
merged 16 commits into from
Aug 16, 2022
Merged

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Aug 15, 2022

Description

Closes #544

Closes #551

In #509 I forgot to add one piece of the conda packages extraction logic. This resulted in failed installs. Readding this piece fixes #544.

However, this error wasn't caught by the CI pipeline because of #551!

Note that the silent mode of NSIS installers (/S) does not print anything to STDOUT. Additionally, most of the dialogs are configured to "ignore" the warning if an error occurred, thus moving on. The exit code does not change with errors! We can't rely on that to detect whether something failed. Instead, the solution was:

  • The CI now uses a special NSIS build with log-dumping abilities (normal NSIS does not have this feature). I want to add *log* variants to the nsis package in conda-forge/defaults, but for now we are manually patching.
  • Errors are printed to the log file, even if ignored
  • conda install ... step will throw an error too, if needed (it didn't before!)
  • The CI will inspect the log for :error: strings planted by our NSI script
  • If there were any errors, these are printed and reported the installer is reported as failing.

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?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Aug 15, 2022
@jaimergp
Copy link
Contributor Author

Finally, we have eyes on Windows :D

This is ready for review now.

@jaimergp jaimergp marked this pull request as ready for review August 16, 2022 09:22
@jaimergp jaimergp requested a review from a team as a code owner August 16, 2022 09:22
@jaimergp
Copy link
Contributor Author

jaimergp commented Aug 16, 2022

Note Windows are now failing, as reported in #544

We want to merge with failures. I'll debug 544 in a separate PR.

Edit: looks like the fix is simpler! I had forgotten a block in the NSI template.

@jaimergp jaimergp mentioned this pull request Aug 16, 2022
2 tasks
Copy link

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

LGTM +1 for merge

@hoechenberger hoechenberger merged commit a16696a into main Aug 16, 2022
@hoechenberger
Copy link
Contributor

Thanks @jaimergp!

@jaimergp jaimergp mentioned this pull request Aug 30, 2022
3 tasks
@dbast dbast deleted the fix-windows-directories branch December 8, 2022 12:01
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Dec 9, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[CI] Windows tests can pass with errors Latest multi-dev breaks windows
4 participants