Re: [pcre-dev] PCRE autotools patch drg1

Inizio della pagina
Delete this message
Autore: Craig Silverstein
Data:  
To: skunk
CC: pcre-dev
Oggetto: Re: [pcre-dev] PCRE autotools patch drg1
} "make dist" and "make distcheck" now work.

That's great!

} * Tweaked the handling of pcre.h, pcre_stringpiece.h and
} pcrecpparg.h so that they don't get distributed. (These three, as
} you recall, are generated from *.h.in files)

I agree with Philip that you should distribute pcre.h, even though
it's auto-generated.

Also, could you put a comment before the nodist_includes_HEADERS lines
that explains why you're not distributing them? (Because you're
distributing the foo.h.in versions instead, right?) Otherwise it can
be confusing why some header files are distributed and some aren't.

} if WITH_PCRE_CPP
} dist_html_DATA += doc/html/pcrecpp.html
} endif

For pcrecpp.3, you had an 'else' with this if statement, that put this
in dist_noinst_FOO. Do you want to do the same thing here? I'm not
sure what the else is for, so it's hard for me to say.

-- RunGrepTest.in:

I'd prefer we not use printf(1) if possible -- I don't know how
standard it is. (I don't think bsd machines have it, for instance.)
Perhaps use /bin/echo -e instead?

-- RunTest.in:

} -if [ "@UTF8@" = "" ] ; then
} +if [ "@UTF8@" = "no" ] ; then

Is this definitely always the right way to go?  What is @UTF8@ going
to be in each of the following cases?
    ./configure --enable-utf8
    ./configure --disable-utf8
    ./configure


Will it ever be the empty string?

-- configure.ac:

} # This facilitates -ansi builds under Linux
} AC_DEFINE([_GNU_SOURCE], [], [Enable GNU extensions in glibc])

If this is giving Philip problems, I suggest we comment it out for
now. I don't think -ansi support needs to be a priority for this
patch. We're making a lot of changes, so it's good to be conservative
where we can afford to be!

} if test "x$enable_shared" = "xno" ; then
}     AC_DEFINE([PCRE_STATIC],[1],[to link statically])
} fi


Indentation nit: should be two spaces. :-)

And maybe put the description on the second line, even though it fits
on one? That way this is consistent with all the other AC_DEFINEs.

} # This must be last; it determines what files are written as well as config.h

Alas, it's no longer last, so maybe remove this comment?

} # Print out a nice little message after configure is run displaying your
} # chosen options.

Cool!

Overall looks good to me.

craig