1.14.19: pkgconfig integration contains no version

[dvzrv]

Hi! :wave:

I package this project for Arch Linux.

With 1.14.19 we noticed that the pkgconfig file no longer carries version information (see https://gitlab.archlinux.org/archlinux/packaging/packages/libupnp/-/issues/1 for an upstream bug report).

We are always building from auto-generated source tarballs and therefore from our end nothing has changed since 1.14.18 (which still contains version information in the file).

cmake emits the following:

CMake Warning at CMakeLists.txt:13 (project):
  VERSION keyword not followed by a value or was followed by a value that
  expanded to nothing.

[dvzrv]

From initial investigation it appears that https://github.com/pupnp/pupnp/commit/470041b0af217af0dd3f85f36ff1d65139d14dc4 broke this.

https://github.com/pupnp/pupnp/blob/fb90d0ca1954bd2abf33b004e55eeb50c07cbb62/cmake/autoheader.cmake uses the files in question to derive the version via rather complex regexes. This seems rather complicated. Can an easier/ less brittle way be found to set the version?

[Vollstrecker]

Unfortunately there is no easier way as libtool calculates the so-version in some "interessting" way. But from a quick look (with my mobile), the overall version should be matched correct (1.14.19 in CMakeCache), only the versions for the libs itself rely on the old format.

Can you check if it works if you remove the spaces after the comma on line 24?

[loqs]

I am not sure I made the change you requested correctly:

diff --git a/cmake/autoheader.cmake b/cmake/autoheader.cmake
index df61622..883ff48 100644
--- a/cmake/autoheader.cmake
+++ b/cmake/autoheader.cmake
@@ -21,7 +21,7 @@ if (NOT PUPNP_VERSION_STRING)
            if (line MATCHES "AC_INIT.* ([0-9]*\\.[0-9]*\\.[0-9]*).*")
                message (STATUS "Setting package-version to ${CMAKE_MATCH_1}")
                set (PUPNP_VERSION_STRING ${CMAKE_MATCH_1} CACHE STRING "Version of the whole package" FORCE)
-           elseif (line MATCHES "[. \t]*AC_DEFINE_UNQUOTED *\\(([^,]*), *([^,]*), *([^\\)]*)")
+           elseif (line MATCHES "[. \t]*AC_DEFINE_UNQUOTED *\\(([^,]*),*([^,]*),*([^\\)]*)")
                set (SAVED_MATCH ${CMAKE_MATCH_1})

                if ("${CMAKE_MATCH_1}" IN_LIST WRITTEN_VARS)

The result is the different in that with the change applied the build fails with:

cd /build/libupnp/src/build/upnp && /usr/bin/cc -DLIBUPNP_EXPORTS -DNDEBUG -Dupnp_shared_EXPORTS -I/build/libupnp/src/pupnp/upnp/src/threadutil -I/build/libupnp/src/pupnp/upnp/inc -I/build/libupnp/src/pupnp/upnp/src/inc -I/build/libupnp/src/build -I/build/libupnp/src/build/upnp/inc -I/build/libupnp/src/pupnp/ixml/inc -I/build/libupnp/src/pupnp/ixml/src/inc -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security         -fstack-clash-protection -fcf-protection -g -ffile-prefix-map=/build/libupnp/src=/usr/src/debug/libupnp -flto=auto -fPIC -fvisibility=hidden -fmacro-prefix-map=/build/libupnp/src/pupnp/=. -MD -MT upnp/CMakeFiles/upnp_shared.dir/src/uuid/md5.c.o -MF CMakeFiles/upnp_shared.dir/src/uuid/md5.c.o.d -o CMakeFiles/upnp_shared.dir/src/uuid/md5.c.o -c /build/libupnp/src/pupnp/upnp/src/uuid/md5.c
/build/libupnp/src/pupnp/upnp/src/genlib/net/http/httpreadwrite.c: In function ‘get_sdk_info’:
/build/libupnp/src/build/autoconfig.h:5:29: error: too many decimal points in number
    5 | #define UPNP_VERSION_STRING 17.1.11
      |                             ^~~~~~~
/build/libupnp/src/pupnp/upnp/src/genlib/net/http/httpreadwrite.c:2311:28: note: in expansion of macro ‘UPNP_VERSION_STRING’
 2311 |                 "devices/" UPNP_VERSION_STRING "\r\n",
      |                            ^~~~~~~~~~~~~~~~~~~
/build/libupnp/src/pupnp/upnp/src/genlib/net/http/httpreadwrite.c:2311:27: error: expected ‘)’ before numeric constant
 2311 |                 "devices/" UPNP_VERSION_STRING "\r\n",
      |                           ^
      |                           )
/build/libupnp/src/pupnp/upnp/src/genlib/net/http/httpreadwrite.c:2308:17: note: to match this ‘(’
 2308 |         snprintf(info,
      |                 ^
/build/libupnp/src/pupnp/upnp/src/genlib/net/http/httpreadwrite.c:2310:19: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
 2310 |                 "%s/%s, UPnP/1.0, Portable SDK for UPnP "
      |                  ~^
      |                   |
      |                   char *
/build/libupnp/src/pupnp/upnp/src/genlib/net/http/httpreadwrite.c:2310:22: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
 2310 |                 "%s/%s, UPnP/1.0, Portable SDK for UPnP "
      |                     ~^
      |                      |
      |                      char *
In file included from /build/libupnp/src/pupnp/upnp/inc/upnp.h:47,
                 from /build/libupnp/src/pupnp/upnp/src/inc/upnputil.h:40,
                 from /build/libupnp/src/pupnp/upnp/src/inc/membuffer.h:40,
                 from /build/libupnp/src/pupnp/upnp/src/urlconfig/urlconfig.c:40:
/build/libupnp/src/build/upnp/inc/upnpconfig.h:41: warning: "UPNP_VERSION_STRING" redefined
   41 | #define UPNP_VERSION_STRING "17.1.11"
      | 
In file included from /build/libupnp/src/pupnp/upnp/src/inc/config.h:36,
                 from /build/libupnp/src/pupnp/upnp/src/urlconfig/urlconfig.c:37:
/build/libupnp/src/build/autoconfig.h:5: note: this is the location of the previous definition
    5 | #define UPNP_VERSION_STRING 17.1.11
      | 
make[2]: *** [upnp/CMakeFiles/upnp_shared.dir/build.make:328: upnp/CMakeFiles/upnp_shared.dir/src/genlib/net/http/httpreadwrite.c.o] Error 1

[loqs]

Did I misunderstand you and the change you meant was:

diff --git a/cmake/autoheader.cmake b/cmake/autoheader.cmake
index df616225..e79e963f 100644
--- a/cmake/autoheader.cmake
+++ b/cmake/autoheader.cmake
diff --git a/cmake/autoheader.cmake b/cmake/autoheader.cmake
index df616225..ce655a6e 100644
--- a/cmake/autoheader.cmake
+++ b/cmake/autoheader.cmake
@@ -18,7 +18,7 @@ if (NOT PUPNP_VERSION_STRING)
            string (REGEX REPLACE ";" "" line ${line})
            string (REGEX REPLACE "[ \t\r\n] *" " " line ${line})

-           if (line MATCHES "AC_INIT.* ([0-9]*\\.[0-9]*\\.[0-9]*).*")
+           if (line MATCHES "AC_INIT.*([0-9]+\\.[0-9]+\\.[0-9]+).*")
                message (STATUS "Setting package-version to ${CMAKE_MATCH_1}")
                set (PUPNP_VERSION_STRING ${CMAKE_MATCH_1} CACHE STRING "Version of the whole package" FORCE)
            elseif (line MATCHES "[. \t]*AC_DEFINE_UNQUOTED *\\(([^,]*), *([^,]*), *([^\\)]*)")

[mrjimenez]

Maybe we should put some comments on autoheader.cmake, it is hard to understand what those regex are doing. You need to master regex, M4, cmake, ... I can see that we are trying to get the version, but from where, in what order, from what part, etc, that would help a lot.

Best regards.

[Vollstrecker]

@loqs I meant the first change.

It seems there should be some quotes as the version STRING is interpreted as number. At least in autoconfig.h, as the mentioned redefinition is correct. So I guess the first regex change with the right quoting in the autoconfig.h.cm (rough guess as I didn't find any call regarding this), should fix this. Or the redefinition will just have another content.

I'm back home on monday to investigate this if it's not found till then.

@mrjimenez: Sure comments would help, I hoped to never touch this again, I remember a week where I tried to replicate the behaviour of libtool based on your description. Basically it iterates over the parts of the version in LT_SUBST (?) and does the math. If I am the one to fix that, I'll find the thread from these days and put in some explanations.

[loqs]

@Vollstrecker the second change I mentioned fixes it for me.

[mrjimenez]

I created this pull request #443, tell me what you think.

[mrjimenez]

Hi @Vollstrecker

@mrjimenez: Sure comments would help, I hoped to never touch this again, I remember a week where I tried to replicate the behaviour of libtool based on your description. Basically it iterates over the parts of the version in LT_SUBST (?) and does the math. If I am the one to fix that, I'll find the thread from these days and put in some explanations.

I feel your pain :laughing: , regexes per si are hell, quoted regexes are a torture...

[mrjimenez]

However, I did notice lots of warings like this on the cmake build:

[ 75%] Building C object upnp/CMakeFiles/upnp_static.dir/src/ssdp/ssdp_server.c.o

                 from /home/mroberto/programs/pupnp/maint/github-creator/upnp/src/inc/upnputil.h:40,
                 from /home/mroberto/programs/pupnp/maint/github-creator/upnp/src/inc/membuffer.h:40,
                 from /home/mroberto/programs/pupnp/maint/github-creator/upnp/src/inc/httpparser.h:41,
                 from /home/mroberto/programs/pupnp/maint/github-creator/upnp/src/inc/ssdplib.h:45,
                 from /home/mroberto/programs/pupnp/maint/github-creator/upnp/src/ssdp/ssdp_server.c:45:
/home/mroberto/programs/pupnp/maint/github-creator/upnp/inc/upnpconfig.h:42: warning: "UPNP_VERSION_STRING" redefined
   42 | #define UPNP_VERSION_STRING "1.14.20"
      | 
In file included from /home/mroberto/programs/pupnp/maint/github-creator/upnp/src/inc/config.h:36,
                 from /home/mroberto/programs/pupnp/maint/github-creator/upnp/src/ssdp/ssdp_server.c:41:
/home/mroberto/programs/pupnp/maint/github-creator/build/autoconfig.h:5: note: this is the location of the previous definition
    5 | #define UPNP_VERSION_STRING "17.1.11"
      | 
/home/mroberto/programs/pupnp/maint/github-creator/upnp/inc/upnpconfig.h:45: warning: "UPNP_VERSION_MAJOR" redefined
   45 | #define UPNP_VERSION_MAJOR 1
      | 
/home/mroberto/programs/pupnp/maint/github-creator/build/autoconfig.h:8: note: this is the location of the previous definition
    8 | #define UPNP_VERSION_MAJOR 17
      | 
/home/mroberto/programs/pupnp/maint/github-creator/upnp/inc/upnpconfig.h:48: warning: "UPNP_VERSION_MINOR" redefined
   48 | #define UPNP_VERSION_MINOR 14
      | 
/home/mroberto/programs/pupnp/maint/github-creator/build/autoconfig.h:11: note: this is the location of the previous definition
   11 | #define UPNP_VERSION_MINOR 1
      | 
/home/mroberto/programs/pupnp/maint/github-creator/upnp/inc/upnpconfig.h:51: warning: "UPNP_VERSION_PATCH" redefined
   51 | #define UPNP_VERSION_PATCH 20
      | 
/home/mroberto/programs/pupnp/maint/github-creator/build/autoconfig.h:18: note: this is the location of the previous definition
   18 | #define UPNP_VERSION_PATCH 11
      | 

Is this normal? Is there a fix?

[mrjimenez]

I've also created this pull request #444 to fix those annoying warnings, but it depends on #443.