Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCHv3 17/18] packages: refactor checks using BR_BUILDING
Date: Sun, 26 Apr 2015 16:18:15 +0200	[thread overview]
Message-ID: <20150426141815.GC4809@free.fr> (raw)
In-Reply-To: <20150426114247.0686ffc2@free-electrons.com>

Thomas, All,

On 2015-04-26 11:42 +0200, Thomas Petazzoni spake thusly:
> On Sat, 25 Apr 2015 22:52:57 +0200, Yann E. MORIN wrote:
> 
> > >  # Checks to give errors that the user can understand
> > > -ifeq ($(filter source,$(MAKECMDGOALS)),)
> > > +ifeq ($(BR_BUILDING),y)
> > >  ifeq ($(BR2_TARGET_AT91BOOTSTRAP3_USE_DEFCONFIG),y)
> > 
> > I think it would be good to have a single way we do those checks.
> > 
> > For example, compare at91bootstrap above and at91bootstrap3 here.
> > The former is doing the BR2_BUILDING check and package-enabled check
> > in a single ifeq, while the latter does it with two ifeq.
> > 
> > I don't really care which we use, but we should use the same everywhere.
> > Maybe we could favour doing it with feq, for those packages that want to
> > do multiple checks.
> > 
> > Applicable to the other packages, of course.
> 
> I don't quite agree. For at91bootstrap, there is only one check being
> done, while for at91bootstrap3, two checks are being done.
> 
> So basically, the current code is:
> 
> ifeq ($(BR_BUILDING),y)
> ifeq ($(BR2_TARGET_AT91BOOTSTRAP3_USE_DEFCONFIG),y)
> ifeq ($(call qstrip,$(BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG)),)
> $(error No at91bootstrap3 defconfig name specified, check your BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG setting)
> endif
> endif
> 
> ifeq ($(BR2_TARGET_AT91BOOTSTRAP3_USE_CUSTOM_CONFIG),y)
> ifeq ($(call qstrip,$(BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_CONFIG_FILE)),)
> $(error No at91bootstrap3 configuration file specified, check your BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_CONFIG_FILE setting)
> endif
> endif
> endif
> 
> While you're proposing to unfactorize the BR_BUILDING test:

I thionk I actually meant the other way around: always use two ifeq,
that is always factorise the check on BR_BUILDING. But there was some
typing errors in my comment, which I clarified in a further reply:
    http://lists.busybox.net/pipermail/buildroot/2015-April/126861.html

What I really meant is that I think it would be good to have a single
and uniform way of doing the checks, so we have sane examples on how to
do it. For example, you even had an issue with xloader that completely
broke the 'source' rule. ;-)

But I won;t be pedantic on this. ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

  reply	other threads:[~2015-04-26 14:18 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-25 14:42 [Buildroot] [PATCHv3 00/18] Package based 'source', 'legal-info', 'source-check' and 'external-deps' Thomas Petazzoni
2015-04-25 14:42 ` [Buildroot] [PATCHv3 01/18] fs: only add non rootfs- targets to PACKAGES Thomas Petazzoni
2015-04-25 16:36   ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 02/18] pkg-generic: take into account patch dependencies in source, external-deps and legal-info Thomas Petazzoni
2015-04-25 16:41   ` Yann E. MORIN
2015-04-26  9:38     ` Thomas Petazzoni
2015-04-25 14:42 ` [Buildroot] [PATCHv3 03/18] Makefile: use the package infra based external-deps Thomas Petazzoni
2015-04-25 14:42 ` [Buildroot] [PATCHv3 04/18] pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE Thomas Petazzoni
2015-04-25 14:42 ` [Buildroot] [PATCHv3 05/18] Makefile: move source-check outside of noconfig_targets Thomas Petazzoni
2015-04-25 14:42 ` [Buildroot] [PATCHv3 06/18] pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro Thomas Petazzoni
2015-04-25 17:15   ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 07/18] pkg-generic: implement source-check targets Thomas Petazzoni
2015-04-25 17:28   ` Yann E. MORIN
2015-04-26  9:39     ` Thomas Petazzoni
2015-04-26 14:12       ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 08/18] Makefile: implement a package based source-check target Thomas Petazzoni
2015-04-25 14:42 ` [Buildroot] [PATCHv3 09/18] pkg-generic: remove the .stamp_rsync_sourced fake stamp file Thomas Petazzoni
2015-04-25 17:33   ` Yann E. MORIN
2015-04-25 17:49     ` Thomas Petazzoni
2015-04-25 17:52       ` Yann E. MORIN
2015-04-28 20:13       ` Arnout Vandecappelle
2015-04-25 17:53   ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 10/18] pkg-generic: don't use DL_MODE in .stamp_downloaded Thomas Petazzoni
2015-04-25 17:34   ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 11/18] pkg-download: get rid of DL_MODE Thomas Petazzoni
2015-04-25 17:35   ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 12/18] pkg-download: fix indentation for SOURCE_CHECK_* macros Thomas Petazzoni
2015-04-25 17:37   ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 13/18] pkg-generic: introduce a <pkg>_ALL_DOWNLOADS variable and factorize code Thomas Petazzoni
2015-04-25 17:58   ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 14/18] Makefile: implement the 'source' target using the package infrastructure Thomas Petazzoni
2015-04-25 18:51   ` Yann E. MORIN
2015-04-25 14:42 ` [Buildroot] [PATCHv3 15/18] Makefile: remove unneeded variables Thomas Petazzoni
2015-04-25 18:58   ` Yann E. MORIN
2015-04-25 14:43 ` [Buildroot] [PATCHv3 16/18] Makefile: add BR_BUILDING variable Thomas Petazzoni
2015-04-25 20:26   ` Yann E. MORIN
2015-04-25 21:14     ` Yann E. MORIN
2015-04-26  9:40       ` Thomas Petazzoni
2015-04-26 14:13         ` Yann E. MORIN
2015-04-25 14:43 ` [Buildroot] [PATCHv3 17/18] packages: refactor checks using BR_BUILDING Thomas Petazzoni
2015-04-25 20:52   ` Yann E. MORIN
2015-04-25 21:59     ` Yann E. MORIN
2015-04-26  9:42     ` Thomas Petazzoni
2015-04-26 14:18       ` Yann E. MORIN [this message]
2015-04-25 14:43 ` [Buildroot] [PATCHv3 18/18] Makefile: add a few more targets to nobuild_targets Thomas Petazzoni
2015-04-25 14:45 ` [Buildroot] [PATCHv3 00/18] Package based 'source', 'legal-info', 'source-check' and 'external-deps' Thomas Petazzoni

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=20150426141815.GC4809@free.fr \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@busybox.net \
    /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