All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Du, Changbin" <changbin.du@intel.com>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Ingo Molnar <mingo@kernel.org>,
	changbin.du@intel.com, Michal Marek <mmarek@suse.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, X86 ML <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x86, build: Improve the isolinux searching of isoimage generation
Date: Wed, 1 Nov 2017 22:28:49 +0800	[thread overview]
Message-ID: <20171101142849.GA23254@intel.com> (raw)
In-Reply-To: <CAK7LNARFf8RDCsqdWZ9_YDddmTVaKo2i146wbBac-cSCPK5uaQ@mail.gmail.com>

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

Hi Ingo and Yamada,
Thanks for your suggestions. I'll have a try though I am not familiar with kbuild system.

On Wed, Nov 01, 2017 at 12:17:50PM +0900, Masahiro Yamada wrote:
> 2017-10-31 18:39 GMT+09:00 Ingo Molnar <mingo@kernel.org>:
> >
> > * 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:
> >>   - Don't print the raw shell commands. It doesn't make sense to show the
> >>     entire big block.
> >>   - Show a error message instead of silent fail.
> >>   - Add above new paths.
> >>
> >> Now it becomes:
> >> Kernel: arch/x86/boot/bzImage is ready  (#62)
> >> rm -rf arch/x86/boot/isoimage
> >> mkdir arch/x86/boot/isoimage
> >> Using /usr/lib/ISOLINUX/isolinux.bin
> >> Using /usr/lib/syslinux/modules/bios/ldlinux.c32
> >> cp arch/x86/boot/bzImage arch/x86/boot/isoimage/linux
> >> ...
> >>
> >> 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
> >> arch/x86/boot/Makefile:161: recipe for target 'isoimage' failed
> >> make[1]: *** [isoimage] Error 1
> >
> > I like these changes. Could we please further improve it: for example the boot
> > image build messages are still pretty unstructured, while regular build system
> > messages come in the following format:
> >
> >   CC      arch/x86/events/msr.o
> >   RELOCS  arch/x86/realmode/rm/realmode.relocs
> >   OBJCOPY arch/x86/realmode/rm/realmode.bin
> >   CC      arch/x86/kernel/signal.o
> >   AS      arch/x86/realmode/rmpiggy.o
> >   CC      ipc/msg.o
> >   AR      arch/x86/ia32/built-in.o
> >   CC      arch/x86/events/amd/iommu.o
> >   CC      init/do_mounts.o
> >   AR      arch/x86/realmode/built-in.o
> >
> > So instead of:
> >
> >> Kernel: arch/x86/boot/bzImage is ready  (#62)
> >> rm -rf arch/x86/boot/isoimage
> >> mkdir arch/x86/boot/isoimage
> >> Using /usr/lib/ISOLINUX/isolinux.bin
> >> Using /usr/lib/syslinux/modules/bios/ldlinux.c32
> >> cp arch/x86/boot/bzImage arch/x86/boot/isoimage/linux
> >
> > Could we make it something more streamlined and similar to the rest of the build
> > as well, like:
> >
> >   GEN     arch/x86/boot/bzImage
> >   GEN     arch/x86/boot/isoimage
> >   GEN     arch/x86/boot/isoimage/linux
> >
> > I.e. only mention the new files built, with an appropriate prefix.
> >
> > I've Cc:-ed the kbuild maintainers, maybe they have a better suggestion instead of
> > the 'GEN' abbreviation?
> >
> 
> Generally, the abbreviation is the tool that has processed the target,
> but if you do not find an appropriate one, 'GEN' is fine.
> 
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

-- 
Thanks,
Changbin Du

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

      reply	other threads:[~2017-11-01 14:36 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-31  4:45 [PATCH] x86, build: Improve the isolinux searching of isoimage generation changbin.du
2017-10-31  9:39 ` Ingo Molnar
2017-11-01  3:17   ` Masahiro Yamada
2017-11-01 14:28     ` Du, Changbin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171101142849.GA23254@intel.com \
    --to=changbin.du@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=mmarek@suse.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.