On Wed Mar 09, 2022 at 08:28:15AM +0100, Rafael Sadowski wrote:
> On Tue Mar 08, 2022 at 07:43:04PM +0000, Klemens Nanni wrote:
> > On Mon, Mar 07, 2022 at 07:53:49PM +0000, Stuart Henderson wrote:
> > > On 2022/03/06 18:21, Rafael Sadowski wrote:
> > > > > Yet another cmake patch, which needs a full bulk test. Many of you
> > > > > will
> > > > > certainly know it, our cmake's SHARED_LIBS handling is broken for new
> > > > > shared libs.
> > > > >
> > > > > The default "0.0" version has been broken for several months/years.
> > > > > Here is a attempt to fix this. With the following patch you get back
> > > > > the
> > > > > following lines, if (a) LIBxxx_VERSION is not set and (b) SOVERSION
> > > > > for
> > > > > the shared lib is set by cmake. (a) is clear but (b) helps us to
> > > > > handle
> > > > > shared libs and not plugins (dlopen) aka shared libs without version.
> > > > >
> > > > > ...
> > > > > Warning: unregistered shared lib(s)
> > > > > SHARED_LIBS += fmt 0.0 # 0.0
> > > > > /usr/ports/devel/fmt/pkg/PLIST is new
> > > > >
> > > > >
> > > > > Patch changes:
> > > > >
> > > > > - Remove MODULE_LIBRARY processing.
> > > > > "MODULE libraries are plugins that are not linked into other targets
> > > > > but may be loaded dynamically at runtime using dlopen-like
> > > > > functionality." --
> > > > > https://cmake.org/cmake/help/latest/command/add_library.html
> > > > >
> > > > > - Add default "0.0" version:
> > > > > if type SHARED_LIBRARY AND empty LIBxxx_VERSION BUT SOVERSION is
> > > > > set.
> > > > >
> > > > > I would appreciate a bulk test, unfortunately I can't do one.
> > >
> > > Not finished yet, but this one looks a bit odd so I'm sending it early.
> > > net/dino fails; despite having
> > >
> > > SHARED_LIBS += dino 1.0 # 0.0
> > >
> > > the actual file produced is
> > >
> > > -rw-r--r-- 1 _pbuild _pbuild 3085740 Mar 7 12:32 libdino.so.0.0
> >
> > Their main lib's target is called "libdino" but they install it with
> > `PREFIX ""'... maybe to avoid conflicts with the main project "Dino"?
>
> Hi Klemens,
>
> yes to distinguish two targets we have to define tow different
> target-names in cmake. I see this in mariadb/libmariadb ledger/libledger...
>
> I think it's better to handle this in the cmake openbsd patch by
> evaluating PREFIX. Other Opinions?
>
With the results from Stuart's bulk build and Klemens analyses, I end up
with a new diff.
- Handle target names like libXXX with PREFIX set to "".
With this, all reported errors are happy. Afterwards, we can delete
all patches. For example, I can remove all x11/kde-application cmake
hackish patches.
- Use cmSystemTools::GetEnv instead getenv(3)
- More shorter C++ code
Thank you to all. A final bulk build would be advisable, wouldn't it?
Rafael
diff --git a/devel/cmake/Makefile b/devel/cmake/Makefile
index 013bf49411c..204bde44ce2 100644
--- a/devel/cmake/Makefile
+++ b/devel/cmake/Makefile
@@ -8,7 +8,7 @@ VER = 3.20.3
EPOCH = 0
DISTNAME = cmake-${VER}
CATEGORIES = devel
-REVISION = 4
+REVISION = 5
HOMEPAGE = https://www.cmake.org/
diff --git a/devel/cmake/patches/patch-Source_cmGeneratorTarget_cxx
b/devel/cmake/patches/patch-Source_cmGeneratorTarget_cxx
index 14a878c0d42..33c42b31af8 100644
--- a/devel/cmake/patches/patch-Source_cmGeneratorTarget_cxx
+++ b/devel/cmake/patches/patch-Source_cmGeneratorTarget_cxx
@@ -1,57 +1,48 @@
-$OpenBSD: patch-Source_cmGeneratorTarget_cxx,v 1.16 2021/05/09 14:46:15
rsadowski Exp $
-
Index: Source/cmGeneratorTarget.cxx
--- Source/cmGeneratorTarget.cxx.orig
+++ Source/cmGeneratorTarget.cxx
-@@ -4810,9 +4810,14 @@ cmGeneratorTarget::Names cmGeneratorTarget::GetLibrary
- // Check for library version properties.
- cmProp version = this->GetProperty("VERSION");
- cmProp soversion = this->GetProperty("SOVERSION");
-+#if defined(__OpenBSD__)
-+ if (this->GetType() != cmStateEnums::SHARED_LIBRARY &&
-+ this->GetType() != cmStateEnums::MODULE_LIBRARY) {
-+#else
- if (!this->HasSOName(config) ||
- this->Makefile->IsOn("CMAKE_PLATFORM_NO_VERSIONED_SONAME") ||
- this->IsFrameworkOnApple()) {
-+#endif
- // Versioning is supported only for shared libraries and modules,
- // and then only when the platform supports an soname flag.
- version = nullptr;
-@@ -4836,6 +4841,36 @@ cmGeneratorTarget::Names cmGeneratorTarget::GetLibrary
-
+@@ -4837,6 +4837,44 @@ cmGeneratorTarget::Names cmGeneratorTarget::GetLibrary
// The library name.
targetNames.Output = prefix + targetNames.Base + suffix;
-+
+
+#if defined(__OpenBSD__)
+ // Override shared library version using LIBxxx_VERSION
+ // environment variable. Needed for OpenBSD ports system.
-+ if (this->GetType() == cmStateEnums::SHARED_LIBRARY ||
-+ this->GetType() == cmStateEnums::MODULE_LIBRARY) {
++ if (this->GetType() == cmStateEnums::SHARED_LIBRARY) {
++ // Target name libXXX with PREFIX set to ""
++ auto getLibName = [&](std::string& baseName) {
++ const std::string toMatch = "lib";
++ if (baseName.find(toMatch) == 0 && prefix == "")
++ return baseName.substr(toMatch.length());
++ return baseName;
++ };
+
-+ std::string env_vers;
-+ const std::string env_name("LIB" + targetNames.Base + "_VERSION");
-+ if (char * env = getenv(env_name.c_str()))
-+ env_vers = env;
++ std::string lib_version;
++ const std::string lib_name = "LIB" + getLibName(targetNames.Base) +
"_VERSION";
++ if (cmSystemTools::GetEnv(lib_name, lib_version)) {
++ if (!lib_version.empty()) {
++ const size_t first = lib_version.find_first_of(".");
++ const size_t last = lib_version.find_last_of(".");
+
-+ if (!env_vers.empty()) {
-+ const size_t first = env_vers.find_first_of(".");
-+ const size_t last = env_vers.find_last_of(".");
-+
-+ if ((first != last) || (first == std::string::npos)) {
-+ std::string msg = "Bad ";
-+ msg += env_name;
-+ msg += " specification: ";
-+ msg += env_vers;
-+ this->LocalGenerator->IssueMessage(MessageType::INTERNAL_ERROR, msg);
-+ } else {
-+ version = new std::string(env_vers);
-+ soversion = new std::string(env_vers);
++ if ((first != last) || (first == std::string::npos)) {
++ const std::string msg = "Bad " + lib_name + " specification: " +
lib_version;
++ this->LocalGenerator->IssueMessage(MessageType::INTERNAL_ERROR,
msg);
++ }
++ else {
++ version = new std::string(lib_version);
++ soversion = new std::string(lib_version);
++ }
++ }
++ }
++ else {
++ if (soversion) {
++ version = new std::string("0.0");
++ soversion = new std::string("0.0");
+ }
+ }
+ }
+#endif
+
-
if (this->IsFrameworkOnApple()) {
targetNames.Real = prefix;
+ if (!this->Makefile->PlatformIsAppleEmbedded()) {
