Raku / nqp

NQP
Other
343 stars 131 forks source link

Simplify MoarVM backend runner's Makefile template #728

Closed MasterDuke17 closed 3 years ago

MasterDuke17 commented 3 years ago

Remove one redundant entry ('moar') and two unneeded entries ('sha1', 'tinymt').

MasterDuke17 commented 3 years ago

@vrurg is there any way to figure out these include paths at build time? Currently, the need to s/libtommath/gmp/ is the only thing preventing NQP and Rakudo from building against my replace-libtommath-with-gmp MoarVM branch/PR without needing their own branches.

vrurg commented 3 years ago

@MasterDuke17 it depends on whether MoarVM can provide this information. At this moment, as I checked --show-config output, moar does provide a list of include paths, but they're in form of -I options and paths are relative to moar's build dir. So, they're pretty much useless for NQP build.

MasterDuke17 commented 3 years ago

Well, --show-config does have the prefix, and we know the MoarVM install creates the $PREFIX/include/* directories. Is is possible at NQP build time to just look in $PREFIX/include/ and add any directories found?

[dan@alexandria nqp]$ ./nqp-m --show-config | grep include
moar::shaincludedir=3rdparty/sha1
moar::moar_cincludes= -I3rdparty/libuv/include -I3rdparty/libuv/src -I3rdparty/libatomicops/src -I3rdparty/gmp -I3rdparty/dyncall/dynload -I3rdparty/dyncall/dyncall -I3rdparty/dyncall/dyncallback
moar::lincludes=
moar::install=  $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libuv"   $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libuv/uv"        $(CP) 3rdparty/libuv/include/*.h "$(DESTDIR)$(PREFIX)/include/libuv"    $(CP) 3rdparty/libuv/include/uv/*.h "$(DESTDIR)$(PREFIX)/include/libuv/uv"        $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/armcc"  $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/gcc"    $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/hpc"      $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/ibmc"   $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/icc"    $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/loadstore"        $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/msftc"  $(MKPATH) "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/sunc"   $(CP) 3rdparty/libatomicops/src/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops"   $(CP) 3rdparty/libatomicops/src/atomic_ops/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops"   $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps"     $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/armcc/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/armcc"       $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/gcc/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/gcc"     $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/hpc/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/hpc"   $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/ibmc/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/ibmc"   $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/icc/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/icc"   $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/loadstore/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/loadstore" $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/msftc/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/msftc"       $(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/sunc/*.h "$(DESTDIR)$(PREFIX)/include/libatomic_ops/atomic_ops/sysdeps/sunc"   $(MKPATH) "$(DESTDIR)$(PREFIX)/include/gmp"     $(CP) 3rdparty/gmp/*.h "$(DESTDIR)$(PREFIX)/include/gmp"        $(MKPATH) "$(DESTDIR)$(PREFIX)/include/dyncall"   $(CP) 3rdparty/dyncall/dynload/*.h "$(DESTDIR)$(PREFIX)/include/dyncall"        $(CP) 3rdparty/dyncall/dyncall/*.h "$(DESTDIR)$(PREFIX)/include/dyncall"        $(CP) 3rdparty/dyncall/dyncallback/*.h "$(DESTDIR)$(PREFIX)/include/dyncall"
moar::cincludes=

Or more precisely, just those things that are $(MKPATH)ed in the moar::install?

vrurg commented 3 years ago

I don't like either idea. Both are based on guessing. Parsing would also depend on the otherwise unreliable command line format, so I'm not going even consider it. As to the directories, an error in de-installation or upgrade process may result in a directory left behind. Locating compile-time errors caused by conflicting includes might be quite time consuming then.

MoarVM knows what 3rd party libs it uses and can provide this information explicitly, without guessing on client's side.

MasterDuke17 commented 3 years ago

Both are based on guessing.

True, but right now it's just a hard-coded list that doesn't take into account things like whether MoarVM was configured with --has-libuv. I'm not sure how those --show-config things get set by MoarVM, but you think it should create some sort of nicely formatted list of the directories it creates?

vrurg commented 3 years ago

I'm not much into details of how things are done in moar build, but what floats on surface is that Configure.pl takes all or some of the keys on $config and creates src/gen/config.c. I have also spotted -thirdparty key on the hash which is likely is the place where used 3rdparty libs are recorded.

This is as much information as managed to collect so far.

niner commented 3 years ago

Please keep in mind that MoarVM, NQP and Rakudo can be installed into different prefixes. $PREFIX is only NQP's prefix, not necessarily the one where MoarVM installs those headers.

MasterDuke17 commented 3 years ago

I just added a hllincludes key to %config in MoarVM's Configure.pl and pushed directory names on:

diff --git Configure.pl Configure.pl
index c94d86d36..21b7ee95c 100755
--- Configure.pl
+++ Configure.pl
@@ -155,6 +155,7 @@ $config{dllexport}                = '' unless defined $config{dllexport};
 $config{dlllocal}                 = '' unless defined $config{dlllocal};
 $config{translate_newline_output} = 0  unless defined $config{translate_newline_output};
 $config{vectorizerspecifier}      = '' unless defined $config{vectorizerspecifier};
+$config{hllincludes}              = ['moar'];

 # assume the compiler can be used as linker frontend
 $config{ld}           = $config{cc} unless defined $config{ld};
@@ -268,6 +269,7 @@ else {
                         . "\t\$(MKPATH) \"\$(DESTDIR)\$(PREFIX)/include/libuv/uv\"\n"
                         . "\t\$(CP) 3rdparty/libuv/include/*.h \"\$(DESTDIR)\$(PREFIX)/include/libuv\"\n"
                         . "\t\$(CP) 3rdparty/libuv/include/uv/*.h \"\$(DESTDIR)\$(PREFIX)/include/libuv/uv\"\n";
+    push @{$config{hllincludes}}, 'libuv';
 }

 if ($args{'has-libatomic_ops'}) {
@@ -297,6 +299,7 @@ else {
                         . "\t\$(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/loadstore/*.h \"$lao/atomic_ops/sysdeps/loadstore\"\n"
                         . "\t\$(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/msftc/*.h \"$lao/atomic_ops/sysdeps/msftc\"\n"
                         . "\t\$(CP) 3rdparty/libatomicops/src/atomic_ops/sysdeps/sunc/*.h \"$lao/atomic_ops/sysdeps/sunc\"\n";
+    push @{$config{hllincludes}}, 'libatomic_ops';
 }

 if ($args{'has-libtommath'}) {
@@ -315,6 +318,7 @@ else {
     $config{moar_cincludes} .= ' ' . $defaults{ccinc} . '3rdparty/libtommath';
     $config{install}   .= "\t\$(MKPATH) \"\$(DESTDIR)\$(PREFIX)/include/libtommath\"\n"
                         . "\t\$(CP) 3rdparty/libtommath/*.h \"\$(DESTDIR)\$(PREFIX)/include/libtommath\"\n";
+    push @{$config{hllincludes}}, 'libtommath';
 }

 if ($args{'has-libffi'}) {
@@ -362,6 +366,7 @@ else {
                         . "\t\$(CP) 3rdparty/dyncall/dynload/*.h \"\$(DESTDIR)\$(PREFIX)/include/dyncall\"\n"
                         . "\t\$(CP) 3rdparty/dyncall/dyncall/*.h \"\$(DESTDIR)\$(PREFIX)/include/dyncall\"\n"
                         . "\t\$(CP) 3rdparty/dyncall/dyncallback/*.h \"\$(DESTDIR)\$(PREFIX)/include/dyncall\"\n";
+    push @{$config{hllincludes}}, 'dyncall';
 }

 # The ZSTD_CStream API is only exposed starting at version 1.0.0
@@ -592,6 +597,7 @@ unless (defined $config{jit_obj}) {
 if ($config{cc} eq 'cl') {
     $config{install}   .= "\t\$(MKPATH) \"\$(DESTDIR)\$(PREFIX)/include/msinttypes\"\n"
                         . "\t\$(CP) 3rdparty/msinttypes/*.h \"\$(DESTDIR)\$(PREFIX)/include/msinttypes\"\n";
+    push @{$config{hllincludes}}, 'msinttypes';
 }

 if ($^O eq 'aix' && $config{ptr_size} == 4) {

These now show up in a --show-configure like so:

[dan@alexandria nqp]$ ./nqp-m --show-config | grep hllincludes
moar::hllincludes[4]=dyncall
moar::hllincludes[3]=libtommath
moar::hllincludes[2]=libatomic_ops
moar::hllincludes[1]=libuv
moar::hllincludes[0]=moar

and in NQP's build I somehow want to make the includes for the runner compilation be map { '@moar::prefix@\include\' . $_dirname_ } @{$config{hllincludes}}. However, I'm not sure how to make it happen between Configure.pl and tools/templates/moar/Makefile.in.

vrurg commented 3 years ago

nqp-configure is not actually able to parse array form of --show-config variables. It'd be better to have a space-separated list in the variable. But if there is good reasoning for this approach I can improve the parser.

vrurg commented 3 years ago

Ah, and I will patch the template when everything is in place for this.

MasterDuke17 commented 3 years ago

Is https://github.com/MoarVM/MoarVM/pull/1508 what you have in mind?

vrurg commented 3 years ago

@MasterDuke17 aha, looks good!

MasterDuke17 commented 3 years ago

Cool. So is this PR still needed? Or do you have some change in mind that will make it redundant?

vrurg commented 3 years ago

We can still merge this PR until moar version is bumped. But after the bump it's not a big deal in use the new variable and automate this process.