[oe] [PATCH] Added ImageMagick 6.7.2-3

Fri Oct 7 20:55:46 UTC 2011

Am Freitag, den 07.10.2011, 18:58 +0200 schrieb Paul Menzel:
> Am Freitag, den 07.10.2011, 09:38 -0700 schrieb Gavin Swanson:
> > This is my first attempt at submitting a patch. Please bear with me.
> 
> I will. Thank you for following up on my review comments.
> 
> > I read through the style guide and used the oe-stylize script on my
> > changed version. I still have some questions.
> > 
> > On Fri, Oct 7, 2011 at 3:01 AM, Paul Menzel wrote:
> 
> > > Am Donnerstag, den 06.10.2011, 16:01 -0700 schrieb gavinswanson at gmail.com:
> > >> From: Gavin Swanson <gavinswanson at gmail.com>
> > >
> > > Please refer to our commit policy [1] and style guide [2].
> > > (Unfortunately the Wiki is down at the moment.)
> > >
> > > Change the commit summary to
> > >
> > >        imagemagick: Add version 6.7.2-3
> > >
> > > and add some more information to the commit message. What distribution
> > > and machine did you use for testing? Have you run tested the new
> > > version.
> > >
> > > You seem to have used an existing recipe as a template. Please note that
> > > too.
> > >
> > > At last please add you Signed-off-by line.
> > >
> > >> ---
> > >>  recipes/imagemagick/imagemagick_6.7.2-3.bb |   36 ++++++++++++++++++++++++++++
> > >>  1 files changed, 36 insertions(+), 0 deletions(-)
> > >>  create mode 100644 recipes/imagemagick/imagemagick_6.7.2-3.bb
> > >>
> > >> diff --git a/recipes/imagemagick/imagemagick_6.7.2-3.bb b/recipes/imagemagick/imagemagick_6.7.2-3.bb
> > >> new file mode 100644
> > >> index 0000000..14746c9
> > >> --- /dev/null
> > >> +++ b/recipes/imagemagick/imagemagick_6.7.2-3.bb
> > >> @@ -0,0 +1,36 @@
> > >> +DESCRIPTION = "ImageMagick is an image convertion tools"
> > >> +SECTION = "console/utils"
> > >> +LICENSE = "GPL"
> > >
> > > I updated those meta fields in the repository. Please update the above
> > > too.
> > 
> > I'm using the gumstix repo,
> 
> Please include such information in your commit message.
> 
> > so I didn't have these changes, I looked them up though and have
> > included them.
> 
> Great. I am sorry for the hassle.
> 
> […]
> 
> > >> +
> > >> +SRC_URI = "ftp://ftp.nluug.nl/pub/ImageMagick/ImageMagick-${PV}.tar.bz2 \
> > >> +#           file://PerlMagic_MakePatch;apply=yes \
> > >
> > > Why is this patch not needed anymore? You can just delete the line then.
> > >
> > 
> > Not sure why it was needed in the first place, but it builds and runs
> > fine without it.
> > 
> > >> +          "
> > >> +
> > >> +IMVER = "6.7.2-3"
> > >
> > > I think the old recipes strip the `-3` to get the correct directory
> > > names. This way `IMVER` is equal to `PN`.
> > 
> > The build does not succeed without the -3 in it
> 
> What was the error? But as noted below, they seem to have changed the
> directory name in the source archive.
> 
> > >> +
> > >> +S = "${WORKDIR}/ImageMagick-${IMVER}"
> > >
> > > It looks like you can delete this line for this version, since in your
> > > version the archive seems to have a “correctly” named folder included.
> > 
> > This line has been removed with success

Are you sure? Testing this, it looks like the default is to make
everything lower case and it broke for me.

| + do_install
| + autotools_do_install
| + oe_runmake DESTDIR=/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/imagemagick-6.7.2-3-r1/image install
| + oenote make DESTDIR=/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/imagemagick-6.7.2-3-r1/image install
| + echo NOTE: 'make DESTDIR=/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/imagemagick-6.7.2-3-r1/image install'
| NOTE: make DESTDIR=/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/imagemagick-6.7.2-3-r1/image install
| + make DESTDIR=/oe/build-minimal-uclibc/minimal-uclibc-dev/work/armv7a-oe-linux-uclibceabi/imagemagick-6.7.2-3-r1/image install
| make: *** No rule to make target `install'.  Stop.
| + die 'oe_runmake failed'
| + oefatal 'oe_runmake failed'
| + echo FATAL: 'oe_runmake failed'
| FATAL: oe_runmake failed
| + exit 1

> > > $ ls minimal-dev/work/armv7a-oe-linux-gnueabi/imagemagick-6.4.4-1-r7/
> > > ImageMagick-6.4.4  temp
> > >
> > > $ ls minimal-dev/work/armv7a-oe-linux-gnueabi/imagemagick-6.7.2-3-r1/
> > > ImageMagick-6.7.2-3  temp
> > >
> > >> +
> > >> +inherit autotools binconfig pkgconfig
> > >> +
> > >> +EXTRA_OECONF = "--program-prefix= --without-x --without-freetype --without-perl --disable-openmp"
> > >> +
> > >> +FILES_${PN} += "${libdir}/ImageMagick-${IMVER}/modules-Q16/*/*.so \
> > >> +                ${libdir}/ImageMagick-${IMVER}/modules-Q16/*/*.la \
> > >> +                ${libdir}/ImageMagick-${IMVER}/config/ \
> > >> +                ${datadir}/ImageMagick-${IMVER}"
> > >
> > > Could you please look into the following warnings.
> > >
> > >        NOTE: package imagemagick-6.7.2-3-r1: task do_qa_staging: Succeeded
> > >        WARNING: the following files were installed but not shipped in any package:
> > >        WARNING:   /usr/share/ImageMagick-6.7.2/francais.xml
> > >        WARNING:   /usr/share/ImageMagick-6.7.2/locale.xml
> > >        WARNING:   /usr/share/ImageMagick-6.7.2/english.xml
> > >        WARNING:   /usr/share/ImageMagick-6.7.2/configure.xml
> > >
> > 
> > Not sure where those warnings show up for you, I don't get them during
> > the build, is there some kind of extra warnings flag I should be
> > using?
> 
> I am using `minimal` (EGLIBC) for `MACHINE = "beagleboard"`. Are these
> files included in your final package?
> 
> > > Here you can also see that the directory name seems to only include
> > > 6.7.2 instead of 6.7.2-3. So you should check where `IMVER` should be
> > > used and where `PV`, which is 6.7.2-3.
> > 
> > Not sure what you mean by this. As stated the build fails without the
> > -3 on the IMVER
> 
> As seen with the warnings above, there seems to be a directory where the
> `-3` needs to be stripped. You should use `PV` instead of `IMVER` where
> possible like below too.

So currently I think the following should be correct.

        IMVER = "6.7.2"

        S = "${WORKDIR}/ImageMagick-${PV}"

Maybe safe the current directory `packages-split/` somewhere and compare
it with the changes above.

> > >> +
> > >> +FILES_${PN}-dev += "${libdir}/ImageMagick-${IMVER}/modules-Q16/*/*.a"
> > >> +
> > >> +FILES_${PN}-dbg += "${libdir}/ImageMagick-${IMVER}/modules-Q16/*/.debug/*"
> > >> +
> > >> +BBCLASSEXTEND = "native"
> > >> +
> > >> +LEAD_SONAME = "libMagickCore.so.*"
> > >> +
> > >> +
> > >> +SRC_URI[md5sum] = "62e12090d2d385f4800c06f58f477bea"
> > >> +SRC_URI[sha256sum] = "50fe6e1af9e900e5d4bef45879a3a7e15512bb9ab3dbbafb521825359cb6d2c7"

Thanks,

Paul

> > > [1] http://www.openembedded.org/wiki/Commit_Policy
> > > [2] http://www.openembedded.org/index.php?title=Styleguide
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.openembedded.org/pipermail/openembedded-devel/attachments/20111007/32619f49/attachment-0002.sig>