Skip to content

[release/6.0][wasm] Remove USE_ZLIB=1 and include the sources instead - #64907

Merged
safern merged 1 commit into
dotnet:release/6.0from
akoeplinger:zlib-6.0
Feb 11, 2022
Merged

[release/6.0][wasm] Remove USE_ZLIB=1 and include the sources instead#64907
safern merged 1 commit into
dotnet:release/6.0from
akoeplinger:zlib-6.0

Conversation

@akoeplinger

@akoeplingerakoeplinger commented Feb 7, 2022

Copy link
Copy Markdown
Member

Backport of #64628 to release/6.0

/cc @lewing@radical

Customer Impact

During the dotnet/runtime build Emscripten would download and cache zlib sources for wasm builds that set USE_ZLIB=1. As we saw in #62553 this can lead the failures if that download fails and also introduces an unnecessary dependency that we can avoid since we already have the sources in the repo.

Note: in main the zlib sources were refactored to not be in a Windows-specific folder path, but for release/6.0 including them from the Windows-specific path is less churn.

Testing

Manual and CI testing.

Risk

Low. We're replacing the zlib sources that emscripten downloads with the (identical) sources we already have in the repo.

@ghost

ghost commented Feb 7, 2022

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghostghost assigned akoeplingerFeb 7, 2022
@akoeplingerakoeplinger added the Servicing-consider Issue for next servicing release review label Feb 7, 2022
@akoeplingerakoeplinger added this to the 6.0.x milestone Feb 7, 2022
@akoeplinger
akoeplinger marked this pull request as ready for review February 7, 2022 16:15
@akoeplinger
akoeplinger requested review from lewing and radical and removed request for marek-safarFebruary 7, 2022 16:15
@akoeplingerakoeplinger changed the title [wasm] Remove USE_ZLIB=1 and include the sources instead[release/6.0][wasm] Remove USE_ZLIB=1 and include the sources insteadFeb 7, 2022
@leecowleecow removed the Servicing-consider Issue for next servicing release review label Feb 10, 2022
@leecowleecow modified the milestones: 6.0.x, 6.0.3Feb 10, 2022
@danmoseley

Copy link
Copy Markdown
Contributor

Does this mean src/mono/mono/zlib/* are dead? If so there's an advantage in deleting them, because they are still scanned for hypothetical future vulnerabilities etc.

@lewing

Copy link
Copy Markdown
Member

Does this mean src/mono/mono/zlib/* are dead? If so there's an advantage in deleting them, because they are still scanned for hypothetical future vulnerabilities etc.

Unfortunately I believe that is yet another zlib.

@danmoseley

Copy link
Copy Markdown
Contributor

Ah, I see it was cleaned up in main already. 0e499ac

@lewinglewing added the Servicing-approved Approved for servicing release label Feb 11, 2022
@safern

Copy link
Copy Markdown
Member

@mmitche this PR was marked as 6.0.3 on yesterday's tactics meeting, should we still merge it?

@safern
safern merged commit a5a0004 into dotnet:release/6.0Feb 11, 2022
@akoeplinger
akoeplinger deleted the zlib-6.0 branch February 11, 2022 23:33
@ghostghost locked as resolved and limited conversation to collaborators Mar 14, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Build-monoServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@akoeplinger@danmoseley@lewing@safern@leecow@radical