From 925f4dca26f32138f1d58b153f766992516d3d28 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Mon, 24 Jul 2023 14:06:51 +0100
Subject: [PATCH 01/11] ncm-spma: apt: Use latest release of CAF

---
 ncm-spma/src/main/perl/spma/apt.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index bfe542283a..50c5428e78 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -46,7 +46,7 @@ Packages listed under C</software/packages> will be installed, version and archi
 =cut
 
 use parent qw(NCM::Component CAF::Path);
-use CAF::Path 17.3.1;
+use CAF::Path 21.12.1;
 use CAF::Process;
 use CAF::FileWriter;
 use CAF::FileEditor;

From 8a44cfc0108240a3bba2403d1dcf445edb78c1d1 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Wed, 24 Jan 2018 13:32:22 +0000
Subject: [PATCH 02/11] ncm-spma: apt: Handle exit status of commands

Provide an internal wrapper for apt commands.
Provide a flag to allow errors to become warnings.
---
 ncm-spma/src/main/perl/spma/apt.pm | 43 +++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index 50c5428e78..ced842191c 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -1,5 +1,9 @@
 #${PMpre} NCM::Component::spma::apt${PMpost}
 
+use Data::Dumper;
+$Data::Dumper::Indent = 0; # Supress indentation and new-lines
+$Data::Dumper::Terse = 1; # Output values only, supress variable names if possible
+
 =head1 NAME
 
 C<NCM::Component::spma::apt> - NCM SPMA backend for apt
@@ -77,6 +81,23 @@ Readonly my $CMD_DPKG_QUERY => [$BIN_DPKG_QUERY, qw(-W -f=${db:Status-Abbrev};${
 
 our $NoActionSupported = 1;
 
+# Wrapper function for calling apt commands
+sub _call_apt
+{
+    my ($self, $cmd, $ok) = @_;
+    $self->debug(5, '_call_apt: Called with args ', Dumper($cmd));
+
+    my $proc = CAF::Process->new($cmd);
+    my $output = $proc->output();
+    my $exitstatus = $? >> 8; # Get exit status from highest 8-bits
+    $self->debug(5, "_call_apt: $proc exited with $exitstatus");
+    if ($exitstatus > 0) {
+        $output =~ tr{\n}{ };
+        my $method = $ok ? 'warn' : 'error';
+        $self->$method("_call_apt: $proc failed with \"$output\"");
+    }
+    return $ok || $exitstatus == 0;
+}
 
 # If user specified sources (userrepos) are not allowed, removes any
 # sources present in the system that are not listed in $allowed_sources.
@@ -166,8 +187,9 @@ sub get_installed_pkgs
     $self->debug(5, 'Entered get_installed_pkgs()');
 
     my $out = CAF::Process->new($CMD_DPKG_QUERY, keeps_state => 1) ->output();
-    if ($?) {
-        $self->debug(5, "dpkg command returned $?");
+    my $exitstatus = $? >> 8; # Get exit status from highest 8-bits
+    if ($exitstatus) {
+        $self->debug(5, "dpkg command returned $exitstatus");
         return 0;
     }
     # db:Status-Abbrev is three characters, we are looking for
@@ -262,8 +284,7 @@ sub resynchronize_package_index
     my $self = shift;
     $self->debug(5, 'Entered resynchronize_package_index()');
 
-    my $cmd = CAF::Process->new($CMD_APT_UPDATE, keeps_state => 1);
-    return $cmd->execute() ? 1 : undef;
+    return $self->_call_apt($CMD_APT_UPDATE);
 }
 
 
@@ -273,8 +294,9 @@ sub upgrade_packages
     my ($self) = @_;
     $self->debug(5, 'Entered upgrade_packages()');
 
-    my $cmd = CAF::Process->new($CMD_APT_UPGRADE) ;
-    return $cmd->execute() ? 1 : undef;
+    # it's ok if this produces errors (eg unfinished stuff)
+    # TODO: add support for 'apt --fix-broken install' and things like that
+    return $self->_call_apt($CMD_APT_UPGRADE, 1);
 }
 
 
@@ -284,8 +306,7 @@ sub install_packages
     my ($self, $packages) = @_;
     $self->debug(5, 'Entered install_packages()');
 
-    my $cmd = CAF::Process->new([@$CMD_APT_INSTALL, @$packages]) ;
-    return $cmd->execute() ? 1 : undef;
+    return $self->_call_apt([@$CMD_APT_INSTALL, @$packages]);
 }
 
 
@@ -296,8 +317,7 @@ sub mark_packages_auto
     my ($self, $packages) = @_;
     $self->debug(5, 'Entered mark_packages_auto()');
 
-    my $cmd = CAF::Process->new([@$CMD_APT_MARK, 'auto', @$packages]) ;
-    return $cmd->execute() ? 1 : undef;
+    return $self->_call_apt([@$CMD_APT_MARK, 'auto', @$packages]);
 }
 
 
@@ -307,8 +327,7 @@ sub autoremove_packages
     my ($self) = @_;
     $self->debug(5, 'Entered autoremove_packages()');
 
-    my $cmd = CAF::Process->new([@$CMD_APT_AUTOREMOVE]) ;
-    return $cmd->execute() ? 1 : undef;
+    return $self->_call_apt([@$CMD_APT_AUTOREMOVE]);
 }
 
 

From 3c49bc4171a413cdaa214ba967b13c6e84ff7197 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Wed, 24 Jan 2018 13:39:09 +0000
Subject: [PATCH 03/11] ncm-spma: apt: Output error if rendering apt config
 fails

---
 ncm-spma/src/main/perl/spma/apt.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index ced842191c..32f80acdc0 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -175,9 +175,9 @@ sub configure_apt
     if ($tr) {
         my $fh = $tr->filewriter($FILE_CONFIG);
         return $fh->close() || 0; # handle undef
-    } else {
-        return 0;
     }
+    $self->error('configure_apt: TextRender failed to render configuration');
+    return 0;
 }
 
 # Returns a set of all installed packages

From 422211d9ed0199a29d2c137c7a4a21fdcb4737af Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Wed, 26 Jul 2023 11:22:17 +0100
Subject: [PATCH 04/11] ncm-spma: apt: Prefix debugging with subroutine name

Reduce level of more frequently desired debug messages to seperate them from the noise.
Don't log entry to Configure.
---
 ncm-spma/src/main/perl/spma/apt.pm | 39 +++++++++++++++---------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index 32f80acdc0..b2d0cfb2f9 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -104,7 +104,7 @@ sub _call_apt
 sub cleanup_old_sources
 {
     my ($self, $sources_dir, $allowed_sources) = @_;
-    $self->debug(5, 'Entered cleanup_old_sources()');
+    $self->debug(5, "cleanup_old_sources: Called with args ", $sources_dir, $allowed_sources);
 
     if ($self->directory_exists($sources_dir)) {
         my $current = Set::Scalar->new(@{$self->listdir($sources_dir, filter => qr{\.list$}, adddir => 1)});
@@ -129,7 +129,7 @@ sub cleanup_old_sources
 sub initialize_sources_dir
 {
     my ($self, $sources_dir) = @_;
-    $self->debug(5, 'Entered initialize_sources_dir()');
+    $self->debug(5, "initialize_sources_dir: Called with args($sources_dir)");
 
     if (! $self->directory($sources_dir)) {
         $self->error("Unable to create source dir $sources_dir: $self->{fail}");
@@ -145,7 +145,7 @@ sub initialize_sources_dir
 sub generate_sources
 {
     my ($self, $sources_dir, $sources, $template) = @_;
-    $self->debug(5, 'Entered generate_sources()');
+    $self->debug(5, "generate_sources: Called with args($sources_dir, $sources, $template)");
 
     my $changes = 0;
 
@@ -169,7 +169,7 @@ sub generate_sources
 sub configure_apt
 {
     my ($self, $config) = @_;
-    $self->debug(5, 'Entered configure_apt()');
+    $self->debug(5, 'configure_apt: Called with args', Dumper($config));
 
     my $tr = EDG::WP4::CCM::TextRender->new($TEMPLATE_CONFIG, $config, relpath => 'spma');
     if ($tr) {
@@ -183,8 +183,8 @@ sub configure_apt
 # Returns a set of all installed packages
 sub get_installed_pkgs
 {
-    my $self = shift;
-    $self->debug(5, 'Entered get_installed_pkgs()');
+    my ($self) = @_;
+    $self->debug(5, 'get_installed_pkgs: Called');
 
     my $out = CAF::Process->new($CMD_DPKG_QUERY, keeps_state => 1) ->output();
     my $exitstatus = $? >> 8; # Get exit status from highest 8-bits
@@ -206,7 +206,7 @@ sub get_installed_pkgs
 sub get_package_version_arch
 {
     my ($self, $name, $details) = @_;
-    $self->debug(5, 'Entered get_package_version_arch()');
+    $self->debug(5, "get_package_version_arch: Called with args($name, ", Dumper($details), ")");
 
     my @versions;
 
@@ -216,19 +216,21 @@ sub get_package_version_arch
             $version = unescape($version);
             if ($params->{arch}) {
                 foreach my $arch (sort keys %{ $params->{arch} }) {
-                    $self->debug(5, '  Adding package ', $name, ' with version ', $version, ' and architecture ', $arch, ' to list');
+                    $self->debug(4, 'get_package_version_arch: Adding package ', $name, ' with version ', $version, ' and architecture ', $arch, ' to list');
                     push(@versions, sprintf('%s:%s=%s', $name, $arch, $version));
                 }
             } else {
-                $self->debug(5, '  Adding package ', $name, ' with version ', $version, ' but without architecture to list');
+                $self->debug(4, 'get_package_version_arch: Adding package ', $name, ' with version ', $version, ' but without architecture to list');
                 push(@versions, sprintf('%s=%s', $name, $version));
             }
         }
     } else {
-        $self->debug(5, '  Adding package ', $name, ' without version or architecture to list');
+        $self->debug(4, 'get_package_version_arch: Adding package ', $name, ' without version or architecture to list');
         push(@versions, $name);
     }
 
+    $self->debug(5, 'get_package_version_arch: returning arrayref:', Dumper(\@versions));
+
     return \@versions;
 }
 
@@ -238,7 +240,7 @@ sub apply_package_version_arch
 {
     my ($self, $packagelist, $packagetree) = @_;
 
-    $self->debug(5, 'Entered apply_package_version_arch()');
+    $self->debug(5, "apply_package_version_arch: Called with args", $packagelist, Dumper($packagetree));
 
     my @results;
     my @notfound;
@@ -262,7 +264,7 @@ sub apply_package_version_arch
 sub get_desired_pkgs
 {
     my ($self, $pkgs) = @_;
-    $self->debug(5, 'Entered get_desired_pkgs()');
+    $self->debug(5, "get_desired_pkgs: Called with args", Dumper($pkgs));
 
     my $packages = Set::Scalar->new();
 
@@ -281,8 +283,8 @@ sub get_desired_pkgs
 # Update package metadata from upstream sourcesitories
 sub resynchronize_package_index
 {
-    my $self = shift;
-    $self->debug(5, 'Entered resynchronize_package_index()');
+    my ($self) = @_;
+    $self->debug(5, 'resynchronize_package_index: Called');
 
     return $self->_call_apt($CMD_APT_UPDATE);
 }
@@ -292,7 +294,7 @@ sub resynchronize_package_index
 sub upgrade_packages
 {
     my ($self) = @_;
-    $self->debug(5, 'Entered upgrade_packages()');
+    $self->debug(5, 'upgrade_packages: Called');
 
     # it's ok if this produces errors (eg unfinished stuff)
     # TODO: add support for 'apt --fix-broken install' and things like that
@@ -304,7 +306,7 @@ sub upgrade_packages
 sub install_packages
 {
     my ($self, $packages) = @_;
-    $self->debug(5, 'Entered install_packages()');
+    $self->debug(5, 'install_packages: Called with args', Dumper($packages));
 
     return $self->_call_apt([@$CMD_APT_INSTALL, @$packages]);
 }
@@ -315,7 +317,7 @@ sub install_packages
 sub mark_packages_auto
 {
     my ($self, $packages) = @_;
-    $self->debug(5, 'Entered mark_packages_auto()');
+    $self->debug(5, "mark_packages_auto: Called with args", Dumper($packages));
 
     return $self->_call_apt([@$CMD_APT_MARK, 'auto', @$packages]);
 }
@@ -325,7 +327,7 @@ sub mark_packages_auto
 sub autoremove_packages
 {
     my ($self) = @_;
-    $self->debug(5, 'Entered autoremove_packages()');
+    $self->debug(5, 'autoremove_packages: Called');
 
     return $self->_call_apt([@$CMD_APT_AUTOREMOVE]);
 }
@@ -334,7 +336,6 @@ sub autoremove_packages
 sub Configure
 {
     my ($self, $config) = @_;
-    $self->debug(5, 'Entered Configure()');
 
     # Get configuration trees
     my $tree_sources = $config->getTree($TREE_SOURCES);

From 931adb5b99de644c19de5577b0f60017fb5cda01 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Wed, 24 Jan 2018 13:50:08 +0000
Subject: [PATCH 05/11] ncm-spma: apt: Evaluate details as hashref (bugfix)

---
 ncm-spma/src/main/perl/spma/apt.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index b2d0cfb2f9..f528a10e87 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -210,7 +210,7 @@ sub get_package_version_arch
 
     my @versions;
 
-    if ($details) {
+    if (defined($details) and %$details) {
         foreach my $version (sort keys %$details) {
             my $params = $details->{$version};
             $version = unescape($version);

From e330301b937622bd7e737c3be5b63efd93bd2c32 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Wed, 24 Jan 2018 13:56:16 +0000
Subject: [PATCH 06/11] ncm-spma: apt: Log profile trees in debug 5

Reduce debug level of package lists to allow them to be inspected with
less noise.
---
 ncm-spma/src/main/perl/spma/apt.pm | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index f528a10e87..bb585b81d8 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -339,8 +339,11 @@ sub Configure
 
     # Get configuration trees
     my $tree_sources = $config->getTree($TREE_SOURCES);
+    $self->debug(5, 'TREE_SOURCES ', $TREE_SOURCES, Dumper $tree_sources);
     my $tree_pkgs = $config->getTree($TREE_PKGS);
+    $self->debug(5, 'TREE_PKGS ', $TREE_PKGS, Dumper $tree_pkgs);
     my $tree_component = $config->getTree($self->prefix());
+    $self->debug(5, 'tree_component ', $self->prefix, Dumper $tree_component);
 
     $self->configure_apt($tree_component) or return 0;
 
@@ -365,13 +368,13 @@ sub Configure
     my $packages_desired = $self->get_desired_pkgs($tree_pkgs) or return 0;
     my $packages_unwanted = $packages_installed->difference($packages_desired);
 
-    $self->debug(5, 'Installed packages:', $packages_installed);
-    $self->debug(5, 'Desired packages:', $packages_desired);
-    $self->debug(5, 'Packages installed but unwanted:', $packages_unwanted);
+    $self->debug(4, 'Installed packages: ', $packages_installed);
+    $self->debug(4, 'Desired packages: ', $packages_desired);
+    $self->debug(4, 'Packages installed but unwanted: ', $packages_unwanted);
 
     my $packages_to_install = $self->apply_package_version_arch($packages_desired, $tree_pkgs) or return 0;
 
-    $self->debug(5, 'Packages to install ', $packages_to_install);
+    $self->debug(4, 'Packages to install (desired but not installed): ', $packages_to_install);
 
     $self->install_packages($packages_to_install) or return 0;
 

From 6b59160dfd566d96c99ca5ce62d2692f97e74318 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Wed, 24 Jan 2018 14:25:06 +0000
Subject: [PATCH 07/11] ncm-spma: apt: Apply version and architectures in a
 seperate step

---
 ncm-spma/src/main/perl/spma/apt.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index bb585b81d8..909cb2150b 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -366,17 +366,17 @@ sub Configure
 
     my $packages_installed = $self->get_installed_pkgs() or return 0;
     my $packages_desired = $self->get_desired_pkgs($tree_pkgs) or return 0;
+
     my $packages_unwanted = $packages_installed->difference($packages_desired);
+    my $packages_to_install = $packages_desired->difference($packages_installed);
 
     $self->debug(4, 'Installed packages: ', $packages_installed);
     $self->debug(4, 'Desired packages: ', $packages_desired);
     $self->debug(4, 'Packages installed but unwanted: ', $packages_unwanted);
-
-    my $packages_to_install = $self->apply_package_version_arch($packages_desired, $tree_pkgs) or return 0;
-
     $self->debug(4, 'Packages to install (desired but not installed): ', $packages_to_install);
 
-    $self->install_packages($packages_to_install) or return 0;
+    my $apt_packages_to_install = $self->apply_package_version_arch($packages_to_install, $tree_pkgs);
+    $self->install_packages($apt_packages_to_install) or return 0;
 
     # If user installed packages are not permitted, mark all unlisted packages as automatically installed and
     # ask apt to remove any of these that are not required to satisfy dependencies of the desired package list

From 9b3c9eaf91e2a8afc1a32e6082d829808860a2f7 Mon Sep 17 00:00:00 2001
From: stdweird <stijn.deweirdt@ugent.be>
Date: Fri, 27 Apr 2018 15:25:35 +0200
Subject: [PATCH 08/11] ncm-spma: apt: invert gpgcheck logic to define forced
 trust

---
 ncm-spma/src/main/resources/apt/source.tt     |  2 +-
 .../resources/tests/profiles/apt-sources.pan  | 23 ++++++++++++++++---
 .../{apt-sources => apt-sources/standard}     |  6 ++---
 .../tests/regexps/apt-sources/trusted         | 10 ++++++++
 4 files changed, 34 insertions(+), 7 deletions(-)
 rename ncm-spma/src/main/resources/tests/regexps/{apt-sources => apt-sources/standard} (70%)
 create mode 100644 ncm-spma/src/main/resources/tests/regexps/apt-sources/trusted

diff --git a/ncm-spma/src/main/resources/apt/source.tt b/ncm-spma/src/main/resources/apt/source.tt
index 629e07f9e8..cddc3f1d17 100644
--- a/ncm-spma/src/main/resources/apt/source.tt
+++ b/ncm-spma/src/main/resources/apt/source.tt
@@ -10,7 +10,7 @@
 # Name: [% name %]
 # Owner: [% owner %]
 
-[%- trust = gpgcheck ? "trusted=yes " : "" -%]
+[%- trust = gpgcheck ? "" : "[trusted=yes] " -%]
 [%- FOREACH source IN protocols %]
 # Protocol type: [% source.name %]
 deb [% trust %][% source.url %]
diff --git a/ncm-spma/src/main/resources/tests/profiles/apt-sources.pan b/ncm-spma/src/main/resources/tests/profiles/apt-sources.pan
index ae6367f676..351c6ffb04 100644
--- a/ncm-spma/src/main/resources/tests/profiles/apt-sources.pan
+++ b/ncm-spma/src/main/resources/tests/profiles/apt-sources.pan
@@ -8,9 +8,10 @@ include "components/spma/apt/schema";
 '/software/packages' = dict();
 
 prefix '/software/repositories/0';
-'name' = 'a_source';
+'name' = 'standard_source';
 'owner' = 'localuser@localdomain';
 'enabled' = true;
+'gpgcheck' = true;
 'protocols' = list(
     dict(
         'name', 'http',
@@ -18,14 +19,30 @@ prefix '/software/repositories/0';
     ),
     dict(
         'name', 'http',
-        'url', 'http://another.example.org/another/path trusty main',
+        'url', 'https://second.example.com/path/to/things trusty main',
     ),
 );
 'includepkgs' = list(
     'foo',
-    'bar',
 );
 'excludepkgs' = list(
     'baz',
+);
+
+prefix '/software/repositories/1';
+'name' = 'trusted_source';
+'owner' = 'localuser@localdomain';
+'enabled' = true;
+'gpgcheck' = false;
+'protocols' = list(
+    dict(
+        'name', 'http',
+        'url', 'http://another.example.org/another/path trusty main',
+    ),
+);
+'includepkgs' = list(
+    'bar',
+);
+'excludepkgs' = list(
     'quux',
 );
diff --git a/ncm-spma/src/main/resources/tests/regexps/apt-sources b/ncm-spma/src/main/resources/tests/regexps/apt-sources/standard
similarity index 70%
rename from ncm-spma/src/main/resources/tests/regexps/apt-sources
rename to ncm-spma/src/main/resources/tests/regexps/apt-sources/standard
index 46ff9e7238..a2f65b2391 100644
--- a/ncm-spma/src/main/resources/tests/regexps/apt-sources
+++ b/ncm-spma/src/main/resources/tests/regexps/apt-sources/standard
@@ -1,12 +1,12 @@
 Check that basic apt sources are rendered correctly
 ---
-//software
+renderpath=/software
 rendermodule=apt/source
 contentspath=/software/repositories/0
 ---
-^# Name: a_source$
+^# Name: standard_source$
 ^# Owner: localuser@localdomain$
 ^# Protocol type: http$
 ^deb http://first.example.com/path/to/stuff trusty main$
 ^# Protocol type: http$
-^deb http://another.example.org/another/path trusty main$
+^deb https://second.example.com/path/to/things trusty main$
diff --git a/ncm-spma/src/main/resources/tests/regexps/apt-sources/trusted b/ncm-spma/src/main/resources/tests/regexps/apt-sources/trusted
new file mode 100644
index 0000000000..f9e6433a4c
--- /dev/null
+++ b/ncm-spma/src/main/resources/tests/regexps/apt-sources/trusted
@@ -0,0 +1,10 @@
+Check that trusted (gpgcheck=false) apt sources are rendered correctly
+---
+renderpath=/software
+rendermodule=apt/source
+contentspath=/software/repositories/1
+---
+^# Name: trusted_source$
+^# Owner: localuser@localdomain$
+^# Protocol type: http$
+^deb \[trusted=yes\] http://another.example.org/another/path trusty main

From d13b23a9b8c9f5c5c117b306407c5113038913d6 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Tue, 25 Jul 2023 16:25:29 +0100
Subject: [PATCH 09/11] ncm-spma: apt: Return undef on errors

In generate_sources and configure_apt. Handle those undefs.
---
 ncm-spma/src/main/perl/spma/apt.pm            | 12 ++++++------
 ncm-spma/src/test/perl/apt-sources-generate.t |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index 909cb2150b..8cc92472c4 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -158,7 +158,7 @@ sub generate_sources
             $changes += $fh->close() || 0; # handle undef
         } else {
             $self->error("Invalid template '$template' passed to generate_sources");
-            return 0;
+            return;
         }
     }
 
@@ -177,7 +177,7 @@ sub configure_apt
         return $fh->close() || 0; # handle undef
     }
     $self->error('configure_apt: TextRender failed to render configuration');
-    return 0;
+    return;
 }
 
 # Returns a set of all installed packages
@@ -345,20 +345,20 @@ sub Configure
     my $tree_component = $config->getTree($self->prefix());
     $self->debug(5, 'tree_component ', $self->prefix, Dumper $tree_component);
 
-    $self->configure_apt($tree_component) or return 0;
+    defined($self->configure_apt($tree_component)) or return 0;
 
-    $self->initialize_sources_dir($DIR_SOURCES) or return 0;
+    defined($self->initialize_sources_dir($DIR_SOURCES)) or return 0;
 
     # Remove unknown sources if allow_user_sources is not set
     if (! $tree_component->{usersources}) {
         $self->cleanup_old_sources($DIR_SOURCES, $tree_sources) or return 0;
     };
 
-    $self->generate_sources(
+    defined($self->generate_sources(
         $DIR_SOURCES,
         $tree_sources,
         $TEMPLATE_SOURCES,
-    ) or return 0;
+    )) or return 0;
 
     $self->resynchronize_package_index() or return 0;
 
diff --git a/ncm-spma/src/test/perl/apt-sources-generate.t b/ncm-spma/src/test/perl/apt-sources-generate.t
index c762937d1c..25c5413c69 100644
--- a/ncm-spma/src/test/perl/apt-sources-generate.t
+++ b/ncm-spma/src/test/perl/apt-sources-generate.t
@@ -84,7 +84,7 @@ to disk.
 =cut
 
 remove_any("$SOURCES_DIR/$name.list");
-is($cmp->generate_sources($SOURCES_DIR, $sources, "an invalid template name"), 0, "Invalid template name is detected");
+is($cmp->generate_sources($SOURCES_DIR, $sources, "an invalid template name"), undef, "Invalid template name is detected");
 is($cmp->{ERROR}, 1, "Errors on template rendering are reported");
 $fh = get_file("$SOURCES_DIR/$name.list");
 ok(!defined($fh), "No file could be opened, render failed");

From 0701eb9f22814a3afa33315e033db635eb4fbd30 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Tue, 25 Jul 2023 14:35:12 +0100
Subject: [PATCH 10/11] ncm-spma: apt: Log main steps in Configure

---
 ncm-spma/src/main/perl/spma/apt.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index 8cc92472c4..6557215c36 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -351,17 +351,21 @@ sub Configure
 
     # Remove unknown sources if allow_user_sources is not set
     if (! $tree_component->{usersources}) {
+        $self->info('Removing unknown source lists');
         $self->cleanup_old_sources($DIR_SOURCES, $tree_sources) or return 0;
     };
 
+    $self->info('Generating ', scalar(@$tree_sources), ' source lists');
     defined($self->generate_sources(
         $DIR_SOURCES,
         $tree_sources,
         $TEMPLATE_SOURCES,
     )) or return 0;
 
+    $self->info('Synchronizing package index');
     $self->resynchronize_package_index() or return 0;
 
+    $self->info('Applying upgrades to installed packages');
     $self->upgrade_packages() or return 0;
 
     my $packages_installed = $self->get_installed_pkgs() or return 0;
@@ -376,11 +380,13 @@ sub Configure
     $self->debug(4, 'Packages to install (desired but not installed): ', $packages_to_install);
 
     my $apt_packages_to_install = $self->apply_package_version_arch($packages_to_install, $tree_pkgs);
+    $self->info('Installing ', $packages_to_install->size,' missing packages');
     $self->install_packages($apt_packages_to_install) or return 0;
 
     # If user installed packages are not permitted, mark all unlisted packages as automatically installed and
     # ask apt to remove any of these that are not required to satisfy dependencies of the desired package list
     if (! $tree_component->{userpkgs}) {
+        $self->info('Marking ', $packages_unwanted->size, ' packages as unwanted and removing any that are not dependencies of installed packages');
         $self->mark_packages_auto($packages_unwanted) or return 0;
         $self->autoremove_packages() or return 0;
     }

From 49ef1d3a16696dbb1a86c28766aa75fc4223e511 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Wed, 26 Jul 2023 12:47:17 +0100
Subject: [PATCH 11/11] ncm-spma: apt: Warn if any packages appear to be
 unavailable

This could be because they have been renamed (rare) or they are virtual
(common) in both cases when run interactively apt will warn the user:

e.g.
> Note, selecting 'man-db' instead of 'man'

However this warning is supressed when the output is not a TTY so we
have to contrive another means of identifying these cases.
---
 ncm-spma/src/main/perl/spma/apt.pm | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/ncm-spma/src/main/perl/spma/apt.pm b/ncm-spma/src/main/perl/spma/apt.pm
index 6557215c36..b9ace7e83a 100644
--- a/ncm-spma/src/main/perl/spma/apt.pm
+++ b/ncm-spma/src/main/perl/spma/apt.pm
@@ -71,12 +71,14 @@ Readonly my $TREE_SOURCES => "/software/repositories";
 Readonly my $TREE_PKGS => "/software/packages";
 Readonly my $BIN_APT_GET => "/usr/bin/apt-get";
 Readonly my $BIN_APT_MARK => "/usr/bin/apt-mark";
+Readonly my $BIN_APT_CACHE => "/usr/bin/apt-cache";
 Readonly my $BIN_DPKG_QUERY => "/usr/bin/dpkg-query";
 Readonly my $CMD_APT_UPDATE => [$BIN_APT_GET, qw(-qq update)];
 Readonly my $CMD_APT_UPGRADE => [$BIN_APT_GET, qw(-qq dist-upgrade)];
 Readonly my $CMD_APT_INSTALL => [$BIN_APT_GET, qw(-qq install)];
 Readonly my $CMD_APT_AUTOREMOVE => [$BIN_APT_GET, qw(-qq autoremove)];
 Readonly my $CMD_APT_MARK => [$BIN_APT_MARK, qw(-qq)];
+Readonly my $CMD_APT_AVAILABLE => [$BIN_APT_CACHE, qw(pkgnames)];
 Readonly my $CMD_DPKG_QUERY => [$BIN_DPKG_QUERY, qw(-W -f=${db:Status-Abbrev};${Package}\n)];
 
 our $NoActionSupported = 1;
@@ -201,6 +203,23 @@ sub get_installed_pkgs
     return Set::Scalar->new(@pkgs);
 }
 
+# Returns a set of all available package names
+sub get_available_pkgs
+{
+    my ($self) = @_;
+    $self->debug(5, 'get_available_pkgs: Called');
+
+    my $out = CAF::Process->new($CMD_APT_AVAILABLE, keeps_state => 1) ->output();
+    my $exitstatus = $? >> 8; # Get exit status from highest 8-bits
+    if ($exitstatus) {
+        $self->debug(5, "dpkg command returned $exitstatus");
+        return 0;
+    }
+    my @pkgs = split("\n", $out);
+
+    return Set::Scalar->new(@pkgs);
+}
+
 # For a given package name, extract version and architecture from tree passed in details
 # returns an arrayref of packages formatted with name, version and architecture for use with apt
 sub get_package_version_arch
@@ -369,13 +388,20 @@ sub Configure
     $self->upgrade_packages() or return 0;
 
     my $packages_installed = $self->get_installed_pkgs() or return 0;
+    my $packages_available = $self->get_available_pkgs() or return 0;
     my $packages_desired = $self->get_desired_pkgs($tree_pkgs) or return 0;
 
     my $packages_unwanted = $packages_installed->difference($packages_desired);
     my $packages_to_install = $packages_desired->difference($packages_installed);
+    my $packages_unavailable = $packages_desired->difference($packages_available);
+
+    if ($packages_unavailable->size > 0) {
+        $self->warn('The following packages are unavailable, they may have been renamed or virtual: ', $packages_unavailable);
+    }
 
     $self->debug(4, 'Installed packages: ', $packages_installed);
     $self->debug(4, 'Desired packages: ', $packages_desired);
+    $self->debug(4, 'Unavailable packages: ', $packages_unavailable);
     $self->debug(4, 'Packages installed but unwanted: ', $packages_unwanted);
     $self->debug(4, 'Packages to install (desired but not installed): ', $packages_to_install);