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

Upgrade NSIS to v3.08 #2208

Merged
merged 1 commit into from
Jan 8, 2022
Merged

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Jan 2, 2022

Short description of changes

Upgrade NSIS to 3.08.

Context: Fixes an issue?
Might fix autobuild failures. But I doubt it...

Does this change need documentation? What needs to be documented and how?

No

Status of this Pull Request
Not yet fully tested.

What is missing until this pull request can be merged?
Installation on Windows

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@henkdegroot
Copy link
Contributor

@ann0see, I noticed the comment in the deploy_windows.ps1 script related to the redirect URL for sourceforge downloads.
The link in the powershell takes me to an article at powershellmagazine. Here there is also a comment placed with a reference to a fix. I.e. that you need to include a "close" to prevent time-out to occur. Perhaps this is what is missing in our script?
When I tried the current function in our script a few times, I indeed get to a point that I don't get the redirected URL.
When I use the function as provided in the comment, I can call as many times as I want and always get the redirected URL.

It may have been luck this time...but sounds like that could be an improvement to test?
Another thing which probably can be build in the script is a check to see if there was actually something downloaded or not and then perform a retry if not?

@ann0see
Copy link
Member Author

ann0see commented Jan 7, 2022

If this modification solves the issue it would be good to implement that. However we might also host the NSIS file in jamulussoftware/assets

Copy link
Member

@softins softins left a comment

Choose a reason for hiding this comment

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

Looks fine to me, and I've done a test download from the URL. Plus the CI is happy.

@softins
Copy link
Member

softins commented Jan 8, 2022

I have also just downloaded the Windows installer produced by the CI, and it installs without issues. This can probably be marked as Ready for review.

@ann0see ann0see marked this pull request as ready for review January 8, 2022 13:40
@pljones pljones merged commit f83c83c into jamulussoftware:master Jan 8, 2022
@pljones pljones modified the milestone: Release 3.8.2 Feb 13, 2022
@ann0see ann0see deleted the upgrade/updateNsis branch March 23, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants