NACK: [Unstable][PATCH] UBUNTU: [Packaging] Rewrite debian/scripts/misc/insert-changes.pl in Python
Thadeu Lima de Souza Cascardo
cascardo at canonical.com
Tue Nov 2 10:10:10 UTC 2021
On Tue, Nov 02, 2021 at 08:45:38AM +0100, Juerg Haefliger wrote:
> On Mon, 1 Nov 2021 15:04:17 -0300
> Thadeu Lima de Souza Cascardo <cascardo at canonical.com> wrote:
>
> > On Mon, Nov 01, 2021 at 10:40:05AM +0100, Juerg Haefliger wrote:
> > > Rewrite the insert-changes.pl script in Python to get us one step closer
> > > to dropping Perl as an Ubuntu kernel build dependency.
> > >
> > > Signed-off-by: Juerg Haefliger <juergh at canonical.com>
> > > ---
> > > debian/rules.d/1-maintainer.mk | 2 +-
> > > debian/scripts/misc/insert-changes | 35 ++++++++++++++++++++++
> > > debian/scripts/misc/insert-changes.pl | 43 ---------------------------
> > > 3 files changed, 36 insertions(+), 44 deletions(-)
> > > create mode 100755 debian/scripts/misc/insert-changes
> > > delete mode 100755 debian/scripts/misc/insert-changes.pl
> > >
> > > diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk
> > > index 7c13fcdaa98a..5917b89bbd49 100644
> > > --- a/debian/rules.d/1-maintainer.mk
> > > +++ b/debian/rules.d/1-maintainer.mk
> > > @@ -113,7 +113,7 @@ printchanges:
> > > $(DROOT)/scripts/misc/git-ubuntu-log $(ubuntu_log_opts)
> > >
> > > insertchanges: autoreconstruct finalchecks
> > > - @perl -w -f $(DROOT)/scripts/misc/insert-changes.pl $(DROOT) $(DEBIAN)
> > > + $(DROOT)/scripts/misc/insert-changes $(DROOT) $(DEBIAN)
> > >
> > > autoreconstruct:
> > > # No need for reconstruct for -rc kernels since we don't upload an
> > > diff --git a/debian/scripts/misc/insert-changes b/debian/scripts/misc/insert-changes
> > > new file mode 100755
> > > index 000000000000..284b6788cc13
> > > --- /dev/null
> > > +++ b/debian/scripts/misc/insert-changes
> > > @@ -0,0 +1,35 @@
> > > +#!/usr/bin/python3
> > > +
> > > +import os
> > > +import sys
> > > +
> > > +from subprocess import check_output
> > > +
> > > +droot = 'debian'
> > > +if len(sys.argv) > 1:
> > > + droot = sys.argv[1]
> > > +
> > > +debian = 'debian'
> >
> > Should be
> > debian = 'debian.master'
>
> Indeed.
>
>
> > > +if len(sys.argv) > 2:
> > > + debian = sys.argv[2]
> > > +
> > > +rules = os.path.join(droot, 'rules')
> > > +changelog = os.path.join(debian, 'changelog')
> > > +changelog_new = os.path.join(debian, 'changelog.new')
> > > +
> > > +# Generate the list of new changes
> > > +changes = check_output(['make', '-s', '-f', rules, 'printchanges']).decode('UTF-8')
> > > +
> >
> > Once again, I wonder if we can make this better: make calling a script
> > calling make calling a script. Make is good when you have a dependency
> > tree, but that is not the case here. I am just thinking that we should
> > consider the option where we keep these tools in tree, and still allow them
> > to be run from make if we really care, but have cranky start calling them
> > directly.
>
> Good point. Will look into it.
>
>
> > > +# Insert the new changes into the changelog
> > > +with open(changelog) as orig, open(changelog_new, 'w') as new:
> > > + printed = False
> > > + for line in orig:
> > > + if line.startswith(' CHANGELOG: '):
> > > + if not printed:
> > > + printed = True
> > > + new.write(changes)
> > > + else:
> > > + new.write(line)
> > > +
> >
> > The newline flag is missing here. It has certainly caused a regression, after I
> > gave it a try.
>
> What regression is that? The output looks just fine to me. No double empty
> lines in the modified changelog entry.
>
> Oh, if debian.master/changes is empty? When would that happen?
>
> ...Juerg
>
Whenever we rebase from a master kernel and do not have a tracking bug. Which
happens ever so often when there are CVEs-only releases.
Cascardo.
>
> > Cascardo.
> >
> > > +# Replace the original changelog with the new one
> > > +os.rename(changelog_new, changelog)
> > > diff --git a/debian/scripts/misc/insert-changes.pl b/debian/scripts/misc/insert-changes.pl
> > > deleted file mode 100755
> > > index 4eed4e28f9d3..000000000000
> > > --- a/debian/scripts/misc/insert-changes.pl
> > > +++ /dev/null
> > > @@ -1,43 +0,0 @@
> > > -#!/usr/bin/perl -w
> > > -
> > > -my $debian;
> > > -$droot = $ARGV[0] if (defined $ARGV[0]);
> > > -$droot = 'debian' if (!defined $droot);
> > > -$debian = $ARGV[1] if (defined $ARGV[1]);
> > > -$debian = 'debian.master' if (!defined $debian);
> > > -
> > > -system("make -s -f $droot/rules printchanges > $debian/changes");
> > > -
> > > -open(CHANGELOG, "< $debian/changelog") or die "Cannot open changelog";
> > > -open(CHANGES, "< $debian/changes") or die "Cannot open new changes";
> > > -open(NEW, "> $debian/changelog.new") or die "Cannot open new changelog";
> > > -
> > > -$printed = 0;
> > > -my $skip_newline = 0;
> > > -
> > > -while (<CHANGELOG>) {
> > > - if (/^ CHANGELOG: /) {
> > > - next if $printed;
> > > -
> > > - $skip_newline = 1;
> > > - while (<CHANGES>) {
> > > - $skip_newline = 0;
> > > - print NEW;
> > > - }
> > > -
> > > - $printed = 1;
> > > - } else {
> > > - if (/^$/ && $skip_newline == 1) {
> > > - $skip_newline = 0;
> > > - next;
> > > - }
> > > - print NEW;
> > > - }
> > > -}
> > > -
> > > -close(NEW);
> > > -close(CHANGES);
> > > -close(CHANGELOG);
> > > -
> > > -rename("$debian/changelog.new", "$debian/changelog");
> > > -unlink("$debian/changes");
> > > --
> > > 2.30.2
> > >
> > >
> > > --
> > > kernel-team mailing list
> > > kernel-team at lists.ubuntu.com
> > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
More information about the kernel-team
mailing list