Repository: conan-io/conan
Pull request: [BazelDeps][bugfix] Windows and shared libraries
Changelog: Bugfix: BazelDeps did not find DLL files as Conan does not model them in the Windows platform.
Docs: omit
Close: https://github.com/conan-io/conan/issues/16691


Problem statement:
[bug] generated BUILD.bazel file does not import dll when shared=True for some packages
### Describe the bug

On Windows, with msvc, and shared=True, the bazel build file is not generated with an import statement for the DLL, for many conan packages:

The missing piece is:
```
cc_import(
    name = "libcurl_precompiled",
    interface_library = "lib/libcurl_imp.lib",
    shared_library = "bin/libcurl.dll",
)
and a reference to this target from the main cc_library statement
```

### conanfile.py
```
    # Binary configuration
    settings = "os", "arch", "compiler", "build_type"
    options = {"shared": [True, False]}
    default_options = {"shared": True, "libcurl/*:shared": True, "openssl/*:shared": True, "zlib/*:shared": True}

    # Requirements"
    requires = "libcurl/8.8.0"

    # Sources
    exports_sources = "src/library1/*"

    def layout(self):
        bazel_layout(self)

    def generate(self):
        bz = BazelDeps(self)
        bz.generate()
        bz = BazelToolchain(self)
        bz.generate()

    def build(self):
        bazel = Bazel(self)
        bazel.build(target="//src/library1", clean=False)
```

### libcurl:
```
cat d:\workdir\conan\p\b\confi4444e89a7f020\b\conan\libcurl\BUILD.bazel
# Components precompiled libs
< missing cc_import statement >

# Root package precompiled libs

# Components libraries declaration
cc_library(
    name = "libcurl-curl",
    hdrs = glob([
        "include/**",
    ]),
    includes = [
        "include",
    ],
    linkopts = [
        "/DEFAULTLIB:ws2_32",
    ],
    visibility = ["//visibility:public"],
    deps = [
        "@openssl//:openssl",
        "@zlib//:zlib",
    ],
)
```

### openssl:
```
cat d:\workdir\conan\p\b\confi4444e89a7f020\b\conan\openssl\BUILD.bazel
# Components precompiled libs
< missing cc_import statement >

# Root package precompiled libs

# Components libraries declaration
cc_library(
    name = "openssl-crypto",
    hdrs = glob([
        "include/**",
    ]),
    includes = [
        "include",
    ],
    linkopts = [
        "/DEFAULTLIB:crypt32",
        "/DEFAULTLIB:ws2_32",
        "/DEFAULTLIB:advapi32",
        "/DEFAULTLIB:user32",
        "/DEFAULTLIB:bcrypt",
    ],
    visibility = ["//visibility:public"],
    deps = [
        "@zlib//:zlib",
    ],
)
```

### How to reproduce it

_No response_

Hints from issue discussion:
HI @peakschris 

Sorry for the huge delay 😢 
Yes, it's a bug so thanks a lot for reporting this. The problem is when the components have all the information about the library names instead of the root package.
I'll open a PR to solve this ASAP 😁 
Wonderful! I've been running code that patches the build files after conan has generated them, it will be great to remove that and be able to use the conan ones directly :-)

Commit message: Added test

Patch to review:
diff --git a/conan/tools/google/bazeldeps.py b/conan/tools/google/bazeldeps.py
index 4cb213e8878..9b89fe7e9d4 100644
--- a/conan/tools/google/bazeldeps.py
+++ b/conan/tools/google/bazeldeps.py
@@ -10,8 +10,8 @@
 from conans.model.dependencies import get_transitive_requires
 from conans.util.files import save
 
-_BazelTargetInfo = namedtuple("DepInfo", ['repository_name', 'name', 'requires', 'cpp_info'])
-_LibInfo = namedtuple("LibInfo", ['name', 'is_shared', 'lib_path', 'interface_lib_path'])
+_BazelTargetInfo = namedtuple("DepInfo", ['repository_name', 'name', 'ref_name', 'requires', 'cpp_info'])
+_LibInfo = namedtuple("LibInfo", ['name', 'is_shared', 'lib_path', 'import_lib_path'])
 
 
 def _get_name_with_namespace(namespace, name):
@@ -72,18 +72,19 @@ def _get_requirements(conanfile, build_context_activated):
         yield require, dep
 
 
-def _get_libs(dep, cpp_info=None) -> list:
+def _get_libs(dep, cpp_info=None, reference_name=None) -> list:
     """
     Get the static/shared library paths
 
     :param dep: normally a <ConanFileInterface obj>
     :param cpp_info: <CppInfo obj> of the component.
+    :param reference_name: <str> Package/Component's reference name. ``None`` by default.
     :return: list of tuples per static/shared library ->
-             [(lib_name, is_shared, library_path, interface_library_path)]
+             [(lib_name, is_shared, library_path, import_library_path)]
              Note: ``library_path`` could be both static and shared ones in case of UNIX systems.
                     Windows would have:
-                        * shared: library_path as DLL, and interface_library_path as LIB
-                        * static: library_path as LIB, and interface_library_path as None
+                        * shared: library_path as DLL, and import_library_path as LIB
+                        * static: library_path as LIB, and import_library_path as None
     """
     def _is_shared():
         """
@@ -93,22 +94,52 @@ def _is_shared():
         return {"shared-library": True,
                 "static-library": False}.get(str(dep.package_type), default_value)
 
-    def _save_lib_path(lib_, lib_path_):
+    def _save_lib_path(file_name, file_path):
         """Add each lib with its full library path"""
-        formatted_path = lib_path_.replace("\\", "/")
-        _, ext_ = os.path.splitext(formatted_path)
-        if is_shared and ext_ == ".lib":  # Windows interface library
-            interface_lib_paths[lib_] = formatted_path
-        else:
-            lib_paths[lib_] = formatted_path
+        name, ext = file_name.split('.', maxsplit=1)  # ext could be .if.lib
+        formatted_path = file_path.replace("\\", "/")
+        lib_name = None
+        # Users may not name their libraries in a conventional way. For example, directly
+        # use the basename of the lib file as lib name, e.g., cpp_info.libs = ["liblib1.a"]
+        # Issue related: https://github.com/conan-io/conan/issues/11331
+        if file_name in libs:  # let's ensure that it has any extension
+            lib_name = file_name
+        elif name in libs:
+            lib_name = name
+        elif name.startswith("lib"):
+            short_name = name[3:]  # libpkg -> pkg
+            if short_name in libs:
+                lib_name = short_name
+        # FIXME: CPS will be in charge of defining correctly this part. At the moment, we can
+        #        only guess the name of the DLL binary as it does not follow any Conan pattern.
+        if ext == "dll" or ext.endswith(".dll"):
+            if lib_name:
+                shared_windows_libs[lib_name] = formatted_path
+            elif total_libs_number == 1 and libs[0] not in shared_windows_libs:
+                shared_windows_libs[libs[0]] = formatted_path
+            else:  # let's cross the fingers... This is the last chance.
+                for lib in libs:
+                    if ref_name in name and ref_name in lib and lib not in shared_windows_libs:
+                        shared_windows_libs[lib] = formatted_path
+                        break
+        elif lib_name is not None:
+            lib_paths[lib_name] = formatted_path
 
     cpp_info = cpp_info or dep.cpp_info
+    libs = cpp_info.libs[:]  # copying the values
+    if not libs:  # no libraries declared
+        return []
     is_shared = _is_shared()
     libdirs = cpp_info.libdirs
     bindirs = cpp_info.bindirs if is_shared else []  # just want to get shared libraries
-    libs = cpp_info.libs[:]  # copying the values
+    ref_name = reference_name or dep.ref.name
+    if hasattr(cpp_info, "aggregated_components"):
+        # Global cpp_info
+        total_libs_number = len(cpp_info.aggregated_components().libs)
+    else:
+        total_libs_number = len(libs)
     lib_paths = {}
-    interface_lib_paths = {}
+    shared_windows_libs = {}
     for libdir in set(libdirs + bindirs):
         if not os.path.exists(libdir):
             continue
@@ -117,35 +148,22 @@ def _save_lib_path(lib_, lib_path_):
             full_path = os.path.join(libdir, f)
             if not os.path.isfile(full_path):  # Make sure that directories are excluded
                 continue
-            name, ext = os.path.splitext(f)
-            # Users may not name their libraries in a conventional way. For example, directly
-            # use the basename of the lib file as lib name, e.g., cpp_info.libs = ["liblib1.a"]
-            # Issue related: https://github.com/conan-io/conan/issues/11331
-            if ext and f in libs:  # let's ensure that it has any extension
+            _, ext = os.path.splitext(f)
+            if is_shared and ext in (".so", ".dylib", ".lib", ".dll"):
+                _save_lib_path(f, full_path)
+            elif not is_shared and ext in (".a", ".lib"):
                 _save_lib_path(f, full_path)
-                continue
-            if name not in libs and name.startswith("lib"):
-                name = name[3:]  # libpkg -> pkg
-            # FIXME: Should it read a conf variable to know unexpected extensions?
-            if (is_shared and ext in (".so", ".dylib", ".lib", ".dll")) or \
-               (not is_shared and ext in (".a", ".lib")):
-                if name in libs:
-                    _save_lib_path(name, full_path)
-                    continue
-                else:  # last chance: some cases the name could be pkg.if instead of pkg
-                    name = name.split(".", maxsplit=1)[0]
-                    if name in libs:
-                        _save_lib_path(name, full_path)
 
     libraries = []
     for lib, lib_path in lib_paths.items():
-        interface_lib_path = None
-        if lib_path.endswith(".dll"):
-            if lib not in interface_lib_paths:
+        import_lib_path = None
+        if is_shared and os.path.splitext(lib_path)[1] == ".lib":
+            if lib not in shared_windows_libs:
                 raise ConanException(f"Windows needs a .lib for link-time and .dll for runtime."
                                      f" Only found {lib_path}")
-            interface_lib_path = interface_lib_paths.pop(lib)
-        libraries.append((lib, is_shared, lib_path, interface_lib_path))
+            import_lib_path = lib_path  # .lib
+            lib_path = shared_windows_libs.pop(lib)  # .dll
+        libraries.append((lib, is_shared, lib_path, import_lib_path))
     # TODO: Would we want to manage the cases where DLLs are provided by the system?
     return libraries
 
@@ -345,8 +363,8 @@ class _BazelBUILDGenerator:
         {% else %}
         static_library = "{{ lib_info.lib_path }}",
         {% endif %}
-        {% if lib_info.interface_lib_path %}
-        interface_library = "{{ lib_info.interface_lib_path }}",
+        {% if lib_info.import_lib_path %}
+        interface_library = "{{ lib_info.import_lib_path }}",
         {% endif %}
     )
     {% endfor %}
@@ -498,12 +516,12 @@ def fill_info(info):
                 libs_info = []
                 bindirs = [_relativize_path(bindir, package_folder_path)
                            for bindir in cpp_info.bindirs]
-                for (lib, is_shared, lib_path, interface_lib_path) in libs:
+                for (lib, is_shared, lib_path, import_lib_path) in libs:
                     # Bazel needs to relativize each path
                     libs_info.append(
                         _LibInfo(lib, is_shared,
                                  _relativize_path(lib_path, package_folder_path),
-                                 _relativize_path(interface_lib_path, package_folder_path))
+                                 _relativize_path(import_lib_path, package_folder_path))
                     )
                 ret.update({
                     "libs": libs_info,
@@ -599,7 +617,8 @@ def components_info(self):
             comp_requires_names = self._get_cpp_info_requires_names(cpp_info)
             comp_name = _get_component_name(self._dep, comp_ref_name)
             # Save each component information
-            components_info.append(_BazelTargetInfo(None, comp_name, comp_requires_names, cpp_info))
+            components_info.append(_BazelTargetInfo(None, comp_name, comp_ref_name,
+                                                    comp_requires_names, cpp_info))
         return components_info
 
     @property
@@ -622,7 +641,8 @@ def root_package_info(self):
                 for req in self._transitive_reqs.values()
             ]
         cpp_info = self._dep.cpp_info
-        return _BazelTargetInfo(repository_name, pkg_name, requires, cpp_info)
+        return _BazelTargetInfo(repository_name, pkg_name, _get_package_reference_name(self._dep),
+                                requires, cpp_info)
 
 
 class BazelDeps:
diff --git a/test/integration/toolchains/google/test_bazeldeps.py b/test/integration/toolchains/google/test_bazeldeps.py
index d4b99059ad3..62c4a45a20b 100644
--- a/test/integration/toolchains/google/test_bazeldeps.py
+++ b/test/integration/toolchains/google/test_bazeldeps.py
@@ -4,6 +4,7 @@
 
 from conan.test.assets.genconanfile import GenConanfile
 from conan.test.utils.tools import TestClient
+from conan.tools.files import load
 
 
 def test_bazel():
@@ -983,3 +984,132 @@ def package_info(self):
         # Now make sure we can actually build with build!=host context
         c.run("install app -s:h build_type=Debug --build=missing")
         assert "Install finished successfully" in c.out  # the asserts in build() didn't fail
+
+
+
+def test_shared_windows_find_libraries():
+    """
+    Testing the ``_get_libs`` mechanism in Windows, the shared libraries and their
+    import ones are correctly found.
+
+    Note: simulating dependencies with openssl, libcurl, and zlib packages:
+
+    zlib:
+        - (zlib package) bin/zlib1.dll AND lib/zdll.lib
+    libcurl:
+        - (curl component) bin/libcurl.dll AND lib/libcurl_imp.lib
+    openssl:
+        - (crypto component) bin/libcrypto-3-x64.dll AND lib/libcrypto.lib
+        - (ssl component) bin/libssl-3-x64.dll AND lib/libssl.lib
+
+
+    Issue: https://github.com/conan-io/conan/issues/16691
+    """
+    c = TestClient()
+    zlib = textwrap.dedent("""
+        import os
+        from conan import ConanFile
+        from conan.tools.files import save
+        class Example(ConanFile):
+            name = "zlib"
+            version = "1.0"
+            options = {"shared": [True, False]}
+            default_options = {"shared": False}
+            def package(self):
+                bindirs = os.path.join(self.package_folder, "bin")
+                libdirs = os.path.join(self.package_folder, "lib")
+                save(self, os.path.join(bindirs, "zlib1.dll"), "")
+                save(self, os.path.join(libdirs, "zdll.lib"), "")
+            def package_info(self):
+                self.cpp_info.libs = ["zdll"]
+            """)
+    openssl = textwrap.dedent("""
+        import os
+        from conan import ConanFile
+        from conan.tools.files import save
+        class Pkg(ConanFile):
+            name = "openssl"
+            version = "1.0"
+            options = {"shared": [True, False]}
+            default_options = {"shared": False}
+            def package(self):
+                bindirs = os.path.join(self.package_folder, "bin")
+                libdirs = os.path.join(self.package_folder, "lib")
+                save(self, os.path.join(bindirs, "libcrypto-3-x64.dll"), "")
+                save(self, os.path.join(bindirs, "libssl-3-x64.dll"), "")
+                save(self, os.path.join(libdirs, "libcrypto.lib"), "")
+                save(self, os.path.join(libdirs, "libssl.lib"), "")
+            def package_info(self):
+                self.cpp_info.components["crypto"].libs = ["libcrypto"]
+                self.cpp_info.components["ssl"].libs = ["libssl"]
+        """)
+    libcurl = textwrap.dedent("""
+        import os
+        from conan import ConanFile
+        from conan.tools.files import save
+        class Example(ConanFile):
+            name = "libcurl"
+            version = "1.0"
+            options = {"shared": [True, False]}
+            default_options = {"shared": False}
+            def package(self):
+                bindirs = os.path.join(self.package_folder, "bin")
+                libdirs = os.path.join(self.package_folder, "lib")
+                save(self, os.path.join(bindirs, "libcurl.dll"), "")
+                save(self, os.path.join(libdirs, "libcurl_imp.lib"), "")
+            def package_info(self):
+                self.cpp_info.components["curl"].libs = ["libcurl_imp"]
+            """)
+    consumer = textwrap.dedent("""
+        [requires]
+        zlib/1.0
+        openssl/1.0
+        libcurl/1.0
+        [options]
+        *:shared=True
+    """)
+    c.save({"conanfile.txt": consumer,
+            "zlib/conanfile.py": zlib,
+            "openssl/conanfile.py": openssl,
+            "libcurl/conanfile.py": libcurl})
+    c.run("export-pkg zlib -o:a shared=True")
+    c.run("export-pkg openssl -o:a shared=True")
+    c.run("export-pkg libcurl -o:a shared=True")
+    c.run("install . -g BazelDeps")
+    libcurl_bazel_build = load(None, os.path.join(c.current_folder, "libcurl", "BUILD.bazel"))
+    zlib_bazel_build = load(None, os.path.join(c.current_folder, "zlib", "BUILD.bazel"))
+    openssl_bazel_build = load(None, os.path.join(c.current_folder, "openssl", "BUILD.bazel"))
+    libcurl_expected = textwrap.dedent("""\
+    # Components precompiled libs
+    cc_import(
+        name = "libcurl_imp_precompiled",
+        shared_library = "bin/libcurl.dll",
+        interface_library = "lib/libcurl_imp.lib",
+    )
+    """)
+    openssl_expected = textwrap.dedent("""\
+    # Components precompiled libs
+    cc_import(
+        name = "libcrypto_precompiled",
+        shared_library = "bin/libcrypto-3-x64.dll",
+        interface_library = "lib/libcrypto.lib",
+    )
+
+    cc_import(
+        name = "libssl_precompiled",
+        shared_library = "bin/libcrypto-3-x64.dll",
+        interface_library = "lib/libssl.lib",
+    )
+    """)
+    zlib_expected = textwrap.dedent("""\
+    # Components precompiled libs
+    # Root package precompiled libs
+    cc_import(
+        name = "zdll_precompiled",
+        shared_library = "bin/zlib1.dll",
+        interface_library = "lib/zdll.lib",
+    )
+    """)
+    assert libcurl_expected in libcurl_bazel_build
+    assert zlib_expected in zlib_bazel_build
+    assert openssl_expected in openssl_bazel_build
diff --git a/test/unittests/tools/google/test_bazel.py b/test/unittests/tools/google/test_bazel.py
index dae3611bf25..5c5aa583693 100644
--- a/test/unittests/tools/google/test_bazel.py
+++ b/test/unittests/tools/google/test_bazel.py
@@ -4,12 +4,11 @@
 
 import pytest
 
+from conan.test.utils.mocks import ConanFileMock
+from conan.test.utils.test_files import temp_folder
 from conan.tools.files import save
 from conan.tools.google import Bazel
 from conan.tools.google.bazeldeps import _relativize_path, _get_libs
-from conans.model.options import Options
-from conan.test.utils.mocks import ConanFileMock
-from conan.test.utils.test_files import temp_folder
 
 
 @pytest.fixture(scope="module")
@@ -28,10 +27,12 @@ def cpp_info():
     save(ConanFileMock(), os.path.join(libdirs, "mylibwin2.if.lib"), "")
     save(ConanFileMock(), os.path.join(libdirs, "libmylib.so"), "")
     save(ConanFileMock(), os.path.join(libdirs, "subfolder", "libmylib.a"), "")  # recursive
-    cpp_info_mock = MagicMock(_base_folder=None, libdirs=None, bindirs=None, libs=None)
+    cpp_info_mock = MagicMock(_base_folder=None, libdirs=[], bindirs=[], libs=[],
+                              aggregated_components=MagicMock())
     cpp_info_mock._base_folder = folder.replace("\\", "/")
     cpp_info_mock.libdirs = [libdirs]
     cpp_info_mock.bindirs = [bindirs]
+    cpp_info_mock.aggregated_components.return_value = cpp_info_mock
     return cpp_info_mock
 
 
@@ -82,15 +83,11 @@ def test_bazeldeps_relativize_path(path, pattern, expected):
     # Win + shared
     (["mylibwin"], True, [('mylibwin', True, '{base_folder}/bin/mylibwin.dll', '{base_folder}/lib/mylibwin.lib')]),
     # Win + shared (interface with another ext)
-    (["mylibwin2"], True,
-     [('mylibwin2', True, '{base_folder}/bin/mylibwin2.dll', '{base_folder}/lib/mylibwin2.if.lib')]),
-    # Win + Mac + shared
-    (["mylibwin", "mylibmac"], True, [('mylibmac', True, '{base_folder}/bin/mylibmac.dylib', None),
-                                      ('mylibwin', True, '{base_folder}/bin/mylibwin.dll',
-                                       '{base_folder}/lib/mylibwin.lib')]),
-    # Linux + Mac + static
-    (["myliblin", "mylibmac"], False, [('mylibmac', False, '{base_folder}/lib/mylibmac.a', None),
-                                       ('myliblin', False, '{base_folder}/lib/myliblin.a', None)]),
+    (["mylibwin2"], True, [('mylibwin2', True, '{base_folder}/bin/mylibwin2.dll', '{base_folder}/lib/mylibwin2.if.lib')]),
+    # Mac + shared
+    (["mylibmac"], True, [('mylibmac', True, '{base_folder}/bin/mylibmac.dylib', None)]),
+    # Mac + static
+    (["mylibmac"], False, [('mylibmac', False, '{base_folder}/lib/mylibmac.a', None)]),
     # mylib + shared (saved as libmylib.so) -> removing the leading "lib" if it matches
     (["mylib"], True, [('mylib', True, '{base_folder}/lib/libmylib.so', None)]),
     # mylib + static (saved in a subfolder subfolder/libmylib.a) -> non-recursive at this moment
@@ -113,8 +110,10 @@ def test_bazeldeps_get_libs(cpp_info, libs, is_shared, expected):
         if interface_lib_path:
             interface_lib_path = interface_lib_path.format(base_folder=cpp_info._base_folder)
         ret.append((lib, is_shared, lib_path, interface_lib_path))
-    found_libs = _get_libs(ConanFileMock(options=Options(options_values={"shared": is_shared})),
-                           cpp_info)
+    dep = MagicMock()
+    dep.options.get_safe.return_value = is_shared
+    dep.ref.name = "my_pkg"
+    found_libs = _get_libs(dep, cpp_info)
     found_libs.sort()
     ret.sort()
     assert found_libs == ret


Code review comments:
diff_hunk: @@ -72,18 +72,19 @@ def _get_requirements(conanfile, build_context_activated):
         yield require, dep
 
 
-def _get_libs(dep, cpp_info=None) -> list:
+def _get_libs(dep, cpp_info=None, reference_name=None) -> list:
     """
     Get the static/shared library paths
 
     :param dep: normally a <ConanFileInterface obj>
     :param cpp_info: <CppInfo obj> of the component.
+    :param reference_name: <str> Package/Component's reference name. ``None`` by default.
     :return: list of tuples per static/shared library ->
-             [(lib_name, is_shared, library_path, interface_library_path)]
+             [(lib_name, is_shared, library_path, import_library_path)]
path: conan/tools/google/bazeldeps.py
text: @user1:
These names do not match the actual names of the tuple

@author:
I just removed my previous comments. I think that they match, don't they?