Re: [PATCH 1/2] Add mkelfimage and support for generating kernel+initrd ELF payloads to OpenEmbedded

[Raymond Danks <ray.danks@se-eng.com>]

Thanks Paul.  I am putting together new patches to address your feedback 
on style guide, mailer line-feed, and patch format/commit policy.

As far as the build configuration this was tested on is concerned, this 
is Ubuntu 10.04 x86_64-linux targeting i686-oe-linux using a one-off of 
the i686-generic.conf machine targeting a 4MB flash part.

Thanks again,
Ray

On 07/15/2011 12:48 PM, Paul Menzel wrote:
> Dear Raymond,
>
>
> I am putting you in CC not knowing if you read the lists.
>
>
> Am Freitag, den 15.07.2011, 10:55 -0600 schrieb Raymond Danks:
>
> The correct commit summary is
>
> 	mkelfimage: Add version c045b4cc from Git (initial recipe)
>
>> On x86, and ELF image file may be stored as a coreboot payload.  This
> s/and/an/
>
>> image file may be a kernel or a kernel+initrd.
>>
>> Documentation on this has been started on the coreboot wiki:
>> http://www.coreboot.org/Mkelfimage
>>
>> We have seen success using the mkelfimage utility to construct a
>> kernel+initrd image using the micro-image OpenEmbedded image as an initrd.
>> http://www.se-eng.com/wiki/index.php/Open_Embedded_Setup
> What is your build configuration for testing this recipe?
>
>> The first patch contains the meta files for the mkelfimage utility in
>> OpenEmbedded.
> Please remove that line above. The commit message should be describing
> the commit only.
>
> Unfortunately we cannot commit your patch, especially because your
> Signed-off-by line is missing – you can add `-s` to the Git commands
> `git commit` or `git format-patch` and your mailer mangled the patch by
> automatic line breaks.
>
> Please follow the commit policy [1] and it would be great if you could
> send us a corrected patch [2].
>
>> ---
>>    recipes/mkelfimage/files/no-stack-protector.patch |   11 +++++++++++
>>    recipes/mkelfimage/mkelfimage-native_git.bb       |   11 +++++++++++
>>    recipes/mkelfimage/mkelfimage.inc                 |    3 +++
>>    recipes/mkelfimage/mkelfimage_git.bb              |   17 +++++++++++++++++
>>    4 files changed, 42 insertions(+), 0 deletions(-)
>>    create mode 100644 recipes/mkelfimage/files/no-stack-protector.patch
>>    create mode 100644 recipes/mkelfimage/mkelfimage-native_git.bb
>>    create mode 100644 recipes/mkelfimage/mkelfimage.inc
>>    create mode 100644 recipes/mkelfimage/mkelfimage_git.bb
>>
>> diff --git a/recipes/mkelfimage/files/no-stack-protector.patch
>> b/recipes/mkelfimage/files/no-stack-protector.patch
>> new file mode 100644
>> index 0000000..135de8d
>> --- /dev/null
>> +++ b/recipes/mkelfimage/files/no-stack-protector.patch
>> @@ -0,0 +1,11 @@
>> +--- mkelfImage/Makefile.orig    2011-06-17 15:20:46.816870941 -0600
>> ++++ mkelfImage/Makefile    2011-06-17 15:20:59.619382362 -0600
> Please add a header describing this patch [3].
>
>> +@@ -14,7 +14,7 @@
>> + pkglibdir = $(libdir)/$(PACKAGE)
>> + pkgincludedir = $(includedir)/$(PACKAGE)
>> +
>> +-CPPFLAGS=-DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(RELEASE_DATE)"'
>> -I include
> Here you can see an automatic line break.
>
>> ++CPPFLAGS=-fno-stack-protector -DVERSION='"$(VERSION)"'
>> -DRELEASE_DATE='"$(RELEASE_DATE)"' -I include $(CFLAGS) $(LDFLAGS)
>> + HOST_CPPFLAGS=$(CPPFLAGS)
>> + I386_CPPFLAGS=$(CPPFLAGS) -I arch/i386/include
>> + IA64_CPPFLAGS=$(CPPFLAGS) -I arch/ia64/include
> Please send that patch separately to the coreboot list if it is useful
> for upstream. You should be able to use Gerrit and you also need a
> Signed-off-by line [4].
>
>> diff --git a/recipes/mkelfimage/mkelfimage-native_git.bb
>> b/recipes/mkelfimage/mkelfimage-native_git.bb
>> new file mode 100644
>> index 0000000..e2f251e
>> --- /dev/null
>> +++ b/recipes/mkelfimage/mkelfimage-native_git.bb
>> @@ -0,0 +1,11 @@
>> +require mkelfimage_${PV}.bb
>> +inherit native
>> +
>> +do_stage () {
> Please follow the style guide [5], i. e., no space before `()`.
>
>> +    install -m 0755 objdir/sbin/mkelfImage ${STAGING_BINDIR}/
>> +}
> `do_stage ()` is deprecated [6].
>
>> +
>> +do_install () {
>> +    :
>> +}
>> +
>> diff --git a/recipes/mkelfimage/mkelfimage.inc
>> b/recipes/mkelfimage/mkelfimage.inc
>> new file mode 100644
>> index 0000000..e06ed67
>> --- /dev/null
>> +++ b/recipes/mkelfimage/mkelfimage.inc
>> @@ -0,0 +1,3 @@
>> +LICENSE = "GPLv2"
>> +DESCRIPTION = "A utility to create ELF boot images from Linux kernel
>> images"
>> +SECTION = "console/utils"
> Please order those as in the style guide and please also add the
> `HOMEPAGE` variable.
>
>> diff --git a/recipes/mkelfimage/mkelfimage_git.bb
>> b/recipes/mkelfimage/mkelfimage_git.bb
>> new file mode 100644
>> index 0000000..7855f58
>> --- /dev/null
>> +++ b/recipes/mkelfimage/mkelfimage_git.bb
>> @@ -0,0 +1,17 @@
>> +require mkelfimage.inc
>> +
>> +SRCREV = "c045b4cc458a62dbb1dd99ae6a9343e52d1fe1e0"
>> +PV = "1.0.0+gitr${SRCPV}"
>> +PR = "r1"
> We start with `r0`.
>
>> +
>> +S = "${WORKDIR}/git/util/mkelfImage"
>> +
>> +SRC_URI =
>> "git://review.coreboot.org/p/coreboot;protocol=http;branch=master \
>> +       file://no-stack-protector.patch"
>> +
>> +DEFAULT_PREFERENCE = "1"
> Not needed since this is the only recipe.
>
>> +
>> +inherit autotools
>> +
>> +SRC_URI[md5sum] = "719b22bdd34431203f9349250c67c5e3"
>> +SRC_URI[sha256sum] =
>> "9186e6e7302f38c1f77b763f0f14843fabc873ac7c81ede8ee094a47055668b5"
> Is that needed? There is no archive as far as I know.
>
>
> Thanks,
>
> Paul
>
>
> [1] http://openembedded.org/index.php/Commit_Policy
> [2] http://openembedded.org/index.php/How_to_submit_a_patch_to_OpenEmbedded
> [3] http://openembedded.org/index.php/Commit_Patch_Message_Guidelines
> [4] http://www.coreboot.org/Development_Guidelines
> [5] http://openembedded.org/index.php/Styleguide
> [6] http://openembedded.org/index.php/Legacy_staging