RFC: JamVM - Taste the New Flavour! --with-jamvm-src-zip option - committed.

Wed Feb 23 08:47:02 PST 2011

On 2011-02-23 17:35, Dr Andrew John Hughes wrote:
> On 10:27 Wed 23 Feb     , Xerxes Rånby wrote:
>> On 2011-02-22 02:48, Xerxes Rånby wrote:
>>> On 2011-02-21 22:40, Robert Lougher wrote:
>>>> Hi,
>>>>
>>>> On 21 February 2011 21:22, Dr Andrew John Hughes<ahughes at redhat.com>
>>>> wrote:
>>>>> On 21:20 Sat 19 Feb , Xerxes Ranby wrote:
>>>>>> Greetings!
>>>>>>
>>>>>> "JamVM's got a shiny new Git repository, which contains the port to the
>>>>>> OpenJDK class-library."
>>>>>> http://draenog.blogspot.com/2011/02/openjdkjamvm-git-repository.html
>>>>>>
>>>>>> The attached patch adds Robert Lougher's new JamVM flavour to Icedtea6!
>>>>>> +stamps/download-jamvm.stamp:
>>>>>> +if BUILD_JAMVM
>>>>>> + if ! echo "$(JAMVM_SHA256SUM) $(JAMVM_SRC_ZIP)" \
>>>>>> + | $(SHA256SUM) --check ; \
>>>>>> + then \
>>>>>> + if [ $(JAMVM_SRC_ZIP) ] ; \
>>>>>> + then \
>>>>>> + mv $(JAMVM_SRC_ZIP) $(JAMVM_SRC_ZIP).old ; \
>>>>>> + fi ; \
>>>>>> + $(WGET) $(JAMVM_URL) -O $(JAMVM_SRC_ZIP); \
>>>>>> + if ! echo "$(JAMVM_SHA256SUM) $(JAMVM_SRC_ZIP)" \
>>>>>> + | $(SHA256SUM) --check ; \
>>>>>> + then echo "ERROR: Bad download of JamVM zip"; false; \
>>>>>> + fi; \
>>>>>> + fi
>>>>>> +endif
>>>>>> + mkdir -p stamps
>>>>>> + touch $@
>>>>> We need an option equivalent to --with-cacao-src-zip. I'd suggest
>>>>> adding this in a follow up patch. As with the drop tarballs, we
>>>>> should verify the checksum for pre-provided tarballs as well as
>>>>> download ones. It should be just a matter of symlinking the tarball
>>>>> here if an alternate one is specified.
>>>>>
>>> Noted, I will add --with-cacao-src-zip in another follow up patch.
>>>
>> Hi again!
>>
>> I have attached the follow up patch to implement the new
>> --with-jamvm-src-zip option.
>>
>> with-jamvm-src-zip.23feb.patch
>>
>> Ok to push?
> Comments inline.
>
>> Cheers
>> Xerxes
>> Index: icedtea6/Makefile.am
>> ===================================================================
>> --- icedtea6.orig/Makefile.am	2011-02-23 09:20:37.000000000 +0100
>> +++ icedtea6/Makefile.am	2011-02-23 09:34:26.000000000 +0100
>> @@ -768,6 +768,9 @@
>>  
>>  stamps/download-jamvm.stamp:
>>  if BUILD_JAMVM
>> +if USE_ALT_JAMVM_SRC_ZIP
>> +	ln -sf $(ALT_JAMVM_SRC_ZIP) $(JAMVM_SRC_ZIP)
>> +endif
>>  	if ! echo "$(JAMVM_SHA256SUM)  $(JAMVM_SRC_ZIP)" \
>>  	 | $(SHA256SUM) --check ; \
>>  	then \
>> Index: icedtea6/acinclude.m4
>> ===================================================================
>> --- icedtea6.orig/acinclude.m4	2011-02-23 09:23:56.000000000 +0100
>> +++ icedtea6/acinclude.m4	2011-02-23 09:30:05.000000000 +0100
>> @@ -875,6 +875,23 @@
>>    AC_SUBST(ENABLE_JAMVM)
>>  ])
>>  
>> +AC_DEFUN([IT_CHECK_WITH_JAMVM_SRC_ZIP],
>> +[
>> +  AC_MSG_CHECKING([for a JamVM source zip])
>> +  AC_ARG_WITH([jamvm-src-zip],
>> +	      [AS_HELP_STRING(--with-jamvm-src-zip,specify the location of the JamVM source zip)],
>> +  [
>> +    ALT_JAMVM_SRC_ZIP=${withval}
>> +    AM_CONDITIONAL(USE_ALT_JAMVM_SRC_ZIP, test x = x)
>> +  ],
>> +  [
>> +    ALT_JAMVM_SRC_ZIP="not specified"
>> +    AM_CONDITIONAL(USE_ALT_JAMVM_SRC_ZIP, test x != x)
>> +  ])
>> +  AC_MSG_RESULT(${ALT_JAMVM_SRC_ZIP})
>> +  AC_SUBST(ALT_JAMVM_SRC_ZIP)
>> +])
>> +
> It might be cleaner to have one AM_CONDITIONAL:
>
> AM_CONDITIONAL(USE_ALT_JAMVM_SRC_ZIP, test x${ALT_JAMVM_SRC_ZIP} != "xnot specified")
>
> I think the likelihood of the zip being called 'not specified' is suitably low...
>
> Also it would be good to check it's actually a valid file, which should also handle the yes/no
> produced by --with-jamvm-src-zip and --without-jamvm-src-zip.
> Something like:
>
> if [ ! -f ${ALT_JAMVM_SRC_ZIP ] ; then
>   AC_MSG_ERROR([Invalid JamVM source zip specified: ${ALT_JAMVM_SRC_ZIP}])
> fi
>
> in the first block.
>
> I realise I'm probably holding this to higher standards than some of the other macros.
> That's because those other macros probably weren't reviewed... ;-)
>
>>  AC_DEFUN([IT_CHECK_ENABLE_CACAO],
>>  [
>>    AC_MSG_CHECKING(whether to use CACAO as VM)
>> Index: icedtea6/configure.ac
>> ===================================================================
>> --- icedtea6.orig/configure.ac	2011-02-23 09:30:52.000000000 +0100
>> +++ icedtea6/configure.ac	2011-02-23 09:31:34.000000000 +0100
>> @@ -177,6 +177,7 @@
>>  IT_CHECK_WITH_CACAO_SRC_DIR
>>  
>>  IT_CHECK_ENABLE_JAMVM
>> +IT_CHECK_WITH_JAMVM_SRC_ZIP
>>  
>>  IT_DISABLE_OPTIMIZATIONS
>>  IT_SET_SHARK_BUILD
>> Index: icedtea6/ChangeLog
>> ===================================================================
>> --- icedtea6.orig/ChangeLog	2011-02-23 09:43:06.000000000 +0100
>> +++ icedtea6/ChangeLog	2011-02-23 10:19:30.000000000 +0100
>> @@ -1,5 +1,16 @@
>>  2011-02-23  Xerxes Ranby  <xerxes at zafena.se>
>>  
>> +	JamVM: configure --with-jamvm-src-zip
>> +	* NEWS: Updated.
>> +	* Makefile.am (stamps/download-jamvm.stamp):
>> +	Link in jamvm source zip dependent on USE_ALT_JAMVM_SRC_ZIP
>> +	* acinclude.m4 (IT_CHECK_WITH_JAMVM_SRC_ZIP): New macro.
>> +	(USE_ALT_JAMVM_SRC_ZIP): New conditional.
>> +	(ALT_JAMVM_SRC_ZIP): New variable.
>> +	* configure.ac: Call IT_CHECK_WITH_JAMVM_SRC_ZIP.
>> +
>> +2011-02-23  Xerxes Ranby  <xerxes at zafena.se>
>> +
>>  	JamVM: Try --with-additional-vms=jamvm topping!
>>  	* NEWS: Updated.
>>  	* Makefile.am (stamps/add-jamvm.stamp): New make target.
>> Index: icedtea6/NEWS
>> ===================================================================
>> --- icedtea6.orig/NEWS	2011-02-23 09:35:17.000000000 +0100
>> +++ icedtea6/NEWS	2011-02-23 09:39:53.000000000 +0100
>> @@ -18,6 +18,8 @@
>>  * Added out-of-the-box JamVM support using --enable-jamvm
>>  * Allow building of JamVM beside the default VM by using
>>    --with-additional-vms=jamvm
>> +* Distributions can specify the location of the JamVM source zip by using
>> +  --with-jamvm-src-zip
>>  * Import of OpenJDK6 b21 including upgrade to HotSpot 19
>>    - S6961870: More rebranding fixes for templates/gpl-*-header files
>>    - S6976186: Shark build system changes
> There's nothing distro-specific about using --with-jamvm-src-zip. I use it
> on a daily basis (and haven't yet tried the JamVM build as it's missing).
> I would just say:
>
> 'Allow the location of the JamVM source zip to be specified using --with-jamvm-src-zip'.

NEWS entry fixed, I have committed initial version to:
http://icedtea.classpath.org/hg/icedtea6/rev/a4352c2385ef

> You should also document it in INSTALL.  I just added a JamVM section for you ;-)
Thank you for writing the JamVM INSTALL section!

Cheers
Xerxes