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

add msix/1.7 #6475

Merged
merged 18 commits into from
Sep 29, 2021
7 changes: 7 additions & 0 deletions recipes/msix/all/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
cmake_minimum_required(VERSION 2.8.0)
project(cmake_wrapper)

include(conanbuildinfo.cmake)
conan_basic_setup()

add_subdirectory("source_subfolder")
10 changes: 10 additions & 0 deletions recipes/msix/all/conandata.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
sources:
"1.7":
url: "https://github.com/microsoft/msix-packaging/archive/refs/tags/v1.7.zip"
sha256: "23179d577d0b9d34e93374ae38284ab9f1247ff6cc9175851983fb114ab00087"
patches:
"1.7":
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
- base_path: "source_subfolder"
patch_file: "patches/1.7/cmake.patch"
- base_path: "source_subfolder"
patch_file: "patches/1.7/signaturevalidator.patch"
136 changes: 136 additions & 0 deletions recipes/msix/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import os
from conans import CMake, ConanFile, tools
from conans.errors import ConanInvalidConfiguration

SSE4 marked this conversation as resolved.
Show resolved Hide resolved

required_conan_version = ">=1.33.0"


class MsixConan(ConanFile):
name = "msix"
license = "MIT"
url = "https://github.com/conan-io/conan-center-index"
homepage = "https://github.com/microsoft/msix-packaging"
description = "An SDK for creating MSIX packages"
topics = ("msix", "sdk", "packaging", "conan-recipe")

settings = "os", "compiler", "build_type", "arch"
options = {
"shared": [True, False],
"fPIC": [True, False],
"crypto_lib": ["crypt32", "openssl"],
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
"pack": [True, False],
"skip_bundles": [True, False],
"use_external_zlib": [True, False],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to always use external libs.
Using a an included/included vendored zlib will open the possibility of having some symbols available twice.
The linker can then choose symbols from msix or from zlib.

So please remove this option and always use external zlib (=our cci package)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option allows to choose system libraries for compression when building on some platforms:

set(COMPRESSION_LIB "zlib")
if(((IOS) OR (MACOS)) AND (NOT USE_EXTERNAL_ZLIB))
    set(COMPRESSION_LIB "libCompression")
elseif((AOSP) AND (NOT USE_EXTERNAL_ZLIB))
    set(COMPRESSION_LIB "inbox zlib")
endif()

If this option is on, the zlib package from CCI will be used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upstream didn't rename the variable, we did it so that it reflects the idea that when it's on, the build will use the external (CCI) package for zlib. The original name and description is:

option(USE_MSIX_SDK_ZLIB "Use zlib implementation under lib/zlib. If off, uses inbox compression library. For Windows and Linux this is no-opt." OFF)

The modified version:

option(USE_EXTERNAL_ZLIB "Use an external zlib package. If off, uses inbox compression library. For Windows and Linux this is no-opt." OFF)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vendored libraries are often provided for users without package manager as a way to quickly get building without building all dependencies.
We're using conan here with the cci library. So I think it's best to remove this option and unconditionally disable using a vendored zlib.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the term "vendored" we meant libraries whose sources had been made a part of the library, when I wrote about system zlib I meant these ones:

https://developer.android.com/ndk/guides/stable_apis#zlib_compression

https://developer.apple.com/documentation/compression

https://github.com/microsoft/msix-packaging/blob/3fbd28851b0ac525324781ec20634a2f0515c0ef/src/msix/CMakeLists.txt#L292_L298

We can omit these possibilities, shall we?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we want to avoid (at all cost) is to use the vendored zlib here: https://github.com/microsoft/msix-packaging/blob/5f977a79d4f2bd189c1d99ae8f501316a282191a/lib/CMakeLists.txt#L22

Does the current combination of options + patches completely avoid that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the current combination of options + patches completely avoid that?

The current patch completely removes processing of the lib subdirectory:

conan-center-index/cmake.patch at a3144aa · conan-io/conan-center-index
https://github.com

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As point out the patch delete the usage of this option so it can be removed from the recipe!

Suggested change
"use_external_zlib": [True, False],

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The option hasn't been deleted, the commit just reverts it's name to the original one.

"use_validation_parser": [True, False],
"xml_parser": ["applexml", "javaxml", "msxml6", "xerces"]
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
}
default_options = {
"shared": False,
"fPIC": True,
"crypto_lib": "openssl",
"pack": False,
"skip_bundles": False,
"use_external_zlib": True,
"use_validation_parser": False,
"xml_parser": "xerces"
}

generators = "cmake"
exports_sources = "CMakeLists.txt", "patches/**"

_cmake = None

@property
def _source_subfolder(self):
return "source_subfolder"

def _configure_cmake(self):
if self._cmake:
return self._cmake
self._cmake = CMake(self)
if self.settings.os == "Android":
self._cmake.definitions["AOSP"] = True
if self.settings.os == "Linux":
self._cmake.definitions["LINUX"] = True
if self.settings.os == "Macos":
self._cmake.definitions["MACOS"] = True
self._cmake.definitions["CRYPTO_LIB"] = self.options.crypto_lib
self._cmake.definitions["MSIX_PACK"] = self.options.pack
self._cmake.definitions["MSIX_SAMPLES"] = False
self._cmake.definitions["MSIX_TESTS"] = False
self._cmake.definitions["SKIP_BUNDLES"] = self.options.skip_bundles
self._cmake.definitions["USE_EXTERNAL_ZLIB"] = self.options.use_external_zlib
self._cmake.definitions["USE_SHARED_ZLIB"] = self.options["zlib"].shared
self._cmake.definitions["USE_VALIDATION_PARSER"] = self.options.use_validation_parser
self._cmake.definitions["XML_PARSER"] = self.options.xml_parser
self._cmake.configure()
return self._cmake

def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
self.options.crypto_lib = "crypt32"

def configure(self):
if self.options.shared:
del self.options.fPIC
if self.options.xml_parser == "xerces":
self.options["xerces-c"].char_type = "char16_t"
self.options["xerces-c"].shared = False
if self.settings.compiler.get_safe("cppstd"):
tools.check_min_cppstd(self, "14")

def requirements(self):
if self.settings.os == "Linux" and not self.options.skip_bundles:
self.requires("icu/69.1")
if self.options.crypto_lib == "openssl":
self.requires("openssl/1.0.2t")
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
if self.options.use_external_zlib:
self.requires("zlib/1.2.11")
if self.options.xml_parser == "xerces":
self.requires("xerces-c/3.2.3")

def validate(self):
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
if self.settings.os != "Android" and self.options.xml_parser == "javaxml":
raise ConanInvalidConfiguration("javaxml is supported only for Android")
if self.settings.os == "Linux" and self.settings.compiler != "clang":
raise ConanInvalidConfiguration("Only clang is supported on Linux")
if self.settings.os != "Macos" and self.options.xml_parser == "applexml":
raise ConanInvalidConfiguration("applexml is supported only for MacOS")
if self.settings.os != "Windows" and self.options.crypto_lib == "crypt32":
raise ConanInvalidConfiguration("crypt32 is supported only for Windows")
if self.settings.os != "Windows" and self.options.xml_parser == "msxml6":
raise ConanInvalidConfiguration("msxml6 is supported only for Windows")
if self.options.pack:
if self.settings.os == "Macos":
if not self.options.use_external_zlib:
raise ConanInvalidConfiguration("Using libCompression APIs and packaging features is not supported")
if self.options.xml_parser != "xerces":
raise ConanInvalidConfiguration("Xerces is the only supported parser for MacOS pack")
if not self.options.use_validation_parser:
raise ConanInvalidConfiguration("Packaging requires validation parser")

def source(self):
tools.get(**self.conan_data["sources"][self.version], destination=self._source_subfolder, strip_root=True)

def build(self):
for patch in self.conan_data.get("patches", {}).get(self.version, []):
tools.patch(**patch)
cmake = self._configure_cmake()
cmake.build()

def package(self):
cmake = self._configure_cmake()
cmake.install()

def package_info(self):
self.cpp_info.libs = tools.collect_libs(self)
if self.settings.os == "Windows":
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
self.cpp_info.system_libs = ["runtimeobject"]
if self.settings.compiler == "Visual Studio":
self.cpp_info.system_libs.append("delayimp")
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
if self.options.crypto_lib == "crypt32":
self.cpp_info.system_libs.extend(["bcrypt", "crypt32", "wintrust"])
if self.options.xml_parser == "msxml6":
self.cpp_info.system_libs.append("msxml6")
Loading