<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    On 12-01-17 08:28 PM, Matt Turner wrote:
    <blockquote
cite="mid:CAEdQ38GhbSygBhdVz1cn5EsmO-5_FFSe4vrZaD3fQGvbzaZQCw@mail.gmail.com"
      type="cite">
      <pre wrap="">On Tue, Jan 17, 2012 at 7:21 PM, Gaetan Nadon <a class="moz-txt-link-rfc2396E" href="mailto:memsize@videotron.ca">&lt;memsize@videotron.ca&gt;</a> wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="">On 12-01-17 03:58 PM, Matt Turner wrote:
</pre>
        <blockquote type="cite">
          <pre wrap="">---
&nbsp;src/mesa/drivers/dri/i965/Makefile.am | &nbsp; &nbsp;4 +---
&nbsp;1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/Makefile.am b/src/mesa/drivers/dri/i965/Makefile.am
index 5512381..93937b1 100644
--- a/src/mesa/drivers/dri/i965/Makefile.am
+++ b/src/mesa/drivers/dri/i965/Makefile.am
@@ -26,7 +26,7 @@ include Makefile.sources
&nbsp;# Hack to make some of the non-automake variables work.
&nbsp;TOP=$(top_builddir)

-AM_CFLAGS = \
+AM_CPPFLAGS = \
&nbsp; &nbsp; &nbsp; -I$(top_srcdir)/include \
&nbsp; &nbsp; &nbsp; -I$(top_srcdir)/src/ \
&nbsp; &nbsp; &nbsp; -I$(top_srcdir)/src/mapi \
</pre>
        </blockquote>
        <pre wrap="">This looks suspicious and/or error prone. Are you sure that none of the
variables assigned to AM_CPPFLAGS will ever contain any C compiler
flags? While AM_CFLAGS can contain pre-processor flags, AM_CPPFLAGS
cannot contain C compiler flags. I was not able to find where
INTEL_CFLAGS is assigned, but as its name implies, it may contain C
flags. If it is not used, it should be deleted. Automake has all the
features to allow a user to append C flags on the make command line.
</pre>
      </blockquote>
      <pre wrap="">
It's a little unclear, but I think

PKG_CHECK_MODULES([INTEL], [libdrm_intel &gt;= $LIBDRM_INTEL_REQUIRED])

is defining INTEL_{CFLAGS,LIBS}:

INTEL_CFLAGS = -I/usr/include/libdrm
INTEL_LIBS = -ldrm_intel -ldrm</pre>
    </blockquote>
    <br>
    I thought it was a private variable.<br>
    <br>
    Anyway, now I am sure there is issue. You cannot be sure that libdrm
    or any package for that matter will not put a compiler flag in
    there. In fact, that is what it was designed for.<br>
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; --cflags<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; This prints pre-processor and compile flags required
    to&nbsp; compile<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; the&nbsp; packages on the command line, including flags for
    all their<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; dependencies. Flags are "compressed" so that each
    identical flag<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; appears&nbsp; only&nbsp; once.&nbsp; pkg-config exits with a nonzero
    code if it<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; can't find metadata for one or more of the packages on
    the&nbsp; com&#8208;<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; mand line.<br>
    <br>
    For example, the xserver put the visibility flag in the
    xorg-server.pc file:<br>
    <blockquote>Cflags: -I${sdkdir} -fvisibility=hidden<br>
    </blockquote>
    There is this option which has never been used (which would have to
    be in pkg-config &gt;= 0.22):<br>
    <blockquote>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; --cflags-only-I<br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; This&nbsp; prints&nbsp; the -I part of "--cflags". That is, it
      defines the<br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; header search path but doesn't specify anything
      else.<br>
    </blockquote>
    I guess it all depends on the level of risk one is willing to take.
    If you control libdrm and can make sure of what goes in there...<br>
    <blockquote
cite="mid:CAEdQ38GhbSygBhdVz1cn5EsmO-5_FFSe4vrZaD3fQGvbzaZQCw@mail.gmail.com"
      type="cite">
      <pre wrap="">

and for radeon and nouveau:

RADEON_CFLAGS = -I/usr/include/libdrm
RADEON_LIBS = -ldrm_radeon

NOUVEAU_CFLAGS = -I/usr/include/libdrm -I/usr/include/nouveau
NOUVEAU_LIBS = -ldrm_nouveau

so those look appropriate for AM_CPPFLAGS, right?

Thanks,
Matt

</pre>
    </blockquote>
    <br>
  </body>
</html>