Re: [PATCH] Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Sat, 14 Jun 2008, chromatic via RT wrote: On Tuesday 07 August 2007 10:11:20 Andy Dougherty wrote: This next patch does a little more cleanup on the attributes checking. I revised config/auto/attributes.pm to use the existing Configure routines cc_build() and conf-data_add() rather than re-implementing them. I also removed some now unnecessary #includesparrot/compiler.h statements and cc_inc fiddling in config/init/hints/solaris.pm I've applied everything but the attributes.pm part as r28365, because there was some fuzz and my best approach to getting it to work disabled attribute detection from me. (That's what I get for not applying patches much more frequently; sorry!) I'd take a new patch for the latter, if you're interested. Thanks for the offer. However, given that the related ticket [perl #44381] was rejected, I'm not inclined to invest more effort in attribute detection. -- Andy Dougherty [EMAIL PROTECTED]
Re: [PATCH] Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Tuesday 07 August 2007 10:11:20 Andy Dougherty wrote: This next patch does a little more cleanup on the attributes checking. I revised config/auto/attributes.pm to use the existing Configure routines cc_build() and conf-data_add() rather than re-implementing them. I also removed some now unnecessary #includesparrot/compiler.h statements and cc_inc fiddling in config/init/hints/solaris.pm I've applied everything but the attributes.pm part as r28365, because there was some fuzz and my best approach to getting it to work disabled attribute detection from me. (That's what I get for not applying patches much more frequently; sorry!) I'd take a new patch for the latter, if you're interested. -- c
[PATCH] Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Mon, 6 Aug 2007, Mark Glines via RT wrote: On Mon, 6 Aug 2007 15:20:39 -0700 chromatic [EMAIL PROTECTED] wrote: That's not exactly the same as a stringified hash. I think we can just remove the cc_run() bits and be fine (at least, it worked for me). Indeed, with the attached patch I have working attributes again. Thanks. This next patch does a little more cleanup on the attributes checking. I revised config/auto/attributes.pm to use the existing Configure routines cc_build() and conf-data_add() rather than re-implementing them. I also removed some now unnecessary #includesparrot/compiler.h statements and cc_inc fiddling in config/init/hints/solaris.pm There should be no functional changes. diff -r -u parrot-orig/config/auto/attributes.pm parrot-andy/config/auto/attributes.pm --- parrot-orig/config/auto/attributes.pm 2007-08-07 03:15:09.0 -0400 +++ parrot-andy/config/auto/attributes.pm 2007-08-07 13:09:29.0 -0400 @@ -63,62 +63,24 @@ my $cc = $conf-option_or_data( 'cc' ); cc_gen('config/auto/attributes/test_c.in'); -my $ccflags = $conf-option_or_data( 'ccflags'); -my $tryflags = $ccflags -D$attr; +my $tryflags = -D$attr; # These are OK to fail, because we're trying them out. -my $command_line = $cc -o test -Iinclude $tryflags test.c; -$verbose and print , $command_line, $/; +eval { cc_build($tryflags) } ; -my $exit_code = Parrot::Configure::Step::_run_command( -$command_line, 'test.cco', 'test.ccr' ); -$verbose and print exit code: $exit_code$/; - -return if $exit_code; +if ($@) { +print Failed: [EMAIL PROTECTED] if $verbose; +} +else { +print Ok.\n if $verbose; +$conf-data-add(' ', ccflags = $tryflags); +} -$conf-data-set( ccflags = $tryflags ); +cc_clean(); return; } -sub blerugh { -my ( $self, $conf, $attr ) = @_; - -my $verbose = $conf-options-get('verbose'); -if ( 0 ) { -my $hints_used = 0; - -my $hints = init::hints:: . lc($^O); - -print [ $hints if $verbose; - -eval use $hints; -die $@ if $@; - -# Call the runstep method if it exists. -# Otherwise the step must have done -# its work when it was loaded. -$hints-runstep( $conf, @_ ) if $hints-can('runstep'); -$hints_used++; - -$hints = init::hints::local; -print $hints if $verbose; -eval use $hints; -unless ($@) { -$hints-runstep( $conf, @_ ) if $hints-can('runstep'); -$hints_used++; -} - -if ( $hints_used == 0 ) { -print (no hints) if $verbose; -} - -print ] if $verbose; -} - -return $self; -} - 1; # Local Variables: diff -r -u parrot-orig/config/auto/gcc/test_c.in parrot-andy/config/auto/gcc/test_c.in --- parrot-orig/config/auto/gcc/test_c.in 2007-08-06 19:15:12.0 -0400 +++ parrot-andy/config/auto/gcc/test_c.in 2007-08-07 09:15:53.0 -0400 @@ -4,8 +4,6 @@ #include stdio.h #include stdlib.h -#include parrot/compiler.h - int main(int argc, char **argv) diff -r -u parrot-orig/config/auto/msvc/test_c.in parrot-andy/config/auto/msvc/test_c.in --- parrot-orig/config/auto/msvc/test_c.in 2007-08-06 19:15:13.0 -0400 +++ parrot-andy/config/auto/msvc/test_c.in 2007-08-07 12:57:23.0 -0400 @@ -4,8 +4,6 @@ #include stdio.h #include stdlib.h -#include parrot/compiler.h - int main(int argc, char *argv[]) diff -r -u parrot-orig/config/init/hints/solaris.pm parrot-andy/config/init/hints/solaris.pm --- parrot-orig/config/init/hints/solaris.pm2007-08-03 11:15:16.0 -0400 +++ parrot-andy/config/init/hints/solaris.pm2007-08-07 09:03:36.0 -0400 @@ -35,8 +35,7 @@ # Can't call cc_build since we haven't set all the flags yet. # This should suffice for this test. -my $cc_inc = $conf-data-get('cc_inc'); -Parrot::Configure::Step::_run_command( $cc -o test test.c $cc_inc, 'test.cco', 'test.cco' ) +Parrot::Configure::Step::_run_command( $cc -o test test.c, 'test.cco', 'test.cco' ) and confess C compiler failed (see test.cco); %gnuc = eval cc_run() or die Can't run the test program: $!; if ( defined $gnuc{__GNUC__} ) { -- Andy Dougherty [EMAIL PROTECTED]
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On 8/3/07, jerry gay [EMAIL PROTECTED] wrote: On 8/3/07, via RT Andy Dougherty [EMAIL PROTECTED] wrote: # New Ticket Created by Andy Dougherty # Please include the string: [perl #44379] # in the subject line of all future correspondence about this issue. # URL: http://rt.perl.org/rt3/Ticket/Display.html?id=44379 The new attributes scanning ought to use its own test_c.in file, instead of trying to piggy-back off existing test_c.in files. The scheme in place has several problems: 1. It has no fallback. From attributes.pm, the following snippet: if ( $cc =~ /gcc/ ) { cc_gen('config/auto/gcc/test_c.in'); } elsif ( $cc eq 'cl' ) { cc_gen('config/auto/msvc/test_c.in'); } does nothing for other compilers. Obviously, all the compiles fail, since no test.c file is generated. I don't know, for example, if 'icc' handles attributes, or if it might in the future. 2. The name of the compiler is not the best test of whether you are using gcc. On Linux, for example, if you use Configure.pl --cc=cc, then this test, if ( $cc =~ /gcc/ ) { fails, even though you are using gcc. A better test is to check gccversion. 3. The attributes testing is now duplicated in two test files, gcc/test_c.in and msvc/test_c.in. 4. It's now potentially more difficult to use gcc/test_c.in for its intended use. For example, the Solaris hints file needs to know if we're using gcc, and it used to try to compile the gcc/test_c.in program. Now, it has to add in -Iinclude/ to pick up parrot/compilers.h which does the #ifdef dance around all the attributes. All this compilication just to see if we're using gcc. A much better plan, in my humble opinion, is to revert most of these changes and simply make a *new* test_c.in file, config/auto/attributes/test_c.in. Each test file can then concentrate on doing one test. Thanks for listening, you're right, on all points. i hacked that code in to get msvc working, and figured i'd clean up the damage later. sorry, i didn't think of testing icc before i committed--of course it'll break. i don't expect to have the tuits until monday, though. hopefully somebody will step in before then and clean up the mess andy and i made. otherwise, i'll get to it first when i'm available. ~jerry i think i've fixed it up as of r20521. let me know if it still behaves unexpectedly. ~jerry
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Monday 06 August 2007 14:06:53 jerry gay wrote: i think i've fixed it up as of r20521. let me know if it still behaves unexpectedly. I find these two lines confusing: my %eval = eval cc_run(); return if !%eval; cc_run returns a string; why evaluate it? Further, why assign the result to a hash? The test_c.in says: +/* as long as the file compiles, everything is okay */ Although my x86 Linux system with GCC supports most of these attributes, they don't get set here. It seems like if the file can compile, we've identified enough information. -- c
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Mon, Aug 06, 2007 at 02:40:06PM -0700, chromatic ([EMAIL PROTECTED]) wrote: On Monday 06 August 2007 14:06:53 jerry gay wrote: i think i've fixed it up as of r20521. let me know if it still behaves unexpectedly. I find these two lines confusing: my %eval = eval cc_run(); return if !%eval; Have you looked at the string it returns? It looks like this: ( GCC_VERSION = 3, GCC_MINOR = 4 ); Or something similar. It perplexed me as well. -- Andy Lester = [EMAIL PROTECTED] = www.petdance.com = AIM:petdance
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Monday 06 August 2007 15:10:30 Andy Lester wrote: On Mon, Aug 06, 2007 at 02:40:06PM -0700, chromatic ([EMAIL PROTECTED]) wrote: I find these two lines confusing: my %eval = eval cc_run(); return if !%eval; Have you looked at the string it returns? It looks like this: ( GCC_VERSION = 3, GCC_MINOR = 4 ); Or something similar. It perplexed me as well. I thought so too, but the probe uses config/auto/attributes/test_c.in: /* as long as the file compiles, everything is okay */ int main(int argc, char *argv[]) { return EXIT_SUCCESS; } That's not exactly the same as a stringified hash. I think we can just remove the cc_run() bits and be fine (at least, it worked for me). -- c
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Mon, 6 Aug 2007 15:20:39 -0700 chromatic [EMAIL PROTECTED] wrote: On Monday 06 August 2007 15:10:30 Andy Lester wrote: On Mon, Aug 06, 2007 at 02:40:06PM -0700, chromatic ([EMAIL PROTECTED]) wrote: I find these two lines confusing: my %eval = eval cc_run(); return if !%eval; Have you looked at the string it returns? It looks like this: ( GCC_VERSION = 3, GCC_MINOR = 4 ); Or something similar. It perplexed me as well. I thought so too, but the probe uses config/auto/attributes/test_c.in: /* as long as the file compiles, everything is okay */ int main(int argc, char *argv[]) { return EXIT_SUCCESS; } That's not exactly the same as a stringified hash. I think we can just remove the cc_run() bits and be fine (at least, it worked for me). I just chased this down, and then found this mailing list thread. So please excuse my arriving late to the party :) Currently (r20530), I'm not getting *any* attributes defined on my x86 linux box, because of this. This despite the fact that gcc 3.4.6 supports lots (if not all) of the attributes, and Configure.pl --verbose reports exit code 0 (meaning, success). cc_run() returns , eval cc_run() returns (), which fails the !%eval check on the line below. Which means lots of nice things, PARROT_DOES_NOT_RETURN for instance, are currently noops in my build. Removing the cc_run bit sounds like the right fix to me, too. The whole stringified hash thing seems to be used elsewhere to detect compiler version, but we don't need anything compiler-specific in this particular check. The important part is that it built, which was already checked. Mark
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Mon, 6 Aug 2007 15:20:39 -0700 chromatic [EMAIL PROTECTED] wrote: That's not exactly the same as a stringified hash. I think we can just remove the cc_run() bits and be fine (at least, it worked for me). Indeed, with the attached patch I have working attributes again. Mark Index: config/auto/attributes.pm === --- config/auto/attributes.pm (revision 20530) +++ config/auto/attributes.pm (working copy) @@ -66,7 +66,7 @@ my $ccflags = $conf-option_or_data( 'ccflags'); my $tryflags = $ccflags -D$attr; -# These are OK to fail, becuase we're trying them out. +# These are OK to fail, because we're trying them out. my $command_line = $cc -o test -Iinclude $tryflags test.c; $verbose and print , $command_line, $/; @@ -75,9 +75,6 @@ return if $exit_code; -my %eval = eval cc_run(); -return if !%eval; - $conf-data-set( ccflags = $tryflags ); return;
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On Monday 06 August 2007 22:25:06 Mark Glines wrote: Indeed, with the attached patch I have working attributes again. Works for me too. Applied as r20531. -- c
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
does nothing for other compilers. Obviously, all the compiles fail, since no test.c file is generated. I don't know, for example, if 'icc' handles attributes, or if it might in the future. icc handles all gcc attributes. I'd started digging into why the attributes were being ignored by icc and your post has helpt to answer that question :-) Paul
Re: [perl #44379] config/auto/attributes.pm ought to use its own test_c.in
On 8/3/07, via RT Andy Dougherty [EMAIL PROTECTED] wrote: # New Ticket Created by Andy Dougherty # Please include the string: [perl #44379] # in the subject line of all future correspondence about this issue. # URL: http://rt.perl.org/rt3/Ticket/Display.html?id=44379 The new attributes scanning ought to use its own test_c.in file, instead of trying to piggy-back off existing test_c.in files. The scheme in place has several problems: 1. It has no fallback. From attributes.pm, the following snippet: if ( $cc =~ /gcc/ ) { cc_gen('config/auto/gcc/test_c.in'); } elsif ( $cc eq 'cl' ) { cc_gen('config/auto/msvc/test_c.in'); } does nothing for other compilers. Obviously, all the compiles fail, since no test.c file is generated. I don't know, for example, if 'icc' handles attributes, or if it might in the future. 2. The name of the compiler is not the best test of whether you are using gcc. On Linux, for example, if you use Configure.pl --cc=cc, then this test, if ( $cc =~ /gcc/ ) { fails, even though you are using gcc. A better test is to check gccversion. 3. The attributes testing is now duplicated in two test files, gcc/test_c.in and msvc/test_c.in. 4. It's now potentially more difficult to use gcc/test_c.in for its intended use. For example, the Solaris hints file needs to know if we're using gcc, and it used to try to compile the gcc/test_c.in program. Now, it has to add in -Iinclude/ to pick up parrot/compilers.h which does the #ifdef dance around all the attributes. All this compilication just to see if we're using gcc. A much better plan, in my humble opinion, is to revert most of these changes and simply make a *new* test_c.in file, config/auto/attributes/test_c.in. Each test file can then concentrate on doing one test. Thanks for listening, you're right, on all points. i hacked that code in to get msvc working, and figured i'd clean up the damage later. sorry, i didn't think of testing icc before i committed--of course it'll break. i don't expect to have the tuits until monday, though. hopefully somebody will step in before then and clean up the mess andy and i made. otherwise, i'll get to it first when i'm available. ~jerry
[perl #44379] config/auto/attributes.pm ought to use its own test_c.in
# New Ticket Created by Andy Dougherty # Please include the string: [perl #44379] # in the subject line of all future correspondence about this issue. # URL: http://rt.perl.org/rt3/Ticket/Display.html?id=44379 The new attributes scanning ought to use its own test_c.in file, instead of trying to piggy-back off existing test_c.in files. The scheme in place has several problems: 1. It has no fallback. From attributes.pm, the following snippet: if ( $cc =~ /gcc/ ) { cc_gen('config/auto/gcc/test_c.in'); } elsif ( $cc eq 'cl' ) { cc_gen('config/auto/msvc/test_c.in'); } does nothing for other compilers. Obviously, all the compiles fail, since no test.c file is generated. I don't know, for example, if 'icc' handles attributes, or if it might in the future. 2. The name of the compiler is not the best test of whether you are using gcc. On Linux, for example, if you use Configure.pl --cc=cc, then this test, if ( $cc =~ /gcc/ ) { fails, even though you are using gcc. A better test is to check gccversion. 3. The attributes testing is now duplicated in two test files, gcc/test_c.in and msvc/test_c.in. 4. It's now potentially more difficult to use gcc/test_c.in for its intended use. For example, the Solaris hints file needs to know if we're using gcc, and it used to try to compile the gcc/test_c.in program. Now, it has to add in -Iinclude/ to pick up parrot/compilers.h which does the #ifdef dance around all the attributes. All this compilication just to see if we're using gcc. A much better plan, in my humble opinion, is to revert most of these changes and simply make a *new* test_c.in file, config/auto/attributes/test_c.in. Each test file can then concentrate on doing one test. Thanks for listening, -- Andy Dougherty [EMAIL PROTECTED]