public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation
@ 2017-11-05  9:18 changbin.du
  2017-11-05  9:18 ` [PATCH 1/4] x86, build: Fact out fdimage/isoimage generation commands to standalone script changbin.du
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: changbin.du @ 2017-11-05  9:18 UTC (permalink / raw)
  To: hpa, tglx; +Cc: mingo, x86, linux-kernel, yamada.masahiro, Changbin Du

From: Changbin Du <changbin.du@intel.com>

This serias is a expansion of below original patch after discussion with Ingo
and Yamada.
[PATCH] x86, build: Improve the isolinux searching of isoimage generation

Changbin Du (4):
  x86, build: Fact out fdimage/isoimage generation commands to
    standalone script
  x86, build: Add new paths for isolinux.bin and ldlinux.c32
  x86, build: Specify -input-charset=utf-8 for mkisofs
  x86, boot: Add some generated files to the .gitignore file

 arch/x86/boot/.gitignore  |   3 ++
 arch/x86/boot/Makefile    |  59 +++++------------------
 arch/x86/boot/genimage.sh | 119 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+), 48 deletions(-)
 create mode 100644 arch/x86/boot/genimage.sh

-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] x86, build: Fact out fdimage/isoimage generation commands to standalone script
  2017-11-05  9:18 [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation changbin.du
@ 2017-11-05  9:18 ` changbin.du
  2017-11-05  9:32   ` Ingo Molnar
  2017-11-05  9:18 ` [PATCH 2/4] x86, build: Add new paths for isolinux.bin and ldlinux.c32 changbin.du
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: changbin.du @ 2017-11-05  9:18 UTC (permalink / raw)
  To: hpa, tglx; +Cc: mingo, x86, linux-kernel, yamada.masahiro, Changbin Du

From: Changbin Du <changbin.du@intel.com>

The build message for fdimage/isoimage are pretty unstructured. The raw
shell command blocks are printed. We can improve them as regular build
system messages. Besides, writing commands in shell script is much more
easier than in a Makefile.

See Ingo's suggestion here https://lkml.org/lkml/2017/10/31/124.

This patch fact out the commands used for fdimage/isoimage generation from
arch/x86/boot/Makefile to new script arch/x86/boot/genimage.sh. Then add a
new kbuild command 'genimage' which invokes the new script. All
fdimages/isoimage now is generated by call to 'genimage' with different
parameters.

Now 'make isoimage' becomes:
...
Kernel: arch/x86/boot/bzImage is ready  (#30)
  GENIMAGE arch/x86/boot/image.iso
Size of boot image is 4 sectors -> No emulation
 15.37% done, estimate finish Sun Nov  5 23:36:57 2017
 30.68% done, estimate finish Sun Nov  5 23:36:57 2017
 46.04% done, estimate finish Sun Nov  5 23:36:57 2017
 61.35% done, estimate finish Sun Nov  5 23:36:57 2017
 76.69% done, estimate finish Sun Nov  5 23:36:57 2017
 92.00% done, estimate finish Sun Nov  5 23:36:57 2017
Total translation table size: 2048
Total rockridge attributes bytes: 659
Total directory bytes: 0
Path table size(bytes): 10
Max brk space used 0
32608 extents written (63 MB)
Kernel: arch/x86/boot/image.iso is ready

Before:
Kernel: arch/x86/boot/bzImage is ready  (#63)
rm -rf arch/x86/boot/isoimage
mkdir arch/x86/boot/isoimage
for i in lib lib64 share end ; do \
	if [ -f /usr/$i/syslinux/isolinux.bin ] ; then \
		cp /usr/$i/syslinux/isolinux.bin arch/x86/boot/isoimage ; \
		if [ -f /usr/$i/syslinux/ldlinux.c32 ]; then \
			cp /usr/$i/syslinux/ldlinux.c32 arch/x86/boot/isoimage ; \
		fi ; \
		break ; \
	fi ; \
	if [ $i = end ] ; then exit 1 ; fi ; \
done
...

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Changbin Du <changbin.du@intel.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
---
 arch/x86/boot/Makefile    |  59 +++++---------------------
 arch/x86/boot/genimage.sh | 105 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+), 48 deletions(-)
 create mode 100644 arch/x86/boot/genimage.sh

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index d88a2fd..9b5adae 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -123,63 +123,26 @@ image_cmdline = default linux $(FDARGS) $(if $(FDINITRD),initrd=initrd.img,)
 $(obj)/mtools.conf: $(src)/mtools.conf.in
 	sed -e 's|@OBJ@|$(obj)|g' < $< > $@
 
+quiet_cmd_genimage = GENIMAGE $3
+cmd_genimage = sh $(srctree)/$(src)/genimage.sh $2 $3 $(obj)/bzImage \
+			$(obj)/mtools.conf '$(image_cmdline)' $(FDINITRD)
+
 # This requires write access to /dev/fd0
 bzdisk: $(obj)/bzImage $(obj)/mtools.conf
-	MTOOLSRC=$(obj)/mtools.conf mformat a:			; sync
-	syslinux /dev/fd0					; sync
-	echo '$(image_cmdline)' | \
-		MTOOLSRC=$(src)/mtools.conf mcopy - a:syslinux.cfg
-	if [ -f '$(FDINITRD)' ] ; then \
-		MTOOLSRC=$(obj)/mtools.conf mcopy '$(FDINITRD)' a:initrd.img ; \
-	fi
-	MTOOLSRC=$(obj)/mtools.conf mcopy $(obj)/bzImage a:linux	; sync
+	$(call cmd,genimage,bzdisk,/dev/fd0)
 
 # These require being root or having syslinux 2.02 or higher installed
 fdimage fdimage144: $(obj)/bzImage $(obj)/mtools.conf
-	dd if=/dev/zero of=$(obj)/fdimage bs=1024 count=1440
-	MTOOLSRC=$(obj)/mtools.conf mformat v:			; sync
-	syslinux $(obj)/fdimage					; sync
-	echo '$(image_cmdline)' | \
-		MTOOLSRC=$(obj)/mtools.conf mcopy - v:syslinux.cfg
-	if [ -f '$(FDINITRD)' ] ; then \
-		MTOOLSRC=$(obj)/mtools.conf mcopy '$(FDINITRD)' v:initrd.img ; \
-	fi
-	MTOOLSRC=$(obj)/mtools.conf mcopy $(obj)/bzImage v:linux	; sync
+	$(call cmd,genimage,fdimage144,$(obj)/fdimage)
+	@$(kecho) 'Kernel: $(obj)/fdimage is ready'
 
 fdimage288: $(obj)/bzImage $(obj)/mtools.conf
-	dd if=/dev/zero of=$(obj)/fdimage bs=1024 count=2880
-	MTOOLSRC=$(obj)/mtools.conf mformat w:			; sync
-	syslinux $(obj)/fdimage					; sync
-	echo '$(image_cmdline)' | \
-		MTOOLSRC=$(obj)/mtools.conf mcopy - w:syslinux.cfg
-	if [ -f '$(FDINITRD)' ] ; then \
-		MTOOLSRC=$(obj)/mtools.conf mcopy '$(FDINITRD)' w:initrd.img ; \
-	fi
-	MTOOLSRC=$(obj)/mtools.conf mcopy $(obj)/bzImage w:linux	; sync
+	$(call cmd,genimage,fdimage288,$(obj)/fdimage)
+	@$(kecho) 'Kernel: $(obj)/fdimage is ready'
 
 isoimage: $(obj)/bzImage
-	-rm -rf $(obj)/isoimage
-	mkdir $(obj)/isoimage
-	for i in lib lib64 share end ; do \
-		if [ -f /usr/$$i/syslinux/isolinux.bin ] ; then \
-			cp /usr/$$i/syslinux/isolinux.bin $(obj)/isoimage ; \
-			if [ -f /usr/$$i/syslinux/ldlinux.c32 ]; then \
-				cp /usr/$$i/syslinux/ldlinux.c32 $(obj)/isoimage ; \
-			fi ; \
-			break ; \
-		fi ; \
-		if [ $$i = end ] ; then exit 1 ; fi ; \
-	done
-	cp $(obj)/bzImage $(obj)/isoimage/linux
-	echo '$(image_cmdline)' > $(obj)/isoimage/isolinux.cfg
-	if [ -f '$(FDINITRD)' ] ; then \
-		cp '$(FDINITRD)' $(obj)/isoimage/initrd.img ; \
-	fi
-	mkisofs -J -r -o $(obj)/image.iso -b isolinux.bin -c boot.cat \
-		-no-emul-boot -boot-load-size 4 -boot-info-table \
-		$(obj)/isoimage
-	isohybrid $(obj)/image.iso 2>/dev/null || true
-	rm -rf $(obj)/isoimage
+	$(call cmd,genimage,isoimage,$(obj)/image.iso)
+	@$(kecho) 'Kernel: $(obj)/image.iso is ready'
 
 bzlilo: $(obj)/bzImage
 	if [ -f $(INSTALL_PATH)/vmlinuz ]; then mv $(INSTALL_PATH)/vmlinuz $(INSTALL_PATH)/vmlinuz.old; fi
diff --git a/arch/x86/boot/genimage.sh b/arch/x86/boot/genimage.sh
new file mode 100644
index 0000000..75a9de1
--- /dev/null
+++ b/arch/x86/boot/genimage.sh
@@ -0,0 +1,105 @@
+#!/bin/sh
+#
+# This file is subject to the terms and conditions of the GNU General Public
+# License.  See the file "COPYING" in the main directory of this archive
+# for more details.
+#
+# Copyright (C) 2017 by Changbin Du <changbin.du@intel.com>
+#
+# Adapted from code in arch/x86/boot/Makefile by H. Peter Anvin and others
+#
+# "make fdimage/fdimage144/fdimage288/isoimage" script for x86 architecture
+#
+# Arguments:
+#   $1 - fdimage format
+#   $2 - target image file
+#   $3 - kernel bzImage file
+#   $4 - mtool configuration file
+#   $5 - kernel cmdline
+#   $6 - inird image file
+#
+
+verify () {
+	if [ ! -f "$1" ]; then
+		echo ""                                                   1>&2
+		echo " *** Missing file: $1"                              1>&2
+		echo ""                                                   1>&2
+		exit 1
+	fi
+}
+
+
+export MTOOLSRC=$4
+FIMAGE=$2
+FBZIMAGE=$3
+KCMDLINE=$5
+FDINITRD=$6
+
+# Make sure the files actually exist
+verify "$FBZIMAGE"
+verify "$MTOOLSRC"
+
+genbzdisk() {
+	mformat a:
+	syslinux $FIMAGE
+	echo "$KCMDLINE" | mcopy - a:syslinux.cfg
+	if [ -f "$FDINITRD" ] ; then
+		mcopy "$FDINITRD" a:initrd.img
+	fi
+	mcopy $FBZIMAGE a:linux
+}
+
+genfdimage144() {
+	dd if=/dev/zero of=$FIMAGE bs=1024 count=1440
+	mformat v:
+	syslinux $FIMAGE
+	echo "$KCMDLINE" | mcopy - v:syslinux.cfg
+	if [ -f "$FDINITRD" ] ; then
+		mcopy "$FDINITRD" v:initrd.img
+	fi
+	mcopy $FBZIMAGE v:linux
+}
+
+genfdimage288() {
+	dd if=/dev/zero of=$FIMAGE bs=1024 count=2880
+	mformat w:
+	syslinux $FIMAGE
+	echo "$KCMDLINE" | mcopy - W:syslinux.cfg
+	if [ -f "$FDINITRD" ] ; then
+		mcopy "$FDINITRD" w:initrd.img
+	fi
+	mcopy $FBZIMAGE w:linux
+}
+
+genisoimage() {
+	tmp_dir=`dirname $FIMAGE`/isoimage
+	rm -rf $tmp_dir
+	mkdir $tmp_dir
+	for i in lib lib64 share end ; do
+		if [ -f /usr/$i/syslinux/isolinux.bin ] ; then
+			cp /usr/$i/syslinux/isolinux.bin $tmp_dir
+			if [ -f /usr/$i/syslinux/ldlinux.c32 ]; then
+				cp /usr/$i/syslinux/ldlinux.c32 $tmp_dir
+			fi
+			break
+		fi
+		if [ $i = end ] ; then exit 1 ; fi ;
+	done
+	cp $FBZIMAGE $tmp_dir/linux
+	echo "$KCMDLINE" > $tmp_dir/isolinux.cfg
+	if [ -f "$FDINITRD" ] ; then
+		cp "$FDINITRD" $tmp_dir/initrd.img
+	fi
+	mkisofs -J -r -o $FIMAGE -b isolinux.bin -c boot.cat \
+		-no-emul-boot -boot-load-size 4 -boot-info-table $tmp_dir
+	isohybrid $FIMAGE 2>/dev/null || true
+	rm -rf $tmp_dir
+}
+
+case $1 in
+	bzdisk)     genbzdisk;;
+	fdimage144) genfdimage144;;
+	fdimage288) genfdimage288;;
+	isoimage)   genisoimage;;
+	*)          echo 'Unknown image format'; exit 1;
+esac
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] x86, build: Add new paths for isolinux.bin and ldlinux.c32
  2017-11-05  9:18 [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation changbin.du
  2017-11-05  9:18 ` [PATCH 1/4] x86, build: Fact out fdimage/isoimage generation commands to standalone script changbin.du
@ 2017-11-05  9:18 ` changbin.du
  2017-11-05  9:33   ` Ingo Molnar
  2017-11-05  9:18 ` [PATCH 3/4] x86, build: Specify -input-charset=utf-8 for mkisofs changbin.du
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: changbin.du @ 2017-11-05  9:18 UTC (permalink / raw)
  To: hpa, tglx; +Cc: mingo, x86, linux-kernel, yamada.masahiro, Changbin Du

From: Changbin Du <changbin.du@intel.com>

Recently I failed to build isoimage target, because the path of isolinux.bin
changed to /usr/xxx/ISOLINUX/isolinux.bin, as well as ldlinux.c32 which
changed to /usr/xxx/syslinux/modules/bios/ldlinux.c32.

This patch has a improvement of the file search:
  - Show a error message instead of silent fail.
  - Add above new paths.

Signed-off-by: Changbin Du <changbin.du@intel.com>
---
 arch/x86/boot/genimage.sh | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/arch/x86/boot/genimage.sh b/arch/x86/boot/genimage.sh
index 75a9de1..f7994ad 100644
--- a/arch/x86/boot/genimage.sh
+++ b/arch/x86/boot/genimage.sh
@@ -76,14 +76,27 @@ genisoimage() {
 	rm -rf $tmp_dir
 	mkdir $tmp_dir
 	for i in lib lib64 share end ; do
-		if [ -f /usr/$i/syslinux/isolinux.bin ] ; then
-			cp /usr/$i/syslinux/isolinux.bin $tmp_dir
-			if [ -f /usr/$i/syslinux/ldlinux.c32 ]; then
-				cp /usr/$i/syslinux/ldlinux.c32 $tmp_dir
+		for j in syslinux ISOLINUX ; do
+			if [ -f /usr/$i/$j/isolinux.bin ] ; then
+				isolinux=/usr/$i/$j/isolinux.bin
+				echo "Using $isolinux"
+				cp $isolinux $tmp_dir
 			fi
+		done
+		for j in syslinux syslinux/modules/bios ; do
+			if [ -f /usr/$i/$j/ldlinux.c32 ]; then
+				ldlinux=/usr/$i/$j/ldlinux.c32
+				echo "Using $ldlinux"
+				cp $ldlinux $tmp_dir
+			fi
+		done
+		if [ -n "$isolinux" -a -n "$ldlinux" ] ; then
 			break
 		fi
-		if [ $i = end ] ; then exit 1 ; fi ;
+		if [ $i = end -a -z "$isolinux" ] ; then
+			echo 'Need isolinux.bin, please install syslinux/isolinux'
+			exit 1
+		fi
 	done
 	cp $FBZIMAGE $tmp_dir/linux
 	echo "$KCMDLINE" > $tmp_dir/isolinux.cfg
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] x86, build: Specify -input-charset=utf-8 for mkisofs
  2017-11-05  9:18 [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation changbin.du
  2017-11-05  9:18 ` [PATCH 1/4] x86, build: Fact out fdimage/isoimage generation commands to standalone script changbin.du
  2017-11-05  9:18 ` [PATCH 2/4] x86, build: Add new paths for isolinux.bin and ldlinux.c32 changbin.du
@ 2017-11-05  9:18 ` changbin.du
  2017-11-05  9:34   ` Ingo Molnar
  2017-11-05  9:18 ` [PATCH 4/4] x86, boot: Add some generated files to the .gitignore file changbin.du
  2017-11-05  9:37 ` [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation Ingo Molnar
  4 siblings, 1 reply; 11+ messages in thread
From: changbin.du @ 2017-11-05  9:18 UTC (permalink / raw)
  To: hpa, tglx; +Cc: mingo, x86, linux-kernel, yamada.masahiro, Changbin Du

From: Changbin Du <changbin.du@intel.com>

This can omit this:
I: -input-charset not specified, using utf-8 (detected in locale settings)

Signed-off-by: Changbin Du <changbin.du@intel.com>
---
 arch/x86/boot/genimage.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/genimage.sh b/arch/x86/boot/genimage.sh
index f7994ad..b411102 100644
--- a/arch/x86/boot/genimage.sh
+++ b/arch/x86/boot/genimage.sh
@@ -103,8 +103,9 @@ genisoimage() {
 	if [ -f "$FDINITRD" ] ; then
 		cp "$FDINITRD" $tmp_dir/initrd.img
 	fi
-	mkisofs -J -r -o $FIMAGE -b isolinux.bin -c boot.cat \
-		-no-emul-boot -boot-load-size 4 -boot-info-table $tmp_dir
+	mkisofs -J -r -input-charset=utf-8 -o $FIMAGE -b isolinux.bin \
+		-c boot.cat -no-emul-boot -boot-load-size 4 -boot-info-table \
+		$tmp_dir
 	isohybrid $FIMAGE 2>/dev/null || true
 	rm -rf $tmp_dir
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] x86, boot: Add some generated files to the .gitignore file
  2017-11-05  9:18 [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation changbin.du
                   ` (2 preceding siblings ...)
  2017-11-05  9:18 ` [PATCH 3/4] x86, build: Specify -input-charset=utf-8 for mkisofs changbin.du
@ 2017-11-05  9:18 ` changbin.du
  2017-11-05  9:37 ` [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation Ingo Molnar
  4 siblings, 0 replies; 11+ messages in thread
From: changbin.du @ 2017-11-05  9:18 UTC (permalink / raw)
  To: hpa, tglx; +Cc: mingo, x86, linux-kernel, yamada.masahiro, Changbin Du

From: Changbin Du <changbin.du@intel.com>

Ignore generated files fdimage,mtools.conf and image.iso.

Signed-off-by: Changbin Du <changbin.du@intel.com>
---
 arch/x86/boot/.gitignore | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/boot/.gitignore b/arch/x86/boot/.gitignore
index e3cf9f6..09d25dd 100644
--- a/arch/x86/boot/.gitignore
+++ b/arch/x86/boot/.gitignore
@@ -7,3 +7,6 @@ zoffset.h
 setup
 setup.bin
 setup.elf
+fdimage
+mtools.conf
+image.iso
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] x86, build: Fact out fdimage/isoimage generation commands to standalone script
  2017-11-05  9:18 ` [PATCH 1/4] x86, build: Fact out fdimage/isoimage generation commands to standalone script changbin.du
@ 2017-11-05  9:32   ` Ingo Molnar
  2017-11-06  2:58     ` Du, Changbin
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2017-11-05  9:32 UTC (permalink / raw)
  To: changbin.du; +Cc: hpa, tglx, mingo, x86, linux-kernel, yamada.masahiro


A few spelling fixes:

in the title:

s/Fact out
 /Factor out

* changbin.du@intel.com <changbin.du@intel.com> wrote:

> From: Changbin Du <changbin.du@intel.com>
> 
> The build message for fdimage/isoimage are pretty unstructured. The raw
> shell command blocks are printed. We can improve them as regular build
> system messages. Besides, writing commands in shell script is much more
> easier than in a Makefile.

s/much more easier
 /much more easy

> 
> See Ingo's suggestion here https://lkml.org/lkml/2017/10/31/124.
> 
> This patch fact out the commands used for fdimage/isoimage generation from
> arch/x86/boot/Makefile to new script arch/x86/boot/genimage.sh. Then add a
> new kbuild command 'genimage' which invokes the new script. All
> fdimages/isoimage now is generated by call to 'genimage' with different
> parameters.

s/fact out
 /factors out

s/to new script
  to a new script

s/Then add
 /Then it adds

s/a new kbuild command 'genimage'
 /the new 'genimage' kbuild command

s/All fdimages/isoimage now is generated by call to
 /All fdimage/isoimage files are now generated by a call to

> +#   $3 - kernel bzImage file
> +#   $4 - mtool configuration file
> +#   $5 - kernel cmdline
> +#   $6 - inird image file
> +#

The new script is much easier to read!

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] x86, build: Add new paths for isolinux.bin and ldlinux.c32
  2017-11-05  9:18 ` [PATCH 2/4] x86, build: Add new paths for isolinux.bin and ldlinux.c32 changbin.du
@ 2017-11-05  9:33   ` Ingo Molnar
  2017-11-06  3:00     ` Du, Changbin
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2017-11-05  9:33 UTC (permalink / raw)
  To: changbin.du; +Cc: hpa, tglx, mingo, x86, linux-kernel, yamada.masahiro


* changbin.du@intel.com <changbin.du@intel.com> wrote:

> From: Changbin Du <changbin.du@intel.com>
> 
> Recently I failed to build isoimage target, because the path of isolinux.bin
> changed to /usr/xxx/ISOLINUX/isolinux.bin, as well as ldlinux.c32 which
> changed to /usr/xxx/syslinux/modules/bios/ldlinux.c32.
> 
> This patch has a improvement of the file search:
>   - Show a error message instead of silent fail.
>   - Add above new paths.

How about:

  This patch improves the file search logic:
    - Show an error message instead of failing silently
    - Add the new paths listed above.


> +		if [ $i = end -a -z "$isolinux" ] ; then
> +			echo 'Need isolinux.bin, please install syslinux/isolinux'

How about:

			echo 'Need an isolinux.bin file, please install syslinux/isolinux'

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] x86, build: Specify -input-charset=utf-8 for mkisofs
  2017-11-05  9:18 ` [PATCH 3/4] x86, build: Specify -input-charset=utf-8 for mkisofs changbin.du
@ 2017-11-05  9:34   ` Ingo Molnar
  0 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2017-11-05  9:34 UTC (permalink / raw)
  To: changbin.du; +Cc: hpa, tglx, mingo, x86, linux-kernel, yamada.masahiro


* changbin.du@intel.com <changbin.du@intel.com> wrote:

> From: Changbin Du <changbin.du@intel.com>
> 
> This can omit this:
> I: -input-charset not specified, using utf-8 (detected in locale settings)

How about:

   Avoids the following warning triggered by newer versions of mkisofs:

     I: -input-charset not specified, using utf-8 (detected in locale settings)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation
  2017-11-05  9:18 [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation changbin.du
                   ` (3 preceding siblings ...)
  2017-11-05  9:18 ` [PATCH 4/4] x86, boot: Add some generated files to the .gitignore file changbin.du
@ 2017-11-05  9:37 ` Ingo Molnar
  4 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2017-11-05  9:37 UTC (permalink / raw)
  To: changbin.du
  Cc: hpa, tglx, mingo, x86, linux-kernel, yamada.masahiro,
	Michal Marek


* changbin.du@intel.com <changbin.du@intel.com> wrote:

> From: Changbin Du <changbin.du@intel.com>
> 
> This serias is a expansion of below original patch after discussion with Ingo
> and Yamada.
> [PATCH] x86, build: Improve the isolinux searching of isoimage generation
> 
> Changbin Du (4):
>   x86, build: Fact out fdimage/isoimage generation commands to
>     standalone script
>   x86, build: Add new paths for isolinux.bin and ldlinux.c32
>   x86, build: Specify -input-charset=utf-8 for mkisofs
>   x86, boot: Add some generated files to the .gitignore file
> 
>  arch/x86/boot/.gitignore  |   3 ++
>  arch/x86/boot/Makefile    |  59 +++++------------------
>  arch/x86/boot/genimage.sh | 119 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 133 insertions(+), 48 deletions(-)
>  create mode 100644 arch/x86/boot/genimage.sh

Very nice - it looks good to me with the edits I suggested.

Would be nice if the kbuild maintainers could take a look as well - is this how 
boot image construction should be done at the arch level?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] x86, build: Fact out fdimage/isoimage generation commands to standalone script
  2017-11-05  9:32   ` Ingo Molnar
@ 2017-11-06  2:58     ` Du, Changbin
  0 siblings, 0 replies; 11+ messages in thread
From: Du, Changbin @ 2017-11-06  2:58 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: changbin.du, hpa, tglx, mingo, x86, linux-kernel, yamada.masahiro

[-- Attachment #1: Type: text/plain, Size: 1646 bytes --]

On Sun, Nov 05, 2017 at 10:32:08AM +0100, Ingo Molnar wrote:
> 
> A few spelling fixes:
> 
> in the title:
> 
> s/Fact out
>  /Factor out
> 
> * changbin.du@intel.com <changbin.du@intel.com> wrote:
> 
> > From: Changbin Du <changbin.du@intel.com>
> > 
> > The build message for fdimage/isoimage are pretty unstructured. The raw
> > shell command blocks are printed. We can improve them as regular build
> > system messages. Besides, writing commands in shell script is much more
> > easier than in a Makefile.
> 
> s/much more easier
>  /much more easy
> 
> > 
> > See Ingo's suggestion here https://lkml.org/lkml/2017/10/31/124.
> > 
> > This patch fact out the commands used for fdimage/isoimage generation from
> > arch/x86/boot/Makefile to new script arch/x86/boot/genimage.sh. Then add a
> > new kbuild command 'genimage' which invokes the new script. All
> > fdimages/isoimage now is generated by call to 'genimage' with different
> > parameters.
> 
> s/fact out
>  /factors out
> 
> s/to new script
>   to a new script
> 
> s/Then add
>  /Then it adds
> 
> s/a new kbuild command 'genimage'
>  /the new 'genimage' kbuild command
> 
> s/All fdimages/isoimage now is generated by call to
>  /All fdimage/isoimage files are now generated by a call to
> 
Sorry for these grammar errors. I alwyas forgot to write complete sentences in
English. :)

> > +#   $3 - kernel bzImage file
> > +#   $4 - mtool configuration file
> > +#   $5 - kernel cmdline
> > +#   $6 - inird image file
> > +#
> 
> The new script is much easier to read!
> 
> Thanks,
> 
> 	Ingo

-- 
Thanks,
Changbin Du

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] x86, build: Add new paths for isolinux.bin and ldlinux.c32
  2017-11-05  9:33   ` Ingo Molnar
@ 2017-11-06  3:00     ` Du, Changbin
  0 siblings, 0 replies; 11+ messages in thread
From: Du, Changbin @ 2017-11-06  3:00 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: changbin.du, hpa, tglx, mingo, x86, linux-kernel, yamada.masahiro

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

Hi Ingo,
On Sun, Nov 05, 2017 at 10:33:53AM +0100, Ingo Molnar wrote:
> 
> * changbin.du@intel.com <changbin.du@intel.com> wrote:
> 
> > From: Changbin Du <changbin.du@intel.com>
> > 
> > Recently I failed to build isoimage target, because the path of isolinux.bin
> > changed to /usr/xxx/ISOLINUX/isolinux.bin, as well as ldlinux.c32 which
> > changed to /usr/xxx/syslinux/modules/bios/ldlinux.c32.
> > 
> > This patch has a improvement of the file search:
> >   - Show a error message instead of silent fail.
> >   - Add above new paths.
> 
> How about:
> 
>   This patch improves the file search logic:
>     - Show an error message instead of failing silently
>     - Add the new paths listed above.
> 
> 
> > +		if [ $i = end -a -z "$isolinux" ] ; then
> > +			echo 'Need isolinux.bin, please install syslinux/isolinux'
> 
> How about:
> 
> 			echo 'Need an isolinux.bin file, please install syslinux/isolinux'
>
The new description is much better, will update. Thanks.

> Thanks,
> 
> 	Ingo

-- 
Thanks,
Changbin Du

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-11-06  3:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-05  9:18 [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation changbin.du
2017-11-05  9:18 ` [PATCH 1/4] x86, build: Fact out fdimage/isoimage generation commands to standalone script changbin.du
2017-11-05  9:32   ` Ingo Molnar
2017-11-06  2:58     ` Du, Changbin
2017-11-05  9:18 ` [PATCH 2/4] x86, build: Add new paths for isolinux.bin and ldlinux.c32 changbin.du
2017-11-05  9:33   ` Ingo Molnar
2017-11-06  3:00     ` Du, Changbin
2017-11-05  9:18 ` [PATCH 3/4] x86, build: Specify -input-charset=utf-8 for mkisofs changbin.du
2017-11-05  9:34   ` Ingo Molnar
2017-11-05  9:18 ` [PATCH 4/4] x86, boot: Add some generated files to the .gitignore file changbin.du
2017-11-05  9:37 ` [PATCH 0/4] x86, kbuild: Some improvements for fdimage/isoimage generation Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox