All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Bernd Edlinger <bernd.edlinger@hotmail.de>,
	Borislav Petkov <bp@suse.de>, Sam Ravnborg <sam@ravnborg.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] objtool: move libelf check out of top Makefile
Date: Tue, 16 Oct 2018 11:15:36 -0500	[thread overview]
Message-ID: <20181016161536.pnek3dnyem6mspwr@treble> (raw)
In-Reply-To: <CAK7LNARLW4NmeY0Rq19Bj_5kOFbp6YDtsUDhNaAfVXjL85cAMg@mail.gmail.com>

On Wed, Oct 17, 2018 at 12:51:40AM +0900, Masahiro Yamada wrote:
> > ifdef CONFIG_UNWINDER_ORC
> >
> > chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y.  " \
> >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
> >
> > else
> >
> > chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y.  " \
> >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> >
> > endif
> >
> >
> > What do you think?
> 
> 
> It is ugly.
> 
> Do you need such detailed information like ORC metadata stuff here?
> 
> This Makefile aims to error out, showing why the build failed.
> That's it.

Yeah, it is kind of ugly.  But the "showing why the build failed" part
is important.  I was trying to give the user a clear error message,
similar to what we have today.

Without context, the user won't know what objtool is, or why it needs to
be built.

If we have just a single error message for all cases, it should at least
mention the config option.  Like

   "Cannot build objtool for CONFIG_STACK_VALIDATION."

But then, most users will only have that enabled because of ORC.  So an
ORC-specific message would be more appropriate in most cases.

So maybe it can just be something more vague:

msg_stack_validation = "Cannot build objtool for CONFIG_UNWINDER_ORC and/or CONFIG_STACK_VALIDATION.  " \
		       "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."

That would probably be good enough.  Then we could drop the ugly ifdef.

-- 
Josh

  reply	other threads:[~2018-10-16 16:15 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-16  9:10 [PATCH 0/3] kbuild: add scripts/Makefile.toolcheck to check necessary host tools Masahiro Yamada
2018-10-16  9:10 ` [PATCH 1/3] kbuild: provide a new place " Masahiro Yamada
2018-10-16 15:47   ` Sam Ravnborg
2018-10-16  9:10 ` [PATCH 2/3] objtool: move libelf check out of top Makefile Masahiro Yamada
2018-10-16 14:25   ` Josh Poimboeuf
2018-10-16 15:51     ` Masahiro Yamada
2018-10-16 16:15       ` Josh Poimboeuf [this message]
2018-10-19  6:04         ` Masahiro Yamada
2018-10-19 19:28           ` Josh Poimboeuf
2018-10-16  9:10 ` [PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary Masahiro Yamada
2018-10-16 19:54   ` Borislav Petkov
2018-10-17  8:59     ` Masahiro Yamada
2018-10-17  9:10       ` Borislav Petkov
2018-10-17  9:25         ` Masahiro Yamada
2018-10-17  9:47           ` Borislav Petkov

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=20181016161536.pnek3dnyem6mspwr@treble \
    --to=jpoimboe@redhat.com \
    --cc=bernd.edlinger@hotmail.de \
    --cc=bp@suse.de \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=mingo@redhat.com \
    --cc=sam@ravnborg.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.