[rfc][icedtea-web] Generate Bash Completion file

Lukasz Dracz ldracz at redhat.com
Thu Jun 18 16:11:52 UTC 2015


Hello,

I have included an updated patch, that is not finished and doesn't work at the moment.
I have a few questions.

I think the main question is what generates all the install-am and install-*Something*-am 's that then reference something
like install-*SOMETHING*-local from the makefile.am. I can't seem to figure this part. I can see that the automake that generates the .in file from the .am
adds these in but what tells automake to do this ? Is there a convention to follow that once followed means automake will do this ?

----- Original Message -----
> From: "Jiri Vanek" <jvanek at redhat.com>
> To: "IcedTea Distro List" <distro-pkg-dev at openjdk.java.net>, "Lukasz Dracz" <ldracz at redhat.com>,
> aazores at icedtea.classpath.org
> Sent: Sunday, June 14, 2015 2:06:10 PM
> Subject: re: [rfc][icedtea-web] Generate Bash Completion file
> 
> 
> 
> 
> hi!
> 
> A lot of nits just from quick web based reading.
> 
> 
> diff --git a/Makefile.am b/Makefile.am
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -259,13 +259,13 @@
>    check-local: $(RHINO_TESTS) $(JUNIT_TESTS)
> 
>    clean-local: clean-netx clean-plugin clean-liveconnect \
> - clean-native-ecj clean-launchers clean-desktop-files clean-docs
> clean-generated-docs clean-tests clean-bootstrap-directory
> + clean-native-ecj clean-launchers clean-desktop-files clean-docs
> clean-generated-docs clean-generated-bash-completion clean-tests
> clean-bootstrap-directory
>    	if [ -e stamps ] ; then \
>    	  rmdir stamps ; \
>    	fi
> 
>    .PHONY: clean-IcedTeaPlugin clean-add-netx clean-add-netx-debug
>    clean-add-plugin clean-add-plugin-debug \
> - clean-bootstrap-directory clean-native-ecj clean-desktop-files
> clean-netx-docs clean-docs clean-plugin-docs clean-generated-docs \
> + clean-bootstrap-directory clean-native-ecj clean-desktop-files
> clean-netx-docs clean-docs clean-plugin-docs clean-generated-docs
> clean-generated-bash-completion \
>     clean-tests check-local clean-launchers stamps/check-pac-functions.stamp
>     stamps/run-netx-unit-tests.stamp clean-netx-tests \
>     clean-junit-runner clean-netx-unit-tests
> 
> @@ -504,6 +504,12 @@
>    	sed -i '/RhinoBasedPacEvaluator/ d' $@
>    endif
> 
> +stamps/generate-bash-completion.stamp: stamps/netx.stamp
> +	BC_COMMAND="$(SYSTEM_JRE_DIR)/bin/java -cp $(NETX_DIR)
> net.sourceforge.jnlp.OptionsDefinitions" ; \
> +	$$BC_COMMAND; \
> +	sudo mv icedteaweb-completion /etc/bash_completion.d/ ; \
> 
> This is absolyutely no go. You must split this to two targets
>    - first during make. during  make you generate this file o BUILD_DIR
>    - second, during install, you install the genrated file to
>    PREFIX/etc/bash_completion.d/icedteaweb-completion



> You do not need to handle permissions - user decide in configure time what is
> PREFIX, and then in install time he use root or not, acording to his prefix.
> By default prefix is "/" so by default you hav eto be root to call make
> install.
> But only make install. not make.

What is the point of PREFIX ? is it for if the user has /etc/bash_comletion.d/ located under a folder
something like /randomDir/etc/bash_completion.d/ or does it have something to do with permissions ?

> +	touch $@ ;
> +
>    stamps/generate-docs.stamp: stamps/netx.stamp
>    	mkdir -p "$(DOCS_DIR)" ; \
>    	HTML_DOCS_TARGET_DIR="$(DOCS_DIR)/html"  ; \
> @@ -571,7 +577,7 @@
>    	mkdir -p stamps
>    	touch $@
> 
> 
> acordingly the clean phases have to be splited.
> 
> By the way - make install is generated target. Yo just add generated
> icedteaweb-completion to list of cared files.

Where do I add to this list ?

> Also, please dont use stamps fo simple generation. The target is file itself.
> ( see lower)

Okay
 
> -stamps/netx-dist.stamp: stamps/netx.stamp $(abs_top_builddir)/netx.manifest
> stamps/generate-docs.stamp
> +stamps/netx-dist.stamp: stamps/netx.stamp $(abs_top_builddir)/netx.manifest
> stamps/generate-docs.stamp stamps/generate-bash-completion.stamp stamps/
>    	(cd $(NETX_DIR) ; \
>    	 mkdir -p lib ; \
>    	 $(SYSTEM_JDK_DIR)/bin/jar cfm lib/classes.jar \
> @@ -686,6 +692,8 @@
>    	rm -rf "$(DOCS_DIR)"
>    	rm -f stamps/generate-docs.stamp
> 
> +clean-generated-bash-completion:
> +	rm -rf stamps/generate-bash-completion.stamp
> 
> Ok. Here is the bug. What the target  is cleaning?  It is removing stamp! not
> generated file!

Okay use of stamps removed.

>    # check
>    # ==========================
> diff --git a/icedteaweb-completion b/icedteaweb-completion
> deleted file mode 100644
> --- a/icedteaweb-completion
> +++ /dev/null
> 
> Hmm... Why to remove. The file in javacode is absolutely unreadable. I would
> rater move this file to
> icedteaweb-completion.in and durng generation just replaced the stings ifn
> "opts"

Okay moved to icedteaweb-completion.in and added one "-verbose" flag to policyeditor that I noticed was added.

> @@ -1,44 +0,0 @@
> -#/bin/bash
> -#place this file over to /etc/bash_completion.d/ to make this file useful
> -#Note: If you do not have bash-completion you will need to install it
> -_itweb-settings()
> -{
> -    local cur prev opts base
> -    cur="${COMP_WORDS[COMP_CWORD]}"
> -    prev="${COMP_WORDS[COMP_CWORD-1]}"
> -
> -    # Icedtea-web settings Options
> -    opts="-check -get -headless -help -info -list -reset -set -verbose"
> -
> -    COMPREPLY=($(compgen -W "${opts}" -- ${cur}))
> -    return 0
> -}
> -complete -F _itweb-settings itweb-settings
> -
> -_policyeditor()
> -{
> -    local cur prev opts base
> -    cur="${COMP_WORDS[COMP_CWORD]}"
> -    prev="${COMP_WORDS[COMP_CWORD-1]}"
> -
> -    # PolicyEditor Options
> -    opts="-codebase -file -help"
> -
> -    COMPREPLY=($(compgen -W "${opts}" -- ${cur}))
> -    return 0
> -}
> -complete -F _policyeditor policyeditor
> -
> -_javaws()
> -{
> -    local cur prev opts base
> -    cur="${COMP_WORDS[COMP_CWORD]}"
> -    prev="${COMP_WORDS[COMP_CWORD-1]}"
> -
> -    # JavaWs Options
> -    opts="-about -help -license -viewer -Xclearcache -allowredirect -arg
> -headless -html -jnlp -nosecurity -noupdate -param -property -strict -update
> -verbose -version -Xignoreheaders -xml -Xnofork -Xoffline -Xtrustnone"
> -
> -    COMPREPLY=($(compgen -W "${opts}" -- ${cur}))
> -    return 0
> -}
> -complete -F _javaws javaws
> diff --git a/netx/net/sourceforge/jnlp/OptionsDefinitions.java
> b/netx/net/sourceforge/jnlp/OptionsDefinitions.java
> --- a/netx/net/sourceforge/jnlp/OptionsDefinitions.java
> +++ b/netx/net/sourceforge/jnlp/OptionsDefinitions.java
> @@ -36,6 +36,9 @@
>    */
>    package net.sourceforge.jnlp;
> 
> +import java.io.File;
> +import java.io.IOException;
> +import java.nio.file.Files;
>    import java.util.ArrayList;
>    import java.util.Arrays;
>    import java.util.List;
> @@ -223,5 +226,65 @@
>            return l;
>        }
> 
> -
> +    public static void main(String [ ] args) throws IOException {
> +        makeTabCompletionFile();
> +    }
> +
> +    public static String transformToLine (List<OPTIONS>  options) {
> +        String line = "";
> +        for (OPTIONS option : options) {
> +            line = line + option.option + " ";
> +        }
> +        return line;
> +    }
> +    public static void makeTabCompletionFile() throws IOException {
> 
> Is there any difference against orignal file? if no,please use .in format.
> 
> If yes, then please split this patch to two parts. Firt, replace already
> pushed file by this new powerful (and it will be valid also for 1.6!) and as
> second patch, chnage it to .in form, and proceed with generation.

No difference other than the -verbose flag I noticed was missing.

Thanks for the review !

Regards,
Lukasz Dracz

-------------- next part --------------
A non-text attachment was scrubbed...
Name: generate-bash-completion-4.patch
Type: text/x-patch
Size: 4510 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20150618/3bb7fe46/generate-bash-completion-4-0001.patch>


More information about the distro-pkg-dev mailing list