Bug 35441 - [PATCH] Mesa does not find nouveau include files with --enable-shared-dricore
[PATCH] Mesa does not find nouveau include files with --enable-shared-dricore
Status: RESOLVED FIXED
Product: Mesa
Classification: Unclassified
Component: Mesa core
git
Other All
: highest critical
Assigned To: mesa-dev
:
: 35609 37147 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-18 20:43 UTC by Robert White
Modified: 2011-06-02 20:02 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments
a suitable patch (456 bytes, patch)
2011-03-20 00:15 UTC, Себастьян Gliţa Κατινα
Details | Splinter Review
just nouveau *.pc (449 bytes, patch)
2011-03-24 02:49 UTC, Себастьян Gliţa Κατινα
Details | Splinter Review
use pkgconfig in configure.ac and INCLUDES in Makefiles (5.88 KB, patch)
2011-03-25 01:36 UTC, Себастьян Gliţa Κατινα
Details | Splinter Review
uses pkgconfig in configure.ac and configure and INCLUDES in Makefiles (11.51 KB, patch)
2011-03-25 01:38 UTC, Себастьян Gliţa Κατινα
Details | Splinter Review
use pkgconfig in configure.ac and INCLUDES in Makefiles (6.18 KB, patch)
2011-03-29 09:30 UTC, Себастьян Gliţa Κατινα
Details | Splinter Review
uses pkgconfig in configure.ac and configure and INCLUDES in Makefiles (11.81 KB, patch)
2011-03-29 09:31 UTC, Себастьян Gliţa Κατινα
Details | Splinter Review
split Makefile.template and use pkgconfig in configure.ac and INCLUDES in Makefiles (16.09 KB, patch)
2011-03-30 03:57 UTC, Себастьян Gliţa Κατινα
Details | Splinter Review
split Makefile.template and use pkgconfig in configure.ac and configure and INCLUDES in Makefiles (21.72 KB, patch)
2011-03-30 03:58 UTC, Себастьян Gliţa Κατινα
Details | Splinter Review
Final fix! (19.70 KB, patch)
2011-05-14 10:58 UTC, Johannes Obermayr
Details | Splinter Review
Final fix #2 (19.70 KB, patch)
2011-05-21 14:02 UTC, Johannes Obermayr
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Robert White 2011-03-18 20:43:25 UTC
The current git repository as of this date is missing at least two header files referenced by src/mesa/drivers/dri/nouveau/nouveau_driver.h

The file nouveau_device.h is missing and when I just experimentally commented that out then nouveau_gobj.h cropped up missing so I stopped.

All the other header files in the same block of includes that I could locate are all apparently local to this directory. They are also all local includes (e.g. in "" not <> quotes) so I figure they are all from here.

spot checking I think

nouveau_device.h, nouveau_grobj.h, nouveau_channel.h, and nouveau_notifier.h and any .c files they front for are suspiciously missing or live somewhere inobvious to me.

This was first observed about two weeks ago but I didn't realize this was the right place to report it. /doh.
Comment 1 Себастьян Gliţa Κατινα 2011-03-20 00:15:41 UTC
Created attachment 44629 [details] [review]
a suitable patch

In file `src/mesa/drivers/dri/nouveau/Makefile.am' use `LIBDRM_CFLAGS' instead of just `CFLAGS': the pkg-config flags from `libdrm_nouveau.pc' (installed by `libdrm') must yield its `-I/usr/include/nouveau' where you find all these "missing" header files. (`INCLUDES' variable instead does not seem to work).

c.
Comment 2 Себастьян Gliţa Κατινα 2011-03-24 02:49:54 UTC
Created attachment 44779 [details] [review]
just nouveau *.pc

no need to for libdrm, see configs/linux-dri
(it was `../Makefile' not `../Makefile.am')
Comment 3 Себастьян Gliţa Κατινα 2011-03-24 02:56:53 UTC
*** Bug 35609 has been marked as a duplicate of this bug. ***
Comment 4 Francisco Jerez 2011-03-24 11:00:51 UTC
Apparently the problem is just that CFLAGS is not taken into account during the DRI driver compilation when using "--enable-shared-dricore".

Christopher, was that the intended behavior of that option?
Comment 5 Себастьян Gliţa Κατινα 2011-03-25 01:36:40 UTC
Created attachment 44812 [details] [review]
use pkgconfig in configure.ac and INCLUDES in Makefiles


- moved `+=' stuff after "include ../Makefile.template"

- placing stuff in configure.ac

shortcoming:
should consider for nouveau also nv50 etc. like for radeon r300 r600 ...

proposal:
split `Makefile.template' into `Makefile.common' and `Makefile.targets'
Comment 6 Себастьян Gliţa Κατινα 2011-03-25 01:38:48 UTC
Created attachment 44813 [details] [review]
uses pkgconfig in configure.ac and configure and INCLUDES in Makefiles

same patch but added stuff also for `configure' script
Comment 7 Peter Hjalmarsson 2011-03-27 04:51:33 UTC
Comment on attachment 44813 [details] [review]
uses pkgconfig in configure.ac and configure and INCLUDES in Makefiles

Emm, why rename the radeon variable?


-RADEON_LIBS = `pkg-config --libs libdrm_radeon`
+NOUVEAU_LIBS = `shell pkg-config libdrm_nouveau --libs`
+NOUVEAU_CFLAGS = `shell pkg-config libdrm_nouveau --cflags`
+
+RADEON_LDFLAGS = `pkg-config --libs libdrm_radeon`
Comment 8 Себастьян Gliţa Κατινα 2011-03-27 12:44:29 UTC
(In reply to comment #7)
> (From update of attachment 44813 [details] [review])
> Emm, why rename the radeon variable?
> 
> 
> -RADEON_LIBS = `pkg-config --libs libdrm_radeon`
> +NOUVEAU_LIBS = `shell pkg-config libdrm_nouveau --libs`
> +NOUVEAU_CFLAGS = `shell pkg-config libdrm_nouveau --cflags`
> +
> +RADEON_LDFLAGS = `pkg-config --libs libdrm_radeon`

It seemed as a bug w.r.t. uninformed use in all Makefiles.

RADEON_LIBS looks like gone (not LIBDRM_RADEON_LIBS though with one occurence in `configure.ac':1053); it's not coming from normal PKG_CHECK_MODULES m4 macro: LIBDRM_RADEON_* do in lines 870-873, and then are read in lines 1052-1053.

Is this better?

-RADEON_LIBS = `pkg-config --libs libdrm_radeon`
-RADEON_CFLAGS = `pkg-config --cflags libdrm_radeon`
+LIBDRM_RADEON_LIBS = `pkg-config --libs libdrm_radeon`
+LIBDRM_RADEON_CFLAGS = `pkg-config --cflags libdrm_radeon`
+RADEON_CFLAGS = "-DHAVE_LIBDRM_RADEON=1 $LIBDRM_RADEON_CFLAGS"
+RADEON_LDFLAGS = "$LIBDRM_RADEON_LIBS"
Comment 9 Себастьян Gliţa Κατινα 2011-03-29 09:30:05 UTC
Created attachment 45008 [details] [review]
use pkgconfig in configure.ac and INCLUDES in Makefiles

configs/linux-dri is a makefile
Comment 10 Себастьян Gliţa Κατινα 2011-03-29 09:31:01 UTC
Created attachment 45009 [details] [review]
uses pkgconfig in configure.ac and configure and INCLUDES in Makefiles

configs/linux-dri is a makefile
Comment 11 Себастьян Gliţa Κατινα 2011-03-30 03:57:03 UTC
Created attachment 45037 [details] [review]
split Makefile.template and use pkgconfig in configure.ac and INCLUDES in Makefiles

split `Makefile.template' into `Makefile.defines' and `Makefile.targets'
Comment 12 Себастьян Gliţa Κατινα 2011-03-30 03:58:57 UTC
Created attachment 45038 [details] [review]
split Makefile.template and use pkgconfig in configure.ac and configure and INCLUDES in Makefiles

split `Makefile.template' into `Makefile.defines' and `Makefile.targets'
Comment 13 Fabio Pedretti 2011-05-13 00:00:18 UTC
*** Bug 37147 has been marked as a duplicate of this bug. ***
Comment 14 Johannes Obermayr 2011-05-14 10:58:48 UTC
Created attachment 46722 [details] [review]
Final fix!

This patches fixes whole compiling.

Tested on openSUSE (with and without --enable-shared-dricore).

https://build.opensuse.org/package/show?package=Mesa&project=home%3Ajobermayr
(with --enable-shared-dricore)
Comment 15 Johannes Obermayr 2011-05-14 11:01:46 UTC
Set 'Component: Mesa core' since the patch touches core parts.
Comment 16 Johannes Obermayr 2011-05-21 14:02:26 UTC
Created attachment 46984 [details] [review]
Final fix #2

Thanks to Chris Bandy for the hint that line 48 missed a trailing double quote.

But I am still wondering that it needs ~ 1 week and a further hint by me on IRC until somebody had a (first?) look on it ...

Because some gave me hints on IRC that my (hard) wording sometimes is not reasonable I am referring to:
http://tsdgeos.blogspot.com/2010/10/qt-gitorious-merge-requests-and-open.html
(My worry is that this fix also will pushed someday after a couple of releases or even forgotten ...)

Btw. somebody told me that quality is: Away from accident.
Another one told me that source code should always be in a compileable state (indifferent whether you use [supported] configure switches) ...
Some other projects (e.g. Gnash or Lightspark or Gluon) are happy if they get hints for compile errors or compiler warnings on specific distributions/configure switches (and there the main/core developers are also reachable on their IRC channels ...)
Comment 17 Johannes Obermayr 2011-06-02 12:39:35 UTC
Set RESOLVED -> WONTFIX since nobody is man enough to push or comment on the patch which solves the issue within more than two weeks ...
Comment 18 Carl Worth 2011-06-02 15:53:02 UTC
(In reply to comment #17)
> Set RESOLVED -> WONTFIX since nobody is man enough to push or comment on the
> patch which solves the issue within more than two weeks ...

Hi Johannes,

It seems clear that you're frustrated that you're not getting a faster response to your contribution. However, WONTFIX is not the correct resolution for a bug that simply hasn't been examined closely enough yet.

(I'm not personally in a position to evalutate a change to Mesa's build system, but I would prefer to see a bug like this left open so that someone who is in such a position could find it.)

-Carl
Comment 19 Brian Paul 2011-06-02 16:21:01 UTC
I don't think anyone's actively working on the old nouveau driver any more (changes to such drivers tend to get ignored).
I applied the patch here and it seems OK (though I can't test nouveau).
I'll commit it in a minute.  It can always be reverted if anyone has problems.
Comment 20 Johannes Obermayr 2011-06-02 20:02:21 UTC
Just to clarify when and why I am getting angry:

1. Who is the person I must call? Usually I do not know it exactly: https://bugs.freedesktop.org/show_bug.cgi?id=37471 ; http://people.freedesktop.org/~cbrill/dri-log/index.php?date=2011-05-18&channel=dri-devel&highlight_names=&update=Update&date=2011-05-19 (at 10:50, 11:15 and )

2. Here I am lucky to know the person who is responsible for nouveau_vieux.so but assigned it to Mesa core (as a matter of prudence ...)

3. Write to the maintainer on IRC (at 17:03): http://people.freedesktop.org/~cbrill/dri-log/index.php?date=2011-05-14&channel=nouveau&highlight_names=&update=Update&date=2011-05-14

4. Waiting a week to get answer from Mesa core developers -> no response ...

5. Reminder (at 09:07): http://people.freedesktop.org/~cbrill/dri-log/index.php?date=2011-05-21&channel=dri-devel&highlight_names=&update=Update&date=2011-05-21

6. Packed developers on their honor (comment #17) after ~ 3 weeks

7. The patch got applied ...

8. I know it is not the correct way but it was not the first time developers/maintainers make a decision only after I packed them on their honor (also on other projects) ...

So my suggestion:
Overthink the development process and provide information about maintainers and people with permission to commit (I have not known who is part of Mesa core team ...).