PR#2: Use recommended directory for bash-completion file - rpms/bodhi

rpms / bodhi

#2 Use recommended directory for bash-completion file

Closed 4 years ago by bowlofeggs. Opened 4 years ago by tmz.

rpms/ tmz/bodhi master into master

Simplify and improve git ignore rules

Todd Zullinger • 4 years ago

3cc1779

Use recommended directory for bash-completion file

Todd Zullinger • 4 years ago

f1d4cdb

.gitignore

file modified

+4 -65

		`@@ -1,65 +1,4 @@`
		`- bodhi-0.7.5.tar.bz2`
		`- bodhi-0.7.7.tar.bz2`
		`- /bodhi-0.7.8.tar.bz2`
		`- bodhi-0.7.9.tar.bz2`
		`- bodhi-0.7.10.tar.bz2`
		`- bodhi-0.7.11.tar.bz2`
		`- /bodhi-0.7.12.tar.bz2`
		`- bodhi-0.7.13.tar.bz2`
		`- bodhi-0.7.14.tar.bz2`
		`- /bodhi-0.7.15.tar.bz2`
		`- /bodhi-0.8.0.tar.bz2`
		`- /bodhi-0.8.3.tar.bz2`
		`- /bodhi-0.8.4.tar.bz2`
		`- /bodhi-0.8.5-1.fc15.src.rpm`
		`- /bodhi-0.8.5.tar.bz2`
		`- /bodhi-0.9.2.tar.bz2`
		`- /bodhi-0.9.3.tar.bz2`
		`- /bodhi-0.9.4.tar.bz2`
		`- /bodhi-0.9.5.tar.bz2`
		`- /bodhi-0.9.7.tar.bz2`
		`- /bodhi-0.9.8.tar.bz2`
		`- /bodhi-0.9.9.tar.bz2`
		`- /bodhi-0.9.9.2.tar.bz2`
		`- /bodhi-0.9.10.tar.bz2`
		`- /bodhi-0.9.11.tar.bz2`
		`- /bodhi-0.9.12.2.tar.bz2`
		`- /2.1.8.tar.gz`
		`- /2.2.0.tar.gz`
		`- /2.2.1.tar.gz`
		`- /2.2.2.tar.gz`
		`- /2.2.3.tar.gz`
		`- /2.2.4.tar.gz`
		`- /2.3.0.tar.gz`
		`- /2.3.1.tar.gz`
		`- /2.3.2.tar.gz`
		`- /2.3.3.tar.gz`
		`- /2.4.0.tar.gz`
		`- /bodhi-2.5.0.tar.gz`
		`- /bodhi-2.6.0.tar.gz`
		`- /bodhi-2.6.1.tar.gz`
		`- /bodhi-2.6.2.tar.gz`
		`- /bodhi-2.7.0.tar.gz`
		`- /bodhi-2.8.0.tar.gz`
		`- /bodhi-2.8.1.tar.gz`
		`- /bodhi-2.9.0.tar.gz`
		`- /bodhi-2.9.1.tar.gz`
		`- /bodhi-2.10.0.tar.gz`
		`- /bodhi-2.10.1.tar.gz`
		`- /bodhi-2.11.0.tar.gz`
		`- /bodhi-2.12.0.tar.gz`
		`- /bodhi-2.12.1.tar.gz`
		`- /bodhi-2.12.2.tar.gz`
		`- /bodhi-3.0.0.tar.gz`
		`- /bodhi-3.1.0.tar.gz`
		`- /bodhi-3.2.0.tar.gz`
		`- /bodhi-3.3.0.tar.gz`
		`- /bodhi-3.4.0.tar.gz`
		`- /bodhi-3.5.0.tar.gz`
		`- /bodhi-3.5.1.tar.gz`
		`- /bodhi-3.5.2.tar.gz`
		`- /bodhi-3.6.0.tar.gz`
		`- /bodhi-3.6.1.tar.gz`
		`- /bodhi-3.7.0.tar.gz`
		`- /bodhi-3.8.0.tar.gz`
		`- /bodhi-3.8.1.tar.gz`
		`+ /*.rpm`
		`+ /*.tar.gz`
		`+ /bodhi-*/`
		`+ /results_*/`

bodhi.rpmlintrc

file modified

-1

		`@@ -4,7 +4,6 @@`
		`addFilter('explicit-lib-dependency liberation-mono-fonts')`
		`addFilter('hidden-file-or-dir /usr/lib/python2.7/site-packages/bodhi/server/locale/.placeholder')`
		`addFilter('hidden-file-or-dir /usr/share/doc/bodhi-docs/html/.buildinfo')`
		`- addFilter('non-conffile-in-etc /etc/bash_completion.d/bodhi-complete.sh')`
		`# These two files contain secrets`
		`addFilter('non-readable /etc/bodhi/alembic.ini')`
		`addFilter('non-readable /etc/bodhi/production.ini')`

bodhi.spec

file modified

+10 -5

		`@@ -1,6 +1,9 @@`
		`+ %global bashcompdir %(pkg-config --variable=completionsdir bash-completion 2>/dev/null)`
		`+ %global bashcomproot %(dirname %{bashcompdir} 2>/dev/null)`
		`+`
		`Name: bodhi`
		`Version: 3.8.1`
		`- Release: 2%{?dist}`
		`+ Release: 3%{?dist}`
		`BuildArch: noarch`

		`License: GPLv2+`
		`@@ -76,6 +79,7 @@`
		`BuildRequires: koji`
		`BuildRequires: liberation-mono-fonts`
		`BuildRequires: packagedb-cli`
		`+ BuildRequires: pkgconfig(bash-completion)`
		`BuildRequires: pungi >= 4.1.20`
		`BuildRequires: python2-createrepo_c`
		`BuildRequires: python2-devel`
		`@@ -107,7 +111,6 @@`
		`Summary: Bodhi Client`
		`Group: Applications/Internet`

		`- Requires: filesystem`
		`Requires: python3-bodhi-client == %{version}-%{release}`


		`@@ -307,14 +310,13 @@`

		`%{__mkdir_p} %{buildroot}/var/lib/bodhi`
		`%{__mkdir_p} %{buildroot}/var/cache/bodhi`
		`- %{__mkdir_p} %{buildroot}%{_sysconfdir}/bash_completion.d`
		`%{__mkdir_p} %{buildroot}%{_sysconfdir}/httpd/conf.d`
		`%{__mkdir_p} %{buildroot}%{_sysconfdir}/fedmsg.d`
		`%{__mkdir_p} %{buildroot}%{_sysconfdir}/bodhi`
		`%{__mkdir_p} %{buildroot}%{_datadir}/%{name}`
		`%{__mkdir_p} -m 0755 %{buildroot}/%{_localstatedir}/log/bodhi`

		`- %{__install} -m 0755 bodhi-complete.sh %{buildroot}%{_sysconfdir}/bash_completion.d`
		`+ %{__install} -Dpm 0755 bodhi-complete.sh %{buildroot}%{bashcompdir}/%{name}`
		`%{__install} -m 644 apache/%{name}.conf %{buildroot}%{_sysconfdir}/httpd/conf.d/%{name}.conf`
		`%{__install} -m 640 production.ini %{buildroot}%{_sysconfdir}/%{name}/production.ini`
		`%{__install} -m 640 alembic.ini %{buildroot}%{_sysconfdir}/%{name}/alembic.ini`
		`@@ -366,7 +368,7 @@`
		`%files client`
		`%license COPYING`
		`%doc README.rst`
		`- %{_sysconfdir}/bash_completion.d/bodhi-complete.sh`
		`+ %{bashcomproot}`
		`%{_bindir}/bodhi`
		`%{_mandir}/man1/bodhi.1*`

		`@@ -444,6 +446,9 @@`


		`%changelog`
		`+ * Thu Jun 21 2018 Todd Zullinger <tmz@pobox.com> - 3.8.1-3`
		`+ - Use recommended directory for bash-completion file`
		`+`
		`* Tue Jun 19 2018 Miro Hrončok <mhroncok@redhat.com> - 3.8.1-2`
		`- Rebuilt for Python 3.7`

tmz commented 4 years ago

Hi,

I noticed that bodhi-client installs its bash-completion script in /etc/bash_completion.d. The recommended location is /usr/share/bash-completion/completions (defined via pkg-config). The old location has several downsides (primarily the lack of on-demand loading of completions and the rpmlint non-conffile-in-etc warning).

The completion is installed as bodhi because the on-demand completion support looks for the completion file based on the the command name being completed. Let me know if that's something which you feel should be noted in the commit message and/or spec file.

While I was here, I simplified the .gitignore file. I also dropped the mkdir -p for the completion directory and used install -D to install the completion file; install -D creates all leading directories.

tmz commented 4 years ago

As a side note, the 'Signed-off-by' requirement is surprising to see on package repos. It required me to run git rebase -i --signoff and force push to my fork. That's not much effort for me, but I maintain the git package so rebasing is something I do frequently. I know many other contributors aren't as familiar with how to fix things when they get the rejection due to this requirement.

I'm not sure how much value there is in requiring it for package repositories to be honest. But perhaps if it's going to be required it would be good to create a README file which pagure will display stating the requirement so contributors have a better chance of seeing it before hitting the rejection. [Ideally, pagure itself would link to some documentation describing how to go about fixing the missing signoffs and updating the source branch of the PR for users who are not intimately familiar with git, but that's a bug/PR to file with pagure. :)]

Thanks.