[rfc][icedtea] makefiel part of fix of rhbz 580478

[Omair Majid]

On 01/05/2012 10:33 AM, Jiri Vanek wrote:
> 2011-12-21  Jiri Vanek <jvanek at redhat.com>
>
>      fixes rhbz#580478
>      * Makefile.am:
>      (EXTRA_DIST) console.desktop policytool.desktop replaced by *.in files
>      (clean-local) now depends on new target clean-desktop-files
>      (jconsole.desktop) new target which transforms
>      jconsole.desktop.in to jconsole.desktop, using default or set value
>      (policytool.desktop) new target which transforms
>      policytool.desktop.in to  policytool.desktop, using default or set
> value
>      (clean-desktop-files) new target, removes policytool.desktop and
> jconsole.desktop
>      (stamps/icedtea.stamp) now depends also on console.desktop
> policytool.desktop
>      (stamps/icedtea-debug.stamp) likewise, as enforced by
> stamps/icedtea.stamp comment
>      * configure.ac: abs-install-dir block moved outside conditional
> ENABLE_SYSTEMTAP block
>      * jconsole.desktop:  removed, replaced by jconsole.desktop.in
>      * jconsole.desktop.in: new file, stub for desktop file, have variable
>       to be substitued during make
>      * policytool.desktop:  removed, replaced by policytool.desktop.in
>      * policytool.desktop.in: new file, stub for desktop file, have
> variable
>       to be substituted during make
>
>
> Flowing Andrew's advice, now using abs-install-dir.
> Your remaining hints were also fixed.  Looks better now :).
>

Thanks. The patch looks fine to me. I do have some notes/suggestions 
below, but you can ignore them.

>
>
> diff -r cf80d2049346 Makefile.am
> --- a/Makefile.am	Tue Dec 20 13:49:11 2011 -0500
> +++ b/Makefile.am	Thu Jan 05 15:59:47 2012 +0100
> @@ -616,7 +616,7 @@
>   	$(top_srcdir)/patches/* \
>   	contrib arm_port \
>   	overlays \
> -	jconsole.desktop policytool.desktop \
> +	jconsole.desktop.in policytool.desktop.in \
>   	$(JTREG_SRCS) HACKING pulseaudio fsg.sh \
>   	hotspot.map \
>   	autogen.sh \
> @@ -641,7 +641,7 @@
>    clean-icedtea-against-ecj clean-extract-ecj clean-generated clean-replace-hotspot \
>    clean-rewriter clean-rewrite-rhino clean-rt clean-bootstrap-directory \
>    clean-bootstrap-directory-ecj clean-bootstrap-directory-symlink \
> - clean-bootstrap-directory-symlink-ecj clean-fonts
> + clean-bootstrap-directory-symlink-ecj clean-fonts clean-desktop-files
>   	if [ -e bootstrap ]; then \
>   	  rmdir bootstrap ; \
>   	fi
> @@ -702,8 +702,21 @@
>   # OpenJDK Source Preparation Targets
>   # ==================================
>
> -# Download OpenJDK sources.
> +jconsole.desktop: jconsole.desktop.in
> +	xabs_path=$(ABS_JAVA_HOME_DIR) ; \
> +	if [ "$$xabs_path" =  ""  ]; then \
> +	  xabs_path=/usr ; \
> +	fi ; \
> +	sed s, at JCONSOLE_DESKTOP@,$$xabs_path/bin/jconsole,<  jconsole.desktop.in>  jconsole.desktop

It might be safer to use @JCONSOLE_DESTKOP[@] (or some variation 
involving putting the @ in square brackets) to ensure that if an 
autoconf var named JCONSOLE_DESKTOP is ever defined it wont replace 
@JCONSOLE_DESTKOP@ at configure-time.

>
> +policytool.desktop: policytool.desktop.in
> +	xabs_path=$(ABS_JAVA_HOME_DIR) ; \
> +	if [ "$$xabs_path" =  ""  ]; then \
> +	  xabs_path=/usr ; \
> +	fi ; \
> +	sed s, at POLICYTOOL_DESKTOP@,$$xabs_path/bin/policytool,<  policytool.desktop.in>  policytool.desktop
> +

As it stands, you could also have used AC_CONFIG_FILES to perform the 
substitution at configure-time. However this version will work better 
once prefix is supported in icedtea6. Nice!

>
> +clean-desktop-files:
> +	-rm -f policytool.desktop
> +	-rm -f jconsole.desktop
> +

I am curious why the '-' is required. I thought -f will stop rm from 
returning an error exit status if the file is not present.

Anyway, okay for HEAD.

Cheers,
Omair