Bug 2267321

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7095301
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2267321-rtmpdump/fedora-rawhide-x86_64/07095301-rtmpdump/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

> Version:        2.4
> Release:        25.%{gitdate}git%{shortcommit}%{?dist}

Please switch to snapshot versioning in the Version field.

Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots

> %install
> make CRYPTO=GNUTLS DESTDIR=%{buildroot} prefix=%{_prefix} mandir=%{_mandir} libdir=%{_libdir} install

Please use %make_install and split the long line

(In reply to Neal Gompa from comment #2)
> > Version:        2.4
> > Release:        25.%{gitdate}git%{shortcommit}%{?dist}
> 
> Please switch to snapshot versioning in the Version field.
> 
> Cf.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> #_snapshots

No. This is a post-release snapshot and upstream has just announced the intention to release 2.6:
https://lists.mplayerhq.hu/pipermail/rtmpdump/2024-March/002554.html

(In reply to Xavier Bachelot from comment #3)
> > %install
> > make CRYPTO=GNUTLS DESTDIR=%{buildroot} prefix=%{_prefix} mandir=%{_mandir} libdir=%{_libdir} install
> 
> Please use %make_install and split the long line

OK. Anything else?

(In reply to Dominik 'Rathann' Mierzejewski from comment #4)
> (In reply to Neal Gompa from comment #2)
> > > Version:        2.4
> > > Release:        25.%{gitdate}git%{shortcommit}%{?dist}
> > 
> > Please switch to snapshot versioning in the Version field.
> > 
> > Cf.
> > https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> > #_snapshots
> 
> No. This is a post-release snapshot and upstream has just announced the
> intention to release 2.6:
> https://lists.mplayerhq.hu/pipermail/rtmpdump/2024-March/002554.html

On second thought, OK.

(In reply to Xavier Bachelot from comment #3)
> > %install
> > make CRYPTO=GNUTLS DESTDIR=%{buildroot} prefix=%{_prefix} mandir=%{_mandir} libdir=%{_libdir} install
> 
> Please use %make_install and split the long line

Done. It's no longer so long after the change. :)

Spec URL: https://rathann.fedorapeople.org/review/rtmpdump/rtmpdump.spec
SRPM URL: https://rathann.fedorapeople.org/review/rtmpdump/rtmpdump-2.4%5e20240228gitb7c7976-1.fc41.src.rpm

* Tue Mar 05 2024 Dominik Mierzejewski <dominik> - 2.4^20240228gitb7c7976-1
- use recommended snapshot versioning
- use modern macros

Created attachment 2020126 [details]
The .spec file difference from Copr build 7095301 to 7106479

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7106479
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2267321-rtmpdump/fedora-rawhide-x86_64/07106479-rtmpdump/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

[fedora-review-service-build]

- ensure Fedora CFLAGS and LDFLAGS are used consistently

Created attachment 2020134 [details]
The .spec file difference from Copr build 7106479 to 7106653

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7106653
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2267321-rtmpdump/fedora-rawhide-x86_64/07106653-rtmpdump/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Maybe missing an explicit BR: make rather than relying on deps ?
Otherwise looks good, will try and do a full review asap.

(In reply to Fedora Review Service from comment #13)
> Copr build:
> https://copr.fedorainfracloud.org/coprs/build/7106653
> (succeeded)
> 
> Review template:
> https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-
> review-2267321-rtmpdump/fedora-rawhide-x86_64/07106653-rtmpdump/fedora-
> review/review.txt
> 
> Please take a look if any issues were found.
> 
> 
> ---
> This comment was created by the fedora-review-service
> https://github.com/FrostyX/fedora-review-service https://fnafgames.io
> 
> If you want to trigger a new Copr build, add a comment containing new
> Spec and SRPM URLs or [fedora-review-service-build] string.

Thanks for the info

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7158010
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2267321-rtmpdump/fedora-rawhide-x86_64/07158010-rtmpdump/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Could you please take a look at the following rpmlint warnings ?
librtmp.x86_64: W: crypto-policy-non-compliance-gnutls-2 /usr/lib64/librtmp.so.1 gnutls_priority_init
librtmp.x86_64: W: crypto-policy-non-compliance-gnutls-1 /usr/lib64/librtmp.so.1 gnutls_priority_set_direct

See https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/

And also :
librtmp.x86_64: W: binary-or-shlib-calls-gethostbyname /usr/lib64/librtmp.so.1

Both shall probably at least be notified to upstream.

(In reply to Xavier Bachelot from comment #14)
> Maybe missing an explicit BR: make rather than relying on deps ?

Good point, will add when importing.

> Otherwise looks good, will try and do a full review asap.

Thanks!

(In reply to Xavier Bachelot from comment #17)
> Could you please take a look at the following rpmlint warnings ?
> librtmp.x86_64: W: crypto-policy-non-compliance-gnutls-2
> /usr/lib64/librtmp.so.1 gnutls_priority_init
> librtmp.x86_64: W: crypto-policy-non-compliance-gnutls-1
> /usr/lib64/librtmp.so.1 gnutls_priority_set_direct
> 
> See https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/
> 
> And also :
> librtmp.x86_64: W: binary-or-shlib-calls-gethostbyname
> /usr/lib64/librtmp.so.1
> 
> Both shall probably at least be notified to upstream.

They are valid and I sent them upstream. However, I wonder if RTMP
even supports IPv6 or if there are specific old crypto
requirements at work here.

rtmpdump upstream advice is not to include it in Fedora at all.

Only two RPM Fusion packages depend on rtmpdump: get-flash-videos and qarte.

First is dead (last commit 4 years ago) and the second is specific to one website (arte.tv).

So... probably not worth packaging. Withdrawing review request.