Skip to contentSkip to conversation

tcl: rework logic for databases support

PATCH New Vicente Olivert Riera 3 participants 8 messages Mar 30, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tcl: rework logic for databases support

- 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/

---
 package/tcl/tcl.mk | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)
package/tcl/tcl.mk +7 -11
index 9fd1195..1ec7741 100644
@@ -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))
Signature · 1 line
2.7.3
#1 lore raw

PATCH 1/1 tcl: rework logic for databases support

Hello,

package/tcl/tcl.mk HOST_TCL_CONF_OPTS = \ · line 33

Quoted 9 lines · Vicente Olivert Riera
package/tcl/tcl.mk +1 -1
@@ -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* ...

package/tcl/tcl.mk define HOST_TCL_REMOVE_PACKAGES · line 43

Quoted 13 lines
package/tcl/tcl.mk +3 -4
@@ -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.

package/tcl/tcl.mk endef · line 80

Quoted 8 lines
package/tcl/tcl.mk +2 -0
 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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 tcl: rework logic for databases support

Hi Thomas,

Quoted 16 lines · Thomas Petazzoni

Hello,

package/tcl/tcl.mk HOST_TCL_CONF_OPTS = \ · line 33

Quoted 9 lines · Vicente Olivert Riera
package/tcl/tcl.mk +1 -1
@@ -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:

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

No problem.

Quoted 9 lines
Quoted 9 lines
package/tcl/tcl.mk +2 -3
@@ -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]) \
Quoted 8 lines

package/tcl/tcl.mk define HOST_TCL_REMOVE_PACKAGES · line 43

Quoted 4 lines
package/tcl/tcl.mk +1 -1
 		$(@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?

Quoted 14 lines

package/tcl/tcl.mk endef · line 80

Quoted 9 lines
package/tcl/tcl.mk +2 -0
 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 ...)
Quoted 1 line

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.

Quoted 5 lines

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.

Quoted 4 lines

Thanks!

Thomas

#3 lore raw

PATCH 1/1 tcl: rework logic for databases support

Hello,

Quoted 2 lines · Vicente Olivert Riera

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

Yes, explain this in a comment :)

Quoted 2 lines

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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH 1/1 tcl: rework logic for databases support

Hi Thomas,

Quoted 8 lines · Thomas Petazzoni

Hello,

Quoted 2 lines · Vicente Olivert Riera

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

Yes, explain this in a comment :)

Sure.

Quoted 4 lines
Quoted 2 lines

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.

Quoted 5 lines

Thanks!

Thomas

#5 lore raw

PATCH 1/1 tcl: rework logic for databases support

In reply to Vicente Olivert Riera
Quoted 6 lines · Vicente Olivert Riera

Hi Thomas,

Quoted 3 lines · Thomas Petazzoni

Hello,

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

[snip]

Quoted 8 lines
Quoted 8 lines

package/tcl/tcl.mk endef · line 80

Quoted 3 lines
  TCL_DEPENDENCIES = $(if $(BR2_PACKAGE_SQLITE),sqlite)
package/tcl/tcl.mk +2 -0
+TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_MYSQL),mysql)
+TCL_DEPENDENCIES += $(if $(BR2_PACKAGE_POSTGRESQL),postgresql)
TCL_DEPENDENCIES = \
	$(if ...) \
	$(if ...)
Quoted 4 lines
Quoted 1 line

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]

Signature · 6 lines
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
#6 lore raw

PATCH 1/1 tcl: rework logic for databases support

Hello,

Quoted 10 lines · Arnout Vandecappelle
Quoted 2 lines

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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#7 lore raw

PATCH 1/1 tcl: rework logic for databases support

Quoted 20 lines · Thomas Petazzoni

Hello,

Quoted 7 lines · Arnout Vandecappelle
Quoted 2 lines

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.

Quoted 3 lines

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:

Quoted 7 lines

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

Signature · 6 lines
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