RFC: Fix --enable-hg in IcedTea6

[Andrew John Hughes]

2009/6/1 Omair Majid <omajid at redhat.com>:
> Andrew John Hughes wrote:
>>
>> 2009/6/1 Omair Majid <omajid at redhat.com>:
>>>
>>> Hi,
>>>
>>> Trying to build IcedTea6 with "--enable-hg --with-hg-revision=jdk6-b16"
>>> fails when applying the patches, even though jdk6-b16 is a known good
>>> build.
>>> The issues appears to be that the Makefile removes the old hotspot only
>>> if
>>> the openjdk directory doesn't already exist.
>>>
>>> The attached patch checks USE_HG and then removes the hotspot directory
>>> so
>>> it can be replaced with HS14.
>>>
>>> ChangeLog:
>>> 2009-06-01  Omair Majid  <omajid at redhat.com>
>>>
>>>   * Makefile.am (stamps/extract.stamp): Remove hotspot if using
>>>   --enable-hg and an alternate hotspot build.
>>>
>>> Can someone please review the patch?
>>>
>>> Cheers,
>>> Omair
>>>
>>
>> The presumption was that anyone wanting to use Mercurial as a source
>> wanted the whole thing.
>
> I see. What about the patches then? Should we apply patches from the
> original hotspot? Or leave out the hotspot patches completely?
>

Sorry, I should have been more clear.  I was just explaining why the
current status quo is as it is.  Changing it as you say is the right
thing to do.  Things are different in 7 (we don't replace HotSpot,
there are a heck of a lot more forests) so it makes more sense to use
a pristine checkout.

BTW, I wouldn't worry too much about --enable-hg not building.  It's
really there for people who want to live life on the edge i.e. if you
want to use live Mercurial sources to build, you should know what
you're letting yourself in for :)
Again, it's a lot more useful with 7 because there are lots of rapidly
changing upstream forests.

>>
>> I don't see how the attached patch can work.  This would stop the
>> hotspot directory not being removed when Mercurial is disabled, and
>> doesn't do anything about the if block which checks for the openjdk
>> directory.
>
> Err...too many negatives in that sentence, so apologies if I misunderstand
> something.

Sorry on my part :)

The patch works (as in I can do a 'make patch' with this change
> applied and everything works)

With what configuration options? Do both non-hg and hg builds still work?
Does a full build work?

. If mercurial is disabled, it does nothing -
> it only comes into play if --enable-hg is used. In that case, it deletes the
> hotspot directory so the alternate hotspot tarball is used.

This is my point; we should always delete the hotspot directory and
use an alternate tarball if alternate HotSpot is turned on (which it
is by default).  Looking at this, it will stop HotSpot being replaced
for a normal build and we'll end up with hs11 and broken patches...

> Currently
> hotspot is only replaced if there is no openjdk directory. In case of
> --enable-hg, there is, so hotspot wasnt being replaced.
> The patch explicitly
> rm -rf's the old hotspot direcotry so it is replaced with HS14.

Right, but that should happen, hg or no hg.  With your patch, it's only with hg.

> Anyway, the
> entire point is moot if the purpose of --enable-hg is to provide the
> unmodified hotspot.

In the case of 6, I think we should still replace HotSpot.

>
>>
>> stamps/extract.stamp: stamps/download.stamp
>> if OPENJDK_SRC_DIR_FOUND
>>        cp -a $(OPENJDK_SRC_DIR) openjdk
>> else
>>        if ! test -d openjdk ; \
>>        then \
>>          mkdir openjdk ; \
>>          $(TAR) xf $(OPENJDK_SRC_ZIP) -C openjdk; \
>>          chmod -R ug+w openjdk ; \
>>          if test "x${HSBUILD}" != "xoriginal"; then \
>>            rm -rf openjdk/hotspot ; \
>>          fi ; \
>>          sh $(abs_top_srcdir)/fsg.sh ; \
>>        fi
>> if WITH_ALT_HSBUILD
>>        if test -e ${HOTSPOT_SRC_ZIP} ; \
>>        then \
>>          if ! test -d openjdk/hotspot ; \
>>          then \
>>            $(TAR) xf $(HOTSPOT_SRC_ZIP) ; \
>>            chmod -R ug+w master-* ; \
>>            mv master-$$($(AWK) 'version==$$1 {print $$2}'
>> version=$(HSBUILD) \
>>              $(abs_top_srcdir)/hotspot.map) openjdk/hotspot ; \
>>          fi ; \
>>        fi
>> endif
>> endif
>>
>> The simplest solution is probably to add an additional target for the
>> HotSpot replacement so it isn't so tightly related to extraction.
>
> That makes sense, I will see what I can do.
>

I think it would make the whole thing less confusing and easier to work on.
Thanks.

> Cheers,
> Omair
>

-- 
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