| Message ID | 1431848603-30142-1-git-send-email-julien.viarddegalbert@openwide.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello Julien, On Sun, 17 May 2015 09:43:23 +0200, julien.viarddegalbert@openwide.fr wrote: > diff --git a/package/Config.in b/package/Config.in > index af4d2b7..cc0bd79 100644 > --- a/package/Config.in > +++ b/package/Config.in > @@ -61,6 +61,7 @@ menu "Debugging, profiling and benchmark" > source "package/bonnie/Config.in" > source "package/cache-calibrator/Config.in" > source "package/dhrystone/Config.in" > + source "package/dieharder/Config.in" > source "package/dmalloc/Config.in" > source "package/dropwatch/Config.in" > source "package/dstat/Config.in" > diff --git a/package/dieharder/Config.in b/package/dieharder/Config.in > new file mode 100644 > index 0000000..9f81876 > --- /dev/null > +++ b/package/dieharder/Config.in > @@ -0,0 +1,9 @@ > +config BR2_PACKAGE_DIEHARDER > + bool "dieharder" > + select BR2_PACKAGE_GSL > + help > + dieharder is a fairly involved random number/uniform deviate generator > + tester. It is thus suitable for use in testing both software RNG's and > + hardware RNG's. Those help lines look fairly too long. Make sure they're not longer than 72 columns. > diff --git a/package/dieharder/dieharder.mk b/package/dieharder/dieharder.mk > new file mode 100644 > index 0000000..2a3d46b > --- /dev/null > +++ b/package/dieharder/dieharder.mk > @@ -0,0 +1,26 @@ > +################################################################################ > +# > +# dieharder > +# > +################################################################################ > + > +DIEHARDER_VERSION = 3.31.1 > +DIEHARDER_SITE = http://www.phy.duke.edu/~rgb/General/dieharder/ > +DIEHARDER_SOURCE = dieharder-$(DIEHARDER_VERSION).tgz > +DIEHARDER_SUBDIR = dieharder-$(DIEHARDER_VERSION) > +DIEHARDER_LICENSE = GPLv2b What is GPLv2b ? We normally have GPLv2 or GPLv2+. > +DIEHARDER_LICENSE_FILES = $(DIEHARDER_SUBDIR)/COPYING > +DIEHARDER_DEPENDENCIES = gsl > + > +DIEHARDER_CONF_OPTS = --includedir=$(STAGING_DIR)/usr/include Why do you need this? This looks wrong, as it would install the headers in a completely wrong location, if dieharder would be installing headers. --includedir is not used to specify where a program should look for headers of libraries, but to tell where it should install its own headers. > +# fix endiannes detection > +ifeq ($(BR2_ENDIAN),"BIG") > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=big > +else > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=little > +endif We generally pass such values in <pkg>_CONF_ENV instead. > + > +# parallel build fail, disable it > +DIEHARDER_MAKE=$(MAKE1) Spaces around = sign please. Thanks! Thomas
On Sun, May 17, 2015 at 10:20:36AM +0200, Thomas Petazzoni wrote: > Hello Julien, > > On Sun, 17 May 2015 09:43:23 +0200, julien.viarddegalbert@openwide.fr > wrote: > > > diff --git a/package/Config.in b/package/Config.in > > index af4d2b7..cc0bd79 100644 > > --- a/package/Config.in > > +++ b/package/Config.in > > @@ -61,6 +61,7 @@ menu "Debugging, profiling and benchmark" > > source "package/bonnie/Config.in" > > source "package/cache-calibrator/Config.in" > > source "package/dhrystone/Config.in" > > + source "package/dieharder/Config.in" > > source "package/dmalloc/Config.in" > > source "package/dropwatch/Config.in" > > source "package/dstat/Config.in" > > diff --git a/package/dieharder/Config.in b/package/dieharder/Config.in > > new file mode 100644 > > index 0000000..9f81876 > > --- /dev/null > > +++ b/package/dieharder/Config.in > > @@ -0,0 +1,9 @@ > > +config BR2_PACKAGE_DIEHARDER > > + bool "dieharder" > > + select BR2_PACKAGE_GSL > > + help > > + dieharder is a fairly involved random number/uniform deviate generator > > + tester. It is thus suitable for use in testing both software RNG's and > > + hardware RNG's. > > Those help lines look fairly too long. Make sure they're not longer > than 72 columns. > Ok, also I just sent another patch to document that point. > > diff --git a/package/dieharder/dieharder.mk b/package/dieharder/dieharder.mk > > new file mode 100644 > > index 0000000..2a3d46b > > --- /dev/null > > +++ b/package/dieharder/dieharder.mk > > @@ -0,0 +1,26 @@ > > +################################################################################ > > +# > > +# dieharder > > +# > > +################################################################################ > > + > > +DIEHARDER_VERSION = 3.31.1 > > +DIEHARDER_SITE = http://www.phy.duke.edu/~rgb/General/dieharder/ > > +DIEHARDER_SOURCE = dieharder-$(DIEHARDER_VERSION).tgz > > +DIEHARDER_SUBDIR = dieharder-$(DIEHARDER_VERSION) > > +DIEHARDER_LICENSE = GPLv2b > > What is GPLv2b ? We normally have GPLv2 or GPLv2+. > You are right this should be made more explicit, there is an extra "beverage" clause in the licence file. I could either use the syntax the software uses: DIEHARDER_LICENSE = GPLv2b (b for beverage) Or make it simply: DIEHARDER_LICENSE = GPLv2 with beverage clause What do you think ? > > +DIEHARDER_LICENSE_FILES = $(DIEHARDER_SUBDIR)/COPYING > > +DIEHARDER_DEPENDENCIES = gsl > > + > > +DIEHARDER_CONF_OPTS = --includedir=$(STAGING_DIR)/usr/include > > Why do you need this? This looks wrong, as it would install the > headers in a completely wrong location, if dieharder would be > installing headers. --includedir is not used to specify where a program > should look for headers of libraries, but to tell where it should > install its own headers. > That was to try to fix the build using "/usr/include/" but it's wrong. I overlooked the normal --includedir function. So I guess the upstream scripts are wrong as they use the --includedir value to also specify some include search path... Also it _really_ do install some headers there. We probably don't want those on the target rootfs. So I guess I need to find how to tell it not to do that... > > +# fix endiannes detection > > +ifeq ($(BR2_ENDIAN),"BIG") > > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=big > > +else > > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=little > > +endif > > We generally pass such values in <pkg>_CONF_ENV instead. > OK, I'll do that. > > + > > +# parallel build fail, disable it > > +DIEHARDER_MAKE=$(MAKE1) > > Spaces around = sign please. OK, sorry, I read about that one... my mistake. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
Dear Julien Viard de Galbert, On Sun, 17 May 2015 14:18:15 +0200, Julien Viard de Galbert wrote: > > Those help lines look fairly too long. Make sure they're not longer > > than 72 columns. > > > Ok, also I just sent another patch to document that point. Sure, good idea. > You are right this should be made more explicit, there is an > extra "beverage" clause in the licence file. > I could either use the syntax the software uses: > DIEHARDER_LICENSE = GPLv2b (b for beverage) > > Or make it simply: > DIEHARDER_LICENSE = GPLv2 with beverage clause > > What do you think ? The second solution you propose looks better to me. > > > +DIEHARDER_LICENSE_FILES = $(DIEHARDER_SUBDIR)/COPYING > > > +DIEHARDER_DEPENDENCIES = gsl > > > + > > > +DIEHARDER_CONF_OPTS = --includedir=$(STAGING_DIR)/usr/include > > > > Why do you need this? This looks wrong, as it would install the > > headers in a completely wrong location, if dieharder would be > > installing headers. --includedir is not used to specify where a program > > should look for headers of libraries, but to tell where it should > > install its own headers. > > > That was to try to fix the build using "/usr/include/" but it's wrong. > I overlooked the normal --includedir function. So I guess the upstream > scripts are wrong as they use the --includedir value to also specify > some include search path... > > Also it _really_ do install some headers there. We probably don't want > those on the target rootfs. So I guess I need to find how to tell it not > to do that... As long as headers are installed in $(TARGET_DIR)/usr/include, you shouldn't do anything: Buildroot automatically deletes this directory at the end of the build process. Thanks, Thomas
Hi Julien, Le 17/05/2015 09:43, julien.viarddegalbert@openwide.fr a écrit : > From: Julien Viard de Galbert <julien@vdg.name> > > Signed-off-by: Julien Viard de Galbert <julien@vdg.name> > --- > Changes v2 -> v3 > - remove intermediate variable use (suggested by Baruch Siach) > Changes v1 -> v2 > - fixed typo "bin" instead of "big" (thanks Thomas Petazoni) > - select gls in config (suggested by Romain Naour) > - specified "includedir" to fix unsafe header path > > Signed-off-by: Julien Viard de Galbert <julien@vdg.name> > --- > package/Config.in | 1 + > package/dieharder/Config.in | 9 +++++++++ > package/dieharder/dieharder.mk | 26 ++++++++++++++++++++++++++ > 3 files changed, 36 insertions(+) > create mode 100644 package/dieharder/Config.in > create mode 100644 package/dieharder/dieharder.mk > > diff --git a/package/Config.in b/package/Config.in > index af4d2b7..cc0bd79 100644 > --- a/package/Config.in > +++ b/package/Config.in > @@ -61,6 +61,7 @@ menu "Debugging, profiling and benchmark" > source "package/bonnie/Config.in" > source "package/cache-calibrator/Config.in" > source "package/dhrystone/Config.in" > + source "package/dieharder/Config.in" > source "package/dmalloc/Config.in" > source "package/dropwatch/Config.in" > source "package/dstat/Config.in" > diff --git a/package/dieharder/Config.in b/package/dieharder/Config.in > new file mode 100644 > index 0000000..9f81876 > --- /dev/null > +++ b/package/dieharder/Config.in > @@ -0,0 +1,9 @@ > +config BR2_PACKAGE_DIEHARDER > + bool "dieharder" > + select BR2_PACKAGE_GSL > + help > + dieharder is a fairly involved random number/uniform deviate generator > + tester. It is thus suitable for use in testing both software RNG's and > + hardware RNG's. > + > + http://www.phy.duke.edu/~rgb/General/dieharder.php > diff --git a/package/dieharder/dieharder.mk b/package/dieharder/dieharder.mk > new file mode 100644 > index 0000000..2a3d46b > --- /dev/null > +++ b/package/dieharder/dieharder.mk > @@ -0,0 +1,26 @@ > +################################################################################ > +# > +# dieharder > +# > +################################################################################ > + > +DIEHARDER_VERSION = 3.31.1 > +DIEHARDER_SITE = http://www.phy.duke.edu/~rgb/General/dieharder/ > +DIEHARDER_SOURCE = dieharder-$(DIEHARDER_VERSION).tgz > +DIEHARDER_SUBDIR = dieharder-$(DIEHARDER_VERSION) > +DIEHARDER_LICENSE = GPLv2b > +DIEHARDER_LICENSE_FILES = $(DIEHARDER_SUBDIR)/COPYING > +DIEHARDER_DEPENDENCIES = gsl > + > +DIEHARDER_CONF_OPTS = --includedir=$(STAGING_DIR)/usr/include > +# fix endiannes detection > +ifeq ($(BR2_ENDIAN),"BIG") > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=big > +else > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=little > +endif There is a build issue with musl libc due to missing M_PI when _GNU_SOURCE is not defined. I suggest you add it in CFLAGS by using DIEHARDER_MAKE_OPTS: DIEHARDER_MAKE_OPTS = CFLAGS="$(TARGET_CFLAGS) -D_GNU_SOURCE" Don't forget to take into account Thomas's comment for the next version ;-) Best regards, Romain > + > +# parallel build fail, disable it > +DIEHARDER_MAKE=$(MAKE1) > + > +$(eval $(autotools-package)) >
Julien, Romain, All, On 2015-07-11 01:10 +0200, Romain Naour spake thusly: > Le 17/05/2015 09:43, julien.viarddegalbert@openwide.fr a écrit : > > From: Julien Viard de Galbert <julien@vdg.name> > > > > Signed-off-by: Julien Viard de Galbert <julien@vdg.name> [--SNIP--] > > diff --git a/package/dieharder/dieharder.mk b/package/dieharder/dieharder.mk > > new file mode 100644 > > index 0000000..2a3d46b > > --- /dev/null > > +++ b/package/dieharder/dieharder.mk > > @@ -0,0 +1,26 @@ > > +################################################################################ > > +# > > +# dieharder > > +# > > +################################################################################ > > + > > +DIEHARDER_VERSION = 3.31.1 > > +DIEHARDER_SITE = http://www.phy.duke.edu/~rgb/General/dieharder/ > > +DIEHARDER_SOURCE = dieharder-$(DIEHARDER_VERSION).tgz > > +DIEHARDER_SUBDIR = dieharder-$(DIEHARDER_VERSION) > > +DIEHARDER_LICENSE = GPLv2b > > +DIEHARDER_LICENSE_FILES = $(DIEHARDER_SUBDIR)/COPYING > > +DIEHARDER_DEPENDENCIES = gsl > > + > > +DIEHARDER_CONF_OPTS = --includedir=$(STAGING_DIR)/usr/include > > +# fix endiannes detection > > +ifeq ($(BR2_ENDIAN),"BIG") > > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=big > > +else > > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=little > > +endif > > There is a build issue with musl libc due to missing M_PI when _GNU_SOURCE is > not defined. > > I suggest you add it in CFLAGS by using DIEHARDER_MAKE_OPTS: > > DIEHARDER_MAKE_OPTS = CFLAGS="$(TARGET_CFLAGS) -D_GNU_SOURCE" Better yet: patch dieharder to define it in the source, and submit that patch upstream. Regards, Yann E. MORIN. > Don't forget to take into account Thomas's comment for the next version ;-) > > Best regards, > Romain > > > + > > +# parallel build fail, disable it > > +DIEHARDER_MAKE=$(MAKE1) > > + > > +$(eval $(autotools-package)) > > > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hello All, I've been moving and still need to wait to get internet access at the new home, so this will take some time, but yes, I think it's better to upstream patches than work on our side. Also I'm not sure how soon I'll be able to work on this, I will keep all your emails and review them all at that time. Best regards, Julien ----- Mail original ----- > De: "Yann E. MORIN" <yann.morin.1998@free.fr> > À: "Romain Naour" <romain.naour@openwide.fr> > Cc: "Julien Viard de Galbert" <julien@vdg.name>, buildroot@buildroot.org, "julien viarddegalbert" > <julien.viarddegalbert@openwide.fr> > Envoyé: Samedi 11 Juillet 2015 09:43:44 > Objet: Re: [Buildroot] [PATCH v3] dieharder: new package > > Julien, Romain, All, > > On 2015-07-11 01:10 +0200, Romain Naour spake thusly: > > Le 17/05/2015 09:43, julien.viarddegalbert@openwide.fr a écrit : > > > From: Julien Viard de Galbert <julien@vdg.name> > > > > > > Signed-off-by: Julien Viard de Galbert <julien@vdg.name> > [--SNIP--] > > > diff --git a/package/dieharder/dieharder.mk > > > b/package/dieharder/dieharder.mk > > > new file mode 100644 > > > index 0000000..2a3d46b > > > --- /dev/null > > > +++ b/package/dieharder/dieharder.mk > > > @@ -0,0 +1,26 @@ > > > +################################################################################ > > > +# > > > +# dieharder > > > +# > > > +################################################################################ > > > + > > > +DIEHARDER_VERSION = 3.31.1 > > > +DIEHARDER_SITE = http://www.phy.duke.edu/~rgb/General/dieharder/ > > > +DIEHARDER_SOURCE = dieharder-$(DIEHARDER_VERSION).tgz > > > +DIEHARDER_SUBDIR = dieharder-$(DIEHARDER_VERSION) > > > +DIEHARDER_LICENSE = GPLv2b > > > +DIEHARDER_LICENSE_FILES = $(DIEHARDER_SUBDIR)/COPYING > > > +DIEHARDER_DEPENDENCIES = gsl > > > + > > > +DIEHARDER_CONF_OPTS = --includedir=$(STAGING_DIR)/usr/include > > > +# fix endiannes detection > > > +ifeq ($(BR2_ENDIAN),"BIG") > > > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=big > > > +else > > > +DIEHARDER_CONF_OPTS += ac_cv_c_endian=little > > > +endif > > > > There is a build issue with musl libc due to missing M_PI when > > _GNU_SOURCE is > > not defined. > > > > I suggest you add it in CFLAGS by using DIEHARDER_MAKE_OPTS: > > > > DIEHARDER_MAKE_OPTS = CFLAGS="$(TARGET_CFLAGS) -D_GNU_SOURCE" > > Better yet: patch dieharder to define it in the source, and submit > that > patch upstream. > > Regards, > Yann E. MORIN. > > > Don't forget to take into account Thomas's comment for the next > > version ;-) > > > > Best regards, > > Romain > > > > > + > > > +# parallel build fail, disable it > > > +DIEHARDER_MAKE=$(MAKE1) > > > + > > > +$(eval $(autotools-package)) > > > > > > > _______________________________________________ > > buildroot mailing list > > buildroot@busybox.net > > http://lists.busybox.net/mailman/listinfo/buildroot > > -- > .-----------------.--------------------.------------------.--------------------. > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' > | conspiracy: | > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ > | | > | +33 223 225 172 `------------.-------: X AGAINST | \e/ > | There is no | > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v > | conspiracy. | > '------------------------------^-------^------------------^--------------------' > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
diff --git a/package/Config.in b/package/Config.in index af4d2b7..cc0bd79 100644 --- a/package/Config.in +++ b/package/Config.in @@ -61,6 +61,7 @@ menu "Debugging, profiling and benchmark" source "package/bonnie/Config.in" source "package/cache-calibrator/Config.in" source "package/dhrystone/Config.in" + source "package/dieharder/Config.in" source "package/dmalloc/Config.in" source "package/dropwatch/Config.in" source "package/dstat/Config.in" diff --git a/package/dieharder/Config.in b/package/dieharder/Config.in new file mode 100644 index 0000000..9f81876 --- /dev/null +++ b/package/dieharder/Config.in @@ -0,0 +1,9 @@ +config BR2_PACKAGE_DIEHARDER + bool "dieharder" + select BR2_PACKAGE_GSL + help + dieharder is a fairly involved random number/uniform deviate generator + tester. It is thus suitable for use in testing both software RNG's and + hardware RNG's. + + http://www.phy.duke.edu/~rgb/General/dieharder.php diff --git a/package/dieharder/dieharder.mk b/package/dieharder/dieharder.mk new file mode 100644 index 0000000..2a3d46b --- /dev/null +++ b/package/dieharder/dieharder.mk @@ -0,0 +1,26 @@ +################################################################################ +# +# dieharder +# +################################################################################ + +DIEHARDER_VERSION = 3.31.1 +DIEHARDER_SITE = http://www.phy.duke.edu/~rgb/General/dieharder/ +DIEHARDER_SOURCE = dieharder-$(DIEHARDER_VERSION).tgz +DIEHARDER_SUBDIR = dieharder-$(DIEHARDER_VERSION) +DIEHARDER_LICENSE = GPLv2b +DIEHARDER_LICENSE_FILES = $(DIEHARDER_SUBDIR)/COPYING +DIEHARDER_DEPENDENCIES = gsl + +DIEHARDER_CONF_OPTS = --includedir=$(STAGING_DIR)/usr/include +# fix endiannes detection +ifeq ($(BR2_ENDIAN),"BIG") +DIEHARDER_CONF_OPTS += ac_cv_c_endian=big +else +DIEHARDER_CONF_OPTS += ac_cv_c_endian=little +endif + +# parallel build fail, disable it +DIEHARDER_MAKE=$(MAKE1) + +$(eval $(autotools-package))