windows: integrate visibility attributes without conflicts #249
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I am not qualified to comment on the CMake proposals, nor do I really understand the Windows features (and I can't claim to know much about Autotools either). If this proposal solves the MinGW and Cmake issues without breaking anything else, then let's go for it. |
carenas
force-pushed
the
visibility
branch
3 times, most recently
from
4b3539d to
7d1a1e4
Compare
carenas
commented
May 15, 2023
When using a DLL in Windows, the function declarations (and definitions) that are public are decorated with attributes but those declarations would conflict with the ones that are selected when the compiler supports the visibility feature. Define instead a new macro that would be added to the corresponding macros independently and while at it allow setting visibility with cmake.
carenas
commented
May 16, 2023
| -e 'if(/PCRE2_EXP_DEFN/){print"/* #undef PCRE2_EXP_DEFN */\n";$$blank=0;next;}' \ | ||
| -e 'if(/to make a symbol visible/){next;}' \ | ||
| -e 'if(/__attribute__ \(\(visibility/){next;}' \ | ||
| -e 'if(/(.+?)\s*__attribute__ \(\(visibility/){print"$$1\n";$$blank=0;next;}' \ |
There was a problem hiding this comment.
this could break if the compiler used to build this doesn't support visibility attributes (but that could be addressed later if the maintainer moves his workstation to Windows)
There was a problem hiding this comment.
This maintainer won't ever be moving to Windows. :-)
Closed
carenas
added a commit
to carenas/pcre2
that referenced
this pull request
Oct 19, 2023
Since a8a875e (windows: integrate visibility attributes without conflicts (PCRE2Project#249), 2023-05-16), the use of PCRE2_STATIC has been deprecated so set PCRE2POSIX_SHARED instead when relevant.
PhilipHazel
pushed a commit
that referenced
this pull request
Oct 20, 2023
carenas
added a commit
to carenas/pcre2
that referenced
this pull request
Oct 20, 2023
Since a8a875e (windows: integrate visibility attributes without conflicts (PCRE2Project#249), 2023-05-16) setting CFLAGS when using a DLL is recommended, but the pcre2-config and pkgconfig files generated by cmake didn't include that, so correct that. While at it, refactor the logic so it will be more modern and it wouldn't unnecessarily install the pcre2posix pkgconfig if not needed.
carenas
added a commit
to carenas/pcre2
that referenced
this pull request
Oct 21, 2023
Since a8a875e (windows: integrate visibility attributes without conflicts (PCRE2Project#249), 2023-05-16) setting CFLAGS when using a DLL is recommended, but the pcre2-config and pkgconfig files generated by cmake didn't include that logic, unlike the ones generated by the autotools. While at it, refactor the code so it will be more modern and it wouldn't unnecessarily install the pcre2posix pkgconfig if not needed and make sure the generated pcre2-config uses UNIX EOL.
PhilipHazel
pushed a commit
that referenced
this pull request
Oct 21, 2023
Since a8a875e (windows: integrate visibility attributes without conflicts (#249), 2023-05-16) setting CFLAGS when using a DLL is recommended, but the pcre2-config and pkgconfig files generated by cmake didn't include that logic, unlike the ones generated by the autotools. While at it, refactor the code so it will be more modern and it wouldn't unnecessarily install the pcre2posix pkgconfig if not needed and make sure the generated pcre2-config uses UNIX EOL.
carenas
added a commit
to carenas/pcre2
that referenced
this pull request
Jun 6, 2024
Since a8a875e (windows: integrate visibility attributes without conflicts (PCRE2Project#249), 2023-05-16), PCRE2_EXPORT was meant to be defined as an empty value for compilers that don't support -fvisibility in *NIX, but a branch was left uncovered by mistake resulting in compilation issues with propietary compilers in at least Solaris and AIX. define PCRE2_EXPORT even if the compiler is not GCC compatible.
PhilipHazel
pushed a commit
that referenced
this pull request
Jun 7, 2024
Since a8a875e (windows: integrate visibility attributes without conflicts (#249), 2023-05-16), PCRE2_EXPORT was meant to be defined as an empty value for compilers that don't support -fvisibility in *NIX, but a branch was left uncovered by mistake resulting in compilation issues with propietary compilers in at least Solaris and AIX. define PCRE2_EXPORT even if the compiler is not GCC compatible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add PCRE2POSIX_SHARED which will be used Instead of PCRE2_STATIC to indicate that linking with a shared library is expected (specially relevant in Windows), but unlike the previous solution, it will be able to find the function names even if no DLL was available by not changing the names of the functions to ONLY fit the ones expected from a DLL. This also makes the use of PCRE2POSIX_SHARED "optional", as it allows linking with a static or dynamic library when not used.
To avoid the conflict that was triggered by building with MinGW, set a different variable (PCRE2_EXPORT) that could be then embedded as needed into the corresponding
PCRE2*_EXP_*variables.As a bonus, allow for detecting the visibility attributes also with cmake.
Fixes: #104
Fixes: #243