Replace the value for $(MAKE) with the actual path (config/inter/make.pm)

[doughera]

This ugly hack remains. Unfortunately, in the absence of any information about what problem r10027 was attempting to solve, nothing is likely to change.

See  Original RT for more details

[jkeenan]

For reference, I am displaying relevant parts of configuration steps:

config/init/defaults.pm

    207         make          => $Config{make},
    208         make_set_make => $Config{make_set_make},
    209         make_and      => '&&',
    210 
    211         # make_c: Command to emulate GNU make's C<-C directory> 
                  option:  chdir
    212         # to C<directory> before executing $(MAKE)
    213         make_c => '$(PERL) -e \'chdir shift @ARGV; 
                  system q{$(MAKE)}, @ARGV; exit $$? >> 8;\'',

config/inter/make.pm

    106     # setup make_C
    107     _set_make_c($conf, $prog);
    108 
    109     return 1;
    110 }
    111 
    112 sub _set_make_c {
    113     my ($conf, $prog) = @_;
    114     if ( $conf->data->get('gmake_version') ) {
    115         $conf->data->set( make_c => "$prog -C" );
    116     }
    117     else {
    118 
    119         # get the default value
    120         my $make_c = $conf->data->get('make_c');
    121 
    122         # TT #1049: this is an ugly hack
    123         # replace the value for $(MAKE) with the actual path 
                  or we'll end up
    124         # with a variable that recursively refers to itself
    125         $make_c =~ s/\$\(MAKE\)/$prog/;
    126 
    127         $conf->data->set( make_c => $make_c );
    128     }
    129 }

config/gen/makefiles/root.in

    568 # MAKE CONFIGURATION:
...
    572 # This is set to MAKE=$make if your $make command doesn't
    573 # do it for you.
    574 @make_set_make@
    575 MAKE = @make_c@

[doughera]

Replying to jkeenan:

For reference, I am displaying relevant parts of configuration steps:

The key one is this (from root.in):

> config/gen/makefiles/root.in
> 
>     572 # This is set to MAKE=$make if your $make command doesn't
>     573 # do it for you.
>     574 @make_set_make@
>     575 MAKE = @make_c@

This leads to silly things like this in the generated Makefile, where the comment is clearly no longer accurate.

# This is set to MAKE=$make if your $make command doesn't
# do it for you.
#
MAKE = $(PERL) -e 'chdir shift @ARGV; system q{dmake}, @ARGV; exit $$? >> 8;'

As I noted in the original RT ticket: The problem here is that most 'make' commands set the variable $(MAKE) to refer to the calling program. For some unspecified reason, the patch in r10027 changes that and uses the variable $(MAKE) to mean something different (either make -C or a perl equivalent). I can't see how redefining a built-in variable to mean something different is a good idea. But it clearly was a deliberate change, so I'm not sure what to do.

The natural thing to do would be to call that variable MAKE_C. I can't think of any fundamental reason why that wouldn't work.

[jkeenan]

Andy,

Please see the kid51/tt1049_make branch. Everything builds okay, but I was getting this very strange test failure:

   Failed test 'Parrot_PMC_hashvalue'
#   at t/src/extend_vtable.t line 165.
#          got: 'Done!
# '
#     expected: 'Got hash!
# Done!
# '
# './t/src/extend_vtable_2' failed with exit code 0
# Looks like you failed 1 test of 68.
t/src/extend_vtable.t .. 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/68 subtests 

Test Summary Report
-------------------
t/src/extend_vtable.t (Wstat: 256 Tests: 68 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=1, Tests=68,  7 wallclock secs 
  ( 0.03 usr  0.01 sys +  4.90 cusr  1.84 csys =  6.78 CPU)
Result: FAIL

[jkeenan]

dukeleto,

Am cc-ing you on this for any insight. I created the kid51/tt1049_make branch tonight from master. The test shown above fails in branch even though it passes in master, and even though the differences between master and branch are as yet few:

$ git diff -w master...tt1049_make |cat
diff --git a/config/gen/makefiles/root.in b/config/gen/makefiles/root.in
index 45236ee..d303b40 100644
--- a/config/gen/makefiles/root.in
+++ b/config/gen/makefiles/root.in
@@ -572,7 +572,7 @@ EXTRANCITHUNKS_SO   = $(DYNEXT_DIR)/extra_nci_thunks$(LOAD_EXT)
 # This is set to MAKE=$make if your $make command doesn't
 # do it for you.
 @make_set_make@
-MAKE = @make_c@
+MAKE_C = @make_c@
 
 # These need to be above lines that define suffix rules to be portable
 # (certainly FreeBSD make doesn't grok the pir.pbc rule otherwise)
@@ -836,7 +836,7 @@ installable: all $(INSTALLABLEPARROT) $(INSTALLABLEPDUMP) $(INSTALLABLEDIS) $(IN
 
 bootstrap-ops : $(OPS2C)
        $(OPS2C) --core --quiet
-       $(MAKE) .
+       $(MAKE_C) .
 
 runtime/parrot/include/parrotlib.pbc: runtime/parrot/library/parrotlib.pir $(PARROT) $(GEN_PASM_INCLUDES)
        $(PARROT) -o $@ runtime/parrot/library/parrotlib.pir
@@ -1295,7 +1295,7 @@ examples/pasm/hello.pbc: examples/pasm/hello.pasm
 #      $(PARROT) -o examples/pasm/hello$(O) examples/pasm/hello.pbc
 #
 #examples/pasm/hello$(EXE): examples/pasm/hello$(O)
-#      $(MAKE) . EXEC=examples/pasm/hello exec
+#      $(MAKE_C) . EXEC=examples/pasm/hello exec
 examples/pasm/hello$(EXE): examples/pasm/hello.pbc $(PBC_TO_EXE)
        $(PBC_TO_EXE) examples/pasm/hello.pbc
 
@@ -1931,10 +1931,10 @@ html.stub:
        @echo "Perldoc is required, but not detected."
 
 html.dummy :
-       $(MAKE) docs html
+       $(MAKE_C) docs html
 
 html-clean :
-       $(MAKE) docs html-clean
+       $(MAKE_C) docs html-clean
 
 #IF(has_perldoc):htmlhelp : htmlhelp.dummy
 #ELSE:htmlhelp : htmlhelp.stub
@@ -1943,16 +1943,16 @@ htmlhelp.stub:
        @echo "Perldoc is required, but not detected."
 
 htmlhelp.dummy :
-       $(MAKE) docs htmlhelp
+       $(MAKE_C) docs htmlhelp
 
 htmlhelp-clean :
-       $(MAKE) docs htmlhelp-clean
+       $(MAKE_C) docs htmlhelp-clean
 
 pdf:
-       $(MAKE) docs pdf
+       $(MAKE_C) docs pdf
 
 pdf-clean:
-       $(MAKE) docs pdf-clean
+       $(MAKE_C) docs pdf-clean
 
 ###############################################################################
 #
@@ -1970,7 +1970,7 @@ ext-clean:
 ###############################################################################
 
 editor-clean :
-       $(MAKE) editor clean
+       $(MAKE_C) editor clean
 
 ###############################################################################
 #

[jkeenan]

That problem was addressed with an explicit assignment to $(MAKE) in the Makefile.

diff --git a/config/gen/makefiles/root.in b/config/gen/makefiles/root.in
index 45236ee..5dff74e 100644
--- a/config/gen/makefiles/root.in
+++ b/config/gen/makefiles/root.in
@@ -572,7 +572,8 @@ EXTRANCITHUNKS_SO   = $(DYNEXT_DIR)/extra_nci_thunks$(LOAD_EXT)
 # This is set to MAKE=$make if your $make command doesn't
 # do it for you.
 @make_set_make@
-MAKE = @make_c@
+MAKE = @make@
+MAKE_C = @make_c@
...[as before]

I'm not sure why that solved the problem (make fulltest is now passing in this branch), but I do note that there was/is one use of $PConfig{make} inside lib/Parrot/Test.pm:

 655 sub _run_test_file {
 656     my ( $func, $code, $expected, $desc, %extra ) = @_;
 657     my $path_to_parrot = path_to_parrot();
...
 699     if ( $args =~ s/--run-exec// ) {
 700         $run_exec = 1;
 701         my $pbc_f = per_test( '.pbc', $test_no );
 702         my $o_f = per_test( '_pbcexe' . $PConfig{o}, $test_no );
...
 722             if ( -e $o_f ) {
 723                 run_command(
 724                     qq{$PConfig{make} EXEC=$exec_f exec},
 725                     CD     => $path_to_parrot,
 726                     STDOUT => $out_f,
 727                     STDERR => $out_f
 728                 );

[jkeenan]

Replying to jkeenan:

That problem was addressed with an explicit assignment to $(MAKE) in the Makefile.

The problem is not solved, but it's not a problem caused by the revisions in this branch. See: TT #2026 for further discussion.

Apart from that, how does this branch look?

Thank you very much.

kid51

[doughera]

I think you can probably now remove the "ugly hack" in config/inter/make.pm that started this whole ticket. I've attached an (untested!) patch that does just that. It also adds in a '+' prefix to the GNU make -C command so that GNU make still knows how to parallelize sub-makes.

[jkeenan]

Replying to doughera:

I think you can probably now remove the "ugly hack" in config/inter/make.pm that started this whole ticket. I've attached an (untested!) patch that does just that. It also adds in a '+' prefix to the GNU make -C command so that GNU make still knows how to parallelize sub-makes.

Patch tested; had to fix some of the steps tests; applied in branch.

I hope to merge this branch within two days. Any final comments?

Thank you very much.

kid51

[jkeenan]

Most recent diff of this branch against its branch-point

[jkeenan]

Merged at 0164b2c; ran perl Configure.pl && make test with satisfactory results. After you git pull, please do make realclean and then reconfigure. Closing ticket.