On Sat, May 18, 2019 at 12:49:49PM +0200, Marc Espie wrote:
> On Fri, May 17, 2019 at 02:02:24PM +0200, Marc Espie wrote:
> > Please try this instead.
> >
<SNIP>
> >
> > Yes, it's waaay more intricate than what you suggested,
> > but it could be used in other scenarios instead of being
> > python-only.
> >
> > Of course, this does require testing. I may have made some
> > stupid mistake somewhere.
>
> And here is the version without stupid mistakes, along with the manpage
> change, explicitly describing what's going on (somewhat long-winded, but
> I think this required an example :) )
>
> I intend to commit this unless there are objections.
This seems to work well for me, rather annoying that it's so much code
but I like it a lot better than the dirty hack :-)
Tested updating as well as generating a new PLIST seems to work well and
produce what I expect, although I don't do a lot of python ports.
There is some weird whitespace in the diff, not not just spaces at the
end of the line, but also some spaces before a tab. Unfortunately I'm
not familiar enough with the datastructures in-use to positively say
nothing is for sure wrong but I didn't spot anything obvious.
OK afresh1@
> Ports diff:
> ----------
> Index: infrastructure/bin/update-plist
> ===================================================================
> RCS file: /cvs/ports/infrastructure/bin/update-plist,v
> retrieving revision 1.173
> diff -u -p -r1.173 update-plist
> --- infrastructure/bin/update-plist 17 May 2019 10:24:18 -0000 1.173
> +++ infrastructure/bin/update-plist 18 May 2019 10:21:46 -0000
> @@ -52,7 +52,7 @@ package TrackedFile;
> sub new
> {
> my ($class, $name, $ext) = @_;
> - bless {name => $name, ext => $ext, items => []}, $class;
> + bless {name => $name, ext => $ext, items => [], items2 => []}, $class;
> }
>
> sub add
> @@ -61,6 +61,28 @@ sub add
> push(@{$self->{items}}, $item);
> }
>
> +sub add2
> +{
> + my ($self, $item, $p) = @_;
> +
> + if ($item->NoDuplicateNames) {
> + my $s = $p->subst->remove_ignored_vars($item->{prepared});
> + my $s2 = $p->subst->do($s);
> + if (defined (my $k = $item->keyword)) {
> + $s2 =~ s/^\@\Q$k\E\s//;
> + }
> + $p->{stash}{$s2}++;
> +
> + my $comment = $p->subst->{maybe_comment};
> +
> + if ($s ne $item->{prepared} &&
> + $item->{prepared} !~ m/^\Q$comment\E/) {
> + $item->{candidate_for_comment} = $s2;
> + }
> + }
> + push(@{$self->{items2}}, $item);
> +}
> +
> sub fh
> {
> my $self = shift;
> @@ -87,6 +109,16 @@ sub next_item
> }
> }
>
> +sub next_item2
> +{
> + my $self = shift;
> + if (@{$self->{items2}} != 0) {
> + return shift @{$self->{items2}};
> + } else {
> + return undef;
> + }
> +}
> +
> package TrackFile;
>
> sub new
> @@ -125,6 +157,25 @@ sub write_all
>
> while (my $file = pop @stack) {
> while (my $j = $file->next_item) {
> + my $filename = $j->prepare_restate($file, $p);
> + if (defined $filename) {
> + push(@stack, $file);
> + $file = $self->file($filename);
> + }
> + }
> + }
> + }
> +
> + for my $i (@{$p->{base_plists}}) {
> + # we mimic the way pkg_create writes files
> + $p->{restate} = {};
> +
> + my @stack = ();
> + push(@stack, $self->file($i));
> +
> +
> + while (my $file = pop @stack) {
> + while (my $j = $file->next_item2) {
> my $filename = $j->write_restate($file, $p);
> if (defined $filename) {
> push(@stack, $file);
> @@ -390,12 +441,33 @@ sub write_restate
> return undef;
> }
>
> +sub prepare_restate
> +{
> + my ($o, $file, $p) = @_;
> + $o->prepare_backsubst($file, $p);
> + return undef;
> +}
> +
> +sub prepare_backsubst
> +{
> + my ($o, $file, $p) = @_;
> + my $s = $p->subst->do_backsubst($o->fullstring, $o->unsubst, $o);
> + $o->{prepared} = $s;
> + $file->add2($o, $p);
> +}
> +
> # default backsubstitution and writing.
> sub write_backsubst
> {
> my ($o, $file, $p) = @_;
> - my $s = $p->subst->do_backsubst($o->fullstring, $o->unsubst, $o);
> - print {$file->fh} $s, "\n";
> +
> + if (defined (my $s = $o->{candidate_for_comment})) {
> + if ($p->{stash}{$s} > 1) {
> + $o->{prepared} =
> + $p->subst->{maybe_comment}.$o->{prepared};
> + }
> + }
> + print {$file->fh} $o->{prepared}, "\n";
> }
>
> # extra objects that get copied very late (e.g., @extra)
> @@ -665,10 +737,11 @@ sub tie_objects
> }
>
> # we will never do backsubst on CVSTags
> -sub write_backsubst
> +sub prepare_backsubst
> {
> my ($o, $file, $p) = @_;
> - $o->write($file->fh);
> + $o->{prepared} = $o->fullstring;
> + $file->add2($o, $p);
> }
>
> # this is extra stuff that PkgCreate builds but that we don't want to copy
> @@ -694,6 +767,10 @@ sub write_restate
> {
> }
>
> +sub prepare_restate
> +{
> +}
> +
> package OpenBSD::PackingElement::ExtraInfo;
> sub copy_annotations
> {
> @@ -889,6 +966,19 @@ sub write_restate
> return undef;
> }
>
> +sub prepare_restate
> +{
> + my ($self, $file, $p) = @_;
> + # don't do backsubst on fragments, pkg_create does not!
> + $file->add2($self, $p);
> + my $base = $file->name;
> + my $frag = $self->frag;
> + $base =~ s/PFRAG\./PFRAG.$frag-/ or
> + $base =~ s/PLIST/PFRAG.$frag/;
> + return $base if $p->{tracker}{known}{$base};
> + return undef;
> +}
> +
> sub frag
> {
> my $self = shift;
> @@ -1063,7 +1153,7 @@ sub check_lib_version
> }
> }
>
> -sub write_backsubst
> +sub prepare_backsubst
> {
> my ($self, $f, $p) = @_;
> if ($self->name =~ m,^(.*?)lib([^\/]+)\.so\.(\d+\.\d+)$,) {
> @@ -1074,7 +1164,8 @@ sub write_backsubst
> "\@lib ${path}lib$name.so.\$\{$k\}", $self->unsubst, $self);
> $self->check_lib_version($version, $name,
> $p->subst->value($k));
> - print {$f->fh} "$s\n";
> + $self->{prepared} = $s;
> + $f->add2($self, $p);
> } else {
> $self->SUPER::write_backsubst($f, $p);
> }
> @@ -1229,6 +1320,17 @@ sub handle_options
> my $var = shift;
> push(@{$state->{dont_backsubst}}, $var);
> },
> + 'I' => sub {
> + my $var = shift;
> + push(@{$state->{maybe_ignored}}, $var);
> + },
> + 'c' => sub {
> + my $var = shift;
> + if (exists $state->{maybe_comment}) {
> + $state->usage;
> + }
> + $state->{maybe_comment} = '${'.$var.'}';
> + },
> 's' => sub {
> my $var = shift;
> push(@{$state->{start_only}}, $var);
> @@ -1243,10 +1345,10 @@ sub handle_options
> },
>
> };
> - $state->SUPER::handle_options('rvqV:FC:i:j:L:s:S:X:P:w:e:E:',
> - '[-Fmnrvx] [-C dir] [-E ext] [-e ext] [-i var] [-j jobs]',
> - '[-L logfile] [-P pkgdir] [-S var] [-s var] [-V var]',
> - '[-w suffix] [-X path] -- pkg_create_args ...');
> + $state->SUPER::handle_options('rvI:c:qV:FC:i:j:L:s:S:X:P:w:e:E:',
> + '[-Fmnrvx] [-C dir] [-c comment] [-E ext] [-e ext] [-i var]',
> + '[-I ignored] [-j jobs] [-L logfile] [-P pkgdir] [-S var]',
> + '[-s var] [-V var] [-w suffix] [-X path] -- pkg_create_args ...');
> $state->{pkgdir} = $state->opt('P');
> $state->{scan_as_root} = $state->opt('r');
> $state->{verbose} = $state->opt('v');
> @@ -1255,6 +1357,9 @@ sub handle_options
> $state->{extnew} = $state->opt('E') // ".new";
> $state->{extorig} = $state->opt('e') // ".orig";
> $state->{logfile} = $state->opt('L');
> + if (exists $state->{maybe_ignored} && !exists $state->{maybe_comment}) {
> + $state->usage;
> + }
> }
>
> package UpdatePlist;
> Index: infrastructure/lib/OpenBSD/ReverseSubst.pm
> ===================================================================
> RCS file: /cvs/ports/infrastructure/lib/OpenBSD/ReverseSubst.pm,v
> retrieving revision 1.19
> diff -u -p -r1.19 ReverseSubst.pm
> --- infrastructure/lib/OpenBSD/ReverseSubst.pm 17 May 2019 10:04:13
> -0000 1.19
> +++ infrastructure/lib/OpenBSD/ReverseSubst.pm 18 May 2019 10:21:46
> -0000
> @@ -175,8 +175,12 @@ sub new
> isversion => {},
> # under some cases, some variables are a priority
> disregard_count => {},
> + # to be able to inject @comment conditionally on some other
> + # variables
> + maybe_comment => $state->{maybe_comment},
> }, $class;
> - for my $k (qw(dont_backsubst start_only suffix_only no_version)) {
> + for my $k (qw(dont_backsubst start_only suffix_only no_version
weird whitespace here
> + maybe_ignored)) {
> if (defined $state->{$k}) {
> for my $v (@{$state->{$k}}) {
> $o->{$k}{$v} = 1;
> @@ -184,6 +188,16 @@ sub new
> }
> }
> return $o;
> +}
> +
> +sub remove_ignored_vars
> +{
> + my ($self, $s) = @_;
> + for my $v (keys %{$self->{maybe_ignored}}) {
> + while ($s =~ s/\$\{\Q$v\E\}//) {}
> + }
> + $s =~ s,//,/,g;
> + return $s;
> }
>
> # those are actually just passed thru to pkg_create for special
>
>
>
>
>
> Source diff:
> -----------
> Index: update-plist.1
> ===================================================================
> RCS file: /cvs/src/share/man/man1/update-plist.1,v
> retrieving revision 1.2
> diff -u -p -r1.2 update-plist.1
> --- update-plist.1 26 Jun 2018 06:56:07 -0000 1.2
> +++ update-plist.1 18 May 2019 10:48:13 -0000
> @@ -26,8 +26,10 @@
> .Nm
> .Op Fl Fmnqrvx
> .Op Fl C Ar dir
> +.Op Fl c Ar var
> .Op Fl E Ar ext
> .Op Fl e Ar ext
> +.Op Fl I Ar var
> .Op Fl i Ar var
> .Op Fl j Ar jobs
> .Op Fl S Ar var
> @@ -83,6 +85,12 @@ Beware that this directory should then b
> dependencies's packing-lists change.
> But this will speed up packing-list regeneration for ports with
> lots of dependencies significantly.
> +.It Fl c Ar var
> +Variable
> +.Ar var
> +may be used as a way to insert
> +.Cm \@ Ns comment
> +when other variables vanish.
> .It Fl E Ar ext
> Write new files with
> .Ar ext
> @@ -96,6 +104,12 @@ extension instead of the default
> .It Fl F
> Do not try to run
> .Xr pkg_locate 1 .
> +.It Fl I Ar var
> +Variable
> +.Ar var
> +may expand to nothing, in which case the variable from
> +.Fl c Ar var
> +can be used to prevent duplicate entries.
> .It Fl i Ar var
> Ignore variable
> .Ar var
> @@ -214,6 +228,59 @@ and it currently does not add new substi
> .Ev ARCH
> nor
> .Ev MACHINE_ARCH .
> +.Pp
> +Some packages (notoriously Python packages) create some directories
> optionally
> +based on flavors.
> +Options
> +.Fl c
> +and
> +.Fl I
> +can be used to avoid duplicate directory definitions.
> +.Pp
> +Specifically, a generated packing-list would contain
> +.Bd -literal -offset indent -compact
> +lib/python${MODPY_VERSION}/site-packages/bpdb/
> +lib/python${MODPY_VERSION}/site-packages/bpdb/${MODPY_PYCACHE}/
> +.Ed
> +which expands to
> +.Bd -literal -offset indent -compact
> +lib/python3.7/site-packages/bpdb/
> +lib/python3.7/site-packages/bpdb/__pycache__/
> +.Ed
> +for python3, which is fine.
> +.Pp
> +But for python2, variable
> +.Sq MODPY_PYCACHE
> +will be empty, resulting in
> +.Bd -literal -offset indent -compact
> +lib/python2.7/site-packages/bpdb/
> +lib/python2.7/site-packages/bpdb/
> +.Ed
> +thus a duplicate directory,
> +.Xr pkg_create 1
> +won't be happy.
> +.Pp
> +Using
> +.Ev UPDATE_PLIST_ARGS = Fl c Ar MODPY_COMMENT Fl I Ar MODPY_PYCACHE
> +will result in injecting
> +.Sq ${MODPY_COMMENT}
> +whereever
> +.Nm
> +finds a duplicate directory by replacing
> +.Sq ${MODPY_PYCACHE}
> +with nothing.
> +.Pp
> +This yields
> +.Bd -literal -offset indent -compact
> +lib/python${MODPY_VERSION}/site-packages/bpdb/
> +${MODPY_COMMENT}lib/python${MODPY_VERSION}/site-packages/bpdb/MODPY_PYCACHE}/
> +.Ed
> +and for python2 this expands to
> +.Bd -literal -offset indent -compact
> +lib/python2.7/site-packages/bpdb/
> +@comment lib/python2.7/site-packages/bpdb/
> +.Ed
> +which is exactly what we want.
> .Pp
> Specific items such as shared libraries or binaries will gain annotations
> and special handling, for instance
--
andrew - http://afresh1.com
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the Universe
trying to produce bigger and better idiots. So far, the Universe is
winning." -- Rich Cook
