Bug 2229170

Summary: Review Request: libmms - Library for Microsoft Media Server (MMS) streaming protocol
Product: [Fedora] Fedora Reporter: Dominik 'Rathann' Mierzejewski <dominik>
Component: Package ReviewAssignee: Petr Pisar <ppisar>
Status: ASSIGNED --- QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: package-review, ppisar
Target Milestone: ---Flags: ppisar: fedora-review?
Target Release: ---   
Hardware: All   
OS: Linux   
URL: https://www.sf.net/projects/libmms
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 2218117    
Attachments:
Description Flags
The .spec file difference from Copr build 6241061 to 6304903 none

Description Dominik 'Rathann' Mierzejewski 2023-08-04 14:11:45 UTC 
Spec URL: https://rathann.fedorapeople.org/review/libmms/libmms.spec
SRPM URL: https://rathann.fedorapeople.org/review/libmms/libmms-0.6.4-22.fc38.src.rpm
Description:
MMS is a streaming protocol used in Microsoft server products, commonly used to
stream WMV data.  You can encounter mms:// style URLs all over the net,
especially on news sites and other content-serving sites. Libmms allows you to
download content from such sites, making it easy to add MMS support to your
media applications.

Fedora Account System Username: rathann
Comment 1 Fedora Review Service 2023-08-04 14:20:34 UTC 
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6241061
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2229170-libmms/fedora-rawhide-x86_64/06241061-libmms/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.
Comment 2 Petr Pisar 2023-08-10 14:42:21 UTC 
URL and Source0 addresses are Ok.
Source0 archive (SHA-512: 9771c697515f5232eaeaff79e68fe15e34d8aa38aa5d3d68525216357223f314c544d71b5fe18d79a77682f41b1b5d9fb5e7c3c99d0cba5723d2e3de31faec96) is original. Ok.
Summary verified from README. Ok.
Description verified from README. Ok.

Found licenses:

aclocal.m4: FSFULLRWD AND FSFULLR AND GPL-2.0-or-later WITH Libtool-exception AND FSFUL
config.guess: GPL-2.0-or-later WITH Autoconf-exception-generic
config.sub: GPL-2.0-or-later WITH Autoconf-exception-generic
configure: FSFUL AND GPL-2.0-or-later WITH Libtool-exception
COPYING.LIB: LGPL-2.1 text
depcomp: GPL-2.0-or-later WITH Autoconf-exception-generic
install: X11 AND LicenseRef-Fedora-Public-Domain
INSTALL: FSFULLR-like
ltmain.sh: GPL-2.0-or-later WITH Libtool-exception
Makefile.in: FSFULLRWD
missing: GPL-2.0-or-later WITH Autoconf-exception-generic
pkgconfig/Makefile.in: FSFULLRWD
README: LGPL
README.LICENSE: "Relicensed to LGPL"
src/asfheader.h: LGPL-2-or-later
src/bswap.h: LGPL-2-or-later
src/Makefile.in: FSFULLRWD
src/mms-common.h: LGPL-2-or-later
src/mms-common-funcs.h: LGPL-2-or-later
src/mms.c: LGPL-2-or-later
src/mms.h: LGPL-2-or-later
src/mmsx.c: LGPL-2-or-later
src/mmsh.c: LGPL-2-or-later
src/mmsh.h: LGPL-2-or-later
src/mmsio.h: LGPL-2-or-later
src/mmsx.h: LPGL-2-or-later
src/uri.c: LGPL-2-or-later
src/uri.h: LGPL-2-or-later
src/utf.c: LPGL-2-or-later
src/utf.h: LPGL-2-or-later

0002-Add-a-new-testfiledownload.c-example.patch: LGPL-2-or-later
0003-Fix-build-if-strndup-is-missing.patch: Unknown license!

FIX: Use "LGPL-2.1-or-later" in the License tag to match what is in COPYING.LIB file.
FIX: Find where 0003-Fix-build-if-strndup-is-missing.patch comes from. Now it's a piece of code without a license, hence Fedora cannot distribute it. I guess it comes from an old gcc/libiberty.

FIX: Build-require "coreutils" (libmms.spec:38).
FIX: Build-require "bash" (autogen.sh:1).
FIX: Build-require "autoconf" (autogen.sh:2).
FIX: Build-require "automake" (configure.in:3).

NEWS file is empty. Not packaged. Ok.
No upstream tests, no %check section. Ok.
Distribution compiler and linker flags are respected. Ok.

$ rpmlint libmms.spec ../SRPMS/libmms-0.6.4-22.fc40.src.rpm ../RPMS/x86_64/libmms-*
======================================== rpmlint session starts =======================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.12/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 6

libmms.x86_64: E: incorrect-fsf-address /usr/share/licenses/libmms/COPYING.LIB
libmms-devel.x86_64: E: incorrect-fsf-address /usr/include/libmms/mms.h
libmms-devel.x86_64: E: incorrect-fsf-address /usr/include/libmms/mmsh.h
libmms-devel.x86_64: E: incorrect-fsf-address /usr/include/libmms/mmsio.h
libmms-devel.x86_64: E: incorrect-fsf-address /usr/include/libmms/mmsx.h
========= 5 packages and 1 specfiles checked; 5 errors, 0 warnings, 5 badness; has taken 0.4 s ========
rpmlint is Ok.

TODO: Ask upstream to updated FSF address at <https://sourceforge.net/p/libmms/bugs/>.
TODO: Link 0005-Avoid-possible-overflow-in-sprintf.patch to <https://sourceforge.net/p/libmms/bugs/23/> and share the patch there.
TODO: Link 0006-Fix-possible-NULL-Pointer-deref-in-mmsh.c.patch to <https://sourceforge.net/p/libmms/bugs/18/>.

$ rpm -q -lv -p  ../RPMS/x86_64/libmms-0.6.4-22.fc40.x86_64.rpm 
drwxr-xr-x    2 root     root                        0 Aug  4 02:00 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Aug  4 02:00 /usr/lib/.build-id/97
lrwxrwxrwx    1 root     root                       37 Aug  4 02:00 /usr/lib/.build-id/97/6ca36b69649329389f177457a7997a9ff7686e -> ../../../../usr/lib64/libmms.so.0.0.2
lrwxrwxrwx    1 root     root                       15 Aug  4 02:00 /usr/lib64/libmms.so.0 -> libmms.so.0.0.2
-rwxr-xr-x    1 root     root                    73816 Aug  4 02:00 /usr/lib64/libmms.so.0.0.2
drwxr-xr-x    2 root     root                        0 Aug  4 02:00 /usr/share/doc/libmms
-rw-r--r--    1 root     root                      291 Mar 28  2014 /usr/share/doc/libmms/AUTHORS
-rw-r--r--    1 root     root                    16108 Apr  9  2014 /usr/share/doc/libmms/ChangeLog
-rw-r--r--    1 root     root                     2273 Mar 28  2014 /usr/share/doc/libmms/README
-rw-r--r--    1 root     root                      264 Mar 28  2014 /usr/share/doc/libmms/README.LICENSE
drwxr-xr-x    2 root     root                        0 Aug  4 02:00 /usr/share/licenses/libmms
-rw-r--r--    1 root     root                    26528 Mar 28  2014 /usr/share/licenses/libmms/COPYING.LIB
$ rpm -q -lv -p  ../RPMS/x86_64/libmms-devel-0.6.4-22.fc40.x86_64.rpm 
drwxr-xr-x    2 root     root                        0 Aug  4 02:00 /usr/include/libmms
-rw-r--r--    1 root     root                     3009 Mar 28  2014 /usr/include/libmms/mms.h
-rw-r--r--    1 root     root                      221 Apr  9  2014 /usr/include/libmms/mms_config.h
-rw-r--r--    1 root     root                     2224 Mar 28  2014 /usr/include/libmms/mmsh.h
-rw-r--r--    1 root     root                     3758 Mar 28  2014 /usr/include/libmms/mmsio.h
-rw-r--r--    1 root     root                     2316 Mar 28  2014 /usr/include/libmms/mmsx.h
lrwxrwxrwx    1 root     root                       15 Aug  4 02:00 /usr/lib64/libmms.so -> libmms.so.0.0.2
-rw-r--r--    1 root     root                      204 Aug  4 02:00 /usr/lib64/pkgconfig/libmms.pc
File layout and permissions are Ok.

$ rpm -q --requires -p  ../RPMS/x86_64/libmms-0.6.4-22.fc40.x86_64.rpm | sort -f | uniq -c 
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.15)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3)(64bit)
      1 libc.so.6(GLIBC_2.3.4)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libc.so.6(GLIBC_2.7)(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsZstd) <= 5.4.18-1
      1 rtld(GNU_HASH)
$ rpm -q --requires -p  ../RPMS/x86_64/libmms-devel-0.6.4-22.fc40.x86_64.rpm | sort -f | uniq -c 
      1 /usr/bin/pkg-config
      1 libmms(x86-64) = 0.6.4-22.fc40
      1 libmms.so.0()(64bit)
      1 pkgconfig
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsZstd) <= 5.4.18-1
FIX: Run-require "pkgconf-pkg-config" instead of "pkgconfig". /usr/lib64/pkgconfig is owned by pkgconf-pkg-config.

$ rpm -q --provides -p  ../RPMS/x86_64/libmms-0.6.4-22.fc40.x86_64.rpm | sort -f | uniq -c 
      1 libmms = 0.6.4-22.fc40
      1 libmms(x86-64) = 0.6.4-22.fc40
      1 libmms.so.0()(64bit)
$ rpm -q --provides -p  ../RPMS/x86_64/libmms-devel-0.6.4-22.fc40.x86_64.rpm | sort -f | uniq -c 
      1 libmms-devel = 0.6.4-22.fc40
      1 libmms-devel(x86-64) = 0.6.4-22.fc40
      1 pkgconfig(libmms) = 0.6.4
Binary provides are Ok.

$ resolvedeps rawhide  ../RPMS/x86_64/libmms-{,devel-}0.6.4-22.fc40.x86_64.rpm 
Binary dependencies are resolvable. Ok.

The package builds in Fedora 40 (https://koji.fedoraproject.org/koji/taskinfo?taskID=104639563). Ok.

Otherwise, the package is in line with Fedora packaging guidelines.
Please correct the "FIX" items, consider fixing "TODO" items, and provide a new spec file.
Comment 3 Dominik 'Rathann' Mierzejewski 2023-08-16 13:37:46 UTC 
(In reply to Petr Pisar from comment #2)
> Found licenses:
> 
> aclocal.m4: FSFULLRWD AND FSFULLR AND GPL-2.0-or-later WITH
> Libtool-exception AND FSFUL
> config.guess: GPL-2.0-or-later WITH Autoconf-exception-generic
> config.sub: GPL-2.0-or-later WITH Autoconf-exception-generic
> configure: FSFUL AND GPL-2.0-or-later WITH Libtool-exception
> depcomp: GPL-2.0-or-later WITH Autoconf-exception-generic
> install: X11 AND LicenseRef-Fedora-Public-Domain
> INSTALL: FSFULLR-like
> ltmain.sh: GPL-2.0-or-later WITH Libtool-exception
> Makefile.in: FSFULLRWD
> missing: GPL-2.0-or-later WITH Autoconf-exception-generic
> pkgconfig/Makefile.in: FSFULLRWD
> src/Makefile.in: FSFULLRWD

The above do not end up included in binary RPM, so are irrelevant.

[...] 
> 0002-Add-a-new-testfiledownload.c-example.patch: LGPL-2-or-later

This is upstream commit: https://sourceforge.net/p/libmms/code/ci/34060b0c0cb13eed323577becf72a13b43654c00/

> 0003-Fix-build-if-strndup-is-missing.patch: Unknown license!

This is upstream commit: https://sourceforge.net/p/libmms/code/ci/67d54003b8075b8ea8102bc4a808df4543ab113a/
The strndup implementation is not used in the binary, because glibc provides one.

> FIX: Use "LGPL-2.1-or-later" in the License tag to match what is in
> COPYING.LIB file.

Done.

> FIX: Find where 0003-Fix-build-if-strndup-is-missing.patch comes from. Now
> it's a piece of code without a license, hence Fedora cannot distribute it. I
> guess it comes from an old gcc/libiberty.

As above, this is already upstream, but not used in the binary.

> FIX: Build-require "coreutils" (libmms.spec:38).
> FIX: Build-require "bash" (autogen.sh:1).

Unnecessary. These are already present in the initial buildroot.

> FIX: Build-require "autoconf" (autogen.sh:2).
> FIX: Build-require "automake" (configure.in:3).

These are pulled in by libtool, no need to spell them out.

> TODO: Ask upstream to updated FSF address at
> <https://sourceforge.net/p/libmms/bugs/>.

Sure.

> TODO: Link 0005-Avoid-possible-overflow-in-sprintf.patch to
> <https://sourceforge.net/p/libmms/bugs/23/> and share the patch there.

This is upstream already: https://sourceforge.net/p/libmms/code/ci/8b5e303fc1f01521c727e351270dd68c4f15190b/

> TODO: Link 0006-Fix-possible-NULL-Pointer-deref-in-mmsh.c.patch to
> <https://sourceforge.net/p/libmms/bugs/18/>.

This is upstream already: https://sourceforge.net/p/libmms/code/ci/5cface3df0e0213d8bc593d82a9a7c1e648dd71a/

[...]
> $ rpm -q --requires -p  ../RPMS/x86_64/libmms-devel-0.6.4-22.fc40.x86_64.rpm
> | sort -f | uniq -c 
>       1 /usr/bin/pkg-config
>       1 libmms(x86-64) = 0.6.4-22.fc40
>       1 libmms.so.0()(64bit)
>       1 pkgconfig
>       1 rpmlib(CompressedFileNames) <= 3.0.4-1
>       1 rpmlib(FileDigests) <= 4.6.0-1
>       1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
>       1 rpmlib(PayloadIsZstd) <= 5.4.18-1
> FIX: Run-require "pkgconf-pkg-config" instead of "pkgconfig".
> /usr/lib64/pkgconfig is owned by pkgconf-pkg-config.

pkgconfig-pkg-config provides pkgconfig, so... why?

[...]
> Otherwise, the package is in line with Fedora packaging guidelines.
> Please correct the "FIX" items, consider fixing "TODO" items, and provide a
> new spec file.

Thank you for the review!

Spec URL: https://rathann.fedorapeople.org/review/libmms/libmms.spec
SRPM URL: https://rathann.fedorapeople.org/review/libmms/libmms-0.6.4-23.fc38.src.rpm

* Wed Aug 16 2023 Dominik Mierzejewski <dominik> - 0.6.4-23
- fix license field
Comment 4 Fedora Review Service 2023-08-16 13:49:37 UTC 
Created attachment 1983611 [details]
The .spec file difference from Copr build 6241061 to 6304903
Comment 5 Fedora Review Service 2023-08-16 13:49:39 UTC 
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6304903
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2229170-libmms/fedora-rawhide-x86_64/06304903-libmms/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.
Comment 6 Petr Pisar 2023-08-17 11:40:00 UTC 
(In reply to Dominik 'Rathann' Mierzejewski from comment #3)
> (In reply to Petr Pisar from comment #2)
> > Found licenses:
> > 
> > aclocal.m4: FSFULLRWD AND FSFULLR AND GPL-2.0-or-later WITH
> > Libtool-exception AND FSFUL
> > config.guess: GPL-2.0-or-later WITH Autoconf-exception-generic
> > config.sub: GPL-2.0-or-later WITH Autoconf-exception-generic
> > configure: FSFUL AND GPL-2.0-or-later WITH Libtool-exception
> > depcomp: GPL-2.0-or-later WITH Autoconf-exception-generic
> > install: X11 AND LicenseRef-Fedora-Public-Domain
> > INSTALL: FSFULLR-like
> > ltmain.sh: GPL-2.0-or-later WITH Libtool-exception
> > Makefile.in: FSFULLRWD
> > missing: GPL-2.0-or-later WITH Autoconf-exception-generic
> > pkgconfig/Makefile.in: FSFULLRWD
> > src/Makefile.in: FSFULLRWD
> 
> The above do not end up included in binary RPM, so are irrelevant.

Those are relevant for source RPM. Therefore I enumerated them here.

> > 0002-Add-a-new-testfiledownload.c-example.patch: LGPL-2-or-later
> 
> This is upstream commit:
> https://sourceforge.net/p/libmms/code/ci/
> 34060b0c0cb13eed323577becf72a13b43654c00/
> 
> > 0003-Fix-build-if-strndup-is-missing.patch: Unknown license!
> 
> This is upstream commit:
> https://sourceforge.net/p/libmms/code/ci/
> 67d54003b8075b8ea8102bc4a808df4543ab113a/
> The strndup implementation is not used in the binary, because glibc provides
> one.
> 
[...]
> > FIX: Find where 0003-Fix-build-if-strndup-is-missing.patch comes from. Now
> > it's a piece of code without a license, hence Fedora cannot distribute it. I
> > guess it comes from an old gcc/libiberty.
> 
> As above, this is already upstream, but not used in the binary.
>
[...]
> > TODO: Link 0005-Avoid-possible-overflow-in-sprintf.patch to
> > <https://sourceforge.net/p/libmms/bugs/23/> and share the patch there.
> 
> This is upstream already:
> https://sourceforge.net/p/libmms/code/ci/
> 8b5e303fc1f01521c727e351270dd68c4f15190b/
> 
> > TODO: Link 0006-Fix-possible-NULL-Pointer-deref-in-mmsh.c.patch to
> > <https://sourceforge.net/p/libmms/bugs/18/>.
> 
> This is upstream already:
> https://sourceforge.net/p/libmms/code/ci/
> 5cface3df0e0213d8bc593d82a9a7c1e648dd71a/
> 
Could you annotate an origin of the patches in the spec file <https://docs.fedoraproject.org/en-US/packaging-guidelines/PatchUpstreamStatus/>?

> > FIX: Build-require "coreutils" (libmms.spec:38).
> > FIX: Build-require "bash" (autogen.sh:1).
> 
> Unnecessary. These are already present in the initial buildroot.
> 
> > FIX: Build-require "autoconf" (autogen.sh:2).
> > FIX: Build-require "automake" (configure.in:3).
> 
> These are pulled in by libtool, no need to spell them out.
>
Initial build root and transitive dependencies are not a valid argument. Only transitive dependencies of rpm, rpmbuild, and bash can be implicit. All other dependencies must be explicitly listed <https://docs.fedoraproject.org/en-US/packaging-guidelines/#buildrequires>. You are right that bash and coreutils are already there.

FIX: Build-require 'autoconf' and 'automake'.

> > FIX: Run-require "pkgconf-pkg-config" instead of "pkgconfig".
> > /usr/lib64/pkgconfig is owned by pkgconf-pkg-config.
> 
> pkgconfig-pkg-config provides pkgconfig, so... why?
> 
Because it does not rely on a historical name which is provided there only not to break compatibility with old packages. pkgconfig-pkg-config obsoleted pkgconfig in Fedora.

$ rpmlint libmms.spec ../SRPMS/libmms-0.6.4-23.fc40.src.rpm ../RPMS/x86_64/libmms-*
======================================== rpmlint session starts =======================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.12/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 6

libmms.x86_64: E: incorrect-fsf-address /usr/share/licenses/libmms/COPYING.LIB
libmms-devel.x86_64: E: incorrect-fsf-address /usr/include/libmms/mms.h
libmms-devel.x86_64: E: incorrect-fsf-address /usr/include/libmms/mmsh.h
libmms-devel.x86_64: E: incorrect-fsf-address /usr/include/libmms/mmsio.h
libmms-devel.x86_64: E: incorrect-fsf-address /usr/include/libmms/mmsx.h
========= 5 packages and 1 specfiles checked; 5 errors, 0 warnings, 5 badness; has taken 0.4 s ========

rpmlint output is OK.

The package builds in Fedora 40 <https://koji.fedoraproject.org/koji/taskinfo?taskID=104954866>. Ok.

Please add the two dependencies and I will approve this package.