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
Keep only one flavor of the same libs in collect_libs()
#11527
Changes from 2 commits
a604391
4401481
cb6d378
a14726f
799b3c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,21 +43,6 @@ def test_collect_libs(): | |
result = collect_libs(conanfile, folder="custom_folder") | ||
assert ["customlib"] == result | ||
|
||
# Warn same lib different folders | ||
conanfile = ConanFileMock() | ||
conanfile.folders.set_base_package(temp_folder()) | ||
conanfile.cpp_info = CppInfo(conanfile.name, "") | ||
custom_mylib_path = os.path.join(conanfile.package_folder, "custom_folder", "mylib.lib") | ||
lib_mylib_path = os.path.join(conanfile.package_folder, "lib", "mylib.lib") | ||
save(custom_mylib_path, "") | ||
save(lib_mylib_path, "") | ||
conanfile.cpp_info.libdirs = ["lib", "custom_folder"] | ||
result = collect_libs(conanfile) | ||
assert ["mylib"] == result | ||
assert "Library 'mylib' was either already found in a previous "\ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add tests for symlinks There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
conanfile.folders.set_base_package(temp_folder()) | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?