* [Buildroot] [PATCH 1/1] exiv2: new package
@ 2014-11-05 14:30 Nicolas Serafini
2014-11-06 7:30 ` Thomas Petazzoni
0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Serafini @ 2014-11-05 14:30 UTC (permalink / raw)
To: buildroot
Add support for Exiv2 library and utility to manage image metadata
Signed-off-by: Nicolas Serafini <nicolas.serafini@sensefly.com>
---
package/Config.in | 1 +
package/exiv2/Config.in | 33 +++++++++++++++++++++++++++++++++
package/exiv2/exiv2.hash | 4 ++++
package/exiv2/exiv2.mk | 32 ++++++++++++++++++++++++++++++++
4 files changed, 70 insertions(+)
create mode 100644 package/exiv2/Config.in
create mode 100644 package/exiv2/exiv2.hash
create mode 100644 package/exiv2/exiv2.mk
diff --git a/package/Config.in b/package/Config.in
index 28cf703..6abf344 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -625,6 +625,7 @@ menu "Graphics"
source "package/adwaita-icon-theme/Config.in"
source "package/atk/Config.in"
source "package/cairo/Config.in"
+ source "package/exiv2/Config.in"
source "package/fltk/Config.in"
source "package/fontconfig/Config.in"
source "package/freetype/Config.in"
diff --git a/package/exiv2/Config.in b/package/exiv2/Config.in
new file mode 100644
index 0000000..d03447b
--- /dev/null
+++ b/package/exiv2/Config.in
@@ -0,0 +1,33 @@
+config BR2_PACKAGE_EXIV2
+ bool "exiv2"
+ help
+ Exiv2 is a C++ library and a command line utility to manage image metadata.
+ It provides fast and easy read and write access to the Exif,
+ IPTC and XMP metadata of images in various formats.
+
+ Exiv2 is available with GPLv2+ or commercial license.
+
+ http://www.exiv2.org/
+
+if BR2_PACKAGE_EXIV2
+
+config BR2_PACKAGE_EXIV2_COMMERCIAL
+ bool "Enable commercial"
+ help
+ Build the commercial version for closed source project.
+ A commercial license request is needed.
+ http://www.exiv2.org/download.html#license
+
+config BR2_PACKAGE_EXIV2_PNG
+ bool "PNG image support"
+ select BR2_PACKAGE_ZLIB
+ help
+ Build with PNG image support
+
+config BR2_PACKAGE_EXIV2_XMP
+ bool "XMP support"
+ select BR2_PACKAGE_EXPAT
+ help
+ Build with XMP support
+
+endif
diff --git a/package/exiv2/exiv2.hash b/package/exiv2/exiv2.hash
new file mode 100644
index 0000000..d4f8c60
--- /dev/null
+++ b/package/exiv2/exiv2.hash
@@ -0,0 +1,4 @@
+# From http://www.exiv2.org/download.html
+md5 b8a23dc56a98ede85c00718a97a8d6fc exiv2-0.24.tar.gz
+# Locally calculated
+sha256 f4a443e6c7fb9d9f5e787732f76969a64c72c4c04af69b10ed57f949c2dfef8e exiv2-0.24.tar.gz
diff --git a/package/exiv2/exiv2.mk b/package/exiv2/exiv2.mk
new file mode 100644
index 0000000..cbf5bcd
--- /dev/null
+++ b/package/exiv2/exiv2.mk
@@ -0,0 +1,32 @@
+################################################################################
+#
+# exiv2
+#
+################################################################################
+
+EXIV2_VERSION = 0.24
+EXIV2_SOURCE = exiv2-$(EXIV2_VERSION).tar.gz
+EXIV2_SITE = http://www.exiv2.org
+EXIV2_LICENSE = GPLv2+ or commercial
+EXIV2_LICENSE_FILES = COPYING
+EXIV2_INSTALL_STAGING = YES
+
+ifeq ($(BR2_PACKAGE_EXIV2_COMMERCIAL),y)
+EXIV2_CONF_OPTS += --enable-commercial --disable-nls --disable-lensdata
+endif
+
+ifeq ($(BR2_PACKAGE_EXIV2_PNG),y)
+EXIV2_CONF_OPTS += --with-zlib=$(STAGING_DIR)
+EXIV2_DEPENDENCIES += zlib
+else
+EXIV2_CONF_OPTS += --without-zlib
+endif
+
+ifeq ($(BR2_PACKAGE_EXIV2_XMP),y)
+EXIV2_CONF_OPTS += --with-expat=$(STAGING_DIR)/usr/lib
+EXIV2_DEPENDENCIES += expat
+else
+EXIV2_CONF_OPTS += --disable-xmp
+endif
+
+$(eval $(autotools-package))
--
2.1.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] exiv2: new package
2014-11-05 14:30 [Buildroot] [PATCH 1/1] exiv2: new package Nicolas Serafini
@ 2014-11-06 7:30 ` Thomas Petazzoni
2014-11-06 14:18 ` Nicolas Serafini
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Petazzoni @ 2014-11-06 7:30 UTC (permalink / raw)
To: buildroot
Dear Nicolas Serafini,
Thanks for your contribution. See a few comments below.
On Wed, 5 Nov 2014 15:30:29 +0100, Nicolas Serafini wrote:
> diff --git a/package/exiv2/Config.in b/package/exiv2/Config.in
> new file mode 100644
> index 0000000..d03447b
> --- /dev/null
> +++ b/package/exiv2/Config.in
> @@ -0,0 +1,33 @@
> +config BR2_PACKAGE_EXIV2
> + bool "exiv2"
> + help
> + Exiv2 is a C++ library and a command line utility to manage image metadata.
This line is a bit too long. Please wrap the help text so that it
doesn't go over 72 columns.
> diff --git a/package/exiv2/exiv2.mk b/package/exiv2/exiv2.mk
> new file mode 100644
> index 0000000..cbf5bcd
> --- /dev/null
> +++ b/package/exiv2/exiv2.mk
> @@ -0,0 +1,32 @@
> +################################################################################
> +#
> +# exiv2
> +#
> +################################################################################
> +
> +EXIV2_VERSION = 0.24
> +EXIV2_SOURCE = exiv2-$(EXIV2_VERSION).tar.gz
This line is not needed, as it is the default value.
> +EXIV2_SITE = http://www.exiv2.org
> +EXIV2_LICENSE = GPLv2+ or commercial
> +EXIV2_LICENSE_FILES = COPYING
You should probably make this conditional:
ifeq ($(BR2_PACKAGE_EXIV2_COMMERCIAL),y)
EXIV2_LICENSE = commercial
else
EXIV2_LICENSE = GPLv2+
EXIV2_LICENSE_FILES = COPYING
endif
(Assuming COPYING contains the text of the GPLv2)
> +ifeq ($(BR2_PACKAGE_EXIV2_COMMERCIAL),y)
> +EXIV2_CONF_OPTS += --enable-commercial --disable-nls --disable-lensdata
> +endif
Why --disable-nls here? It is already passed automatically by the
package infrastructure when locale support is not available.
Maybe a comment about why --disable-lensdata is passed when the
commercial license is used would be useful.
> +
> +ifeq ($(BR2_PACKAGE_EXIV2_PNG),y)
> +EXIV2_CONF_OPTS += --with-zlib=$(STAGING_DIR)
> +EXIV2_DEPENDENCIES += zlib
> +else
> +EXIV2_CONF_OPTS += --without-zlib
> +endif
> +
> +ifeq ($(BR2_PACKAGE_EXIV2_XMP),y)
> +EXIV2_CONF_OPTS += --with-expat=$(STAGING_DIR)/usr/lib
Maybe --enable-xmp here?
> +EXIV2_DEPENDENCIES += expat
> +else
> +EXIV2_CONF_OPTS += --disable-xmp
> +endif
> +
> +$(eval $(autotools-package))
Other than that, this patch looks good. Could you submit an updated
version that fixes the minor issues mentioned above?
Thanks a lot!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] exiv2: new package
2014-11-06 7:30 ` Thomas Petazzoni
@ 2014-11-06 14:18 ` Nicolas Serafini
2014-11-06 15:01 ` Thomas Petazzoni
0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Serafini @ 2014-11-06 14:18 UTC (permalink / raw)
To: buildroot
Hi Thomas,
Le 06. 11. 14 08:30, Thomas Petazzoni a ?crit :
> Dear Nicolas Serafini,
>
> Thanks for your contribution. See a few comments below.
>
> On Wed, 5 Nov 2014 15:30:29 +0100, Nicolas Serafini wrote:
>
>> diff --git a/package/exiv2/Config.in b/package/exiv2/Config.in
>> new file mode 100644
>> index 0000000..d03447b
>> --- /dev/null
>> +++ b/package/exiv2/Config.in
>> @@ -0,0 +1,33 @@
>> +config BR2_PACKAGE_EXIV2
>> + bool "exiv2"
>> + help
>> + Exiv2 is a C++ library and a command line utility to manage image metadata.
>
> This line is a bit too long. Please wrap the help text so that it
> doesn't go over 72 columns.
Ok I fix this.
I have not found the rule for maximum column in Config.in and *.mk files
in the manual. Maybe we should add.
>
>> diff --git a/package/exiv2/exiv2.mk b/package/exiv2/exiv2.mk
>> new file mode 100644
>> index 0000000..cbf5bcd
>> --- /dev/null
>> +++ b/package/exiv2/exiv2.mk
>> @@ -0,0 +1,32 @@
>> +################################################################################
>> +#
>> +# exiv2
>> +#
>> +################################################################################
>> +
>> +EXIV2_VERSION = 0.24
>> +EXIV2_SOURCE = exiv2-$(EXIV2_VERSION).tar.gz
>
> This line is not needed, as it is the default value.
Ok, I didn't know that it was the default value. I fix this.
>
>> +EXIV2_SITE = http://www.exiv2.org
>> +EXIV2_LICENSE = GPLv2+ or commercial
>> +EXIV2_LICENSE_FILES = COPYING
>
> You should probably make this conditional:
>
> ifeq ($(BR2_PACKAGE_EXIV2_COMMERCIAL),y)
> EXIV2_LICENSE = commercial
> else
> EXIV2_LICENSE = GPLv2+
> EXIV2_LICENSE_FILES = COPYING
> endif
>
> (Assuming COPYING contains the text of the GPLv2)
Ok I fix, good idea.
>
>> +ifeq ($(BR2_PACKAGE_EXIV2_COMMERCIAL),y)
>> +EXIV2_CONF_OPTS += --enable-commercial --disable-nls --disable-lensdata
>> +endif
>
> Why --disable-nls here? It is already passed automatically by the
> package infrastructure when locale support is not available.
>
> Maybe a comment about why --disable-lensdata is passed when the
> commercial license is used would be useful.
The --disable-lensdata disable an included Nikon lens database for
conversion to readable lens name and this database is free use only in
non-commercial projects.
For the --disable-nls, I don't know exactly why it's requested in
commercial version.
This is what is specified in the README file.
>
>> +
>> +ifeq ($(BR2_PACKAGE_EXIV2_PNG),y)
>> +EXIV2_CONF_OPTS += --with-zlib=$(STAGING_DIR)
>> +EXIV2_DEPENDENCIES += zlib
>> +else
>> +EXIV2_CONF_OPTS += --without-zlib
>> +endif
>> +
>> +ifeq ($(BR2_PACKAGE_EXIV2_XMP),y)
>> +EXIV2_CONF_OPTS += --with-expat=$(STAGING_DIR)/usr/lib
>
> Maybe --enable-xmp here?
It works in both cases because XMP is automatically enabled if expat is
found.
>
>> +EXIV2_DEPENDENCIES += expat
>> +else
>> +EXIV2_CONF_OPTS += --disable-xmp
>> +endif
>> +
>> +$(eval $(autotools-package))
>
> Other than that, this patch looks good. Could you submit an updated
> version that fixes the minor issues mentioned above?
>
> Thanks a lot!
>
> Thomas
>
I do the changes and I send the new patch
Thanks for the review.
Nicolas
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] exiv2: new package
2014-11-06 14:18 ` Nicolas Serafini
@ 2014-11-06 15:01 ` Thomas Petazzoni
2014-11-06 15:06 ` Nicolas Serafini
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Petazzoni @ 2014-11-06 15:01 UTC (permalink / raw)
To: buildroot
Dear Nicolas Serafini,
On Thu, 6 Nov 2014 15:18:04 +0100, Nicolas Serafini wrote:
> Ok I fix this.
> I have not found the rule for maximum column in Config.in and *.mk files
> in the manual. Maybe we should add.
Feel free to submit a patch :-)
> >> +ifeq ($(BR2_PACKAGE_EXIV2_COMMERCIAL),y)
> >> +EXIV2_CONF_OPTS += --enable-commercial --disable-nls --disable-lensdata
> >> +endif
> >
> > Why --disable-nls here? It is already passed automatically by the
> > package infrastructure when locale support is not available.
> >
> > Maybe a comment about why --disable-lensdata is passed when the
> > commercial license is used would be useful.
>
> The --disable-lensdata disable an included Nikon lens database for
> conversion to readable lens name and this database is free use only in
> non-commercial projects.
This is *bad*. It's not because you use the GPLv2 version of the
library that you're not making a commercial product. So please add a
separate Config.in option for this lensdata, disabled by default, with
a warning in the Config.in help text.
> For the --disable-nls, I don't know exactly why it's requested in
> commercial version.
> This is what is specified in the README file.
Then please add a comment above the usage of --disable-nls to explain
that.
> >> +ifeq ($(BR2_PACKAGE_EXIV2_XMP),y)
> >> +EXIV2_CONF_OPTS += --with-expat=$(STAGING_DIR)/usr/lib
> >
> > Maybe --enable-xmp here?
>
> It works in both cases because XMP is automatically enabled if expat is
> found.
If --enable-xmp exists, then I would prefer to see it explicitly used.
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] exiv2: new package
2014-11-06 15:01 ` Thomas Petazzoni
@ 2014-11-06 15:06 ` Nicolas Serafini
0 siblings, 0 replies; 5+ messages in thread
From: Nicolas Serafini @ 2014-11-06 15:06 UTC (permalink / raw)
To: buildroot
Dear THomas,
Ok I will fix all this.
I was a little to fast with the v2 of the Patch so do not take into account.
Thanks,
Nicolas
Le 06. 11. 14 16:01, Thomas Petazzoni a ?crit :
> Dear Nicolas Serafini,
>
> On Thu, 6 Nov 2014 15:18:04 +0100, Nicolas Serafini wrote:
>
>> Ok I fix this.
>> I have not found the rule for maximum column in Config.in and *.mk files
>> in the manual. Maybe we should add.
>
> Feel free to submit a patch :-)
>
>>>> +ifeq ($(BR2_PACKAGE_EXIV2_COMMERCIAL),y)
>>>> +EXIV2_CONF_OPTS += --enable-commercial --disable-nls --disable-lensdata
>>>> +endif
>>>
>>> Why --disable-nls here? It is already passed automatically by the
>>> package infrastructure when locale support is not available.
>>>
>>> Maybe a comment about why --disable-lensdata is passed when the
>>> commercial license is used would be useful.
>>
>> The --disable-lensdata disable an included Nikon lens database for
>> conversion to readable lens name and this database is free use only in
>> non-commercial projects.
>
> This is *bad*. It's not because you use the GPLv2 version of the
> library that you're not making a commercial product. So please add a
> separate Config.in option for this lensdata, disabled by default, with
> a warning in the Config.in help text.
>
>> For the --disable-nls, I don't know exactly why it's requested in
>> commercial version.
>> This is what is specified in the README file.
>
> Then please add a comment above the usage of --disable-nls to explain
> that.
>
>>>> +ifeq ($(BR2_PACKAGE_EXIV2_XMP),y)
>>>> +EXIV2_CONF_OPTS += --with-expat=$(STAGING_DIR)/usr/lib
>>>
>>> Maybe --enable-xmp here?
>>
>> It works in both cases because XMP is automatically enabled if expat is
>> found.
>
> If --enable-xmp exists, then I would prefer to see it explicitly used.
>
> Thanks,
>
> Thomas
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-11-06 15:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-05 14:30 [Buildroot] [PATCH 1/1] exiv2: new package Nicolas Serafini
2014-11-06 7:30 ` Thomas Petazzoni
2014-11-06 14:18 ` Nicolas Serafini
2014-11-06 15:01 ` Thomas Petazzoni
2014-11-06 15:06 ` Nicolas Serafini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox