I've conducted a quick audit of the 300 or so uses of @extraunexec
in pkg_add.
All but 5 are actually glorified versions of
rm -rf <globbing pattern>
I propose eventually replacing them with
@extraglob pattern
with several advantages:
- we have file names that we know
- perl itself can expand the glob and decide accordingly to delete
or not, warn or not.
In general, stuff like @extra, @sample should require running (if
necessary) near the end. There were several things simple-minded
with the current approach.
- extra would spew out messages during replacement which is not
always appropriate.
- I need to look again at @sample and sysmerge, it should be possible
to provide a 3-way merge in case things changes.
- more generally, the whole shebang dates back to a simple era, where
adding/deleting packages were atomic operations. @extra actually
needs to take the update into account.
I have a first patch for @extra that takes updates into account:
this will remove spurious deinstallation messages coming from @extra.
The idea is that we register @extra globally during an update/deletion,
so that we only warn about @extra that actually got removed.
(there is more text after this). This patch is currently being tested
with no ill effect, and will probably be committed shortly.
Index: OpenBSD/Delete.pm
===================================================================
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/Delete.pm,v
retrieving revision 1.164
diff -u -p -r1.164 Delete.pm
--- OpenBSD/Delete.pm 6 Jun 2022 07:57:21 -0000 1.164
+++ OpenBSD/Delete.pm 6 Jun 2022 10:36:24 -0000
@@ -645,16 +645,14 @@ use File::Basename;
sub delete
{
my ($self, $state) = @_;
+ return if defined $state->{current_set}{known_extra}{$self->fullname};
my $realname = $self->realname($state);
if ($state->verbose >= 2 && $state->{extra}) {
$state->say("deleting extra file: #1", $realname);
}
return if $state->{not};
return unless -e $realname or -l $realname;
- if ($state->replacing) {
- $state->log("Remember to update #1", $realname);
- $self->mark_dir($state);
- } elsif ($state->{extra}) {
+ if ($state->{extra}) {
unlink($realname) or
$state->say("problem deleting extra file #1: #2",
$realname, $!);
} else {
@@ -669,8 +667,8 @@ sub delete
{
my ($self, $state) = @_;
return unless $state->{extra};
+ return if defined $state->{current_set}{known_extra}{$self->fullname};
my $realname = $self->realname($state);
- return if $state->replacing;
if ($state->{extra}) {
$self->SUPER::delete($state);
} else {
One reason for moving @extra handling near the end is the actual
usage: @extra (and @extraglob) allow registering package generated
data (or user files) that are not part of the package. Putting this
*after* running tags but *before* removing dirs means we can register
files generated by tags in there! stuff like pkg_check would know
they exist in the file system, but they would (hopefully) have actually
vanished during the end run.
As for @extraunexec, there are two approaches:
- I could "parse" the @extraunexec line manually and replace it
(internally) with a globbing approach. While possible (this is indeed
what I did for the audit) this is brittle. There are already somewat
gratuitous variations of
rm -rf, rm -fr, rm -r 2>/dev/null || true
in the tree. It will take only so long until one pattern is not
recognized and handled the same.
I propose instead a new keyword @extraglob which would replace most
of the rm -rf patterns.
Semantics is not yet fully decided, but I can add the code to
recognize it right now, so that when it becomes actual, most people
already have the keyword in their version of pkg_add.
Index: OpenBSD/PackingElement.pm
===================================================================
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/PackingElement.pm,v
retrieving revision 1.281
diff -u -p -r1.281 PackingElement.pm
--- OpenBSD/PackingElement.pm 26 May 2022 21:08:52 -0000 1.281
+++ OpenBSD/PackingElement.pm 7 Jun 2022 07:36:13 -0000
@@ -1817,6 +1817,13 @@ sub destate
&OpenBSD::PackingElement::Extra::destate;
}
+package OpenBSD::PackingElement::ExtraGlob;
+our @ISA=qw(OpenBSD::PackingElement::FileObject);
+
+sub keyword() { 'extraglob' }
+sub absolute_okay() { 1 }
+__PACKAGE__->register_with_factory;
+
package OpenBSD::PackingElement::SpecialFile;
our @ISA=qw(OpenBSD::PackingElement::Unique);