Disable default features in libxml2.

[teo-tsirpanis]

We don't seem to make use of any of them in xml_wrapper.cpp.

Pull Request Checklist

Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:

See the detailed list in the contributing guide.

• C++ Guidelines
• Doxygen docs
• Unit tests
• No unwanted commits/changes
• Descriptive title/description
  • PR is single purpose
  • Related issue listed
• Comments in source
• No typos
• Update changelog
• Not work-in-progress
• External references or docs updated
• Self review of PR done
• Any breaking changes?

[github-actions]

Thank you for your contribution @teo-tsirpanis! We will review the pull request and get back to you soon.

[ahsonkhan]

@teo-tsirpanis can you please help me understand what the consequences are of disabling default features from a dependency like libxml2 and why it's beneficial to do so?

Also, regarding:

We don't seem to make use of any of them in xml_wrapper.cpp.

How did you check and validate that to be true? Maybe we can apply that approach elsewhere too, if useful.

[teo-tsirpanis]

The default features of libxml2 are iconv, lzma and zlib. Upon visual inspection, xml_wrapper.cpp uses libxml2 only for the simple cases of reading and writing XML data in memory, without any compression or character encoding involved.

For one example, the 0 in xmlNewTextWriterMemory means that no compression is enabled:

https://github.com/Azure/azure-sdk-for-cpp/blob/af6d5dcd076abe795285280da0dbfc3c535237b2/sdk/storage/azure-storage-common/src/xml_wrapper.cpp#L538C1-L539C80

Before this PR, consumers of azure-storage-cpp were forced to build libxml2 with these features enabled, bringing in three additional dependencies. Now, azure-storage-cpp requires only the base libxml2, building fewer dependencies by default.

[antkmsft]

Thank you @teo-tsirpanis! The change makes sense, and looks good.
I am approving, but I'll also wait for the Storage team's approval before merging.

P.S.: I pushed a commit into your branch, to also include the "Acknowledgments" section.

[Jinming-Hu]

Is there any way to test this? I tried to add

index af09fe2c5..5e13c2084 100644
--- a/ports/azure-storage-common-cpp/vcpkg.json
+++ b/ports/azure-storage-common-cpp/vcpkg.json
@@ -16,6 +16,7 @@
     },
     {
       "name": "libxml2",
+      "default-features": false,
       "platform": "!windows"
     },
     {

to vcpkg repo and run ./vcpkg install azure-storage-blobs-cpp, but turned out it still installed all the features

The following packages will be built and installed:
  * azure-core-cpp[core,curl,http]:x64-linux -> 1.10.3#2
    azure-storage-blobs-cpp:x64-linux -> 12.10.0
  * azure-storage-common-cpp:x64-linux -> 12.5.0
  * curl[core,non-http,openssl,ssl]:x64-linux -> 8.4.0
  * libiconv:x64-linux -> 1.17#1
  * liblzma:x64-linux -> 5.4.4
  * libxml2[core,iconv,lzma,zlib]:x64-linux -> 2.11.6#1
  * openssl:x64-linux -> 3.1.4#1
  * vcpkg-cmake:x64-linux -> 2023-05-04
  * vcpkg-cmake-config:x64-linux -> 2022-02-06#1
  * vcpkg-cmake-get-vars:x64-linux -> 2023-03-02
  * zlib:x64-linux -> 1.3

[Jinming-Hu]

I really want to test this before merging the PR.

[teo-tsirpanis]

According to vcpkg's documentation default features are not built if the top-level project does not enable them.

I tried building teo-tsirpanis/TileDB@cb67261 and got this:

[cmake] Detecting compiler hash for triplet x64-linux...
[cmake] The following packages will be removed:
[cmake]     libiconv:x64-linux
[cmake]     liblzma:x64-linux
[cmake] The following packages will be built and installed:
[cmake]     azure-storage-blobs-cpp:x64-linux -> 12.10.0 -- /home/teo/code/vcpkg/buildtrees/versioning_/versions/azure-storage-blobs-cpp/902107525b099bb6c915311567519dcd55bd2aea
[cmake]   * azure-storage-common-cpp:x64-linux -> 12.5.0#1 -- /home/teo/code/TileDB/ports/azure-storage-common-cpp
[cmake]     libxml2:x64-linux -> 2.11.6#1 -- /home/teo/code/vcpkg/buildtrees/versioning_/versions/libxml2/b2fd11805c5e6714215c5dd9630400c4ad32e833

I verified that disabling default features in the top-level vcpkg.json without also disabling them in the azure-storage-common-cpp port has no effect.

[antkmsft]

@Jinming-Hu, if a manifest has "default-features": false, that does not mean that vcpkg is going to install libxml2[core].
That means, if you had libxml2 installed as libxml2[core], when user later installs azure-storage-common-cpp, vcpkg will not require rebuilding libxml2 with its default features:

Step 1:

$ ./vcpkg install azure-core-cpp
Computing installation plan...
The following packages will be built and installed:
    azure-core-cpp[core,curl,http]:x64-linux -> 1.10.1
  * curl[core,non-http,openssl,ssl]:x64-linux -> 8.1.2#2
  * openssl:x64-linux -> 3.1.1#1
  * vcpkg-cmake:x64-linux -> 2023-05-04
  * vcpkg-cmake-config:x64-linux -> 2022-02-06#1
  * vcpkg-cmake-get-vars:x64-linux -> 2023-03-02
  * zlib:x64-linux -> 1.2.13
...

$ ./vcpkg install libxml2[core]
Computing installation plan...
The following packages will be built and installed:
    libxml2:x64-linux -> 2.10.3#1
...

Step 2 - with the fix:

$ ./vcpkg install azure-storage-common-cpp
Computing installation plan...
The following packages will be built and installed:
    azure-storage-common-cpp:x64-linux -> 12.3.3
...

Step 2 without this fix:

$ ./vcpkg install azure-storage-common-cpp
Computing installation plan...
The following packages will be rebuilt:
  * libxml2[core,iconv,lzma,zlib]:x64-linux -> 2.10.3#1
The following packages will be built and installed:
    azure-storage-common-cpp:x64-linux -> 12.3.3
Additional packages (*) will be modified to complete this operation.
warning: If you are sure you want to rebuild the above packages, run the command with the --recurse option.

This mechanism is exactly why vcpkg install azure-storage-blobs-cpp is able to produce a library which has HTTP transport implementation: azure-storage-blobs-cpp does only require azure-core-cpp with default-features: false. Which means that before installing azure-storage-blobs-cpp, user could do vcpkg install azure-core-cpp[curl] (i.e. even on Windows, Azure Core will be built with only libcurl transport), or vcpkg install azure-core-cpp[winhttp] (which means that Azure Core will only have WinHTTP transport, and libcurl transport will not be available). That also means that if the user runs vcpkg install azure-core-cpp[core], they get Azure Core with absolutely no HTTP transport implementation, but that is kind of what they are asking, when they install azure-core-cpp with only [core] as feature.
But should user have no azure-core-cpp installed before they run vcpkg install azure-storage-blobs-cpp, vcpkg will still install the azure-core-cpp dependency with its default features, which eventually means both WinHTTP and libcurl transport implementations on Windows, and libcurl transport implementation on other platforms.

I.e. default-features: false does not mean that vcpkg is guaranteed to install you the dependency with NO features, it kind of means that your package is fine if that dependency IS already installed with no features.

Regarding verification, our CI, even for this PR, does already run with vcpkg fetching the dependencies in manifest mode, and should there be problems, it would've expoded right here. As a second thing, our daily vcpkg validation PR (microsoft/vcpkg#34835) will go bad, if we merge this, and there are problems.

[Jinming-Hu]

@teo-tsirpanis @antkmsft Thanks for the pointers. I've tested this. It works for storage.