Re: "load" on Windows

2013-10-05 Thread Gisle Vanem

"Eli Zaretskii"  wrote:


What about setting:
  ent->fptr.func_ptr = func;

too?


They are one and the same, since they are members of a union:

 struct function_table_entry
   {
 union {
char *(*func_ptr) (char *output, char **argv, const char *fname);
char *(*alloc_func_ptr) (const char *fname, int argc, char **argv);
 } fptr;


Okay, you're right. I didn't see the "union".


Thanks.  Your problem is here:


  EXPORT int mk_test_gmk_setup (const gmk_floc *flocp)
  {
gmk_add_function ("hello_world", hello_world, 0, 255, 0);

^^^

Make functions cannot have the '_' character in their names, so it
seems.  Here's why:


Ayyy!! > 1 day wasted on such a triffle. May I suggest more warnings
in define_new_function()? It works fine as "$(hello-world "Hello world").


Btw, Gisle, I don't understand why you needed to use the EXPORT
thingy, the DLL is compiled just fine without it, and only exports
non-static variables and functions anyway. 


Right again. I was thinking more about making a DLL with MSVC.
(without any .def file).

Thanks again.

--gv

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-05 Thread Paul Smith
On Sat, 2013-10-05 at 16:34 +0300, Eli Zaretskii wrote:
> >   EXPORT int mk_test_gmk_setup (const gmk_floc *flocp)
> >   {
> > gmk_add_function ("hello_world", hello_world, 0, 255, 0);
>  ^^^
> Make functions cannot have the '_' character in their names, so it
> seems.  Here's why:
>   /* Look up a function by name.  */
> 
>   static const struct function_table_entry *
>   lookup_function (const char *s)
>   {
> const char *e = s;
> 
> while (*e && ( (*e >= 'a' && *e <= 'z') || *e == '-'))  
>   e++;
> 
> So if you name your function hello-world instead, it will work.
> 
> Paul, if this limitation is deliberate, I suggest to document it where
> we explain the arguments of gmk_add_function.

It's not so much a deliberate restriction, as it was a performance
improvement I added in 2002 along with the switch to hash table lookups,
and completely forgot about afterward :-).  All the existing make
functions were lowercase and contained only '-', so we avoided the
lookup effort if we found a name which could not be a function.

Now that users can define their own functions, I don't think that
restriction is appropriate anymore.  I'll fix this, thanks.

> (I can make these changes in documentation, if you agree.)

I have a vivid memory of adding documentation regarding this so let me
look around for it.


___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-05 Thread Eli Zaretskii
> Date: Sat, 5 Oct 2013 10:22:13 -0400
> From: David Boyce 
> Cc: Gisle Vanem , Paul Smith , bug-make 
> 
> 
> 
> [1:text/plain Hide]
> 
> On Sat, Oct 5, 2013 at 9:34 AM, Eli Zaretskii  wrote:
> 
> > while (*e && ( (*e >= 'a' && *e <= 'z') || *e == '-'))  
> >
> 
> Isn't this awfully ascii-ish anyway? Can it use isalpha() or some similar
> abstraction instead?

isalpha won't cut it, we probably need to test islower as well.

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-05 Thread David Boyce
On Sat, Oct 5, 2013 at 9:34 AM, Eli Zaretskii  wrote:

> while (*e && ( (*e >= 'a' && *e <= 'z') || *e == '-'))  
>

Isn't this awfully ascii-ish anyway? Can it use isalpha() or some similar
abstraction instead?

-David Boyce
___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-05 Thread Eli Zaretskii
> Date: Sat, 05 Oct 2013 16:34:11 +0300
> From: Eli Zaretskii 
> Cc: bug-make@gnu.org
> 
> Paul, if this limitation is deliberate, I suggest to document it where
> we explain the arguments of gmk_add_function.

One other important thing that doesn't seem to be covered in the
manual is the requirements from the function that implements the
Make function added by a loaded object.  If the reader looks at
gnumake.h, she will see that the function's signature should be like
this:

  char *(*func)(const char *nm, int argc, char **argv)

But this had better be documented explicitly in the manual.

More importantly, we never say that the 'char *' value returned by the
extending function must either be a pointer to a buffer allocated by
calling gmk_alloc, or NULL; any other kind of pointer will cause
unexpected results or crashes, because Make will try to free any
non-NULL pointer returned by the extension.  I think it is quite
important to describe this in the manual.

(I can make these changes in documentation, if you agree.)

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-05 Thread Eli Zaretskii
> From: "Gisle Vanem" 
> Date: Sat, 5 Oct 2013 14:33:26 +0200
> 
> "Eli Zaretskii"  wrote:
> 
> > Well, the tests in the test suite that test this feature did work for
> > me at some point, so you may wish first to verify they do for you, and
> > then compare your extension with the ones used by the test suite, to
> > see what's different.
> 
> Well this is just silly. I've added tracing code a lot of places (enabled
> by option --trace) . I do see my mk_test_gmk_setup() gets called, but not 
> the new function added by gmk_add_function(). 
> 
> gmk_add_function() calls define_new_function(), but therein I see a:
>   ent->fptr.alloc_func_ptr = func;
> 
> What about setting:
>   ent->fptr.func_ptr = func;
> 
> too?

They are one and the same, since they are members of a union:

  struct function_table_entry
{
  union {
char *(*func_ptr) (char *output, char **argv, const char *fname);
char *(*alloc_func_ptr) (const char *fname, int argc, char **argv);
  } fptr;

> How else is the new function supposed to be called? I don't understand
> this contorted logic.

I don't understand the question, sorry.  The logic looks quite simple
to me: In expand_builtin_function we have:

  if (!entry_p->alloc_fn)
return entry_p->fptr.func_ptr (o, argv, entry_p->name);

  /* This function allocates memory and returns it to us.
 Write it to the variable buffer, then free it.  */

  p = entry_p->fptr.alloc_func_ptr (entry_p->name, argc, argv);
  if (p)
{
  o = variable_buffer_output (o, p, strlen (p));
  free (p);
}

For loaded functions, define_new_function has this:

  ent->alloc_fn = 1;
  ent->fptr.alloc_func_ptr = func;

So expand_builtin_function should see that alloc_fn is non-zero, and
invoke your function via the alloc_func_ptr pointer.

> For reference, here is my complete mk_test.c:

Thanks.  Your problem is here:

>   EXPORT int mk_test_gmk_setup (const gmk_floc *flocp)
>   {
> gmk_add_function ("hello_world", hello_world, 0, 255, 0);
 ^^^

Make functions cannot have the '_' character in their names, so it
seems.  Here's why:

  /* Look up a function by name.  */

  static const struct function_table_entry *
  lookup_function (const char *s)
  {
const char *e = s;

while (*e && ( (*e >= 'a' && *e <= 'z') || *e == '-'))  
  e++;

So if you name your function hello-world instead, it will work.

Paul, if this limitation is deliberate, I suggest to document it where
we explain the arguments of gmk_add_function.

Btw, Gisle, I don't understand why you needed to use the EXPORT
thingy, the DLL is compiled just fine without it, and only exports
non-static variables and functions anyway.  Doing away with it makes
the code portable to platforms other than Windows.

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-05 Thread Gisle Vanem

"Eli Zaretskii"  wrote:


Well, the tests in the test suite that test this feature did work for
me at some point, so you may wish first to verify they do for you, and
then compare your extension with the ones used by the test suite, to
see what's different.


Well this is just silly. I've added tracing code a lot of places (enabled
by option --trace) . I do see my mk_test_gmk_setup() gets called, but not 
the new function added by gmk_add_function(). 


gmk_add_function() calls define_new_function(), but therein I see a:
 ent->fptr.alloc_func_ptr = func;

What about setting:
 ent->fptr.func_ptr = func;

too? How else is the new function supposed to be called? I don't understand
this contorted logic. Isn't a new function supposed to extend (and possibly
modify) the 'function_table' used from expand_builtin_function()? Thus 
possibly replacing a built-in? Please somebody clarify.


For reference, here is my complete mk_test.c:

 #include 
 #include 
 #include 
 #include 
 #include 
 #include "gnumake.h"

 #define EXPORT __declspec(dllexport)

 EXPORT int plugin_is_GPL_compatible;

 static char *hello_world (const char *func_name, int argc, char **argv)
 {
   char *buf = gmk_alloc(1000), *p = buf;
   int  i;

   fprintf (stdout, "hello_world() called: %s\n", func_name);

   for (i = 0; i < argc; i++)
   {
 fprintf (stdout, "argv[%d]: %s\n", i, argv[i]);
 p += sprintf (p, "%s+", argv[i]);
   }
   return (buf);
 }

 EXPORT int mk_test_gmk_setup (const gmk_floc *flocp)
 {
   gmk_add_function ("hello_world", hello_world, 0, 255, 0);
   fprintf (stdout, "\"%s()\" called from %s (%lu)\n",
__FUNCTION__, flocp->filenm, flocp->lineno);
   return (1);
 }

--gv



___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-04 Thread Eli Zaretskii
> Date: Fri, 04 Oct 2013 09:46:10 +0300
> From: Eli Zaretskii 
> Cc: bug-make@gnu.org
> 
> > From: "Gisle Vanem" 
> > Date: Thu, 3 Oct 2013 23:07:50 +0200
> > 
> > Since I do get "Loaded modules: mk_test.dll" it mean it's loaded okay. But
> > it's never run. So it must be some issue with how I use the extension. I 
> > looked 
> > at section 12.2.4 in the docs and cooked it from that. 
> 
> Well, the tests in the test suite that test this feature did work for
> me at some point

Just tried that with 3.99.93: the tests still work.  (I needed to
replace ".so" with ".dll" and make sure GNU echo.exe is called
instead of the cmd built-in, so watch out.)

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-03 Thread Eli Zaretskii
> From: "Gisle Vanem" 
> Date: Thu, 3 Oct 2013 23:07:50 +0200
> 
> Since I do get "Loaded modules: mk_test.dll" it mean it's loaded okay. But
> it's never run. So it must be some issue with how I use the extension. I 
> looked 
> at section 12.2.4 in the docs and cooked it from that. 

Well, the tests in the test suite that test this feature did work for
me at some point, so you may wish first to verify they do for you, and
then compare your extension with the ones used by the test suite, to
see what's different.

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-03 Thread Gisle Vanem

"Eli Zaretskii"  wrote:


I don't understand the problem you are describing.  First, when Make
is built, it produces gnumake.exe, not make.exe.  And second, if
make.exe that runs is on PATH, rather than in the current directory,
it's a different executable, and there should be no problem linking
it.


I simply rewrote the build process to generate make.exe instead of
gnumake.exe. But this is a minor issue. The "load" is what I want to
resolve.


My little mk_test.dll is simply trying to echo back some text, but my
function is never called.  I checked with "pedump mk_test.dll" that the 
symbols "mk_test_gmk_setup" and "plugin_is_GPL_compatible" are exported.


How did you build your DLL?  In particular, did you link it against
libgnumake-1.dll.a, the import library produced by the build process?


Yes, I modeled this after build_w32.bat. But I made this into a makefile 
with:


make.exe: $(OBJECTS) libsubproc.a
 $(CC) -o $@ $^ -Wl,--out-implib=libgnumake-1.dll.a -luser32 -ladvapi32

And the mk_test.dll:
  $(CC) $(CFLAGS) -shared mk_test.c -o mk_test.dll ./libgnumake-1.dll.a

The mk_test.dll looks okay and 'gnumake -d test_dll' produces no errors
like the one from load.c. That means the dll is okay AFAICS. And make.exe
works fine too. Except for the "load" statement.

Since I do get "Loaded modules: mk_test.dll" it mean it's loaded okay. But
it's never run. So it must be some issue with how I use the extension. I looked 
at section 12.2.4 in the docs and cooked it from that. 



--gv

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


Re: "load" on Windows

2013-10-03 Thread Eli Zaretskii
> From: "Gisle Vanem" 
> Date: Thu, 3 Oct 2013 22:03:14 +0200
> 
> VERSION = 3.99.93
> ifeq ($(MAKE_VERSION),$(VERSION))
> -load ./mk_test.dll
> endif
> 
> The above was needed since I needed to use mingw-make version 3.82.90
> to build this new make.exe. Windows doesn't allow make.exe to be linked
> while make.exe is running.

I don't understand the problem you are describing.  First, when Make
is built, it produces gnumake.exe, not make.exe.  And second, if
make.exe that runs is on PATH, rather than in the current directory,
it's a different executable, and there should be no problem linking
it.

So I guess you need to describe in more detail exactly what you did to
build a new make.exe.

> test_dll: 
>   @echo 'Loaded modules: $(.LOADED)'  
>   @echo 'Make version: $(MAKE_VERSION)' 
>   @echo 'FEATURES: $(.FEATURES)' ; \
>   @echo 'Calling mk_test: $(mk_test "Hello world")'
> 
> The output is:
>   Loaded modules: mk_test.dll
>   Make version: 3.99.93
>   FEATURES: target-specific order-only second-expansion else-if shortest-stem 
>   undefine oneshell archives jobserver output-sync load
>   Calling mk_test:
> 
> See? "load" is there and mk_test.dll is loaded.
> 
> 
> 
> My little mk_test.dll is simply trying to echo back some text, but my
> function is never called.  I checked with "pedump mk_test.dll" that the 
> symbols "mk_test_gmk_setup" and "plugin_is_GPL_compatible" are exported.

How did you build your DLL?  In particular, did you link it against
libgnumake-1.dll.a, the import library produced by the build process?

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make