Re: [Buildroot] [PATCH 1/1] package/docopt.cpp: new package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Eero,

On Wed, 19 Oct 2022 17:32:19 +0300
Eero Aaltonen via buildroot <buildroot@buildroot.org> wrote:

> Add docopt.cpp library that can be used to create intuitive command
> line interfaces with very little code by describing the valid calls
> using the (POSIX.1-2017) command line utility argument syntax.
> 
> Signed-off-by: Eero Aaltonen <eero.aaltonen@vaisala.com>

Thanks a lot for your patch! I'm adding in Cc Angelo, who helped by
reviewing/testing your patch. I have applied your patch, but after
doing a number of changes, which are described below. Angelo: look
below as well, to see the few things that you missed as part of your
review, as it will definitely help in doing more detailed reviews in
the future!

>  DEVELOPERS                         |  3 +++
>  package/Config.in                  |  1 +
>  package/docopt.cpp/Config.in       | 15 +++++++++++++++
>  package/docopt.cpp/docopt.cpp.hash |  6 ++++++
>  package/docopt.cpp/docopt.cpp.mk   | 13 +++++++++++++
>  5 files changed, 38 insertions(+)

I renamed to package/docopt-cpp/. I don't think we have other packages
with a "." in their name, and I didn't want to create a new situation.


> diff --git a/package/docopt.cpp/Config.in b/package/docopt.cpp/Config.in
> new file mode 100644
> index 0000000000..655a656398
> --- /dev/null
> +++ b/package/docopt.cpp/Config.in
> @@ -0,0 +1,15 @@
> +config BR2_PACKAGE_DOCOPT_CPP
> +	bool "docopt.cpp"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_7 # C++11
> +	help
> +	  docopt is a library that lets you define a command line
> +	  interface with the utility argument syntax that has been
> +	  used by command line utilities for decades (POSIX.1-2017).
> +	  From the description, docopt automatically generates a parser
> +	  for the command line arguments.
> +
> +	  docopt Command-line interface description language
> +	  http://docopt.org/
> +
> +	  docopt C++ port https://github.com/docopt/docopt.cpp

The last line of the help text should only contain the upstream URL of
the project, so I reformatted a bit to match this requirement.

Also, you had forgotten a Config.in comment to document the C++ and gcc
4.7+ dependencies, so I have added:

+comment "docopt-cpp needs a toolchain w/ C++, gcc >= 4.7"
+       depends on !BR2_INSTALL_LIBSTDCPP || \
+               !BR2_TOOLCHAIN_GCC_AT_LEAST_4_7

> diff --git a/package/docopt.cpp/docopt.cpp.mk b/package/docopt.cpp/docopt.cpp.mk
> new file mode 100644
> index 0000000000..647071243a
> --- /dev/null
> +++ b/package/docopt.cpp/docopt.cpp.mk
> @@ -0,0 +1,13 @@
> +################################################################################
> +#
> +# docopt.cpp
> +#
> +################################################################################
> +
> +DOCOPT_CPP_VERSION = 0.6.3
> +DOCOPT_CPP_SOURCE = v$(DOCOPT_CPP_VERSION).tar.gz
> +DOCOPT_CPP_SITE = https://github.com/docopt/docopt.cpp/archive/refs/tags

You should have use the github macro here:

+DOCOPT_CPP_VERSION = 0.6.3
+DOCOPT_CPP_SITE = $(call github,docopt,docopt.cpp,v$(DOCOPT_CPP_VERSION))

> +DOCOPT_CPP_INSTALL_STAGING = YES
> +DOCOPT_CPP_LICENSE_FILES = LICENSE-Boost-1.0 LICENSE-MIT

The LICENSE variable was missing, so I've added it:

+DOCOPT_CPP_LICENSE = BSL-1.0 or MIT

Again, applied with all those changes. See:

  https://gitlab.com/buildroot.org/buildroot/-/commit/43f96ced67c9a1f84d3f0edf5cd632178eb176b8

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot