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.

Sign up for GitHub

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

Jump to bottom

Keep only one flavor of the same libs in collect_libs() #11527

Merged
merged 5 commits into from Jul 28, 2022
Merged

Keep only one flavor of the same libs in collect_libs() #11527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a604391
keep only one flavor among real file & symlinks in collect_libs()
SpaceIm Jun 25, 2022
4401481
no more warnings if same lib found several times
SpaceIm Jun 28, 2022
cb6d378
restore tests about duplicated libs in different folders
SpaceIm Jun 29, 2022
a14726f
test symlinks
SpaceIm Jun 29, 2022
799b3c7
skip symlinks tests if Windows
SpaceIm Jun 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 14 additions & 9 deletions conan/tools/files/files.py
View file
Open in desktop
Expand Up @@ -494,24 +494,29 @@ def collect_libs(conanfile, folder=None):
else:
lib_folders = [os.path.join(conanfile.package_folder, folder)
for folder in conanfile.cpp_info.libdirs]
result = []

ref_libs = {}
for lib_folder in lib_folders:
if not os.path.exists(lib_folder):
conanfile.output.warn("Lib folder doesn't exist, can't collect libraries: "
"{0}".format(lib_folder))
continue
# In case of symlinks, only keep shortest file name in the same "group"
files = os.listdir(lib_folder)
for f in files:
name, ext = os.path.splitext(f)
if ext in (".so", ".lib", ".a", ".dylib", ".bc"):
if ext != ".lib" and name.startswith("lib"):
name = name[3:]
if name in result:
conanfile.output.warn("Library '%s' was either already found in a previous "
"'conanfile.cpp_info.libdirs' folder or appears several "
"times with a different file extension" % name)
else:
result.append(name)
real_lib = os.path.basename(os.path.realpath(os.path.join(lib_folder, f)))
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit tricky way to follow symlinks and obtain the name, otherwise it seems very redudant. Better put a comment directly about this line to explain it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't os.path.realpath the most straightforward way to find real file?

if real_lib not in ref_libs or len(f) < len(ref_libs[real_lib]):
ref_libs[real_lib] = f

result = []
for f in ref_libs.values():
name, ext = os.path.splitext(f)
if ext != ".lib" and name.startswith("lib"):
name = name[3:]
if name not in result:
result.append(name)
result.sort()
return result

Expand Down
23 changes: 14 additions & 9 deletions conans/client/tools/files.py
View file
Open in desktop
Expand Up @@ -351,24 +351,29 @@ def collect_libs(conanfile, folder=None):
else:
lib_folders = [os.path.join(conanfile.package_folder, folder)
for folder in conanfile.cpp_info.libdirs]
result = []

ref_libs = {}
for lib_folder in lib_folders:
if not os.path.exists(lib_folder):
conanfile.output.warn("Lib folder doesn't exist, can't collect libraries: "
"{0}".format(lib_folder))
continue
# In case of symlinks, only keep shortest file name in the same "group"
files = os.listdir(lib_folder)
for f in files:
name, ext = os.path.splitext(f)
if ext in VALID_LIB_EXTENSIONS:
if ext != ".lib" and name.startswith("lib"):
name = name[3:]
if name in result:
conanfile.output.warn("Library '%s' was either already found in a previous "
"'conanfile.cpp_info.libdirs' folder or appears several "
"times with a different file extension" % name)
else:
result.append(name)
real_lib = os.path.basename(os.path.realpath(os.path.join(lib_folder, f)))
if real_lib not in ref_libs or len(f) < len(ref_libs[real_lib]):
ref_libs[real_lib] = f

result = []
for f in ref_libs.values():
name, ext = os.path.splitext(f)
if ext != ".lib" and name.startswith("lib"):
name = name[3:]
if name not in result:
result.append(name)
result.sort()
return result

Expand Down
28 changes: 24 additions & 4 deletions conans/test/unittests/tools/files/collect_lib_test.py
View file
Open in desktop
@@ -1,4 +1,7 @@
import os
import platform

import pytest

from conan.tools.files import check_md5, check_sha1, check_sha256, collect_libs
from conans.model.build_info import CppInfo
Expand Down Expand Up @@ -43,7 +46,7 @@ def test_collect_libs():
result = collect_libs(conanfile, folder="custom_folder")
assert ["customlib"] == result

# Warn same lib different folders
# Unicity of lib names
conanfile = ConanFileMock()
conanfile.folders.set_base_package(temp_folder())
conanfile.cpp_info = CppInfo(conanfile.name, "")
Expand All @@ -54,9 +57,6 @@ def test_collect_libs():
conanfile.cpp_info.libdirs = ["lib", "custom_folder"]
result = collect_libs(conanfile)
assert ["mylib"] == result
assert "Library 'mylib' was either already found in a previous "\
Copy link
Member

Choose a reason for hiding this comment

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

Why this test is being removed? Maybe it is fine to remove the warning check, but this test should still work and result == ["mylib"]?
Also, it is also that necessary to remove the warning? If collect_libs() is decided to use one specific name in a group of related ones, isn't it good to show some information about this?

Copy link
Contributor Author

@SpaceIm SpaceIm Jun 29, 2022 •
edited

Choose a reason for hiding this comment

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

Yes, the first assert is still valid. The second assert fails if I keep the warning, because now there is a first pass grouping the same libs in all folders, either symlinks or real file, and it loses the capability to track duplicated real files (but it can still detect libs with different extensions). Displaying a warning for symlinks is unnecessary noise. I can try to track duplicated real files, but I'm not sure it's worth the effort and overhead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add tests for symlinks

Copy link
Member

Choose a reason for hiding this comment

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

Good, lets keep the first asset, and I agree, not worth the effort to keep the warning, it can be removed.

"'conanfile.cpp_info.libdirs' folder or appears several times with a "\
"different file extension" in conanfile.output

# Warn lib folder does not exist with correct result
conanfile = ConanFileMock()
Expand All @@ -70,3 +70,23 @@ def test_collect_libs():
assert ["mylib"] == result
assert "WARN: Lib folder doesn't exist, can't collect libraries: %s" % no_folder_path \
in conanfile.output

@pytest.mark.skipif(platform.system() == "Windows", reason="Needs symlinks support")
def test_collect_libs_symlinks():
# Keep only the shortest lib name per group of symlinks
conanfile = ConanFileMock()
conanfile.folders.set_base_package(temp_folder())
conanfile.cpp_info = CppInfo(conanfile.name, "")
version_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.1.0.0.dylib")
soversion_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.1.dylib")
lib_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.dylib")
lib_mylib2_path = os.path.join(conanfile.package_folder, "lib", "libmylib.2.dylib")
lib_mylib3_path = os.path.join(conanfile.package_folder, "custom_folder", "libmylib.3.dylib")
save(version_mylib_path, "")
os.symlink(version_mylib_path, soversion_mylib_path)
os.symlink(soversion_mylib_path, lib_mylib_path)
save(lib_mylib2_path, "")
save(lib_mylib3_path, "")
conanfile.cpp_info.libdirs = ["lib", "custom_folder"]
result = collect_libs(conanfile)
assert ["mylib", "mylib.2", "mylib.3"] == result
50 changes: 42 additions & 8 deletions conans/test/unittests/util/tools_test.py
View file
Open in desktop
Expand Up @@ -817,7 +817,7 @@ def test_collect_libs(self):
result = tools.collect_libs(conanfile, folder="custom_folder")
self.assertEqual(["customlib"], result)

# Warn same lib different folders
# Unicity of lib names
conanfile = ConanFileMock()
conanfile.folders.set_base_package(temp_folder())
conanfile.cpp_info = CppInfo(conanfile.name, "")
Expand All @@ -828,9 +828,6 @@ def test_collect_libs(self):
conanfile.cpp_info.libdirs = ["lib", "custom_folder"]
result = tools.collect_libs(conanfile)
self.assertEqual(["mylib"], result)
self.assertIn("Library 'mylib' was either already found in a previous "
"'conanfile.cpp_info.libdirs' folder or appears several times with a "
"different file extension", conanfile.output)

# Warn lib folder does not exist with correct result
conanfile = ConanFileMock()
Expand All @@ -845,6 +842,26 @@ def test_collect_libs(self):
self.assertIn("WARN: Lib folder doesn't exist, can't collect libraries: %s"
% no_folder_path, conanfile.output)

@pytest.mark.skipif(platform.system() == "Windows", reason="Needs symlinks support")
def test_collect_libs_symlinks(self):
# Keep only the shortest lib name per group of symlinks
conanfile = ConanFileMock()
conanfile.folders.set_base_package(temp_folder())
conanfile.cpp_info = CppInfo(conanfile.name, "")
version_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.1.0.0.dylib")
soversion_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.1.dylib")
lib_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.dylib")
lib_mylib2_path = os.path.join(conanfile.package_folder, "lib", "libmylib.2.dylib")
lib_mylib3_path = os.path.join(conanfile.package_folder, "custom_folder", "libmylib.3.dylib")
save(version_mylib_path, "")
os.symlink(version_mylib_path, soversion_mylib_path)
os.symlink(soversion_mylib_path, lib_mylib_path)
save(lib_mylib2_path, "")
save(lib_mylib3_path, "")
conanfile.cpp_info.libdirs = ["lib", "custom_folder"]
result = tools.collect_libs(conanfile)
self.assertEqual(["mylib", "mylib.2", "mylib.3"], result)

def test_self_collect_libs(self):
conanfile = ConanFileMock()
# Without package_folder
Expand Down Expand Up @@ -880,7 +897,7 @@ def test_self_collect_libs(self):
result = conanfile.collect_libs(folder="custom_folder")
self.assertEqual(["customlib"], result)

# Warn same lib different folders
# Unicity of lib names
conanfile = ConanFileMock()
conanfile.folders.set_base_package(temp_folder())
conanfile.cpp_info = CppInfo("", "")
Expand All @@ -891,9 +908,6 @@ def test_self_collect_libs(self):
conanfile.cpp_info.libdirs = ["lib", "custom_folder"]
result = conanfile.collect_libs()
self.assertEqual(["mylib"], result)
self.assertIn("Library 'mylib' was either already found in a previous "
"'conanfile.cpp_info.libdirs' folder or appears several times with a "
"different file extension", conanfile.output)

# Warn lib folder does not exist with correct result
conanfile = ConanFileMock()
Expand All @@ -907,3 +921,23 @@ def test_self_collect_libs(self):
self.assertEqual(["mylib"], result)
self.assertIn("WARN: Lib folder doesn't exist, can't collect libraries: %s"
% no_folder_path, conanfile.output)

@pytest.mark.skipif(platform.system() == "Windows", reason="Needs symlinks support")
def test_self_collect_libs_symlinks(self):
# Keep only the shortest lib name per group of symlinks
conanfile = ConanFileMock()
conanfile.folders.set_base_package(temp_folder())
conanfile.cpp_info = CppInfo("", "")
version_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.1.0.0.dylib")
soversion_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.1.dylib")
lib_mylib_path = os.path.join(conanfile.package_folder, "lib", "libmylib.dylib")
lib_mylib2_path = os.path.join(conanfile.package_folder, "lib", "libmylib.2.dylib")
lib_mylib3_path = os.path.join(conanfile.package_folder, "custom_folder", "libmylib.3.dylib")
save(version_mylib_path, "")
os.symlink(version_mylib_path, soversion_mylib_path)
os.symlink(soversion_mylib_path, lib_mylib_path)
save(lib_mylib2_path, "")
save(lib_mylib3_path, "")
conanfile.cpp_info.libdirs = ["lib", "custom_folder"]
result = conanfile.collect_libs()
self.assertEqual(["mylib", "mylib.2", "mylib.3"], result)