* [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