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

only insert custom_conclusion if the file ends in .nsi #701

Merged
merged 5 commits into from
Jul 24, 2023

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Jul 19, 2023

Description

Closes #700

A non-nsi conclusion_file (e.g. a RTF file) would leak into the NSIS template, creating syntax errors.

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 jaimergp added the os::windows relevant to Windows label Jul 19, 2023
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jul 19, 2023
@jaimergp jaimergp marked this pull request as ready for review July 20, 2023 09:48
@jaimergp jaimergp requested a review from a team as a code owner July 20, 2023 09:48
@jaimergp
Copy link
Contributor Author

@larsoner - this should fix the problems you saw in the MNE updates!

constructor/winexe.py Outdated Show resolved Hide resolved
Co-authored-by: Marco Esters <mesters@anaconda.com>
# Custom conclusion file(s)
@CUSTOM_CONCLUSION_FILE@

#else
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pretty fundamental change. To restore old behavior, our custom conclusion files will have to add !insertmacro MUI_PAGE_FINISH, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, that was always the intention IIRC, but this sneaked in. Otherwise you can't replace the last page (like the example tries to do), and you get two "Finish" pages which feels weird.

Copy link
Contributor

Choose a reason for hiding this comment

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

Two finish pages may be desirable from a marketing perspective, but I do agree that we shouldn't force a default page. I'm all for more flexibility

@jaimergp jaimergp merged commit 9c16ce1 into conda:main Jul 24, 2023
15 checks passed
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Jul 24, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2024
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 os::windows relevant to Windows
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Bug when using custom conclusion_file on Windows
3 participants