[PATCH] dpkg-native: Avoid 'file changed' errors from tar

[PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Richard Purdie]

See the patch header for details. Hardlink count duing do_package_write_deb
can change causing dpkg-deb failures. We don't care about this error
case so avoid it by checking the tar exit code.

[YOCTO #7529]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

diff --git a/meta/recipes-devtools/dpkg/dpkg.inc b/meta/recipes-devtools/dpkg/dpkg.inc
index bbbd881..5db1fde 100644
--- a/meta/recipes-devtools/dpkg/dpkg.inc
+++ b/meta/recipes-devtools/dpkg/dpkg.inc
@@ -4,6 +4,8 @@ SECTION = "base"
 
 SRC_URI = "${DEBIAN_MIRROR}/main/d/dpkg/dpkg_${PV}.tar.xz"
 
+SRC_URI_append_class-native = " file://tar-error-code.patch"
+
 DEPENDS = "zlib bzip2 perl ncurses"
 DEPENDS_class-native = "bzip2-replacement-native zlib-native virtual/update-alternatives-native gettext-native perl-native"
 RDEPENDS_${PN} = "${VIRTUAL-RUNTIME_update-alternatives} xz run-postinsts perl"
diff --git a/meta/recipes-devtools/dpkg/dpkg/tar-error-code.patch b/meta/recipes-devtools/dpkg/dpkg/tar-error-code.patch
new file mode 100644
index 0000000..f49e0e9
--- /dev/null
+++ b/meta/recipes-devtools/dpkg/dpkg/tar-error-code.patch
@@ -0,0 +1,54 @@
+When running do_package_write_deb, we have trees of symlinked files
+such as the dbg source files in ${PN}-dbg. If something makes another
+copy of one of those files (or deletes one), the number of links a file
+has changes and tar can notice this, e.g.:
+
+| DEBUG: Executing python function do_package_deb
+| dpkg-deb: building package `sed-ptest' in `/media/build1/poky/build/tmp/work/i586-poky-linux/sed/4.2.2-r0/deploy-debs/i586/sed-ptest_4.2.2-r0.3_i386.deb'.
+| tar: ./usr/lib/sed/ptest/testsuite/tst-regex2: file changed as we read it
+| dpkg-deb: error: subprocess tar -cf returned error exit status 1
+
+Tar returns an error of 1 when files 'change' and other errors codes
+in other error cases. We tweak dpkg-deb here so that it ignores an exit
+code of 1 from tar. The files don't really change (and we have locking in
+place to avoid that kind of issue).
+
+Upsteam-Status: Inappropriate
+RP 2015/3/27
+
+Index: dpkg-1.17.21/dpkg-deb/build.c
+===================================================================
+--- dpkg-1.17.21.orig/dpkg-deb/build.c
++++ dpkg-1.17.21/dpkg-deb/build.c
+@@ -398,7 +398,7 @@ do_build(const char *const *argv)
+   bool subdir;
+   char *tfbuf;
+   int arfd;
+-  int p1[2], p2[2], gzfd;
++  int p1[2], p2[2], gzfd, rc;
+   pid_t c1, c2;
+ 
+   /* Decode our arguments. */
+@@ -493,7 +493,9 @@ do_build(const char *const *argv)
+   }
+   close(p1[0]);
+   subproc_reap(c2, "gzip -9c", 0);
+-  subproc_reap(c1, "tar -cf", 0);
++  rc = subproc_reap(c1, "tar -cf", SUBPROC_RETERROR);
++  if (rc && rc != 1)
++    ohshite(_("subprocess %s returned error exit status %d"), "tar -cf", rc);
+ 
+   if (lseek(gzfd, 0, SEEK_SET))
+     ohshite(_("failed to rewind temporary file (%s)"), _("control member"));
+@@ -581,7 +583,10 @@ do_build(const char *const *argv)
+   /* All done, clean up wait for tar and gzip to finish their job. */
+   close(p1[1]);
+   subproc_reap(c2, _("<compress> from tar -cf"), 0);
+-  subproc_reap(c1, "tar -cf", 0);
++  rc = subproc_reap(c1, "tar -cf", SUBPROC_RETERROR);
++  if (rc && rc != 1)
++    ohshite(_("subprocess %s returned error exit status %d"), "tar -cf", rc);
++
+   /* Okay, we have data.tar as well now, add it to the ar wrapper. */
+   if (deb_format.major == 2) {
+     char datamember[16 + 1];

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Otavio Salvador]

On Sat, Mar 28, 2015 at 5:50 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> See the patch header for details. Hardlink count duing do_package_write_deb
> can change causing dpkg-deb failures. We don't care about this error
> case so avoid it by checking the tar exit code.
>
> [YOCTO #7529]
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

Please don't ask someone to see the patch header for information. This
should be in the commit log and not necessarily read the diff to find
this information.

Please send a v2 adding this information in the commit log.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Richard Purdie]

On Mon, 2015-03-30 at 10:52 -0300, Otavio Salvador wrote:
> On Sat, Mar 28, 2015 at 5:50 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > See the patch header for details. Hardlink count duing do_package_write_deb
> > can change causing dpkg-deb failures. We don't care about this error
> > case so avoid it by checking the tar exit code.
> >
> > [YOCTO #7529]
> >
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> 
> Please don't ask someone to see the patch header for information. This
> should be in the commit log and not necessarily read the diff to find
> this information.
> 
> Please send a v2 adding this information in the commit log.

Just referring someone to the patch header would be incorrect and I
agree a summary of the problem is needed. The commit message in question
does have this. I don't see the point of cut and pasting all the details
from the header into the commit message. The correct place for the
details in a situation like this is the patch header so that someone has
full details of the patch without having to look at the repo history.

Cheers,

Richard

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Otavio Salvador]

On Mon, Mar 30, 2015 at 11:58 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Mon, 2015-03-30 at 10:52 -0300, Otavio Salvador wrote:
>> On Sat, Mar 28, 2015 at 5:50 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > See the patch header for details. Hardlink count duing do_package_write_deb
>> > can change causing dpkg-deb failures. We don't care about this error
>> > case so avoid it by checking the tar exit code.
>> >
>> > [YOCTO #7529]
>> >
>> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>>
>> Please don't ask someone to see the patch header for information. This
>> should be in the commit log and not necessarily read the diff to find
>> this information.
>>
>> Please send a v2 adding this information in the commit log.
>
> Just referring someone to the patch header would be incorrect and I
> agree a summary of the problem is needed. The commit message in question
> does have this. I don't see the point of cut and pasting all the details
> from the header into the commit message. The correct place for the
> details in a situation like this is the patch header so that someone has
> full details of the patch without having to look at the repo history.

+When running do_package_write_deb, we have trees of symlinked files
+such as the dbg source files in ${PN}-dbg. If something makes another
+copy of one of those files (or deletes one), the number of links a file
+has changes and tar can notice this, e.g.:
+
+| DEBUG: Executing python function do_package_deb
+| dpkg-deb: building package `sed-ptest' in
`/media/build1/poky/build/tmp/work/i586-poky-linux/sed/4.2.2-r0/deploy-debs/i586/sed-ptest_4.2.2-r0.3_i386.deb'.
+| tar: ./usr/lib/sed/ptest/testsuite/tst-regex2: file changed as we read it
+| dpkg-deb: error: subprocess tar -cf returned error exit status 1
+
+Tar returns an error of 1 when files 'change' and other errors codes
+in other error cases. We tweak dpkg-deb here so that it ignores an exit
+code of 1 from tar. The files don't really change (and we have locking in
+place to avoid that kind of issue).

This is exactly what I would expect for the long commit log. Sorry but
I strongly disagrese with you as the commit log in the patch does
connect with the short log so I still believe the commit log could be
improved.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Richard Purdie]

On Mon, 2015-03-30 at 12:02 -0300, Otavio Salvador wrote:
> +When running do_package_write_deb, we have trees of symlinked files
> +such as the dbg source files in ${PN}-dbg. If something makes another
> +copy of one of those files (or deletes one), the number of links a file
> +has changes and tar can notice this, e.g.:
> +
> +| DEBUG: Executing python function do_package_deb
> +| dpkg-deb: building package `sed-ptest' in
> `/media/build1/poky/build/tmp/work/i586-poky-linux/sed/4.2.2-r0/deploy-debs/i586/sed-ptest_4.2.2-r0.3_i386.deb'.
> +| tar: ./usr/lib/sed/ptest/testsuite/tst-regex2: file changed as we read it
> +| dpkg-deb: error: subprocess tar -cf returned error exit status 1
> +
> +Tar returns an error of 1 when files 'change' and other errors codes
> +in other error cases. We tweak dpkg-deb here so that it ignores an exit
> +code of 1 from tar. The files don't really change (and we have locking in
> +place to avoid that kind of issue).
> 
> This is exactly what I would expect for the long commit log. Sorry but
> I strongly disagrese with you as the commit log in the patch does
> connect with the short log so I still believe the commit log could be
> improved.

Well, I strong disagree with that, I don't believe it makes sense to cut
and paste the same information into both places, sorry.

Cheers,

Richard

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Otavio Salvador]

On Mon, Mar 30, 2015 at 1:02 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Mon, 2015-03-30 at 12:02 -0300, Otavio Salvador wrote:
>> +When running do_package_write_deb, we have trees of symlinked files
>> +such as the dbg source files in ${PN}-dbg. If something makes another
>> +copy of one of those files (or deletes one), the number of links a file
>> +has changes and tar can notice this, e.g.:
>> +
>> +| DEBUG: Executing python function do_package_deb
>> +| dpkg-deb: building package `sed-ptest' in
>> `/media/build1/poky/build/tmp/work/i586-poky-linux/sed/4.2.2-r0/deploy-debs/i586/sed-ptest_4.2.2-r0.3_i386.deb'.
>> +| tar: ./usr/lib/sed/ptest/testsuite/tst-regex2: file changed as we read it
>> +| dpkg-deb: error: subprocess tar -cf returned error exit status 1
>> +
>> +Tar returns an error of 1 when files 'change' and other errors codes
>> +in other error cases. We tweak dpkg-deb here so that it ignores an exit
>> +code of 1 from tar. The files don't really change (and we have locking in
>> +place to avoid that kind of issue).
>>
>> This is exactly what I would expect for the long commit log. Sorry but
>> I strongly disagrese with you as the commit log in the patch does
>> connect with the short log so I still believe the commit log could be
>> improved.
>
> Well, I strong disagree with that, I don't believe it makes sense to cut
> and paste the same information into both places, sorry.

I disagree someone should go into a patch header to understand it.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Mark Hatle]

On 3/30/15 11:21 AM, Otavio Salvador wrote:
> On Mon, Mar 30, 2015 at 1:02 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Mon, 2015-03-30 at 12:02 -0300, Otavio Salvador wrote:
>>> +When running do_package_write_deb, we have trees of symlinked files
>>> +such as the dbg source files in ${PN}-dbg. If something makes another
>>> +copy of one of those files (or deletes one), the number of links a file
>>> +has changes and tar can notice this, e.g.:
>>> +
>>> +| DEBUG: Executing python function do_package_deb
>>> +| dpkg-deb: building package `sed-ptest' in
>>> `/media/build1/poky/build/tmp/work/i586-poky-linux/sed/4.2.2-r0/deploy-debs/i586/sed-ptest_4.2.2-r0.3_i386.deb'.
>>> +| tar: ./usr/lib/sed/ptest/testsuite/tst-regex2: file changed as we read it
>>> +| dpkg-deb: error: subprocess tar -cf returned error exit status 1
>>> +
>>> +Tar returns an error of 1 when files 'change' and other errors codes
>>> +in other error cases. We tweak dpkg-deb here so that it ignores an exit
>>> +code of 1 from tar. The files don't really change (and we have locking in
>>> +place to avoid that kind of issue).
>>>
>>> This is exactly what I would expect for the long commit log. Sorry but
>>> I strongly disagrese with you as the commit log in the patch does
>>> connect with the short log so I still believe the commit log could be
>>> improved.
>>
>> Well, I strong disagree with that, I don't believe it makes sense to cut
>> and paste the same information into both places, sorry.
> 
> I disagree someone should go into a patch header to understand it.
> 

I agree with Otavio here.  When generating both short and long changelogs, I
always use the commit messages.  I don't inspect the contents of the changes to
understand what happened.  Having to inspect means that I can't automate the
process using the git commands.

--Mark

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Mark Hatle]

On 3/30/15 11:37 AM, Mark Hatle wrote:
> On 3/30/15 11:21 AM, Otavio Salvador wrote:
>> On Mon, Mar 30, 2015 at 1:02 PM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>>> On Mon, 2015-03-30 at 12:02 -0300, Otavio Salvador wrote:
>>>> +When running do_package_write_deb, we have trees of symlinked files
>>>> +such as the dbg source files in ${PN}-dbg. If something makes another
>>>> +copy of one of those files (or deletes one), the number of links a file
>>>> +has changes and tar can notice this, e.g.:
>>>> +
>>>> +| DEBUG: Executing python function do_package_deb
>>>> +| dpkg-deb: building package `sed-ptest' in
>>>> `/media/build1/poky/build/tmp/work/i586-poky-linux/sed/4.2.2-r0/deploy-debs/i586/sed-ptest_4.2.2-r0.3_i386.deb'.
>>>> +| tar: ./usr/lib/sed/ptest/testsuite/tst-regex2: file changed as we read it
>>>> +| dpkg-deb: error: subprocess tar -cf returned error exit status 1
>>>> +
>>>> +Tar returns an error of 1 when files 'change' and other errors codes
>>>> +in other error cases. We tweak dpkg-deb here so that it ignores an exit
>>>> +code of 1 from tar. The files don't really change (and we have locking in
>>>> +place to avoid that kind of issue).
>>>>
>>>> This is exactly what I would expect for the long commit log. Sorry but
>>>> I strongly disagrese with you as the commit log in the patch does
>>>> connect with the short log so I still believe the commit log could be
>>>> improved.
>>>
>>> Well, I strong disagree with that, I don't believe it makes sense to cut
>>> and paste the same information into both places, sorry.
>>
>> I disagree someone should go into a patch header to understand it.
>>
> 
> I agree with Otavio here.  When generating both short and long changelogs, I
> always use the commit messages.  I don't inspect the contents of the changes to
> understand what happened.  Having to inspect means that I can't automate the
> process using the git commands.

Someone pointed out I might not have been as clear as I intended here.  I was
not saying I thought the commit and patch log need to match.  What I meant was
that I don't like seeing "See the patch header for details." in the commit
message, because it causes me problems with git generated changelogs.  (I have
developers/customers who then come back to me asking why I didn't include the
contents of the patch header in my generated changelogs.)

As for the content, the easiest is likely to duplicate the patch header into the
commit message... but of course that isn't required.

--Mark

> --Mark
> 

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Andreas Oberritter]

On 30.03.2015 18:21, Otavio Salvador wrote:
> On Mon, Mar 30, 2015 at 1:02 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Mon, 2015-03-30 at 12:02 -0300, Otavio Salvador wrote:
>>> +When running do_package_write_deb, we have trees of symlinked files

Richard, did you intend to write "hardlinked" here?

>>> +such as the dbg source files in ${PN}-dbg. If something makes another
>>> +copy of one of those files (or deletes one), the number of links a file
>>> +has changes and tar can notice this, e.g.:
>>> +
>>> +| DEBUG: Executing python function do_package_deb
>>> +| dpkg-deb: building package `sed-ptest' in
>>> `/media/build1/poky/build/tmp/work/i586-poky-linux/sed/4.2.2-r0/deploy-debs/i586/sed-ptest_4.2.2-r0.3_i386.deb'.
>>> +| tar: ./usr/lib/sed/ptest/testsuite/tst-regex2: file changed as we read it
>>> +| dpkg-deb: error: subprocess tar -cf returned error exit status 1
>>> +
>>> +Tar returns an error of 1 when files 'change' and other errors codes
>>> +in other error cases. We tweak dpkg-deb here so that it ignores an exit
>>> +code of 1 from tar. The files don't really change (and we have locking in
>>> +place to avoid that kind of issue).
>>>
>>> This is exactly what I would expect for the long commit log. Sorry but
>>> I strongly disagrese with you as the commit log in the patch does
>>> connect with the short log so I still believe the commit log could be
>>> improved.
>>
>> Well, I strong disagree with that, I don't believe it makes sense to cut
>> and paste the same information into both places, sorry.
> 
> I disagree someone should go into a patch header to understand it.
> 

It's my impression that if Richard had omitted the first sentence of his
commit message, the message would still have been descriptive enough,
and nobody would ever have complained.

Regards,
Andreas

Re: [PATCH] dpkg-native: Avoid 'file changed' errors from tar

[Richard Purdie]

On Mon, 2015-03-30 at 20:07 +0200, Andreas Oberritter wrote:
> On 30.03.2015 18:21, Otavio Salvador wrote:
> > On Mon, Mar 30, 2015 at 1:02 PM, Richard Purdie
> > <richard.purdie@linuxfoundation.org> wrote:
> >> On Mon, 2015-03-30 at 12:02 -0300, Otavio Salvador wrote:
> >>> +When running do_package_write_deb, we have trees of symlinked files
> 
> Richard, did you intend to write "hardlinked" here?

I did, yes. I'll correct that, thanks.

> It's my impression that if Richard had omitted the first sentence of his
> commit message, the message would still have been descriptive enough,
> and nobody would ever have complained.

That is the impression I got too. Its odd that a comment intended to
help the user find more information should generate this discussion,
particularly as the details in the patch header are more implementation
oriented and make sense when you're looking at the dpkg code (which you
can in the patch).

So I may simply remove the comment that is causing contention.

Cheers,

Richard