alsa-sane-headers sanity check broken

[Kelly O'Hair]

On Nov 22, 2010, at 1:53 PM, Dr Andrew John Hughes wrote:

> On 22:15 Mon 22 Nov     , Patrick Reinhart wrote:
>> Am 22.11.10 22:09, schrieb Dr Andrew John Hughes:
>>> I'm quite puzzled as to how this hasn't been spotted before now,  
>>> but I
>>> tried to build jdk7/jdk7 today (b118 from hg), using exactly the  
>>> same
>>> script as I usually do, and immediately failed due to a missing
>>> separator in the jdk Sanity.gmk Makefile:
>>>
>>> make[1]: Entering directory `/home/andrew/projects/openjdk/ 
>>> upstream/build/jdk/make'
>>> /home/andrew/projects/openjdk/upstream/build/jdk/make/common/ 
>>> shared/Sanity.gmk:1392: *** missing separator (did you mean TAB  
>>> instead of 8 spaces?).  Stop.
>>> make[1]: Leaving directory `/home/andrew/projects/openjdk/upstream/ 
>>> build/jdk/make'
>>>
>>> I've just confirmed it's also broken in the build tree and the  
>>> icedtea tree
>>> also fails (which is where I first hit the error).
>>>
>>> It seems the sane-alsa-headers target is completely broken.   
>>> What's puzzling
>>> is the changes occur in:
>>>
>>> http://hg.openjdk.java.net/jdk7/jdk7/jdk/rev/30bf00392b6d
>>> changeset:   914:30bf00392b6d
>>> parent:      796:d8eb2738db6b
>>> user:        ohair
>>> date:        Sat Jan 31 17:31:21 2009 -0800
>>> summary:     6799141: Build with --hash-style=both so that  
>>> binaries can work on SuSE 10
>>>
>>> which is nearly two years old.
>>>
>>> I managed to make some headway:
>>>
>>> * The target uses a mix of tabs and spaces.  Replacing with all  
>>> tabs gets things further.
>>> * The next issue is fixed by changing $${alsa_version) to $$ 
>>> {alsa_version}.
>>>
>>> It then fails because we have a conditional structured as:
>>>
>>> if [ "$(ALSA_CHECK)" != "same" -a "$(ALSA_CHECK)" != "newer"  ] ;  
>>> then \
>>> ...
>>> fi \
>>> else \
>>> ...
>>> fi
>>>
>>> There seems to be an if statement missing as, otherwise, having  
>>> REQUIRED_ALSA_VERSION
>>> defined always results in an error.  I assume there should be a  
>>> top level if statement,
>>> similar to the @if [ -f "$(ALSA_VERSION_CHECK)" ]; removed by this  
>>> changeset.  This would
>>> also explain why the if block is printed when usually such things  
>>> are silent.
>>>
>>> Anyone care to enlighten us as to the missing if statement?  I'd  
>>> also love to know how
>>> this has only just started biting me now.
>>>
>>> Thanks,
>> Hi Andrew,
>>
>> See my statement and correcting diff
>> http://mail.openjdk.java.net/pipermail/build-dev/2010-November/003578.html
>> when I tried to get the build running under Fedora 14...
>>
>> Regards Patrick
>>
>>
>
> Yes, as described in my original e-mail, I've already made those  
> changes.
>
> I don't believe the final else block is redundant.  An if check  
> seems to be missing, which
> is further reinforced if you take a look at other checks in the file  
> and the CheckVersions macro;
> we should be looking for ALSA_CHECK being "missing".
>
> I've posted a webrev here: http://cr.openjdk.java.net/~andrew/build/webrev.07/
>
> which contains corrected indenting, the alsa_version fix and the  
> additional if test.
>
> Kelly, does this look ok to push? If so, can I have a bug ID for it?

Looks fine. Feel free to push.

7000225: Sanity check on sane-alsa-headers is broken

Thank you. I filed the bug last week I think, just been so busy  
lately. :^(

-kto

>
> Thanks,
> -- 
> Andrew :)
>
> Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> Support Free Java!
> Contribute to GNU Classpath and the OpenJDK
> http://www.gnu.org/software/classpath
> http://openjdk.java.net
> PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
> Fingerprint = F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8