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 2/3] nanopb: Init at 69e9c1fc8162956feffa32e07a97c53bdb92f5ef
Date: Sat, 10 Dec 2016 15:37:06 +0100	[thread overview]
Message-ID: <20161210153706.4708003f@free-electrons.com> (raw)
In-Reply-To: <20161209011313.9549-3-wak@google.com>

Hello,

Thanks for this new iteration. However, there's a big problem with your
patch: it seems to be corrupt, and therefore cannot be applied. That's
weird because you've used git send-email, which normally always get
things right, but not here:

thomas at skate:~/projets/buildroot (master)$ git pwam 704279
Applying patch #704279 using 'git am -s -3'
Description: [2/3] nanopb: Init at 69e9c1fc8162956feffa32e07a97c53bdb92f5ef
Applying: nanopb: Init at 69e9c1fc8162956feffa32e07a97c53bdb92f5ef
fatal: corrupt patch at line 73
error: could not build fake ancestor
Patch failed at 0001 nanopb: Init at 69e9c1fc8162956feffa32e07a97c53bdb92f5ef
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.

And indeed, looking at your patch below, some lines have been wrapped.

On Thu,  8 Dec 2016 17:13:12 -0800, William A. Kennington III wrote:
> We chose not to use a stable version so that our version of nanopb
> is compatible with pblog. In the future a stable version should be used.

The commit title should be just:

	nanopb: new package

> 
> Change-Id: I3f80c809efbe7c237de9b9de901d048247b4c6c1
> Google-Bug-Id: 33072942

Those two lines should be removed.

> Signed-off-by: William A. Kennington III <wak@google.com>

> +# Locally calculated
> +sha256 e6dbaf58574c54e67b6bdfdee71f62d52d261e77a5f8e3961b7a6a0abbf06a86  
> nanopb-69e9c1fc8162956feffa32e07a97c53bdb92f5ef.tar.gz

Here the line is wrapped while it should not.

> diff --git a/package/nanopb/nanopb.mk b/package/nanopb/nanopb.mk
> new file mode 100644
> index 0000000..a162b82
> --- /dev/null
> +++ b/package/nanopb/nanopb.mk
> @@ -0,0 +1,27 @@
> +################################################################################
> +#
> +# nanopb
> +#
> +################################################################################
> +
> +NANOPB_VERSION = 69e9c1fc8162956feffa32e07a97c53bdb92f5ef
> +NANOPB_SITE = $(call github,nanopb,nanopb,$(NANOPB_VERSION))
> +NANOPB_LICENSE = BSD-3c
> +NANOPB_LICENSE_FILES = LICENSE.txt
> +# no configure script

This comment is not needed.

> +# Need to use host protoc during cross compilation.

How is this comment related to the below dependencies? Your comment
mentions a host tools, but you don't depend on any host package. I know
'protobuf' depends on 'host-protobuf', which will bring you the host
protoc, but that's not very clear from your comment.

> +NANOPB_DEPENDENCIES = protobuf python-protobuf
> +
> +define HOST_NANOPB_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C  
> $(@D)/generator/proto

Line has been wrapped.

> +endef
> +
> +define NANOPB_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C  
> $(@D)/generator/proto

Line has been wrapped.

> +endef
> +
> +# We don't actually need to install anything as you include the library  
> files
> +# directly.

This looks really really weird, I'll review PATCH 3/3 to try to
understand, but this approach doesn't seem very good. Also, this
paragraph has also been badly rewrapped when your patch was sent.

> +$(eval $(host-generic-package))
> +$(eval $(generic-package))

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

  reply	other threads:[~2016-12-10 14:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-09  1:13 [Buildroot] [PATCH 0/3] pblog: Add new package William A. Kennington III
2016-12-09  1:13 ` [Buildroot] [PATCH 1/3] python-protobuf: Allow host building William A. Kennington III
2016-12-10 14:38   ` Thomas Petazzoni
2016-12-09  1:13 ` [Buildroot] [PATCH 2/3] nanopb: Init at 69e9c1fc8162956feffa32e07a97c53bdb92f5ef William A. Kennington III
2016-12-10 14:37   ` Thomas Petazzoni [this message]
2016-12-09  1:13 ` [Buildroot] [PATCH 3/3] pblog: Init at e2e17544950eedac4f3bc59926aed8604d755072 William A. Kennington III
2016-12-10 14:41   ` Thomas Petazzoni
2017-01-12 19:15 ` [Buildroot] [PATCH v2 0/3] pblog: new package William A. Kennington III
2017-01-12 19:15   ` [Buildroot] [PATCH v2 1/3] python-protobuf: Allow host building William A. Kennington III
2017-01-15 21:30     ` Yann E. MORIN
2017-01-17 22:09       ` Arnout Vandecappelle
2017-01-15 21:32     ` Yann E. MORIN
2017-01-12 19:15   ` [Buildroot] [PATCH v2 2/3] nanopb: new package William A. Kennington III
2017-01-15 21:51     ` Yann E. MORIN
2017-01-18 18:59       ` William Kennington
2017-01-12 19:15   ` [Buildroot] [PATCH v2 3/3] pblog: " William A. Kennington III

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=20161210153706.4708003f@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