On Sun, Apr 06, 2008 at 04:41:50PM +0200, Marc Espie wrote:
> Thinking some more about it, I think there's going to be some other
> refactor.
>
> First, I'm unhappy with how pkg_create's error handling would look, and
> also, I see very similar code in make update-plist.
>
> So, I'm probably going to move the subst stuff into OpenBSD::Subst (or
> something) first, then have CMD_SUBST be a very small script under
> ${PORTSDIR}/infrastructure, as I figure the end user won't want it.
>
> I'm also thinking of having make update-plist registering VAR_SUBST in
> the PLIST `before' substitution (so only update-plist and pkg_create
> would need to deal with that). The idea would be that update-plist would
> remember what variables have already been dealt with, and not reintroduce
> back-substitutions if variables are too common. This is probably utterly
> unclear to almost everyone, except porters who have had update-plist be
> ways too smart in its substitution patterns. ;-)
And here's the new patch, this one just factors out the pkg_create code,
script will happens afterwards.
Index: Makefile
===================================================================
RCS file: /cvs/src/usr.sbin/pkg_add/Makefile,v
retrieving revision 1.48
diff -u -p -r1.48 Makefile
--- Makefile 4 Feb 2008 12:15:03 -0000 1.48
+++ Makefile 6 Apr 2008 15:18:42 -0000
@@ -41,6 +41,7 @@ PACKAGES= \
OpenBSD/Search.pm \
OpenBSD/SharedItems.pm \
OpenBSD/SharedLibs.pm \
+ OpenBSD/Subst.pm \
OpenBSD/Temp.pm \
OpenBSD/Update.pm \
OpenBSD/UpdateSet.pm \
Index: pkg_create
===================================================================
RCS file: /cvs/src/usr.sbin/pkg_add/pkg_create,v
retrieving revision 1.115
diff -u -p -r1.115 pkg_create
--- pkg_create 16 Jun 2007 09:29:37 -0000 1.115
+++ pkg_create 6 Apr 2008 15:18:42 -0000
@@ -27,6 +27,7 @@ use OpenBSD::Error;
use OpenBSD::Ustar;
use OpenBSD::ArcCheck;
use OpenBSD::Paths;
+use OpenBSD::Subst;
use File::Basename;
# Extra stuff needed to archive files
@@ -303,34 +304,7 @@ sub close
package main;
-my %defines;
-
-sub dosubst
-{
- local $_ = shift;
- return $_ unless m/\$/o;
- while (my ($k, $v) = each %defines) {
- s/\$\{\Q$k\E\}/$v/g;
- }
- s/\$\\/\$/go;
- return $_;
-}
-
-sub copy_subst_fh
-{
- my ($srcname, $dest) = @_;
- open my $src, '<', $srcname or die "can't open $srcname";
- local $_;
- while (<$src>) {
- print $dest dosubst($_);
- }
-}
-sub copy_subst
-{
- my ($srcname, $destname) = @_;
- open my $dest, '>', $destname or die "can't open $destname";
- copy_subst_fh($srcname, $dest);
-}
+my $subst = OpenBSD::Subst->new;
our ($opt_p, $opt_f, $opt_c, $opt_d, $opt_v, $opt_i, $opt_k,
$opt_S, $opt_s, $opt_O, $opt_A, $opt_L,
@@ -383,14 +357,10 @@ sub read_fragments
$def = 'SHARED_LIBS';
$frag = 'shared';
}
- if (!defined $defines{$def}) {
- die "Error: unknown fragment
$frag";
- } elsif ($defines{$def} == 1) {
+ if ($subst->has_fragment($def, $frag)) {
next GETLINE if defined $not;
- } elsif ($defines{$def} == 0) {
- next GETLINE unless defined
$not;
} else {
- die "Incorrect define for
$frag";
+ next GETLINE unless defined
$not;
}
my $newname = deduce_name($file->name,
$frag, $not);
if (defined $newname) {
@@ -407,7 +377,7 @@ sub read_fragments
Warn "Shared library without
SHARED_LIBS: $_";
$main::errors++;
}
- &$cont(dosubst($_));
+ &$cont($subst->do($_));
}
}
}
@@ -419,7 +389,7 @@ sub add_special_file
my ($plist, $name, $opt) = @_;
if (defined $opt) {
my $o = OpenBSD::PackingElement::File->add($plist, $name);
- copy_subst($opt, $o->fullname) if defined $o->fullname;
+ $subst->copy($opt, $o->fullname) if defined $o->fullname;
}
}
@@ -427,7 +397,7 @@ sub add_description
{
my ($plist, $name, $opt_c, $opt_d) = @_;
my $o = OpenBSD::PackingElement::FDESC->add($plist, $name);
- my $comment = $defines{COMMENT};
+ my $comment = $subst->value('COMMENT');
if (defined $comment) {
if (length $comment > 60) {
print STDERR "Error: comment is too long\n";
@@ -444,23 +414,23 @@ sub add_description
if (defined $o->fullname) {
open(my $fh, '>', $o->fullname) or die "Can't write to DESC: $!";
if (defined $comment) {
- print $fh dosubst($comment), "\n";
+ print $fh $subst->do($comment), "\n";
} else {
if ($opt_c =~ /^\-(.*)$/o) {
print $fh $1, "\n";
} else {
- copy_subst_fh($opt_c, $fh);
+ $subst->copy_fh($opt_c, $fh);
}
}
if ($opt_d =~ /^\-(.*)$/o) {
print $fh $1, "\n";
} else {
- copy_subst_fh($opt_d, $fh);
+ $subst->copy_fh($opt_d, $fh);
}
if (defined $comment) {
- print $fh "\n", dosubst('Maintainer: ${MAINTAINER}'), "\n";
- if (defined $defines{HOMEPAGE} && $defines{HOMEPAGE} ne '') {
- print $fh "\n", dosubst('WWW: ${HOMEPAGE}'), "\n";
+ print $fh "\n", $subst->do('Maintainer: ${MAINTAINER}'), "\n";
+ if (!$subst->empty('HOMEPAGE')) {
+ print $fh "\n", $subst->do('WWW: ${HOMEPAGE}'), "\n";
}
}
close($fh);
@@ -485,15 +455,7 @@ try {
getopts('p:f:c:d:vi:k:M:U:S:hs:OA:L:B:D:P:W:nqQ',
{'D' =>
sub {
- local $_ = shift;
- if (m/^([^=]+)\=(.*)$/o) {
- my ($k, $v) = ($1, $2);
- $v =~ s/^\'(.*)\'$/$1/;
- $v =~ s/^\"(.*)\"$/$1/;
- $defines{$k} = $v;
- } else {
- $defines{$_} = 1;
- }
+ $subst->parse_option(shift);
},
'f' =>
sub {
@@ -590,9 +552,9 @@ if ($regen_package) {
$pkgname =~ s/\.tgz$//o;
$plist->set_pkgname($pkgname);
}
- my $fullpkgpath = $defines{'FULLPKGPATH'};
- my $cdrom = $defines{'PERMIT_PACKAGE_CDROM'};
- my $ftp = $defines{'PERMIT_PACKAGE_FTP'};
+ my $fullpkgpath = $subst->value('FULLPKGPATH');
+ my $cdrom = $subst->value('PERMIT_PACKAGE_CDROM');
+ my $ftp = $subst->value('PERMIT_PACKAGE_FTP');
if (defined $fullpkgpath && defined $cdrom && defined $ftp) {
$cdrom = 'yes' if $cdrom =~ m/^yes$/io;
$ftp = 'yes' if $ftp =~ m/^yes$/io;
Index: OpenBSD/Subst.pm
===================================================================
RCS file: OpenBSD/Subst.pm
diff -N OpenBSD/Subst.pm
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ OpenBSD/Subst.pm 6 Apr 2008 15:18:42 -0000
@@ -0,0 +1,109 @@
+# ex:ts=8 sw=4:
+# $OpenBSD$
+#
+# Copyright (c) 2008 Marc Espie <[EMAIL PROTECTED]>
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+use strict;
+use warnings;
+
+# very simple package, just holds everything needed for substitution
+# according to package rules.
+
+package OpenBSD::Subst;
+
+sub new
+{
+ bless {}, shift;
+}
+
+sub add
+{
+ my ($self, $k, $v) = @_;
+ $self->{$k} = $v;
+}
+
+sub parse_option
+{
+ my ($self, $opt) = @_;
+ if ($opt =~ m/^([^=]+)\=(.*)$/o) {
+ my ($k, $v) = ($1, $2);
+ $v =~ s/^\'(.*)\'$/$1/;
+ $v =~ s/^\"(.*)\"$/$1/;
+ $self->{$k} = $v;
+ } else {
+ $self->{$opt} = 1;
+ }
+}
+
+sub do
+{
+ my $self = shift;
+ local $_ = shift;
+ return $_ unless m/\$/o; # optimization
+ while (my ($k, $v) = each %$self) {
+ s/\$\{\Q$k\E\}/$v/g;
+ }
+ s/\$\\/\$/go;
+ return $_;
+}
+
+sub copy_fh
+{
+ my ($self, $srcname, $dest) = @_;
+ open my $src, '<', $srcname or die "can't open $srcname";
+ local $_;
+ while (<$src>) {
+ print $dest $self->do($_);
+ }
+}
+
+sub copy
+{
+ my ($self, $srcname, $destname) = @_;
+ open my $dest, '>', $destname or die "can't open $destname";
+ $self->copy_fh($srcname, $dest);
+}
+
+sub has_fragment
+{
+ my ($self, $def, $frag) = @_;
+
+ if (!defined $self->{$def}) {
+ die "Error: unknown fragment $frag";
+ } elsif ($self->{$def} == 1) {
+ return 1;
+ } elsif ($self->{$def} == 0) {
+ return 0;
+ } else {
+ die "Incorrect define for $frag";
+ }
+}
+
+sub value
+{
+ my ($self, $k) = @_;
+ return $self->{$k};
+}
+
+sub empty
+{
+ my ($self, $k) = @_;
+ if (defined $self->{$k} && $self->{$k}) {
+ return 0;
+ } else {
+ return 1;
+ }
+}
+
+1;
