[Mesa-dev] [PATCH 1/3] mesa: Add toplevel Android.mk

[Chia-I Wu]

On Wed, Aug 17, 2011 at 5:59 AM, Chad Versace <chad at chad-versace.us> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 08/16/2011 12:30 AM, Chia-I Wu wrote:
>> Hi Chad,
>>
>> On Tue, Aug 16, 2011 at 2:14 PM, Chad Versace <chad at chad-versace.us>
>> wrote:
>>> This is the first step in porting libGLES* and libEGL to Android.
>> I also has a branch[1] that is ready for merge IMHO and is known to work.
>> I do not want to run into a hypothetical situation that we review and NAK
>> each other.  Shouldn't it be better if we and the list discuss what is the
>> best way to merge Android support before going on[2]?
>
> I also wish to avoid a mutual NAK quagmire. Perhaps if we each explicitly list
> our immediate and longterm goals, we can find common ground and move forward.
>
> Immediate Goals
> - ---------------
> 1. That the i965 driver have alpha quality sometime in October, though sooner
> is better.
> 2. That the Android build rarely break for Intel. (This necessitates factoring
> out common source lists into sources.mk's).
> 3. That Android support exist on master.
>
> Longterm Goals
> - --------------
> 4. That the Android i965 driver be well-tested with Piglit.
>
> Regarding goal 2, I think your branch needs reworking.
>
>> I will talk about my approach first.  I plan to submit a series of patches
>> that are either small or isolated.  They can be reviewed as usual.  Then
>> there is a patch that adds Android.mk's to mesa's source tree.  The patch
>> only adds new files for use by Android, and nothing else is touched.  The
>> idea is that at that point, the Android support is known to work, and no
>> regression is possible with the existing ways of building mesa.  Those
>> familiar with Android's build system can review the new build system as a
>> whole by just looking at this patch and give it a try, instead of looking
>> at a fraction at a time.
>>
>> The upside of this approach is that it works and is ready for review. The
>> downside is that it suffers from the same pain as SCons does: when Makefile
>> is changed, Android.mk needs to be updated.
>>
>> If that is a concern,
>
> That is is serious concern of mine. I do not want the i965 driver and
> associated libraries to break when their respective Makefiles change.
>
>> then I would like to suggest another approach that needs help from you
>> and/or José.  The idea is for us to isolate the bits in Makefile's that can
>> be shared with SCons and move them to, say, sources.mk's.  Then we can
>> include sources.mk's in Makefile's and have SCons parses them too.  This
>> way we can improve the existing build systems without cluttering mesa.
>> Then, I will redo my last patch to use sources.mk's.
>>
>> Do you have other concerns regarding either approach?
>
> If we take the approach of "commit all the working Android.mk's now, then
> cleanup later", then all the Android.mk's will immediately require fixing in
> order to use the sources.mk's. This effectively makes those Android.mk's
> throw-away code.
>
> I insist that we do not commit throw-away code. At best, throw-away code
> eventually does get fixed, but at the cost of laborious code-churn. At worst,
> it languishes in disrepair due to procrastination.
These concerns all seem to suggest that we should factor out common
source list to sources.mk's first.  I think we can agree on this and
continue from here.

I prefer not to have such clean-up work disguised as Android support.
Firstly, for people reviewing the clean-up work, changes for Android
are noises, and vice versa.  And since the patches may be sent out
over days, there is no guarantee that we won't end up with a partially
converted tree.  I do not suggest that everything should be converted.
 But it makes sense to convert those that can benefit SCons.
>> As implied by above, the main concern I have with your approach (from the
>> last series) is that I could not verify it and see the whole thing come
>> together.  You could end up doing what I have been doing for Android-x86,
>> and I may have to redo it again for gallium provided your approach is
>> adopted.  The other concern is that I do not think
>>
>> patch 1: fix src/mesa/Makefile patch 2: add src/mesa/Android.mk patch 3:
>> fix src/mesa/drivers/Makefile patch 4: add src/mesa/drivers/Android.mk
>> (follow this pattern for each directory)
>>
>> is cleaner than fixing Makefile's first (say for SCons) and then add
>> Android.mk's as a third build system.
>
> I do not think posting, or committing, a 2382 line patch is a good idea
> either. Such a patch is impossible to meaningfully review. No one can
> understand it in its entirety, and no one can post inline comments.
I can split the patch up into as many patches as they make sense
(within a day, if that helps).

Also, my concerns regarding your series are not addressed.  It is hard
to verify the work (e.g., it could run into issues when linking the
final i965_dri).  Suppose everything builds, do you plan to integrate
with Android or Android-x86 and how?  There could be more questions,
to both our works.  I think this is another reason not to mix
factoring out source lists to sources.mk's with Android support.

>
> - --
> Chad Versace
> chad at chad-versace.us
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iQIcBAEBAgAGBQJOSug9AAoJEAIvNt057x8i548P/0TYsXFD6aCFV73gxnq+QE7p
> 85J5KTII9SCR6mNvJJSgW1fQUiHb4ilju1NLvkYVBOfJ2QmzxndrdN7x4DQ4WRAl
> 9gya38yGgw+ACXAYYh9q2XztHAPE4H77pwwwTGKBUD4jHxZB6sAwrFM1DjVr5+EB
> gAKiwL55RZiW/v8VJxVs/riZtHfif2f4F6fXYx7c1/W5xrlKsdROj4KX62fFemUn
> +qZpny/95cGqbkK51udEgoSshz8Svsy7Pi0+UZGYQG9cuuY9YK5rufFJWnGJnTJ0
> w17GLXMlp1054coq+dvZj8Krh/KCRmR2fs8VGXbtXFx2KhrbjRbYexUvWnqWHZjr
> Vf4n59kB5MUFXuhwFD/OBxU5R5EM5GNowoz2D+wLeACO4lIQRcd8lKy9ybeaqRoz
> E1j5uPauXKJ2LEE94RHN+H7v2ObobcsE+si/mdFHKBWA6FoSZBe676bBdvbuDEJz
> rgID9dxbF2Cgsax3wA91cbvYtktMPyObfH7zqnWy1vDW267MnE+NqohqzJa8NEcC
> OgKnsrsEYvId9T4UXI1UYWZhqgj+zBttsQs3RGVtUCJm3Tpx3rbnJnnYYIAIiuPc
> fFtVSXkrOv1AJzRnOwPFHDX8CUwqEuz880uljY2KFzChS9yBeV7PGiK7IIxRBI6l
> IM4Ui4iwGjfHdt/C5j9p
> =6Fdt
> -----END PGP SIGNATURE-----
>



-- 
olv at LunarG.com