Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 3/6] qemu: code cleanup - (mostly) wrapping lines at 80 characters
Date: Mon, 4 Jul 2016 00:28:43 +0200	[thread overview]
Message-ID: <20160704002843.6a616b0a@free-electrons.com> (raw)
In-Reply-To: <1462348079-7631-3-git-send-email-simonn.maes@gmail.com>

Hello,

On Wed,  4 May 2016 09:47:56 +0200, Simon Maes wrote:

>  config BR2_PACKAGE_QEMU_HAS_EMULS
>  	def_bool y
> -	depends on BR2_PACKAGE_QEMU_SYSTEM || BR2_PACKAGE_QEMU_LINUX_USER || BR2_PACKAGE_QEMU_CUSTOM_TARGETS != ""
> +	depends on BR2_PACKAGE_QEMU_SYSTEM ||     \
> +			BR2_PACKAGE_QEMU_LINUX_USER ||    \
> +			BR2_PACKAGE_QEMU_CUSTOM_TARGETS != ""

We indent continuation lines with just one more tab.

>  if BR2_PACKAGE_QEMU_HAS_EMULS
>  
> @@ -106,7 +108,7 @@ config BR2_PACKAGE_QEMU_FDT
>  	depends on !BR2_STATIC_LIBS # dtc
>  	select BR2_PACKAGE_DTC
>  	help
> -	  Say 'y' here to have QEMU capable of constructing Device Trees,
> +	  Say 'y' to have QEMU capable of constructing Device Trees,

Well, yes, but not super important.

>  	  and passing them to the VMs.
>  
>  comment "FDT support needs a toolchain w/ dynamic library"
> diff --git a/package/qemu/qemu.mk b/package/qemu/qemu.mk
> index 151060b..1910dbd 100644
> --- a/package/qemu/qemu.mk
> +++ b/package/qemu/qemu.mk
> @@ -17,7 +17,8 @@ HOST_QEMU_LICENSE_FILES = COPYING COPYING.LIB
>  #       the non-(L)GPL license texts are specified in the affected
>  #       individual source files.
>  
> -HOST_QEMU_DEPENDENCIES = host-pkgconf host-python host-zlib host-libglib2 host-pixman
> +HOST_QEMU_DEPENDENCIES = host-pkgconf host-python host-zlib host-libglib2 \
> +						 host-pixman

See above.

>  HOST_QEMU_SITE = $(QEMU_SITE)
>  HOST_QEMU_SOURCE = $(QEMU_SOURCE)
>  
> @@ -94,9 +95,15 @@ HOST_QEMU_TARGETS += $(HOST_QEMU_ARCH)-linux-user
>  HOST_QEMU_OPTS += --enable-linux-user
>  
>  # kernel version as major*256 + minor
> -HOST_QEMU_HOST_SYSTEM_VERSION = $(shell uname -r | awk -F. '{ print $$1 * 256 + $$2 }')
> -HOST_QEMU_TARGET_SYSTEM_VERSION = $(shell echo $(BR2_TOOLCHAIN_HEADERS_AT_LEAST) | awk -F. '{ print $$1 * 256 + $$2 }')
> -HOST_QEMU_COMPARE_VERSION = $(shell test $(HOST_QEMU_HOST_SYSTEM_VERSION) -ge $(HOST_QEMU_TARGET_SYSTEM_VERSION) && echo OK)
> +HOST_QEMU_HOST_SYSTEM_VERSION = $(shell uname -r | awk -F. \
> +								'{ print $$1 * 256 + $$2 }')
> +
> +HOST_QEMU_TARGET_SYSTEM_VERSION = $(shell \
> +								echo $(BR2_TOOLCHAIN_HEADERS_AT_LEAST) \
> +								| awk -F. '{ print $$1 * 256 + $$2 }')
> +
> +HOST_QEMU_COMPARE_VERSION = $(shell test $(HOST_QEMU_HOST_SYSTEM_VERSION) -ge \
> +							$(HOST_QEMU_TARGET_SYSTEM_VERSION) && echo OK)

This is actually a lot less readable than it was.

>  ifeq ($(BR_BUILDING),y)
>  ifneq ($(HOST_QEMU_COMPARE_VERSION),OK)
> @@ -120,14 +127,14 @@ endif
>  
>  define HOST_QEMU_CONFIGURE_CMDS
>  	cd $(@D); $(HOST_CONFIGURE_OPTS) ./configure    \
> -		--target-list="$(HOST_QEMU_TARGETS)"    \
> -		--prefix="$(HOST_DIR)/usr"              \
> -		--interp-prefix=$(STAGING_DIR)          \
> -		--cc="$(HOSTCC)"                        \
> -		--host-cc="$(HOSTCC)"                   \
> -		--python=$(HOST_DIR)/usr/bin/python2    \
> -		--extra-cflags="$(HOST_CFLAGS)"         \
> -		--extra-ldflags="$(HOST_LDFLAGS)"       \
> +		--target-list="$(HOST_QEMU_TARGETS)"        \
> +		--prefix="$(HOST_DIR)/usr"                  \
> +		--interp-prefix=$(STAGING_DIR)              \
> +		--cc="$(HOSTCC)"                            \
> +		--host-cc="$(HOSTCC)"                       \
> +		--python=$(HOST_DIR)/usr/bin/python2        \
> +		--extra-cflags="$(HOST_CFLAGS)"             \
> +		--extra-ldflags="$(HOST_LDFLAGS)"           \

Our convention now is to put the \ just one space after the line.

>  define QEMU_INSTALL_TARGET_CMDS
> -	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(QEMU_MAKE_ENV) DESTDIR=$(TARGET_DIR) install
> +	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(QEMU_MAKE_ENV) \
> +			DESTDIR=$(TARGET_DIR) install

Only one more tab indentation for the continuation lines.

So, I've marked your patch as Changes Requested in patchwork. Feel free
to submit an updated version that takes into account the comments if
you're interested.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  reply	other threads:[~2016-07-03 22:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-04  7:47 [Buildroot] [PATCH 1/6] qemu: add support for host-qemu-system Simon Maes
2016-05-04  7:47 ` [Buildroot] [PATCH 2/6] qemu: make qemu and host-qemu packages' version (separately) configurable Simon Maes
2016-05-07 19:01   ` Arnout Vandecappelle
2016-07-03 22:26   ` Thomas Petazzoni
2016-05-04  7:47 ` [Buildroot] [PATCH 3/6] qemu: code cleanup - (mostly) wrapping lines at 80 characters Simon Maes
2016-07-03 22:28   ` Thomas Petazzoni [this message]
2016-05-04  7:47 ` [Buildroot] [PATCH 4/6] qemu: add qemu-system-run make target Simon Maes
2016-07-03 22:29   ` Thomas Petazzoni
2016-05-04  7:47 ` [Buildroot] [PATCH 5/6] vde2: enable building host package Simon Maes
2016-07-03 22:30   ` Thomas Petazzoni
2016-05-04  7:47 ` [Buildroot] [PATCH 6/6] qemu: add support for vde2 Simon Maes
2016-07-03 22:31   ` Thomas Petazzoni
2016-05-04 22:34 ` [Buildroot] [PATCH 1/6] qemu: add support for host-qemu-system Arnout Vandecappelle
2016-07-03 22:25 ` 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=20160704002843.6a616b0a@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --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