[Bug 2137932] Review Request: bzip3 - Tools for compressing and decompressing bzip3 files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=2137932



--- Comment #5 from Petr Pisar <ppisar@xxxxxxxxxx> ---
(In reply to Neal Gompa from comment #4)
> Initial spec review notes:
> 
> > autoreconf -fi
> 
> Please add -v to this so we have output logged.
> 
Nonverbose mode already logs which files it touches:

$ autoreconf -fi
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'build-aux'.
libtoolize: copying file 'build-aux/libtool.m4'
libtoolize: copying file 'build-aux/ltoptions.m4'
libtoolize: copying file 'build-aux/ltsugar.m4'
libtoolize: copying file 'build-aux/ltversion.m4'
libtoolize: copying file 'build-aux/lt~obsolete.m4'
configure.ac:12: installing 'build-aux/compile'
configure.ac:14: installing 'build-aux/config.guess'
configure.ac:14: installing 'build-aux/config.sub'
configure.ac:4: installing 'build-aux/install-sh'
configure.ac:4: installing 'build-aux/missing'
Makefile.am: installing 'build-aux/depcomp'

Enabling a verbose mode, in my opinion, only adds a clutter:

--- old 2022-11-02 09:17:51.763826937 +0100
+++ new 2022-11-02 09:18:35.188951224 +0100
@@ -1,4 +1,10 @@
-$ autoreconf -fi
+$ autoreconf -vfi
+autoreconf: export WARNINGS=
+autoreconf: Entering directory '.'
+autoreconf: configure.ac: not using Gettext
+autoreconf: running: aclocal --force -I build-aux
+autoreconf: configure.ac: tracing
+autoreconf: running: libtoolize --copy --force
 libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
 libtoolize: copying file 'build-aux/ltmain.sh'
 libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'build-aux'.
@@ -7,9 +13,16 @@
 libtoolize: copying file 'build-aux/ltsugar.m4'
 libtoolize: copying file 'build-aux/ltversion.m4'
 libtoolize: copying file 'build-aux/lt~obsolete.m4'
+autoreconf: configure.ac: not using Intltool
+autoreconf: configure.ac: not using Gtkdoc
+autoreconf: running: aclocal --force -I build-aux
+autoreconf: running: /usr/bin/autoconf --force
+autoreconf: configure.ac: not using Autoheader
+autoreconf: running: automake --add-missing --copy --force-missing
 configure.ac:12: installing 'build-aux/compile'
 configure.ac:14: installing 'build-aux/config.guess'
 configure.ac:14: installing 'build-aux/config.sub'
 configure.ac:4: installing 'build-aux/install-sh'
 configure.ac:4: installing 'build-aux/missing'
 Makefile.am: installing 'build-aux/depcomp'
+autoreconf: Leaving directory '.'

Is that really helpful?

> > %{make_build}
> > [...]
> > %{make_install}
> 
> What's with the braces here? It's a bit odd...
> 
Without the braces any positional argument becomes arguments of the spec macro.
Not arguments of the expanded shell command. An explicit termination of the
macro makes the line proof of future changes, either in the macro definition or
in adding new arguments.

> > # Deduplicate identical files
> > if cmp %{buildroot}%{_mandir}/man1/{bz3cat,bunzip3}.1; then
> >     rm %{buildroot}%{_mandir}/man1/bunzip3.1
> >     ln -s bz3cat.1 %{buildroot}%{_mandir}/man1/bunzip3.1
> > fi
> 
> This are already solink man pages, this is unnecessarily extra work.
> 
rpmlint complains on duplicate files.

> > %{_bindir}/*
> > %{_mandir}/man1/*.1*
> 
> This is too promiscuous and needs to be made more restrictive.
> 
> Cf.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists
> 
Thanks for highlighting this new rule. I did not know about it. I will adapt
the spec file.

> > %{_libdir}/libbzip3.so.0
> > %{_libdir}/libbzip3.so.0.*
> 
> This can be simplified to: "%{_libdir}/libbzip3.so.0{,.*}"
> 
Indeed. I will do it. I thought that rpmbuild support shell globs, but not
shell brace expansion.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2137932
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux