Keep only one flavor of the same libs in collect_libs()
#11527
Conversation
SpaceIm
force-pushed
the
bugfix/collect_libs_macos
branch
from
b0c156f
to
877766e
Compare
SpaceIm
force-pushed
the
bugfix/collect_libs_macos
branch
from
877766e
to
a53756d
Compare
SpaceIm
force-pushed
the
bugfix/collect_libs_macos
branch
from
a53756d
to
a604391
Compare
Indeed, it happens all the time with symlinks
There was a problem hiding this comment.
I would be less concerned of something breaking if this didn't remove existing tests.
Also, it would be great if it added one or more tests that actually cover the intended use cases with symlinks.
If there is some risk of breaking, then I'd recommend to do it only in the new conan.tools space, but no in conans.client.tools
| "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))) |
There was a problem hiding this comment.
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.
Isn't os.path.realpath the most straightforward way to find real file?
| 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.
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?
There was a problem hiding this comment.
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.
I'll add tests for symlinks
There was a problem hiding this comment.
Good, lets keep the first asset, and I agree, not worth the effort to keep the warning, it can be removed.
|
This test should be skipped on windows. |
closes #11282
closes #11526
Changelog: Fix: In
conans.tools.collect_libsandconan.tools.files.collect_libs, avoids to list other flavors (version, soversion) of the samedylibon macOS, so that behavior is consistent on Linux & macOS.Docs: conan-io/docs#2610
developbranch, documenting this one.Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.