Bug#987181: RFS: cpufetch/0.97-1 -- Simple yet fancy CPU architecture fetching tool
Tobias Frost <tobi@debian.org> 于2021年6月9日周三 上午2:03写道：
>
> On Tue, Jun 08, 2021 at 02:50:47PM +0800, clay stan wrote:
> > Tobias Frost <tobi@debian.org> 于2021年6月3日周四 上午1:33写道：
> > >
> > > Control: tags -1 moreinfo
> > >
> > > Hi Clay,
> > >
> > > here's a review:
> > > - The patch: The dep3 header, the field Bug-Debian is wrong, the ITP is not
> > > related to the patch
> >
> > Thanks, I've updated.
> >
> > > - The patch looks strange to me: Why do you patch the Makefile? What do you
> > > want to archieve? Parts of the patching seems ok (like avoiding stomping over
> > > CFLAGS, but other parts seems excessive, removing sane parts to me…
> >
> > The original compilation parameters will also cause Lintian to report errors,
> > hardening no bind now, hardening no mandatory functions.
> > I'll try to solve them in debian/rules by
> > https://wiki.debian.org/Hardening, but it doesn't work
> >
> >and the sane parts, you mean?
>
> You've basically completly rewritten a (mostly) working Makefile
> With your comment I assume that you just wanted to have hardening,
> so _just_ those parts should have been patched.
> And that would be:
>
> --- a/Makefile
> +++ b/Makefile
> @@ -1,6 +1,6 @@
> CC ?= gcc
>
> -CFLAGS+=-Wall -Wextra -pedantic -fstack-protector-all -pedantic -std=c99
> +CFLAGS:=-Wall -Wextra -pedantic -fstack-protector-all -pedantic -std=c99 ${CFLAGS} ${CPPFLAGS} ${LDFLAGS}
> SANITY_FLAGS=-Wfloat-equal -Wshadow -Wpointer-arith
>
> PREFIX ?= /usr
>
> (+ adding export DEB_BUILD_MAINT_OPTIONS = hardening=+all to d/rules)
>
> So, see it as recommendation: I'd keep patches really minimal. Or they
> will create you a lot of work…
>
> It is a burden to carry such an intrusive patch, especially if upstream
> explictly expressed that he does not generally welcome patches
> if they target the Makefile.
>
> Just one example where I think the upstream part is better:
> -PREFIX ?= /usr
> +PREFIX = /usr
>
> Some other stuff could be done with debhelper overrides, so
> that the patch can get smaller…
Take your advice, now patch only changed one line.
>
> But you do you… I just strongly recommend to revisit the topic.
> On top, non-upstreamable patches are bad…
>
> > > - Upstream seems to support arm, you patch that out?
> >
> > Upstream support arm means the mobile arm chips, like Snapdragon, MediaTek,
> > not arm PC, They are not supported by Debian.
>
> This is not a strong reason to disable arm support entirely…
> (A quick internet search suggests that there is some support for Debian on e.g
> Snapdragon. And there seems to be derivates; even if not, maybe someone wants
> to enhance cpufetch on the basis of the Debian package.)
Add arm64 now.
>
> > > - There is no LDCFLAGS -> did you mean LDFLAGS?
> >
> > Yeah, I've updated. Thanks again.
> >
> > >
> > > - (not a blocker) Please send the manpage upstream for inclusion.
>
> Regarding the manpage: I've diffed the manpage (cpufetch.1 and
> debian/cpufetch.1). They are almost identical. With just small fixes done by
> you. However, you claim (sole) copyright on it. That is not appropiate.
Previously, because there has a problem with the upstream manpage,
So I used help2man to regenerate a manpage then modify it,
so I claim copyright on it.
But now I have contacted the upstream to repaired his manpage and
remove my manpage.
And I rebuild the package with a new version and upload to mentors:
https://mentors.debian.net/package/cpufetch
You can download the package with dget using this command:
dget -x https://mentors.debian.net/debian/pool/main/c/cpufetch/cpufetch_0.98-1.dsc
Thanks again.
>
> I would patch the manpage, if it needs fixing: The header of the upstream ones
> says that upstream builds it using help2man (but hav not integrated that into
> their Makefile), so possibly this is another route to go: Rebuild at build
> time.
>
> --
> tobi
Reply to: