| Message ID | 20180806031715.7088-1-ricardo.martincoski@gmail.com |
|---|---|
| State | Accepted |
| Commit | 2d8d5ced107ab2a05d023873de4e8f12f081e5af |
| Headers | show |
| Series | [2018.02.x,1/2] makedevs: allow leading whitespace for capabilities | expand |
Hello Ricardo, On Mon, 6 Aug 2018 00:17:14 -0300, Ricardo Martincoski wrote: > Currently makedevs silently ignores extended attributes with leading > whitespace, for example those added to a <PACKAGE>_PERMISSIONS following > the recommended style from check-package. > > Makedevs already ignores leading whitespace for normal entries (file > permission changes and device files creation). Do the same for extended > attributes. > > Fixes: #11191. > > Reported-by: Jean-pierre Cartal <jpcartal@free.fr> > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com> > Cc: Arnout Vandecappelle <arnout@mind.be> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> It is not clear to me why you have the "2018.02.x" prefix in your patch title. I would assume this patch is needed in master, not just in 2018.02.x. We generally include 2018.02.x in the patch title when the patch should only be applied to 2018.02.x. For patches going in master, and needing a backport, we normally rely on Peter being smart and realizing that the patch is a fix that makes sense for 2018.02.x. Could you clarify what was your intention with this 2018.02.x prefix ? Perhaps we need to have some tags ? Backport-to: 2018.02.x when the patch is for master, but needs to be backported. And perhaps: Apply-to: 2018.02.x when the patch is to be applied only on 2018.02.x. Or simply rely on informal notes below the "---" sign ? Best regards, Thomas
Hello, On Thu, Aug 09, 2018 at 06:42 PM, Thomas Petazzoni wrote: > On Mon, 6 Aug 2018 00:17:14 -0300, Ricardo Martincoski wrote: >> Currently makedevs silently ignores extended attributes with leading >> whitespace, for example those added to a <PACKAGE>_PERMISSIONS following >> the recommended style from check-package. >> >> Makedevs already ignores leading whitespace for normal entries (file >> permission changes and device files creation). Do the same for extended >> attributes. >> >> Fixes: #11191. >> >> Reported-by: Jean-pierre Cartal <jpcartal@free.fr> >> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com> >> Cc: Arnout Vandecappelle <arnout@mind.be> >> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > > It is not clear to me why you have the "2018.02.x" prefix in your patch > title. I would assume this patch is needed in master, not just in > 2018.02.x. We generally include 2018.02.x in the patch title when > the patch should only be applied to 2018.02.x. For patches going in > master, and needing a backport, we normally rely on Peter being smart > and realizing that the patch is a fix that makes sense for 2018.02.x. > > Could you clarify what was your intention with this 2018.02.x prefix ? I developed and runtime tested the patch for 2018.02.x. But now I tested the patch (by temporarily adding a printf to makedevs) in the master branch too. It works well. This patch can/should be applied to master. In the branch 2018.02.x this patch unbreaks xattr with leading whitespace. In the master branch (and also 2018.05.x), which use a per-fs target dir, there are other issues with xattr, this patch is only part of the solution to unbreak xattrs, see #11216. > > Perhaps we need to have some tags ? > > Backport-to: 2018.02.x > > when the patch is for master, but needs to be backported. And perhaps: > > Apply-to: 2018.02.x > > when the patch is to be applied only on 2018.02.x. > > Or simply rely on informal notes below the "---" sign ? Sorry, I should have added such informal notes. Regards, Ricardo
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes: Hi, > It is not clear to me why you have the "2018.02.x" prefix in your patch > title. I would assume this patch is needed in master, not just in > 2018.02.x. We generally include 2018.02.x in the patch title when > the patch should only be applied to 2018.02.x. For patches going in > master, and needing a backport, we normally rely on Peter being smart > and realizing that the patch is a fix that makes sense for 2018.02.x. > Could you clarify what was your intention with this 2018.02.x prefix ? > Perhaps we need to have some tags ? > Backport-to: 2018.02.x > when the patch is for master, but needs to be backported. And perhaps: > Apply-to: 2018.02.x > when the patch is to be applied only on 2018.02.x. > Or simply rely on informal notes below the "---" sign ? Just stating it below the --- sign is IMHO good enough, and in the worst case send a mail if I miss something.
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes: > Hello, > On Thu, Aug 09, 2018 at 06:42 PM, Thomas Petazzoni wrote: >> On Mon, 6 Aug 2018 00:17:14 -0300, Ricardo Martincoski wrote: >>> Currently makedevs silently ignores extended attributes with leading >>> whitespace, for example those added to a <PACKAGE>_PERMISSIONS following >>> the recommended style from check-package. >>> >>> Makedevs already ignores leading whitespace for normal entries (file >>> permission changes and device files creation). Do the same for extended >>> attributes. >>> >>> Fixes: #11191. >>> >>> Reported-by: Jean-pierre Cartal <jpcartal@free.fr> >>> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com> >>> Cc: Arnout Vandecappelle <arnout@mind.be> >>> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> >> >> It is not clear to me why you have the "2018.02.x" prefix in your patch >> title. I would assume this patch is needed in master, not just in >> 2018.02.x. We generally include 2018.02.x in the patch title when >> the patch should only be applied to 2018.02.x. For patches going in >> master, and needing a backport, we normally rely on Peter being smart >> and realizing that the patch is a fix that makes sense for 2018.02.x. >> >> Could you clarify what was your intention with this 2018.02.x prefix ? > I developed and runtime tested the patch for 2018.02.x. > But now I tested the patch (by temporarily adding a printf to makedevs) in the > master branch too. It works well. > This patch can/should be applied to master. > In the branch 2018.02.x this patch unbreaks xattr with leading whitespace. > In the master branch (and also 2018.05.x), which use a per-fs target dir, there > are other issues with xattr, this patch is only part of the solution to unbreak > xattrs, see #11216. Ok, thanks. Committed to master, and then backported to 2018.02.x and 2018.05.x, thanks.
diff --git a/package/makedevs/makedevs.c b/package/makedevs/makedevs.c index 1ba5936342..c57b964f5c 100644 --- a/package/makedevs/makedevs.c +++ b/package/makedevs/makedevs.c @@ -510,7 +510,7 @@ int main(int argc, char **argv) linenum++; - if (1 == sscanf(line, "|xattr %254s", xattr)) { + if (1 == sscanf(line, " |xattr %254s", xattr)) { #ifdef EXTENDED_ATTRIBUTES if (!full_name) bb_error_msg_and_die("line %d should be after a file\n", linenum);
Currently makedevs silently ignores extended attributes with leading whitespace, for example those added to a <PACKAGE>_PERMISSIONS following the recommended style from check-package. Makedevs already ignores leading whitespace for normal entries (file permission changes and device files creation). Do the same for extended attributes. Fixes: #11191. Reported-by: Jean-pierre Cartal <jpcartal@free.fr> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com> Cc: Arnout Vandecappelle <arnout@mind.be> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> --- package/makedevs/makedevs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)