```
* Sun Oct 10 2021 Maxwell G <gotmax> - 2021.10.10-1
- Mark LICENSE with %%license instead of %%doc
- Update to 2021.10.10
```
Spec URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/1cbf4229bfc36743d21b52a5bbee2de518ed2f96/yt-dlp.spec
SRPM URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/9895fe63cc1f40abbd619d27c43f34a01b2c9618/yt-dlp-2021.10.10-1.fc36.src.rpm
Git Repository: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/tree/review/
Successful Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=77015823

```
* Sun Oct 10 2021 Maxwell G <gotmax> - 2021.10.10-1
- Mark LICENSE with %%license instead of %%doc
- Update to 2021.10.10
- Make repository REUSE compliant
- Fix non-executable-script rpmlint error
```

Spec URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/2eda17c2bf8b09f2a5fe54a17cb45f5f82c9c77b/yt-dlp.spec
SRPM URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/a5ae95d8cec354b1009394af6fdceedc6f3ec95e/yt-dlp-2021.10.10-1.fc36.src.rpm

Hello,

thanks for packaging yt-dlp. Since I'm just a packager and not a sponsor, the following review is *informal*.

The SPEC file looks quite good. Suggests and Supplements clauses are wisely used and the package mainly follows the latest Python packaging guidelines. Here are a few suggestions:

* you should use the %py3_dist macro for explicit Python dependencies, for example %{py3_dist keyring} instead of python3-keyring. This makes dependency tracking easier, based on PyPi names. This may be useful also in case of naming convention changes for Python packages.

* Why not using %tox for tests in %check section, instead of an explicit call to %pytest (and the corresponding BuildRequires)? The project provides a Tox test suite and it's well supported by the Python packaging scheme (BTW you are generating BRs with "%pyproject_buildrequires -t"). You can pass Pytest options to tox using the PYTEST_ADDOPTS variable:
  
  %check
  export PYTEST_ADDOPTS="-k 'not download'"
  %tox

* Since mutagen is already required by the package (automatically added as Requires from setup.py), the Suggests on AtomicParsley is probably useless.

* according to rpmlint, some Python files contain shebangs whereas they're not executable:

  yt-dlp.noarch: E: non-executable-script /usr/lib/python3.9/site-packages/yt_dlp/YoutubeDL.py 644 /usr/bin/env python3
  yt-dlp.noarch: E: non-executable-script /usr/lib/python3.9/site-packages/yt_dlp/__init__.py 644 /usr/bin/env python3
  yt-dlp.noarch: E: non-executable-script /usr/lib/python3.9/site-packages/yt_dlp/__main__.py 644 /usr/bin/env python3
  yt-dlp.noarch: E: non-executable-script /usr/lib/python3.9/site-packages/yt_dlp/utils.py 644 /usr/bin/env python3

  You can use for example https://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_Python_libraries to fix this, while keeping file timestamp.

Another rpmlint issue:

yt-dlp.src:72: W: rpm-buildroot-usage %build make DESTDIR=%{buildroot} README.txt yt-dlp.1 completion-bash completion-zsh completion-fish

You should remove the DESTDIR variable in this make call. Setting this variable is useless since building these extra files doesn't rely on target installation directory.

Hi Mohamed,

Thank you for your suggestions.

--- Comment #3 from Mohamed El Morabity <pikachu.2014> ---

> * you should use the %py3_dist macro for explicit Python dependencies, for
> example %{py3_dist keyring} instead of python3-keyring. This makes dependency
> tracking easier, based on PyPi names. This may be useful also in case of naming
> convention changes for Python packages.

Alright, I will change that. Isn't there also another way to specify that where the module name is inside parentheses?

> * Why not using %tox for tests in %check section, instead of an explicit call
> to %pytest (and the corresponding BuildRequires)?

I did not use Tox, because upstream's CI just calls pytest directly, and I didn't see the need to introduce the extra complexity of Tox. If you still think I should change it, I can.

> (BTW you are
> generating BRs with "%pyproject_buildrequires -t").

Yes, I am aware of that. I manually added pytest, because it wasn't already listed in the setup.py. Was that wrong? I will also fix that to use the macro, as you recommended.

> * Since mutagen is already required by the package (automatically added as
> Requires from setup.py), the Suggests on AtomicParsley is probably useless.

You are right; the project README also says that they are redundant.

> * according to rpmlint, some Python files contain shebangs whereas they're not
> executable:
> 
>   yt-dlp.noarch: E: non-executable-script
> /usr/lib/python3.9/site-packages/yt_dlp/YoutubeDL.py 644 /usr/bin/env python3
>   yt-dlp.noarch: E: non-executable-script
> /usr/lib/python3.9/site-packages/yt_dlp/__init__.py 644 /usr/bin/env python3
>   yt-dlp.noarch: E: non-executable-script
> /usr/lib/python3.9/site-packages/yt_dlp/__main__.py 644 /usr/bin/env python3
>   yt-dlp.noarch: E: non-executable-script
> /usr/lib/python3.9/site-packages/yt_dlp/utils.py 644 /usr/bin/env python3
> 
>   You can use for example
> https://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_Python_libraries
> to fix this, while keeping file timestamp.

I already fixed that in Comment #2, but I did it differently.

> # See https://github.com/yt-dlp/yt-dlp/commit/5e1dba8ed6a8974405ed038cb1ed7a82cdfaca4b#commitcomment-57786462
> find -type f ! -executable -name '*.py' -print -exec sed -i -e '1{\@^#!.*@d}' '{}' +

I think this solution is more elegant, because it only applies to non-executable files. It doesn't preserve the timestamp, though.

--- Comment #3 from Mohamed El Morabity <pikachu.2014> ---

> Another rpmlint issue:
>
> yt-dlp.src:72: W: rpm-buildroot-usage %build make DESTDIR=%{buildroot} README.txt yt-dlp.1 completion-bash completion-zsh completion-fish
>
> You should remove the DESTDIR variable in this make call. Setting this variable is useless since building these extra files doesn't rely on target installation directory.

I will remove that.


Thanks again,
Maxwell

```
* Sun Oct 10 2021 Maxwell G <gotmax> - 2021.10.10-1
- Mark LICENSE with %%license instead of %%doc
- Update to 2021.10.10
- Make repository REUSE compliant
- Fix non-executable-script rpmlint error
- Use `python3dist(NAME)` for dependencies
- Fix rpm-buildroot-usage rpmlint error
```

Spec URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/f30a5d4f6d5134356f3dd96d0d80d1b9a96a6246/yt-dlp.spec
SRPM URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/1061d765d44193a0a3c54ec230946392f8ddde27/yt-dlp-2021.10.10-1.fc36.src.rpm
Successful Koji Scratch Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=77132960

(In reply to Maxwell G from comment #5)
 
> > * you should use the %py3_dist macro for explicit Python dependencies, for
> > example %{py3_dist keyring} instead of python3-keyring. This makes dependency
> > tracking easier, based on PyPi names. This may be useful also in case of naming
> > convention changes for Python packages.
> 
> Alright, I will change that. Isn't there also another way to specify that
> where the module name is inside parentheses?
Do you think of python3dist(...)? This is the purpose of %py3_dist actually:

$ rpm --eval "%{py3_dist my-nice-blob}"
python3dist(my-nice-blob)


> > * Why not using %tox for tests in %check section, instead of an explicit call
> > to %pytest (and the corresponding BuildRequires)?
> 
> I did not use Tox, because upstream's CI just calls pytest directly, and I
> didn't see the need to introduce the extra complexity of Tox. If you still
> think I should change it, I can.
Sounds legit. pytest is OK then if upstream uses it in its CI.

> > (BTW you are
> > generating BRs with "%pyproject_buildrequires -t").
> 
> Yes, I am aware of that. I manually added pytest, because it wasn't already
> listed in the setup.py. Was that wrong? I will also fix that to use the
> macro, as you recommended.
Since "%pyproject_buildrequires -t" is "%pyproject_buildrequires -r" + tox deps, I thought it would have been consistent to use tox also in %check. As long as BRs are satisfied this way, it's OK, there's nothing wrong :). 


> > * according to rpmlint, some Python files contain shebangs whereas they're not
> > executable:
> 
> I already fixed that in Comment #2, but I did it differently.
Sorry, I made my review on a previous spec file. As long as unwanted shebangs are gone, it's OK too.


(In reply to Maxwell G from comment #6)

> Spec URL:
> https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/f30a5d4f6d5134356f3dd96d0d80d1b9a96a6246/yt-dlp.spec
404 :'(

> (In reply to Maxwell G from comment #6)
> 
> > Spec URL:
> > https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/f30a5d4f6d5134356f3dd96d0d80d1b9a96a6246/yt-dlp.spec
> 404 :'(

I forgot to push. Try it now.

Hi, I am one of the maintainers of youtube-dl.
I think it would made sense to switch to the fork, as the old youtube-dl is not active, and the outdated version is not that useful.

I did look a while ago because the lack of releases worried me, but than I didn't had the time to follow up on the issue.

If you want you can open a PR for youtube-dl [0] and we can switch to yt-dlp - or do you see a reason to keep them both?

[0] https://src.fedoraproject.org/rpms/youtube-dl

Hi David,

There are some packages in the regular Fedora repos and rpmfusion-free that depend on youtube-dl that we'd have to deal with first.

From Fedora:
gpodder
gydl
persepolis
streamtuner
video-downloader

From rpmfusion-free:
celluloid
gnome-mpv
lives
qmplay2
xt7-player-mpv
xvst

(I got these results by parsing the output of `sudo dnf repoquery --whatdepends youtube-dl`, so this also includes weak-deps.)

I'm not sure if yt-dlp would function properly as a drop-in replacement. It is more than just a simple fork; it has many changes, new features, and different binary/library names. [Here][1] is a full list of differences between youtube-dl and yt-dlp.

The last commit to `ytdl-org/youtube-dl` was on July 1at and the last release was on Jun 5th. There are 3.9k open issues and 870 open pull requests. I felt conflicted about completely deprecating the `youtube-dl` package when another user originally [proposed this idea on the mailing list][2]. After looking at those statistics, though, I guess it is fair to consider youtube-dl abandonware, as sad as that is.

Thanks,
Maxwell

[1]: https://github.com/yt-dlp/yt-dlp#new-features
[2]: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/KDGE2AP5KDPKS4J6P22ANHYN56ADFBRB/

I think this is best regarded as a API breaking update [0]

As a first step, I think COPR builds for testing would be needed. Once we are happy, we can then reach out to devel@ and ask for feedback.

[0] https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/

Hi everyone,

I think we should push out yt-dlp and deal with replacing youtube-dl later. Youtube-dl and yt-dlp have different binary names and import paths, so there's no reason they can't both be in the repos.

This will also allow Artem, the maintainer of `video-downloader`, to update that package to the latest release. `video-downloader` just switched from youtube-dl to yt-dlp[1]. I have cc'd Artem on this ticket.

However, I'm still waiting for an official review and a sponsor. Is there something I can do to speed up the process? I already sent a self introduction email[2] to the devel list.

Thanks,
Maxwell

[1]: https://github.com/Unrud/video-downloader/releases/tag/0.9.0

[2]: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/OLSKLGEXG5SEFC3ZSD6L2OGDWWQ4PI7G/#OLSKLGEXG5SEFC3ZSD6L2OGDWWQ4PI7G

Hi,

I have cc'd the other maintainers of youtube-dl for their input, as well.

Thanks,
Maxwell

Any reason to have 3 subpackages for shell-completion variants? 

I've check and just a few packages are doing this way while many other just install the shell-completion files as part of the main package.

(In reply to Mikel Olasagasti Uranga from comment #14)
> Any reason to have 3 subpackages for shell-completion variants? 
> 
> I've check and just a few packages are doing this way while many other just
> install the shell-completion files as part of the main package.

Hi Mikel,

Thank you for asking.

I want users to be able to install only the completions for the shell that they use or even no completions at all. I set this up so that, as long as the user hasn't disabled weak deps[1], dnf will automatically install the bash completions only if `bash-completion` is also installed and the same for the other shell completion packages. I got this idea from one of OpenSUSE's packages, and I think it's quite elegant. Another reviewer seemed to agree[2].

> Suggests and Supplements clauses are wisely used

Thanks,
Maxwell

[1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/WeakDependencies/#_weak_dependencies
[2]: https://bugzilla.redhat.com/show_bug.cgi?id=2012522#c3

```
* Sat Oct 23 2021 Maxwell G <gotmax> - 2021.10.22-1
- Update to 2021.10.22
```

Spec URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/c3ce3ac406bddaed171bbe6b347a05674f31e620/yt-dlp.spec
SRPM URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/be083be9242841b4473b1b541c49567958792ca4/yt-dlp-2021.10.10-1.fc36.src.rpm
Git Repository: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/tree/review/
Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=77733956

(In reply to Maxwell G from comment #15)
> I want users to be able to install only the completions for the shell that
> they use or even no completions at all.

What's the use case for this, other than saving a few kB of storage? FWIW, `rpm -qa|grep completion` doesn't return a single package other than bash-completion itself, so I don't see the point.

Other than that, great spec and thanks for investing the time to get this in the repositories!

(In reply to Maxwell G from comment #12)
> However, I'm still waiting for an official review and a sponsor. Is there
> something I can do to speed up the process?

I had good experiences with asking in #fedora-devel on libera.chat. Other than that: maybe send another post to devel@, with a more catchy subject (along the lines of "looking for a sponsor for yt-dlp")?

Hi Tobias,

Thank you for your advice!

(In reply to Tobias from comment #17)
> (In reply to Maxwell G from comment #15)
> > I want users to be able to install only the completions for the shell that
> > they use or even no completions at all.
> 
> What's the use case for this, other than saving a few kB of storage?

I agree that it is not a major difference. My thinking is that users should not have to install files that are not relevant to them. In this spirit, I am also considering adding `Requires` for the shell package for each respective completions subpackage.

> FWIW, `rpm -qa|grep completion` doesn't return a single package other than
> bash-completion itself, so I don't see the point.

`rpm -qa` only lists packages that are installed on your system. In fact, there are several completions packages present in Fedora. (See below)

> [...]

> (In reply to Maxwell G from comment #12)
> > However, I'm still waiting for an official review and a sponsor. Is there
> > something I can do to speed up the process?
> 
> I had good experiences with asking in #fedora-devel on libera.chat. Other
> than that: maybe send another post to devel@, with a more catchy subject
> (along the lines of "looking for a sponsor for yt-dlp")?

I will try that. Thank you for your advice.

Maxwell

---

(This is from my Fedora 34 machine.)

``` console
$ sudo dnf search completion                                                 
 
Last metadata expiration check: 0:02:55 ago on Sun 24 Oct 2021 04:09:07 PM CDT.
======================================== Name & Summary Matched: completion =========================================
bash-completion.noarch : Programmable completion for Bash
cheat-bash-completion.noarch : Bash completion support for cheat
cheat-fish-completion.noarch : Fish completion support for cheat
cheat-zsh-completion.noarch : Zsh completion support for cheat
coccinelle-bash-completion.noarch : Bash tab-completion for coccinelle
drbd-bash-completion.x86_64 : Programmable bash completion support for drbdadm
epix-bash-completion.noarch : Bash completion support for epix
fx-completion.noarch : Shell completion for fx
gedit-plugin-bracketcompletion.x86_64 : gedit bracketcompletion plugin
gedit-plugin-wordcompletion.x86_64 : gedit wordcompletion plugin
guestfs-tools-bash-completion.noarch : Bash tab-completion scripts for guestfs-tools
ikona-cli-bash-completions.noarch : Bash completions for ikona-cli
ikona-cli-fish-completions.noarch : Fish completions for ikona-cli
ikona-cli-zsh-completions.noarch : ZSH completions for ikona-cli
kf5-kcompletion.x86_64 : KDE Frameworks 5 Tier 2 addon with auto completion widgets and classes
kf5-kcompletion.i686 : KDE Frameworks 5 Tier 2 addon with auto completion widgets and classes
kf5-kcompletion-devel.i686 : Development files for kf5-kcompletion
kf5-kcompletion-devel.x86_64 : Development files for kf5-kcompletion
libguestfs-bash-completion.noarch : Bash tab-completion scripts for libguestfs tools
libnbd-bash-completion.noarch : Bash tab-completion for libnbd
libvirt-bash-completion.x86_64 : Bash completion script
moby-engine-fish-completion.x86_64 : Fish completion files for moby-engine
moby-engine-zsh-completion.x86_64 : Zsh completion files for moby-engine
nbdkit-bash-completion.noarch : Bash tab-completion for nbdkit
perl-Devel-REPL-Plugin-Completion.noarch : Devel-REPL plugin for tab completion
php-stecman-symfony-console-completion.noarch : Automatic BASH completion for Symfony Console based applications
plasma-wallpapers-dynamic-builder-bash-completion.noarch : Bash completion support for
                                                         : plasma-wallpapers-dynamic-builder
plasma-wallpapers-dynamic-builder-fish-completion.noarch : Fish completion support for
                                                         : plasma-wallpapers-dynamic-builder
plasma-wallpapers-dynamic-builder-zsh-completion.noarch : Zsh completion support for
                                                        : plasma-wallpapers-dynamic-builder
python-django-bash-completion.noarch : Bash completion files for Django
python-vitrageclient-bash-completion.noarch : bash completion files for vitrage
python3-click-completion.noarch : Add automatic completion support for fish, Zsh, Bash and PowerShell to Click
quodlibet-zsh-completion.noarch : zsh completion files for quodlibet
rss2email-zsh-completion.noarch : zsh-completion files for rss2email
tarsnap-bash-completion.noarch : Bash completion support for tarsnap
virt-v2v-bash-completion.noarch : Bash tab-completion for virt-v2v
zathura-bash-completion.noarch : bash-completion files for zathura
zathura-fish-completion.noarch : fish-completion files for zathura
zathura-zsh-completion.noarch : zsh-completion files for zathura
============================================= Name Matched: completion ==============================================
cekit-bash-completion.noarch : Container image creation tool
cekit-zsh-completion.noarch : Container image creation tool
```

I will review this package and sponsor you.

> # SPDX-FileCopyrightText: 2021 Maxwell G (@gotmax23)
> # SPDX-License-Identifier: Unlicense OR MIT

Not allowed in Fedora and must be removed.

All Fedora SPECs uses FPCA: https://fedoraproject.org/wiki/Licensing:Main#License_of_Fedora_SPEC_Files

> #Recommends: ffmpeg ffprobe
> #Suggests:      rtmpdump
> #Suggests:      (mplayer or mpv)

Must be removed. Fedora SPECs can't require packages from another repositories.

> # The commented out lines are available in RPMFusion.

This must be done on RPM Fusion's side with adding Recommends: yt-dlp for smplayer and other packages.

> %package bash-completion

I think that extraction of *completion files to a separate packages is not a good idea. They will never be installed, because you don't have Requires on them.

If you want subpackages, feel free to use them, but don't forget to require them using Rich dependencies from the base package:

Requires: (%{name}-bash-completion if bash)
Requires: (%{name}-zsh-completion if zsh)
Requires: (%{name}-fish-completion if fish)

> %{_docdir}/yt_dlp/*

Should be replaced with (without asterisk):

%docdir %{_docdir}/%{name}/
%{_docdir}/%{name}/

All files and directories provided by RPM package must be owned. That's why you need to choose between these two strategies.

Strategy 1: own *-completion directories. Dirty, but a lot of packages uses this.

> %files bash-completion
> %{_datadir}/bash-completion/completions/%{name}

%files bash-completion
%dir %{_datadir}/bash-completion/completions/
%dir %{_datadir}/bash-completion/
%{_datadir}/bash-completion/completions/%{name}

> %files zsh-completion
> %{_datadir}/zsh/site-functions/_%{name}

%files zsh-completion
%dir %{_datadir}/zsh/site-functions
%{_datadir}/zsh/site-functions/_%{name}

> %files fish-completion
> %{_datadir}/fish/vendor_completions.d/%{name}.fish

%files fish-completion
%dir %{_datadir}/fish/vendor_completions.d/
%{_datadir}/fish/vendor_completions.d/%{name}.fish

Strategy 2: explicitly require the corresponding packages.

> %package bash-completion

%package bash-completion
Requires: bash-completion

> %package zsh-completion

%package zsh-completion
Requires: zsh

> %package fish-completion

%package fish-completion
Requires: fish

Strategy 3: move everything into the base package and remove subpackages:

%files
%dir %{_datadir}/bash-completion/completions/
%dir %{_datadir}/bash-completion/
%{_datadir}/bash-completion/completions/%{name}
%dir %{_datadir}/zsh/site-functions
%{_datadir}/zsh/site-functions/_%{name}
%dir %{_datadir}/fish/vendor_completions.d/
%{_datadir}/fish/vendor_completions.d/%{name}.fish

Hi Vitaly,

(In reply to Vitaly Zaitsev from comment #19)
> I will review this package and sponsor you

Thanks for sponsoring me!

(In reply to Vitaly Zaitsev from comment #20)
> > # SPDX-FileCopyrightText: 2021 Maxwell G (@gotmax23)
> > # SPDX-License-Identifier: Unlicense OR MIT
> 
> Not allowed in Fedora and must be removed.
> 
> All Fedora SPECs uses FPCA:
> https://fedoraproject.org/wiki/Licensing:Main#License_of_Fedora_SPEC_Files

> All original Fedora contributions are governed by the Fedora Project Contributor Agreement (FPCA). This means that unless a spec file contains an explicit license attribution within it, it is available under the terms of the MIT license.

https://fedoraproject.org/wiki/Licensing:Main#License_of_Fedora_SPEC_Files:

> Since every Fedora SPEC file is contributed by FPCA signers, every Fedora SPEC is available under these license terms (unless otherwise explicitly licensed). 

I explicitly dual licensed the spec under the Unlicense or MIT.

> > #Recommends: ffmpeg ffprobe
> > #Suggests:      rtmpdump
> > #Suggests:      (mplayer or mpv)
> 
> Must be removed. Fedora SPECs can't require packages from another
> repositories.
> 
> > # The commented out lines are available in RPMFusion.
> 
> This must be done on RPM Fusion's side with adding Recommends: yt-dlp for
> smplayer and other packages.

Okay, I will remove this.

> > %package bash-completion
> 
> I think that extraction of *completion files to a separate packages is not a
> good idea. They will never be installed, because you don't have Requires on
> them.
> 
> If you want subpackages, feel free to use them, but don't forget to require
> them using Rich dependencies from the base package:
> 
> Requires: (%{name}-bash-completion if bash)
> Requires: (%{name}-zsh-completion if zsh)
> Requires: (%{name}-fish-completion if fish)

The `Supplements:` keys in the spec accomplish the same goal, while still allowing users to opt out by disabling weak deps. I explained this in this comment[1].

> 
> > %{_docdir}/yt_dlp/*
> 
> Should be replaced with (without asterisk):
> 
> %docdir %{_docdir}/%{name}/
> %{_docdir}/%{name}/

Done.

(In reply to Vitaly Zaitsev from comment #21)
> All files and directories provided by RPM package must be owned. That's why
> you need to choose between these two strategies.

[...]

> 
> Strategy 2: explicitly require the corresponding packages.
> 
> > %package bash-completion
> 
> %package bash-completion
> Requires: bash-completion
> 
> > %package zsh-completion
> 
> %package zsh-completion
> Requires: zsh
> 
> > %package fish-completion
> 
> %package fish-completion
> Requires: fish

I was going to do this anyways[2], so that's what I will do.

Maxwell

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2012522#c15
[2]: https://bugzilla.redhat.com/show_bug.cgi?id=2012522#c18

Spec URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/7f59e94e3e6f8262aa0811476782682d6b5cab49/yt-dlp.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/gotmax23/yt-dlp/srpm-builds/02942488/yt-dlp-2021.10.22-2.fc35.src.rpm
Git Repository: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/tree/review/

Spec URL: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/455cf3a1a59f806424b7da668065a88a647a2078/yt-dlp.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/gotmax23/yt-dlp/srpm-builds/02943100/yt-dlp-2021.11.10.1-1.fc35.src.rpm
Git Repository: https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/tree/review/

> # SPDX-FileCopyrightText: 2021 Maxwell G (@gotmax23)
> # SPDX-License-Identifier: Unlicense OR MIT

Please remove these lines. Fedora SPECs must not contain any third-party copyrights and/or license identifiers.

LGTM now. Please fix the legal issue with SPEC double licensing and I will approve this package.

I prefer for my repositories to be explicitly licensed, but I understand that this is not allowed in the context of Fedora.

https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/commit/67078ae1f8d38afc3fdd0db9de6707be19210882

(In reply to Maxwell G from comment #27)
> I prefer for my repositories to be explicitly licensed, but I understand
> that this is not allowed in the context of Fedora.
> 
> https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/commit/
> 67078ae1f8d38afc3fdd0db9de6707be19210882

Oops, I did not completely remove it.

https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/commit/51b5f4867569a97f39b6f8e5e043d48bff9d4e0a

https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/commit/6624e7ae1753054773213e2a3a82613b2ce5b503

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

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.
     Note: There is no build directory. Running licensecheck on vanilla
     upstream sources. No licenses found. Please check the source files for
     licenses manually.
[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: Package contains desktop file if it is a GUI application.
[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.
[x]: 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.
[x]: Package contains systemd file(s) if in need.
[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 133120 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]: 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

Python:
[x]: Binary eggs must be removed in %prep
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files

===== SHOULD items =====

Generic:
[x]: Uses parallel make %{?_smp_mflags} macro.
[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 yt-dlp-
     bash-completion , yt-dlp-zsh-completion , yt-dlp-fish-completion
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %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]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: yt-dlp-2021.11.10.1-1.fc36.noarch.rpm
          yt-dlp-bash-completion-2021.11.10.1-1.fc36.noarch.rpm
          yt-dlp-zsh-completion-2021.11.10.1-1.fc36.noarch.rpm
          yt-dlp-fish-completion-2021.11.10.1-1.fc36.noarch.rpm
          yt-dlp-2021.11.10.1-1.fc36.src.rpm
yt-dlp.noarch: W: spelling-error %description -l en_US youtube -> you tube, you-tube, YouTube
yt-dlp.noarch: W: spelling-error %description -l en_US dl -> fl, d, l
yt-dlp-bash-completion.noarch: W: no-documentation
yt-dlp-zsh-completion.noarch: W: no-documentation
yt-dlp-fish-completion.noarch: W: no-documentation
yt-dlp.src: W: spelling-error %description -l en_US youtube -> you tube, you-tube, YouTube
yt-dlp.src: W: spelling-error %description -l en_US dl -> fl, d, l
5 packages and 0 specfiles checked; 0 errors, 7 warnings.




Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:


Source checksums
----------------
https://github.com/yt-dlp/yt-dlp/archive/2021.11.10.1/yt-dlp-2021.11.10.1.tar.gz :
  CHECKSUM(SHA256) this package     : b0bfe3a1e7c3a9a1e24219c36eaf8604328b29cc3560387f75506f45b78ea695
  CHECKSUM(SHA256) upstream package : b0bfe3a1e7c3a9a1e24219c36eaf8604328b29cc3560387f75506f45b78ea695


Requires
--------
yt-dlp (rpmlib, GLIBC filtered):
    /usr/bin/python3
    python(abi)
    python3.10dist(mutagen)
    python3.10dist(pycryptodomex)
    python3.10dist(websockets)

yt-dlp-bash-completion (rpmlib, GLIBC filtered):
    bash-completion
    yt-dlp

yt-dlp-zsh-completion (rpmlib, GLIBC filtered):
    yt-dlp
    zsh

yt-dlp-fish-completion (rpmlib, GLIBC filtered):
    fish
    yt-dlp



Provides
--------
yt-dlp:
    python3.10dist(yt-dlp)
    python3dist(yt-dlp)
    yt-dlp

yt-dlp-bash-completion:
    yt-dlp-bash-completion

yt-dlp-zsh-completion:
    yt-dlp-zsh-completion

yt-dlp-fish-completion:
    yt-dlp-fish-completion



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -b 2012522
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Python, Shell-api
Disabled plugins: C/C++, SugarActivity, Java, Ocaml, R, Perl, fonts, PHP, Haskell
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

LGTM now. Package approved.

I will sponsor your FAS account to the packagers group and send email with getting started instructions.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/yt-dlp

FEDORA-2021-113da959ed has been submitted as an update to Fedora 35. https://bodhi.fedoraproject.org/updates/FEDORA-2021-113da959ed

FEDORA-2021-113da959ed has been pushed to the Fedora 35 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-113da959ed \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-113da959ed

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2021-113da959ed has been pushed to the Fedora 35 stable repository.
If problem still persists, please make note of it in this bug report.