NACK: [Unstable][PATCH] UBUNTU: [Packaging] Rewrite debian/scripts/misc/insert-changes.pl in Python

[Thadeu Lima de Souza Cascardo]

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'

> +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.

> +# 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.

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