RFC: Fix --enable-hg in IcedTea6

[Omair Majid]

Andrew John Hughes wrote:
> 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 :)
Yeah, that's true. I would expect occasional bugs and IcedTea lagging 
behind patches. But failing to build completely with a known good revision?

> Again, it's a lot more useful with 7 because there are lots of rapidly
> changing upstream forests.
> 
Oh. That makes a lot of sense. Should we remove --enable-hg from 6?

>>> 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 :)
> 
No worries.

> 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?
I did try those options with the old patch. See blow for the new list.

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

I got a patch that I *think* works. Here are the configurations I tested:

Just 'make patch'
./autogen.sh && ./configure && make patch
./autogen.sh && ./configure --enable-hg --with-hg-revision=jdk6-b16 && 
make patch
./autogen.sh && ./configure --with-openjdk  && make patch
./autogen.sh && ./configure --with-openjdk && make replace-hotspot
./autogen.sh && ./configure --with-openjdk --disable-docs && make patch
./autogen.sh && ./configure --enable-hg --with-hg-revision=jdk6-b16 
--with-openjdk --disable-docs && make patch
./autogen.sh && ./configure --enable-zero && make patch
./autogen.sh && ./configure --enable-zero --enable-shark && make patch
./autogen.sh && ./configure --enable-plugin --enable-pulse-java 
--enable-visualvm --enable-openjdk-cross-compilation --enable-zero 
--enable-shark --enable-systemtap --enable-cacao  && make patch

Full Build:
./autogen.sh && ./configure && make
./autogen.sh && ./configure --with-openjdk --disable-docs && make


./autogen.sh && ./configure --with-hotspot-build=original && make patch 
failed just like it does with the current Makefile.am (trying to apply 
icedtea-shark.patch)


ChangeLog:
2009-06-03  Omair Majid  <omajid at redhat.com>

     * Makefile.am
     (stamps/ports.stamp): Depend on stamps/replace-hotspot.stamp instead
     of stamps/extract.stamp.
     (stamps/extract.stamp): Dont replace hotspot.
     (stamps/replace-hotspot.stamp): New target. Replace hotspot without
     assuming anything about the build.
     (clean-replace-hotspot): New target.
     (stamps/patch-fsg.stamp): Depend on stamps/replace-hotspot.stamp
     instead of stamps/extract.stamp.
     (stamps/hotspot-tools-source-files.txt): Likewise.
     (rt-source-files.txt): Likewise.
     (stamps/cacao.stamp): Likewise.
     (stamps/visualvm.stamp): Likewise.
     (stamps/nbplatform.stamp): Likewise.
     (replace-hotspot): New alias for stamps/replace-hotspot.stamp.

Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: icedtea6-enable-hg.patch
Type: text/x-patch
Size: 3943 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20090603/4fab1795/icedtea6-enable-hg.patch