Autodesk: Build with zlib static library

[erikaharrison-adsk]

Description of Change(s)

We need to be able to support using a static zlib library. This change provides that.

Fixes Issue(s)

• N/A

• I have verified that all unit tests pass with the proposed changes

• I have submitted a signed Contributor License Agreement

[jesschimein]

Filed as internal issue #USD-9779

[jesschimein]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[dgovil]

A related issue is that we only need Zlib to build on Windows. Does this affect your PR at all? #2988

[yangel2017]

Yes, #2988 will affect my PR. Using static zlib need to build zlib from the source but not from the system installed one. If #2988 merges first, my PR needs to update.

[dgovil]

Would you only need to support static builds on Windows? Or would you need to support them on Linux and Mac too, where the OS vends it?

[yangel2017]

We would like to support static builds on Windows, Mac and Linux.

[dgovil]

Okay. Just a few more questions:

1.If I understand the change correctly, this would make it always statically linked. I think many of us would still prefer to use dynamic linking, especially on platforms where the OS provides it.

2. Zlib is used by multiple dependencies, that all default to being dynamically linked in. Wouldn't this PR cause each of them to statically link Zlib, resulting in multiple runtime copies of it?

3. I presume you also bundle those dependencies into your products.
   Would it be better for you to link against the version of each dependency in your product instead, rather than having USD build them again for you?

[yangel2017]

We propose this because when an application uses usd with dynamic zlib library of version A, and the application itself also dynamic links zlib libraries with version B, if version A and B are not compatible then the problem occurs. Use static libraries can avoid the problem. But it indeed may result in multiple runtime copies of it.

[dgovil]

I think this would need to be an option perhaps? I don't agree with the approach of making this a default as I think being dynamically provided by the OS is the more preferable option for a wider range of uses.

[asluk]

I agree that it needs to be optional 🙏🏼

[meshula]

I think the least complex and most robust solution is that we add an option in build_usd.py to not automatically build zlib. Then you can set up your environment to suit. build_usd.py is optionally building dependencies that require zlib ~ OCIO, OIIO, Ptex, and OpenVdb. All of these packages find zlib in their own manner, eg, for Ptex, https://github.com/wdas/ptex/blob/054047d02b9e06e690420b407114d2872435b953/CMakeLists.txt#L36 ~ so if build_usd.py doesn't build zlib, a zlib in the environment properly installed to satisfy cmake's find_package utility, will take precedence.

zlib is easily built and commonly available, and allowing the use of an existing zlib would allow each user to deal with it as they like, while still allowing an option to build it as a convenience when that makes sense.

I think this would solve for the concerns raised in this thread.

PS: like this

diff --git a/build_scripts/build_usd.py b/build_scripts/build_usd.py
index 6d4e8feb0..25954242e 100644
--- a/build_scripts/build_usd.py
+++ b/build_scripts/build_usd.py
@@ -682,22 +682,30 @@ def AnyPythonDependencies(deps):
 ZLIB_URL = "https://github.com/madler/zlib/archive/v1.2.13.zip"
 
 def InstallZlib(context, force, buildArgs):
-    with CurrentWorkingDirectory(DownloadURL(ZLIB_URL, context, force)):
-        # The following test files aren't portable to embedded platforms.
-        # They're not required for use on any platforms, so we elide them
-        # for efficiency
-        PatchFile("CMakeLists.txt",
-                  [("add_executable(example test/example.c)",
-                    ""),
-                   ("add_executable(minigzip test/minigzip.c)",
-                    ""),
-                   ("target_link_libraries(example zlib)",
-                    ""),
-                   ("target_link_libraries(minigzip zlib)",
-                    ""),
-                   ("add_test(example example)",
-                    "")])
-        RunCMake(context, force, buildArgs)
+    # Linux provides zlib. Skipping it here avoids issues where a host 
+    # application loads a different version of zlib than the one we build against.
+    # Building zlib is the default when a dependency requires it, although
+    # OpenUSD itself does not require it. If you dont want to automatically
+    # install zlib, the --no-zlib flag can be passed to the build script.
+    # In this case, the dependency that requires zlib will use its own mechanisms
+    # to find zlib, allowing a developer to supply their own zlib.
+    if context.buildZlib and not Linux():
+        with CurrentWorkingDirectory(DownloadURL(ZLIB_URL, context, force)):
+            # The following test files aren't portable to embedded platforms.
+            # They're not required for use on any platforms, so we elide them
+            # for efficiency
+            PatchFile("CMakeLists.txt",
+                    [("add_executable(example test/example.c)",
+                        ""),
+                    ("add_executable(minigzip test/minigzip.c)",
+                        ""),
+                    ("target_link_libraries(example zlib)",
+                        ""),
+                    ("target_link_libraries(minigzip zlib)",
+                        ""),
+                    ("add_test(example example)",
+                        "")])
+            RunCMake(context, force, buildArgs)
 
 ZLIB = Dependency("zlib", InstallZlib, "include/zlib.h")
         
@@ -2071,6 +2079,12 @@ subgroup.add_argument("--usdview", dest="build_usdview",
 subgroup.add_argument("--no-usdview", dest="build_usdview",
                       action="store_false", 
                       help="Do not build usdview")
+subgroup.add_argument("--zlib", dest="build_zlib", 
+                      action="store_true", default=True,
+                      help="Install zlib on behalf of dependencies")
+subgroup.add_argument("--no-zlib", dest="build_zlib",
+                      action="store_false",
+                      help="Do not install zlib for dependencies (default)")
 
 group = parser.add_argument_group(title="Imaging Plugin Options")
 subgroup = group.add_mutually_exclusive_group()
@@ -2291,6 +2305,9 @@ class InstallContext:
         self.buildUsdview = (self.buildUsdImaging and 
                              self.buildPython and 
                              args.build_usdview)
+        
+        # - zlib
+        self.buildZlib = args.build_zlib
 
         # - Imaging plugins
         self.buildEmbree = self.buildImaging and args.build_embree
@@ -2361,7 +2378,7 @@ if extraPythonPaths:
 if context.buildOneTBB:
     TBB = ONETBB
 
-requiredDependencies = [ZLIB, TBB]
+requiredDependencies = [TBB]
 
 if context.buildBoostPython:
     requiredDependencies += [BOOST]
@@ -2379,18 +2396,18 @@ if context.buildMaterialX:
 
 if context.buildImaging:
     if context.enablePtex:
-        requiredDependencies += [PTEX]
+        requiredDependencies += [PTEX, ZLIB]
 
     requiredDependencies += [OPENSUBDIV]
 
     if context.enableOpenVDB:
-        requiredDependencies += [BLOSC, BOOST, OPENEXR, OPENVDB, TBB]
+        requiredDependencies += [BLOSC, BOOST, OPENEXR, OPENVDB, TBB, ZLIB]
     
     if context.buildOIIO:
-        requiredDependencies += [BOOST, JPEG, TIFF, PNG, OPENEXR, OPENIMAGEIO]
+        requiredDependencies += [BOOST, JPEG, TIFF, PNG, OPENEXR, OPENIMAGEIO, ZLIB]
 
     if context.buildOCIO:
-        requiredDependencies += [OPENCOLORIO]
+        requiredDependencies += [OPENCOLORIO, ZLIB]
 
     if context.buildEmbree:
         requiredDependencies += [TBB, EMBREE]
@@ -2401,13 +2418,6 @@ if context.buildUsdview:
 if context.buildAnimXTests:
     requiredDependencies += [ANIMX]
 
-# Assume zlib already exists on Linux platforms and don't build
-# our own. This avoids potential issues where a host application
-# loads an older version of zlib than the one we'd build and link
-# our libraries against.
-if Linux():
-    requiredDependencies.remove(ZLIB)
-
 # Error out if user is building monolithic library on windows with draco plugin
 # enabled. This currently results in missing symbols.
 if context.buildDraco and context.buildMonolithic and Windows():

[asluk]

I think @meshula 's approach makes sense and keeps things simple-- we might want some supplemental docs to go over some common setups, to help guide developers as to what options / combination of options might work best for their software stacks.

[yangel2017]

Yes, the above makes sense but I think it's out of the scope of this PR. In this PR, we focus on building with zlib static library rather than dynamic library. It may be more related with #2988 to control whether build zlib?