libFTDI major version update 1. This version can coexists beside the 0.x version (libftdi.so, libftdi1.so).
PATCH 1/1 libFTDI major version update 1. This version can coexists beside the 0.x version (libftdi.so, libftdi1.so).
Hi Thomas
Well im new to buildroot and git and also to linux. I am at the buttom of the learning curve related to this topics. Thank you for your time and feedback.
---- OriginalMessage ----
Quoted 22 lines
From: "Thomas Petazzoni" <[email protected]>
To: "Daniel Sangue" <[email protected]>
CC: buildroot at buildroot.org
Sent: Wed, Oct 01, 2014, 04:26 PM
Subject: Re: [Buildroot] [PATCH 1/1] libFTDI major version update 1. This version can coexists beside the 0.x version (libftdi.so, libftdi1.so).Dear Daniel Sangue,
Thanks for getting back with a proper patch for libftdi1. Definitely looking much better!
One first nit: the way your commit log is formatted. See
http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html.
Your commit log should look like:"""
libftdi1: new packageThis commit adds libftdi1 as a new package. This version should (and can) co-exist besides the 0.x version already packaged as libftdi, because they use different library names and a different API.
Quoted 8 lines
Signed-off-by: ...
"""The most important thing is that the first line should be short, and be an overall summary of what the patch is doing. Then one blank line, and one or several paragraphs of additional details, then one blank line, and the Signed-off-by line.
Ok, i hope i got it. Give it a try next time. Yes this 50/72 principle makes sense.
Quoted 22 lines · Daniel Sangue
Quoted 21 lines
package/libftdi1/Config.in new file +15
-0@@ -0,0 +1,24 @@ +config BR2_PACKAGE_LIBFTDI1 + bool "libftdi1" + depends on BR2_TOOLCHAIN_HAS_THREADS # libusb + select BR2_PACKAGE_LIBUSB + select BR2_PACKAGE_LIBUSB_COMPAT + select BR2_PACKAGE_LIBCONFUSE + help + Userspace access to FTDI USB interface chips (version 1.x) + + http://www.intra2net.com/en/developer/libftdi/index.php + +if BR2_PACKAGE_LIBFTDI1 + +config BR2_PACKAGE_LIBTFDI1_CPP + depends on BR2_INSTALL_LIBSTDCPP
Quoted 9 lines
package/libftdi1/Config.in line 18
Quoted 3 lines
package/libftdi1/Config.in new file +3
-0+ bool "C++ bindings" + help + C++ bindings for libftdiAccording to your libftdi1.mk file comments, it looks like C++ support can be autodetected. So maybe it's not really worth the effort having an option for that: we could just enable C++ support when available, and that's it.
ok, i removed the additional c++ binding option. I admit that i just copy pasted it from the existing libftdi(0) package.
Quoted 15 lines
Quoted 15 lines
package/libftdi1/Config.in new file +6
-0+ +endif # BR2_PACKAGE_LIBFTDI1 + +comment "libftdi1 needs a toolchain w/ threads" + depends on !BR2_TOOLCHAIN_HAS_THREADS +package/libftdi1/libftdi1.mk new file +3
-0@@ -0,0 +1,22 @@ +################################################################################ +# +# libftdi1
Quoted 8 lines
package/libftdi1/libftdi1.mk line 9
Quoted 6 lines
package/libftdi1/libftdi1.mk new file +6
-0+# +################################################################################ + +LIBFTDI1_VERSION = 1.1 +LIBFTDI1_SOURCE = libftdi1-$(LIBFTDI1_VERSION).tar.bz2 +LIBFTDI1_SITE = http://www.intra2net.com/en/developer/libftdi/download/Please add
LIBFTDI1_LICENSEandLIBFTDI1_LICENSE_FILES.
ok, i have added:IBFTDI1_LICENSE = LGPL2.1 GPL2LIBFTDI1_LICENSE_FILES = COPYING
Quoted 15 lines
package/libftdi1/libftdi1.mk line 15
Quoted 7 lines
package/libftdi1/libftdi1.mk new file +6
-0+LIBFTDI1_INSTALL_STAGING = YES +LIBFDTI1_CONF_OPT = --without-examples +LIBFTDI1_DEPENDENCIES = libusb-compat libusb libconfuse + +# configure detect it automaticaly so we need to force it +ifeq ($(BR2_PACKAGE_LIBTFDI1_CPP),y)If you remove the option for C++ support, you could use:
ifeq ($(BR2_INSTALL_LIBSTDCPP),y)
here. It will make sure C++ bindings are automatically built when the toolchain has C++ support.
Quoted 10 lines
package/libftdi1/libftdi1.mk line 21
Quoted 6 lines
package/libftdi1/libftdi1.mk new file +6
-0+LIBFDTI_CONF_OPT += --enable-libftdipp +else +LIBFDTI_CONF_OPT += --disable-libftdipp +endif + +$(eval $(cmake-package))Are you sure --{enable,disable}-libftdipp options are working for CMake? It doesn't look like the usual way of passing CMake configuration options, but maybe I'm missing something here.
No, i am not sure. I just can refere to the buildroot cmake example in the manual where LIBFOO_CONF_OPT is used:
http://buildroot.uclibc.org/downloads/manual/manual.html#_infrastructure_for_cmake_based_packages
yes, ifeq ($(BR2_INSTALL_LIBSTDCPP),y) is good..
Quoted 9 lines
Thanks,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Give me some time because i havent a working smtp-server connection at work. And i can not do an incremental patch - because i have deleted the copy of the repo where the patch was based on. I only can create a new patch and take your proposed git message for it. Is this ok for you or what should i do now?
Greets Dani
PATCH 1/1 libFTDI major version update 1. This version can coexists beside the 0.x version (libftdi.so, libftdi1.so).
Quoted 6 lines · On Thu, 02 Oct 2014 09:52:10 +0200 daniel.sangue at sangue.ch wrote:
Give me some time because i havent a working smtp-server connection at work. And i can not do an incremental patch - because i have deleted the copy of the repo where the patch was based on. I only can create a new patch and take your proposed git message for it. Is this ok for you or what should i do now?
As a long time M.L. reader, (not a reviewer) I feel safe in answering this one. ;)
The comments made where scattered throughout your submission, so I don't think that your having to re-generate a new one will make very much difference.
That is not what you would want to do to fix a single line "Oops" of course, but in this case, it should be ok.
Just be sure to include the [PATCH V2 ... etc in the new title and a comment about V1 -> V2 changes made.
Mike
Quoted 2 lines
Greets Dani
PATCH 1/1 libFTDI major version update 1. This version can coexists beside the 0.x version (libftdi.so, libftdi1.so).
Quoted 4 lines · On Thu, 2 Oct 2014 07:45:33 -0500 Mike Zick <[email protected]> wrote:
The comments made where scattered throughout your submission, so I don't think that your having to re-generate a new one will make very much difference.
**will not** make ...
Not a good morning here.
Mike
PATCH 1/1 libFTDI major version update 1. This version can coexists beside the 0.x version (libftdi.so, libftdi1.so).
In reply to daniel.sangue at sangue.chHello,
Quoted 3 lines · daniel.sangue at sangue.ch
Well im new to buildroot and git and also to linux. I am at the buttom of the learning curve related to this topics. Thank you for your time and feedback.
No problem, you're welcome!
Quoted 7 lines
Quoted 4 lines
According to your libftdi1.mk file comments, it looks like C++ support can be autodetected. So maybe it's not really worth the effort having an option for that: we could just enable C++ support when available, and that's it.
ok, i removed the additional c++ binding option. I admit that i just copy pasted it from the existing libftdi(0) package.
Ah, ok.
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
(quoted diff) +3
-0+LIBFTDI1_VERSION = 1.1 +LIBFTDI1_SOURCE = libftdi1-$(LIBFTDI1_VERSION).tar.bz2 +LIBFTDI1_SITE =Please add
LIBFTDI1_LICENSEandLIBFTDI1_LICENSE_FILES.ok, i have added:
IBFTDI1_LICENSE= LGPL2.1 GPL2LIBFTDI1_LICENSE_FILES= COPYING
That isn't good, the licensing is a bit more complicated. I think it should be:
LIBFTDI1_LICENSE = LGPLv2.1 (libfti1), GPLv2 with exceptions (ftdipp1) LIBFTDI1_LICENSE_FILES = LICENSE COPYING.GPL COPYING.LIB
Quoted 7 lines
Quoted 3 lines
Are you sure --{enable,disable}-libftdipp options are working for CMake? It doesn't look like the usual way of passing CMake configuration options, but maybe I'm missing something here.
No, i am not sure. I just can refere to the buildroot cmake example in the manual where
LIBFOO_CONF_OPTis used:
http://buildroot.uclibc.org/downloads/manual/manual.html#_infrastructure_for_cmake_based_packages
Yes, but it seems like the code you wrote using --enable-libftdipp and --disable-libftdipp was copied from the other libftdi.mk package. Which is autotools based, so --enable/--disable options make sense. But for a CMake package, they clearly do not make sense.
I think, it should instead be:
ifeq ($(BR2_INSTALL_LIBSTDCPP),y)LIBFTDI1_CONF_OPT = -DFTDIPP=ON
elseLIBFTDI1_CONF_OPT = -DFTDIPP=OFF
endif
You should test this with a non-C++ capable toolchain. For example
http://autobuild.buildroot.org/toolchains/configs/br-arm-basic.config
is a configuration with a minimal toolchain, having just thread support.
Thanks,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com