[Buildroot] [PATCH] tcl: rework logic for databases support

[Buildroot] [PATCH] tcl: rework logic for databases support

[Vicente Olivert Riera]

- Always remove the bundled sqlite version.
- Remove the --with-system-sqlite option because it is an option which
  only exists in the bundled sqlite configure script.
- Always build the basic TDBC.
- Build SQLite, MySQL and PostgreSQL TDBC drivers only if these packages
  have been selected in Buildroot. Add them to TCL_DEPENDENCIES
  accordingly.

Fixes:
  http://autobuild.buildroot.net/results/022/02296f8624d3406a63d3a179f53862f245c56dc1/

Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
---
 package/tcl/tcl.mk | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/package/tcl/tcl.mk b/package/tcl/tcl.mk
index 9fd1195..1ec7741 100644
--- a/package/tcl/tcl.mk
+++ b/package/tcl/tcl.mk
@@ -14,15 +14,10 @@ TCL_SUBDIR = unix
 TCL_INSTALL_STAGING = YES
 TCL_AUTORECONF = YES
 
-# Note that --with-system-sqlite will only make a difference
-# in the sqlite package (which gets removed if sqlite not
-# configured).  Don't need to worry about conditionally including
-# it in the configure options
 TCL_CONF_OPTS = \
 	--disable-symbols \
 	--disable-langinfo \
-	--disable-framework \
-	--with-system-sqlite
+	--disable-framework
 
 HOST_TCL_CONF_OPTS = \
 	--disable-symbols \
@@ -32,7 +27,7 @@ HOST_TCL_CONF_OPTS = \
 # I haven't found a good way to force pkgs to not build
 # or configure without just removing the entire pkg directory.
 define HOST_TCL_REMOVE_PACKAGES
-	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9].[0-9] \
+	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9][0-9].[0-9] \
 		$(@D)/pkgs/tdbc[0-9].[0-9].[0-9] \
 		$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9] \
 		$(@D)/pkgs/tdbcodbc[0-9].[0-9].[0-9] \
@@ -41,11 +36,10 @@ define HOST_TCL_REMOVE_PACKAGES
 endef
 HOST_TCL_PRE_CONFIGURE_HOOKS += HOST_TCL_REMOVE_PACKAGES
 define TCL_REMOVE_PACKAGES
-	rm -fr $(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/sqlite[0-9].[0-9].[0-9]) \
-		$(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/tdbc[0-9].[0-9].[0-9]) \
-		$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9] \
+	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9][0-9].[0-9] \
+		$(if $(BR2_PACKAGE_MYSQL),,$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9]) \
 		$(@D)/pkgs/tdbcodbc[0-9].[0-9].[0-9] \
-		$(@D)/pkgs/tdbcpostgres[0-9].[0-9].[0-9] \
+		$(if $(BR2_PACKAGE_POSTGRESQL),,$(@D)/pkgs/tdbcpostgres[0-9].[0-9].[0-9]) \
 		$(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/tdbcsqlite3-[0-9].[0-9].[0-9])
 endef
 TCL_PRE_CONFIGURE_HOOKS += TCL_REMOVE_PACKAGES
@@ -82,6 +76,8 @@ endef
 TCL_POST_INSTALL_TARGET_HOOKS += TCL_REMOVE_EXTRA
 
 TCL_DEPENDENCIES = $(if $(BR2_PACKAGE_SQLITE),sqlite)
+TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_MYSQL),mysql)
+TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_POSTGRESQL),postgresql)
 HOST_TCL_DEPENDENCIES =
 
 $(eval $(autotools-package))
-- 
2.7.3

[Buildroot] [PATCH] tcl: rework logic for databases support

[Thomas Petazzoni]

Hello,

On Wed, 30 Mar 2016 16:21:22 +0100, Vicente Olivert Riera wrote:

> @@ -32,7 +27,7 @@ HOST_TCL_CONF_OPTS = \
>  # I haven't found a good way to force pkgs to not build
>  # or configure without just removing the entire pkg directory.
>  define HOST_TCL_REMOVE_PACKAGES
> -	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9].[0-9] \
> +	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9][0-9].[0-9] \
>  		$(@D)/pkgs/tdbc[0-9].[0-9].[0-9] \
>  		$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9] \
>  		$(@D)/pkgs/tdbcodbc[0-9].[0-9].[0-9] \

Could we switch to using * instead of this silly [0-9] thing? So:

	rm -rf $(@D)/pkgs/sqlite* $(@D)/pkgs/tdbc* ...

> @@ -41,11 +36,10 @@ define HOST_TCL_REMOVE_PACKAGES
>  endef
>  HOST_TCL_PRE_CONFIGURE_HOOKS += HOST_TCL_REMOVE_PACKAGES
>  define TCL_REMOVE_PACKAGES
> -	rm -fr $(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/sqlite[0-9].[0-9].[0-9]) \
> -		$(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/tdbc[0-9].[0-9].[0-9]) \
> -		$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9] \
> +	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9][0-9].[0-9] \
> +		$(if $(BR2_PACKAGE_MYSQL),,$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9]) \
>  		$(@D)/pkgs/tdbcodbc[0-9].[0-9].[0-9] \
> -		$(@D)/pkgs/tdbcpostgres[0-9].[0-9].[0-9] \
> +		$(if $(BR2_PACKAGE_POSTGRESQL),,$(@D)/pkgs/tdbcpostgres[0-9].[0-9].[0-9]) \
>  		$(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/tdbcsqlite3-[0-9].[0-9].[0-9])

OK, I think this would need a comment above to explain what's going on,
it's not trivial to understand why you're removing sqlite and tdbcodbc
unconditionally, and tdbcpostgresq/tdbcsqlite conditionally.

>  endef
>  TCL_PRE_CONFIGURE_HOOKS += TCL_REMOVE_PACKAGES
> @@ -82,6 +76,8 @@ endef
>  TCL_POST_INSTALL_TARGET_HOOKS += TCL_REMOVE_EXTRA
>  
>  TCL_DEPENDENCIES = $(if $(BR2_PACKAGE_SQLITE),sqlite)
> +TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_MYSQL),mysql)
> +TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_POSTGRESQL),postgresql)

TCL_DEPENDENCIES = \
	$(if ...) \
	$(if ...)

Would be nicer.

Maybe this calls for three patches:

 1/ Switch to * instead of [0-9]
 2/ Fix the sqlite usage
 3/ Enable postgresql/mysql usage

Thanks!

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

[Buildroot] [PATCH] tcl: rework logic for databases support

[Vicente Olivert Riera]

Hi Thomas,

On 30/03/16 16:35, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 30 Mar 2016 16:21:22 +0100, Vicente Olivert Riera wrote:
> 
>> @@ -32,7 +27,7 @@ HOST_TCL_CONF_OPTS = \
>>  # I haven't found a good way to force pkgs to not build
>>  # or configure without just removing the entire pkg directory.
>>  define HOST_TCL_REMOVE_PACKAGES
>> -	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9].[0-9] \
>> +	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9][0-9].[0-9] \
>>  		$(@D)/pkgs/tdbc[0-9].[0-9].[0-9] \
>>  		$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9] \
>>  		$(@D)/pkgs/tdbcodbc[0-9].[0-9].[0-9] \
> 
> Could we switch to using * instead of this silly [0-9] thing? So:
> 
> 	rm -rf $(@D)/pkgs/sqlite* $(@D)/pkgs/tdbc* ...

No problem.

>> @@ -41,11 +36,10 @@ define HOST_TCL_REMOVE_PACKAGES
>>  endef
>>  HOST_TCL_PRE_CONFIGURE_HOOKS += HOST_TCL_REMOVE_PACKAGES
>>  define TCL_REMOVE_PACKAGES
>> -	rm -fr $(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/sqlite[0-9].[0-9].[0-9]) \
>> -		$(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/tdbc[0-9].[0-9].[0-9]) \
>> -		$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9] \
>> +	rm -fr $(@D)/pkgs/sqlite[0-9].[0-9][0-9].[0-9] \
>> +		$(if $(BR2_PACKAGE_MYSQL),,$(@D)/pkgs/tdbcmysql[0-9].[0-9].[0-9]) \
>>  		$(@D)/pkgs/tdbcodbc[0-9].[0-9].[0-9] \
>> -		$(@D)/pkgs/tdbcpostgres[0-9].[0-9].[0-9] \
>> +		$(if $(BR2_PACKAGE_POSTGRESQL),,$(@D)/pkgs/tdbcpostgres[0-9].[0-9].[0-9]) \
>>  		$(if $(BR2_PACKAGE_SQLITE),,$(@D)/pkgs/tdbcsqlite3-[0-9].[0-9].[0-9])
> 
> OK, I think this would need a comment above to explain what's going on,
> it's not trivial to understand why you're removing sqlite and tdbcodbc
> unconditionally, and tdbcpostgresq/tdbcsqlite conditionally.

We remove sqlite unconditionally because we don't want to use the
bundled version.

For tdbcodbc I don't have any particular reason to remove it or keep it.
What do you prefer?

> 
>>  endef
>>  TCL_PRE_CONFIGURE_HOOKS += TCL_REMOVE_PACKAGES
>> @@ -82,6 +76,8 @@ endef
>>  TCL_POST_INSTALL_TARGET_HOOKS += TCL_REMOVE_EXTRA
>>  
>>  TCL_DEPENDENCIES = $(if $(BR2_PACKAGE_SQLITE),sqlite)
>> +TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_MYSQL),mysql)
>> +TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_POSTGRESQL),postgresql)
> 
> TCL_DEPENDENCIES = \
> 	$(if ...) \
> 	$(if ...)
> 
> Would be nicer.

That was my first approach and I didn't look nicer to me, so I changed
it :P Anyway, if you prefer that way, I'll do it that way, no problem.

> Maybe this calls for three patches:
> 
>  1/ Switch to * instead of [0-9]
>  2/ Fix the sqlite usage
>  3/ Enable postgresql/mysql usage

I agree. Please answer my question about tdbcodbc and I will write those
three patches.

Regards,

Vincent.

> Thanks!
> 
> Thomas
> 

[Buildroot] [PATCH] tcl: rework logic for databases support

[Thomas Petazzoni]

Hello,

On Wed, 30 Mar 2016 16:39:24 +0100, Vicente Olivert Riera wrote:

> We remove sqlite unconditionally because we don't want to use the
> bundled version.

Yes, explain this in a comment :)

> For tdbcodbc I don't have any particular reason to remove it or keep it.
> What do you prefer?

If it was already removed before your patch, keep it this way.

Thanks!

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

[Buildroot] [PATCH] tcl: rework logic for databases support

[Vicente Olivert Riera]

Hi Thomas,

On 30/03/16 16:41, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 30 Mar 2016 16:39:24 +0100, Vicente Olivert Riera wrote:
> 
>> We remove sqlite unconditionally because we don't want to use the
>> bundled version.
> 
> Yes, explain this in a comment :)

Sure.

>> For tdbcodbc I don't have any particular reason to remove it or keep it.
>> What do you prefer?
> 
> If it was already removed before your patch, keep it this way.

It was, yes.

Thanks for the quick reply.

Regards,

Vincent.

> 
> Thanks!
> 
> Thomas
> 

[Buildroot] [PATCH] tcl: rework logic for databases support

[Arnout Vandecappelle]

On 03/30/16 17:39, Vicente Olivert Riera wrote:
> Hi Thomas,
>
> On 30/03/16 16:35, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Wed, 30 Mar 2016 16:21:22 +0100, Vicente Olivert Riera wrote:
[snip]
>>>   TCL_DEPENDENCIES = $(if $(BR2_PACKAGE_SQLITE),sqlite)
>>> +TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_MYSQL),mysql)
>>> +TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_POSTGRESQL),postgresql)
>>
>> TCL_DEPENDENCIES = \
>> 	$(if ...) \
>> 	$(if ...)
>>
>> Would be nicer.
>
> That was my first approach and I didn't look nicer to me, so I changed
> it :P Anyway, if you prefer that way, I'll do it that way, no problem.

  Like you, Vincent, I also prefer TCL_DEPENDENCIES to be repeated.

- You don't need to move your eyes to see which variable is affected.
- When adding/removing a dependency, you don't have to worry about the backslashes.


  I do prefer in this case that all assignments, including the first one, have +=


  Regards,
  Arnout


[snip]
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] tcl: rework logic for databases support

[Thomas Petazzoni]

Hello,

On Wed, 30 Mar 2016 21:03:27 +0200, Arnout Vandecappelle wrote:

> > That was my first approach and I didn't look nicer to me, so I changed
> > it :P Anyway, if you prefer that way, I'll do it that way, no problem.
> 
>   Like you, Vincent, I also prefer TCL_DEPENDENCIES to be repeated.
> 
> - You don't need to move your eyes to see which variable is affected.
> - When adding/removing a dependency, you don't have to worry about the backslashes.
> 
> 
>   I do prefer in this case that all assignments, including the first one, have +=

Matter of taste, I guess. But if I follow your thought, then we should
do:

LIBGTK3_CONF_OPTS += --disable-glibtest
LIBGTK3_CONF_OPTS += --enable-explicit-deps=no
LIBGTK3_CONF_OPTS += --enable-gtk2-dependency
LIBGTK3_CONF_OPTS += --disable-introspection

I think this is really verbose, but again a matter of taste.

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

[Buildroot] [PATCH] tcl: rework logic for databases support

[Arnout Vandecappelle]

On 03/30/16 21:06, Thomas Petazzoni wrote:
> Hello,
>
> On Wed, 30 Mar 2016 21:03:27 +0200, Arnout Vandecappelle wrote:
>
> >> That was my first approach and I didn't look nicer to me, so I changed
> >> it :P Anyway, if you prefer that way, I'll do it that way, no problem.
> >
> >    Like you, Vincent, I also prefer TCL_DEPENDENCIES to be repeated.
> >
> > - You don't need to move your eyes to see which variable is affected.
> > - When adding/removing a dependency, you don't have to worry about the 
> backslashes.
> >
> >
> >    I do prefer in this case that all assignments, including the first one, 
> have +=
>
> Matter of taste, I guess. But if I follow your thought, then we should
> do:
>
> LIBGTK3_CONF_OPTS += --disable-glibtest
> LIBGTK3_CONF_OPTS += --enable-explicit-deps=no
> LIBGTK3_CONF_OPTS += --enable-gtk2-dependency
> LIBGTK3_CONF_OPTS += --disable-introspection
>
> I think this is really verbose, but again a matter of taste.
>

  Taste is a funny thing... With just plain options I also prefer it 
backslashified, but when there are $(if ...) clauses it somehow looks cleaner 
with the +=.

  Anyway, we have both already and there is no strong reason to choose either.

  Regards,
  Arnout


-- 
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF