Debian Bug report logs - #868473
dh-acc: Incorrect usage of doit (Dh_Lib)

Reply or subscribe to this bug.

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Niels Thykier <niels@thykier.net>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: dh-acc: Incorrect usage of doit (Dh_Lib)

Date: Sat, 15 Jul 2017 21:55:48 +0200

Package: dh-acc
Version: 1.99.22-1
Severity: important

Hi,

There is bug in dh_acc, which is hidden by bug in Dh_lib.  I intend to
fix the latter soon, which will cause issues in dh_acc once that
happens.  This bug is here:


>  	if ($definition) {
>  	    # TODO if next command fails, should output debug/log info?
>  	    doit("abi-compliance-checker -q -l $package -v1 $version -dump $definition -dump-path $abidump");
>  	}
>  	if ($base) {
>  	    doit("abi-compliance-checker -l $package -d1 $base -d2 $abidump -report-path ${path}_report.html");
>  	    doit("abi-compliance-checker -q -l $package -d1 $base -d2 $abidump -xml -report-path ${path}_report.xml");
>  	}

The API of doit intends for it to *not* fork a shell, but the above
only works in a shell call.  In the concrete case, please use a list
instead, e.g.:

  doit('abi-compliance-checker', '-q', '-l', $package, '-v1', $version, '-dump', $definition, '-dump-path', $abidump);

Thanks,
~Niels

Message #10 received at 868473@bugs.debian.org (full text, mbox, reply):

From: Jeremy Bicha <jbicha@ubuntu.com>

To: 868473@bugs.debian.org, Niels Thykier <niels@thykier.net>

Cc: Gianfranco Costamagna <locutusofborg@debian.org>

Subject: Re: dh-acc: Incorrect usage of doit (Dh_Lib)

Date: Thu, 10 Aug 2017 12:30:27 -0400

Hi, I just wanted to point out that Ubuntu uses autopkgtests to check
uploads before allowing them into the regular development repository.
This bug is causing a very large number of autopkgtests to fail, which
is disrupting Ubuntu development work. Generally, anything outputted
to stderr will fail the autopkgtest.

Here's a sample of what we see:

acc                  FAIL stderr: dh_acc: doit() + doit_*() calls will
no longer spawn a shell in compat 11 for single string arguments
(please use complex_doit instead)

See full log in one of the newer failures at
https://ci.debian.net/packages/n/nemo/unstable/amd64/

This affects a lot of packages:
https://codesearch.debian.net/search?q=path%3Adebian%2Ftests%2Fcontrol+dh-acc

Could you not emit that warning to stderr especially since I see only
one package using compat 11 now?

https://lintian.debian.org/tags/package-uses-experimental-debhelper-compat-version.html

Thanks,
Jeremy Bicha

Message #15 received at 868473@bugs.debian.org (full text, mbox, reply):

From: Niels Thykier <niels@thykier.net>

To: Jeremy Bicha <jbicha@ubuntu.com>, 868473@bugs.debian.org

Cc: Gianfranco Costamagna <locutusofborg@debian.org>

Subject: Re: dh-acc: Incorrect usage of doit (Dh_Lib)

Date: Thu, 10 Aug 2017 19:34:00 +0000

Jeremy Bicha:
> Hi,

Hi Jeremy,

I am sorry to hear that debhelper is causing you issues.

> I just wanted to point out that Ubuntu uses autopkgtests to check
> uploads before allowing them into the regular development repository.
> This bug is causing a very large number of autopkgtests to fail, which
> is disrupting Ubuntu development work. Generally, anything outputted
> to stderr will fail the autopkgtest.
>> [...]
> 
> Could you not emit that warning to stderr especially since I see only
> one package using compat 11 now?
> 

Sorry, but that argument is not enough to convince me that removing the
warning in Debian is a good solution.

 * dh-acc is relying on a broken implementation detail that is being
   phased out.  I would expect a warning in such situations.

 * At compat 11 this behaviour becomes an error, so it would ruin the
   point of the warning if we "wait" until compat 11.

 * The change to dh-acc is literally a 3-line change and would solve
   this issue as well (the process described in my original mail).  This
   change is compatible with all supported versions of debhelper.

Again, I understand your frustration, but I think the proper fix is at
the level where it would be faster to deploy than further discussion,
very trivial to do and with plenty of test cases.

Thanks,
~Niels

Message #20 received at 868473@bugs.debian.org (full text, mbox, reply):

From: Gianfranco Costamagna <locutusofborg@debian.org>

To: Niels Thykier <niels@thykier.net>, Jeremy Bicha <jbicha@ubuntu.com>, "868473@bugs.debian.org" <868473@bugs.debian.org>

Subject: Re: dh-acc: Incorrect usage of doit (Dh_Lib)

Date: Thu, 10 Aug 2017 20:42:06 +0000 (UTC)

[Message part 1 (text/plain, inline)]

control: severity -1 serious
control: tags -1 patch

>Sorry, but that argument is not enough to convince me that removing the
>warning in Debian is a good solution.


I agree, specially because the documentation was correct, and it worked
just because of a bug in implementation
>Again, I understand your frustration, but I think the proper fix is at
>the level where it would be faster to deploy than further discussion,
>very trivial to do and with plenty of test cases.


I'm attaching the patch I did try to craft some minutes ago, but in the meanwhile,
I think bumping severity to RC is something needed, because even if not (yet) enforced
by policy, breaking autopkgtests in such a way is source of troubles.
(and maybe the package now is just completely broken, not just autopkgtests)


(I'm still not a perl man, I'm not sure I did it correctly, and moreover I had trobles
in doing the string concatenation)

G.

[debdiff (application/octet-stream, attachment)]

Message #29 received at 868473@bugs.debian.org (full text, mbox, reply):

From: Niels Thykier <niels@thykier.net>

To: Gianfranco Costamagna <locutusofborg@debian.org>, Jeremy Bicha <jbicha@ubuntu.com>, "868473@bugs.debian.org" <868473@bugs.debian.org>

Subject: Re: dh-acc: Incorrect usage of doit (Dh_Lib)

Date: Thu, 10 Aug 2017 21:18:00 +0000

Gianfranco Costamagna:
> [...]
> 
> 
> I'm attaching the patch I did try to craft some minutes ago, [...]
> 

Thanks. :)

> 
> (I'm still not a perl man, I'm not sure I did it correctly, and moreover I had trobles
> in doing the string concatenation)
> 
> G.
> 

No problem, I am happy to assist. :)

> -	    doit("abi-compliance-checker -l $package -d1 $base -d2 $abidump -report-path ${path}_report.html");
> -	    doit("abi-compliance-checker -q -l $package -d1 $base -d2 $abidump -xml -report-path ${path}_report.xml");
> +	    doit('abi-compliance-checker', '-l', $package, '-d1', $base, '-d2', $abidump, '-report-path', ${path}.'_report.html');
> +	    doit('abi-compliance-checker', '-q', '-l', $package, '-d1', $base, '-d2', $abidump, '-xml', '-report-path', ${path}.'_report.xml');

Looks correct to my eye.  Personally, I would probably have used:


 doit(..., '-report-path', "${path}_report.html");
 doit(..., '-report-path', "${path}_report.xml");

But AFAICT your version is equally correct and we are just debating code
style at the moment. :)

Thanks,
~Niels

Message #34 received at 868473@bugs.debian.org (full text, mbox, reply):

From: Gianfranco Costamagna <locutusofborg@debian.org>

To: Niels Thykier <niels@thykier.net>, Jeremy Bicha <jbicha@ubuntu.com>, "868473@bugs.debian.org" <868473@bugs.debian.org>

Subject: Re: dh-acc: Incorrect usage of doit (Dh_Lib)

Date: Fri, 11 Aug 2017 09:09:32 +0000 (UTC)

Hello Niels,
>doit(..., '-report-path', "${path}_report.html");
>doit(..., '-report-path', "${path}_report.xml");
>
>But AFAICT your version is equally correct and we are just debating code
>style at the moment. :)



interesting, for some reasons with single quote, vim wasn't highlighting
the ${path} variable, while with double quoting it was.
Since I can't believe there is a language were single and double quoting have different
meanings (and it case they have, how can I be sure that double quoting is correct?),
I went for the concatenate-string-google way of doing things :)

I was wondering about a vim highlight bug, this is why I preferred to avoid playing with
quotes (it took three attempts for me to have something that was "working" :) )

thanks!

G.

Message #39 received at 868473@bugs.debian.org (full text, mbox, reply):

From: Jeremy Bicha <jbicha@ubuntu.com>

To: Gianfranco Costamagna <locutusofborg@debian.org>

Cc: Niels Thykier <niels@thykier.net>, "868473@bugs.debian.org" <868473@bugs.debian.org>

Subject: Re: dh-acc: Incorrect usage of doit (Dh_Lib)

Date: Thu, 31 Aug 2017 08:07:45 -0400

This bug is still unfixed in Debian right? It makes debci less useful
for packages that use dh-acc.

Thanks,
Jeremy Bicha

Message #44 received at 868473@bugs.debian.org (full text, mbox, reply):

From: Gianfranco Costamagna <locutusofborg@debian.org>

To: 868473@bugs.debian.org

Subject: Re: dh-acc: Incorrect usage of doit (Dh_Lib)

Date: Fri, 1 Sep 2017 07:53:30 +0200

[Message part 1 (text/plain, inline)]

On Thu, 31 Aug 2017 08:07:45 -0400 Jeremy Bicha <jbicha@ubuntu.com> wrote:
> This bug is still unfixed in Debian right? It makes debci less useful
> for packages that use dh-acc.
> 

you are right

the following debdiff is going in unstable *right now*

(I also took the gcc fixes from Ubuntu)

thanks!

G.
> Thanks,
> Jeremy Bicha
> 
> 

[debdiff (text/plain, attachment)]

[signature.asc (application/pgp-signature, attachment)]

Message #49 received at 868473-close@bugs.debian.org (full text, mbox, reply):

From: Gianfranco Costamagna <locutusofborg@debian.org>

To: 868473-close@bugs.debian.org

Subject: Bug#868473: fixed in abi-compliance-checker 1.99.22-1.1

Date: Fri, 01 Sep 2017 06:04:05 +0000

Source: abi-compliance-checker
Source-Version: 1.99.22-1.1

We believe that the bug you reported is fixed in the latest version of
abi-compliance-checker, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 868473@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Gianfranco Costamagna <locutusofborg@debian.org> (supplier of updated abi-compliance-checker package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Format: 1.8
Date: Fri, 01 Sep 2017 07:51:53 +0200
Source: abi-compliance-checker
Binary: abi-compliance-checker dh-acc
Architecture: source
Version: 1.99.22-1.1
Distribution: unstable
Urgency: high
Maintainer: Ryan Niebur <ryanryan52@gmail.com>
Changed-By: Gianfranco Costamagna <locutusofborg@debian.org>
Description:
 abi-compliance-checker - tool to compare ABI compatibility of shared C/C++ library version
 dh-acc     - debhelper addon to compare ABI compatibility of shared C/C++ libr
Closes: 868473
Changes:
 abi-compliance-checker (1.99.22-1.1) unstable; urgency=high
 .
   [ Dmitry Shachnev ]
   * Add a patch to fix tests compilation with new GCC.
   * Add a patch to not rely on GCC for symbols mangling, it is not working
     with GCC 6 and GCC 7 because of PR c++/78040.
 .
   [ Gianfranco Costamagna ]
   * Non-maintainer upload
   * Fix autopkgtests, due to bad doit call (Closes: #868473)
Checksums-Sha1:
 09f41db3019c8566a991a1e2ae94b6308adbdda5 2190 abi-compliance-checker_1.99.22-1.1.dsc
 a3ed0426d538920d9bbb2459478b1a664592bb0d 8028 abi-compliance-checker_1.99.22-1.1.debian.tar.xz
Checksums-Sha256:
 9a7f3e4f769e39af2cfe9d8fb5599763b32637af7b61cd8a3146a81489d5be0e 2190 abi-compliance-checker_1.99.22-1.1.dsc
 d1c589af75f965aa7657504b3ec17db64f23bb167e872ee811794669c3ec20c5 8028 abi-compliance-checker_1.99.22-1.1.debian.tar.xz
Files:
 d5c4c70833e989eaa65e9938b081f1e9 2190 devel optional abi-compliance-checker_1.99.22-1.1.dsc
 e5ad3ab23a362bad0e314c31232af911 8028 devel optional abi-compliance-checker_1.99.22-1.1.debian.tar.xz

-----BEGIN PGP SIGNATURE-----

iQIcBAEBCAAGBQJZqPWHAAoJEPNPCXROn13ZJ1gP/3DsXP8PzUpGFb82cmUuiv+7
5/7JZNGvARukpgU73vnJJPyaNZqM4vh9gYNj7ETfcQL56zjKLcZXM4AfKBr3hnGF
+yjqTkdQfnsiLCpDwKBtbcIobBVQI+UYy013vrnmNHNM28yYrtNs1vzrMFmB4RDp
209UYyzid/Fiwbg+KtkDbYOsTXppHEUHmCc+GkwOdPSArPGpVTilpN8PwYEgr7Y5
CSZAhzU1QTjqXmCFR8B0q6QD0XLzB7bL1GyjVzvtXwJ19eIY77I82YWarl+wleSt
1GbYTEdIFAvQNn4eaWvB8SpwggfgOhnhd1PwcZ/OSyPQ3sIJ24tg2KwtOjpVd2UD
YfiQjOPzO+FePaVGUsx2m1xT27NDftuopGpf1Kvnz/NLAxqt2mJaN/LUR+B7PGDm
vVt9FehcZhlAL+I/BMoMeFzSnWgxTG0uVjYwtsCY5ZFCIINkjJJ+66JZVSrkF1Fk
Hwv6qVJzIX7kZZAdr1LgNVJyUJNfEMEFKAflRfH1rV0SARA8wkOK/PqqFAbqv6N5
T2sSuKcm6/Ht3X5NTFyMgEGciMeGjPLzb4att3YJkvZThaAsp2FpqHVPpNEYjt02
B0bBYttQL60Mcc5QPnI8XVfzMLap/bYJwCt+92ZBjaBsTXx+bF0JXwuVQ8gh9WBn
Hgc+aIQnRZ/5hVPpTP9q
=gyy/
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.