lmi
[Top][All Lists]
Advanced

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

Re: [lmi] [PATCH] Integrate wxPdfDocument into lmi build system


From: Greg Chicares
Subject: Re: [lmi] [PATCH] Integrate wxPdfDocument into lmi build system
Date: Thu, 06 Aug 2015 19:58:28 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.3.0

On 2015-07-24 00:21, Vadim Zeitlin wrote:
[...]
> Handle more options in portable wx-config.
> 
> More options than just --cxxflags and --libs are needed when wx-config is used
> from the configure scripts of the libraries looking for the appropriate
> version of wxWidgets to use, so make wx-config-portable usable in such
> situations as well by handling calling it with more than one option at a time
> and also processing more options.

Please help me understand this better.

I've applied all four of these patches in my local tree so that I can
analyze the total effect. I don't see where the 'wx-config-portable'
script is used for more than one option at a time.
 - target 'portable_script' creates script 'wx-config-portable'
 - which is (conditionally) referenced as $(wx_config_script)
 - which is called (one option at a time) to assign these variables:
     wx_config_cxxflags := $(shell $(wx_config_script) --cxxflags)
     wx_config_libs     := $(shell $(wx_config_script) --libs)
     wx_config_basename := $(shell $(wx_config_script) --basename)
     wx_config_version  := $(shell $(wx_config_script) --version)
but I don't see where it's used with multiple arguments, or where it's
ever used in 'install_wxpdfdoc.make'. What am I missing? Does this
become useful only in a later patch that's still to come?

> (this commit is actually optional and can be skipped if you don't like it,
> which is something I would totally understand as the changes to
> install_wx.make definitely don't make it any prettier -- but then the next
> commit would need to be modified to use the real wx-config in the configure
> configure in install_wxpdfdoc.make as without these changes running
> configure with the portable script would fail)

I'd rather use the patches as submitted. I considered whether it's time
to get rid of the "portable" script (we've upgraded zsh to a version
that supports printf(1), e.g.), but the scary comment block above the
'portable_script' brings back too many unpleasant old memories.

It's kind of too bad that 'make' doesn't support here-document syntax,
but it's easy enough to extract the script from the makefile. I have
only one difficulty understanding it: what's 'this_with_arg' for? Does
it ensure that every multiple-argument invocation has --/c.*flags/ or
--libs as its final argument? If so, why is that desirable?

I don't see why, say, --rescomp is needed, or a do-nothing --host option
is necessary, but maybe the scales will fall from my eyes when I read
your answer to my first question above.

>  Please note that I don't include the changes to the autotools build system
> as I don't want to flood you with patches and I plan to just commit them
> directly when/if the patches above are applied.

Sure.

>  Finally, I did my best to test these changes, notably I verified that
> running install_msw.sh still works

There's another good reason for preferring to commit these patches as
submitted (perhaps with an extra comment or two resulting from the
questions I posed above, if they even turn out to be good questions).

> (provided the necessary adjustments to
> the Cygwin mirror is done and my other patch allowing to use wxWidgets 3.1
> from GitHub is applied).

Both are already in HEAD.

> BTW, as I wanted to test the patch in a fresh VM
> and had to create one anyhow, I also decided to test it under Windows 8.1,
> which I hadn't used with lmi so far (I could have used Windows 10 too, but
> I decided that this would be too new...) and, just in case it can be
> useful, I can say that everything seems to work just fine under this OS
> version.

Thanks, that's good to know.




reply via email to

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