Reini Urban schrieb:
2008/8/24 Allison Randal <[EMAIL PROTECTED]>:
Reini Urban wrote:
You want one patch only against HEAD? That's easy.
But I dislike the idea, as it violates the usage of single tickets.
This is different than the usual case as it's a collection of dependent
patches that can't be evaluated independently. Splitting them out is
actually more work for the reviewer/tester.
I've created a branch cygwin070patches for testing this collection. To do
platform or language testing, please check out that branch. Reini, please
submit further changes as diffs against that branch instead of updates to
your previous patch files.
Overall the work is sane. It'll need a few changes before merging in:
In runtime/parrot/library/config.pir you add commented-out code, and a
mention that certain logic has to be reversed "when installed versions
should run faster than source builds". Those sorts of configuration changes
should never involve commenting out and uncommenting bits of configuration
files. Make it a compile-time or run-time configuration flag instead.
Just was just an internal TODO comment, and is actually fixed. I will
delete the superflous comment.
In lib/Parrot/Configure/Data.pm, you changed some double C<<>> Pod tags to
single C<> Pod tags. But, those code items contain "=>" separating the
key/value arguments, and the '>' in the arrow will terminate the code tag.
The double C<<>> tags avoid terminating on single '>' (which is why they
were double C<<>> tags in the first place). So, I reverted that file before
committing. (See the output from 'perldoc' or any Pod parser on your
modified file.)
Thanks.
In lib/Parrot/Configure/Compiler.pm, I agree that 'CONDITIONED_LINE' and
'INVERSE_CONDITIONED_LINE' aren't the clearest names, but '+' and '-' are
far less clear. Change them to something meaningful like 'SHOW_LINE_IF' and
'HIDE_LINE_IF'. We can note the change in DEPRECATED.pod now, and remove
'CONDITIONED_LINE' and 'INVERSE_CONDITIONED_LINE' after a standard
deprecation cycle (one release).
#+ and #- are well known common lisp idioms (the most well-known
reader-macros),
thats why the reverse polish notation in the logic is used. It's just
so much easier to parse and understand.
Also in lib/Parrot/Configure/Compiler.pm, change the Polish notation of
"(and a b (not c d))" to a saner "(a and b not (c and d))".
This is ALGOL syntax you suggest is really not "sane" :)
To clarify my bold statement:
The ALGOL-like syntax is not "sane" because,
* it is hard to parse,
* it forbids our keywords AND, NOT and OR as config_hash keys
and platform names. With lisp syntax there's a clear distinction
between operator and arguments, with mixed arg op arg syntax
it is not clear and as hard to understand as spoken language.
#+(and foo (not bar)) is a well-established reader-macro syntax in
lisp and a general life-safer in lisp, compared to less developed languages.
Though really,
since you're not implementing the advanced conditions, delete the comment
defining the interface and the TODO comments about implementing it, just add
a TODO RT ticket or keep it on your private TODO list.
I finished now the implementation of this new feature.
I'll implement it for sure, but later. First I wanted to bring out the
packages and
this is just convenience sugar.
In config/gen/makefiles/pge.in, config/gen/makefiles/tge.in, and
config/gen/makefiles/root.in, pick a more meaningful variable name than
'SHRPENV' and a more meaningful condition name than 'cygchkdll'.
SHRPENV is directly from per5, and cygchkdll is they only way to check
now for cygwin
without the 57548-CONDITIONED_LINE_enh.patch. After that we can check against
cygwin, the $^O. I really need some #+(and cygwin win32): sections.
Also in config/gen/makefiles/root.in we don't need a makefile target for
regenerating the makefile. Delete it. And you added a chunk of commented out
code again. Delete it.
That was the framework to autodetect configuration changes, to get rid of
make clean before perl Configure.pl.
I do need to regenerate the makefile, that's why I added it for every makefile.
I'll rather add the missing bits to the missings deps, than delete this logic.
The commented-out section works in 90%, but has still a flaw somewhere,
which could lead to cycles. I'll debug this when I have more time.
Since you've touched the config file and core PIR file for every language,
this branch will need extensive platform and language testing before it can
be merged in.
I tested it extensively for the cygwin package. I believe I'm the only one who
tested it at all because there were so many errors and still are.
And I begin to wonder how others dare to release packages without working
installables and with self.build make recipes.
deb, gentoo, freebsd, rpm, the win32 build, and now my self-build work, but
at least as Makefiles so we will have a standard.
Just test-installable is a bit of work TODO. I had to test this by hand so far.
The branch is failing one test that passes in trunk, should be a quick fix:
t/codingstd/cuddled_else.t
Thanks for reviewing!
Attached are updates to the cygwin070patches branch.
Thanks for applying the patches!
--
Reini Urban
http://phpwiki.org/ http://murbreak.at/
Index: lib/Parrot/Configure/Data.pm
===================================================================
--- lib/Parrot/Configure/Data.pm (revision 30541)
+++ lib/Parrot/Configure/Data.pm (working copy)
@@ -125,7 +125,7 @@
return @[EMAIL PROTECTED];
}
-=item * C<< set($key => $val, ...) >>
+=item * C<set($key => $val, ...)>
=over 4
@@ -171,7 +171,7 @@
return $self;
}
-=item * C<< add($delim, $key => $val, ...) >>
+=item * C<add($delim, $key => $val, ...)>
=over 4
@@ -588,7 +588,7 @@
return @[EMAIL PROTECTED];
}
-=item * C<< set_p5($key => $val, ...) >>
+=item * C<set_p5($key => $val, ...)>
=over 4
Index: lib/Parrot/Configure/Compiler.pm
===================================================================
--- lib/Parrot/Configure/Compiler.pm (revision 30541)
+++ lib/Parrot/Configure/Compiler.pm (working copy)
@@ -189,23 +189,25 @@
If the name of the file being generated ends in C<Makefile>, this option
defaults to true.
-=item conditioned_lines
+=item conditioned_lines #+(): #-():
If conditioned_lines is true, then lines in the file that begin with
-C<#+(var):> are skipped if the var condition is false.
-Lines that begin with C<#-(var):> are skipped if the var condition is true.
-For legacy the old syntax #CONDITIONED_LINE(var): and #INVERSE_CONDITIONED_LINE(var):
-is also supported.
+C<#+(expr):> are skipped if the expr condition is false.
+Lines that begin with C<#-(var):> are skipped if the expr condition is true.
+For legacy the old syntax #CONDITIONED_LINE(var): and
+#INVERSE_CONDITIONED_LINE(var): is also supported.
-A condition var may be a single keyword, which is true if a config key is true
-or equal to the platform name,
-or a logical combination or (and var1 var2...) or (or var1 var2...) or (not var1...)
-as in the common lisp reader, with OR being the default for multiple keys.
-Keys are space seperated.
-Keys may also consist of key=value pairs, where the key is checked for equalness
-to the value. Note that values may contain no spaces here. (TODO: support quotes
-in values)
+A condition expr may be a single keyword, which is true if a config key is true
+or equal to the platform name (case-sensitive),
+or a logical combination of AND, OR and NOT in case-insensitive LISP syntax
+ like (AND var1 var2...) or (OR var1 var2...) or (NOT var1...)
+ as in the common lisp reader, 'OR' being the default for multiple keys.
+Multiple keys are space seperated.
+Keys may also consist of key=value pairs, where the key is checked for
+equalness to the value. Note that values may contain no spaces here.
+TODO: support quotes in values
+
#+(var1 var2...) defaults to #+(or var1 var2...)
#-(var1 var2...) defaults to #-(or var1 var2...)
@@ -352,7 +354,7 @@
last;
}
if ( $options{conditioned_lines} ) {
- # allow nested parens here
+ # allow multiple keys and nested parens here
if ( $line =~ m/^#([-+])\((.+)\):(.*)/s ) {
my $truth = cond_eval($conf, $2);
next if ($1 eq '-') and $truth;
@@ -465,16 +467,61 @@
move_if_diff( "$target.tmp", $target, $options{ignore_pattern} );
}
-# Just checks the logical truth of the hash value (exists and not empty)
-# TODO: also check the platform name if the hash key does not exist
-# TODO: recursive (), evaluate AND, OR, NOT with multiple keys
-# TODO: check for key=value, like #+(ld=gcc)
+# Return the next subexpression from the expression in $_[0] and
+# remove it from the input expression.
+# E.g. "and (not win32) has_glut"
+# => and => (not win32) => has_glut
+sub next_expr {
+ my $s = $_[0];
+ return "" unless $s;
+ if ($s =~ /^\((.+)\)\s*(.*)/) { # longest match to matching closing paren
+ $_[0] = $2 ? $2 : ""; # modify the 2nd arg
+ #print "** \"$s\" => (\"$1\",\"$_[0]\")\n";
+ return $1;
+ } else {
+ $s =~ s/^\s+//; # left-trim to make it more robust
+ $s =~ m/^([^ \(]+)\s*(.*)?/; # shortest match to next whitespace or open paren
+ $_[0] = $2 ? $2 : ""; # modify the 2nd arg
+ #print "** \"$s\" => (\"$1\",\"$_[0]\")\n";
+ return $1;
+ }
+}
+
+# Recursively evaluate AND, OR, NOT for multiple keys as LISP expressions.
+# Checks the logical truth of the hash value: exists and not empty.
+# Also check the platform name if the hash key does not exist.
+# Also check for key=value, like #+(ld=gcc)
sub cond_eval {
- my $conf = shift;
- my $expr = shift;
- # TODO: parse for parens and multiple keys in the expression and
- # evaluate it recursively.
- return $conf->data->get($expr);
+ my $conf = $_[0];
+ my $expr = $_[1];
+ my $key = $expr;
+ my @count = split /\s+/, $expr;
+ if (@count > 1) { # multiple keys: recurse into
+ my $truth;
+ my $op = next_expr($expr);
+ if ($op =~ /^(or|and|not)$/i) {
+ $op = lc($op);
+ $key = next_expr($expr);
+ } else {
+ $key = $op;
+ $op = 'or';
+ }
+ while ($key) {
+ last if $truth and ($op eq 'or'); # logical shortcut on OR and already $truth
+ $truth = cond_eval($conf, $key);
+ if ($op eq 'not') { $truth = $truth ? 0 : 1; }
+ elsif ($op eq 'and') { last unless $truth; } # skip on early fail
+ $key = next_expr($expr);
+ }
+ return $truth;
+ }
+ if ($key =~ /^(\w+)=(.+)$/) {
+ return $conf->data->get($1) eq $2;
+ } else {
+ return exists($conf->data->{c}->{$key})
+ ? $conf->data()->get($key)
+ : $key eq $^O;
+ }
}
sub append_configure_log {
Index: t/steps/gen_makefiles-01.t
===================================================================
--- t/steps/gen_makefiles-01.t (revision 30541)
+++ t/steps/gen_makefiles-01.t (working copy)
@@ -5,9 +5,42 @@
use strict;
use warnings;
-use Test::More tests => 7;
+my @cond_tests;
+my @conf_args = ( dummy1 => 1, dummy2 => 0, dummy3 => 'xx' );
+BEGIN {
+ @cond_tests =
+ (# test true or false
+ ["+(dummy1)", 1],
+ ["+(dummy2)", 0],
+ ["+(dummy1 dummy2)", 1],
+ ["+(and dummy1 dummy2)", 0],
+ ["-(dummy1)", 0],
+ ["-(dummy2)", 1],
+ ["-(dummy1 dummy2)", 0],
+ ["-(and dummy1 dummy2)", 1],
+ ["+(dummy3=xx)", 1],
+ ["+(dummy3=xxy)", 0],
+ ["-(dummy3=xx)", 0],
+ ["-(dummy3=xxy)", 1],
+ ["+(dummy1=1)", 1],
+ ["+(dummy2=0)", 1],
+ ["+($^O)", 1],
+ ["+(not$^O)", 0],
+ ["-($^O)", 0],
+ ["-(not$^O)", 1],
+ ["+(or dummy1 dummy2)", 1],
+ ["-(or dummy1 dummy2)", 0],
+ ["+(or dummy1 (not dummy2))", 1],
+ ["+(and dummy1 (not dummy2))", 1],
+ ["+(and (not dummy2) dummy1)", 1],
+ # break it with whitespace
+ ["+( or dummy1(not dummy2))", 1],
+ ["+(or dummy1(not dummy2))", 1],
+ );
+}
+use Test::More tests => (7 + scalar(@cond_tests));
use Carp;
-use lib qw( lib );
+use lib qw( . lib );
use_ok('config::gen::makefiles');
use Parrot::Configure;
use Parrot::Configure::Options qw( process_options );
@@ -38,6 +71,40 @@
is($missing_SOURCE, 0, "No Makefile source file missing");
ok(-f $step->{CFLAGS_source}, "CFLAGS source file located");
+sub result {
+ my $c = shift;
+ my $s = $c->[0];
+ $s =~ s/^\+/plus_/;
+ $s =~ s/^\-/minus_/;
+ $s =~ s/[\()]//g;
+ $s =~ s/ /_/g;
+ return $s."=".($c->[1]?"true":"false");
+}
+# test #+(keys):line RT #57548
+$conf->data->set( @conf_args );
+open IN, ">", "Makefile_$$.in";
+print IN "# There should only be true results in .out\n";
+for my $c (@cond_tests) {
+ my $result = result($c);
+ print IN "#$c->[0]:$result\n";
+}
+close IN;
+$conf->genfile("Makefile_$$.in", "Makefile_$$.out",
+ (makefile => 1, conditioned_lines => 1));
+open OUT, "<", "Makefile_$$.out";
+my $f;
+{
+ local $/;
+ $f = <OUT>;
+}
+END {
+ unlink "Makefile_$$.in", "Makefile_$$.out";
+}
+for my $c (@cond_tests) {
+ my $result = result($c);
+ ok(($c->[1] ? $f =~ /^$result$/m : $f !~ /^$result$/m), "$result");
+}
+
pass("Completed all tests in $0");
################### DOCUMENTATION ###################
@@ -60,6 +127,8 @@
James E Keenan
+Reini Urban (#+, #-)
+
=head1 SEE ALSO
config::gen::makefiles, F<Configure.pl>.
Index: languages/jako/config/makefiles/root.in
===================================================================
--- languages/jako/config/makefiles/root.in (revision 30541)
+++ languages/jako/config/makefiles/root.in (working copy)
@@ -4,19 +4,30 @@
# $Id$
#
+LANG = jako
+BUILD_DIR = @build_dir@
+LOAD_EXT = @load_ext@
+O = @o@
+BIN_DIR = @bin_dir@
+PERLLIB_DIR = $(shell $(PERL) -V::sitelib:)
+DOC_DIR = @doc_dir@
+MANDIR = @mandir@
+
PERL = @perl@
RM_F = @rm_f@
JAKOC = $(PERL) -I lib jakoc
INTERP = ../../@test_prog@
+PBC_TO_EXE = ../../[EMAIL PROTECTED]@
BUILD_DIR = @build_dir@
RECONFIGURE = $(PERL) @build_dir@/tools/dev/reconfigure.pl
#CONDITIONED_LINE(darwin):
#CONDITIONED_LINE(darwin):# MACOSX_DEPLOYMENT_TARGET must be defined for OS X compilation/linking
#CONDITIONED_LINE(darwin):export MACOSX_DEPLOYMENT_TARGET := @osx_version@
+DOCS = README MAINTAINER
+
.SUFFIXES: .jako .pir
-
# default target
all: \
examples/bench.pir \
@@ -136,5 +147,7 @@
testclean:
#
-# End of file.
-#
+# Local variables:
+# mode: makefile
+# ex: ft=make
+# End:
Index: languages/eclectus/config/makefiles/root.in
===================================================================
--- languages/eclectus/config/makefiles/root.in (revision 30541)
+++ languages/eclectus/config/makefiles/root.in (working copy)
@@ -4,14 +4,21 @@
# Makefile for languages/eclectus
# configuration settings
-BUILD_DIR = @build_dir@
-LOAD_EXT = @load_ext@
-O = @o@
+LANG = eclectus
+BUILD_DIR = @build_dir@
+LOAD_EXT = @load_ext@
+O = @o@
+BIN_DIR = @bin_dir@
+LIB_DIR = @lib_dir@
+DOC_DIR = @doc_dir@
+MANDIR = @mandir@
# Set up commands
PARROT = ../../[EMAIL PROTECTED]@
+PBC_TO_EXE = ../../[EMAIL PROTECTED]@
PERL = @perl@
RM_F = @rm_f@
+CP = @cp@
RECONFIGURE = $(PERL) @build_dir@/tools/dev/reconfigure.pl
BUILD_DYNPMC = $(PERL) @build_dir@/tools/build/dynpmc.pl
#CONDITIONED_LINE(darwin):
@@ -91,18 +98,18 @@
./installable_$(LANG)@exe@ tests-driver.scm
install : installable
- cp installable_$(LANG)@exe@ $(DESTDIR)$(BIN_DIR)/parrot-$(LANG)@exe@
+ $(CP) installable_$(LANG)@exe@ $(DESTDIR)$(BIN_DIR)/parrot-$(LANG)@exe@
cd $(PMC_DIR) && $(BUILD_DYNPMC) copy "--destination=$(DESTDIR)$(LIB_DIR)/parrot/dynext" $(PMCS)
$(POD2MAN) docs/eclectus.pod > $(DESTDIR)$(MANDIR)/man1/parrot-$(LANG).1
mkdir $(DESTDIR)$(DOC_DIR)/languages/$(LANG)
- cp $(DOCS) $(DESTDIR)$(DOC_DIR)/languages/$(LANG)
+ $(CP) $(DOCS) $(DESTDIR)$(DOC_DIR)/languages/$(LANG)
$(LANG)@exe@: driver_nqp.pbc
$(PBC_TO_EXE) driver_nqp.pbc
mv [EMAIL PROTECTED]@ $(LANG)@exe@
installable : installable_$(LANG)@exe@ $(ECLECTUS_GROUP)
- cp installable_$(LANG)@exe@ $(BUILD_DIR)
+ $(CP) installable_$(LANG)@exe@ $(BUILD_DIR)
installable_$(LANG)@exe@ : driver_nqp.pbc
$(PBC_TO_EXE) driver_nqp.pbc --install
Index: languages/pipp/config/makefiles/root.in
===================================================================
--- languages/pipp/config/makefiles/root.in (revision 30541)
+++ languages/pipp/config/makefiles/root.in (working copy)
@@ -8,7 +8,7 @@
# Set up directories
BUILD_DIR = @build_dir@
-LIBRARY_DIR = $(BUILD_DIR)/runtime/parrot/library
+LIBRARY_DIR = @build_dir@/runtime/parrot/library
PMCDIR = src/pmc
PARROT_DYNEXT = @build_dir@/runtime/parrot/dynext
BIN_DIR = @bin_dir@
@@ -22,7 +22,7 @@
PBC_TO_EXE = $(BUILD_DIR)/pbc_to_exe$(EXE)
PERL = @perl@
RM_F = @rm_f@
-RECONFIGURE = $(PERL) $(BUILD_DIR)/tools/dev/reconfigure.pl
+RECONFIGURE = $(PERL) @build_dir@/tools/dev/reconfigure.pl
PMCBUILD = $(PERL) @build_dir@/tools/build/dynpmc.pl
NQP = $(BUILD_DIR)/compilers/nqp/nqp.pbc
PCT = $(BUILD_DIR)/runtime/parrot/library/PCT.pbc
