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

utils.pwsh: Add support for curl resume #232

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

RytoEX
Copy link
Member

@RytoEX RytoEX commented Jan 23, 2024

Description

Note that there is a bug in curl that can prevent curl from successfully interpreting a fully downloaded file as completed. The bug was fixed on October 30, 2023 and is not yet available on curl included with Windows.

Fixed in curl 8.5.0.
curl/curl@225db91

Motivation and Context

I noticed that we have the Resume parameter, but that it wasn't implemented. I wanted to implement it to have a way to skip re-downloading a file that was already downloaded. SkipDeps skips build dependencies. SkipUnpack skips git checkouts and archive extraction but not downloads.

Alternatively, we could add something like SkipDownload and implement that, and remove the Resume parameter.

It may be important to note that due to us invoking MSYS2, the copy of curl from there can take precedence in PATH. From my recent tests, MSYS2 curl (8.1.1 on my machine) is older than the one shipped with Windows, so this may not work unless we attempt to fix the curl executable's path to the Windows copy (which should always be at C:\Windows\System32\curl.exe) or locate it and save its path before invoking MSYS2 (with where.exe curl.exe).

How Has This Been Tested?

Tested locally with curl 8.4.0 (what is currently shipped on Windows) to ensure that the code did not currently run.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@RytoEX RytoEX added the enhancement New feature or request label Jan 23, 2024
@RytoEX RytoEX requested a review from PatTheMav January 23, 2024 23:01
@RytoEX RytoEX self-assigned this Jan 23, 2024
@RytoEX
Copy link
Member Author

RytoEX commented Jul 25, 2024

I checked today and currently Windows ships curl 8.7.1. The MSYS2 package via winget has also been updated and ships curl 8.7.1. As such, this would now work if we were interested in merging it.

Note that there is a bug in older versions of curl that can prevent curl
from successfully interpreting a fully downloaded file as completed. The
bug was fixed on October 30, 2023 and is now available on curl included
with Windows.

Fixed in curl 8.5.0.
curl/curl@225db91
@RytoEX
Copy link
Member Author

RytoEX commented Aug 1, 2024

For posterity, I checked this locally with the qt package, and it seemed to work as expected.

@RytoEX RytoEX merged commit 4196405 into obsproject:master Aug 1, 2024
21 checks passed
@RytoEX RytoEX deleted the add-curl-resume branch August 1, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants