xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	KeirFraser <keir@xen.org>, Ian Campbell <Ian.Campbell@citrix.com>,
	Jan Beulich <JBeulich@suse.com>, Tim Deegan <tim@xen.org>
Subject: Re: [PATCH] xen/build: Remove hacked up version of figlet
Date: Thu, 26 Sep 2013 16:46:06 +0100	[thread overview]
Message-ID: <524456BE.7050307@citrix.com> (raw)
In-Reply-To: <21042.58891.311204.760237@mariner.uk.xensource.com>

On 13/09/13 11:16, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH] xen/build: Remove hacked up version of figlet"):
>> On Fri, 2013-09-13 at 10:40 +0100, Jan Beulich wrote:
> ...
>>>  I'd be more agreeable to this if the whole
>>> change wasn't just to make a tool happy...
>> This is true...
> No, it isn't.  That's just what prompted us to look at this.
>
> When Ian C told me offline about this embedded copy of figlet I said
> WTF?!
>
> We certainly shouldn't be carrying a hacked up version of figlet in
> the Xen tree.  If there are systems without figlet then we should make
> its use conditional, as suggested.  Those systems' users can then
> (a) install figlet by hand (b) live without figleted banners (c) get
> this apparently useful utility into their distro.
>
> Ian.

Coverity aside, we really shouldn't have a hacked up version of figlet here.

How about this (just the makefile changes to safe sending a huge patch
again)? It causes a lack of figlet in the build environment not to be fatal.

~Andrew

-----8<-----

diff --git a/xen/Makefile b/xen/Makefile
index 597972d..1ea2717 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -86,7 +86,7 @@ _clean: delete-unfresh-files
        $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
        rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET)-syms *~ core
        rm -f include/asm-*/asm-offsets.h
-       [ -d tools/figlet ] && rm -f .banner*
+       rm -f .banner
 
 .PHONY: _distclean
 _distclean: clean
@@ -114,10 +114,12 @@ delete-unfresh-files:
        fi
 
 .banner: Makefile
-       $(MAKE) -C tools
-       @tools/figlet/figlet -d tools/figlet Xen $(XEN_FULLVERSION)
2>$@2 >$@1
-       @cat $@1 $@2 >$@
-       @rm -f $@1 $@2
+       @if which figlet >/dev/null 2>&1 ; then \
+               echo " Xen $(XEN_FULLVERSION)" | figlet -f tools/xen.flf
> $@.tmp; \
+       else \
+               echo " Xen $(XEN_FULLVERSION)" > $@.tmp; \
+       fi
+       @mv -f $@.tmp $@
 
 # compile.h contains dynamic build info. Rebuilt on every 'make'
invocation.
 include/xen/compile.h: include/xen/compile.h.in .banner
@@ -132,8 +134,8 @@ include/xen/compile.h: include/xen/compile.h.in .banner
            -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
            -e 's!@@changeset@@!$(shell tools/scmversion $(XEN_ROOT) ||
echo "unavailable")!g' \
            < include/xen/compile.h.in > $@.new
-       @grep \" .banner >> $@.new
-       @grep -v \" .banner
+       @cat .banner
+       @$(PYTHON) tools/fig-to-oct.py < .banner >> $@.new
        @mv -f $@.new $@
 
 include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s

  reply	other threads:[~2013-09-26 15:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-12 15:51 [PATCH] xen/build: Remove hacked up version of figlet Andrew Cooper
2013-09-12 16:27 ` Ian Campbell
2013-09-12 16:31   ` Andrew Cooper
2013-09-12 16:30 ` Keir Fraser
2013-09-13  9:14 ` Jan Beulich
2013-09-13  9:30   ` Ian Campbell
2013-09-13  9:40     ` Jan Beulich
2013-09-13  9:46       ` Ian Campbell
2013-09-13 10:16         ` Ian Jackson
2013-09-26 15:46           ` Andrew Cooper [this message]
2013-10-11 17:59             ` Ian Jackson

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=524456BE.7050307@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=keir@xen.org \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).