[Buildroot] [PATCH] chrony: new package

[Buildroot] [PATCH] chrony: new package

[Nathan Lynch]

Signed-off-by: Nathan Lynch <ntl@pobox.com>
---
 package/Config.in        | 1 +
 package/chrony/Config.in | 5 +++++
 package/chrony/chrony.mk | 6 ++++++
 3 files changed, 12 insertions(+)
 create mode 100644 package/chrony/Config.in
 create mode 100644 package/chrony/chrony.mk

diff --git a/package/Config.in b/package/Config.in
index fe4f56b..d5719a8 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -670,6 +670,7 @@ source "package/bind/Config.in"
 source "package/bmon/Config.in"
 source "package/bridge-utils/Config.in"
 source "package/can-utils/Config.in"
+source "package/chrony/Config.in"
 source "package/connman/Config.in"
 source "package/crda/Config.in"
 source "package/ctorrent/Config.in"
diff --git a/package/chrony/Config.in b/package/chrony/Config.in
new file mode 100644
index 0000000..a42bd77
--- /dev/null
+++ b/package/chrony/Config.in
@@ -0,0 +1,5 @@
+config BR2_PACKAGE_CHRONY
+	bool "chrony"
+	help
+	  Chrony is a pair of programs which are used to maintain the
+	  accuracy of the system clock.
diff --git a/package/chrony/chrony.mk b/package/chrony/chrony.mk
new file mode 100644
index 0000000..b9733f9
--- /dev/null
+++ b/package/chrony/chrony.mk
@@ -0,0 +1,6 @@
+CHRONY_VERSION = 1.27
+CHRONY_SITE = http://download.tuxfamily.org/chrony/
+CHRONY_LICENSE = GPLv2
+CHRONY_LICENSE_FILES = COPYING
+
+$(eval $(autotools-package))
-- 
1.8.1.4

[Buildroot] [PATCH] chrony: new package

[Thomas Petazzoni]

Dear Nathan Lynch,

Thanks, this looks almost good. A few comments below.

On Mon,  6 May 2013 11:12:55 -0500, Nathan Lynch wrote:

> diff --git a/package/chrony/Config.in b/package/chrony/Config.in
> new file mode 100644
> index 0000000..a42bd77
> --- /dev/null
> +++ b/package/chrony/Config.in
> @@ -0,0 +1,5 @@
> +config BR2_PACKAGE_CHRONY
> +	bool "chrony"
> +	help
> +	  Chrony is a pair of programs which are used to maintain the
> +	  accuracy of the system clock.

We generally want a empty new line here, and then the upstream URL of
the project.

Also, did you check that this program indeed builds with a minimal
uClibc toolchain (no thread, no wchar, no locale, no nothing) ?


> diff --git a/package/chrony/chrony.mk b/package/chrony/chrony.mk
> new file mode 100644
> index 0000000..b9733f9
> --- /dev/null
> +++ b/package/chrony/chrony.mk
> @@ -0,0 +1,6 @@

We generally like to have our nice header here, like in all other
packages:

#############################################################
#
# bind
#
#############################################################

> +CHRONY_VERSION = 1.27
> +CHRONY_SITE = http://download.tuxfamily.org/chrony/
> +CHRONY_LICENSE = GPLv2
> +CHRONY_LICENSE_FILES = COPYING
> +
> +$(eval $(autotools-package))

Looks good.

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH] chrony: new package

[Nathan Lynch]

On Mon, 2013-05-06 at 19:20 +0200, Thomas Petazzoni wrote:
> Dear Nathan Lynch,
> 
> Thanks, this looks almost good. A few comments below.
> 
> On Mon,  6 May 2013 11:12:55 -0500, Nathan Lynch wrote:
> 
> > diff --git a/package/chrony/Config.in b/package/chrony/Config.in
> > new file mode 100644
> > index 0000000..a42bd77
> > --- /dev/null
> > +++ b/package/chrony/Config.in
> > @@ -0,0 +1,5 @@
> > +config BR2_PACKAGE_CHRONY
> > +	bool "chrony"
> > +	help
> > +	  Chrony is a pair of programs which are used to maintain the
> > +	  accuracy of the system clock.
> 
> We generally want a empty new line here, and then the upstream URL of
> the project.

Okay, will fix.


> Also, did you check that this program indeed builds with a minimal
> uClibc toolchain (no thread, no wchar, no locale, no nothing) ?

Sorry, no, that was sloppy of me.  I will do so for v2 and try to better
specify the package's needs and dependencies.  I'd be happy to try any
configs you'd care to point out...


> > diff --git a/package/chrony/chrony.mk b/package/chrony/chrony.mk
> > new file mode 100644
> > index 0000000..b9733f9
> > --- /dev/null
> > +++ b/package/chrony/chrony.mk
> > @@ -0,0 +1,6 @@
> 
> We generally like to have our nice header here, like in all other
> packages:
> 
> #############################################################
> #
> # bind
> #
> #############################################################

Will fix.

Thanks for the review, Thomas.

[Buildroot] [PATCH] chrony: new package

[Thomas Petazzoni]

Dear Nathan Lynch,

On Mon, 06 May 2013 12:42:36 -0500, Nathan Lynch wrote:

> > Also, did you check that this program indeed builds with a minimal
> > uClibc toolchain (no thread, no wchar, no locale, no nothing) ?
> 
> Sorry, no, that was sloppy of me.  I will do so for v2 and try to
> better specify the package's needs and dependencies.  I'd be happy to
> try any configs you'd care to point out...

You can try a very minimal build: no thread, no wchar, no locale, no
C++, no nothing. If it builds, you're all set. If your package doesn't
build, then something is missing.

If you don't want to spend too much time building toolchains, you can
use:

 http://autobuild.buildroot.org/toolchains/tarballs/br-arm-basic.tar.bz2
 as an external toolchain that is minimal (no C++, no wchar, no locale,
 no largefile, etc. but it has thread support)

 http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-nothread.tar.bz2
 as an external that has everything (C++, wchar, locale, largefile,
 etc.), but doesn't has thread support.

If your package builds with both, then normally you should be all set.

And anyway, since those toolchains are the ones used on the
autobuilders, if your package doesn't build in some configuration, it
won't be catched by the autobuilders anyway :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH] chrony: new package

[Nathan Lynch]

On Mon, 2013-05-06 at 19:49 +0200, Thomas Petazzoni wrote:
> Dear Nathan Lynch,
> 
> On Mon, 06 May 2013 12:42:36 -0500, Nathan Lynch wrote:
> 
> > > Also, did you check that this program indeed builds with a minimal
> > > uClibc toolchain (no thread, no wchar, no locale, no nothing) ?
> > 
> > Sorry, no, that was sloppy of me.  I will do so for v2 and try to
> > better specify the package's needs and dependencies.  I'd be happy to
> > try any configs you'd care to point out...
> 
> You can try a very minimal build: no thread, no wchar, no locale, no
> C++, no nothing. If it builds, you're all set. If your package doesn't
> build, then something is missing.
> 
> If you don't want to spend too much time building toolchains, you can
> use:
> 
>  http://autobuild.buildroot.org/toolchains/tarballs/br-arm-basic.tar.bz2
>  as an external toolchain that is minimal (no C++, no wchar, no locale,
>  no largefile, etc. but it has thread support)
> 
>  http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-nothread.tar.bz2
>  as an external that has everything (C++, wchar, locale, largefile,
>  etc.), but doesn't has thread support.
> 
> If your package builds with both, then normally you should be all set.

Thank you for this guidance.  I verified that the package builds with
both of these and found no issues (I did realize it needs MMU, however).
I will do similarly for future package submissions.

v2 with the rest of your feedback addressed on its way.

[Buildroot] [PATCH] chrony: new package

[Thomas Petazzoni]

Dear Nathan Lynch,

On Mon, 06 May 2013 14:01:52 -0500, Nathan Lynch wrote:

> Thank you for this guidance.  I verified that the package builds with
> both of these and found no issues (I did realize it needs MMU,
> however). I will do similarly for future package submissions.
> 
> v2 with the rest of your feedback addressed on its way.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com