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

Remove dependency on USE_ZLIB from browser wasm build #45454

Closed
3 tasks
akoeplinger opened this issue Dec 2, 2020 · 5 comments
Closed
3 tasks

Remove dependency on USE_ZLIB from browser wasm build #45454

akoeplinger opened this issue Dec 2, 2020 · 5 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-Build-mono
Milestone

Comments

@akoeplinger
Copy link
Member

akoeplinger commented Dec 2, 2020

This special flag causes the emscripten toolchain to download and build a .zip version of zlib from https://github.com/emscripten-ports/zlib which unnecessarily introduces a network dependency into the build.

We already have a zlib implementation in mono that we can use.

Note that zlib is used both inside of mono and System.Native, we need to make sure only one copy of zlib makes it into the dotnet.wasm.

Three parts to this:

@akoeplinger akoeplinger added arch-wasm WebAssembly architecture area-Build-mono labels Dec 2, 2020
@akoeplinger akoeplinger added this to the 6.0.0 milestone Dec 2, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Dec 2, 2020
@akoeplinger akoeplinger removed the untriaged New issue has not been triaged by the area owner label Dec 2, 2020
@danmoseley
Copy link
Member

We should at some point reduce the two zlibs in our tree to a single copy. Looks like they're identical versions, but the libraries one has some extra files.

@danmoseley
Copy link
Member

danmoseley commented Jan 16, 2021

For my own reference, here's what we have in the repo currently:

src\libraries\Native\Windows\System.IO.Compression.Native\zlib
from https://github.com/madler/zlib/releases/tag/v1.2.11 (=latest - Jan 2017)
Used for Arm and Arm64.

src\mono\mono\zlib
same code as above. I think this is not used in this repo. It is used by by S.IO.C in mono/mono. Do we mirror mono/mono wholesale or is there some way to avoid mirroring unnecessary parts like this?

src\libraries\Native\Windows\System.IO.Compression.Native\zlib-intel
from https://github.com/jtkukunas/zlib/tree/v1.2.11.1_jtkv6.3 (=latest - April 2019)
Used for x64/x86

@akoeplinger
Copy link
Member Author

src\mono\mono\zlib
same code as above. I think this is not used in this repo.

This is used by the mono in this repo on platforms that don't ship a system zlib like WASM or Win32.

@CoffeeFlux
Copy link
Contributor

Now that the mirror is off, it should be safe to get rid of the Mono-specific copy of zlib, assuming it's identical to the one in libraries. However, like @akoeplinger mentioned, we do it need for some platforms so we can't get rid of both copies altogether. This is somewhat separate from getting rid of USE_ZLIB for the wasm build though.

@CoffeeFlux
Copy link
Contributor

The USE_ZLIB flag is no longer set as of #48881, so closing. Will open a separate issue about linking out zlib.

We also still have duplication in the build tree, but it's not particularly urgent.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Development

No branches or pull requests

5 participants