RFC: Fix --enable-hg in IcedTea6

Andrew John Hughes gnu_andrew at member.fsf.org
Mon Jun 8 09:13:24 PDT 2009


2009/6/3 Omair Majid <omajid at redhat.com>:
> 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?
>

No.  While the only real value in --enable-hg
--with-hg-revision=jdk6-b16 is testing, the more general default of
using tip is useful for testing stuff in hg but not yet in a tarball.

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

Looks ok.  Please commit.

> Cheers,
> Omair
>

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



More information about the distro-pkg-dev mailing list