* [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
[not found] <20111011164918.2103A2004B@flycatcher.gentoo.org>
@ 2011-10-11 16:59 ` Samuli Suominen
2011-10-11 17:05 ` Fabian Groffen
0 siblings, 1 reply; 22+ messages in thread
From: Samuli Suominen @ 2011-10-11 16:59 UTC (permalink / raw
To: gentoo-dev
On 10/11/2011 07:49 PM, Fabian Groffen (grobian) wrote:
> grobian 11/10/11 16:49:18
>
> Modified: ChangeLog chrpath-0.13-r2.ebuild
> Log:
> Revert ssuominen's changes that were totally uncalled for and most importantly broke the installation of this package on the main consumer's platform: Prefix
>
> (Portage version: 2.2.01.19295-prefix/cvs/SunOS i386)
>
> Revision Changes Path
> 1.17 app-admin/chrpath/ChangeLog
>
> file : http://sources.gentoo.org/viewvc.cgi/gentoo-x86/app-admin/chrpath/ChangeLog?rev=1.17&view=markup
> plain: http://sources.gentoo.org/viewvc.cgi/gentoo-x86/app-admin/chrpath/ChangeLog?rev=1.17&content-type=text/plain
> diff : http://sources.gentoo.org/viewvc.cgi/gentoo-x86/app-admin/chrpath/ChangeLog?r1=1.16&r2=1.17
>
> Index: ChangeLog
> ===================================================================
> RCS file: /var/cvsroot/gentoo-x86/app-admin/chrpath/ChangeLog,v
> retrieving revision 1.16
> retrieving revision 1.17
> diff -u -r1.16 -r1.17
> --- ChangeLog 10 Oct 2011 22:28:37 -0000 1.16
> +++ ChangeLog 11 Oct 2011 16:49:17 -0000 1.17
> @@ -1,6 +1,11 @@
> # ChangeLog for app-admin/chrpath
> # Copyright 1999-2011 Gentoo Foundation; Distributed under the GPL v2
> -# $Header: /var/cvsroot/gentoo-x86/app-admin/chrpath/ChangeLog,v 1.16 2011/10/10 22:28:37 ssuominen Exp $
> +# $Header: /var/cvsroot/gentoo-x86/app-admin/chrpath/ChangeLog,v 1.17 2011/10/11 16:49:17 grobian Exp $
> +
> + 11 Oct 2011; Fabian Groffen <grobian@gentoo.org> chrpath-0.13-r2.ebuild:
> + Revert ssuominen's changes that were totally uncalled for and most
> + importantly broke the installation of this package on the main consumer's
> + platform: Prefix
>
> 10 Oct 2011; Samuli Suominen <ssuominen@gentoo.org> chrpath-0.13-r2.ebuild:
> Remove unnecessary static libraries because chrpath is dlopening the dynamic
>
>
>
> 1.3 app-admin/chrpath/chrpath-0.13-r2.ebuild
>
> file : http://sources.gentoo.org/viewvc.cgi/gentoo-x86/app-admin/chrpath/chrpath-0.13-r2.ebuild?rev=1.3&view=markup
> plain: http://sources.gentoo.org/viewvc.cgi/gentoo-x86/app-admin/chrpath/chrpath-0.13-r2.ebuild?rev=1.3&content-type=text/plain
> diff : http://sources.gentoo.org/viewvc.cgi/gentoo-x86/app-admin/chrpath/chrpath-0.13-r2.ebuild?r1=1.2&r2=1.3
>
> Index: chrpath-0.13-r2.ebuild
> ===================================================================
> RCS file: /var/cvsroot/gentoo-x86/app-admin/chrpath/chrpath-0.13-r2.ebuild,v
> retrieving revision 1.2
> retrieving revision 1.3
> diff -u -r1.2 -r1.3
> --- chrpath-0.13-r2.ebuild 10 Oct 2011 22:28:37 -0000 1.2
> +++ chrpath-0.13-r2.ebuild 11 Oct 2011 16:49:17 -0000 1.3
> @@ -1,13 +1,15 @@
> # Copyright 1999-2011 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> -# $Header: /var/cvsroot/gentoo-x86/app-admin/chrpath/chrpath-0.13-r2.ebuild,v 1.2 2011/10/10 22:28:37 ssuominen Exp $
> +# $Header: /var/cvsroot/gentoo-x86/app-admin/chrpath/chrpath-0.13-r2.ebuild,v 1.3 2011/10/11 16:49:17 grobian Exp $
>
> -EAPI=4
> -inherit autotools eutils
> +EAPI="2"
> +
> +inherit eutils autotools
>
> DESCRIPTION="chrpath can modify the rpath and runpath of ELF executables"
> HOMEPAGE="http://directory.fsf.org/project/chrpath/"
> -SRC_URI="mirror://gentoo/${P}.tar.gz"
> +# original upstream no longer exists (ftp://ftp.hungry.com/pub/hungry)
> +SRC_URI="http://ftp.tux.org/pub/X-Windows/ftp.hungry.com/chrpath/${P}.tar.gz"
>
> LICENSE="GPL-2"
> SLOT="0"
> @@ -15,21 +17,14 @@
> IUSE=""
>
> src_prepare() {
> - epatch \
> - "${FILESDIR}"/${P}-multilib.patch \
> - "${FILESDIR}"/${PN}-keepgoing.patch \
> - "${FILESDIR}"/${P}-testsuite-1.patch
> -
> + epatch "${FILESDIR}"/${P}-multilib.patch
> + epatch "${FILESDIR}"/${PN}-keepgoing.patch
> + epatch "${FILESDIR}"/${P}-testsuite-1.patch
> + sed -i -e '/^docdir/d' Makefile.am # use standard docdir
> eautoreconf
> }
>
> src_install() {
> - emake \
> - DESTDIR="${D}" \
> - docdir=/usr/share/doc/${PF} \
> - install
> -
> - rm -f \
> - "${ED}"usr/lib*/lib*.{a,la} \
> - "${ED}"usr/share/doc/${PF}/{COPYING,INSTALL}
> + emake install DESTDIR="${D}" || die
> + dodoc ChangeLog AUTHORS NEWS README
> }
You just broke the package again by reverting technically correct commits:
Documentation:
>>> /usr/share/doc/chrpath-0.13-r2/ChangeLog.bz2
>>> /usr/share/doc/chrpath-0.13-r2/AUTHORS.bz2
>>> /usr/share/doc/chrpath-0.13-r2/NEWS.bz2
>>> /usr/share/doc/chrpath-0.13-r2/README.bz2
>>> /usr/share/doc/-chrpath-/
>>> /usr/share/doc/-chrpath-/README
>>> /usr/share/doc/-chrpath-/NEWS
>>> /usr/share/doc/-chrpath-/INSTALL
>>> /usr/share/doc/-chrpath-/ChangeLog
>>> /usr/share/doc/-chrpath-/COPYING
>>> /usr/share/doc/-chrpath-/AUTHORS
Unused libtool files:
>>> /usr/lib64/libchrpath64.la
>>> /usr/lib64/libchrpath32.la
Libs used only by chrpath itself now install static versions of
themself, unused:
>>> /usr/lib64/libchrpath32.a
>>> /usr/lib64/libchrpath64.a
So I've missed one ${EPREFIX} for docdir= ? How about just fixing that,
and not crapping all over the package?
- Samuli
^ permalink raw reply [flat|nested] 22+ messages in thread
* [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 16:59 ` [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild Samuli Suominen
@ 2011-10-11 17:05 ` Fabian Groffen
2011-10-11 18:01 ` Samuli Suominen
0 siblings, 1 reply; 22+ messages in thread
From: Fabian Groffen @ 2011-10-11 17:05 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 486 bytes --]
On 11-10-2011 19:59:13 +0300, Samuli Suominen wrote:
> So I've missed one ${EPREFIX} for docdir= ? How about just fixing that,
> and not crapping all over the package?
How about first asking the maintainer before you completely rewrite an
ebuild? I'm not innocent on this topic either (ask Diego for example),
and I do allow you to make a lot of changes to my packages. Just don't
force your style and preferences on me.
--
Fabian Groffen
Gentoo on a different level
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 17:05 ` Fabian Groffen
@ 2011-10-11 18:01 ` Samuli Suominen
2011-10-11 18:13 ` Fabian Groffen
0 siblings, 1 reply; 22+ messages in thread
From: Samuli Suominen @ 2011-10-11 18:01 UTC (permalink / raw
To: gentoo-dev
On 10/11/2011 08:05 PM, Fabian Groffen wrote:
> On 11-10-2011 19:59:13 +0300, Samuli Suominen wrote:
>> So I've missed one ${EPREFIX} for docdir= ? How about just fixing that,
>> and not crapping all over the package?
>
> How about first asking the maintainer before you completely rewrite an
> ebuild? I'm not innocent on this topic either (ask Diego for example),
> and I do allow you to make a lot of changes to my packages. Just don't
> force your style and preferences on me.
>
>
So basically you are saying you reverted tehnically correct changes for
cosmetics. What ticked you off, the \ lines changes? I believe that was
the only change that wasn't about fixing something broken.
And so have you, changed dozens of my ebuilds for PREFIX compability or
other random fixes, not everything turned out correct, mistakes were
made and were clearly accidental. I've fixed them instead of wasting
both of our times. Is it too much to ask for same in return?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 18:01 ` Samuli Suominen
@ 2011-10-11 18:13 ` Fabian Groffen
2011-10-11 18:34 ` Samuli Suominen
0 siblings, 1 reply; 22+ messages in thread
From: Fabian Groffen @ 2011-10-11 18:13 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]
On 11-10-2011 21:01:40 +0300, Samuli Suominen wrote:
> On 10/11/2011 08:05 PM, Fabian Groffen wrote:
> > On 11-10-2011 19:59:13 +0300, Samuli Suominen wrote:
> >> So I've missed one ${EPREFIX} for docdir= ? How about just fixing that,
> >> and not crapping all over the package?
> >
> > How about first asking the maintainer before you completely rewrite an
> > ebuild? I'm not innocent on this topic either (ask Diego for example),
> > and I do allow you to make a lot of changes to my packages. Just don't
> > force your style and preferences on me.
>
> So basically you are saying you reverted tehnically correct changes for
> cosmetics. What ticked you off, the \ lines changes? I believe that was
> the only change that wasn't about fixing something broken.
No, you broke the package for Prefix. Next you bumped it to EAPI=4,
then you removed SRC_URI for no particular reason, dropped libtool files
and dropped static archives. Next to this you did some reordering and
other cosmetic changes.
> And so have you, changed dozens of my ebuilds for PREFIX compability or
> other random fixes, not everything turned out correct, mistakes were
> made and were clearly accidental. I've fixed them instead of wasting
> both of our times. Is it too much to ask for same in return?
I already told you that you can change quite a lot to my ebuilds,
without me complaining. Do you mind me reverting your stuff and redoing
it now?
--
Fabian Groffen
Gentoo on a different level
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 18:13 ` Fabian Groffen
@ 2011-10-11 18:34 ` Samuli Suominen
2011-10-11 18:46 ` Fabian Groffen
2011-10-11 18:50 ` Matt Turner
0 siblings, 2 replies; 22+ messages in thread
From: Samuli Suominen @ 2011-10-11 18:34 UTC (permalink / raw
To: gentoo-dev
On 10/11/2011 09:13 PM, Fabian Groffen wrote:
> On 11-10-2011 21:01:40 +0300, Samuli Suominen wrote:
>> On 10/11/2011 08:05 PM, Fabian Groffen wrote:
>>> On 11-10-2011 19:59:13 +0300, Samuli Suominen wrote:
>>>> So I've missed one ${EPREFIX} for docdir= ? How about just fixing that,
>>>> and not crapping all over the package?
>>>
>>> How about first asking the maintainer before you completely rewrite an
>>> ebuild? I'm not innocent on this topic either (ask Diego for example),
>>> and I do allow you to make a lot of changes to my packages. Just don't
>>> force your style and preferences on me.
>>
>> So basically you are saying you reverted tehnically correct changes for
>> cosmetics. What ticked you off, the \ lines changes? I believe that was
>> the only change that wasn't about fixing something broken.
>
> No, you broke the package for Prefix. Next you bumped it to EAPI=4,
> then you removed SRC_URI for no particular reason, dropped libtool files
> and dropped static archives. Next to this you did some reordering and
> other cosmetic changes.
>
>> And so have you, changed dozens of my ebuilds for PREFIX compability or
>> other random fixes, not everything turned out correct, mistakes were
>> made and were clearly accidental. I've fixed them instead of wasting
>> both of our times. Is it too much to ask for same in return?
>
> I already told you that you can change quite a lot to my ebuilds,
> without me complaining. Do you mind me reverting your stuff and redoing
> it now?
>
>
Thanks, the end result of installed files look now OK. Care to reopen
the stabilization bug? The changes are trivial.
I just hope nobody will take an example of the ebuild with code
duplication (multiple epatch calls), overquoting, redudant use of find
when rm is more than enough, ...
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 18:34 ` Samuli Suominen
@ 2011-10-11 18:46 ` Fabian Groffen
2011-10-11 19:38 ` Samuli Suominen
2011-10-11 18:50 ` Matt Turner
1 sibling, 1 reply; 22+ messages in thread
From: Fabian Groffen @ 2011-10-11 18:46 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 726 bytes --]
On 11-10-2011 21:34:22 +0300, Samuli Suominen wrote:
> Thanks, the end result of installed files look now OK. Care to reopen
> the stabilization bug? The changes are trivial.
Shall we stick to the policy and wait 30 days without bugs first?
> I just hope nobody will take an example of the ebuild with code
> duplication (multiple epatch calls), overquoting, redudant use of find
> when rm is more than enough, ...
I just like the documented way of doing things [1], so I hope many
people will just stick to that for maintainability, in favour of pseudo
efficiency.
[1] http://devmanual.gentoo.org/ebuild-writing/functions/src_prepare/epatch/index.html
--
Fabian Groffen
Gentoo on a different level
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 18:34 ` Samuli Suominen
2011-10-11 18:46 ` Fabian Groffen
@ 2011-10-11 18:50 ` Matt Turner
2011-10-11 19:48 ` Michał Górny
1 sibling, 1 reply; 22+ messages in thread
From: Matt Turner @ 2011-10-11 18:50 UTC (permalink / raw
To: gentoo-dev
On Tue, Oct 11, 2011 at 2:34 PM, Samuli Suominen <ssuominen@gentoo.org> wrote:
> I just hope nobody will take an example of the ebuild with code
> duplication (multiple epatch calls), overquoting, redudant use of find
> when rm is more than enough, ...
I haven't looked, but if we don't already, a little style guide would
be very cool. I wouldn't think of most of these things without seeing
it somewhere else first.
It looks like the epatch devmanual page uses multiple calls. :\
Matt
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 18:46 ` Fabian Groffen
@ 2011-10-11 19:38 ` Samuli Suominen
2011-10-11 19:49 ` Fabian Groffen
0 siblings, 1 reply; 22+ messages in thread
From: Samuli Suominen @ 2011-10-11 19:38 UTC (permalink / raw
To: gentoo-dev
On 10/11/2011 09:46 PM, Fabian Groffen wrote:
> On 11-10-2011 21:34:22 +0300, Samuli Suominen wrote:
>> Thanks, the end result of installed files look now OK. Care to reopen
>> the stabilization bug? The changes are trivial.
>
> Shall we stick to the policy and wait 30 days without bugs first?
OK, no hurry.
>> I just hope nobody will take an example of the ebuild with code
>> duplication (multiple epatch calls), overquoting, redudant use of find
>> when rm is more than enough, ...
>
> I just like the documented way of doing things [1], so I hope many
> people will just stick to that for maintainability, in favour of pseudo
> efficiency.
>
>
> [1] http://devmanual.gentoo.org/ebuild-writing/functions/src_prepare/epatch/index.html
>
This document should be fixed. Any comments about the patches belong to
header of those patches, available for possible upstreams as well.
Doesn't belong to ebuilds. So it very rarely makes sense to call epatch
multiple times, and certainly doesn't improve maintainability.
- Samuli
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 18:50 ` Matt Turner
@ 2011-10-11 19:48 ` Michał Górny
2011-10-12 3:30 ` [gentoo-dev] " Steven J Long
0 siblings, 1 reply; 22+ messages in thread
From: Michał Górny @ 2011-10-11 19:48 UTC (permalink / raw
To: gentoo-dev; +Cc: mattst88
[-- Attachment #1: Type: text/plain, Size: 947 bytes --]
On Tue, 11 Oct 2011 14:50:30 -0400
Matt Turner <mattst88@gentoo.org> wrote:
> On Tue, Oct 11, 2011 at 2:34 PM, Samuli Suominen
> <ssuominen@gentoo.org> wrote:
> > I just hope nobody will take an example of the ebuild with code
> > duplication (multiple epatch calls), overquoting, redudant use of
> > find when rm is more than enough, ...
>
> I haven't looked, but if we don't already, a little style guide would
> be very cool. I wouldn't think of most of these things without seeing
> it somewhere else first.
>
> It looks like the epatch devmanual page uses multiple calls. :\
I don't think that passing multiple files to epatch actually improves
readability. Simple example:
# bug #123456, foo, bar
epatch "${FILESDIR}"/${P}-foo.patch
# bug #234567, baz bazinga blah blah
epatch "${FILESDIR}"/${P}-baz.patch
With multiple arguments, you can't put comments in the middle.
--
Best regards,
Michał Górny
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 19:38 ` Samuli Suominen
@ 2011-10-11 19:49 ` Fabian Groffen
2011-10-11 20:00 ` Samuli Suominen
0 siblings, 1 reply; 22+ messages in thread
From: Fabian Groffen @ 2011-10-11 19:49 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 513 bytes --]
On 11-10-2011 22:38:10 +0300, Samuli Suominen wrote:
> This document should be fixed. Any comments about the patches belong to
> header of those patches, available for possible upstreams as well.
> Doesn't belong to ebuilds.
The devmanual doesn't suggest this is the way to go, does it?
> So it very rarely makes sense to call epatch multiple times, and
> certainly doesn't improve maintainability.
Conclusion doesn't follow from argumentation.
--
Fabian Groffen
Gentoo on a different level
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 19:49 ` Fabian Groffen
@ 2011-10-11 20:00 ` Samuli Suominen
2011-10-11 20:04 ` Fabian Groffen
0 siblings, 1 reply; 22+ messages in thread
From: Samuli Suominen @ 2011-10-11 20:00 UTC (permalink / raw
To: gentoo-dev
On 10/11/2011 10:49 PM, Fabian Groffen wrote:
> On 11-10-2011 22:38:10 +0300, Samuli Suominen wrote:
>> This document should be fixed. Any comments about the patches belong to
>> header of those patches, available for possible upstreams as well.
>> Doesn't belong to ebuilds.
>
> The devmanual doesn't suggest this is the way to go, does it?
No, but it should. See below.
>> So it very rarely makes sense to call epatch multiple times, and
>> certainly doesn't improve maintainability.
>
> Conclusion doesn't follow from argumentation.
>
>
http://dev.gentoo.org/~vapier/clean-patches
Look for "here's a check list of things to keep in the patch header:"
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 20:00 ` Samuli Suominen
@ 2011-10-11 20:04 ` Fabian Groffen
2011-10-11 20:13 ` Samuli Suominen
0 siblings, 1 reply; 22+ messages in thread
From: Fabian Groffen @ 2011-10-11 20:04 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 220 bytes --]
On 11-10-2011 23:00:19 +0300, Samuli Suominen wrote:
> > The devmanual doesn't suggest this is the way to go, does it?
>
> No, but it should.
different topic
--
Fabian Groffen
Gentoo on a different level
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 20:04 ` Fabian Groffen
@ 2011-10-11 20:13 ` Samuli Suominen
0 siblings, 0 replies; 22+ messages in thread
From: Samuli Suominen @ 2011-10-11 20:13 UTC (permalink / raw
To: gentoo-dev
On 10/11/2011 11:04 PM, Fabian Groffen wrote:
> On 11-10-2011 23:00:19 +0300, Samuli Suominen wrote:
>>> The devmanual doesn't suggest this is the way to go, does it?
>>
>> No, but it should.
>
> different topic
still on the same one.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [gentoo-dev] Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-11 19:48 ` Michał Górny
@ 2011-10-12 3:30 ` Steven J Long
2011-10-12 19:19 ` Samuli Suominen
0 siblings, 1 reply; 22+ messages in thread
From: Steven J Long @ 2011-10-12 3:30 UTC (permalink / raw
To: gentoo-dev
Michał Górny wrote:
> I don't think that passing multiple files to epatch actually improves
> readability. Simple example:
>
> # bug #123456, foo, bar
> epatch "${FILESDIR}"/${P}-foo.patch
> # bug #234567, baz bazinga blah blah
> epatch "${FILESDIR}"/${P}-baz.patch
>
> With multiple arguments, you can't put comments in the middle.
>
++ It's also a lot easier to remove the single patches when they're no
longer needed. In the context of configuring, building and installing a
package, the extra overhead is miniscule, whereas the above is *much*
easier to maintain.
--
#friendly-coders -- We're friendly, but we're not /that/ friendly ;-)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-12 3:30 ` [gentoo-dev] " Steven J Long
@ 2011-10-12 19:19 ` Samuli Suominen
2011-10-12 19:33 ` Mike Frysinger
2011-10-14 4:39 ` [gentoo-dev] " Steven J Long
0 siblings, 2 replies; 22+ messages in thread
From: Samuli Suominen @ 2011-10-12 19:19 UTC (permalink / raw
To: gentoo-dev
On 10/12/2011 06:30 AM, Steven J Long wrote:
> Michał Górny wrote:
>> I don't think that passing multiple files to epatch actually improves
>> readability. Simple example:
>>
>> # bug #123456, foo, bar
>> epatch "${FILESDIR}"/${P}-foo.patch
>> # bug #234567, baz bazinga blah blah
>> epatch "${FILESDIR}"/${P}-baz.patch
>>
>> With multiple arguments, you can't put comments in the middle.
>>
> ++ It's also a lot easier to remove the single patches when they're no
> longer needed.
Removing an 'epatch foo' line is easier than 'foo \' ? You are kidding,
right?
> In the context of configuring, building and installing a
> package, the extra overhead is miniscule, whereas the above is *much*
> easier to maintain.
Based on what argument?
Having the comments inside the patch allows everyone, including
_upstreams_ straight up see what's it for and link to the bug it's
coming from. Where as keeping them in ebuilds makes it Gentoo specific,
which is not what we are about.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-12 19:19 ` Samuli Suominen
@ 2011-10-12 19:33 ` Mike Frysinger
2011-10-12 19:44 ` Alec Warner
2011-10-14 4:39 ` [gentoo-dev] " Steven J Long
1 sibling, 1 reply; 22+ messages in thread
From: Mike Frysinger @ 2011-10-12 19:33 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 2251 bytes --]
On Wednesday 12 October 2011 15:19:25 Samuli Suominen wrote:
> On 10/12/2011 06:30 AM, Steven J Long wrote:
> > Michał Górny wrote:
> >> I don't think that passing multiple files to epatch actually improves
> >> readability. Simple example:
> >>
> >> # bug #123456, foo, bar
> >> epatch "${FILESDIR}"/${P}-foo.patch
> >> # bug #234567, baz bazinga blah blah
> >> epatch "${FILESDIR}"/${P}-baz.patch
> >>
> >> With multiple arguments, you can't put comments in the middle.
> >
> > ++ It's also a lot easier to remove the single patches when they're no
> > longer needed.
>
> Removing an 'epatch foo' line is easier than 'foo \' ? You are kidding,
> right?
>
> > In the context of configuring, building and installing a
> > package, the extra overhead is miniscule, whereas the above is *much*
> > easier to maintain.
>
> Based on what argument?
>
> Having the comments inside the patch allows everyone, including
> _upstreams_ straight up see what's it for and link to the bug it's
> coming from. Where as keeping them in ebuilds makes it Gentoo specific,
> which is not what we are about.
i personally prefer:
epatch "${FILESDIR}"/${P}-foo.patch #12345
epatch "${FILESDIR}"/${P}-bar.patch #19512 #91991
epatch "${FILESDIR}"/${P}-fatcow.patch #19291
because i personally like to have just the bug number there
i know other people prefer to pass these all on one line:
epatch \
"${FILESDIR}"/${P}-foo.patch \
"${FILESDIR}"/${P}-bar.patch \
"${FILESDIR}"/${P}-fatcow.patch
there is no standard here (i think they're more or less equally common) and
maintainers are free to pick what they like best. arguing about the merits
between the two above styles is a waste of everyone's time. go fix some bugs
instead you lazy wankers :P.
the one thing Samuli is correct about though and largely has nothing to do
with style is that the patch itself needs to have all the relevant
information. doing the following is wrong:
# here i explain what the patch is for #12351
epatch "${FILESDIR}"/${P}-bar.patch
(and the bar patch contains only the diff)
rather than rehash why you're wrong if you do the above, please read:
http://dev.gentoo.org/~vapier/clean-patches
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-12 19:33 ` Mike Frysinger
@ 2011-10-12 19:44 ` Alec Warner
2011-10-12 19:51 ` Mike Frysinger
0 siblings, 1 reply; 22+ messages in thread
From: Alec Warner @ 2011-10-12 19:44 UTC (permalink / raw
To: gentoo-dev
On Wed, Oct 12, 2011 at 12:33 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 12 October 2011 15:19:25 Samuli Suominen wrote:
>> On 10/12/2011 06:30 AM, Steven J Long wrote:
>> > Michał Górny wrote:
>> >> I don't think that passing multiple files to epatch actually improves
>> >> readability. Simple example:
>> >>
>> >> # bug #123456, foo, bar
>> >> epatch "${FILESDIR}"/${P}-foo.patch
>> >> # bug #234567, baz bazinga blah blah
>> >> epatch "${FILESDIR}"/${P}-baz.patch
>> >>
>> >> With multiple arguments, you can't put comments in the middle.
>> >
>> > ++ It's also a lot easier to remove the single patches when they're no
>> > longer needed.
>>
>> Removing an 'epatch foo' line is easier than 'foo \' ? You are kidding,
>> right?
>>
>> > In the context of configuring, building and installing a
>> > package, the extra overhead is miniscule, whereas the above is *much*
>> > easier to maintain.
>>
>> Based on what argument?
>>
>> Having the comments inside the patch allows everyone, including
>> _upstreams_ straight up see what's it for and link to the bug it's
>> coming from. Where as keeping them in ebuilds makes it Gentoo specific,
>> which is not what we are about.
>
> i personally prefer:
> epatch "${FILESDIR}"/${P}-foo.patch #12345
> epatch "${FILESDIR}"/${P}-bar.patch #19512 #91991
> epatch "${FILESDIR}"/${P}-fatcow.patch #19291
> because i personally like to have just the bug number there
>
> i know other people prefer to pass these all on one line:
> epatch \
> "${FILESDIR}"/${P}-foo.patch \
> "${FILESDIR}"/${P}-bar.patch \
> "${FILESDIR}"/${P}-fatcow.patch
The problem with the latter is the same problem I have w/python lists
and commas.
If I want to add a patch to the list I might forget to to add the \
epatch \
"${FILESDIR}"/${P}-foo.patch \
"${FILESDIR}"/${P}-last.patch # <-- Oops I forgot to add a \ here
"${FILESDIR}"/${P}-my-new.patch
Or I delete the last patch and forget to remove the \
epatch \
"${FILESDIR}"/${P}-foo.patch \
"${FILESDIR}"/${P}-bar.patch \ # <-- oops again!
>
> there is no standard here (i think they're more or less equally common) and
> maintainers are free to pick what they like best. arguing about the merits
> between the two above styles is a waste of everyone's time. go fix some bugs
> instead you lazy wankers :P.
I enjoy wasting time :)
>
> the one thing Samuli is correct about though and largely has nothing to do
> with style is that the patch itself needs to have all the relevant
> information. doing the following is wrong:
> # here i explain what the patch is for #12351
> epatch "${FILESDIR}"/${P}-bar.patch
> (and the bar patch contains only the diff)
>
> rather than rehash why you're wrong if you do the above, please read:
> http://dev.gentoo.org/~vapier/clean-patches
> -mike
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-12 19:44 ` Alec Warner
@ 2011-10-12 19:51 ` Mike Frysinger
2011-10-12 19:57 ` Tomáš Chvátal
0 siblings, 1 reply; 22+ messages in thread
From: Mike Frysinger @ 2011-10-12 19:51 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 499 bytes --]
On Wednesday 12 October 2011 15:44:53 Alec Warner wrote:
> If I want to add a patch to the list I might forget to to add the \
admittedly, i hit this every once in a while, and with all the "|| die" being
implicit, it doesn't get caught right away. fortunately latest portage will
issue a QA warning at the end along the lines of "command not found:
${FILESDIR}/${P}-my-new.patch". so one can hope the maintainer tests their
ebuilds and reviews QA output before committing ;).
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-12 19:51 ` Mike Frysinger
@ 2011-10-12 19:57 ` Tomáš Chvátal
2011-10-12 19:59 ` Tomáš Chvátal
2011-10-12 20:15 ` Mike Frysinger
0 siblings, 2 replies; 22+ messages in thread
From: Tomáš Chvátal @ 2011-10-12 19:57 UTC (permalink / raw
To: gentoo-dev
2011/10/12 Mike Frysinger <vapier@gentoo.org>:
> On Wednesday 12 October 2011 15:44:53 Alec Warner wrote:
>> If I want to add a patch to the list I might forget to to add the \
>
> admittedly, i hit this every once in a while, and with all the "|| die" being
> implicit, it doesn't get caught right away. fortunately latest portage will
> issue a QA warning at the end along the lines of "command not found:
> ${FILESDIR}/${P}-my-new.patch". so one can hope the maintainer tests their
> ebuilds and reviews QA output before committing ;).
> -mike
>
That is the reason for the patch array implemented in the base eclass
and why i want it in eapi5.
PATCHES=(
"patch1" # mycomment
"patch2" # another bug number
)
Example from wild:
PATCHES=(
"${FILESDIR}/${PN}-includes-libs-perl.patch"
"${FILESDIR}/${PN}-fix_wrong_linker_opts.patch"
"${FILESDIR}/${PN}-1.0.2-no_harfbuzz_tests.patxh"
)
See the typo on last patch.
>>> Preparing source in /var/tmp/portage/media-gfx/graphite2-1.0.3/work/graphite2-1.0.3 ...
* Applying graphite2-includes-libs-perl.patch ...
[ ok ]
* Applying graphite2-fix_wrong_linker_opts.patch ...
[ ok ]
* QA: File or directory
"/home/scarabeus/gentoo/gentoo-x86/media-gfx/graphite2/files/graphite2-1.0.2-no_harfbuzz_tests.patxh"
does not exist.
* QA: Check your PATCHES array or add missing file/directory.
* ERROR: media-gfx/graphite2-1.0.3 failed (prepare phase):
* Some patches failed. See above messages.
Which is why i really avoid calling epatch myself, the only reason is
to have conditional patches, which are root of all evil, because they
can be broken with update and you won't notice anyway.
Cheers
Tom
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-12 19:57 ` Tomáš Chvátal
@ 2011-10-12 19:59 ` Tomáš Chvátal
2011-10-12 20:15 ` Mike Frysinger
1 sibling, 0 replies; 22+ messages in thread
From: Tomáš Chvátal @ 2011-10-12 19:59 UTC (permalink / raw
To: gentoo-dev
Hmm for the command-not-found, it should be fatal not just warning I suppose.
I was not even aware of this fancy portage feature :)
Tom
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [gentoo-dev] Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-12 19:57 ` Tomáš Chvátal
2011-10-12 19:59 ` Tomáš Chvátal
@ 2011-10-12 20:15 ` Mike Frysinger
1 sibling, 0 replies; 22+ messages in thread
From: Mike Frysinger @ 2011-10-12 20:15 UTC (permalink / raw
To: gentoo-dev
[-- Attachment #1: Type: Text/Plain, Size: 929 bytes --]
On Wednesday 12 October 2011 15:57:45 Tomáš Chvátal wrote:
> 2011/10/12 Mike Frysinger:
> > On Wednesday 12 October 2011 15:44:53 Alec Warner wrote:
> >> If I want to add a patch to the list I might forget to to add the \
> >
> > admittedly, i hit this every once in a while, and with all the "|| die"
> > being implicit, it doesn't get caught right away. fortunately latest
> > portage will issue a QA warning at the end along the lines of "command
> > not found: ${FILESDIR}/${P}-my-new.patch". so one can hope the
> > maintainer tests their ebuilds and reviews QA output before committing
>
> That is the reason for the patch array implemented in the base eclass
> and why i want it in eapi5.
i'm aware of this being in base.eclass, but personally, i've avoided that
thing (and will continue to do so). if it's part of the EAPI, that'd be cool
and i'd probably start using it in my ebuilds.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [gentoo-dev] Re: Re: Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild
2011-10-12 19:19 ` Samuli Suominen
2011-10-12 19:33 ` Mike Frysinger
@ 2011-10-14 4:39 ` Steven J Long
1 sibling, 0 replies; 22+ messages in thread
From: Steven J Long @ 2011-10-14 4:39 UTC (permalink / raw
To: gentoo-dev
Samuli Suominen wrote:
> On 10/12/2011 06:30 AM, Steven J Long wrote:
>> Michał Górny wrote:
>>> I don't think that passing multiple files to epatch actually improves
>>> readability. Simple example:
>>>
>>> # bug #123456, foo, bar
>>> epatch "${FILESDIR}"/${P}-foo.patch
>>> # bug #234567, baz bazinga blah blah
>>> epatch "${FILESDIR}"/${P}-baz.patch
>>>
>>> With multiple arguments, you can't put comments in the middle.
>>>
>> ++ It's also a lot easier to remove the single patches when they're no
>> longer needed.
>
> Removing an 'epatch foo' line is easier than 'foo \' ? You are kidding,
> right?
>
No; in general, most editors make it really simple to delete a line. Not
having to worry about line-continuation is just another routine memo that
doesn't have to be kept in mind*, and I'd argue that it's useful to readers
of the ebuild, to have the bug # in the ebuild.
* All right, not a *lot* easier, just a bit ;-)
>> In the context of configuring, building and installing a
>> package, the extra overhead is miniscule, whereas the above is *much*
>> easier to maintain.
>
> Based on what argument?
Based on it being easier to edit, and easier to look up the bug straight
from the ebuild, should someone working with an ebuild choose (or need) to
do so. I guess I'm arguing for Mike's style within the ebuild (tho that
PATCHES array looks nice and allows bug # comments.)
Again, perhaps '*much*' went a bit far; sorry for that.
> Having the comments inside the patch allows everyone, including
> _upstreams_ straight up see what's it for and link to the bug it's
> coming from. Where as keeping them in ebuilds makes it Gentoo specific,
> which is not what we are about.
Having a bug # in the ebuild doesn't excuse anyone from having correct
comments in the patch; they're orthogonal imo. Personally I think you should
do both; bug # in ebuild* so a user can quickly look up what's happening,
and both full bug URL, and a fuller explanation in the patch. Bug # in
ebuild is also useful when you get an ebuild from someone and no patches in
files/ as it's effectively a fragment. You can argue it's redundant; I see
it as equivalent to a doubly-linked list: you don't _need_ both links to
function, but it makes things easier.
In any event you're the maintainer; but this was a discussion about style,
well "readability" at least, and I think it's better practice to have bug #
in ebuild.
Regards,
igli.
----
* After all, you only add the bug id once, when you're adding the patch and
are well aware of what bug/s you're working on; it's not an ongoing burden.
--
#friendly-coders -- We're friendly, but we're not /that/ friendly ;-)
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2011-10-14 4:36 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20111011164918.2103A2004B@flycatcher.gentoo.org>
2011-10-11 16:59 ` [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in app-admin/chrpath: ChangeLog chrpath-0.13-r2.ebuild Samuli Suominen
2011-10-11 17:05 ` Fabian Groffen
2011-10-11 18:01 ` Samuli Suominen
2011-10-11 18:13 ` Fabian Groffen
2011-10-11 18:34 ` Samuli Suominen
2011-10-11 18:46 ` Fabian Groffen
2011-10-11 19:38 ` Samuli Suominen
2011-10-11 19:49 ` Fabian Groffen
2011-10-11 20:00 ` Samuli Suominen
2011-10-11 20:04 ` Fabian Groffen
2011-10-11 20:13 ` Samuli Suominen
2011-10-11 18:50 ` Matt Turner
2011-10-11 19:48 ` Michał Górny
2011-10-12 3:30 ` [gentoo-dev] " Steven J Long
2011-10-12 19:19 ` Samuli Suominen
2011-10-12 19:33 ` Mike Frysinger
2011-10-12 19:44 ` Alec Warner
2011-10-12 19:51 ` Mike Frysinger
2011-10-12 19:57 ` Tomáš Chvátal
2011-10-12 19:59 ` Tomáš Chvátal
2011-10-12 20:15 ` Mike Frysinger
2011-10-14 4:39 ` [gentoo-dev] " Steven J Long
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox