PR#57: Backport macros.shell-completions from Fedora - rpms/epel-rpm-macros

Note this fix %bash_completion_dir on epel-rpm-macros which should be %bash_completions_dir

Would this be better as (

%bash_completion_dir %{warn:%%{bash_completion_dir} is deprecated. Use %%{bash_completions_dir} instead.}%{bash_completions_dir}

gotmax23 commented on line 13 of epel-rpm-macros.spec a year ago

I'm a hesitant about pinning to a branch, because it's ambiguous, but I'll defer to you.

If there are no annoyances with nor objections to adding such a warning, then sure. I don't have any strong preference for or against.

On EL7, the warning is printed without a newline. I don't know if that will matter in practical use:

$ rpm -E '%{warn:%%{bash_completion_dir} is deprecated. Use %%{bash_completions_dir} instead.}%{bash_completions_dir}'
%{bash_completion_dir} is deprecated. Use %{bash_completions_dir} instead./usr/share/bash-completion/completions

I'm a hesitant about pinning to a branch, because it's ambiguous, but I'll defer to you.

Since we're not downloading the source via the URL each time, I don't think it's a problem. It makes it slightly easier to refresh if/when desired though, by just removing the current file and using spectool -g. And it saves the need for a comment line. That's my thinking, anyway.

I don't have a strong preference either way. :)

Since we're not downloading the source via the URL each time, I don't think it's a problem. It makes it slightly easier to refresh if/when desired though, by just removing the current file and using spectool -g. And it saves the need for a comment line. That's my thinking, anyway.

Fair enough. I'm the original author of these macros, so I'll sync this if I make any changes.

On EL7, the warning is printed without a newline. I don't know if that will matter in practical use:

As an example, I added %global _bindir %{warn:error message}%{_bindir} to a specfile and parsed it with rpmspec -P on EL 7. %{_bindir} still evaluated properly. You can also run rpm -E .... 2>/dev/null and you'll see that that the result is the same with or without the %{warn}.

Either way, I'm thinking we can go without the warning. We're not going to remove this in a stable release, so I don't think it's worth the trouble.

LGTM

[1]
https://sourcegraph.com/search?q=context:global+r:src.fedoraproject.org+file:.*%5C.spec+bash_completion_dir&patternType=regexp&case=yes&sm=0

in my point of view after check [1] and saw that we just have 4 cases with the wrong %bash_completion_dir

in warn I would write %{warn:%{bash_completion_dir} is a typo ....

and add 4 pull request fixing the 4 packages

But first lets approve this , build it and bring it to stable repositories just after that we can start fix all rpms specs that have bash_completion_dir

@everyone can I processed we need this ? this is a fix of one typo on %bash_completion_dir on epel-rpm-macros which should be %bash_completions_dir with "s" , without this in stable we can't fix the spec that use %bash_completions_dir with the typo

FWIW, I was waiting a bit to see whether any of the maintainers had comments before making any further changes here (such as adding the %{warning} macro).

9 days isn't enough ?

Until we have some feedback from the maintainers, I don't see what we can do except wait?

If the maintainers say they would prefer to have %{warn:...} around the old %bash_completion_dir macro, that's easy to add.

As a reminder (to myself as much as anyone), if we do mark %bash_completion_dir as deprecated in epel7/epel8, we should do the same for epel9 and try to ensure that it is not copied into epel10.

Indeed!