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()) {