Re: [PATCH] fix module loading (was: [PATCH] build: fix bootstrapping)

[Gary V. Vaughan]

Hi Pavel,

> On Nov 22, 2014, at 3:57 PM, Pavel Raiskup <address@hidden> wrote:
> 
> On Friday 21 of November 2014 09:49:45 Pavel Raiskup wrote:
>> 0001-build-fix-bootstrap-fail.patch
> 
> This is OK now, thanks for fixing it in HEAD.
> 
>> 0002-modules-inclusions-fix-path-searching-issues.patch
> 
> Here I tried to reformat once more the patch against master.  Result is
> attached as 0001-modules-inclusions-fix-path-searching-issues.patch. That
> patch is to make the modules work at all for me.
> 
> However, thinking about it over and over again, there is something wrong.
> FWIW, the KO Myung-Hun's patch [1] seems to deal with the same.

Yes, indeed.  I'll consider both in parallel before I decide what to commit,
thanks for pointing out the similarities in intent.

> When I run ./src/m4, it fails - because, by loading of required 'm4'
> module, 'm4' binary search the $PWD directory.  Because it also searches
> for modules without suffix, directory 'm4' is found and dlopen() is
> performed on it.
> 
> * Do we really want M4PATH (thus -I) paths use for module loading?
>  It seems to me that something like M4_EXT_PATH is needed and we should
>  not mix 'include' with 'load'.

The idea is to minimize the proliferation of additional language keywords
in the GNU version, because each new builtin affects the behaviour of
macro expansion compared to a non-GNU make when a matching bareword is
encountered.  Arguably only in pathological cases, but even so, I'd like
to avoid additional builtins where it can be done elegantly.

I'm inclined to agree that conflating m4 text modules with loadable modules
is a mistake, and M4_EXT_PATH is a good solution - especially as it might
be desirable to put binary modules under /usr/local/lib and text modules
under /usr/local/share (for example).

I'll try to commit some improvements in this direction over the coming days.
Thanks for the pointer!

> * Is it desired that $PWD is used for module searching?  That seems to be
>  rather security hole.

Certainly for POSIX compatibility with text modules, we must support loading
from the current directory.  But, I agree that one can do rather more harm
with a binary loadable module, so it's definitely a win to separate out the
search paths for each.

> * Don't we want rather load only modules having concrete platform-default
>  file extension (e.g. '*.so*' files on Linux)?

Yes, this is an artifact of unfinished removal of reliance on libltdl.  I'll
try to commit some improvements in this area over the coming days too.

> For the beginning, would anyone be willing to review/push patches for
> that?

Subject to my comments above, most definitely!  And it may be a few days
before I get to it, so please feel free to beat me to the punch if you
have time :-)

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)

> [1] http://www.mail-archive.com/address@hidden/msg01116.html
> 
> Pavel
> <0001-modules-inclusions-fix-path-searching-issues.patch>_______________________________________________
> M4-patches mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/m4-patches