* [Buildroot] Add package statserial
@ 2011-06-06 3:03 Francis Mendes
2011-06-06 19:21 ` Thomas Petazzoni
0 siblings, 1 reply; 6+ messages in thread
From: Francis Mendes @ 2011-06-06 3:03 UTC (permalink / raw)
To: buildroot
Hi,
I needed this package and added it to Buildroot.
Can anyone review the attached patch for any mistakes?
Thanks,
Francis
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20110606/ffa8edeb/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-package-statserial.patch
Type: text/x-patch
Size: 3438 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20110606/ffa8edeb/attachment.bin>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] Add package statserial
2011-06-06 3:03 [Buildroot] Add package statserial Francis Mendes
@ 2011-06-06 19:21 ` Thomas Petazzoni
2011-06-07 1:54 ` Francis Mendes
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2011-06-06 19:21 UTC (permalink / raw)
To: buildroot
Hello Francis,
Le Mon, 6 Jun 2011 00:03:36 -0300,
Francis Mendes <francis.mendes@gmail.com> a ?crit :
> I needed this package and added it to Buildroot.
>
> Can anyone review the attached patch for any mistakes?
Thanks for this submission!
It would be nicer to attach the patch inline, as it is easier to make
comments through the patch.
Two comments :
* You should "select" BR2_PACKAGE_NCURSES instead of "depends on".
That's the dependency type we use for the dependency of programs on
libraries.
* Your statserial-1.1-fixmakefile.patch should have a header with a
description saying why it is needed, and with a Signed-off-by line.
From a quick read, I don't see why the patch is needed.
* Your installation line should be
$(INSTALL) -D -m 0755 $(@D)/statserial $(TARGET_DIR)/usr/bin/statserial
otherwise your statserial binary will be named "bin" in
$(TARGET_DIR)/usr if the usr/bin directory does not exist.
Otherwise, looks good.
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] Add package statserial
2011-06-06 19:21 ` Thomas Petazzoni
@ 2011-06-07 1:54 ` Francis Mendes
2011-06-07 10:33 ` Peter Korsgaard
0 siblings, 1 reply; 6+ messages in thread
From: Francis Mendes @ 2011-06-07 1:54 UTC (permalink / raw)
To: buildroot
Hello Thomas,
On Mon, Jun 6, 2011 at 16:21, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:
> Hello Francis,
>
> Le Mon, 6 Jun 2011 00:03:36 -0300,
> Francis Mendes <francis.mendes@gmail.com> a ?crit :
>
> > I needed this package and added it to Buildroot.
> >
> > Can anyone review the attached patch for any mistakes?
>
> Thanks for this submission!
>
> It would be nicer to attach the patch inline, as it is easier to make
> comments through the patch.
>
Sorry about that. The new patch is attached inline below.
> Two comments :
>
> * You should "select" BR2_PACKAGE_NCURSES instead of "depends on".
> That's the dependency type we use for the dependency of programs on
> libraries.
>
Fixed.
> * Your statserial-1.1-fixmakefile.patch should have a header with a
> description saying why it is needed, and with a Signed-off-by line.
> From a quick read, I don't see why the patch is needed.
>
The original Makefile compiles the source code to statserial.o and then
links it. I don't know exactly why, but the linker fails with the message
"undefined reference to `atexit'". Searching in the web, I found someone
suggesting to compile and link in a single step using gcc, and it worked for
me. The purpose of the patch is to compile and link the source code this
way. If someone has a better way to fix the problem, let me know.
> * Your installation line should be
> $(INSTALL) -D -m 0755 $(@D)/statserial $(TARGET_DIR)/usr/bin/statserial
> otherwise your statserial binary will be named "bin" in
> $(TARGET_DIR)/usr if the usr/bin directory does not exist.
>
Fixed.
Otherwise, looks good.
>
Thanks for the input. Below is the new patch. If there's still something
wrong, let me know.
Francis
=====================================================
Add package statserial
Signed-off-by: Francis M. de P. Mendes <francis.mendes@gmail.com>
create mode 100644 package/statserial/Config.in
create mode 100644 package/statserial/statserial-1.1-fixmakefile.patch
create mode 100644 package/statserial/statserial.mk
diff --git a/package/Config.in b/package/Config.in
index 40f523d..ddcdf53 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -203,6 +203,7 @@ source "package/smartmontools/Config.in"
source "package/squashfs/Config.in"
source "package/squashfs3/Config.in"
source "package/sshfs/Config.in"
+source "package/statserial/Config.in"
source "package/sysstat/Config.in"
source "package/udev/Config.in"
source "package/usb_modeswitch/Config.in"
diff --git a/package/statserial/Config.in b/package/statserial/Config.in
new file mode 100644
index 0000000..b4d69ea
--- /dev/null
+++ b/package/statserial/Config.in
@@ -0,0 +1,11 @@
+config BR2_PACKAGE_STATSERIAL
+ bool "statserial"
+ select BR2_PACKAGE_NCURSES
+ help
+ Displays a table of the signals on a standard
+ 9-pin or 25-pin serial port, and indicates the
+ status of the handshaking lines. It can be
+ useful for debugging problems with serial
+ ports or modems.
+
+ https://sites.google.com/site/tranter/software
diff --git a/package/statserial/statserial-1.1-fixmakefile.patch
b/package/statserial/statserial-1.1-fixmakefile.patch
new file mode 100644
index 0000000..88cdd3f
--- /dev/null
+++ b/package/statserial/statserial-1.1-fixmakefile.patch
@@ -0,0 +1,26 @@
+Compile and link the source in one step only
+
+
+Signed-off-by: Francis M. de P. Mendes <francis.mendes@gmail.com>
+
+diff --git a/Makefile b/Makefile
+index 2ed79f1..bf82034 100644
+--- a/Makefile
++++ b/Makefile
+@@ -9,11 +9,8 @@ LD = gcc
+ CFLAGS = -Wall -O3 -fomit-frame-pointer
+ LDFLAGS = -s -N
+
+-statserial: statserial.o
+- $(LD) $(LDFLAGS) -o statserial statserial.o -lcurses
+-
+-statserial.o: statserial.c
+- $(CC) $(CFLAGS) -c statserial.c
++statserial:
++ $(CC) $(CFLAGS) -lcurses -o statserial statserial.c
+
+ install: statserial
+ install -m 555 statserial /usr/local/bin/statserial
+--
+1.7.0.4
+
diff --git a/package/statserial/statserial.mk b/package/statserial/
statserial.mk
new file mode 100644
index 0000000..500a97b
--- /dev/null
+++ b/package/statserial/statserial.mk
@@ -0,0 +1,20 @@
+#############################################################
+#
+# statserial
+#
+#############################################################
+STATSERIAL_VERSION = 1.1
+STATSERIAL_SOURCE = statserial-$(STATSERIAL_VERSION).tar.gz
+STATSERIAL_SITE = http://www.ibiblio.org/pub/Linux/system/serial/
+STATSERIAL_DEPENDENCIES = ncurses
+
+define STATSERIAL_BUILD_CMDS
+ $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" CFLAGS="$(TARGET_CFLAGS)"
LDFLAGS="$(TARGET_LDFLAGS)" -C $(@D)
+endef
+
+define STATSERIAL_INSTALL_TARGET_CMDS
+ $(INSTALL) -D -m 0755 $(@D)/statserial $(TARGET_DIR)/usr/bin/statserial
+endef
+
+$(eval $(call GENTARGETS,package,statserial))
+
--
1.7.0.4
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20110606/10eb3e04/attachment.html>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] Add package statserial
2011-06-07 1:54 ` Francis Mendes
@ 2011-06-07 10:33 ` Peter Korsgaard
2011-06-07 23:40 ` Francis Mendes
0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2011-06-07 10:33 UTC (permalink / raw)
To: buildroot
>>>>> "Francis" == Francis Mendes <francis.mendes@gmail.com> writes:
Hi,
Looks good, but here's a few more comments.
Francis> The original Makefile compiles the source code to statserial.o
Francis> and then links it. I don't know exactly why, but the linker
Francis> fails with the message "undefined reference to
Francis> `atexit'". Searching in the web, I found someone suggesting to
Francis> compile and link in a single step using gcc, and it worked for
Francis> me. The purpose of the patch is to compile and link the source
Francis> code this way. If someone has a better way to fix the problem,
Francis> let me know. ?
Francis> Thanks for the input. Below is the new patch. If there's still
Francis> something wrong, let me know.
Francis> Francis
Francis> =====================================================
Francis> Add package statserial
Francis> +++ b/package/statserial/Config.in
Francis> @@ -0,0 +1,11 @@
Francis> +config BR2_PACKAGE_STATSERIAL
Francis> +??? bool "statserial"
Francis> +??? select BR2_PACKAGE_NCURSES
Francis> +??? help
Francis> +??? ??? Displays a table of the signals on a standard
Francis> +??? ??? 9-pin or 25-pin serial port, and indicates the
Francis> +??? ??? status of the handshaking lines. It can be
Francis> +??? ??? useful for debugging problems with serial
Francis> +??? ??? ports or modems.
Francis> +
Francis> +??? ??? https://sites.google.com/site/tranter/software
Config.in files should be indented with <tab> rather than spaces - So
the bool/select/help files should be indented with <tab>, and the help
text lines with <tab><space><space>.
Francis> diff --git a/package/statserial/statserial-1.1-fixmakefile.patch b/package/
Francis> statserial/statserial-1.1-fixmakefile.patch
Francis> new file mode 100644
Francis> index 0000000..88cdd3f
Francis> --- /dev/null
Francis> +++ b/package/statserial/statserial-1.1-fixmakefile.patch
Francis> @@ -0,0 +1,26 @@
Francis> +Compile and link the source in one step only
Francis> +
Francis> +
Francis> +Signed-off-by: Francis M. de P. Mendes <francis.mendes@gmail.com>
Francis> +
Francis> +diff --git a/Makefile b/Makefile
Francis> +index 2ed79f1..bf82034 100644
Francis> +--- a/Makefile
Francis> ++++ b/Makefile
Francis> +@@ -9,11 +9,8 @@ LD??? = gcc
Francis> + CFLAGS??? = -Wall -O3 -fomit-frame-pointer
Francis> + LDFLAGS = -s -N
The problem is that the package expects LD to be gcc.
Francis> +++ b/package/statserial/statserial.mk
Francis> @@ -0,0 +1,20 @@
Francis> +#############################################################
Francis> +#
Francis> +# statserial
Francis> +#
Francis> +#############################################################
Francis> +STATSERIAL_VERSION = 1.1
Francis> +STATSERIAL_SOURCE = statserial-$(STATSERIAL_VERSION).tar.gz
Francis> +STATSERIAL_SITE = http://www.ibiblio.org/pub/Linux/system/serial/
Francis> +STATSERIAL_DEPENDENCIES = ncurses
Francis> +
Francis> +define STATSERIAL_BUILD_CMDS
Francis> +??? $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)" CFLAGS="$(TARGET_CFLAGS)"
So it would presumably work if you just passed LD="$(TARGET_CC) $(CFLAGS)"
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] Add package statserial
2011-06-07 10:33 ` Peter Korsgaard
@ 2011-06-07 23:40 ` Francis Mendes
2011-06-12 11:09 ` Peter Korsgaard
0 siblings, 1 reply; 6+ messages in thread
From: Francis Mendes @ 2011-06-07 23:40 UTC (permalink / raw)
To: buildroot
Hello Peter,
On Tue, Jun 7, 2011 at 07:33, Peter Korsgaard <jacmet@uclibc.org> wrote:
> >>>>> "Francis" == Francis Mendes <francis.mendes@gmail.com> writes:
>
> Hi,
>
> Looks good, but here's a few more comments.
>
> Francis> The original Makefile compiles the source code to statserial.o
> Francis> and then links it. I don't know exactly why, but the linker
> Francis> fails with the message "undefined reference to
> Francis> `atexit'". Searching in the web, I found someone suggesting to
> Francis> compile and link in a single step using gcc, and it worked for
> Francis> me. The purpose of the patch is to compile and link the source
> Francis> code this way. If someone has a better way to fix the problem,
> Francis> let me know.
>
> Francis> Thanks for the input. Below is the new patch. If there's still
> Francis> something wrong, let me know.
>
> Francis> Francis
>
> Francis> =====================================================
>
> Francis> Add package statserial
>
> Francis> +++ b/package/statserial/Config.in
> Francis> @@ -0,0 +1,11 @@
> Francis> +config BR2_PACKAGE_STATSERIAL
> Francis> + bool "statserial"
> Francis> + select BR2_PACKAGE_NCURSES
> Francis> + help
> Francis> + Displays a table of the signals on a standard
> Francis> + 9-pin or 25-pin serial port, and indicates the
> Francis> + status of the handshaking lines. It can be
> Francis> + useful for debugging problems with serial
> Francis> + ports or modems.
> Francis> +
> Francis> + https://sites.google.com/site/tranter/software
>
> Config.in files should be indented with <tab> rather than spaces - So
> the bool/select/help files should be indented with <tab>, and the help
> text lines with <tab><space><space>.
>
Fixed.
> Francis> diff --git a/package/statserial/statserial-1.1-fixmakefile.patch
> b/package/
> Francis> statserial/statserial-1.1-fixmakefile.patch
> Francis> new file mode 100644
> Francis> index 0000000..88cdd3f
> Francis> --- /dev/null
> Francis> +++ b/package/statserial/statserial-1.1-fixmakefile.patch
> Francis> @@ -0,0 +1,26 @@
> Francis> +Compile and link the source in one step only
> Francis> +
> Francis> +
> Francis> +Signed-off-by: Francis M. de P. Mendes <
> francis.mendes at gmail.com>
> Francis> +
> Francis> +diff --git a/Makefile b/Makefile
> Francis> +index 2ed79f1..bf82034 100644
> Francis> +--- a/Makefile
> Francis> ++++ b/Makefile
> Francis> +@@ -9,11 +9,8 @@ LD = gcc
> Francis> + CFLAGS = -Wall -O3 -fomit-frame-pointer
> Francis> + LDFLAGS = -s -N
>
> The problem is that the package expects LD to be gcc.
>
> Francis> +++ b/package/statserial/statserial.mk
> Francis> @@ -0,0 +1,20 @@
> Francis> +#############################################################
> Francis> +#
> Francis> +# statserial
> Francis> +#
> Francis> +#############################################################
> Francis> +STATSERIAL_VERSION = 1.1
> Francis> +STATSERIAL_SOURCE = statserial-$(STATSERIAL_VERSION).tar.gz
> Francis> +STATSERIAL_SITE =
> http://www.ibiblio.org/pub/Linux/system/serial/
> Francis> +STATSERIAL_DEPENDENCIES = ncurses
> Francis> +
> Francis> +define STATSERIAL_BUILD_CMDS
> Francis> + $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_LD)"
> CFLAGS="$(TARGET_CFLAGS)"
>
> So it would presumably work if you just passed LD="$(TARGET_CC) $(CFLAGS)"
>
You're absolutely right. And the answer to this was all the time in the
original Makefile. I feel so stupid :-(
Now it's fixed and there's no need for the patch. Thanks!
Francis
====================================================
Add package statserial
Signed-off-by: Francis M. de P. Mendes <francis.mendes@gmail.com>
---
package/Config.in | 1 +
package/statserial/Config.in | 11 +++++++++++
package/statserial/statserial.mk | 20 ++++++++++++++++++++
3 files changed, 32 insertions(+), 0 deletions(-)
create mode 100644 package/statserial/Config.in
create mode 100644 package/statserial/statserial.mk
diff --git a/package/Config.in b/package/Config.in
index 40f523d..ddcdf53 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -203,6 +203,7 @@ source "package/smartmontools/Config.in"
source "package/squashfs/Config.in"
source "package/squashfs3/Config.in"
source "package/sshfs/Config.in"
+source "package/statserial/Config.in"
source "package/sysstat/Config.in"
source "package/udev/Config.in"
source "package/usb_modeswitch/Config.in"
diff --git a/package/statserial/Config.in b/package/statserial/Config.in
new file mode 100644
index 0000000..b7211bb
--- /dev/null
+++ b/package/statserial/Config.in
@@ -0,0 +1,11 @@
+config BR2_PACKAGE_STATSERIAL
+ bool "statserial"
+ select BR2_PACKAGE_NCURSES
+ help
+ Displays a table of the signals on a standard
+ 9-pin or 25-pin serial port, and indicates the
+ status of the handshaking lines. It can be
+ useful for debugging problems with serial
+ ports or modems.
+
+ https://sites.google.com/site/tranter/software
diff --git a/package/statserial/statserial.mk b/package/statserial/
statserial.mk
new file mode 100644
index 0000000..e072d30
--- /dev/null
+++ b/package/statserial/statserial.mk
@@ -0,0 +1,20 @@
+#############################################################
+#
+# statserial
+#
+#############################################################
+STATSERIAL_VERSION = 1.1
+STATSERIAL_SOURCE = statserial-$(STATSERIAL_VERSION).tar.gz
+STATSERIAL_SITE = http://www.ibiblio.org/pub/Linux/system/serial/
+STATSERIAL_DEPENDENCIES = ncurses
+
+define STATSERIAL_BUILD_CMDS
+ $(MAKE) CC="$(TARGET_CC)" LD="$(TARGET_CC)" CFLAGS="$(TARGET_CFLAGS)"
LDFLAGS="$(TARGET_LDFLAGS)" -C $(@D)
+endef
+
+define STATSERIAL_INSTALL_TARGET_CMDS
+ $(INSTALL) -D -m 0755 $(@D)/statserial $(TARGET_DIR)/usr/bin/statserial
+endef
+
+$(eval $(call GENTARGETS,package,statserial))
+
--
1.7.0.4
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20110607/6d34761b/attachment-0001.html>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] Add package statserial
2011-06-07 23:40 ` Francis Mendes
@ 2011-06-12 11:09 ` Peter Korsgaard
0 siblings, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2011-06-12 11:09 UTC (permalink / raw)
To: buildroot
>>>>> "Francis" == Francis Mendes <francis.mendes@gmail.com> writes:
Hi,
Francis> You're absolutely right. And the answer to this was all the
Francis> time in the original Makefile. I feel so stupid :-(
Francis> Now it's fixed and there's no need for the patch. Thanks!
Thanks, committed.
Your mailer seems to have corrupted your patch, so I had to apply it
manually. Please consider using 'git send-email' in the future.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-06-12 11:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-06 3:03 [Buildroot] Add package statserial Francis Mendes
2011-06-06 19:21 ` Thomas Petazzoni
2011-06-07 1:54 ` Francis Mendes
2011-06-07 10:33 ` Peter Korsgaard
2011-06-07 23:40 ` Francis Mendes
2011-06-12 11:09 ` Peter Korsgaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox