From 2a01fd8f31309262b4d1cd1901055883198c0fb2 Mon Sep 17 00:00:00 2001 From: gorloffslava Date: Mon, 29 Jul 2024 20:05:34 +0500 Subject: [PATCH 1/5] + Added ability to build Brotli Python bindings against system-provided brotli instead of vendored one --- pyproject.toml | 2 +- python/README.md | 10 +++ setup.py | 228 +++++++++++++++++++++++++++-------------------- 3 files changed, 142 insertions(+), 98 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index fed528d4a..37259982b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,3 @@ [build-system] -requires = ["setuptools"] +requires = ["setuptools", "pkgconfig"] build-backend = "setuptools.build_meta" diff --git a/python/README.md b/python/README.md index 4b6f63f1d..d67cd3787 100644 --- a/python/README.md +++ b/python/README.md @@ -17,6 +17,16 @@ following command from this directory: $ make install +If you already have native Brotli installed on your system and want to use this one instead of the vendored sources, you +should set the `USE_SYSTEM_BROTLI=1` environment variable when building the wheel, like this: + + $ USE_SYSTEM_BROTLI=1 pip install brotli --no-binary brotli + +Brotli is found via the `pkg-config` utility. Moreover, you must build all 3 `brotlicommon`, `brotlienc`, and `brotlidec` +components. If you're installing brotli from the package manager, you need the development package, like this on Fedora: + + $ dnf install brotli brotli-devel + ### Development You may run the following commands from this directory: diff --git a/setup.py b/setup.py index 6cd325d8c..b3a434c2b 100644 --- a/setup.py +++ b/setup.py @@ -19,10 +19,23 @@ from distutils import dep_util from distutils import log +from pkgconfig import configure_extension as pkgconfig_configure_extension + CURR_DIR = os.path.abspath(os.path.dirname(os.path.realpath(__file__))) +def bool_from_environ(key: str): + value = os.environ.get(key) + if not value: + return False + if value == "1": + return True + if value == "0": + return False + raise ValueError(f"Environment variable {key} has invalid value {value}. Please set it to 1, 0 or an empty string") + + def read_define(path, macro): """ Return macro value from the given file. """ with open(path, 'r') as f: @@ -172,103 +185,124 @@ def build_extension(self, ext): PY_MODULES = ['brotli'] -EXT_MODULES = [ - Extension( - '_brotli', - sources=[ - 'python/_brotli.c', - 'c/common/constants.c', - 'c/common/context.c', - 'c/common/dictionary.c', - 'c/common/platform.c', - 'c/common/shared_dictionary.c', - 'c/common/transform.c', - 'c/dec/bit_reader.c', - 'c/dec/decode.c', - 'c/dec/huffman.c', - 'c/dec/state.c', - 'c/enc/backward_references.c', - 'c/enc/backward_references_hq.c', - 'c/enc/bit_cost.c', - 'c/enc/block_splitter.c', - 'c/enc/brotli_bit_stream.c', - 'c/enc/cluster.c', - 'c/enc/command.c', - 'c/enc/compound_dictionary.c', - 'c/enc/compress_fragment.c', - 'c/enc/compress_fragment_two_pass.c', - 'c/enc/dictionary_hash.c', - 'c/enc/encode.c', - 'c/enc/encoder_dict.c', - 'c/enc/entropy_encode.c', - 'c/enc/fast_log.c', - 'c/enc/histogram.c', - 'c/enc/literal_cost.c', - 'c/enc/memory.c', - 'c/enc/metablock.c', - 'c/enc/static_dict.c', - 'c/enc/utf8_util.c', - ], - depends=[ - 'c/common/constants.h', - 'c/common/context.h', - 'c/common/dictionary.h', - 'c/common/platform.h', - 'c/common/shared_dictionary_internal.h', - 'c/common/transform.h', - 'c/common/version.h', - 'c/dec/bit_reader.h', - 'c/dec/huffman.h', - 'c/dec/prefix.h', - 'c/dec/state.h', - 'c/enc/backward_references.h', - 'c/enc/backward_references_hq.h', - 'c/enc/backward_references_inc.h', - 'c/enc/bit_cost.h', - 'c/enc/bit_cost_inc.h', - 'c/enc/block_encoder_inc.h', - 'c/enc/block_splitter.h', - 'c/enc/block_splitter_inc.h', - 'c/enc/brotli_bit_stream.h', - 'c/enc/cluster.h', - 'c/enc/cluster_inc.h', - 'c/enc/command.h', - 'c/enc/compound_dictionary.h', - 'c/enc/compress_fragment.h', - 'c/enc/compress_fragment_two_pass.h', - 'c/enc/dictionary_hash.h', - 'c/enc/encoder_dict.h', - 'c/enc/entropy_encode.h', - 'c/enc/entropy_encode_static.h', - 'c/enc/fast_log.h', - 'c/enc/find_match_length.h', - 'c/enc/hash.h', - 'c/enc/hash_composite_inc.h', - 'c/enc/hash_forgetful_chain_inc.h', - 'c/enc/hash_longest_match64_inc.h', - 'c/enc/hash_longest_match_inc.h', - 'c/enc/hash_longest_match_quickly_inc.h', - 'c/enc/hash_rolling_inc.h', - 'c/enc/hash_to_binary_tree_inc.h', - 'c/enc/histogram.h', - 'c/enc/histogram_inc.h', - 'c/enc/literal_cost.h', - 'c/enc/memory.h', - 'c/enc/metablock.h', - 'c/enc/metablock_inc.h', - 'c/enc/params.h', - 'c/enc/prefix.h', - 'c/enc/quality.h', - 'c/enc/ringbuffer.h', - 'c/enc/static_dict.h', - 'c/enc/static_dict_lut.h', - 'c/enc/utf8_util.h', - 'c/enc/write_bits.h', - ], - include_dirs=[ - 'c/include', - ]), -] +USE_SYSTEM_BROTLI = bool_from_environ('USE_SYSTEM_BROTLI') +print(f"[EUGO] USE_SYSTEM_BROTLI={USE_SYSTEM_BROTLI}") +if USE_SYSTEM_BROTLI: + brotli_extension = Extension( + '_brotli', + sources=[ + 'python/_brotli.c' + ] + ) + + REQUIRED_BROTLI_SYSTEM_LIBRARIES = ["libbrotlicommon", "libbrotlienc", "libbrotlidec"] + pkgconfig_configure_extension(brotli_extension, " ".join(REQUIRED_BROTLI_SYSTEM_LIBRARIES)) + + print(f"[EUGO] {brotli_extension.include_dirs}") + print(f"[EUGO] {brotli_extension.library_dirs}") + print(f"[EUGO] {brotli_extension.libraries}") + print(f"[EUGO] {brotli_extension.extra_compile_args}") + print(f"[EUGO] {brotli_extension.extra_link_args}") + + EXT_MODULES = [brotli_extension] +else: + EXT_MODULES = [ + Extension( + '_brotli', + sources=[ + 'python/_brotli.c', + 'c/common/constants.c', + 'c/common/context.c', + 'c/common/dictionary.c', + 'c/common/platform.c', + 'c/common/shared_dictionary.c', + 'c/common/transform.c', + 'c/dec/bit_reader.c', + 'c/dec/decode.c', + 'c/dec/huffman.c', + 'c/dec/state.c', + 'c/enc/backward_references.c', + 'c/enc/backward_references_hq.c', + 'c/enc/bit_cost.c', + 'c/enc/block_splitter.c', + 'c/enc/brotli_bit_stream.c', + 'c/enc/cluster.c', + 'c/enc/command.c', + 'c/enc/compound_dictionary.c', + 'c/enc/compress_fragment.c', + 'c/enc/compress_fragment_two_pass.c', + 'c/enc/dictionary_hash.c', + 'c/enc/encode.c', + 'c/enc/encoder_dict.c', + 'c/enc/entropy_encode.c', + 'c/enc/fast_log.c', + 'c/enc/histogram.c', + 'c/enc/literal_cost.c', + 'c/enc/memory.c', + 'c/enc/metablock.c', + 'c/enc/static_dict.c', + 'c/enc/utf8_util.c', + ], + depends=[ + 'c/common/constants.h', + 'c/common/context.h', + 'c/common/dictionary.h', + 'c/common/platform.h', + 'c/common/shared_dictionary_internal.h', + 'c/common/transform.h', + 'c/common/version.h', + 'c/dec/bit_reader.h', + 'c/dec/huffman.h', + 'c/dec/prefix.h', + 'c/dec/state.h', + 'c/enc/backward_references.h', + 'c/enc/backward_references_hq.h', + 'c/enc/backward_references_inc.h', + 'c/enc/bit_cost.h', + 'c/enc/bit_cost_inc.h', + 'c/enc/block_encoder_inc.h', + 'c/enc/block_splitter.h', + 'c/enc/block_splitter_inc.h', + 'c/enc/brotli_bit_stream.h', + 'c/enc/cluster.h', + 'c/enc/cluster_inc.h', + 'c/enc/command.h', + 'c/enc/compound_dictionary.h', + 'c/enc/compress_fragment.h', + 'c/enc/compress_fragment_two_pass.h', + 'c/enc/dictionary_hash.h', + 'c/enc/encoder_dict.h', + 'c/enc/entropy_encode.h', + 'c/enc/entropy_encode_static.h', + 'c/enc/fast_log.h', + 'c/enc/find_match_length.h', + 'c/enc/hash.h', + 'c/enc/hash_composite_inc.h', + 'c/enc/hash_forgetful_chain_inc.h', + 'c/enc/hash_longest_match64_inc.h', + 'c/enc/hash_longest_match_inc.h', + 'c/enc/hash_longest_match_quickly_inc.h', + 'c/enc/hash_rolling_inc.h', + 'c/enc/hash_to_binary_tree_inc.h', + 'c/enc/histogram.h', + 'c/enc/histogram_inc.h', + 'c/enc/literal_cost.h', + 'c/enc/memory.h', + 'c/enc/metablock.h', + 'c/enc/metablock_inc.h', + 'c/enc/params.h', + 'c/enc/prefix.h', + 'c/enc/quality.h', + 'c/enc/ringbuffer.h', + 'c/enc/static_dict.h', + 'c/enc/static_dict_lut.h', + 'c/enc/utf8_util.h', + 'c/enc/write_bits.h', + ], + include_dirs=[ + 'c/include', + ]), + ] TEST_SUITE = 'setup.get_test_suite' From b93502da88797373d0fba303145fdce40f1849f3 Mon Sep 17 00:00:00 2001 From: Benjamin Leff Date: Thu, 29 Aug 2024 19:17:37 -0500 Subject: [PATCH 2/5] fix: renamed print --- setup.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/setup.py b/setup.py index b3a434c2b..c313d5283 100644 --- a/setup.py +++ b/setup.py @@ -186,7 +186,7 @@ def build_extension(self, ext): PY_MODULES = ['brotli'] USE_SYSTEM_BROTLI = bool_from_environ('USE_SYSTEM_BROTLI') -print(f"[EUGO] USE_SYSTEM_BROTLI={USE_SYSTEM_BROTLI}") +print(f"[BROTLI] USE_SYSTEM_BROTLI={USE_SYSTEM_BROTLI}") if USE_SYSTEM_BROTLI: brotli_extension = Extension( '_brotli', @@ -198,11 +198,11 @@ def build_extension(self, ext): REQUIRED_BROTLI_SYSTEM_LIBRARIES = ["libbrotlicommon", "libbrotlienc", "libbrotlidec"] pkgconfig_configure_extension(brotli_extension, " ".join(REQUIRED_BROTLI_SYSTEM_LIBRARIES)) - print(f"[EUGO] {brotli_extension.include_dirs}") - print(f"[EUGO] {brotli_extension.library_dirs}") - print(f"[EUGO] {brotli_extension.libraries}") - print(f"[EUGO] {brotli_extension.extra_compile_args}") - print(f"[EUGO] {brotli_extension.extra_link_args}") + print(f"[BROTLI] {brotli_extension.include_dirs}") + print(f"[BROTLI] {brotli_extension.library_dirs}") + print(f"[BROTLI] {brotli_extension.libraries}") + print(f"[BROTLI] {brotli_extension.extra_compile_args}") + print(f"[BROTLI] {brotli_extension.extra_link_args}") EXT_MODULES = [brotli_extension] else: From 0ea9927a2ba8a918b74a5e68c53d15c78a1a07c6 Mon Sep 17 00:00:00 2001 From: gorloffslava Date: Thu, 21 Nov 2024 16:11:26 +0500 Subject: [PATCH 3/5] + Improved setup.py formatting + Updated dependency resolution for edge cases in setup.py --- setup.py | 242 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 127 insertions(+), 115 deletions(-) diff --git a/setup.py b/setup.py index c313d5283..c0ae61b2c 100644 --- a/setup.py +++ b/setup.py @@ -19,126 +19,127 @@ from distutils import dep_util from distutils import log -from pkgconfig import configure_extension as pkgconfig_configure_extension +import pkgconfig CURR_DIR = os.path.abspath(os.path.dirname(os.path.realpath(__file__))) def bool_from_environ(key: str): - value = os.environ.get(key) - if not value: - return False - if value == "1": - return True - if value == "0": - return False - raise ValueError(f"Environment variable {key} has invalid value {value}. Please set it to 1, 0 or an empty string") + value = os.environ.get(key) + if not value: + return False + if value == "1": + return True + if value == "0": + return False + raise ValueError(f"Environment variable {key} has invalid value {value}. Please set it to 1, 0 or an empty string") def read_define(path, macro): - """ Return macro value from the given file. """ - with open(path, 'r') as f: - for line in f: - m = re.match(rf'#define\s{macro}\s+(.+)', line) - if m: - return m.group(1) - return '' + """ Return macro value from the given file. """ + with open(path, 'r') as f: + for line in f: + m = re.match(rf'#define\s{macro}\s+(.+)', line) + if m: + return m.group(1) + + return '' def get_version(): - """ Return library version string from 'common/version.h' file. """ - version_file_path = os.path.join(CURR_DIR, 'c', 'common', 'version.h') - major = read_define(version_file_path, 'BROTLI_VERSION_MAJOR') - minor = read_define(version_file_path, 'BROTLI_VERSION_MINOR') - patch = read_define(version_file_path, 'BROTLI_VERSION_PATCH') - if not major or not minor or not patch: - return '' - return f'{major}.{minor}.{patch}' + """ Return library version string from 'common/version.h' file. """ + version_file_path = os.path.join(CURR_DIR, 'c', 'common', 'version.h') + major = read_define(version_file_path, 'BROTLI_VERSION_MAJOR') + minor = read_define(version_file_path, 'BROTLI_VERSION_MINOR') + patch = read_define(version_file_path, 'BROTLI_VERSION_PATCH') + if not major or not minor or not patch: + return '' + return f'{major}.{minor}.{patch}' def get_test_suite(): - test_loader = unittest.TestLoader() - test_suite = test_loader.discover('python', pattern='*_test.py') - return test_suite + test_loader = unittest.TestLoader() + test_suite = test_loader.discover('python', pattern='*_test.py') + return test_suite class BuildExt(build_ext): - def get_source_files(self): - filenames = build_ext.get_source_files(self) - for ext in self.extensions: - filenames.extend(ext.depends) - return filenames - - def build_extension(self, ext): - if ext.sources is None or not isinstance(ext.sources, (list, tuple)): - raise errors.DistutilsSetupError( - "in 'ext_modules' option (extension '%s'), " - "'sources' must be present and must be " - "a list of source filenames" % ext.name) - - ext_path = self.get_ext_fullpath(ext.name) - depends = ext.sources + ext.depends - if not (self.force or dep_util.newer_group(depends, ext_path, 'newer')): - log.debug("skipping '%s' extension (up-to-date)", ext.name) - return - else: - log.info("building '%s' extension", ext.name) - - c_sources = [] - for source in ext.sources: - if source.endswith('.c'): - c_sources.append(source) - extra_args = ext.extra_compile_args or [] - - objects = [] - - macros = ext.define_macros[:] - if platform.system() == 'Darwin': - macros.append(('OS_MACOSX', '1')) - elif self.compiler.compiler_type == 'mingw32': - # On Windows Python 2.7, pyconfig.h defines "hypot" as "_hypot", - # This clashes with GCC's cmath, and causes compilation errors when - # building under MinGW: http://bugs.python.org/issue11566 - macros.append(('_hypot', 'hypot')) - for undef in ext.undef_macros: - macros.append((undef,)) - - objs = self.compiler.compile( - c_sources, - output_dir=self.build_temp, - macros=macros, - include_dirs=ext.include_dirs, - debug=self.debug, - extra_postargs=extra_args, - depends=ext.depends) - objects.extend(objs) - - self._built_objects = objects[:] - if ext.extra_objects: - objects.extend(ext.extra_objects) - extra_args = ext.extra_link_args or [] - # when using GCC on Windows, we statically link libgcc and libstdc++, - # so that we don't need to package extra DLLs - if self.compiler.compiler_type == 'mingw32': - extra_args.extend(['-static-libgcc', '-static-libstdc++']) - - ext_path = self.get_ext_fullpath(ext.name) - # Detect target language, if not provided - language = ext.language or self.compiler.detect_language(c_sources) - - self.compiler.link_shared_object( - objects, - ext_path, - libraries=self.get_libraries(ext), - library_dirs=ext.library_dirs, - runtime_library_dirs=ext.runtime_library_dirs, - extra_postargs=extra_args, - export_symbols=self.get_export_symbols(ext), - debug=self.debug, - build_temp=self.build_temp, - target_lang=language) + def get_source_files(self): + filenames = build_ext.get_source_files(self) + for ext in self.extensions: + filenames.extend(ext.depends) + return filenames + + def build_extension(self, ext): + if ext.sources is None or not isinstance(ext.sources, (list, tuple)): + raise errors.DistutilsSetupError( + "in 'ext_modules' option (extension '%s'), " + "'sources' must be present and must be " + "a list of source filenames" % ext.name) + + ext_path = self.get_ext_fullpath(ext.name) + depends = ext.sources + ext.depends + if not (self.force or dep_util.newer_group(depends, ext_path, 'newer')): + log.debug("skipping '%s' extension (up-to-date)", ext.name) + return + else: + log.info("building '%s' extension", ext.name) + + c_sources = [] + for source in ext.sources: + if source.endswith('.c'): + c_sources.append(source) + extra_args = ext.extra_compile_args or [] + + objects = [] + + macros = ext.define_macros[:] + if platform.system() == 'Darwin': + macros.append(('OS_MACOSX', '1')) + elif self.compiler.compiler_type == 'mingw32': + # On Windows Python 2.7, pyconfig.h defines "hypot" as "_hypot", + # This clashes with GCC's cmath, and causes compilation errors when + # building under MinGW: http://bugs.python.org/issue11566 + macros.append(('_hypot', 'hypot')) + for undef in ext.undef_macros: + macros.append((undef,)) + + objs = self.compiler.compile( + c_sources, + output_dir=self.build_temp, + macros=macros, + include_dirs=ext.include_dirs, + debug=self.debug, + extra_postargs=extra_args, + depends=ext.depends) + objects.extend(objs) + + self._built_objects = objects[:] + if ext.extra_objects: + objects.extend(ext.extra_objects) + extra_args = ext.extra_link_args or [] + # when using GCC on Windows, we statically link libgcc and libstdc++, + # so that we don't need to package extra DLLs + if self.compiler.compiler_type == 'mingw32': + extra_args.extend(['-static-libgcc', '-static-libstdc++']) + + ext_path = self.get_ext_fullpath(ext.name) + # Detect target language, if not provided + language = ext.language or self.compiler.detect_language(c_sources) + + self.compiler.link_shared_object( + objects, + ext_path, + libraries=self.get_libraries(ext), + library_dirs=ext.library_dirs, + runtime_library_dirs=ext.runtime_library_dirs, + extra_postargs=extra_args, + export_symbols=self.get_export_symbols(ext), + debug=self.debug, + build_temp=self.build_temp, + target_lang=language) NAME = 'Brotli' @@ -186,23 +187,34 @@ def build_extension(self, ext): PY_MODULES = ['brotli'] USE_SYSTEM_BROTLI = bool_from_environ('USE_SYSTEM_BROTLI') -print(f"[BROTLI] USE_SYSTEM_BROTLI={USE_SYSTEM_BROTLI}") -if USE_SYSTEM_BROTLI: - brotli_extension = Extension( - '_brotli', - sources=[ - 'python/_brotli.c' - ] - ) +if USE_SYSTEM_BROTLI: REQUIRED_BROTLI_SYSTEM_LIBRARIES = ["libbrotlicommon", "libbrotlienc", "libbrotlidec"] - pkgconfig_configure_extension(brotli_extension, " ".join(REQUIRED_BROTLI_SYSTEM_LIBRARIES)) - print(f"[BROTLI] {brotli_extension.include_dirs}") - print(f"[BROTLI] {brotli_extension.library_dirs}") - print(f"[BROTLI] {brotli_extension.libraries}") - print(f"[BROTLI] {brotli_extension.extra_compile_args}") - print(f"[BROTLI] {brotli_extension.extra_link_args}") + define_macros = [] + include_dirs = [] + libraries = [] + library_dirs = [] + + for required_system_library in REQUIRED_BROTLI_SYSTEM_LIBRARIES: + package_configuration = pkgconfig.parse(required_system_library) + + define_macros += package_configuration["define_macros"] + include_dirs += package_configuration["include_dirs"] + libraries += package_configuration["libraries"] + library_dirs += package_configuration["library_dirs"] + + brotli_extension = Extension( + '_brotli', + sources=[ + 'python/_brotli.c' + ], + include_dirs=include_dirs, + define_macros=define_macros, + libraries=libraries, + library_dirs=library_dirs + ) + EXT_MODULES = [brotli_extension] else: From 0e0ed5f0cc91b05f8d895984108e30099325c7be Mon Sep 17 00:00:00 2001 From: Benjamin Leff Date: Tue, 27 May 2025 08:53:51 -0500 Subject: [PATCH 4/5] perf: conditionally install pkgconfig if using system brotli --- setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c0ae61b2c..c7b5df551 100644 --- a/setup.py +++ b/setup.py @@ -19,7 +19,6 @@ from distutils import dep_util from distutils import log -import pkgconfig CURR_DIR = os.path.abspath(os.path.dirname(os.path.realpath(__file__))) @@ -189,6 +188,8 @@ def build_extension(self, ext): USE_SYSTEM_BROTLI = bool_from_environ('USE_SYSTEM_BROTLI') if USE_SYSTEM_BROTLI: + import pkgconfig + REQUIRED_BROTLI_SYSTEM_LIBRARIES = ["libbrotlicommon", "libbrotlienc", "libbrotlidec"] define_macros = [] From dcc24d051d3caeaa00a1d7583b20130817864519 Mon Sep 17 00:00:00 2001 From: Benjamin Leff Date: Wed, 28 May 2025 14:13:46 -0500 Subject: [PATCH 5/5] chore: fixing trailing whitespace in python/README.md --- python/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/README.md b/python/README.md index d67cd3787..5ea135ba5 100644 --- a/python/README.md +++ b/python/README.md @@ -24,7 +24,7 @@ should set the `USE_SYSTEM_BROTLI=1` environment variable when building the whee Brotli is found via the `pkg-config` utility. Moreover, you must build all 3 `brotlicommon`, `brotlienc`, and `brotlidec` components. If you're installing brotli from the package manager, you need the development package, like this on Fedora: - + $ dnf install brotli brotli-devel ### Development