libupnp built with cmake is not functional

[kyak]

Hi,

There has been a lengthy discussion/investigation in https://github.com/gerbera/gerbera/issues/2000. But it has boiled down to the fact that when I configure libupnp with cmake, then gerbera is not functional. Whenever I configure libupnp with autoconf, then gerbera works fine.

I'm using Arch Linux ARM on armv7.

libupnp versions that I've tried are 1.14.12 and 1.14.7.

If there is additional information that you need to help solve this problem, I'm ready to provide it.

I want to add that tests (make check) pass no matter how I configure - with cmake or autoconf.

[kyak]

One huge difference between cmake and autoconf configuration is that autoconf defines large file support:

define UPNP_LARGEFILE_SENSITIVE 1

While cmake doesn't.

I don't know yet if this is the reason why gerbera doesn't work, because I can't force cmake detect large file support. -DUPNP_LARGEFILE_SENSITIVE=TRUE doesn't do anything.

[whyman]

Yep, looks like the LFS flags are indeed not set, but the header is correctly populated.

We use this FindLFS module in Gerbera for the same: https://github.com/gerbera/gerbera/blob/master/cmake/FindLFS.cmake

[Vollstrecker]

Iirc I just didn't implement the test and assume that support is present. Just because I found nowhere any hint about systems where this support is missing and I don't wrtie checks where I can't check if they fail correctly.

[kyak]

Yep, looks like the LFS flags are indeed not set, but the header is correctly populated.

The upnpconfig.h header generated by cmake build doesn't specify UPNP_LARGEFILE_SENSITIVE. Why do you say it is correctly populated?

Iirc I just didn't implement the test and assume that support is present.

I don't get it. If cmake build assumes that LFS support is present, why it doesn't populate header file properly?

[whyman]

Iirc I just didn't implement the test and assume that support is present. Just because I found nowhere any hint about systems where this support is missing and I don't wrtie checks where I can't check if they fail correctly.

This affects any 32 bit systems. So x86, up to armv7 etc

[Vollstrecker]

The pr should bring all needed flags and can be tested. Is UPNP_LARGEFILE_SENSITIVE needed in the exports defined also? I mean the var so other projects can check that. My guess would be no, as they should get the flags from the interface if they are needed.

[whyman]

I think the UPNP_LARGEFILE_SENSITIVE needs to be in the headers so users can check its status, because it has to match between client and the library or you get errors.

[kyak]

Unfortunately, it doesn't work. The LFS support is still not detected when doing cmake build.

I will be happy to provide more information. What do you need?

[Vollstrecker]

Information about how you use libupnp, idealy what is missing. If you say it is not detected, what do you mean with that?

[Vollstrecker]

oh, and if you compile libupnp, the output of the cmake run would be nice.The console-output, not the logs.

[mrjimenez]

Can we add a minimal test to our test suite for this case?

[kyak]

Here is the output of cmake:

-- Found Git: /usr/bin/git (found version "2.34.1")
-- Setting package-version to 1.14.12
-- Setting ixml-soversion to 11.1.2
-- Setting upnp-soversion to 17.1.3
-- The C compiler identification is GNU 10.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Looking for ws2tcpip.h
-- Looking for ws2tcpip.h - not found
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of socklen_t
-- Check size of socklen_t - done
-- Check size of off_t
-- Check size of off_t - done
-- Looking for fseeko
-- Looking for fseeko - found
-- Looking for strnlen
-- Looking for strnlen - found
-- Looking for strndup
-- Looking for strndup - found
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
-- Check if compiler accepts -pthread
-- Check if compiler accepts -pthread - yes
-- Found Threads: TRUE
-- Check size of pthread_rwlock_t
-- Check size of pthread_rwlock_t - done
-- Configuring done
-- Generating done

I'm using libupnp with gerbera. More specifically, I observe that gerbera doesn't play files when libupnp has been configured with cmake.

When I say that LFS support is not being detected I mean that CMakeCache.txt contains UPNP_LARGEFILE_SENSITIVE:BOOL=FALSE.

[Vollstrecker]

Can we add a minimal test to our test suite for this case?

Sure, someone that knows how missing LFS looks. I think we can just try to us it if it should be present, then it fails if it's not. But to be honest, I have no clue about this topic.

The log seem to not execute some lines, compared to here: https://github.com/pupnp/pupnp/runs/4729558703?check_suite_focus=true it jumps from 25 to 27. The checks for lfs are exactly there. I'll take a look tomorrow.

[Vollstrecker]

I just found that my raspi is a machine where I can test this with.

The interesting part first. The check gives it's result in OFF_T_SIZE, but it also sets HAVE_OFF_T_SIZE, and when that one is set, it just doesn't repeat the test.

And while it's ok to give _FILE_OFF_SET_BITS=64 to target_compile_definitions, this test needs -D_FILE_OFFSET_BITS=64, hopefully this doesn't kill the test on win.

After all I changed that _LARGE_FILES is only defined if LARGE_FILES is really needed. The UPNP_LARGEFILE_SENSITIVE is set in both cases, so importing projects can just see that there are flags needed. If they use other names for their defines, they will have to check anyways.

Long story short, I based the fix on 1.14.x branch, so you should be able now to test this without merging first. https://github.com/pupnp/pupnp/tree/1.14-lfs-fix

[kyak]

Thanks a lot! I can confirm that it is working fine.