maria-developers team mailing list archive

[Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>]

Arjen Lentz <arjen@xxxxxxxxxxxxx> writes:

> We filed a wishlist bug for this,
> https://bugs.launchpad.net/maria/+bug/470580 which also includes a
> patch by Antony Curtis. It creates a mysql- 
> glob.h during the build process, which we could put into a mariadb- 
> plugin-dev package.
>
> Then, some mariadb-xxxx packages can be build in the main mariadb
> build, and some can be separate.
> It also allows developers to work on plugins sanely. It cleans up the
> entire dev environment for this - it's not 100% pretty as it's a nasty
> big include, but it's a good step given the current mess.

I took a look at this (sorry for slight delay, have been swamped with lots of
small tasks the last couple of days).

I very much agree with the idea of having a plugin-dev package that is
sufficient for building plugins. It seems it should be doable with not too
much work.

I know we already did some fixes in the MariaDB tree to make this
possible. Basically some configure/CFLAGS options could affect the binary
interface (think it was DEBUG / SAFEMALLOC flags). These were moved into
config.h so that it is not necessary to guess the right values of these
options to build a plugin that will load without crashing.

We can do more of these kinds of fixes as needed.

I do not understand the approach taken in the patch though. Seems to me to be
the wrong approach? Though I could be misunderstanding, the patch doesn't
contain much in the way of explanation / comments yet.

So I was expecting to see somthing that collected a set of include files
needed to build (storage engine) plugins, and putting them somewhere plugins
could reference them.

Instead, the patch seems to run the C pre-processor on mysql_priv.h, I assume
to get a _single_ .h file with everything? And the result is passed through
some Perl filter and munged in various interesting ways.

So I guess my first question is why to do it this roundabout way at all? Why
not just collect the necessary .h files? Seems much cleaner, simpler, easier
to maintain, etc. If there are some issues with this, maybe we could instead
fix these issues in the source?

Thanks,

 - Kristian.

> === modified file 'sql/Makefile.am'
> --- old/sql/Makefile.am	2009-09-15 10:46:35 +0000
> +++ new/sql/Makefile.am	2009-11-02 09:19:32 +0000
> @@ -20,6 +20,7 @@
>  MYSQLBASEdir=		$(prefix)
>  MYSQLLIBdir=            $(pkglibdir)
>  pkgplugindir =		$(pkglibdir)/plugin
> +globincldir =		$(pkgincludedir)/mysql/$(VERSION)
>  INCLUDES =		@ZLIB_INCLUDES@ \
>  			-I$(top_builddir)/include -I$(top_srcdir)/include \
>  			-I$(top_srcdir)/regex -I$(srcdir) $(openssl_includes) \
> @@ -29,6 +30,7 @@
>  libexec_PROGRAMS =	mysqld
>  EXTRA_PROGRAMS =	gen_lex_hash
>  bin_PROGRAMS =		mysql_tzinfo_to_sql
> +globincl_HEADERS =	mysql-glob.h
>  
>  noinst_LTLIBRARIES=	libndb.la \
>  			udf_example.la
> @@ -179,6 +181,12 @@
>  		./gen_lex_hash$(EXEEXT) > $@-t
>  		$(MV) $@-t $@
>  
> +mysql-glob.h:	mysql_priv.h
> +	$(CXXCPP) -x c++-header $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) \
> +		$(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) \
> +		-CC -dDI $^ | $(PERL) $(srcdir)/make-mysql-glob.pl > $@-t
> +	$(MV) $@-t $@
> +
>  # For testing of udf_example.so
>  udf_example_la_SOURCES= udf_example.c
>  udf_example_la_LDFLAGS= -module -rpath $(pkglibdir)
> 
> === added file 'sql/make-mysql-glob.pl'
> --- old/sql/make-mysql-glob.pl	1970-01-01 00:00:00 +0000
> +++ new/sql/make-mysql-glob.pl	2009-11-02 09:39:43 +0000
> @@ -0,0 +1,179 @@
> +#!/usr/bin/perl
> +
> +use strict;
> +
> +my $current_file;
> +my $current_line;
> +my $current_path;
> +
> +my @command_line= ();
> +
> +my $until_file;
> +my $until_line;
> +
> +my $last_include;
> +
> +my $echo= \&echo_on;
> +my $echo_line;
> +
> +my $first_define;
> +my $define_suffix = sub { };
> +
> +sub canon($)
> +{
> +  my ($path)= @_;
> +  my $tmp= $path;
> +  $tmp =~ s@/[^/]+/\.\./@/@;
> +  return canon($tmp) if $tmp ne $path;
> +  return $path;
> +}
> +
> +$first_define = sub {
> +  my ($line) = @_;
> +  if ($line =~ m/^#define\s+([^\s]+)/)
> +  {
> +    my ($e,$d)= ($echo, $1);
> +    $first_define = sub { return 0; };
> +    &$e("#ifndef MYSQL_GLOB_H\n#define MYSQL_GLOB_H\n#ifdef $d\n#error cannot mix headers\n#endif\n$line\n");
> +    foreach (@command_line)
> +    {
> +      &$e($_);
> +    }
> +    
> +    $define_suffix= sub
> +    {
> +      &$e("#endif /* MYSQL_GLOB_H / $d */\n");
> +    };
> +
> +    return 1;
> +  }
> +  return 0;
> +};
> +
> +sub echo_on($)
> +{
> +  my ($line) = @_;
> +  
> +  return if &$first_define($line);
> +  return if $line =~ m/^#define\s+SHOW_FUNC\s+/;
> +  
> +  print $echo_line if defined $echo_line;
> +  print $line;
> +  undef $echo_line;
> +}
> +
> +sub echo_off($)
> +{
> +  my ($line)= @_;
> +  if (($current_file eq $until_file) && ($current_line == $until_line))
> +  {
> +    $echo= \&echo_on;
> +    undef $until_file;
> +    undef $until_line;
> +    undef $echo_line;
> +    echo_on($line) if !($line =~ m/^# /);
> +  }
> +}
> +
> +sub echo_cmdline($)
> +{
> +  my ($line)= @_;
> +  push @command_line, $line if
> +      ($current_file ne $until_file) || ($current_line != $until_line);
> +  echo_off($line);
> +}
> +
> +sub test_builtin($$)
> +{
> +  ($until_line, $until_file)= @_ if !defined($until_file);
> +  $echo= \&echo_off;
> +  return 0;
> +}
> +
> +sub test_cmdline($$)
> +{
> +  ($until_line, $until_file)= @_ if !defined($until_file);
> +  $echo= \&echo_cmdline;
> +  return 0;
> +}
> +
> +sub test_incpath($$)
> +{
> +  if ($current_line == 1)
> +  {
> +    ($until_line, $until_file)= @_ if !defined($until_file);
> +    if ($current_line == 1)
> +    {
> +      undef $echo_line;
> +      &$echo($last_include);
> +      $echo= \&echo_off;
> +    }
> +  }
> +  return 0;
> +}
> +
> +while (<>)
> +{
> +  my ($last_line, $last_file)= ($current_line, $current_file);
> +  my $test= sub { return 1; };
> +
> +  if (m/^# (\d+) "(.*)"/)
> +  {
> +    ($current_line, $current_file)= (int $1,$2);
> +    
> +    if ($current_file =~ m/\/$/)
> +    {
> +      $current_path= $`;
> +      ($current_line, $current_file)= ($last_line,$last_file);
> +      next;
> +    }
> +
> +    $echo_line= $_;
> +
> +    $test= sub
> +    {
> +      my $saved_echo= $echo;
> +      $echo= sub
> +      {
> +        my ($line)= @_;
> +        $echo= $saved_echo;
> +        echo_off($line);
> +      };
> +      return 1;
> +    };
> +    
> +    $test= sub { return test_builtin($last_line, $last_file); } if
> +        $current_file eq '<built-in>';
> +    $test= sub { return test_cmdline($last_line, $last_file); } if
> +        $current_file eq '<command line>';
> +    $test= sub { return test_incpath($last_line, $last_file); } if
> +        $current_file =~ m/^\//;
> +
> +    if (!($current_file =~ m/^\//))
> +    {
> +      my $new_path= canon($current_path.$current_file);
> +      $echo_line =~ s/"$current_file"/"$new_path"/;
> +    }
> +    $echo_line=~ s/^# /#line /;
> +    
> +  }
> +
> +  if (m/^#include /)
> +  {
> +    $test= sub {
> +      my $saved_echo= $echo;
> +      $echo= sub {
> +        ($last_include)= @_;
> +        $echo= $saved_echo;
> +      };
> +      return 1;
> +    };
> +  }
> +
> +  &$echo($_) if &$test();
> +  $current_line++;  
> +}
> +
> +&$define_suffix();
> +
> +exit 0;
>