[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Request for review / upload for libintl-perl



Hi Gregor, and anyone else who may be able to assist,

I have resolved a number of the below Lintian results, and pushed to origin.

Having trouble with the shebang changes needed by Lintian, and the scripts indicated.
I can run the command locally as:
sed -Ei -e '1s|^#!\s*/usr/local/bin/perl|#!/usr/bin/perl|' ./sample/simplecal/bin/simplecal.pl
gdh sample/simplecal/bin/simplecal.pl
diff --git a/sample/simplecal/bin/simplecal.pl b/sample/simplecal/bin/simplecal.pl
index ec184ba..8dee002 100644
--- a/sample/simplecal/bin/simplecal.pl
+++ b/sample/simplecal/bin/simplecal.pl
@@ -1,4 +1,4 @@
-#! /usr/local/bin/perl -w
+#!/usr/bin/perl -w
 # vim: tabstop=4
 
 use strict;
and the file is updated properly.

However, it fails to update the files under a build with the following in the `debian/rules` file:
override_dh_installexamples:
    dh_installexamples
    find $(TMP)/usr/share/doc/$(PACKAGE)/examples -type f -print0 | \
         xargs -r0 sed -E -i -e '1s|^#!\s*/usr/local/bin/perl|#!/usr/bin/perl|'


The first observation is that there are no `TMP` or `PACKAGE` environment variables defined, so the `find` would be akin to:
override_dh_installexamples:
    dh_installexamples
    find /usr/share/doc//examples -type f -print0 | \
         xargs -r0 sed -E -i -e '1s|^#!\s*/usr/local/bin/perl|#!/usr/bin/perl|'


The sample file used above to prove the command works manually, is located under:
./debian/libintl-perl/usr/share/doc/libintl-perl/examples/simplecal/lib/SimpleCal.pm
and no environment variables exist with the appropriate values for:
TMP = ./debian/libintl-perl
PACKAGE = libintl-perl/

Any ideas are welcome.

Thanks
--
Ken Ibbotson
E: keni@computer.org

"Reality is merely an illusion, albeit a very persistent one."
    - Albert Einstein (1879-1955)


On Mon, 18 Jan 2021 at 01:29, gregor herrmann <gregoa@debian.org> wrote:
On Mon, 11 Jan 2021 20:48:27 +1030, Ken Ibbotson wrote:

> Please review and upload if acceptable.
> dpt checkout libintl-perl

Thanks for your work on this package!

Some remarks:

- d/changelog:
  + "Peter Eisentraut <petere@debian.org> moved to Uploaders." should
    be removed, as you have later correctly removed them from
    Uploaders.
  + Maybe document some more of your changes (e.g. the conversion of
    d/copyright is missing).

- d/copyright:
  + including old copyright holders for debian/* might be nice,
    cf. `dh-make-perl refresh --only copyright'
  + you used "Artistic or GPL-1+" for debian/*, which is of course
    fine; in general we use the superset of upstream license and perl
    license, aka "GPL-3+ or Artistic or GPL-1+"

- d/control:
  + "perl (>= 5.8.0-7)" -> the version can be dropped, as even oldoldstable
    has 5.20; cf. `cme fix dpkg-control'
  + this also removes the version from "dpkg-dev (>= 1.16.1~)", and
    an unversioned dpkg-dev can be dropped completely

- debian/.gbp.conf
  does this work? gbp.conf(5) mentions debian/gbp.conf.

- lintian overrides: they make me curious, especially because they
  don't have a comment, explaining what they do :)
  Let's see:
  I: libintl-xs-perl: unused-override unusual-interpreter
  So this one can be removed
  O: libintl-perl: unusual-interpreter usr/share/perl5/Locale/Messages.pm #!/bin/false
  Hm, ok weird but well …

- debian/rules:
  + PERL_MM_USE_DEFAULT=1 can be removed (I believe it's set by the
    debhelper perl makemaker buildsystem)
  + The old d/rules had some none-default commands which are missing
    now. Let's see what they do …
    Test 1: building twice in a row. This fails with
    dpkg-source: info: local changes detected, the modified files are:
     libintl-perl-1.26/ReleaseNotes
     libintl-perl-1.26/config.log
    So these two files should be put into debian/clean

    Then we had "dh_install -i --exclude=xs", which surprised me but
    explains the lintian warning
    W: libintl-perl source: binaries-have-file-conflict libintl-perl libintl-xs-perl usr/share/man/man3/Locale::gettext_xs.3pm.gz
    i.e. this file is in both packages which is prevented by the
    --exclude. - Hm, how is this written nowadays?
    The following seems to work:

    override_dh_install:
        dh_install -a
        dh_install -i --exclude=xs

- autopkgtests fail or are skipped … We need
  + debian/tests/pkg-perl/smoke-files with test.pl and tests/
    and then it still fails *sigh*
    cannot open './xs_disabled': No such file or directory at ./test.pl line 59.
    So we need a smoke-setup as well … and here we go.
  + debian/tests/pkg-perl/use-name with the name of one perl module,
    maybe Locale::Recode
  + I've committed and pushed the autopkgtests changes as I already
    have them here :)

> 2 patches have been raised to handle spelling and POD markup corrections.

Thanks.

> I did have to increment the dot version in changelog as the 3000 build
> action currently happening updated to version, and the 'dpt takeover' used
> the same version number '1.26-2.1`.

This (-2.2) is not correct. -X.Y are for non-maintainer uploads by
convention (as the -2.1 from Holger) but since we are doing a regular
maintainer upload now, -3 is the next "regular" debian revision.

See also lintian's
W: libintl-perl source: maintainer-upload-has-incorrect-version-number 1.26-2.2

> Not sure if this may be a bug with the takeover script.

Well, what dpt-takeover did was
+libintl-perl (1.26-2.1ubuntu1) UNRELEASED; urgency=medium
which points to a different enironment :)

(In general, dpt-takeover just calls `dch' which picks the next
version.)

> Lintian does show a couple more items - please advise on the same if not
> ready for upload.

Let's see what else we have besides the messages already discussed:

W: libintl-perl: national-encoding usr/share/doc/libintl-perl/examples/simplecal/po/ar.po
Well, yes. Might warrant an override.

I: libintl-perl source: duplicate-short-description libintl-perl libintl-xs-perl
Hm, yes, the short description is identical. If there is some space
in <80 characters, libintl-xs-perl could add something about XS
there.
If not, no big deal.

I: libintl-perl: example-wrong-path-for-interpreter usr/share/doc/libintl-perl/examples/simplecal/Makefile.PL (#!/usr/local/bin/perl != /usr/bin/perl)
I: libintl-perl: example-wrong-path-for-interpreter usr/share/doc/libintl-perl/examples/simplecal/bin/simplecal.pl (#!/usr/local/bin/perl != /usr/bin/
P: libintl-perl: example-unusual-interpreter usr/share/doc/libintl-perl/examples/simplecal/lib/SimpleCal.pm #!/bin/false

I think these should be fixed (at least the first two).
https://perl-team.pages.debian.net/debhelper.html has some recipes,
cf. the "Note on paths" and "Fixing Interpreter Shebang Lines"
sections

I: libintl-perl source: patch-not-forwarded-upstream debian/patches/dot-inc.patch
Might be worth taking a look at (if it's already fixed upstream or
needs forwarding etc.)


Ok, I think that's all for now :)


Cheers,
gregor


--
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   NP: Mark Knopfler: Irish Boy

Reply to: