Re: cmake sporadic library versioning problem

On Fri, Sep 11, 2020 at 08:11:09PM +0200, Rafael Sadowski wrote:

> On Thu Aug 13, 2020 at 04:47:47PM +0200, Christian Weisgerber wrote:
> > Occasionally, print/poppler fails to build with a curious error:
> >
> > A shared library has been built with the version number from
> > SHARED_LIBS, but during the fake stage this is forgotten and the
> > upstream version number used. This produces an error, because the
> > file doesn't exist.
> >
> > It only happens _sometimes_ and this has been going on for months.
> >
> > Now in my latest bulk build, x11/kde-applications/marble has
> > failed the same way:
> >
> > ===> build
> > c++ -fPIC -O2 -pipe -std=c++0x -DNDEBUG -shared -Wl,-soname,libmarblewidget-qt5.so.1.0 -o src/lib/marble/libmarblewidget-qt5.so.1.0 ...
> > ===> fake
> > CMake Error at src/lib/marble/cmake_install.cmake:41 (file):
> > file INSTALL cannot find
> > "/usr/obj/ports/marble-kf5-19.12.3/build-amd64/src/lib/marble/libmarblewidget-qt5.so.0.28.0":
> > No such file or directory.
> >
> > So whatever this is, it does not affect only poppler.
> >
> > The fact that this type of error happens only during _some_ bulk
> > builds suggests that it might be triggered by something else on the
> > machine. Another port that happens to be installed at the same
> > time, or something that is building at the same time.
> > (Cf. how "rm /tmp/conftest*" in misc/screen caused mysterious
> > configure failures in other ports.)
> >
>
> I need a little bit C background knowledge and/or feedback. Let's have a
> look at our LIBxxx_VERSION handling in cmake. Full handling below.
>
> Look at:
>
> + char *env_vers_cstr = getenv(env_name.c_str());
>
> and
>
> + version = env_vers_cstr;
> + soversion = env_vers_cstr;
>
> we set env_vers_cstr in version and soversion. This both are used deeper
> in CMake. So far so good.
>
> Is it possible that char* (env_vers_cstr) points to null or garbage
> because env changed? Or is this nonsense and this cannot happen and
> env_vers_cstr is always valid with the first match?

No, Posix allows previous values of getenv() to be invalidated by
subsequenct calls to setenv(), plus a few other conditions.

https://pubs.opengroup.org/onlinepubs/9699919799/

Our implementation does reallocate the interenal environment space if
needed.

-Otto

>
>
> $OpenBSD: patch-Source_cmGeneratorTarget_cxx,v 1.13 2020/05/25 05:12:00 rsadowski Exp $
>
> +#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) {
> + std::string env_name = "LIB" + targetNames.Base + "_VERSION";
> + char *env_vers_cstr = getenv(env_name.c_str());
> +
> + if (env_vers_cstr && strlen(env_vers_cstr) > 0) {
> + // This means an override is present.
> + std::string env_vers = std::string(env_vers_cstr);
> +
> + size_t first = env_vers.find_first_of(".");
> + 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 = env_vers_cstr;
> + soversion = env_vers_cstr;
> + }
> + }
> + }
> +