Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.0.0-0.fc38.1.src.rpm Description: Open Broadcaster Software is free and open source software for video recording and live streaming. Fedora Account System Username: ngompa
Copr build: https://copr.fedorainfracloud.org/coprs/build/5356241 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2165399-obs-studio/srpm-builds/05356241/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
obs-studio is in rpmfusion-free already. While this does not exclude a fedora package, please comment on the relation to that package (featurewise) and on a plan for possible collaboration (if features differ, as indicated by your bconds). I would hate for obs-studio to be yet another package where we make users' life harder by packaging a "reduced fetature set" version of an existing rpmfusion package and thereby force rpmfusion to pull the full feature version or somehow work around. In particular, since this is a leaf package, it does not improve fedora as a platform either (but may worsen the app situation).
There is work ongoing for migrating this package from RPM Fusion to Fedora. In particular, I'm also doing upstream work to adapt things for Fedora's constraints. The obs-x264 plugin (which is where the x264 dependency comes from) can be built and shipped separately, and that will eventually live in rpmfusion-free. Insofar as the other dependencies, I just did get librist into Fedora and updated ffmpeg-free in Fedora to support it. VLC is being worked on for Fedora too. I'm not planning to import this package until I'm satisfied it works (as I'm using this package on a regular basis and have been de facto maintaining it in RPM Fusion for a while too).
Perfect, thanks Neal! A formal review should probably wait for the dependencies then.
Another update: Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.0.2-0.fc39.1.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/5541387 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2165399-obs-studio/fedora-rawhide-x86_64/05541387-obs-studio/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.
Now a reviewable update: Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~git202303261743.ad859a3-0.fc39.1.src.rpm
Created attachment 1953968 [details] The .spec file difference from Copr build 5541387 to 5712616
Copr build: https://copr.fedorainfracloud.org/coprs/build/5712616 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2165399-obs-studio/fedora-rawhide-x86_64/05712616-obs-studio/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.
It looks like this has a bunch of bundled dependencies under different licenses. These should all be declared in Provides: and the license tag updated accordingly. See the attached licensecheck.
Created attachment 1953970 [details] licensecheck
Other issues from a quick review: [ ]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/icons/hicolor/128x128, /usr/share/icons/hicolor/512x512, /usr/share/icons/hicolor/scalable, /usr/share/icons/hicolor/scalable/apps, /usr/share/icons/hicolor/128x128/apps, /usr/share/icons/hicolor, /usr/share/icons/hicolor/256x256, /usr/share/icons/hicolor/256x256/apps, /usr/share/icons/hicolor/512x512/apps You're probabily missing a runtime dep for these [!]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 11335680 bytes in /usr/share obs- studio-29.1.0~git202303261743.ad859a3-0.fc39.1.aarch64.rpm:11335680 See: https://fedoraproject.org/wiki/Packaging:ReviewGuidelines#Package_Review_Guidelines And from rpmlint, these should be deleted obs-studio.aarch64: E: zero-length /usr/share/obs/obs-plugins/decklink-captions/.keepme obs-studio.aarch64: E: zero-length /usr/share/obs/obs-plugins/decklink-output-ui/.keepme obs-studio.aarch64: E: zero-length /usr/share/obs/obs-plugins/linux-pipewire/.gitkeep and these are probably fine but might warrant a comment or a lint override: obs-studio-libs.aarch64: W: devel-file-in-non-devel-package /usr/lib64/libobs-frontend-api.so obs-studio-libs.aarch64: W: devel-file-in-non-devel-package /usr/lib64/libobs-opengl.so obs-studio-libs.aarch64: W: devel-file-in-non-devel-package /usr/lib64/libobs-scripting.so obs-studio-libs.aarch64: W: devel-file-in-non-devel-package /usr/lib64/libobs.so obs-studio-libs.aarch64: W: devel-file-in-non-devel-package /usr/lib64/libobsglad.so
New build with some of the feedback addressed (still in progress) Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~beta1-0.fc39.1.src.rpm
Created attachment 1954465 [details] The .spec file difference from Copr build 5712616 to 5726827
Copr build: https://copr.fedorainfracloud.org/coprs/build/5726827 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2165399-obs-studio/fedora-rawhide-x86_64/05726827-obs-studio/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.
What's the purpose for setting __brp_check_rpaths to nil? Whatever it is please include it as a comment above the macro definition. What's the purpose of defining __python? The comment says it is to bytecompile with Python 3, but that should be the default now. I suspect this is something left over from before that was the case, so it can probably be removed. The lua_scripting conditional is disabled on %{power64} because luajit isn't available on that arch. luajit is also not available on s390x, so the %ifarch should be updated to include that as well. Please run licensecheck on the sources and update the license fields accordingly. There are many more licenses involved here than just GPL-2.0-or-later. Also review the deps directory and see which bundled libraries you can unbundled. All of them must be either unbundled or marked as bundled libraries (provides and license fields). Please switch negated with conditions (e.g. `%if ! %{with x264}`) to without conditions (e.g. `%if %{without x264}`) for readability. Currently the license files are only included in obs-studio. It's possible to install obs-studio-libs subpackage without getting the license files. The license files must be installed when any subpackage combination is installed. You could either include the license files in all subpackages, or perhaps move the license files to obs-studio-libs, since both obs-studio and obs-studio-devel require it.
Most of the feedback in comment 16 has been addressed with the below update. However, I want to point out a note of clarification: the deps directory does not necessarily imply it's third-party code. Some of it is common code between the frontend and backend components. That said, I've attempted to address this as much as possible. However, I'm not switching the conditionals from "! with" to "without" because I've experienced issues with that in the past. Here's the new version: Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~beta1-0.fc39.2.src.rpm
Created attachment 1955390 [details] The .spec file difference from Copr build 5726827 to 5737820
Copr build: https://copr.fedorainfracloud.org/coprs/build/5737820 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2165399-obs-studio/fedora-rawhide-x86_64/05737820-obs-studio/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.
Update to 29.1.0~beta2: Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~beta2-0.fc39.1.src.rpm
Created attachment 1955811 [details] The .spec file difference from Copr build 5737820 to 5744443
Copr build: https://copr.fedorainfracloud.org/coprs/build/5744443 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2165399-obs-studio/fedora-rawhide-x86_64/05744443-obs-studio/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.
Update to 29.1.0~beta3: Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~beta3-0.fc39.1.src.rpm
Created attachment 1956026 [details] The .spec file difference from Copr build 5744443 to 5749313
Copr build: https://copr.fedorainfracloud.org/coprs/build/5749313 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2165399-obs-studio/fedora-rawhide-x86_64/05749313-obs-studio/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.
The licenses and bundled libraries are still not fully addressed. - There is no license breakdown comment to indicate which files are under which license. - The bundled blake2 library is (CC0-1.0 or OpenSSL or Apache-2.0), but that combination is not included in the License field. Per the guidelines conjunctive license expressions need to be placed in parenthesis [0], and the License field must reflect the all of the bundled licenses [1]. - The license files for bundled libraries need to be shipped with the final package. - rnnoise (plugins/obs-filters/rnnoise) appears to be a bundled library, but there is no bundled provides for it, nor any attempt to debundled it and build against the system rnnoise library. - simde, json11, and uthash are bundled (and properly indicated), but exists as a system libraries. Does upstream provide a way to build against the system libraries? - deps/glad/include/KHR/khrplatform.h is identified by licensecheck as being under the Khronos license. I don't see this on the Fedora allowed license list [2]. It does look quite similar to the MIT license. I did find another package that took the liberty of using the MIT identifier for Khronos licensed code, but I'm skeptical that is appropriate. This probably justifies filing an issue with Fedora legal to get approval for the license and to get it added to the approved list. - There are numerous other directories in deps, plugins, and libobs/util (libobs/graphics/libnsgif, plugins/obs-qsv11/libmfx, plugins/obs-outputs/librtmp, and deps/jansson to name a few) that look like they might be bundled libraries, but aren't provided as bundled or have steps taken to debundle. Please look again at the output of licensecheck and ensure that you are indicating all the necessary licenses and bundled provides. [0] https://docs.fedoraproject.org/en-US/legal/license-field/#_combined_disjunctive_and_conjunctive_license_expressions [1] https://docs.fedoraproject.org/en-US/legal/license-field/#_bundled_or_vendored_dependencies [2] https://docs.fedoraproject.org/en-US/legal/allowed-licenses/
Update to 29.1.0~beta4: Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~beta4-0.fc39.1.src.rpm I've made further progress on bundled dependency and license identification. However, some of the dependencies only have their licenses in their source files, since they are short enough to be included that way... I'm not sure what to do in this case.
What I've done in cases like that is: - extract the text of the license from the source file and include it in a text file - list that text file as an additional Source in the spec file - include a comment indicating what source file the license text was extracted from - copy that additional source file to the build dir during %prep, so it can be picked up by %license easily That fulfills the license requirement to include the copyright notice in distributed copies. It also aligns with the packaging guidance for what to do when upstreams don't include the license text at all. > Include a copy of what they believe the license text is intended to be, as part of the Fedora package in %license, in order to remain in compliance. https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text
> - deps/glad/include/KHR/khrplatform.h is identified by licensecheck as being under the Khronos license. I don't see this on the Fedora allowed license list [2]. It does look quite similar to the MIT license. I did find another package that took the liberty of using the MIT identifier for Khronos licensed code, but I'm skeptical that is appropriate. This probably justifies filing an issue with Fedora legal to get approval for the license and to get it added to the approved list. Any progress on this item? I noticed that libglvnd-devel contains the file /usr/include/KHR/khrplatform.h. I don't see that as a directly build requirement, but your libGL-devel build requirement resolves to mesa-libGL-devel, which itself requires libglvnd-devel. Can you confirm that the build is using the system copy instead of the bundled one? If so, consider deleting the bundled copy in %prep. It would also be good to list libglvnd-devel as an explicit build requirement to ensure it's always pulled in. If it's not unbundle-able, then I recommend following the License Review Process to get the Khronos license added to the list of allowed licenses. libglvnd identifies itself as MIT, which I don't think is correct, so this step would be good regardless. https://docs.fedoraproject.org/en-US/legal/license-review-process/
Khronos EGL headers have been stripped: Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~beta4-0.fc39.2.src.rpm
License stanzas have been extracted and collected as license files now. Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~beta4-0.fc39.3.src.rpm
Several files in the qsv11 plugin have the following header: This file is provided under a dual BSD/GPLv2 license. When using or redistributing this file, you may do so under either license. If I'm reading that correctly, you'll need (BSD-3-Clause or GPL-2.0-only) in your license field.
Several files in the outputs plugin have headers indicating they are Public Domain. You'll need to add LicenseRef-Fedora-Public-Domain to your license field.
qsv11's license stanza has been extracted and collected as a license file now. Updated librtmp's license clause. Spec URL: https://ngompa.fedorapeople.org/for-review/obs-studio.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/obs-studio-29.1.0~beta4-0.fc39.5.src.rpm
Created attachment 1957940 [details] The .spec file difference from Copr build 5749313 to 5800381
Copr build: https://copr.fedorainfracloud.org/coprs/build/5800381 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2165399-obs-studio/fedora-rawhide-x86_64/05800381-obs-studio/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.
Package is approved. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [x]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in obs- studio-libs , obs-studio-devel [x]: Package functions as described. [x]: Latest version is packaged. [-]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified.
The Pagure repository was created at https://src.fedoraproject.org/rpms/obs-studio
FEDORA-2023-a407953f74 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-a407953f74
FEDORA-2023-abe9a1c6f5 has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2023-abe9a1c6f5
FEDORA-2023-0cdde78201 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-0cdde78201
FEDORA-2023-a407953f74 has been pushed to the Fedora 38 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-a407953f74 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-a407953f74 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2023-0cdde78201 has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-0cdde78201 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-0cdde78201 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2023-abe9a1c6f5 has been pushed to the Fedora 36 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-abe9a1c6f5 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-abe9a1c6f5 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2023-a407953f74 has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2023-0cdde78201 has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2023-abe9a1c6f5 has been pushed to the Fedora 36 stable repository. If problem still persists, please make note of it in this bug report.