Spec URL: https://farchord.fedorapeople.org/reviews/libspeechprovider/libspeechprovider.spec SRPM URL: https://farchord.fedorapeople.org/reviews/libspeechprovider/libspeechprovider-1.0.3-1.fc41.src.rpm Description: This project includes the resources needed for building a Spiel speech provider. Fedora Account System Username: farchord
Copr build: https://copr.fedorainfracloud.org/coprs/build/8500490 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2337050-libspeechprovider/fedora-rawhide-x86_64/08500490-libspeechprovider/fedora-review/review.txt Found issues: - libspeechprovider : /usr/include/speech-provider/speech-provider-common.h libspeechprovider : /usr/include/speech-provider/speech-provider-dbus-enums.h libspeechprovider : /usr/include/speech-provider/speech-provider-stream-reader.h libspeechprovider : /usr/include/speech-provider/speech-provider-stream-writer.h libspeechprovider : /usr/include/speech-provider/speech-provider-version.h libspeechprovider : /usr/include/speech-provider/speech-provider.h Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Please know that there can be false-positives. --- 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.
The URL appears to be for a different project. Instead of `convert_version`, I would name it `tag` and use Lua to avoid an external process: %global tag %{lua: print("SPEECHPROVIDER_" .. string.gsub(macros.version, "%.", "_"))} As noted by fedora-review, headers should go in a `-devel` package, as should pkgconfig. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Does the shared library not set a soversion? https://docs.fedoraproject.org/en-US/packaging-guidelines/#_downstream_so_name_versioning Is `cmake` really needed, as the build system is `meson`, and there are no CMake dependencies listed? Also consider using %autorelease/%autochangelog.
(In reply to Elliott Sales de Andrade from comment #2) > The URL appears to be for a different project. > > Instead of `convert_version`, I would name it `tag` and use Lua to avoid an > external process: > %global tag %{lua: print("SPEECHPROVIDER_" .. string.gsub(macros.version, > "%.", "_"))} > > As noted by fedora-review, headers should go in a `-devel` package, as > should pkgconfig. > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > > Does the shared library not set a soversion? > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_downstream_so_name_versioning > > Is `cmake` really needed, as the build system is `meson`, and there are no > CMake dependencies listed? > > Also consider using %autorelease/%autochangelog. 1- My logic for not making a -devel subpackage is that this package itself is a subpackage. There is no shared libraries it would seem. 2- cmake is indeed necessary. Build fails if it's not provided. 3- Using autorelease/autochangelog has drawbacks and I usually avoid it. I'll look into the macro in a bit
Spec URL: https://farchord.fedorapeople.org/reviews/libspeechprovider/libspeechprovider.spec SRPM URL: https://farchord.fedorapeople.org/reviews/libspeechprovider/libspeechprovider-1.0.3-1.fc41.src.rpm Updated tag, fixed project url
(In reply to Steve Cossette from comment #3) > 1- My logic for not making a -devel subpackage is that this package itself > is a subpackage. There is no shared libraries it would seem. What do you mean? You have: %{_libdir}/libspeech-provider-1.0.so > 2- cmake is indeed necessary. Build fails if it's not provided. That was in fact a rhetorical question; I already tested without the dependency and it built fine, so I don't know what failure you're seeing. > 3- Using autorelease/autochangelog has drawbacks and I usually avoid it. If you say so. > I'll look into the macro in a bit
Created attachment 2065439 [details] The .spec file difference from Copr build 8500490 to 8500574
Copr build: https://copr.fedorainfracloud.org/coprs/build/8500574 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2337050-libspeechprovider/fedora-rawhide-x86_64/08500574-libspeechprovider/fedora-review/review.txt Found issues: - libspeechprovider : /usr/include/speech-provider/speech-provider-common.h libspeechprovider : /usr/include/speech-provider/speech-provider-dbus-enums.h libspeechprovider : /usr/include/speech-provider/speech-provider-stream-reader.h libspeechprovider : /usr/include/speech-provider/speech-provider-stream-writer.h libspeechprovider : /usr/include/speech-provider/speech-provider-version.h libspeechprovider : /usr/include/speech-provider/speech-provider.h Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Please know that there can be false-positives. --- 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.
Unless I'm completely misunderstanding, or I'm using the wrong words here, if a library ends in .so, it's usually a development library. If it has a number after, then it's an installable library. I might be wrong, I guess? As far as the cmake dependancy goes: ``` + /usr/bin/meson setup --buildtype=plain --prefix=/usr --libdir=/usr/lib64 --libexecdir=/usr/libexec --bindir=/usr/bin --sbindir=/usr/sbin --includedir=/usr/include --datadir=/usr/share --mandir=/usr/share/man --infodir=/usr/share/info --localedir=/usr/share/locale --sysconfdir=/etc --localstatedir=/var --sharedstatedir=/var/lib --wrap-mode=nodownload --auto-features=enabled . redhat-linux-build The Meson build system Version: 1.6.1 Source dir: /builddir/build/BUILD/libspeechprovider-1.0.3-build/libspeechprovider-SPEECHPROVIDER_1_0_3 Build dir: /builddir/build/BUILD/libspeechprovider-1.0.3-build/libspeechprovider-SPEECHPROVIDER_1_0_3/redhat-linux-build Build type: native build Project name: libspeechprovider Project version: 1.0.3 C compiler for the host machine: gcc (gcc 14.2.1 "gcc (GCC) 14.2.1 20241104 (Red Hat 14.2.1-6)") C linker for the host machine: gcc ld.bfd 2.43.50.20241126 Host machine cpu family: x86_64 Host machine cpu: x86_64 Configuring config.h using configuration Compiler for C supports arguments -Wcast-align: YES Compiler for C supports arguments -Wdeclaration-after-statement: YES Compiler for C supports arguments -Werror=address: YES Compiler for C supports arguments -Werror=array-bounds: YES Compiler for C supports arguments -Werror=empty-body: YES Compiler for C supports arguments -Werror=implicit: YES Compiler for C supports arguments -Werror=implicit-function-declaration: YES Compiler for C supports arguments -Werror=incompatible-pointer-types: YES Compiler for C supports arguments -Werror=init-self: YES Compiler for C supports arguments -Werror=int-conversion: YES Compiler for C supports arguments -Werror=int-to-pointer-cast: YES Compiler for C supports arguments -Werror=main: YES Compiler for C supports arguments -Werror=misleading-indentation: YES Compiler for C supports arguments -Werror=missing-braces: YES Compiler for C supports arguments -Werror=missing-include-dirs: YES Compiler for C supports arguments -Werror=nonnull: YES Compiler for C supports arguments -Werror=overflow: YES Compiler for C supports arguments -Werror=parenthesis: NO Compiler for C supports arguments -Werror=pointer-arith: YES Compiler for C supports arguments -Werror=pointer-to-int-cast: YES Compiler for C supports arguments -Werror=redundant-decls: YES Compiler for C supports arguments -Werror=return-type: YES Compiler for C supports arguments -Werror=sequence-point: YES Compiler for C supports arguments -Werror=shadow: YES Compiler for C supports arguments -Werror=strict-prototypes: YES Compiler for C supports arguments -Werror=trigraphs: YES Compiler for C supports arguments -Werror=undef: YES Compiler for C supports arguments -Werror=write-strings: YES Compiler for C supports arguments -Wformat-nonliteral: YES Compiler for C supports arguments -Wignored-qualifiers: YES Compiler for C supports arguments -Wimplicit-function-declaration: YES Compiler for C supports arguments -Wlogical-op: YES Compiler for C supports arguments -Wmissing-declarations: YES Compiler for C supports arguments -Wmissing-format-attribute: YES Compiler for C supports arguments -Wmissing-include-dirs: YES Compiler for C supports arguments -Wmissing-noreturn: YES Compiler for C supports arguments -Wnested-externs: YES Compiler for C supports arguments -Wno-cast-function-type: YES Compiler for C supports arguments -Wno-dangling-pointer: YES Compiler for C supports arguments -Wno-missing-field-initializers: YES Compiler for C supports arguments -Wno-sign-compare: YES Compiler for C supports arguments -Wno-unused-parameter: YES Compiler for C supports arguments -Wold-style-definition: YES Compiler for C supports arguments -Wpointer-arith: YES Compiler for C supports arguments -Wredundant-decls: YES Compiler for C supports arguments -Wstrict-prototypes: YES Compiler for C supports arguments -Wswitch-default: YES Compiler for C supports arguments -Wswitch-enum: YES Compiler for C supports arguments -Wundef: YES Compiler for C supports arguments -Wuninitialized: YES Compiler for C supports arguments -Wunused: YES Compiler for C supports arguments -fno-strict-aliasing: YES Compiler for C supports arguments -Werror=format-security -Werror=format=2: YES Program python3 found: YES (/usr/bin/python3) Configuring speech-provider-version.h using configuration Found pkg-config: YES (/usr/bin/pkg-config) 2.3.0 Did not find CMake 'cmake' Found CMake: NO Run-time dependency gobject-2.0 found: NO (tried pkgconfig) libspeechprovider/meson.build:49:2: ERROR: Dependency "gobject-2.0" not found, tried pkgconfig A full log can be found at /builddir/build/BUILD/libspeechprovider-1.0.3-build/libspeechprovider-SPEECHPROVIDER_1_0_3/redhat-linux-build/meson-logs/meson-log.txt error: Bad exit status from /var/tmp/rpm-tmp.lAjYDv (%build) Bad exit status from /var/tmp/rpm-tmp.lAjYDv (%build) ```
That was before I added a bunch of dependancies though I assume cmake is now added by one of them.
Spec URL: https://farchord.fedorapeople.org/reviews/libspeechprovider/libspeechprovider.spec SRPM URL: https://farchord.fedorapeople.org/reviews/libspeechprovider/libspeechprovider-1.0.3-1.fc41.src.rpm Forgot to add %doc and %license
Created attachment 2065442 [details] The .spec file difference from Copr build 8500574 to 8500725
Copr build: https://copr.fedorainfracloud.org/coprs/build/8500725 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2337050-libspeechprovider/fedora-rawhide-x86_64/08500725-libspeechprovider/fedora-review/review.txt Found issues: - libspeechprovider : /usr/include/speech-provider/speech-provider-common.h libspeechprovider : /usr/include/speech-provider/speech-provider-dbus-enums.h libspeechprovider : /usr/include/speech-provider/speech-provider-stream-reader.h libspeechprovider : /usr/include/speech-provider/speech-provider-stream-writer.h libspeechprovider : /usr/include/speech-provider/speech-provider-version.h libspeechprovider : /usr/include/speech-provider/speech-provider.h Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Please know that there can be false-positives. --- 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.
(In reply to Steve Cossette from comment #8) > Unless I'm completely misunderstanding, or I'm using the wrong words here, > if a library ends in .so, it's usually a development library. Usually yes, but the extension doesn't matter on Linux. What matters is the contents, and the contents are a shared library. So the linked guidelines still apply. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shared_libraries > If it has a > number after, then it's an installable library. I might be wrong, I guess? But it is installed since it's listed in the RPM, so I'm not sure what you mean by "installable". > > As far as the cmake dependancy goes: > > ``` > Found pkg-config: YES (/usr/bin/pkg-config) 2.3.0 > Did not find CMake 'cmake' > Found CMake: NO > Run-time dependency gobject-2.0 found: NO (tried pkgconfig) > libspeechprovider/meson.build:49:2: ERROR: Dependency "gobject-2.0" not > found, tried pkgconfig > A full log can be found at > /builddir/build/BUILD/libspeechprovider-1.0.3-build/libspeechprovider- > SPEECHPROVIDER_1_0_3/redhat-linux-build/meson-logs/meson-log.txt > error: Bad exit status from /var/tmp/rpm-tmp.lAjYDv (%build) > Bad exit status from /var/tmp/rpm-tmp.lAjYDv (%build) > ``` That isn't a requirement on CMake; that's Meson attempting to use CMake because it didn't find the requirement with pkgconfig. Once you've added the pkgconfig BuildRequires, you'll see that CMake is no longer attempted.
Ref.: https://github.com/project-spiel/libspeechprovider/issues/2 I'll update the spec with the removal of cmake and addition of the devel subpackage once I hear back. And/or the addition of versioning if upstream is unresponsive.
(In reply to Steve Cossette from comment #8) > Unless I'm completely misunderstanding, or I'm using the wrong words here, > if a library ends in .so, it's usually a development library. If it has a > number after, then it's an installable library. I might be wrong, I guess? That's true for libraries that applications are expected to link to at build time (which is most libraries). For plugins that are expected to be dynamically loaded at runtime, then it's normal to have no versioning, but such plugins should not be installed directly into a linker search path to ensure they are not linked to by mistake. So any library ending in .so *usually* goes in the -devel package and therefore does not get installed. But not always (if it's a plugin installed into a custom location). In this case, yes the unversioned library should be a symlink and it belongs in a -devel subpackage.
Spec URL: https://farchord.fedorapeople.org/reviews/libspeechprovider/libspeechprovider.spec SRPM URL: https://farchord.fedorapeople.org/reviews/libspeechprovider/libspeechprovider-1.0.3-1.fc41.src.rpm Created devel subpackage, patch to add soname versioning, removed cmake BR
Created attachment 2066003 [details] The .spec file difference from Copr build 8500725 to 8513458
Copr build: https://copr.fedorainfracloud.org/coprs/build/8513458 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2337050-libspeechprovider/fedora-rawhide-x86_64/08513458-libspeechprovider/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.
One issue I can find that cropped up about at the same time he fixed the soname ticket is this: https://github.com/project-spiel/libspeechprovider/commit/32bcdd66a2e589fab5409d32aa6281e4be902f92 The COPYING file still mentions it being an Apache License, but now LGPL-2.1-or-later is also added? But there's no text for that one. Am I misreading this? It might not be worth it to worry about it right now as it's not in the current version, but I'm guessing I should probably fill a ticket about this too, no? Or maybe make a PR.
Yes, create an upstream issue report to ask about the license. I see plenty of Apache licensed files, so the license is definitely not LGPL alone. The License: field in the spec file is verbose and needs to list every license, so that will be a point to address during the package review.
https://github.com/project-spiel/libspeechprovider/issues/3