Hello Andreas,
thanks for working on this patch!
I want to test your patch in Kali and make it ready for inclusion. But
there are things that I don't really understand...
On Wed, 02 Mar 2016, Andreas Henriksson wrote:
> Please note that perl warns about the smartmatch being experimental.
> Someone should probably replace that part with a better check....
So I have read about what smartmatch does... but basically I think it
does not do what you wanted to do. So can you tell me what you wanted
to test?
I assume you wanted to compare both data structures and just know whether
they contain the same thing or not. But the structure is an array of
hashes. So the smart match compares each hash individually, but the hash
comparison only ensures that both hashes have the same keys (and does
not check the values). So basically the smart match will always return
true.
But I also don't understand why you compare the result of two consecutive
get_link_closure() calls. It reads (possibly recursively) the unit file
given in parameter and builds a structure ouf of it. If the underlying
unit file has not changed, the two calls should return the same value...
Even if "systemctl preset" has been called in between, that should not
have affected files in /lib/systemd/system/
So I need some explanations on what you tried to do here...
Still I replaced the smartmatch with a dedicated comparison function
in case you need it:
diff --git a/script/deb-systemd-helper b/script/deb-systemd-helper
index a8aa834..153bed0 100755
--- a/script/deb-systemd-helper
+++ b/script/deb-systemd-helper
@@ -227,6 +227,18 @@ sub is_enabled {
return (@missing_links == 0);
}
+sub are_link_closures_different {
+ my ($list_a, $list_b) = @_;
+ my $len_a = scalar(@$list_a);
+ return 1 if $len_a != scalar(@$list_b);
+ for(my $i = 0; $i < $len_a; $i++) {
+ my ($hash_a, $hash_b) = ($list_a->[$i], $list_b->[$i]);
+ return 1 if $hash_a->{'dest'} ne $hash_b->{'dest'};
+ return 1 if $hash_a->{'src'} ne $hash_b->{'src'};
+ }
+ return 0;
+}
+
sub make_systemd_links {
my ($scriptname, $service_path) = @_;
@@ -251,7 +263,7 @@ sub make_systemd_links {
#$changed_sth = ($prev_enabled != $now_enabled);
# N.B. Trying a "Smart Matching operator" posing like I actually
# know perl or if this even does the right thing.
- $changed_sth = !(@prev_links ~~ @links);
+ $changed_sth = are_link_closures_different(\@prev_links, \@links);
# FIXME: We should really update statefile here when we still
# know something about the changed links instead of badly
# piggy-backing on the fallback which has no idea.
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/
