Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2] wiringpi: new package
@ 2015-11-24 21:36 Peter Seiderer
  2015-11-30 21:10 ` Thomas Petazzoni
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Seiderer @ 2015-11-24 21:36 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
Changes v1 -> v2:
  - fix typo in commit message (wirinpi vs wiringpi)
---
 package/Config.in                                  |  1 +
 ...-Fix-devLib-gpio-include-and-library-path.patch | 40 ++++++++++++++++++++++
 package/wiringpi/Config.in                         |  6 ++++
 package/wiringpi/wiringpi.mk                       | 29 ++++++++++++++++
 4 files changed, 76 insertions(+)
 create mode 100644 package/wiringpi/0001-Fix-devLib-gpio-include-and-library-path.patch
 create mode 100644 package/wiringpi/Config.in
 create mode 100644 package/wiringpi/wiringpi.mk

diff --git a/package/Config.in b/package/Config.in
index bdc3063..9d273e7 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -438,6 +438,7 @@ endif
 	source "package/w_scan/Config.in"
 	source "package/wf111/Config.in"
 	source "package/wipe/Config.in"
+	source "package/wiringpi/Config.in"
 	source "package/xorriso/Config.in"
 endmenu
 
diff --git a/package/wiringpi/0001-Fix-devLib-gpio-include-and-library-path.patch b/package/wiringpi/0001-Fix-devLib-gpio-include-and-library-path.patch
new file mode 100644
index 0000000..2a7e754
--- /dev/null
+++ b/package/wiringpi/0001-Fix-devLib-gpio-include-and-library-path.patch
@@ -0,0 +1,40 @@
+From d0e4c2ac47776e60fac64143a58b4fbe23f433be Mon Sep 17 00:00:00 2001
+From: Peter Seiderer <ps.report@gmx.net>
+Date: Tue, 24 Nov 2015 22:26:13 +0100
+Subject: [PATCH] Fix devLib/gpio include and library path.
+
+Signed-off-by: Peter Seiderer <ps.report@gmx.net>
+---
+ devLib/Makefile | 2 +-
+ gpio/Makefile   | 2 +-
+ 2 files changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/devLib/Makefile b/devLib/Makefile
+index 0fb0033..f956abe 100644
+--- a/devLib/Makefile
++++ b/devLib/Makefile
+@@ -37,7 +37,7 @@ DYNAMIC=libwiringPiDev.so.$(VERSION)
+ #DEBUG	= -g -O0
+ DEBUG	= -O2
+ CC	= gcc
+-INCLUDE	= -I.
++INCLUDE	= -I$(DESTDIR)$(PREFIX)/wiringPi  -I$(DESTDIR)$(PREFIX)/devLib
+ DEFS	= -D_GNU_SOURCE
+ CFLAGS	= $(DEBUG) $(DEFS) -Wformat=2 -Wall -Winline $(INCLUDE) -pipe -fPIC
+ 
+diff --git a/gpio/Makefile b/gpio/Makefile
+index 7dcd090..f5b588a 100644
+--- a/gpio/Makefile
++++ b/gpio/Makefile
+@@ -33,7 +33,7 @@ endif
+ #DEBUG	= -g -O0
+ DEBUG	= -O2
+ CC	= gcc
+-INCLUDE	= -I$(DESTDIR)$(PREFIX)/include
++INCLUDE	= -I$(DESTDIR)$(PREFIX)/wiringPi  -I$(DESTDIR)$(PREFIX)/devLib
+ CFLAGS	= $(DEBUG) -Wall $(INCLUDE) -Winline -pipe
+ 
+ LDFLAGS	= -L$(DESTDIR)$(PREFIX)/lib
+-- 
+2.1.4
+
diff --git a/package/wiringpi/Config.in b/package/wiringpi/Config.in
new file mode 100644
index 0000000..9275b82
--- /dev/null
+++ b/package/wiringpi/Config.in
@@ -0,0 +1,6 @@
+config BR2_PACKAGE_WIRINGPI
+	bool "wiringpi"
+	help
+	  wiringPi libraries (and gpio command)
+
+	  http://wiringpi.com/
diff --git a/package/wiringpi/wiringpi.mk b/package/wiringpi/wiringpi.mk
new file mode 100644
index 0000000..258bb25
--- /dev/null
+++ b/package/wiringpi/wiringpi.mk
@@ -0,0 +1,29 @@
+WIRINGPI_VERSION = 2.29
+WIRINGPI_SITE = git://git.drogon.net/wiringPi
+WIRINGPI_INSTALL_STAGING = YES
+
+define WIRINGPI_BUILD_CMDS
+	$(MAKE) -C $(@D)/wiringPi CC=$(TARGET_CC)
+	$(INSTALL) -D -m 0755 $(@D)/wiringPi/libwiringPi.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPi.so.2.29
+	ln -sf $(STAGING_DIR)/usr/lib/libwiringPi.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPi.so
+	$(MAKE) -C $(@D)/devLib CC=$(TARGET_CC) PREFIX=$(@D) DESTDIR=
+	$(INSTALL) -D -m 0755 $(@D)/devLib/libwiringPiDev.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPiDev.so.2.29
+	ln -sf $(STAGING_DIR)/usr/lib/libwiringPiDev.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPiDev.so
+	$(MAKE) -C $(@D)/gpio CC=$(TARGET_CC) PREFIX=$(@D) DESTDIR=
+endef
+
+define WIRINGPI_INSTALL_STAGING_CMDS
+	$(INSTALL) -D -m 0644 $(@D)/wiringPi/*.h $(STAGING_DIR)/usr/include
+	$(INSTALL) -D -m 0644 $(@D)/devLib/*.h $(STAGING_DIR)/usr/include
+endef
+
+define WIRINGPI_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/wiringPi/libwiringPi.so* $(TARGET_DIR)/usr/lib
+	ln -sf libwiringPi.so.2.29  $(TARGET_DIR)/usr/lib/libwiringPi.so
+	$(INSTALL) -D -m 0755 $(@D)/devLib/libwiringPiDev.so* $(TARGET_DIR)/usr/lib
+	ln -sf libwiringPiDev.so.2.29 $(TARGET_DIR)/usr/lib/libwiringPiDev.so
+	$(INSTALL) -D -m 0755 $(@D)/gpio/gpio $(TARGET_DIR)/usr/bin
+	$(INSTALL) -D -m 0755 $(@D)/gpio/pintest $(TARGET_DIR)/usr/bin
+endef
+
+$(eval $(generic-package))
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [Buildroot] [PATCH v2] wiringpi: new package
  2015-11-24 21:36 [Buildroot] [PATCH v2] wiringpi: new package Peter Seiderer
@ 2015-11-30 21:10 ` Thomas Petazzoni
  2015-12-03 21:06   ` Peter Seiderer
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Petazzoni @ 2015-11-30 21:10 UTC (permalink / raw)
  To: buildroot

Dear Peter Seiderer,

On Tue, 24 Nov 2015 22:36:31 +0100, Peter Seiderer wrote:

> diff --git a/package/Config.in b/package/Config.in
> index bdc3063..9d273e7 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -438,6 +438,7 @@ endif
>  	source "package/w_scan/Config.in"
>  	source "package/wf111/Config.in"
>  	source "package/wipe/Config.in"
> +	source "package/wiringpi/Config.in"

Isn't wiringpi mainly a library ? Maybe it should go under Libraries ->
Hardware handling.

> +diff --git a/devLib/Makefile b/devLib/Makefile
> +index 0fb0033..f956abe 100644
> +--- a/devLib/Makefile
> ++++ b/devLib/Makefile
> +@@ -37,7 +37,7 @@ DYNAMIC=libwiringPiDev.so.$(VERSION)
> + #DEBUG	= -g -O0
> + DEBUG	= -O2
> + CC	= gcc
> +-INCLUDE	= -I.
> ++INCLUDE	= -I$(DESTDIR)$(PREFIX)/wiringPi  -I$(DESTDIR)$(PREFIX)/devLib
> + DEFS	= -D_GNU_SOURCE
> + CFLAGS	= $(DEBUG) $(DEFS) -Wformat=2 -Wall -Winline $(INCLUDE) -pipe -fPIC
> + 
> +diff --git a/gpio/Makefile b/gpio/Makefile
> +index 7dcd090..f5b588a 100644
> +--- a/gpio/Makefile
> ++++ b/gpio/Makefile
> +@@ -33,7 +33,7 @@ endif
> + #DEBUG	= -g -O0
> + DEBUG	= -O2
> + CC	= gcc
> +-INCLUDE	= -I$(DESTDIR)$(PREFIX)/include
> ++INCLUDE	= -I$(DESTDIR)$(PREFIX)/wiringPi  -I$(DESTDIR)$(PREFIX)/devLib

This patch is not correct I believe. It makes the assumption that
wiringPi is already installed in $(DESTDIR)$(PREFIX), which it is not
when you are building it.

> diff --git a/package/wiringpi/wiringpi.mk b/package/wiringpi/wiringpi.mk
> new file mode 100644
> index 0000000..258bb25
> --- /dev/null
> +++ b/package/wiringpi/wiringpi.mk
> @@ -0,0 +1,29 @@

Missing comment header.

> +WIRINGPI_VERSION = 2.29
> +WIRINGPI_SITE = git://git.drogon.net/wiringPi
> +WIRINGPI_INSTALL_STAGING = YES

Missing license + license file information.

> +
> +define WIRINGPI_BUILD_CMDS
> +	$(MAKE) -C $(@D)/wiringPi CC=$(TARGET_CC)

It would be a lot better to use $(TARGET_CONFIGURE_OPTS), and
$(TARGET_MAKE_ENV), i.e:

	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/wiringPi

the CC=$(TARGET_CC) is already part of TARGET_CONFIGURE_OPTS. Also you
will probably have to adjust the Makefile to turn CFLAGS = into CFLAGS
+=.

> +	$(INSTALL) -D -m 0755 $(@D)/wiringPi/libwiringPi.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPi.so.2.29
> +	ln -sf $(STAGING_DIR)/usr/lib/libwiringPi.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPi.so

Why are you doing installation within the <pkg>_BUILDS_CMDS ? This is not good ?

> +	$(MAKE) -C $(@D)/devLib CC=$(TARGET_CC) PREFIX=$(@D) DESTDIR=

Why is DESTDIR= needed here ?

> +	$(INSTALL) -D -m 0755 $(@D)/devLib/libwiringPiDev.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPiDev.so.2.29
> +	ln -sf $(STAGING_DIR)/usr/lib/libwiringPiDev.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPiDev.so

Ditto installation.

> +	$(MAKE) -C $(@D)/gpio CC=$(TARGET_CC) PREFIX=$(@D) DESTDIR=
> +endef
> +
> +define WIRINGPI_INSTALL_STAGING_CMDS
> +	$(INSTALL) -D -m 0644 $(@D)/wiringPi/*.h $(STAGING_DIR)/usr/include
> +	$(INSTALL) -D -m 0644 $(@D)/devLib/*.h $(STAGING_DIR)/usr/include

This is not good because $(INSTALL) -D expect a full destination path.
I guess the easiest is just:

	cp -dpfr $(@D)/wiringPi/*.h $(STAGING_DIR)/usr/include

> +define WIRINGPI_INSTALL_TARGET_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/wiringPi/libwiringPi.so* $(TARGET_DIR)/usr/lib
> +	ln -sf libwiringPi.so.2.29  $(TARGET_DIR)/usr/lib/libwiringPi.so
> +	$(INSTALL) -D -m 0755 $(@D)/devLib/libwiringPiDev.so* $(TARGET_DIR)/usr/lib
> +	ln -sf libwiringPiDev.so.2.29 $(TARGET_DIR)/usr/lib/libwiringPiDev.so
> +	$(INSTALL) -D -m 0755 $(@D)/gpio/gpio $(TARGET_DIR)/usr/bin
> +	$(INSTALL) -D -m 0755 $(@D)/gpio/pintest $(TARGET_DIR)/usr/bin

Same comments here: $(INSTALL) -D requires a full destination path.
Otherwise, if $(TARGET_DIR)/usr/bin doesn't exist as a directory, you
will have you "pintest" program installed as "bin" in $(TARGET_DIR)/usr.

Moreover, the upstream project has some "install" and "install-static"
targets. You should use them instead of doing manual installation.

Finally, if you're installing unconditionally shared libraries, then it
means that they are always being built. So your package should depend
on !BR2_STATIC_LIBS.

Can you fix those comments and send an updated version ?

Thanks!

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Buildroot] [PATCH v2] wiringpi: new package
  2015-11-30 21:10 ` Thomas Petazzoni
@ 2015-12-03 21:06   ` Peter Seiderer
  0 siblings, 0 replies; 3+ messages in thread
From: Peter Seiderer @ 2015-12-03 21:06 UTC (permalink / raw)
  To: buildroot

Hello Thomas,

On Mon, 30 Nov 2015 22:10:30 +0100, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Dear Peter Seiderer,
> 
> On Tue, 24 Nov 2015 22:36:31 +0100, Peter Seiderer wrote:
> 
> > diff --git a/package/Config.in b/package/Config.in
> > index bdc3063..9d273e7 100644
> > --- a/package/Config.in
> > +++ b/package/Config.in
> > @@ -438,6 +438,7 @@ endif
> >  	source "package/w_scan/Config.in"
> >  	source "package/wf111/Config.in"
> >  	source "package/wipe/Config.in"
> > +	source "package/wiringpi/Config.in"
> 
> Isn't wiringpi mainly a library ? Maybe it should go under Libraries ->
> Hardware handling.

O.k., will fix it...

> 
> > +diff --git a/devLib/Makefile b/devLib/Makefile
> > +index 0fb0033..f956abe 100644
> > +--- a/devLib/Makefile
> > ++++ b/devLib/Makefile
> > +@@ -37,7 +37,7 @@ DYNAMIC=libwiringPiDev.so.$(VERSION)
> > + #DEBUG	= -g -O0
> > + DEBUG	= -O2
> > + CC	= gcc
> > +-INCLUDE	= -I.
> > ++INCLUDE	= -I$(DESTDIR)$(PREFIX)/wiringPi  -I$(DESTDIR)$(PREFIX)/devLib
> > + DEFS	= -D_GNU_SOURCE
> > + CFLAGS	= $(DEBUG) $(DEFS) -Wformat=2 -Wall -Winline $(INCLUDE) -pipe -fPIC
> > + 
> > +diff --git a/gpio/Makefile b/gpio/Makefile
> > +index 7dcd090..f5b588a 100644
> > +--- a/gpio/Makefile
> > ++++ b/gpio/Makefile
> > +@@ -33,7 +33,7 @@ endif
> > + #DEBUG	= -g -O0
> > + DEBUG	= -O2
> > + CC	= gcc
> > +-INCLUDE	= -I$(DESTDIR)$(PREFIX)/include
> > ++INCLUDE	= -I$(DESTDIR)$(PREFIX)/wiringPi  -I$(DESTDIR)$(PREFIX)/devLib
> 
> This patch is not correct I believe. It makes the assumption that
> wiringPi is already installed in $(DESTDIR)$(PREFIX), which it is not
> when you are building it.

Intention was to set DESTDIR/PREFIX values to the build directory to avoid
the install step before linking gpio (as the original build script does)...

> 
> > diff --git a/package/wiringpi/wiringpi.mk b/package/wiringpi/wiringpi.mk
> > new file mode 100644
> > index 0000000..258bb25
> > --- /dev/null
> > +++ b/package/wiringpi/wiringpi.mk
> > @@ -0,0 +1,29 @@
> 
> Missing comment header.

O.k., will fix (should have marked this patch as early draft ;-) )
> 
> > +WIRINGPI_VERSION = 2.29
> > +WIRINGPI_SITE = git://git.drogon.net/wiringPi
> > +WIRINGPI_INSTALL_STAGING = YES
> 
> Missing license + license file information.

O.k., will fix...

> 
> > +
> > +define WIRINGPI_BUILD_CMDS
> > +	$(MAKE) -C $(@D)/wiringPi CC=$(TARGET_CC)
> 
> It would be a lot better to use $(TARGET_CONFIGURE_OPTS), and
> $(TARGET_MAKE_ENV), i.e:
> 
> 	$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/wiringPi
> 
> the CC=$(TARGET_CC) is already part of TARGET_CONFIGURE_OPTS. Also you

Did not work, because the original Makefile used 'CC=gcc' which is only
overridden by command line options, not by environment variables...

> will probably have to adjust the Makefile to turn CFLAGS = into CFLAGS
> +=.
> 
> > +	$(INSTALL) -D -m 0755 $(@D)/wiringPi/libwiringPi.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPi.so.2.29
> > +	ln -sf $(STAGING_DIR)/usr/lib/libwiringPi.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPi.so
> 
> Why are you doing installation within the <pkg>_BUILDS_CMDS ? This is not good ?

Because I tried to not patch the original build system which assumes:

 - build wiringPi, install wiringPi
 - build devLib, install devLib
 - build gpio, install gpio

But will complete rework my patch...

> 
> > +	$(MAKE) -C $(@D)/devLib CC=$(TARGET_CC) PREFIX=$(@D) DESTDIR=
> 
> Why is DESTDIR= needed here ?
> 
> > +	$(INSTALL) -D -m 0755 $(@D)/devLib/libwiringPiDev.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPiDev.so.2.29
> > +	ln -sf $(STAGING_DIR)/usr/lib/libwiringPiDev.so.2.29 $(STAGING_DIR)/usr/lib/libwiringPiDev.so
> 
> Ditto installation.

Will be changed in next version...

> 
> > +	$(MAKE) -C $(@D)/gpio CC=$(TARGET_CC) PREFIX=$(@D) DESTDIR=
> > +endef
> > +
> > +define WIRINGPI_INSTALL_STAGING_CMDS
> > +	$(INSTALL) -D -m 0644 $(@D)/wiringPi/*.h $(STAGING_DIR)/usr/include
> > +	$(INSTALL) -D -m 0644 $(@D)/devLib/*.h $(STAGING_DIR)/usr/include
> 
> This is not good because $(INSTALL) -D expect a full destination path.
> I guess the easiest is just:

O.k, will fix...

> 
> 	cp -dpfr $(@D)/wiringPi/*.h $(STAGING_DIR)/usr/include
> 
> > +define WIRINGPI_INSTALL_TARGET_CMDS
> > +	$(INSTALL) -D -m 0755 $(@D)/wiringPi/libwiringPi.so* $(TARGET_DIR)/usr/lib
> > +	ln -sf libwiringPi.so.2.29  $(TARGET_DIR)/usr/lib/libwiringPi.so
> > +	$(INSTALL) -D -m 0755 $(@D)/devLib/libwiringPiDev.so* $(TARGET_DIR)/usr/lib
> > +	ln -sf libwiringPiDev.so.2.29 $(TARGET_DIR)/usr/lib/libwiringPiDev.so
> > +	$(INSTALL) -D -m 0755 $(@D)/gpio/gpio $(TARGET_DIR)/usr/bin
> > +	$(INSTALL) -D -m 0755 $(@D)/gpio/pintest $(TARGET_DIR)/usr/bin
> 
> Same comments here: $(INSTALL) -D requires a full destination path.
> Otherwise, if $(TARGET_DIR)/usr/bin doesn't exist as a directory, you
> will have you "pintest" program installed as "bin" in $(TARGET_DIR)/usr.
> 
> Moreover, the upstream project has some "install" and "install-static"
> targets. You should use them instead of doing manual installation.
> 
O.k., will be in the next version...

> Finally, if you're installing unconditionally shared libraries, then it
> means that they are always being built. So your package should depend
> on !BR2_STATIC_LIBS.
> 
> Can you fix those comments and send an updated version ?

Yes, will do...

Thanks for review...

Regards,
Peter

> 
> Thanks!
> 
> Thomas

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-12-03 21:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-24 21:36 [Buildroot] [PATCH v2] wiringpi: new package Peter Seiderer
2015-11-30 21:10 ` Thomas Petazzoni
2015-12-03 21:06   ` Peter Seiderer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox