[v2] package/exiv2: cleanup options and licenses
diff mbox series

Message ID 20190508103129.100751-1-nicolas.serafini@sensefly.com
State Superseded, archived
Headers show
Series
  • [v2] package/exiv2: cleanup options and licenses
Related show

Commit Message

Nicolas Serafini May 8, 2019, 10:32 a.m. UTC
exiv2 no longer requires a commercial option for lens database
integration since version 0.27.

Legacy handling for the removed options _COMMERCIAL and _LENSDATA is
not needed, since now they are always enabled.

Add the dedicated BSD-3-Clause license file for CMakeLists.txt,
config/FindEXPAT.cmake and config/FindMSGFMT.cmake files.

Signed-off-by: Nicolas Serafini <nicolas.serafini@sensefly.com>

---
Changes v1 -> v2:
  - remove _LENSDATA option (suggested by Arnout Vandecappelle)
  - remove commercial license in Config.in help
  - add missing BSD-3-Clause flag
  - Add details to the patch message

---
 package/exiv2/Config.in  | 23 -----------------------
 package/exiv2/exiv2.hash |  1 +
 package/exiv2/exiv2.mk   | 17 +++--------------
 3 files changed, 4 insertions(+), 37 deletions(-)

Comments

Thomas Petazzoni May 26, 2019, 8:36 p.m. UTC | #1
Hello Nicolas,

Thanks for the patch!

On Wed, 8 May 2019 10:32:21 +0000
Nicolas Serafini <nicolas.serafini@sensefly.com> wrote:

> exiv2 no longer requires a commercial option for lens database
> integration since version 0.27.
> 
> Legacy handling for the removed options _COMMERCIAL and _LENSDATA is
> not needed, since now they are always enabled.

While I understand the reasoning for the removal of the _COMMERCIAL
option, I don't understand the relationship with the removal of the
_LENSDATA option. Why is this option removed and the installation of
lens data made mandatory ?

And if there's a reason for it, why is it done as part of the patch
dropping the _COMMERCIAL option ?

> diff --git a/package/exiv2/exiv2.hash b/package/exiv2/exiv2.hash
> index f99cb8cb87..b753c03854 100644
> --- a/package/exiv2/exiv2.hash
> +++ b/package/exiv2/exiv2.hash
> @@ -1,3 +1,4 @@
>  # Locally calculated
>  sha256 1b3766b2c203ce213a4195de14d61694017ec1a69d15d4575bccecef130990fe exiv2-0.27.1.tar.gz
>  sha256 a7ba75cb966aca374711e2af49e5f3aea6a4443a803440f5d93e73a5a1222f66 COPYING
> +sha256 46cde7dc11e64c78d650b4851b88f6704b4665ff60f22a1caf68ceb15e217e5b COPYING-CMAKE-SCRIPTS
> diff --git a/package/exiv2/exiv2.mk b/package/exiv2/exiv2.mk
> index ba9d9b7303..c557c08fb8 100644
> --- a/package/exiv2/exiv2.mk
> +++ b/package/exiv2/exiv2.mk
> @@ -7,27 +7,16 @@
>  EXIV2_VERSION = 0.27.1
>  EXIV2_SITE = $(call github,Exiv2,exiv2,$(EXIV2_VERSION))
>  EXIV2_INSTALL_STAGING = YES
> +EXIV2_LICENSE = GPL-2.0+ BSD-3-Clause

The proper separator for the _LICENSE variable is a comma, i.e:

EXIV2_LICENSE = GPL-2.0+, BSD-3-Clause

I can fix that when applying, but I'd like to understand the rationale
for the removal of the _LENSDATA option.

Thanks!

Thomas
Arnout Vandecappelle May 26, 2019, 9:50 p.m. UTC | #2
On 26/05/2019 22:36, Thomas Petazzoni wrote:
> Hello Nicolas,
> 
> Thanks for the patch!
> 
> On Wed, 8 May 2019 10:32:21 +0000
> Nicolas Serafini <nicolas.serafini@sensefly.com> wrote:
> 
>> exiv2 no longer requires a commercial option for lens database
>> integration since version 0.27.
>>
>> Legacy handling for the removed options _COMMERCIAL and _LENSDATA is
>> not needed, since now they are always enabled.
> 
> While I understand the reasoning for the removal of the _COMMERCIAL
> option, I don't understand the relationship with the removal of the
> _LENSDATA option. Why is this option removed and the installation of
> lens data made mandatory ?
> 
> And if there's a reason for it, why is it done as part of the patch
> dropping the _COMMERCIAL option ?

 This was discussed in v1 of the patch (and indeed should have been part of the
commit log).

The BR2_PACKAGE_EXIV2_LENSDATA option only existed as a separate option because
of hte license concerns (incompatibility with the commercial license). Since
these concerns have gone away, and the license database is only 80KB, remove
this option as well.

 It could have been done in a separate patch, but since anyway the help text
would have to change in this patch, I think it's not worth splitting off.

 Regards,
 Arnout

> 
>> diff --git a/package/exiv2/exiv2.hash b/package/exiv2/exiv2.hash
>> index f99cb8cb87..b753c03854 100644
>> --- a/package/exiv2/exiv2.hash
>> +++ b/package/exiv2/exiv2.hash
>> @@ -1,3 +1,4 @@
>>  # Locally calculated
>>  sha256 1b3766b2c203ce213a4195de14d61694017ec1a69d15d4575bccecef130990fe exiv2-0.27.1.tar.gz
>>  sha256 a7ba75cb966aca374711e2af49e5f3aea6a4443a803440f5d93e73a5a1222f66 COPYING
>> +sha256 46cde7dc11e64c78d650b4851b88f6704b4665ff60f22a1caf68ceb15e217e5b COPYING-CMAKE-SCRIPTS
>> diff --git a/package/exiv2/exiv2.mk b/package/exiv2/exiv2.mk
>> index ba9d9b7303..c557c08fb8 100644
>> --- a/package/exiv2/exiv2.mk
>> +++ b/package/exiv2/exiv2.mk
>> @@ -7,27 +7,16 @@
>>  EXIV2_VERSION = 0.27.1
>>  EXIV2_SITE = $(call github,Exiv2,exiv2,$(EXIV2_VERSION))
>>  EXIV2_INSTALL_STAGING = YES
>> +EXIV2_LICENSE = GPL-2.0+ BSD-3-Clause
> 
> The proper separator for the _LICENSE variable is a comma, i.e:
> 
> EXIV2_LICENSE = GPL-2.0+, BSD-3-Clause
> 
> I can fix that when applying, but I'd like to understand the rationale
> for the removal of the _LENSDATA option.
> 
> Thanks!
> 
> Thomas
>
Thomas Petazzoni May 27, 2019, 7:06 a.m. UTC | #3
Hello,

On Sun, 26 May 2019 23:50:15 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> > And if there's a reason for it, why is it done as part of the patch
> > dropping the _COMMERCIAL option ?  
> 
>  This was discussed in v1 of the patch (and indeed should have been part of the
> commit log).
> 
> The BR2_PACKAGE_EXIV2_LENSDATA option only existed as a separate option because
> of hte license concerns (incompatibility with the commercial license). Since
> these concerns have gone away, and the license database is only 80KB, remove
> this option as well.
> 
>  It could have been done in a separate patch, but since anyway the help text
> would have to change in this patch, I think it's not worth splitting off.

Thanks for the additional explanation. It should have been part of the
commit log indeed.

What do we do now? Nicolas, do you submit a v3 with an improved commit
log? Do we resurrect v2 from patchwork, and adjust the commit log when
applying?

Thomas
Nicolas Serafini June 3, 2019, 8:49 a.m. UTC | #4
Hello,

Sorry for the late response I was on holiday last week.


On Mon, 27 May 2019 09:06:33 +0200 Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

>Hello,
>
>On Sun, 26 May 2019 23:50:15 +0200
>Arnout Vandecappelle <arnout@mind.be> wrote:
>
>> > And if there's a reason for it, why is it done as part of the patch
>> > dropping the _COMMERCIAL option ?  
>> 
>>  This was discussed in v1 of the patch (and indeed should have been
>> part of the commit log).
>> 
>> The BR2_PACKAGE_EXIV2_LENSDATA option only existed as a separate
>> option because of hte license concerns (incompatibility with the
>> commercial license). Since these concerns have gone away, and the
>> license database is only 80KB, remove this option as well.
>> 
>>  It could have been done in a separate patch, but since anyway the
>> help text would have to change in this patch, I think it's not worth
>> splitting off.
>
>Thanks for the additional explanation. It should have been part of the
>commit log indeed.
>
>What do we do now? Nicolas, do you submit a v3 with an improved commit
>log? Do we resurrect v2 from patchwork, and adjust the commit log when
>applying?

I will submit a v3 with improved commit message.

Nicolas

>
>Thomas

Patch
diff mbox series

diff --git a/package/exiv2/Config.in b/package/exiv2/Config.in
index acce81c593..72c2a88040 100644
--- a/package/exiv2/Config.in
+++ b/package/exiv2/Config.in
@@ -16,24 +16,10 @@  config BR2_PACKAGE_EXIV2
 	  access to the Exif, IPTC and XMP metadata of images in
 	  various formats.
 
-	  Exiv2 is available under the GPLv2+ or under a 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.
-
-	  The Nikon lens name database and the NLS support is disabled
-	  for copyright reasons.
-
-	  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
@@ -46,13 +32,4 @@  config BR2_PACKAGE_EXIV2_XMP
 	help
 	  Build with XMP support
 
-config BR2_PACKAGE_EXIV2_LENSDATA
-	bool "Nikon lens name database"
-	depends on !BR2_PACKAGE_EXIV2_COMMERCIAL
-	help
-	  Integrate Nikon lens name database.
-
-	  This database is integrated but comes from a thirdparty:
-	  http://www.rottmerhusen.com/objektives/lensid/thirdparty.html.
-
 endif
diff --git a/package/exiv2/exiv2.hash b/package/exiv2/exiv2.hash
index f99cb8cb87..b753c03854 100644
--- a/package/exiv2/exiv2.hash
+++ b/package/exiv2/exiv2.hash
@@ -1,3 +1,4 @@ 
 # Locally calculated
 sha256 1b3766b2c203ce213a4195de14d61694017ec1a69d15d4575bccecef130990fe exiv2-0.27.1.tar.gz
 sha256 a7ba75cb966aca374711e2af49e5f3aea6a4443a803440f5d93e73a5a1222f66 COPYING
+sha256 46cde7dc11e64c78d650b4851b88f6704b4665ff60f22a1caf68ceb15e217e5b COPYING-CMAKE-SCRIPTS
diff --git a/package/exiv2/exiv2.mk b/package/exiv2/exiv2.mk
index ba9d9b7303..c557c08fb8 100644
--- a/package/exiv2/exiv2.mk
+++ b/package/exiv2/exiv2.mk
@@ -7,27 +7,16 @@ 
 EXIV2_VERSION = 0.27.1
 EXIV2_SITE = $(call github,Exiv2,exiv2,$(EXIV2_VERSION))
 EXIV2_INSTALL_STAGING = YES
+EXIV2_LICENSE = GPL-2.0+ BSD-3-Clause
+EXIV2_LICENSE_FILES = COPYING COPYING-CMAKE-SCRIPTS
 
 EXIV2_CONF_OPTS += -DEXIV2_ENABLE_BUILD_SAMPLES=OFF
+EXIV2_CONF_OPTS += -DEXIV2_ENABLE_LENSDATA=ON
 
 # The following CMake variable disables a TRY_RUN call in the -pthread
 # test which is not allowed when cross-compiling.
 EXIV2_CONF_OPTS += -DTHREADS_PTHREAD_ARG=OFF
 
-ifeq ($(BR2_PACKAGE_EXIV2_LENSDATA),)
-EXIV2_CONF_OPTS += -DEXIV2_ENABLE_LENSDATA=OFF
-endif
-
-ifeq ($(BR2_PACKAGE_EXIV2_COMMERCIAL),y)
-EXIV2_LICENSE = commercial
-# NLS support is disabled in commercial version due to the copyright
-# of the translated texts.
-EXIV2_CONF_OPTS += -DEXIV2_ENABLE_COMMERCIAL=ON -DEXIV2_ENABLE_NLS=OFF
-else
-EXIV2_LICENSE = GPL-2.0+
-EXIV2_LICENSE_FILES = COPYING
-endif
-
 ifeq ($(BR2_PACKAGE_EXIV2_PNG),y)
 EXIV2_CONF_OPTS += -DEXIV2_ENABLE_PNG=ON
 EXIV2_DEPENDENCIES += zlib