tool_chain

[Christian Helmuth]

Hello Julian,

thanks for the patch, but I have some questions...

On Wed, May 27, 2009 at 10:30:22PM +0200, Julian Stecklina wrote:
> diff --git a/tool/tool_chain b/tool/tool_chain
> index e830712..887b899 100755
> --- a/tool/tool_chain
> +++ b/tool/tool_chain
> @@ -14,9 +14,10 @@ BINUTILS_DOWNLOAD_URL = http://ftp.gnu.org/gnu/binutils
>  #
>  # Tool versions and install location
>  #
> -GCC_VERSION      = 4.2.4
> -BINUTILS_VERSION = 2.18
> -INSTALL_LOCATION = /usr/local/genode-gcc
> +GCC_VERSION      ?= 4.2.4
> +BINUTILS_VERSION ?= 2.18
> +INSTALL_LOCATION ?= /usr/local/genode-gcc
> +USE_SUDO         ?= yes

This variable is not used later in the script. Did you mean NOSUDO?

>  #
>  # Platform-specific configuration
> @@ -46,6 +47,13 @@ BRIGHT_COL  = \033[01;33m
>  DEFAULT_COL = \033[0m
>  ECHO        = @echo -e
>  
> +ifdef NOSUDO
> +  SUDO      =
> +  SUDO_MSG  =
> +else
> +  SUDO      = sudo
> +  SUDO_MSG  =  (requires root privileges)
> +endif

I would not care about the SUDO_MSG (or change it to "may require")
and just use SUDO as configuration variable:

  SUDO = sudo   -> use sudo
  SUDO =        -> just call subsequent command

>  help:
>  	$(ECHO) "Build tool chain for the Genode OS Framework"
> @@ -53,11 +61,16 @@ help:
>  	$(ECHO) "The tool chain consists of GCC $(GCC_VERSION) and binutils $(BINUTILS_VERSION)"
>  	$(ECHO) "and will be installed at $(INSTALL_LOCATION)."
>  	$(ECHO)
> +	$(ECHO)
>  	$(ECHO) "--- available commands ---"
>  	$(ECHO) "x86       - download, build, and install gcc and binutils for x86"
>  	$(ECHO) "clean     - clean everything except downloaded archives"
>  	$(ECHO) "cleanall  - clean everything including downloaded archives"
>  	$(ECHO) "uninstall - remove installation from $(INSTALL_LOCATION)"
> +	$(ECHO)
> +	$(ECHO) "--- customization ---"
> +	$(ECHO) "INSTALL_LOCATION=/your/directory - install into /your/directory"
> +	$(ECHO) "NOSUDO=yes                       - do not use sudo"

Please add all customizable variables.

>  #
>  # We use the binaries 'objdump' and 'g++' as examples for declaring
> @@ -101,8 +114,8 @@ binutils-$(BINUTILS_VERSION)/binutils/objdump: binutils-$(BINUTILS_VERSION)/Make
>  	@make -C binutils-$(BINUTILS_VERSION)
>  
>  $(INSTALL_LOCATION)/bin/$(PROGRAM_PREFIX)objdump: binutils-$(BINUTILS_VERSION)/binutils/objdump
> -	$(ECHO) "$(BRIGHT_COL)installing binutils... (requires root privileges)$(DEFAULT_COL)"
> -	@sudo make -C binutils-$(BINUTILS_VERSION) install
> +	$(ECHO) "$(BRIGHT_COL)installing binutils...$(SUDO_MSG)$(DEFAULT_COL)"
> +	@$(SUDO) make -C binutils-$(BINUTILS_VERSION) install
>  
>  gcc_build_$(PLATFORM)/Makefile: gcc-$(GCC_VERSION)
>  	$(ECHO) "$(BRIGHT_COL)configuring gcc...$(DEFAULT_COL)"
> @@ -114,8 +127,8 @@ gcc_build_$(PLATFORM)/gcc/g++: gcc_build_$(PLATFORM)/Makefile
>  	@make -C gcc_build_$(PLATFORM)
>  
>  $(INSTALL_LOCATION)/bin/$(PROGRAM_PREFIX)g++: gcc_build_$(PLATFORM)/gcc/g++
> -	$(ECHO) "$(BRIGHT_COL)installing gcc... (requires root privileges)$(DEFAULT_COL)"
> -	@sudo make -C gcc_build_$(PLATFORM) install
> +	$(ECHO) "$(BRIGHT_COL)installing gcc... $(SUDO_MSG)$(DEFAULT_COL)"
> +	@$(SUDO) make -C gcc_build_$(PLATFORM) install
>  
>  clean:
>  	rm -rf binutils-$(BINUTILS_VERSION) gcc-$(GCC_VERSION)
> @@ -124,7 +137,7 @@ cleanall: clean
>  	rm -rf binutils-$(BINUTILS_VERSION).tar.gz
>  	rm -rf gcc-core-$(GCC_VERSION).tar.bz2
>  	rm -rf gcc-g++-$(GCC_VERSION).tar.bz2
> -	rm -rf gcc_build_$(PLATFORM)
> +	rm -rf gcc_build_x86

Why not use PLATFORM here?

>  uninstall:
> -	sudo rm -rf $(INSTALL_LOCATION)
> +	$(SUDO) rm -rf $(INSTALL_LOCATION)

Cheers
-- 
Christian Helmuth
Genode Labs

http://www.genode-labs.com/ · http://genode.org/