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

Bazel build system maintenance #1953

Open
palemieux opened this issue Jan 14, 2025 · 3 comments
Open

Bazel build system maintenance #1953

palemieux opened this issue Jan 14, 2025 · 3 comments

Comments

@palemieux
Copy link

palemieux commented Jan 14, 2025

I am adding a new compressor (#1883) that depends on the OpenJPH open-source library.

Is there a way to auto-generate bazel/*.patch files? Similarly, is BUILD.bazel intended to be edited by hand? I ask because it duplicates much information already present in the CMake makefiles.

@Vertexwahn

https://academysoftwarefdn.slack.com/archives/CMLRW4N73/p1736808979009609

@Vertexwahn
Copy link
Contributor

Vertexwahn commented Jan 15, 2025

Currently, BUILD.bazel files are edited by hand - I have some internal hacked scripts that can support the conversion of a Visual Studio 2022 C++ project to BUILD.bazel files (CMake -> VS2022 -> Bazel) but, this is very unstable, and the generated output still needs manual rework.

For OpenJPH we need a patch file that creates this content here:

cc_binary(
    name = "ojph_compress",
    srcs = ["src/apps/ojph_compress/ojph_compress.cpp"],
    visibility = ["//visibility:public"],
    deps = [":ojph_expand"],
)

cc_library(
    name = "ojph_expand",
    srcs = [
        "src/apps/ojph_expand/ojph_expand.cpp",
        "src/apps/others/ojph_img_io.cpp",
    ],
    hdrs = [
        "src/apps/common/ojph_img_io.h",
    ],
    includes = [
        "src/apps/common",
    ],
    visibility = ["//visibility:public"],
    deps = [":openjph"],
)

cc_library(
    name = "openjph",
    srcs = [
        "src/core/codestream/ojph_bitbuffer_read.h",
        "src/core/codestream/ojph_bitbuffer_write.h",
        "src/core/codestream/ojph_codeblock.cpp",
        "src/core/codestream/ojph_codeblock.h",
        "src/core/codestream/ojph_codeblock_fun.cpp",
        "src/core/codestream/ojph_codeblock_fun.h",
        "src/core/codestream/ojph_codestream.cpp",
        "src/core/codestream/ojph_codestream_gen.cpp",
        "src/core/codestream/ojph_codestream_local.cpp",
        "src/core/codestream/ojph_params.cpp",
        "src/core/codestream/ojph_params_local.h",
        "src/core/codestream/ojph_precinct.cpp",
        "src/core/codestream/ojph_precinct.h",
        "src/core/codestream/ojph_resolution.cpp",
        "src/core/codestream/ojph_resolution.h",
        "src/core/codestream/ojph_subband.cpp",
        "src/core/codestream/ojph_subband.h",
        "src/core/codestream/ojph_tile.cpp",
        "src/core/codestream/ojph_tile.h",
        "src/core/codestream/ojph_tile_comp.cpp",
        "src/core/codestream/ojph_tile_comp.h",
        "src/core/coding/ojph_block_common.cpp",
        "src/core/coding/ojph_block_common.h",
        "src/core/coding/ojph_block_decoder.h",
        "src/core/coding/ojph_block_decoder32.cpp",
        "src/core/coding/ojph_block_decoder64.cpp",
        "src/core/coding/ojph_block_encoder.cpp",
        "src/core/coding/ojph_block_encoder.h",
        "src/core/coding/table0.h",
        "src/core/coding/table1.h",
        "src/core/common/ojph_arch.h",
        "src/core/common/ojph_base.h",
        "src/core/common/ojph_codestream.h",
        "src/core/common/ojph_defs.h",
        "src/core/common/ojph_file.h",
        "src/core/common/ojph_message.h",
        "src/core/common/ojph_params.h",
        "src/core/common/ojph_version.h",
        "src/core/others/ojph_arch.cpp",
        "src/core/others/ojph_file.cpp",
        "src/core/others/ojph_mem.cpp",
        "src/core/others/ojph_message.cpp",
        "src/core/transform/ojph_colour.cpp",
        "src/core/transform/ojph_colour.h",
        "src/core/transform/ojph_colour_local.h",
        "src/core/transform/ojph_transform.cpp",
        "src/core/transform/ojph_transform.h",
        "src/core/transform/ojph_transform_local.h",
    ],
    hdrs = [
        "src/core/common/ojph_arg.h",
        "src/core/common/ojph_mem.h",
    ],
    defines = [
        "OJPH_DISABLE_SIMD"
        #"OJPH_DISABLE_SSE2",
        #"OJPH_DISABLE_SSSE3",
        #"OJPH_DISABLE_SSE4",
        #"OJPH_DISABLE_AVX",
        #"OJPH_DISABLE_AVX2",
        #"OJPH_DISABLE_AVX512",
        #"OJPH_DISABLE_NEON",
    ],
    includes = [
        "src/core/codestream",
        "src/core/coding",
        "src/core/common",
        "src/core/others",
        "src/core/transform",
    ],
    visibility = ["//visibility:public"],
)

Bazel also supports globbing - with globbing we would not required to list each source file individually.

Only header files added to the hdrs attribute are visible from outside - maybe more headers here need to be moved from the srcs section to the hdrs section. Also for now SIMD is disabled. Would start for now without SIMD and later if it works add configuration setting to enable SIMD.

UPDATE

Add openexr/bazel/openjph_add_build_file.patch:

--- /dev/null
+++ BUILD.bazel
@@ -0,0 +1,116 @@
+load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")
+load("@rules_license//rules:license.bzl", "license")
+
+package(
+    default_applicable_licenses = [":license"],
+)
+
+exports_files([
+    "LICENSE",
+])
+
+license(
+    name = "license",
+    license_kinds = ["@rules_license//licenses/spdx:BSD-2-Clause"],
+    license_text = "LICENSE",
+)
+
+cc_binary(
+    name = "ojph_compress",
+    srcs = ["src/apps/ojph_compress/ojph_compress.cpp"],
+    visibility = ["//visibility:public"],
+    deps = [":ojph_expand"],
+)
+
+cc_library(
+    name = "ojph_expand",
+    srcs = [
+        "src/apps/ojph_expand/ojph_expand.cpp",
+        "src/apps/others/ojph_img_io.cpp",
+    ],
+    hdrs = [
+        "src/apps/common/ojph_img_io.h",
+    ],
+    includes = [
+        "src/apps/common",
+    ],
+    visibility = ["//visibility:public"],
+    deps = [":openjph"],
+)
+
+cc_library(
+    name = "openjph",
+    srcs = [
+        "src/core/codestream/ojph_bitbuffer_read.h",
+        "src/core/codestream/ojph_bitbuffer_write.h",
+        "src/core/codestream/ojph_codeblock.cpp",
+        "src/core/codestream/ojph_codeblock.h",
+        "src/core/codestream/ojph_codeblock_fun.cpp",
+        "src/core/codestream/ojph_codeblock_fun.h",
+        "src/core/codestream/ojph_codestream.cpp",
+        "src/core/codestream/ojph_codestream_gen.cpp",
+        "src/core/codestream/ojph_codestream_local.cpp",
+        "src/core/codestream/ojph_codestream_local.h",
+        "src/core/codestream/ojph_params.cpp",
+        "src/core/codestream/ojph_params_local.h",
+        "src/core/codestream/ojph_precinct.cpp",
+        "src/core/codestream/ojph_precinct.h",
+        "src/core/codestream/ojph_resolution.cpp",
+        "src/core/codestream/ojph_resolution.h",
+        "src/core/codestream/ojph_subband.cpp",
+        "src/core/codestream/ojph_subband.h",
+        "src/core/codestream/ojph_tile.cpp",
+        "src/core/codestream/ojph_tile.h",
+        "src/core/codestream/ojph_tile_comp.cpp",
+        "src/core/codestream/ojph_tile_comp.h",
+        "src/core/coding/ojph_block_common.cpp",
+        "src/core/coding/ojph_block_common.h",
+        "src/core/coding/ojph_block_decoder.h",
+        "src/core/coding/ojph_block_decoder32.cpp",
+        "src/core/coding/ojph_block_decoder64.cpp",
+        "src/core/coding/ojph_block_encoder.cpp",
+        "src/core/coding/ojph_block_encoder.h",
+        "src/core/coding/table0.h",
+        "src/core/coding/table1.h",
+        "src/core/common/ojph_arch.h",
+        "src/core/common/ojph_base.h",
+        "src/core/common/ojph_codestream.h",
+        "src/core/common/ojph_defs.h",
+        "src/core/common/ojph_file.h",
+        "src/core/common/ojph_message.h",
+        "src/core/common/ojph_params.h",
+        "src/core/common/ojph_version.h",
+        "src/core/others/ojph_arch.cpp",
+        "src/core/others/ojph_file.cpp",
+        "src/core/others/ojph_mem.cpp",
+        "src/core/others/ojph_message.cpp",
+        "src/core/transform/ojph_colour.cpp",
+        "src/core/transform/ojph_colour.h",
+        "src/core/transform/ojph_colour_local.h",
+        "src/core/transform/ojph_transform.cpp",
+        "src/core/transform/ojph_transform.h",
+        "src/core/transform/ojph_transform_local.h",
+    ],
+    hdrs = [
+        "src/core/common/ojph_arg.h",
+        "src/core/common/ojph_mem.h",
+    ],
+    defines = [
+        "OJPH_DISABLE_SIMD",
+        #"OJPH_DISABLE_SSE2",
+        #"OJPH_DISABLE_SSSE3",
+        #"OJPH_DISABLE_SSE4",
+        #"OJPH_DISABLE_AVX",
+        #"OJPH_DISABLE_AVX2",
+        #"OJPH_DISABLE_AVX512",
+        #"OJPH_DISABLE_NEON",
+    ],
+    includes = [
+        "src/core/codestream",
+        "src/core/coding",
+        "src/core/common",
+        "src/core/others",
+        "src/core/transform",
+    ],
+    visibility = ["//visibility:public"],
+)

Add openexr/bazel/openjph_module_dot_bazel.patch

--- MODULE.bazel
+++ MODULE.bazel
@@ -0,0 +1,8 @@
+module(
+    name = "openjph",
+    version = "0.18.2",
+    compatibility_level = 1,
+)
+
+bazel_dep(name = "rules_cc", version = "0.1.0")
+bazel_dep(name = "rules_license", version = "1.0.0")

@palemieux I added above the two patch files needed for OpenJPH

You need to add to the openexr/MODULE.bazel file:

bazel_dep(name = "openjph")

archive_override(
    module_name = "openjph",
    patches = [
        "//bazel:openjph_add_build_file.patch",
        "//bazel:openjph_module_dot_bazel.patch",
    ],
    strip_prefix = "OpenJPH-0.18.2",
    urls = ["https://github.com/aous72/OpenJPH/archive/refs/tags/0.18.2.tar.gz"], # Would pin it to 0.18.2 - later we can switch to main...
)

I will add this also to the BCR the next days.

You can also comment out the Bazel build jobs for now - and I can fix them later once OpenJPH related changes are merged...

@palemieux
Copy link
Author

Thanks. Will implement and report.

@Vertexwahn
Copy link
Contributor

I created a PR to the BCR to add OpenJPH: bazelbuild/bazel-central-registry#3609
Once this is merged is it enough to add bazel_dep(name = "openjph", version = "0.18.2") to the MODULE.bazel file. Since we want to "Live at head" for OpenJPH we still need openexr/bazel/openjph_module_dot_bazel.patch, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants