Re: [gentoo-dev] epatch: reject patches with relative paths
On Friday, December 31, 2010 14:22:31 Enrico Weigelt wrote: > * Mike Frysinger schrieb: > > On Friday, December 31, 2010 09:16:27 Enrico Weigelt wrote: > > > * Mike Frysinger schrieb: > > > > sounds like overkill. people will file bugs and they'll get fixed. > > > > once it goes fatal, people will fix even faster. i dont plan on > > > > making it fatal anytime soon. > > > > > > An option to make it fatal would be helpful. > > > > no, it wouldnt > > Dont you wanna let users decide themselfes whether they want it ? if you want to force such useless behavior, then export EPATCH_OPTS yourself -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] epatch: reject patches with relative paths
> "MF" == Mike Frysinger writes: MF> along those lines, we should start rejecting relative paths. we MF> cant auto- skip the leading elements since relative paths could MF> appear anywhere. You do not need .. or . in a path for it to be a relative path. :; diff -uNr a/foo b/foo generates a patch with relative paths. The warning needs to be more specific lest it cause undue confusion and support load. -JimC -- James Cloos OpenPGP: 1024D/ED7DAEA6
Re: [gentoo-dev] epatch: reject patches with relative paths
* Mike Frysinger schrieb: > On Friday, December 31, 2010 09:16:27 Enrico Weigelt wrote: > > * Mike Frysinger schrieb: > > > sounds like overkill. people will file bugs and they'll get fixed. once > > > it goes fatal, people will fix even faster. i dont plan on making it > > > fatal anytime soon. > > > > An option to make it fatal would be helpful. > > no, it wouldnt Dont you wanna let users decide themselfes whether they want it ? cu -- -- Enrico Weigelt, metux IT service -- http://www.metux.de/ phone: +49 36207 519931 email: weig...@metux.de mobile: +49 151 27565287 icq: 210169427 skype: nekrad666 -- Embedded-Linux / Portierung / Opensource-QM / Verteilte Systeme --
Re: [gentoo-dev] epatch: reject patches with relative paths
On Friday, December 31, 2010 09:16:27 Enrico Weigelt wrote: > * Mike Frysinger schrieb: > > sounds like overkill. people will file bugs and they'll get fixed. once > > it goes fatal, people will fix even faster. i dont plan on making it > > fatal anytime soon. > > An option to make it fatal would be helpful. no, it wouldnt -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] epatch: reject patches with relative paths
* Mike Frysinger schrieb: > sounds like overkill. people will file bugs and they'll get fixed. once it > goes fatal, people will fix even faster. i dont plan on making it fatal > anytime soon. An option to make it fatal would be helpful. cu -- -- Enrico Weigelt, metux IT service -- http://www.metux.de/ phone: +49 36207 519931 email: weig...@metux.de mobile: +49 151 27565287 icq: 210169427 skype: nekrad666 -- Embedded-Linux / Portierung / Opensource-QM / Verteilte Systeme --
Re: [gentoo-dev] epatch: reject patches with relative paths
On Friday, December 31, 2010 02:02:40 Robin H. Johnson wrote: > On Fri, Dec 31, 2010 at 12:17:26AM -0500, Mike Frysinger wrote: > > http://dev.gentoo.org/~vapier/clean-patches > > Nice document. Just two contradictory points that I've noticed been > useful: > 1. Sometimes I've been given patches without information as to which > version they apply to. The timestamp on the --- line was critical to > tracing that, because it matched the source timestamp in the SCM. i wouldnt say it's contradictory ... if my advice was followed, this sleuthing wouldnt have been necessary, and thus the timestamp is still useless. > 2. The .orig suffix on the --- line has been very useful in seeing that > somebody accidentally reversed the patch when generating it. meh. i think this is minor/rare enough to ignore. > One additional request: Using RFC822-style headers for patch text data. i'll add these tips inline with the other stuff -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] epatch: reject patches with relative paths
On Fri, Dec 31, 2010 at 12:17:26AM -0500, Mike Frysinger wrote: > On Thursday, December 30, 2010 21:03:54 Enrico Weigelt wrote: > > * Mike Frysinger schrieb: > > > On Thursday, December 30, 2010 20:05:01 Enrico Weigelt wrote: > > > > IMHO, in longer terms, all patches should normalized, created w/ > > > > diff -ruN and applied w/ -p1. Thats how most people do it, so > > > > a kind of semi-standard. > > > > > > not worth developer's time to force it since it poses no practical > > > positive benefit to us > > > > It makes it easier for everyone who'll want to work on these > > patches (eg. people besides the actual ebuild maintainers). > > > > BTW: I'm not proposing to rework all the patches right now, > > just set a policy for new ones. > > suggestions are fine, but these arent a requirement we're going to force on > developers. i already put together a list of suggestions for people long ago: > http://dev.gentoo.org/~vapier/clean-patches Nice document. Just two contradictory points that I've noticed been useful: 1. Sometimes I've been given patches without information as to which version they apply to. The timestamp on the --- line was critical to tracing that, because it matched the source timestamp in the SCM. 2. The .orig suffix on the --- line has been very useful in seeing that somebody accidentally reversed the patch when generating it. One additional request: Using RFC822-style headers for patch text data. Nice example in many of our MySQL patches, like this one: Gentoo-Bug: 238487 Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=238487 MySQL-Bug-URL: http://bugs.mysql.com/bug.php?id=39288 MySQL-Bug: 39288 MySQL-Lists-URL: http://lists.mysql.com/internals/35947 X-Patch-URL: http://bugs.gentoo.org/attachment.cgi?id=188019&action=view Signed-off-by: Jorge Manuel B. S. Vicetto Signed-off-by: Robin H. Johnson Signed-off-by: Kristian Nielsen The above taken from: http://git.overlays.gentoo.org/gitweb/?p=proj/mysql-extras.git;a=blob_plain;f=02040_all_embedded-library-shared-maria-5.1.50.patch;hb=499f0f3c94f6d4dc1c46adafc622e9583ee6a315 -- Robin Hugh Johnson Gentoo Linux: Developer, Trustee & Infrastructure Lead E-Mail : robb...@gentoo.org GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
Re: [gentoo-dev] epatch: reject patches with relative paths
On Thursday, December 30, 2010 21:01:49 Markos Chandras wrote: > On Thu, Dec 30, 2010 at 08:28:42PM -0500, Mike Frysinger wrote: > > + eqawarn "QA Notice: Your patch has relative paths." > > + eqawarn " In the future this will cause a failure." > > Maybe we should open a tracker to identify which packages use relative > paths in their patches before making this control check fatal. sounds like overkill. people will file bugs and they'll get fixed. once it goes fatal, people will fix even faster. i dont plan on making it fatal anytime soon. a simple grep of in tree patches shows only a handful of hits. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] epatch: reject patches with relative paths
On Thursday, December 30, 2010 21:03:54 Enrico Weigelt wrote: > * Mike Frysinger schrieb: > > On Thursday, December 30, 2010 20:05:01 Enrico Weigelt wrote: > > > IMHO, in longer terms, all patches should normalized, created w/ > > > diff -ruN and applied w/ -p1. Thats how most people do it, so > > > a kind of semi-standard. > > > > not worth developer's time to force it since it poses no practical > > positive benefit to us > > It makes it easier for everyone who'll want to work on these > patches (eg. people besides the actual ebuild maintainers). > > BTW: I'm not proposing to rework all the patches right now, > just set a policy for new ones. suggestions are fine, but these arent a requirement we're going to force on developers. i already put together a list of suggestions for people long ago: http://dev.gentoo.org/~vapier/clean-patches > Even you might not like to hear this, Debian is much better at this > point i could care less > they a patchqueue per each package, which can be applied > fully automatically (w/o additional code in the invididual package > descriptors). it'd be trivial to do the same thing in Gentoo, but it doesnt make sense. Debian doesnt maintain a unified package tree of multiple versions. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] epatch: reject patches with relative paths
* Mike Frysinger schrieb: > On Thursday, December 30, 2010 20:05:01 Enrico Weigelt wrote: > > IMHO, in longer terms, all patches should normalized, created w/ > > diff -ruN and applied w/ -p1. Thats how most people do it, so > > a kind of semi-standard. > > not worth developer's time to force it since it poses no practical positive > benefit to us It makes it easier for everyone who'll want to work on these patches (eg. people besides the actual ebuild maintainers). BTW: I'm not proposing to rework all the patches right now, just set a policy for new ones. Even you might not like to hear this, Debian is much better at this point - they a patchqueue per each package, which can be applied fully automatically (w/o additional code in the invididual package descriptors). This allows easy importing into other systems (like I'm doing w/ my normalized git repositories within the oss-qm project). cu -- -- Enrico Weigelt, metux IT service -- http://www.metux.de/ phone: +49 36207 519931 email: weig...@metux.de mobile: +49 151 27565287 icq: 210169427 skype: nekrad666 -- Embedded-Linux / Portierung / Opensource-QM / Verteilte Systeme --
Re: [gentoo-dev] epatch: reject patches with relative paths
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 12/30/2010 07:28 PM, Mike Frysinger wrote:
> On Thursday, December 30, 2010 19:42:35 Robin H. Johnson wrote:
>> On Thu, Dec 30, 2010 at 07:04:25PM -0500, Mike Frysinger wrote:
>>> epatch was changed to auto-skip the first path element when it is
>>> absolute (starts with a slash). the reason was to avoid issues with
>>> patches touching files outside of $PWD (which is bad if sandbox is
>>> disabled).
>>
>> +1 from me, but can we have a QA prefix on the ewarn output?
>
> --- eutils.eclass 22 Nov 2010 00:31:03 - 1.352
> +++ eutils.eclass 31 Dec 2010 01:28:37 -
> @@ -360,6 +360,13 @@ epatch() {
> count=1
> printf "NOTE: skipping -p0 due to absolute paths in
> patch:\n%s\n" "${abs_paths}" >> "${STDERR_TARGET}"
> fi
> + # Similar reason, but with relative paths.
> + local rel_paths=$(egrep -n '^[-+]{3} [^ ]*[.][.]/'
> "${PATCH_TARGET}")
> + if [[ -n ${rel_paths} ]] ; then
> + eqawarn "QA Notice: Your patch has relative paths."
> + eqawarn " In the future this will cause a failure."
> + eqawarn "${rel_paths}"
> + fi
>
> # Dynamically detect the correct -p# ... i'm lazy, so shoot me
> :/
> while [[ ${count} -lt 5 ]] ; do
> -mike
+1 from me!!!
- --
==
Jory A. Pratt anarchy -at- gentoo.org
Gentoo Mozilla Lead
GPG: 2C1D 6AF9 F35D 5122 0E8F 9123 C270 3B43 5674 6127
==
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.16 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk0dOmEACgkQwnA7Q1Z0YSfEHwCgrI5PjbcIPBsCh2yzcTPa1gxf
+1AAn2w97jB4wYKo/k69jS6wj5wPfcPW
=tJ2w
-END PGP SIGNATURE-
Re: [gentoo-dev] epatch: reject patches with relative paths
On Thu, Dec 30, 2010 at 08:28:42PM -0500, Mike Frysinger wrote:
> On Thursday, December 30, 2010 19:42:35 Robin H. Johnson wrote:
> > On Thu, Dec 30, 2010 at 07:04:25PM -0500, Mike Frysinger wrote:
> > > epatch was changed to auto-skip the first path element when it is
> > > absolute (starts with a slash). the reason was to avoid issues with
> > > patches touching files outside of $PWD (which is bad if sandbox is
> > > disabled).
> >
> > +1 from me, but can we have a QA prefix on the ewarn output?
>
> --- eutils.eclass 22 Nov 2010 00:31:03 - 1.352
> +++ eutils.eclass 31 Dec 2010 01:28:37 -
> @@ -360,6 +360,13 @@ epatch() {
> count=1
> printf "NOTE: skipping -p0 due to absolute paths in
> patch:\n%s\n" "${abs_paths}" >> "${STDERR_TARGET}"
> fi
> + # Similar reason, but with relative paths.
> + local rel_paths=$(egrep -n '^[-+]{3} [^ ]*[.][.]/'
> "${PATCH_TARGET}")
> + if [[ -n ${rel_paths} ]] ; then
> + eqawarn "QA Notice: Your patch has relative paths."
> + eqawarn " In the future this will cause a failure."
> + eqawarn "${rel_paths}"
> + fi
>
> # Dynamically detect the correct -p# ... i'm lazy, so shoot me
> :/
> while [[ ${count} -lt 5 ]] ; do
> -mike
Mike,
Maybe we should open a tracker to identify which packages use relative paths
in their patches before making this control check fatal.
Regards,
--
Markos Chandras (hwoarang)
Gentoo Linux Developer
Web: http://hwoarang.silverarrow.org
Key ID: 441AC410
Key FP: AAD0 8591 E3CD 445D 6411 3477 F7F7 1E8E 441A C410
pgphxSM73NFN6.pgp
Description: PGP signature
Re: [gentoo-dev] epatch: reject patches with relative paths
On Thursday, December 30, 2010 19:42:35 Robin H. Johnson wrote:
> On Thu, Dec 30, 2010 at 07:04:25PM -0500, Mike Frysinger wrote:
> > epatch was changed to auto-skip the first path element when it is
> > absolute (starts with a slash). the reason was to avoid issues with
> > patches touching files outside of $PWD (which is bad if sandbox is
> > disabled).
>
> +1 from me, but can we have a QA prefix on the ewarn output?
--- eutils.eclass 22 Nov 2010 00:31:03 - 1.352
+++ eutils.eclass 31 Dec 2010 01:28:37 -
@@ -360,6 +360,13 @@ epatch() {
count=1
printf "NOTE: skipping -p0 due to absolute paths in
patch:\n%s\n" "${abs_paths}" >> "${STDERR_TARGET}"
fi
+ # Similar reason, but with relative paths.
+ local rel_paths=$(egrep -n '^[-+]{3} [^ ]*[.][.]/'
"${PATCH_TARGET}")
+ if [[ -n ${rel_paths} ]] ; then
+ eqawarn "QA Notice: Your patch has relative paths."
+ eqawarn " In the future this will cause a failure."
+ eqawarn "${rel_paths}"
+ fi
# Dynamically detect the correct -p# ... i'm lazy, so shoot me
:/
while [[ ${count} -lt 5 ]] ; do
-mike
signature.asc
Description: This is a digitally signed message part.
Re: [gentoo-dev] epatch: reject patches with relative paths
On Thursday, December 30, 2010 20:05:01 Enrico Weigelt wrote: > IMHO, in longer terms, all patches should normalized, created w/ > diff -ruN and applied w/ -p1. Thats how most people do it, so > a kind of semi-standard. not worth developer's time to force it since it poses no practical positive benefit to us -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] epatch: reject patches with relative paths
* Robin H. Johnson schrieb: > On Thu, Dec 30, 2010 at 07:04:25PM -0500, Mike Frysinger wrote: > > epatch was changed to auto-skip the first path element when it is absolute > > (starts with a slash). the reason was to avoid issues with patches touching > > files outside of $PWD (which is bad if sandbox is disabled). > +1 from me, but can we have a QA prefix on the ewarn output? IMHO, in longer terms, all patches should normalized, created w/ diff -ruN and applied w/ -p1. Thats how most people do it, so a kind of semi-standard. cu -- -- Enrico Weigelt, metux IT service -- http://www.metux.de/ phone: +49 36207 519931 email: weig...@metux.de mobile: +49 151 27565287 icq: 210169427 skype: nekrad666 -- Embedded-Linux / Portierung / Opensource-QM / Verteilte Systeme --
Re: [gentoo-dev] epatch: reject patches with relative paths
On Thu, Dec 30, 2010 at 07:04:25PM -0500, Mike Frysinger wrote: > epatch was changed to auto-skip the first path element when it is absolute > (starts with a slash). the reason was to avoid issues with patches touching > files outside of $PWD (which is bad if sandbox is disabled). +1 from me, but can we have a QA prefix on the ewarn output? -- Robin Hugh Johnson Gentoo Linux: Developer, Trustee & Infrastructure Lead E-Mail : robb...@gentoo.org GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85 pgp7M2ptV92wv.pgp Description: PGP signature
[gentoo-dev] epatch: reject patches with relative paths
epatch was changed to auto-skip the first path element when it is absolute
(starts with a slash). the reason was to avoid issues with patches touching
files outside of $PWD (which is bad if sandbox is disabled).
along those lines, we should start rejecting relative paths. we cant auto-
skip the leading elements since relative paths could appear anywhere.
rather than making it fatal right away, this patch adds some ewarns. after a
while, we can convert it to a die.
-mike
--- eutils.eclass 22 Nov 2010 00:31:03 - 1.352
+++ eutils.eclass 30 Dec 2010 23:52:41 -
@@ -360,6 +360,12 @@ epatch() {
count=1
printf "NOTE: skipping -p0 due to absolute paths in
patch:\n%s\n" "${abs_paths}" >> "${STDERR_TARGET}"
fi
+ # Similar reason, but with relative paths.
+ local rel_paths=$(egrep -n '^[-+]{3} [^\t]*[.][.]/'
"${PATCH_TARGET}")
+ if [[ -n ${rel_paths} ]] ; then
+ ewarn "Your patch has relative paths; in the future
this will fail:"
+ ewarn "${rel_paths}"
+ fi
# Dynamically detect the correct -p# ... i'm lazy, so shoot me
:/
while [[ ${count} -lt 5 ]] ; do
signature.asc
Description: This is a digitally signed message part.
