lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Best way to integrate PCRE


From: Vadim Zeitlin
Subject: Re: [lmi] Best way to integrate PCRE
Date: Thu, 30 Sep 2021 14:50:24 +0200

On Thu, 30 Sep 2021 01:38:12 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 9/28/21 8:09 PM, Greg Chicares wrote:
GC> [...]
GC> > The root cause seems to be here, in 'workhorse.make':
GC> > 
GC> >   # The link command promiscuously mentions libxml2 for all targets.
GC> >   # Measurements show that this costs one-tenth of a second on
GC> >   # reasonable hardware, and it saves the trouble of maintaining a list
GC> >   # of which targets require which libraries.
GC> >   #
GC> >   # TODO ?? Consider refining it anyway, because it's unclean: libxml2
GC> >   # isn't actually required for all targets.
GC> >   
GC> >   REQUIRED_LIBS := \
GC> >     $(xml_libraries) \
GC> > so maybe it's time to resolve that defect once and for all.
GC> 
GC> Below is a rough patch that I think will resolve this appropriately.

 Hello,

 This would indeed work, but I took to heart the "suppose
'test_coding_rules' actually required libxml2" part of your message and so
didn't even try doing this, but rather wanted to ensure that xml_libraries
were defined correctly, i.e. accordingly to the currently being used
LMI_TRIPLET, instead. I don't have a solution doing this that I really like
yet, however, I hoped to return to it today because what I did so far
doesn't look very nice.

GC> Obviously this part:
GC>   -REQUIRED_LIBS := \
GC>   +UN_REQUIRED_LIBS := \
GC>      $(xml_libraries) \
GC> should be removed, and the associated comments revised or removed.
GC> 
GC> In 'workhorse.make', the changes are mostly like this:
GC>   +liblmi.a liblmi$(SHREXT): REQUIRED_LIBS := $(xml_libraries)
GC>    liblmi.a liblmi$(SHREXT): EXTRA_LDFLAGS :=
GC> where ":=" seems appropriate. In included file 'objects.make',
GC> ":=" won't do, and "+=" is needed, though I don't know why; maybe
GC> tomorrow I'll try plain "=" in both files and see if that works.

 As your follow-up message explains, this does work, of course, but
personally I don't like using recursively-expanded make variables. They are
subject to "spooky action at a distance" which makes makefiles behaviour
even more unpredictable. FWIW, I've used them to define xml_libraries
correctly in my attempts to fix this so far, and this is actually the main
reason I don't like my current solution.

GC> Some disused targets (not exercised by 'nychthemeral_test.sh')
GC> aren't modified in this patch; they could either be modified for
GC> concinnity, or expunged for simplicity.
GC> 
GC> Any comments?

 I had agreed with your previous post and still think that ideal would be
to ensure that xml_libraries are defined correctly, rather than simply not
using them for test_coding_rules target at all. OTOH I also realize that
it's quite unlikely that test_coding_rules ever depends on libxml2 (if we
ever need to parse XML to check C++ code style, I'm switching to another
language). OT3H I don't think it's completely inconceivable that we add
some other developer-only tool only built under Linux and using libxml2 in
the future.

 So the first question for me is whether we want to fix wrong xml_libraries
definition. I think it would be preferable, but I certainly won't insist on
doing it if you don't think it's worth it.

 The second question is whether we it is fine to use recursively-expanded
variables or if we want to avoid this. As explained above, I'd rather try
to avoid them but, again, if you think it's not worth it, using them is
probably the simplest solution to write (although I'd still argue that it's
not the simplest one to read and understand and debug later).

 Final question is whether I should make the changes -- based on your
answers to the questions above -- or if you will just make them in master
yourself and then I could rebase use-pcre branch on the latest master to
ensure that physical closure check works for all commits?

 Thanks in advance,
VZ

Attachment: pgpMT9G9UNFtY.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]