* [Buildroot] [PATCH 1/1] linuxconsoletools: new package
@ 2017-06-17 7:04 Koen Martens
2017-06-17 13:23 ` Thomas Petazzoni
0 siblings, 1 reply; 3+ messages in thread
From: Koen Martens @ 2017-06-17 7:04 UTC (permalink / raw)
To: buildroot
Linuxconsoletools (http://sf.net/projects/linuxconsole/)
contains the inputattach utility to attach legacy serial
devices to the Linux kernel input layer and joystick
utilities to calibrate and test joysticks and joypads.
The buildroot package adds options to build only certain
tools and makes installation of man pages optional.
Signed-off-by: Koen Martens <gmc@sonologic.nl>
---
DEVELOPERS | 3 +
package/Config.in | 1 +
.../linuxconsoletools/0001-conditional-build.patch | 107 +++++++++++++++++++++
package/linuxconsoletools/0002-sdl-config.patch | 20 ++++
package/linuxconsoletools/Config.in | 43 +++++++++
package/linuxconsoletools/linuxconsoletools.hash | 2 +
package/linuxconsoletools/linuxconsoletools.mk | 52 ++++++++++
7 files changed, 228 insertions(+)
create mode 100644 package/linuxconsoletools/0001-conditional-build.patch
create mode 100644 package/linuxconsoletools/0002-sdl-config.patch
create mode 100644 package/linuxconsoletools/Config.in
create mode 100644 package/linuxconsoletools/linuxconsoletools.hash
create mode 100644 package/linuxconsoletools/linuxconsoletools.mk
diff --git a/DEVELOPERS b/DEVELOPERS
index 6ade852..ae4c902 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -994,6 +994,9 @@ F: package/ramsmp/
N: Kevin Joly <kevin.joly@sensefly.com>
F: package/libgphoto2/
+N: Koen Martens <gmc@sonologic.nl>
+F: package/linuxconsoletools/
+
N: Laurent Cans <laurent.cans@gmail.com>
F: package/aircrack-ng/
diff --git a/package/Config.in b/package/Config.in
index c997e2a..cb0bbe6 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -422,6 +422,7 @@ endmenu
source "package/lcdproc/Config.in"
source "package/libuio/Config.in"
source "package/libump/Config.in"
+ source "package/linuxconsoletools/Config.in"
source "package/linux-backports/Config.in"
source "package/lirc-tools/Config.in"
source "package/lm-sensors/Config.in"
diff --git a/package/linuxconsoletools/0001-conditional-build.patch b/package/linuxconsoletools/0001-conditional-build.patch
new file mode 100644
index 0000000..7e98963
--- /dev/null
+++ b/package/linuxconsoletools/0001-conditional-build.patch
@@ -0,0 +1,107 @@
+Selectively build groups of tools (inputattach,
+joystick tools and/or force-feedback tools).
+
+Make installation of documentation (man pages)
+optional.
+
+Signed-off-by: Koen Martens <gmc@sonologic.nl>
+
+diff -Naur a/docs/Makefile b/docs/Makefile
+--- a/docs/Makefile 2016-04-19 23:45:26.000000000 +0200
++++ b/docs/Makefile 2017-06-13 09:54:38.046181000 +0200
+@@ -19,9 +19,26 @@
+ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ # 02110-1301 USA.
+
+-MANPAGES = inputattach.1 jstest.1 jscal.1 fftest.1 \
+- ffmvforce.1 ffset.1 ffcfstress.1 jscal-store.1 \
+- jscal-restore.1 evdev-joystick.1
++MANPAGES =
++
++ifdef ENABLE_INPUTATTACH
++MANPAGES += inputattach.1
++endif
++
++ifdef ENABLE_JOYSTICK
++MANPAGES += jstest.1
++MANPAGES += jscal.1
++MANPAGES += jscal-store.1
++MANPAGES += jscal-restore.1
++MANPAGES += evdev-joystick.1
++endif
++
++ifdef ENABLE_FORCEFEEDBACK
++MANPAGES += fftest.1
++MANPAGES += ffmvforce.1
++MANPAGES += ffset.1
++MANPAGES += ffcfstress.1
++endif
+
+ PREFIX ?= /usr/local
+
+diff -Naur a/Makefile b/Makefile
+--- a/Makefile 2016-10-08 12:42:02.000000000 +0200
++++ b/Makefile 2017-06-13 10:07:31.156543000 +0200
+@@ -30,7 +30,9 @@
+
+ install:
+ $(MAKE) -C utils $@
++ifdef ENABLE_DOCS
+ $(MAKE) -C docs $@
++endif
+
+ dist: clean
+ rm -rf $(PACKAGE)
+diff -Naur a/utils/Makefile b/utils/Makefile
+--- a/utils/Makefile 2016-04-19 23:28:36.000000000 +0200
++++ b/utils/Makefile 2017-06-13 10:02:44.037055000 +0200
+@@ -27,8 +27,26 @@
+
+ CFLAGS ?= -g -O2 -Wall
+
+-PROGRAMS = inputattach jstest jscal fftest ffmvforce ffset \
+- ffcfstress jscal-restore jscal-store evdev-joystick
++PROGRAMS =
++
++ifdef ENABLE_INPUTATTACH
++PROGRAMS += inputattach
++endif
++
++ifdef ENABLE_JOYSTICK
++PROGRAMS += jstest
++PROGRAMS += jscal
++PROGRAMS += jscal-restore
++PROGRAMS += jscal-store
++PROGRAMS += evdev-joystick
++endif
++
++ifdef ENABLE_FORCEFEEDBACK
++PROGRAMS += fftest
++PROGRAMS += ffmvforce
++PROGRAMS += ffset
++PROGRAMS += ffcfstress
++endif
+
+ PREFIX ?= /usr/local
+
+@@ -79,13 +97,20 @@
+ 80-stelladaptor-joystick.rules: 80-stelladaptor-joystick.rules.in
+ sed "s^@@PREFIX@@^$(PREFIX)^g" < $^ > $@
+
++INSTALL_DEP = compile
++ifdef ENABLE_JOYSTICK
++INSTALL_DEP += 80-stelladaptor-joystick.rules
++endif
++
+ install: compile 80-stelladaptor-joystick.rules
+ install -d $(DESTDIR)$(PREFIX)/bin
+ install $(PROGRAMS) $(DESTDIR)$(PREFIX)/bin
++ifdef ENABLE_JOYSTICK
+ install -d $(DESTDIR)$(PREFIX)/share/joystick
+ install extract filter ident $(DESTDIR)$(PREFIX)/share/joystick
+ install -d $(DESTDIR)/lib/udev/rules.d
+ install js-set-enum-leds $(DESTDIR)/lib/udev
+ install -m 644 80-stelladaptor-joystick.rules $(DESTDIR)/lib/udev/rules.d
++endif
+
+ .PHONY: compile clean distclean install
diff --git a/package/linuxconsoletools/0002-sdl-config.patch b/package/linuxconsoletools/0002-sdl-config.patch
new file mode 100644
index 0000000..c19c274
--- /dev/null
+++ b/package/linuxconsoletools/0002-sdl-config.patch
@@ -0,0 +1,20 @@
+Specify sdl-config to use instead of using host sdl-config.
+
+Signed-off-by: Koen Martens <gmc@sonologic.nl>
+
+diff -Naur a/utils/Makefile b/utils/Makefile
+--- a/utils/Makefile 2017-06-16 18:48:28.402824790 +0200
++++ b/utils/Makefile 2017-06-16 18:48:53.898696647 +0200
+@@ -69,10 +69,10 @@
+ $(CC) $(CFLAGS) $(CPPFLAGS) -funsigned-char $^ $(LDFLAGS) -lm -o $@
+
+ ffmvforce.o: ffmvforce.c
+- $(CC) $(CFLAGS) $(CPPFLAGS) -c $^ -o $@ `sdl-config --cflags`
++ $(CC) $(CFLAGS) $(CPPFLAGS) -c $^ -o $@ `${SDL_CONFIG} --cflags`
+
+ ffmvforce: ffmvforce.o
+- $(CC) $^ -o $@ $(LDFLAGS) -g -lm `sdl-config --libs`
++ $(CC) $^ -o $@ $(LDFLAGS) -g -lm `${SDL_CONFIG} --libs`
+
+ axbtnmap.o: axbtnmap.c axbtnmap.h
+
diff --git a/package/linuxconsoletools/Config.in b/package/linuxconsoletools/Config.in
new file mode 100644
index 0000000..557edae
--- /dev/null
+++ b/package/linuxconsoletools/Config.in
@@ -0,0 +1,43 @@
+config BR2_PACKAGE_LINUXCONSOLETOOLS
+ bool "linuxconsoletools"
+ default n
+ help
+ Linuxconsoletools (http://sf.net/projects/linuxconsole/)
+ contains the inputattach utility to attach legacy serial
+ devices to the Linux kernel input layer and joystick
+ utilities to calibrate and test joysticks and joypads.
+
+if BR2_PACKAGE_LINUXCONSOLETOOLS
+
+config BR2_PACKAGE_LINUXCONSOLETOOLS_INPUTATTACH
+ bool "build inputattach"
+ default y
+ help
+ The inputattach utility attaches legacy serial devices
+ to the Linux kernel input layer.
+
+config BR2_PACKAGE_LINUXCONSOLETOOLS_JOYSTICK
+ bool "build joystick utilities"
+ default n
+ help
+ Build joystick utilities (jstest, jscal, jscal-store,
+ jscal-restore, evdev-joystick).
+
+config BR2_PACKAGE_LINUXCONSOLETOOLS_FORCEFEEDBACK
+ bool "build force-feedback utilities"
+ default n
+ depends on BR2_PACKAGE_SDL
+ help
+ Build force-feedback driver utilities (fftest,
+ ffmvforce, ffset, ffcfstress).
+
+comment "build force-feedback utilities depends on SDL"
+ depends on !BR2_PACKAGE_SDL
+
+config BR2_PACKAGE_LINUXCONSOLETOOLS_DOCS
+ bool "install man pages"
+ default n
+ help
+ Install documentation (manpages).
+
+endif
diff --git a/package/linuxconsoletools/linuxconsoletools.hash b/package/linuxconsoletools/linuxconsoletools.hash
new file mode 100644
index 0000000..756c9f4
--- /dev/null
+++ b/package/linuxconsoletools/linuxconsoletools.hash
@@ -0,0 +1,2 @@
+# Locally calculated
+sha256 ced2efed00b67b45f82eddc69be07385835d558f658016315ac621fe2eaa8146 linuxconsoletools-1.6.0.tar.bz2
diff --git a/package/linuxconsoletools/linuxconsoletools.mk b/package/linuxconsoletools/linuxconsoletools.mk
new file mode 100644
index 0000000..25271ad
--- /dev/null
+++ b/package/linuxconsoletools/linuxconsoletools.mk
@@ -0,0 +1,52 @@
+################################################################################
+#
+# linuxconsoletools
+#
+################################################################################
+
+LINUXCONSOLETOOLS_VERSION = 1.6.0
+LINUXCONSOLETOOLS_SOURCE = linuxconsoletools-${LINUXCONSOLETOOLS_VERSION}.tar.bz2
+LINUXCONSOLETOOLS_SITE = https://downloads.sourceforge.net/project/linuxconsole
+LINUXCONSOLETOOLS_LICENSE = GPLv2+
+LINUXCONSOLETOOLS_LICENSE_FILES = COPYING
+LINUXCONSOLETOOLS_DEPENDENCIES =
+LINUXCONSOLETOOLS_ENV =
+
+SDL_CONFIG =
+
+ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_INPUTATTACH),y)
+ LINUXCONSOLETOOLS_ENV += ENABLE_INPUTATTACH=1
+endif
+ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_JOYSTICK),y)
+ LINUXCONSOLETOOLS_ENV += ENABLE_JOYSTICK=1
+endif
+ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_FORCEFEEDBACK),y)
+ LINUXCONSOLETOOLS_ENV += ENABLE_FORCEFEEDBACK=1
+ LINUXCONSOLETOOLS_DEPENDENCIES += sdl
+ SDL_CONFIG = $(STAGING_DIR)/usr/bin/sdl-config
+endif
+ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_DOCS),y)
+ LINUXCONSOLETOOLS_ENV += ENABLE_DOCS=1
+endif
+
+define LINUXCONSOLETOOLS_BUILD_CMDS
+ $(LINUXCONSOLETOOLS_ENV) \
+ $(TARGET_MAKE_ENV) \
+ CC="$(TARGET_CC)" \
+ CFLAGS="$(TARGET_CFLAGS)" \
+ SDL_CONFIG="$(SDL_CONFIG)" \
+ $(MAKE) -C $(@D)
+endef
+
+define LINUXCONSOLETOOLS_INSTALL_TARGET_CMDS
+ $(LINUXCONSOLETOOLS_ENV) \
+ $(TARGET_MAKE_ENV) \
+ CC="$(TARGET_CC)" \
+ CFLAGS="$(TARGET_CFLAGS)" \
+ SDL_CONFIG="$(SDL_CONFIG)" \
+ DESTDIR="$(TARGET_DIR)" \
+ PREFIX=/usr \
+ make -C $(@D) install
+endef
+
+$(eval $(generic-package))
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* [Buildroot] [PATCH 1/1] linuxconsoletools: new package
2017-06-17 7:04 [Buildroot] [PATCH 1/1] linuxconsoletools: new package Koen Martens
@ 2017-06-17 13:23 ` Thomas Petazzoni
2017-06-17 14:20 ` Koen Martens
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Petazzoni @ 2017-06-17 13:23 UTC (permalink / raw)
To: buildroot
Hello,
On Sat, 17 Jun 2017 09:04:52 +0200, Koen Martens wrote:
> Linuxconsoletools (http://sf.net/projects/linuxconsole/)
> contains the inputattach utility to attach legacy serial
> devices to the Linux kernel input layer and joystick
> utilities to calibrate and test joysticks and joypads.
>
> The buildroot package adds options to build only certain
> tools and makes installation of man pages optional.
>
> Signed-off-by: Koen Martens <gmc@sonologic.nl>
Thanks for this first contribution! See below some comments about your
patch.
> diff --git a/package/linuxconsoletools/0001-conditional-build.patch b/package/linuxconsoletools/0001-conditional-build.patch
> new file mode 100644
> index 0000000..7e98963
> --- /dev/null
> +++ b/package/linuxconsoletools/0001-conditional-build.patch
> @@ -0,0 +1,107 @@
> +Selectively build groups of tools (inputattach,
> +joystick tools and/or force-feedback tools).
> +
> +Make installation of documentation (man pages)
> +optional.
> +
> +Signed-off-by: Koen Martens <gmc@sonologic.nl>
Do we really need to make all of this configurable? What is the
installed size of the different tools?
In any case, making the documentation installation optional is useless:
Buildroot unconditionally removes the documentation at the end of the
build. So just let the package Makefile install the documentation, it
will be removed anyway.
> diff --git a/package/linuxconsoletools/Config.in b/package/linuxconsoletools/Config.in
> new file mode 100644
> index 0000000..557edae
> --- /dev/null
> +++ b/package/linuxconsoletools/Config.in
> @@ -0,0 +1,43 @@
> +config BR2_PACKAGE_LINUXCONSOLETOOLS
> + bool "linuxconsoletools"
> + default n
Not needed, this is the default.
> + help
> + Linuxconsoletools (http://sf.net/projects/linuxconsole/)
> + contains the inputattach utility to attach legacy serial
> + devices to the Linux kernel input layer and joystick
> + utilities to calibrate and test joysticks and joypads.
Please add a blank line here, and then the upstream URL of the project
(rather than between parenthesis as you did).
> +if BR2_PACKAGE_LINUXCONSOLETOOLS
> +
> +config BR2_PACKAGE_LINUXCONSOLETOOLS_INPUTATTACH
> + bool "build inputattach"
Again I'm not sure we need sub-options. But if you keep them remove the
"build", i.e just use "inputattach" as the prompt.
> + default y
> + help
> + The inputattach utility attaches legacy serial devices
> + to the Linux kernel input layer.
> +
> +config BR2_PACKAGE_LINUXCONSOLETOOLS_JOYSTICK
> + bool "build joystick utilities"
Ditto.
> + default n
Not needed, please remove.
> + help
> + Build joystick utilities (jstest, jscal, jscal-store,
> + jscal-restore, evdev-joystick).
> +
> +config BR2_PACKAGE_LINUXCONSOLETOOLS_FORCEFEEDBACK
> + bool "build force-feedback utilities"
Remove "build".
> + default n
Ditto.
> + depends on BR2_PACKAGE_SDL
If you keep this option, please use a "select" instead.
> + help
> + Build force-feedback driver utilities (fftest,
> + ffmvforce, ffset, ffcfstress).
> +
> +comment "build force-feedback utilities depends on SDL"
> + depends on !BR2_PACKAGE_SDL
... and drop this comment.
> +
> +config BR2_PACKAGE_LINUXCONSOLETOOLS_DOCS
> + bool "install man pages"
Remove this option, we never install documentation in the target
filesystem in Buildroot.
> +LINUXCONSOLETOOLS_VERSION = 1.6.0
> +LINUXCONSOLETOOLS_SOURCE = linuxconsoletools-${LINUXCONSOLETOOLS_VERSION}.tar.bz2
Please use $(...) to reference variables, not ${...}.
> +LINUXCONSOLETOOLS_SITE = https://downloads.sourceforge.net/project/linuxconsole
> +LINUXCONSOLETOOLS_LICENSE = GPLv2+
We use SPDX license codes now, so please use GPL-2.0+.
> +LINUXCONSOLETOOLS_LICENSE_FILES = COPYING
> +LINUXCONSOLETOOLS_DEPENDENCIES =
> +LINUXCONSOLETOOLS_ENV =
Remove those two lines, they are not needed.
> +SDL_CONFIG =
Remove this line, it's not needed, and potentially harmful.
> +
> +ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_INPUTATTACH),y)
> + LINUXCONSOLETOOLS_ENV += ENABLE_INPUTATTACH=1
Please avoid trailing whitespaces. And also, don't indent such lines.
Only one space after += sign.
> +endif
One blank line here please.
Please fix globally.
> +ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_JOYSTICK),y)
> + LINUXCONSOLETOOLS_ENV += ENABLE_JOYSTICK=1
> +endif
> +ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_FORCEFEEDBACK),y)
> + LINUXCONSOLETOOLS_ENV += ENABLE_FORCEFEEDBACK=1
> + LINUXCONSOLETOOLS_DEPENDENCIES += sdl
> + SDL_CONFIG = $(STAGING_DIR)/usr/bin/sdl-config
Very wrong. The namespace of variables in Buildroot is global, so when
you set a variable named SDL_<something> you are messing with the SDL
package. What you want here is create a LINUXCONSOLETOOLS_MAKE_OPTS
variable, that will be passed to the build commands, and here you do:
LINUXCONSOLETOOLS_MAKE_OPTS += SDL_CONFIG=$(STAGING_DIR)/usr/bin/sdl-config
> +endif
> +ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_DOCS),y)
> + LINUXCONSOLETOOLS_ENV += ENABLE_DOCS=1
> +endif
Not needed, we don't want documentation.
> +
> +define LINUXCONSOLETOOLS_BUILD_CMDS
> + $(LINUXCONSOLETOOLS_ENV) \
> + $(TARGET_MAKE_ENV) \
> + CC="$(TARGET_CC)" \
> + CFLAGS="$(TARGET_CFLAGS)" \
> + SDL_CONFIG="$(SDL_CONFIG)" \
> + $(MAKE) -C $(@D)
This should be:
$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) \
$(LINUXCONSOLETOOLS_MAKE_OPTS)
> +endef
> +
> +define LINUXCONSOLETOOLS_INSTALL_TARGET_CMDS
> + $(LINUXCONSOLETOOLS_ENV) \
> + $(TARGET_MAKE_ENV) \
> + CC="$(TARGET_CC)" \
> + CFLAGS="$(TARGET_CFLAGS)" \
> + SDL_CONFIG="$(SDL_CONFIG)" \
> + DESTDIR="$(TARGET_DIR)" \
> + PREFIX=/usr \
> + make -C $(@D) install
and this should be:
$(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) \
$(LINUXCONSOLETOOLS_MAKE_OPTS) \
DESTDIR="$(TARGET_DIR)" \
PREFIX=/usr \
install
Could you fix the above issues, and send an updated version?
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 3+ messages in thread* [Buildroot] [PATCH 1/1] linuxconsoletools: new package
2017-06-17 13:23 ` Thomas Petazzoni
@ 2017-06-17 14:20 ` Koen Martens
0 siblings, 0 replies; 3+ messages in thread
From: Koen Martens @ 2017-06-17 14:20 UTC (permalink / raw)
To: buildroot
Hi,
Thanks for your comments. I will try and find some time to address them. One reason i made the options to select groups of utils is that personally i am only interested in inputattach and not in any of the joystick stuff. The inputattach utility is quite distinct and used for older touchscreen devices. In that use case, the joystick stuff is not needed.
But worse, i don't want a dependency on libsdl just because i need inputattach. How should the package deal with that?
Thanks,
Koen
On 17 June 2017 15:23:53 CEST, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
>Hello,
>
>On Sat, 17 Jun 2017 09:04:52 +0200, Koen Martens wrote:
>> Linuxconsoletools (http://sf.net/projects/linuxconsole/)
>> contains the inputattach utility to attach legacy serial
>> devices to the Linux kernel input layer and joystick
>> utilities to calibrate and test joysticks and joypads.
>>
>> The buildroot package adds options to build only certain
>> tools and makes installation of man pages optional.
>>
>> Signed-off-by: Koen Martens <gmc@sonologic.nl>
>
>Thanks for this first contribution! See below some comments about your
>patch.
>
>> diff --git a/package/linuxconsoletools/0001-conditional-build.patch
>b/package/linuxconsoletools/0001-conditional-build.patch
>> new file mode 100644
>> index 0000000..7e98963
>> --- /dev/null
>> +++ b/package/linuxconsoletools/0001-conditional-build.patch
>> @@ -0,0 +1,107 @@
>> +Selectively build groups of tools (inputattach,
>> +joystick tools and/or force-feedback tools).
>> +
>> +Make installation of documentation (man pages)
>> +optional.
>> +
>> +Signed-off-by: Koen Martens <gmc@sonologic.nl>
>
>Do we really need to make all of this configurable? What is the
>installed size of the different tools?
>
>In any case, making the documentation installation optional is useless:
>Buildroot unconditionally removes the documentation at the end of the
>build. So just let the package Makefile install the documentation, it
>will be removed anyway.
>
>> diff --git a/package/linuxconsoletools/Config.in
>b/package/linuxconsoletools/Config.in
>> new file mode 100644
>> index 0000000..557edae
>> --- /dev/null
>> +++ b/package/linuxconsoletools/Config.in
>> @@ -0,0 +1,43 @@
>> +config BR2_PACKAGE_LINUXCONSOLETOOLS
>> + bool "linuxconsoletools"
>> + default n
>
>Not needed, this is the default.
>
>> + help
>> + Linuxconsoletools (http://sf.net/projects/linuxconsole/)
>> + contains the inputattach utility to attach legacy serial
>> + devices to the Linux kernel input layer and joystick
>> + utilities to calibrate and test joysticks and joypads.
>
>Please add a blank line here, and then the upstream URL of the project
>(rather than between parenthesis as you did).
>
>> +if BR2_PACKAGE_LINUXCONSOLETOOLS
>> +
>> +config BR2_PACKAGE_LINUXCONSOLETOOLS_INPUTATTACH
>> + bool "build inputattach"
>
>Again I'm not sure we need sub-options. But if you keep them remove the
>"build", i.e just use "inputattach" as the prompt.
>
>> + default y
>> + help
>> + The inputattach utility attaches legacy serial devices
>> + to the Linux kernel input layer.
>> +
>> +config BR2_PACKAGE_LINUXCONSOLETOOLS_JOYSTICK
>> + bool "build joystick utilities"
>
>Ditto.
>
>> + default n
>
>Not needed, please remove.
>
>> + help
>> + Build joystick utilities (jstest, jscal, jscal-store,
>> + jscal-restore, evdev-joystick).
>> +
>> +config BR2_PACKAGE_LINUXCONSOLETOOLS_FORCEFEEDBACK
>> + bool "build force-feedback utilities"
>
>Remove "build".
>
>> + default n
>
>Ditto.
>
>> + depends on BR2_PACKAGE_SDL
>
>If you keep this option, please use a "select" instead.
>
>> + help
>> + Build force-feedback driver utilities (fftest,
>> + ffmvforce, ffset, ffcfstress).
>> +
>> +comment "build force-feedback utilities depends on SDL"
>> + depends on !BR2_PACKAGE_SDL
>
>... and drop this comment.
>
>> +
>> +config BR2_PACKAGE_LINUXCONSOLETOOLS_DOCS
>> + bool "install man pages"
>
>Remove this option, we never install documentation in the target
>filesystem in Buildroot.
>
>> +LINUXCONSOLETOOLS_VERSION = 1.6.0
>> +LINUXCONSOLETOOLS_SOURCE =
>linuxconsoletools-${LINUXCONSOLETOOLS_VERSION}.tar.bz2
>
>Please use $(...) to reference variables, not ${...}.
>
>> +LINUXCONSOLETOOLS_SITE =
>https://downloads.sourceforge.net/project/linuxconsole
>> +LINUXCONSOLETOOLS_LICENSE = GPLv2+
>
>We use SPDX license codes now, so please use GPL-2.0+.
>
>> +LINUXCONSOLETOOLS_LICENSE_FILES = COPYING
>> +LINUXCONSOLETOOLS_DEPENDENCIES =
>> +LINUXCONSOLETOOLS_ENV =
>
>Remove those two lines, they are not needed.
>
>> +SDL_CONFIG =
>
>Remove this line, it's not needed, and potentially harmful.
>
>> +
>> +ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_INPUTATTACH),y)
>> + LINUXCONSOLETOOLS_ENV += ENABLE_INPUTATTACH=1
>
>Please avoid trailing whitespaces. And also, don't indent such lines.
>Only one space after += sign.
>
>> +endif
>
>One blank line here please.
>
>Please fix globally.
>
>
>> +ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_JOYSTICK),y)
>> + LINUXCONSOLETOOLS_ENV += ENABLE_JOYSTICK=1
>> +endif
>> +ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_FORCEFEEDBACK),y)
>> + LINUXCONSOLETOOLS_ENV += ENABLE_FORCEFEEDBACK=1
>> + LINUXCONSOLETOOLS_DEPENDENCIES += sdl
>> + SDL_CONFIG = $(STAGING_DIR)/usr/bin/sdl-config
>
>Very wrong. The namespace of variables in Buildroot is global, so when
>you set a variable named SDL_<something> you are messing with the SDL
>package. What you want here is create a LINUXCONSOLETOOLS_MAKE_OPTS
>variable, that will be passed to the build commands, and here you do:
>
>LINUXCONSOLETOOLS_MAKE_OPTS +=
>SDL_CONFIG=$(STAGING_DIR)/usr/bin/sdl-config
>
>> +endif
>> +ifeq ($(BR2_PACKAGE_LINUXCONSOLETOOLS_DOCS),y)
>> + LINUXCONSOLETOOLS_ENV += ENABLE_DOCS=1
>> +endif
>
>Not needed, we don't want documentation.
>
>> +
>> +define LINUXCONSOLETOOLS_BUILD_CMDS
>> + $(LINUXCONSOLETOOLS_ENV) \
>> + $(TARGET_MAKE_ENV) \
>> + CC="$(TARGET_CC)" \
>> + CFLAGS="$(TARGET_CFLAGS)" \
>> + SDL_CONFIG="$(SDL_CONFIG)" \
>> + $(MAKE) -C $(@D)
>
>This should be:
>
> $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) \
> $(LINUXCONSOLETOOLS_MAKE_OPTS)
>
>> +endef
>> +
>> +define LINUXCONSOLETOOLS_INSTALL_TARGET_CMDS
>> + $(LINUXCONSOLETOOLS_ENV) \
>> + $(TARGET_MAKE_ENV) \
>> + CC="$(TARGET_CC)" \
>> + CFLAGS="$(TARGET_CFLAGS)" \
>> + SDL_CONFIG="$(SDL_CONFIG)" \
>> + DESTDIR="$(TARGET_DIR)" \
>> + PREFIX=/usr \
>> + make -C $(@D) install
>
>and this should be:
>
> $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D) \
> $(LINUXCONSOLETOOLS_MAKE_OPTS) \
> DESTDIR="$(TARGET_DIR)" \
> PREFIX=/usr \
> install
>
>Could you fix the above issues, and send an updated version?
>
>Best regards,
>
>Thomas
>--
>Thomas Petazzoni, CTO, Free Electrons
>Embedded Linux and Kernel engineering
>http://free-electrons.com
>_______________________________________________
>buildroot mailing list
>buildroot at busybox.net
>http://lists.busybox.net/mailman/listinfo/buildroot
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170617/fdc0a8dd/attachment.html>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-17 14:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-17 7:04 [Buildroot] [PATCH 1/1] linuxconsoletools: new package Koen Martens
2017-06-17 13:23 ` Thomas Petazzoni
2017-06-17 14:20 ` Koen Martens
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.