alsa-sane-headers sanity check broken

[Dr Andrew John Hughes]

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?

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