Re: [RFC i-g-t v3 08/13] lib/stubs: Add stubs for intel_bufmgr.

From: Tomeu Vizoso <tomeu.vizoso@collabora.com>
To: Daniel Vetter <daniel@ffwll.ch>, Emil Velikov <emil.l.velikov@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	daniel.stone@collabora.com
Subject: Re: [RFC i-g-t v3 08/13] lib/stubs: Add stubs for intel_bufmgr.
Date: Wed, 22 Jun 2016 12:40:23 +0200	[thread overview]
Message-ID: <f38b6969-343b-2848-8a01-e4687a88ac9d@collabora.com> (raw)
In-Reply-To: <20160622081222.GM23520@phenom.ffwll.local>

On 06/22/2016 10:12 AM, Daniel Vetter wrote:
> On Tue, Jun 21, 2016 at 02:41:29PM +0100, Emil Velikov wrote:
>> On 21 June 2016 at 13:50, Marius Vlad <marius.c.vlad@intel.com> wrote:
>>> On Mon, Jun 20, 2016 at 03:52:35PM +0100, Emil Velikov wrote:
>>>> Hi Rob,
>>>>
>>>> A couple of nitpicks and a case of missing git add :-)
>>>>
>>>> On 15 June 2016 at 10:51,  <robert.foss@collabora.com> wrote:
>>>>
>>>>> +if HAVE_LIBDRM_INTEL
>>>>> +else
>>>>> +    libintel_tools_la_SOURCES +=       \
>>>>> +        stubs/drm/intel_bufmgr.c       \
>>>>> +        stubs/drm/intel_bufmgr.h
>>>>> +endif
>>>>> +
>>>> I believe I mentioned if before - please use the following construct
>>>>
>>>> if !HAVE_LIBDRM_INTEL
>>>>     libintel_tools_la_SOURCES +=       \
>>>>         stubs/drm/intel_bufmgr.c       \
>>>>         stubs/drm/intel_bufmgr.h
>>>> endif
>>>>
>>>>>  AM_CPPFLAGS = -I$(top_srcdir)
>>>>>  AM_CFLAGS = $(CWARNFLAGS) $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(LIBUNWIND_CFLAGS) $(DEBUG_CFLAGS) \
>>>>>             -DIGT_SRCDIR=\""$(abs_top_srcdir)/tests"\" \
>>>>> diff --git a/lib/stubs/drm/README b/lib/stubs/drm/README
>>>>> new file mode 100644
>>>>> index 0000000..dec6a1d
>>>>> --- /dev/null
>>>>> +++ b/lib/stubs/drm/README
>>>>> @@ -0,0 +1,4 @@
>>>>> +intel_bufmgr.h is a copy the file provided in libdrm (intel/intel_bufmgr.h).
>>>>> +
>>>> s/copy the file provided in/local copy of the file provided by/
>>>>
>>>> Sadly that's not the case atm. There's a fair few changes in the local 'copy'.
>>>> Perhaps you forgot to git add ?
>>>>
>>>>> +Before releasing i-g-t a current copy of intel_bufmgr.h should be copied into
>>>>> +this directory of i-g-t.
>>>
>>> Auch, but synchronizing this wouldn't be a nightmare? I mean stubs
>>> might be added/removed in the future.
>>>
>> Maybe slightly annoying, but definitely not nightmare. As with any
>> other library one _cannot_ remove API without bumping major (and/or
>> changing the library name), so things should be fine on that regard.
>> Guess one should stress that intel_bufmgr.h should be copied from the
>> latest libdrm release and
>>
>>>>
>>>> Thinking about it... I'm not sure that the release manager will find
>>>> this note here :-\
>>>>
>>>> Daniel V, Marius, do you guys have a tick-off list somewhere where
>>>> this could be added ?
>>>
>>> README and NEWS. Don't know of any other.
>>>
>> Can I ask that you/Daniel can update those ? I'm afraid
> 
> Instead of documenting, can't we enforce this with an autobuilder which
> runs with the stubbed-out libdrm always? Collabora has such a thing I
> heard ;-)

Freedesktop.org has, actually: https://jenkins.freedesktop.org/view/IGT/

For a good while I haven't got any false positives, so maybe I should
configure the job to spam this list or dri-devel?

Regards,

Tomeu

> Imo trying to fix up the build for releases is way too late, we should
> never have a broken build. Or at least not for long.
> -Daniel
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx