-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Remove the need for intermediate .zip
file in PackageSupplier
#2876
Conversation
Geod24
commented
Feb 22, 2024
✅ PR OK, no changes in deprecations or warnings Total deprecations: 0 Total warnings: 0 Build statistics: statistics (-before, +after)
-executable size=5368480 bin/dub
-rough build time=61s
+executable size=5356064 bin/dub
+rough build time=62s Full build output
|
This fails CI |
Actually it was just broken... |
This makes the source of the issue more obvious when using the verbose flag.
We were using the default template parameter, `char`, and as the documentation for `get` states: > If asking for char, content will be converted from > the connection character set (specified in HTTP > response headers or FTP connection properties, > both ISO-8859-1 by default) to UTF-8. However we are downloading a binary so converting it to UTF-8 will fail. Especially if we intend to cast it to `ubyte[]` afterwards.
By using a different overload of `retryDownload`, we return the data instead of writing to disk, ensuring that we don't have a lingering file in memory and reducing IO. This follow the same approach for deprecations as previous `PackageSupplier` deprecations: it's an inevitable, but easily fixable, breaking change for classes that implement `PackageSupplier`, but it's a proper deprecation for client code, based on the assumption that all the classes implementing `PackageSupplier` are in Dub.
e6233b7
to
c6a12b2
Compare
Yep that did it |
@s-ludwig : Your input here would be appreciated. Do you see a reason why this is the wrong direction ? In the long run I think having the |