[lzma] Locate correct release/debug libraries in static build#5625
[lzma] Locate correct release/debug libraries in static build#5625seanwarren wants to merge 5 commits intomicrosoft:masterfrom
Conversation
|
Could you try commit 0b5a576 |
|
Unfortunately that commit does not fully resolve the issue (I'm still getting debug/lib/liblzma.a linked in). Some kind of systematic resolution to this problem would be great though! |
|
what other dependencies do you have? Could be that one of those is linking it in if it wasn't also build with the fix. Maybe for testing add |
|
OK I'll have a look tomorrow (Syndey time). I'm building a project outside of vcpkg which depends on a number of ports. However I rebuilt all ports depending on liblzma. |
|
@Neumann-A , to me it looks like that commit won't fix the problem for multi-configuration (i.e. MSVC) builds, or am I missing something? |
|
probably #5621 |
|
At the moment |
|
@Rastaban, please could you let me know what is failing with this PR? |
Regression Status
@Rastaban can you please provide MacOS logs? |
c76d69d to
72c7b56
Compare
|
Unfortunately I can't reproduce any of the |
|
Hi @seanwarren , please see the MacOS logs. |
|
Thanks @NancyLi013, unfortunately the logs are not publicly accessible, please could you attach them directly or provide an accessible link |
|
Hello, please could you post the detailed error logs for the failing ports? The linked file only lists which port has failed. |
|
@Rastaban please provide MacOS logs |
|
@seanwarren where do you see the wrong linkage of lzma? With #5543 and libxml2 as a test I see the correct libraries linked into for Release and Debug. See the config of libxml2: |
|
@Neumann-A, I've just tested #5543 and this seems to resolve the issue, at least for liblzma (nice work!). I'm happy to close this issue and wait for the more general solution |
|
@seanwarren: Just checked #5543 for file changes and forgot that the necessary change (0b5a576) is already merged within master (was added in #5574). So you dont need to wait, just update ;). |
|
for what concern my problems, unfortunately @Neumann-A solution didn't fix anything, while this PR was very promising. Any possibilities you will open it again? Otherwise I will submit something similar in my OpenCV4 PR to fix LibLZMA, because otherwise the built is broken for downstream projects (not something that CI would catch without proper tests) when OpenCV library is built with static linking |
|
@cenit - I'm linking to OpenCV statically on windows and mac and seem to be linking the correct debug/release configurations for LibLZMA. Does the same problem occur for you with the current OpenCV 3 port? |
|
I just rebuilt opencv from scratch with the latest master.
which is even worse than what I was expecting and described in #5169 (comment) and still wrong because OpenCV knows only about debug lib, while it should have the correct
I am saying "worse" because I fear (not tested, just finished building) that it is not even working now from the latest master, since the VCPKG_* symbols should stay out of the installed ports... Since it looks like you did #5857 , can you confirm me that it is working? Is it even legit? Sorry for asking but this looked really new for me. |
This is a workaround to replace absolute paths which prevent the port being exported by vcpkg. It works correctly as the |
|
Looking again at my |
|
looks like opencv #5857 is missing a (in general the opencv portfile needs a lot of cleanup itself) |
|
@Neumann-A unfortunately |
|
@cenit I am just saying that the problem does not lie within the lzma port nor the find_package(LZMA) call. #5625 (comment) shows that the find_package call works as intended. The problem is the opencv portfile and simply adding TL;DR:
|
|
@seanwarren If you want to fix it use a general fix of what you are doing in the vcpkg_cmake_wrapper.cmake for lzma and add it for all possible packages to vcpkg.cmake. See TODOs in #5543. Im happy to add PRs/changes to #5543. (need to filter the list of defined package variables a bit further to only _LIBRARIES and _LIBRARY (excluding _RELEASE and _DEBUG) and add more logic if something must be changed) |
|
@Neumann-A it seems you missed the point. Your config log does not prove any point. It is the same for OpenCV. What you are showing was already happening, even well before your PR. The debug config finds only the debug library, the release config finds only the release library. |
|
@cenit so your are basically telling me that the todo in #5543 (comment)
should be answered with a clear yes and both paths should be there. My question is a bit different: so I went ahead and installed opencv....... First I don't see any dependency on lzma please also mentions features in the future (i assume TIFF for now) @cenit: Next time please add a full problem description with all circumstances.
Solution: update vcpkg.cmake to check packname_LIBRARY|LIBRARIES for existence of optimized & debug and add them + paths if it does not contain those keywords. |
|
@Neumann-A please moderate your tones, this problem is old and well known and it's not like i have to rewrite a long post every time for every new user appearing: again, you are misunderstanding the problem without proper investigation. Re-read the conversation, please, in full details. Anyway, I appreciate your reply in full length, so here it is another advice: try a static build for a downstream project. It will fail, with or without multi-configuration generators, for debug or release depending on the patchset you are trying, because at link time the compiler will always be instructed to link to a specific version of lzma, not one dynamically selected if release or debug, since only one is known from the OpenCVModules.cmake. Only dynamic linking are without any problem, since external referencies to second order dependencies disappear. If you try a static one, you will recognize a mismatch between release and debug crt due to LZMA. The proper way to deal with it is in CMake itself or through wrappers like the one proposed in this PR. Any other attempt to generalize a problem is just a big new coat of painting on a damaged wall, not just a dirty one, requiring too much effort for little gain. I will submit a PR for LZMA to fix it at the source. |
|
I should also add that this issue is not restricted to I am concerned that any more general solution to this problem would risk significant off-target effects. I am not sure that the following statement is always true
To give one example, some ports (e.g. |
|
@cenit: (The important conclusion of the comment is in bold.)
OK, you want to have a fight of arguments lets go :) Building opencv:x64-windows-static (sry for the wrong triplet before). From the debug build (using debug path liblzma): From the release build (using release path liblzma): (inserted line breaks to make the difference more obvious) so OpenCVModules.cmake is different between release and debug builds. vcpkg installs only on of those or overwrites one by installing the other (does not really matter what exactly is happening either cmake tries to install the release version and sees it is already there or the debug install overwrites the release version [or somebody is messing around in the portfile (see end of comment)]).... It is not the find_package call which is to blame, it works as intended within its constrains (although I really would like to see it working as you want it to but I don't know how it should work if the name of the library is the same for debug and release build. It would require an extra keyword like DEBUG|RELEASE_LIBRARY_SEARCHPATH or similar). There are different ways to fix the issue:
If you want to blame someone. Blame the person who put this line into the portfile (So it is not CMakes failure of find_package) p.s.: Not just lzma has this problem. There are others packages which have the same problem, so a more general solution within vcpkg would be nice. |
I think that problem is easily solved by checking the variable before changing it. So only variables which contain the vcpkg directory would be changed. |
|
@Neumann-A Since I wrote those lines, I think I know that problem and I am the one to blame 😉. The problem is well documented, in many appropriate places, and also here on vcpkg #5169 (comment) No need to insert line breaks but thanks for the explanation. And like @seanwarren is trying to tell you, it's not just an opencv problem. It's very simple to reproduce and it's an old cmake problem, which is very severe for any serious project trying to use LibLZMA in a world in which not just the release library is built. And like you also say, it's not just for LZMA. |
|
@cenit Ok what i really don't understand is why you then did not include a simple
to fix the OpenCVModules.cmake. (the find_package issue in multi generator mode is a different issue) |
|
just a curious question. (I'm not a cmake expert) Cmakes find_package / find_library is required if cmake is used outside of an "controlled" environment. This is not the case with vcpgk. Normally build should be easy ;). There are at maximum n lib files from previous compilations that need to be found. Wouldn't it be useful if cmake offers an plugin interface for controlled build environments, that replaces the default find_package find_library? This would have a couple of advantages:
|
|
@Fleischner - vcpkg does in fact intercept calls to |
|
@Neumann-A , if you can come up with a general solution to this problem that would be great. However unfortunately I'm not able to devote time to that at the moment so will continue to work on this PR which solves the problem for |
|
@seanwarren Try #5543. I pushed changes yesterday which should take care of the problems. Currently running |
|
Looks like this PR will be superseded by #6000 or used in combination with it.
|
|
@vicroms - please could you let me know what has failed here? Thanks |
|
@seanwarren has #6000 fixed your problems (now merged into master)? I sincerely hope so... |
|
This issue should have already been fixed on master |
|
Yes this can now be closed |
Currently, FindLZMA always detects the debug library for LZMA
This PR fixes the LZMA libraries detected by FindLZMA to ensure the correct configuration is used.