-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[wxwidgets] Fix wx-config --libs output #18580
Conversation
@JackBoosY There is improvement!!! Also how
|
@talregev Please check again:
|
@JackBoosY |
You can learn how the flags should be as how we compile Also past here how we use ld flags when compile wxwidgets. |
@JackBoosY Same errors after your change
|
+ # Do nothing | ||
+ else() | ||
+ string(REGEX MATCH "^lib(.*).a$" CURR_STATIC_NAME ${dep_name}) | ||
+ string(REGEX MATCH "^lib(.*).so$" CURR_DYNAMIC_NAME ${dep_name}) |
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 stores the whole match (i.e.
libz.a
) inCURR_<...>_NAME
. You will need to useCMAKE_MATCH_1
to get the "namespec" (i.e.z
). - This seems to ignore osx (
.dylib
).
@JackBoosY
to:
|
@JackBoosY |
I cannot deal with this today, but might come back to this soon.
|
Since it's a upstream bug, so we should wait for the upstream approval the changes. |
Can you send me the link? |
See #9184 |
Current output:
The abs path should be handled by portfile.cmake. |
Okay, I already create an upstream PR wxWidgets/wxWidgets#2402, let's wait for the official response. |
@JackBoosY
I think it connect to this PR #17111 |
I check only this PR. There was a progress! Stackoverflow solution: I think we should focus on this PR only, then this PR #17111, then the combination of both. |
@JackBoosY I re-arrange your current output with multi line for the flags to spot the problem:
I think it missing the for example:
|
This part is fixing in #17111. |
This part is a bug of |
You still didn't solve the libx bug:
|
Actually I just disable this Cotire/PCH thing now by adding |
@dg0yt Do you want to add this patch to this PR? |
For Multiple Configuration Generators, e.g. Visual Studio / xcode,
BTW, this is for modern cmake, i.e. a higher version of cmake. |
@playgithub The patching is for building in vcpkg. AFAIK vcpkg doesn't use Multiple Configuration Generators at the moment. |
Why not patch it in vcpkg first instead of waiting for the official fix? |
@playgithub That's becuase we need to wait for the upstream approval. |
@JackBoosY People don't understand what is upstream. Upstream mean |
In general packaging, it is fairly clear what upstream is. It is not only used in vcpkg. |
Since the upstream approval my changes, I can combine this PR to #17111. |
Are you going to put this patch in vcpkg or just use the commit where your change was merged upstream as the source for the port? |
@JackBoosY Thanks! |
@JackBoosY Any news? Can you merge this PR to the master? |
Fix command
./wx-config --libs
output, add-l
to the system library.From:
to:
Fixes #18298.
Depends on #17111.
Waiting for the upstream approval in wxWidgets/wxWidgets#2402.