xubuntu-dev team mailing list archive

[Steve Langasek <mp+435314@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing

Let me know if you want to rebase and merge this to deal with the unicode and whitespace breakage on my side.  Want to get agreement on the dropping of the xubuntu-minimal-specific preseed stuff first tho!

Diff comments:

> diff --git a/Makefile b/Makefile
> index c0440fc..c5d0d03 100755
> --- a/Makefile
> +++ b/Makefile
> @@ -2,7 +2,7 @@
>  
>  # Main Makefile for YACS
>  #
> -# Copyright 1999 Rapha�l Hertzog <hertzog@xxxxxxxxxx>
> +# Copyright 1999 Rapha�l Hertzog <hertzog@xxxxxxxxxx>

Unfortunately this file is in iso8859-1 encoding.  Either this file can be converted to utf-8, or this line needs to be preserved. (in the diff it appears the non-utf8 character was replaced by a literal '�'

>  # See the README file for the license
>  
>  # The environment variables must have been set
> @@ -251,34 +251,42 @@ else
>   CDBASE = $(CODENAME)-desktop-$(FULLARCH)
>      endif
>     else
> -    ifneq (,$(findstring $(CODENAME),warty hoary breezy))
> - CDBASE = $(CODENAME)-live-$(FULLARCH)
> +	ifeq ($(PROJECT),xubuntu)

space-to-tab munging; please use 4 spaces instead of a tab

> +     ifeq ($(SUBPROJECT),minimal)
> + CDBASE = $(CODENAME)-minimal-$(FULLARCH)
> +     else
> + CDBASE = $(CODENAME)-desktop-$(FULLARCH)
> +     endif
>      else
> -     ifeq ($(PROJECT),ubuntu-server)
> - CDBASE = $(CODENAME)-live-server-$(FULLARCH)
> +     ifneq (,$(findstring $(CODENAME),warty hoary breezy))
> + CDBASE = $(CODENAME)-live-$(FULLARCH)

inserting it at the top makes for a confusing diff.  Could we add the minimal subproject handling toward the bottom instead, where the existing SUBPROJECT cases are?  I don't think it needs to be guarded by a xubuntu check, if someone else wanted a 'minimal' image I think we would want to default to matching handling.

>       else
> -      ifeq ($(PROJECT),ubuntu-mid)
> - CDBASE = $(CODENAME)-mid-$(FULLARCH)
> +      ifeq ($(PROJECT),ubuntu-server)
> + CDBASE = $(CODENAME)-live-server-$(FULLARCH)
>        else
> -       ifeq ($(PROJECT),ubuntu-netbook)
> - CDBASE = $(CODENAME)-netbook-$(FULLARCH)
> +       ifeq ($(PROJECT),ubuntu-mid)
> + CDBASE = $(CODENAME)-mid-$(FULLARCH)
>         else
> -        ifeq ($(PROJECT),kubuntu-netbook)
> +        ifeq ($(PROJECT),ubuntu-netbook)
>   CDBASE = $(CODENAME)-netbook-$(FULLARCH)
>          else
> -         ifeq ($(PROJECT),ubuntu-moblin-remix)
> - CDBASE = $(CODENAME)-moblin-remix-$(FULLARCH)
> +         ifeq ($(PROJECT),kubuntu-netbook)
> + CDBASE = $(CODENAME)-netbook-$(FULLARCH)
>           else
> -          ifeq ($(PROJECT),kubuntu-mobile)
> - CDBASE = $(CODENAME)-mobile-$(FULLARCH)
> +          ifeq ($(PROJECT),ubuntu-moblin-remix)
> + CDBASE = $(CODENAME)-moblin-remix-$(FULLARCH)
>            else
> -           ifeq ($(SUBPROJECT),canary)
> - CDBASE = $(CODENAME)-desktop-canary-$(FULLARCH)
> +           ifeq ($(PROJECT),kubuntu-mobile)
> + CDBASE = $(CODENAME)-mobile-$(FULLARCH)
>             else
> -            ifeq ($(SUBPROJECT),legacy)
> - CDBASE = $(CODENAME)-desktop-legacy-$(FULLARCH)
> +            ifeq ($(SUBPROJECT),canary)
> + CDBASE = $(CODENAME)-desktop-canary-$(FULLARCH)
>              else
> +             ifeq ($(SUBPROJECT),legacy)
> + CDBASE = $(CODENAME)-desktop-legacy-$(FULLARCH)
> +             else
>   CDBASE = $(CODENAME)-desktop-$(FULLARCH)
> +             endif
>              endif
>             endif
>            endif
> @@ -287,7 +295,7 @@ else
>         endif
>        endif
>       endif
> -    endif
> +	endif

whitespace

>     endif
>    endif
>   endif
> @@ -1285,7 +1293,7 @@ pi-makelist:
>  # Generate only one image number $(CD)
>  image: bin-image
>  bin-image: ok bin-md5list $(OUT)
> -	@echo "Generating the binary iso image n�$(CD) ..."
> +	@echo "Generating the binary iso image n�$(CD) ..."
>  	@test -n "$(CD)" || (echo "Give me a CD=<num> parameter !" && false)

more unicode breakage

>  	set -e; cd $(BDIR); opts=`cat $(CD).mkisofs_opts`; \
>  	 volid=`cat $(CD).volid`; \
> @@ -1309,7 +1317,7 @@ bin-image: ok bin-md5list $(OUT)
>           fi
>  
>  src-image: ok src-md5list $(OUT)
> -	@echo "Generating the source iso image n�$(CD) ..."
> +	@echo "Generating the source iso image n�$(CD) ..."

more unicode breakage

>  	@test -n "$(CD)" || (echo "Give me a CD=<num> parameter !" && false)
>  	set -e; cd $(SDIR); opts=`cat $(CD).mkisofs_opts`; \
>  	 volid=`cat $(CD).volid`; \
> diff --git a/data/lunar/preseed/xubuntu-minimal/xubuntu-minimal.seed b/data/lunar/preseed/xubuntu-minimal/xubuntu-minimal.seed
> new file mode 100644
> index 0000000..8751c89
> --- /dev/null
> +++ b/data/lunar/preseed/xubuntu-minimal/xubuntu-minimal.seed
> @@ -0,0 +1,5 @@
> +# Install the Xubuntu desktop.
> +tasksel	tasksel/first	multiselect xubuntu-desktop-minimal
> +d-i	preseed/early_command	string . /usr/share/debconf/confmodule; db_get debconf/priority; case $RET in low|medium) db_fset tasksel/first seen false; echo 'tasksel tasksel/first seen false' >>/var/lib/preseed/log ;; esac
> +# No XFCE translation packages yet.
> +d-i	pkgsel/language-pack-patterns	string

I still think this can be dropped and use a single xubuntu.seed for both images.  ubiquity doesn't make any use of tasksel for determining what packages to install on the target system - it simply installs the packages already present as part of the live image!



-- 
https://code.launchpad.net/~xubuntu-dev/debian-cd/+git/debian-cd/+merge/435314
Your team Xubuntu Developers is subscribed to branch ~xubuntu-dev/debian-cd:xubuntu-core.