| Message ID | 1516007331-17509-1-git-send-email-sergio.prado@e-labworks.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | aoetools: new package | expand |
Hello, On Mon, 15 Jan 2018 07:08:51 -0200, Sergio Prado wrote: > The aoetools are programs for users of the ATA over Ethernet (AoE) > network storage protocol, a simple protocol for using storage over an > ethernet LAN. > > Tested on Beaglebone Black. > > Build tested with test-pkg. > > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> I've applied, but after fixing a number of issues, see below. > diff --git a/package/aoetools/Config.in b/package/aoetools/Config.in > new file mode 100644 > index 000000000000..25623743b884 > --- /dev/null > +++ b/package/aoetools/Config.in > @@ -0,0 +1,12 @@ > +config BR2_PACKAGE_AOETOOLS > + depends on BR2_TOOLCHAIN_HAS_THREADS > + bool "aoetools" The bool property goes before the depends on. This is reported by check-package. > + help > + The aoetools are programs for users of the ATA over Ethernet (AoE) > + network storage protocol, a simple protocol for using storage over > + an ethernet LAN. The lines here are too long. This is also reported by check-package. > diff --git a/package/aoetools/aoetools.mk b/package/aoetools/aoetools.mk > new file mode 100644 > index 000000000000..e11a4f72f924 > --- /dev/null > +++ b/package/aoetools/aoetools.mk > @@ -0,0 +1,22 @@ > +################################################################################ > +# > +# aoetools > +# > +################################################################################ > + > +AOETOOLS_VERSION = 37 > +AOETOOLS_SITE = https://github.com/OpenAoE/aoetools/archive You should have used the github helper here, since there is no tarball uploaded by the maintainer. > +AOETOOLS_LICENSE = GPLv2 This should have been GPL-2.0, which is the SPDX license code for the GPLv2. > +AOETOOLS_LICENSE_FILES = COPYING > + > +define AOETOOLS_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(MAKE) CC=$(TARGET_CC) -C $(@D) And here, I've changed to use $(TARGET_CONFIGURE_OPTS) instead of just CC, so that the CFLAGS are properly passed/used. One thing I didn't fix is that the aoe-stat script installed by this package uses /bin/bash in its shebang, so it won't work because this package doesn't depend on bash. Does this script really needs bash? Should we remove it in a post-install target hook? Something else? Best regards, Thomas
Hi Thomas, Thanks for reviewing/applying the patch. > > +config BR2_PACKAGE_AOETOOLS > > + depends on BR2_TOOLCHAIN_HAS_THREADS > > + bool "aoetools" > > The bool property goes before the depends on. This is reported by > check-package. My mistake. I should definitely have run check-package. > > + help > > + The aoetools are programs for users of the ATA over Ethernet (AoE) > > + network storage protocol, a simple protocol for using storage over > > + an ethernet LAN. > > The lines here are too long. This is also reported by check-package. Yep. I'll make sure to always run check-package in the next contributions. > > +AOETOOLS_VERSION = 37 > > +AOETOOLS_SITE = https://github.com/OpenAoE/aoetools/archive > > You should have used the github helper here, since there is no tarball > uploaded by the maintainer. So the tarballs in https://github.com/OpenAoE/aoetools/releases was not uploaded by the developer? It was generated automatically by GitHub? How to know that? > > +AOETOOLS_LICENSE = GPLv2 > > This should have been GPL-2.0, which is the SPDX license code for the > GPLv2. Right. > > +AOETOOLS_LICENSE_FILES = COPYING > > + > > +define AOETOOLS_BUILD_CMDS > > + $(TARGET_MAKE_ENV) $(MAKE) CC=$(TARGET_CC) -C $(@D) > > And here, I've changed to use $(TARGET_CONFIGURE_OPTS) instead of just > CC, so that the CFLAGS are properly passed/used. OK. > One thing I didn't fix is that the aoe-stat script installed by this > package uses /bin/bash in its shebang, so it won't work because this > package doesn't depend on bash. Does this script really needs bash? > Should we remove it in a post-install target hook? Something else? It does not seem to need specifically /bin/bash. I'll propose a change to /bin/sh to the developer. If I did not get any response in the next weeks, I'll remove it with a post-install target hook. Best regards, Sergio Prado Embedded Labworks > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com <div dir="ltr">Hi Thomas,<br><br>Thanks for reviewing/applying the patch.<br><br>> > +config BR2_PACKAGE_AOETOOLS<br>> > + depends on BR2_TOOLCHAIN_HAS_THREADS<br>> > + bool "aoetools"<br>><br>> The bool property goes before the depends on. This is reported by<br>> check-package.<div><br></div><div>My mistake. I should definitely have run check-package.</div><div> <br>> > + help<br>> > + The aoetools are programs for users of the ATA over Ethernet (AoE)<br>> > + network storage protocol, a simple protocol for using storage over<br>> > + an ethernet LAN.<br>><br>> The lines here are too long. This is also reported by check-package.<br><br></div><div>Yep. I'll make sure to always run check-package in the next contributions.</div><div><br></div><div>> > +AOETOOLS_VERSION = 37<br>> > +AOETOOLS_SITE = <a href="https://github.com/OpenAoE/aoetools/archive">https://github.com/OpenAoE/aoetools/archive</a><br>><br>> You should have used the github helper here, since there is no tarball<br>> uploaded by the maintainer.<br><br></div><div>So the tarballs in <a href="https://github.com/OpenAoE/aoetools/releases">https://github.com/OpenAoE/aoetools/releases</a> was not uploaded by the developer? It was generated automatically by GitHub? How to know that?</div><div><br></div><div>> > +AOETOOLS_LICENSE = GPLv2<br>><br>> This should have been GPL-2.0, which is the SPDX license code for the<br>> GPLv2.<br><br></div><div>Right.</div><div><br></div><div>> > +AOETOOLS_LICENSE_FILES = COPYING<br>> > +<br>> > +define AOETOOLS_BUILD_CMDS<br>> > + $(TARGET_MAKE_ENV) $(MAKE) CC=$(TARGET_CC) -C $(@D)<br>><br>> And here, I've changed to use $(TARGET_CONFIGURE_OPTS) instead of just<br>> CC, so that the CFLAGS are properly passed/used.<br><br></div><div>OK.</div><div><br></div><div>> One thing I didn't fix is that the aoe-stat script installed by this<br>> package uses /bin/bash in its shebang, so it won't work because this<br>> package doesn't depend on bash. Does this script really needs bash?<br>> Should we remove it in a post-install target hook? Something else?<br><br></div><div>It does not seem to need specifically /bin/bash. I'll propose a change to /bin/sh to the developer. If I did not get any response in the next weeks, I'll remove it with a post-install target hook.</div><div><br></div><div>Best regards,</div><div><br></div><div>Sergio Prado</div><div>Embedded Labworks</div><div><br></div><div>><br>> Best regards,<br>><br>> Thomas<br>> --<br>> Thomas Petazzoni, CTO, Free Electrons<br>> Embedded Linux and Kernel engineering<br>> <a href="http://free-electrons.com">http://free-electrons.com</a></div></div>
diff --git a/DEVELOPERS b/DEVELOPERS index 2f7d051e8a98..ec960dee9ab1 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -1583,6 +1583,7 @@ F: package/rtl8189fs/ F: package/xr819-xradio/ N: Sergio Prado <sergio.prado@e-labworks.com> +F: package/aoetools/ F: package/curlpp/ F: package/daq/ F: package/libgdiplus/ diff --git a/package/Config.in b/package/Config.in index 01f4095be5aa..9625dae121d3 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1615,6 +1615,7 @@ endmenu menu "Networking applications" source "package/aircrack-ng/Config.in" + source "package/aoetools/Config.in" source "package/apache/Config.in" source "package/argus/Config.in" source "package/arp-scan/Config.in" diff --git a/package/aoetools/Config.in b/package/aoetools/Config.in new file mode 100644 index 000000000000..25623743b884 --- /dev/null +++ b/package/aoetools/Config.in @@ -0,0 +1,12 @@ +config BR2_PACKAGE_AOETOOLS + depends on BR2_TOOLCHAIN_HAS_THREADS + bool "aoetools" + help + The aoetools are programs for users of the ATA over Ethernet (AoE) + network storage protocol, a simple protocol for using storage over + an ethernet LAN. + + http://aoetools.sourceforge.net/ + +comment "aoetools needs a toolchain w/ threads" + depends on !BR2_TOOLCHAIN_HAS_THREADS diff --git a/package/aoetools/aoetools.hash b/package/aoetools/aoetools.hash new file mode 100644 index 000000000000..76c305624d5e --- /dev/null +++ b/package/aoetools/aoetools.hash @@ -0,0 +1,5 @@ +# Locally computed: +sha256 477e796f5c18e8c0e61b5d88e1759c68249e8e0210c2f3de2b98680e2cc63e32 aoetools-37.tar.gz + +# Hash for license files: +sha256 32b1062f7da84967e7019d01ab805935caa7ab7321a7ced0e30ebe75e5df1670 COPYING diff --git a/package/aoetools/aoetools.mk b/package/aoetools/aoetools.mk new file mode 100644 index 000000000000..e11a4f72f924 --- /dev/null +++ b/package/aoetools/aoetools.mk @@ -0,0 +1,22 @@ +################################################################################ +# +# aoetools +# +################################################################################ + +AOETOOLS_VERSION = 37 +AOETOOLS_SITE = https://github.com/OpenAoE/aoetools/archive + +AOETOOLS_LICENSE = GPLv2 +AOETOOLS_LICENSE_FILES = COPYING + +define AOETOOLS_BUILD_CMDS + $(TARGET_MAKE_ENV) $(MAKE) CC=$(TARGET_CC) -C $(@D) +endef + +define AOETOOLS_INSTALL_TARGET_CMDS + $(TARGET_MAKE_ENV) $(MAKE) DESTDIR=$(TARGET_DIR) \ + -C $(@D) install +endef + +$(eval $(generic-package))
The aoetools are programs for users of the ATA over Ethernet (AoE) network storage protocol, a simple protocol for using storage over an ethernet LAN. Tested on Beaglebone Black. Build tested with test-pkg. Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> --- DEVELOPERS | 1 + package/Config.in | 1 + package/aoetools/Config.in | 12 ++++++++++++ package/aoetools/aoetools.hash | 5 +++++ package/aoetools/aoetools.mk | 22 ++++++++++++++++++++++ 5 files changed, 41 insertions(+) create mode 100644 package/aoetools/Config.in create mode 100644 package/aoetools/aoetools.hash create mode 100644 package/aoetools/aoetools.mk