The following patch "removes" pure package additions in pkg_add: instead of having separate code paths for "normal" installs and for "updates" (with extract then install), everything follows the extract then install road.
There are several reasons behind this patch: first, it simplifies the code, and makes it simpler to fix some details later on (in particular, I think there are now conditions where you may end up with a slightly suboptimal /var/db/pkg if you interrupt at the wrong time). Second, it should allow me to do some interesting things with extraction, namely reorder archives wrt packing-lists, which is a desireable property to speed up incremental updates. Index: OpenBSD/Add.pm =================================================================== RCS file: /build/data/openbsd/cvs/src/usr.sbin/pkg_add/OpenBSD/Add.pm,v retrieving revision 1.135 diff -u -p -r1.135 Add.pm --- OpenBSD/Add.pm 5 Jan 2014 10:24:30 -0000 1.135 +++ OpenBSD/Add.pm 5 Jan 2014 10:25:11 -0000 @@ -102,14 +102,21 @@ sub perform_installation { my ($handle, $state) = @_; - $state->{archive} = $handle->{location}; $state->{end_faked} = 0; - $handle->{partial} //= {}; - $state->{partial} = $handle->{partial}; $state->progress->visit_with_size($handle->{plist}, 'install', $state); $handle->{location}->finish_and_close; } +sub perform_extraction +{ + my ($handle, $state) = @_; + + $handle->{partial} = {}; + $state->{partial} = $handle->{partial}; + $state->{archive} = $handle->{location}; + $state->progress->visit_with_size($handle->{plist}, 'extract', $state); +} + my $user_tagged = {}; sub extract_pkgname @@ -174,13 +181,21 @@ sub prepare_for_addition { } +sub extract +{ + my ($self, $state) = @_; + $state->{partial}->{$self} = 1; + if ($state->{interrupted}) { + die "Interrupted"; + } +} + sub install { my ($self, $state) = @_; if ($state->{interrupted}) { die "Interrupted"; } - $state->{partial}->{$self} = 1; } sub copy_info @@ -352,6 +367,7 @@ package OpenBSD::PackingElement::FileBas use OpenBSD::Error; use File::Basename; use File::Path; +use OpenBSD::Temp; sub prepare_for_addition { @@ -375,53 +391,6 @@ sub prepare_for_addition } } -sub install -{ - my ($self, $state) = @_; - $self->SUPER::install($state); - my $fullname = $self->fullname; - my $destdir = $state->{destdir}; - if ($fullname =~ m,^$state->{localbase}/share/doc/pkg-readmes/,) { - $state->{readmes}++; - } - - if ($state->{extracted_first}) { - if ($state->{not}) { - $state->say("moving tempfile -> #1", - $destdir.$fullname) if $state->verbose >= 5; - return; - } - File::Path::mkpath(dirname($destdir.$fullname)); - if (defined $self->{link}) { - link($destdir.$self->{link}, $destdir.$fullname); - } elsif (defined $self->{symlink}) { - symlink($self->{symlink}, $destdir.$fullname); - } else { - rename($self->{tempname}, $destdir.$fullname) or - $state->fatal("can't move #1 to #2: #3", - $self->{tempname}, $fullname, $!); - $state->say("moving #1 -> #2", - $self->{tempname}, $destdir.$fullname) - if $state->verbose >= 5; - undef $self->{tempname}; - } - } else { - my $file = $self->prepare_to_extract($state); - - $state->say("extracting #1", $destdir.$fullname) - if $state->verbose >= 5; - if ($state->{not}) { - $state->{archive}->skip; - return; - } else { - $file->create; - $self->may_check_digest($file, $state); - - } - } - $self->set_modes($state, $destdir.$fullname); -} - sub prepare_to_extract { my ($self, $state) = @_; @@ -467,6 +436,96 @@ sub prepare_to_extract return $file; } +sub extract +{ + my ($self, $state) = @_; + + my $file = $self->prepare_to_extract($state); + + if (defined $self->{link} || defined $self->{symlink}) { + $state->{archive}->skip; + return; + } + + $self->SUPER::extract($state); + + # figure out a safe directory where to put the temp file + my $d = dirname($file->{destdir}.$file->name); + # we go back up until we find an existing directory. + # hopefully this will be on the same file system. + while (!-d $d && -e _ || defined $state->{noshadow}->{$d}) { + $d = dirname($d); + } + if ($state->{not}) { + $state->say("extracting tempfile under #1", $d) + if $state->verbose >= 3; + $state->{archive}->skip; + } else { + if (!-e _) { + File::Path::mkpath($d); + } + my ($fh, $tempname) = OpenBSD::Temp::permanent_file($d, "pkg"); + $self->{tempname} = $tempname; + + # XXX don't apply destdir twice + $file->{destdir} = ''; + $file->set_name($tempname); + + if ($self->{tieto}) { + my $src = $self->{tieto}->realname($state); + unlink($tempname); + $state->say("linking #1 to #2", $src, $tempname) + if $state->verbose >= 3; + if (link($src, $tempname) || + $state->copy_file($src, $tempname)) { + # we still need to adjust properties + $file->set_modes; + $state->{archive}->skip; + return; + } + # okay, it didn't work. recreate tempname. + open $fh, ">", $tempname; + } + + $state->say("extracting #1", $tempname) if $state->verbose >= 3; + + $file->create; + $self->may_check_digest($file, $state); + } +} + +sub install +{ + my ($self, $state) = @_; + $self->SUPER::install($state); + my $fullname = $self->fullname; + my $destdir = $state->{destdir}; + if ($fullname =~ m,^$state->{localbase}/share/doc/pkg-readmes/,) { + $state->{readmes}++; + } + + if ($state->{not}) { + $state->say("moving tempfile -> #1", + $destdir.$fullname) if $state->verbose >= 5; + return; + } + File::Path::mkpath(dirname($destdir.$fullname)); + if (defined $self->{link}) { + link($destdir.$self->{link}, $destdir.$fullname); + } elsif (defined $self->{symlink}) { + symlink($self->{symlink}, $destdir.$fullname); + } else { + rename($self->{tempname}, $destdir.$fullname) or + $state->fatal("can't move #1 to #2: #3", + $self->{tempname}, $fullname, $!); + $state->say("moving #1 -> #2", + $self->{tempname}, $destdir.$fullname) + if $state->verbose >= 5; + undef $self->{tempname}; + } + $self->set_modes($state, $destdir.$fullname); +} + package OpenBSD::PackingElement::RcScript; sub install { @@ -512,6 +571,10 @@ sub prepare_for_addition } } +sub extract +{ +} + sub install { my ($self, $state) = @_; @@ -556,6 +619,9 @@ sub install } package OpenBSD::PackingElement::Sampledir; +sub extract +{ +} sub install { @@ -618,6 +684,20 @@ sub install } package OpenBSD::PackingElement::Dir; +sub extract +{ + my ($self, $state) = @_; + my $fullname = $self->fullname; + my $destdir = $state->{destdir}; + + return if -e $destdir.$fullname; + $self->SUPER::extract($state); + $state->say("new directory #1", $destdir.$fullname) + if $state->verbose >= 3; + return if $state->{not}; + File::Path::mkpath($destdir.$fullname); +} + sub install { my ($self, $state) = @_; Index: OpenBSD/PkgAdd.pm =================================================================== RCS file: /build/data/openbsd/cvs/src/usr.sbin/pkg_add/OpenBSD/PkgAdd.pm,v retrieving revision 1.43 diff -u -p -r1.43 PkgAdd.pm --- OpenBSD/PkgAdd.pm 4 Jan 2014 14:13:05 -0000 1.43 +++ OpenBSD/PkgAdd.pm 5 Jan 2014 02:49:32 -0000 @@ -740,42 +740,39 @@ sub really_add OpenBSD::OldLibs->save($set, $state); } - if ($replacing && !$state->{delete_first}) { - $state->{extracted_first} = 1; - for my $handle ($set->newer) { - next if $state->{size_only}; - $set->setup_header($state, $handle, "extracting"); - - try { - OpenBSD::Replace::perform_extraction($handle, - $state); - } catchall { - unless ($state->{interrupted}) { - $state->errsay($_); - $errors++; - } - }; - if ($state->{interrupted} || $errors) { - $state->fatal(partial_install("Installation of ". - $handle->pkgname." failed", $set, $state)); + if ($state->{delete_first}) { + delete_old_packages($set, $state); + } + + for my $handle ($set->newer) { + next if $state->{size_only}; + $set->setup_header($state, $handle, "extracting"); + + try { + OpenBSD::Add::perform_extraction($handle, + $state); + } catchall { + unless ($state->{interrupted}) { + $state->errsay($_); + $errors++; } + }; + if ($state->{interrupted} || $errors) { + $state->fatal(partial_install("Installation of ". + $handle->pkgname." failed", $set, $state)); } - } else { - $state->{extracted_first} = 0; } - - if ($replacing) { + if (!$state->{delete_first}) { delete_old_packages($set, $state); } iterate($set->newer, sub { return if $state->{size_only}; my $handle = shift; - my $pkgname = $handle->pkgname; my $plist = $handle->plist; - $set->setup_header($state, $handle, - $replacing ? "installing" : undef); + + $set->setup_header($state, $handle, "installing"); $state->set_name_from_handle($handle, '+'); try { @@ -787,6 +784,7 @@ sub really_add } }; + unlink($plist->infodir.CONTENTS); if ($state->{interrupted} || $errors) { $state->fatal(partial_install("Installation of $pkgname failed", Index: OpenBSD/Replace.pm =================================================================== RCS file: /build/data/openbsd/cvs/src/usr.sbin/pkg_add/OpenBSD/Replace.pm,v retrieving revision 1.84 diff -u -p -r1.84 Replace.pm --- OpenBSD/Replace.pm 28 Apr 2012 12:00:10 -0000 1.84 +++ OpenBSD/Replace.pm 5 Jan 2014 02:41:48 -0000 @@ -33,102 +33,6 @@ sub can_update sub update_issue { undef } -sub extract -{ - my ($self, $state) = @_; - $state->{partial}->{$self} = 1; - if ($state->{interrupted}) { - die "Interrupted"; - } -} - -package OpenBSD::PackingElement::FileBase; -use OpenBSD::Temp; - -sub extract -{ - my ($self, $state) = @_; - - my $file = $self->prepare_to_extract($state); - - if (defined $self->{link} || defined $self->{symlink}) { - $state->{archive}->skip; - return; - } - - $self->SUPER::extract($state); - - # figure out a safe directory where to put the temp file - my $d = dirname($file->{destdir}.$file->name); - # we go back up until we find an existing directory. - # hopefully this will be on the same file system. - while (!-d $d && -e _ || defined $state->{noshadow}->{$d}) { - $d = dirname($d); - } - if ($state->{not}) { - $state->say("extracting tempfile under #1", $d) - if $state->verbose >= 3; - $state->{archive}->skip; - } else { - if (!-e _) { - File::Path::mkpath($d); - } - my ($fh, $tempname) = OpenBSD::Temp::permanent_file($d, "pkg"); - $self->{tempname} = $tempname; - - # XXX don't apply destdir twice - $file->{destdir} = ''; - $file->set_name($tempname); - - if ($self->{tieto}) { - my $src = $self->{tieto}->realname($state); - unlink($tempname); - $state->say("linking #1 to #2", $src, $tempname) - if $state->verbose >= 3; - if (link($src, $tempname) || - $state->copy_file($src, $tempname)) { - # we still need to adjust properties - $file->set_modes; - $state->{archive}->skip; - return; - } - # okay, it didn't work. recreate tempname. - open $fh, ">", $tempname; - } - - $state->say("extracting #1", $tempname) if $state->verbose >= 3; - - $file->create; - $self->may_check_digest($file, $state); - } -} - -package OpenBSD::PackingElement::Dir; -sub extract -{ - my ($self, $state) = @_; - my $fullname = $self->fullname; - my $destdir = $state->{destdir}; - - return if -e $destdir.$fullname; - $self->SUPER::extract($state); - $state->say("new directory #1", $destdir.$fullname) - if $state->verbose >= 3; - return if $state->{not}; - File::Path::mkpath($destdir.$fullname); -} - - -package OpenBSD::PackingElement::Sample; -sub extract -{ -} - -package OpenBSD::PackingElement::Sampledir; -sub extract -{ -} - package OpenBSD::PackingElement::Exec; sub update_issue { @@ -155,16 +59,6 @@ sub update_issue { undef } package OpenBSD::Replace; -sub perform_extraction -{ - my ($handle, $state) = @_; - - $handle->{partial} = {}; - $state->{partial} = $handle->{partial}; - $state->{archive} = $handle->{location}; - $state->progress->visit_with_size($handle->{plist}, 'extract', $state); -} - sub check_plist_exec { my ($plist, $state, $new) = @_; @@ -231,6 +125,5 @@ sub is_set_safe return 0; } } - 1;