Bug 255725 - [Bug] pkg-config of librtlsdr (comms/rtl-sdr) unnecessarily forces -std=gnu89
Summary: [Bug] pkg-config of librtlsdr (comms/rtl-sdr) unnecessarily forces -std=gnu89
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Ganael LAPLANCHE
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-09 14:12 UTC by Nico Sonack
Modified: 2021-05-10 18:57 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (martymac)


Attachments
Proposed Patch (554 bytes, patch)
2021-05-09 14:12 UTC, Nico Sonack
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nico Sonack 2021-05-09 14:12:28 UTC
Created attachment 224791 [details]
Proposed Patch

When I tried to build a C99 compliant piece of C code that 
depended on librtlsdr, I ran into the problem that the pkg-config
of comms/rtl-sdr in /usr/local/libdata/pkgconfig/librtlsdr.pc 
caused all builds to fail, because of C standards violation, 
which is very annoying, as the original library doesn't seem
require GNU89 C.

Building rtl-sdr by hand from the upstream source and comparing
the two .pc files, I see:

[nico@hades ~/build/rtl-sdr]$ cat librtlsdr.pc
prefix=/usr/local
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: RTL-SDR Library
Description: C Utility Library
Version: 0.5.4.38-0847
Cflags: -I${includedir}/
Libs: -L${libdir} -lrtlsdr
Libs.private:  -lusb-1.0  -lusb 
[nico@hades ~/build/rtl-sdr]$ cat /usr/local/libdata/pkgconfig/librtlsdr.pc 
prefix=/usr/local
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: RTL-SDR Library
Description: C Utility Library
Version: UNKNOWN
Cflags: -I${includedir}/ -O2 -pipe  -fstack-protector-strong -fno-strict-aliasing -std=gnu89  
Libs: -L${libdir} -lrtlsdr
Libs.private:  -lusb 
[nico@hades ~/build/rtl-sdr]$ 

Having that in mind, I removed the line in the Makefile of the
port that forced the gnu89 "standard" and everything seems to be
working just fine.
Also, I noticed that in order to build the port, GNU make is not
required.

Attached I have a proposal for a patch to remove those strange
requirements. I have tested it on: 

[nico@hades /usr/ports/comms/rtl-sdr]$ uname -apKU
FreeBSD hades.herrhotzenplotz.geek 13.0-RELEASE FreeBSD 13.0-RELEASE #0 releng/13.0-n244733-ea31abc261f: Fri Apr  9 04:24:09 UTC 2021     root@releng1.nyi.freebsd.org:/usr/obj/usr/src/amd64.amd64/sys/GENERIC  amd64 amd64 1300139 1300139
[nico@hades /usr/ports/comms/rtl-sdr]$ 

If the dependency on GNU Make or GNU89 C is because of a different 
CPU architecture, I would suggest to make those requirements 
specific to the platform.

Please correct me if I am wrong with any of my previous statements.
Comment 1 Daniel Engberg freebsd_committer 2021-05-09 21:29:57 UTC
Should probably also use https://github.com/librtlsdr/librtlsdr as repo
Comment 2 Nico Sonack 2021-05-10 08:22:09 UTC
> Should probably also use https://github.com/librtlsdr/librtlsdr as repo

That indeed sounds like a plan. However when I look at the autotools build
of the upstream source, I can see that noone really seems to be concerned
about that because it is utterly broken. The option of less pain would be 
to use CMake as well.

Switching to CMake, the build is broken in another way as of v0.8.0 because 
of a missing include of limits.h in src/librtlsdr.c. I will go ahead and submit
a patch upstream. Then we might consider upgrading to version 0.8.0.
Comment 3 Ganael LAPLANCHE freebsd_committer 2021-05-10 10:19:22 UTC
Hello Nico,

Thanks for your report.

CFLAGS+=       -fgnu89-inline

was introduced in 6b27edfc052a4a7c1ffda25689a671e6b90f7b1f to fix build with Clang 4.0. It was later changed to

USE_CSTD=      gnu89

in 18878f8fafee33f540a03f2744f10716e416e044. We probably do not need it anymore, so your patch seems OK.

I'd prefer wait before switching to librtlsdr/librtlsdr to see how the project evolves as :

- this project is not Osmocom's official one
- it has not received updates since october
- most distros still use osmocom version

Best regards,

Ganael.
Comment 4 Nico Sonack 2021-05-10 10:32:17 UTC
Thank you for your reply and feedback!

Regarding the possible change to librtlsdr/librtlsdr and to people 
who read this out of interest:

I managed to fix the seriously broken things in the autotools build:
https://github.com/librtlsdr/librtlsdr/pull/110
I will see what is going to happen there...

Until then I am totally okay with staying at the official version and 
what most package repositories do.
Comment 5 commit-hook freebsd_committer 2021-05-10 10:33:22 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=e806a5b226b205e1c99b597e3ec4bcc90427b2bb

commit e806a5b226b205e1c99b597e3ec4bcc90427b2bb
Author:     Ganael LAPLANCHE <martymac@FreeBSD.org>
AuthorDate: 2021-05-10 10:22:58 +0000
Commit:     Ganael LAPLANCHE <martymac@FreeBSD.org>
CommitDate: 2021-05-10 10:32:41 +0000

    comms/rtl-sdr: Do not force gnu89 C std anymore

    - gnu89 do not seem to be needed anymore
    - also, remove useless dependency to gmake

    PR:             255725
    Reported by:    Nico Sonack <nsonack@outlook.com>

 comms/rtl-sdr/Makefile | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Comment 6 Ganael LAPLANCHE freebsd_committer 2021-05-10 16:37:36 UTC
> I managed to fix the seriously broken things in the autotools build:
> https://github.com/librtlsdr/librtlsdr/pull/110
> I will see what is going to happen there...

Thanks for the link, I'll keep an eye on it, but it seems that pull request has been closed meanwhile ?
Comment 7 Nico Sonack 2021-05-10 18:31:11 UTC
> but it seems that pull request has been closed meanwhile ?
Yes, I closed it again, but the patch is still available in case someone actually wants to fix it. Also, I can reopen the PR if necessary.

As the actual topic of this bug report has been addressed, I would suggest to close it. I feel like discussions on an update/change of the repo should be lead 
in a separate bug report.
Comment 8 Ganael LAPLANCHE freebsd_committer 2021-05-10 18:57:47 UTC
Yes, feel free to open a dedicated PR for that :)

Thanks again,

Best regards,

Ganael.