On Fri, May 17, 2019 at 02:02:24PM +0200, Marc Espie wrote:
> Please try this instead.
>
> What this does:
>
> adds two complementary options to update-plist:
> -c maybe_comment
> -I maybe_ignored
>
> For python, that would be
> UPDATE_PLIST_ARGS += -c MODPY_PYCACHE -I MODPY_COMMENT
>
>
> maybe_ignored is a variable that may expand to nothing
> depending on the flavor, and maybe_comment is a variable
> that may expand to @comment depending on the flavor.
>
> This patch replaces write_restate with a 2-pass algorithm.
> First pass prepares the substituted lines.
>
> Second line does write stuff to the files.
>
> The way the variables work is as follows:
> - first pass counts keyed items for each plist, by making the
> ignored vars vanish, and normalizing directories.
> - second pass notices keyed items that would be duplicated,
> and injects ${MODPY_COMMENT} at the beginning of the line.
>
> 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.
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
+ 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
