Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Olivain via buildroot <buildroot@buildroot.org>
To: viper@landau.one
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/1] package/cpp-argparse: new package
Date: Sat, 01 Aug 2026 13:52:24 +0200	[thread overview]
Message-ID: <720f85aa2477fe660f66a45354973035@free.fr> (raw)
In-Reply-To: <20260729152659.3813837-1-viper@landau.one>

Hi Konstantin,

Thanks for the patch. I have few comments, see below.

On 29/07/2026 17:26, viper@landau.one wrote:
> From: Konstantin Menyaev <viper@landau.one>
> 
> A Modern C++ header-only library
> for parsing command-line arguments.
> 
> https: //github.com/p-ranav/argparse
> 
> Signed-off-by: Konstantin Menyaev <viper@landau.one>
> ---
>  package/Config.in                    |  1 +
>  package/cpp-argparse/Config.in       | 12 ++++++++++++
>  package/cpp-argparse/cpp-argparse.mk | 16 ++++++++++++++++
>  3 files changed, 29 insertions(+)
>  create mode 100644 package/cpp-argparse/Config.in
>  create mode 100644 package/cpp-argparse/cpp-argparse.mk

Your submission misses the cpp-argparse.hash file. Could you add
it please? See:
https://nightly.buildroot.org/manual.html#adding-packages-hash

You can check that with commands:

make cpp-argparse-source
WARNING: no hash file for cpp-argparse-v3.2.tar.gz

make cpp-argparse-legal-info
WARNING: no hash file for LICENSE

[...]
> diff --git a/package/cpp-argparse/Config.in 
> b/package/cpp-argparse/Config.in
> new file mode 100644
> index 0000000000..7ad91f7bbc
> --- /dev/null
> +++ b/package/cpp-argparse/Config.in
> @@ -0,0 +1,12 @@
> +config BR2_PACKAGE_CPP_ARGPARSE
> +	bool "cpp-argparse"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_7

Upstream says the package requires gcc >= 8. Could you align to
this version please?
https://github.com/p-ranav/argparse/blob/v3.2/README.md?plain=1#L1432

> +	help
> +	  Argument Parser for Modern C++. A header-only library
> +	  for parsing command-line arguments.
> +
> +	  https://github.com/p-ranav/argparse
> +
> +comment "cpp-argparse needs a toolchain w/ C++, GCC >= 7"
> +	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_GCC_AT_LEAST_7

Please adjust the gcc version here too.

> diff --git a/package/cpp-argparse/cpp-argparse.mk 
> b/package/cpp-argparse/cpp-argparse.mk
> new file mode 100644
> index 0000000000..7978fd224c
> --- /dev/null
> +++ b/package/cpp-argparse/cpp-argparse.mk
> @@ -0,0 +1,16 @@
> +################################################################################
> +#
> +# cpp-argparse
> +#
> +################################################################################
> +
> +CPP_ARGPARSE_VERSION = v3.2
> +CPP_ARGPARSE_SITE = $(call 
> github,p-ranav,argparse,$(CPP_ARGPARSE_VERSION))

For automated release monitoring, could you please move the 'v'
to _SITE to have instead:

CPP_ARGPARSE_VERSION = 3.2
CPP_ARGPARSE_SITE = $(call 
github,p-ranav,argparse,v$(CPP_ARGPARSE_VERSION))

> +CPP_ARGPARSE_LICENSE = MIT
> +CPP_ARGPARSE_LICENSE_FILES = LICENSE
> +CPP_ARGPARSE_INSTALL_STAGING = YES
> +CPP_ARGPARSE_INSTALL_TARGET = NO
> +
> +CPP_ARGPARSE_CONF_OPTS = -DARGPARSE_BUILD_SAMPLES=OFF 
> -DARGPARSE_BUILD_TESTS=OFF
> +
> +$(eval $(cmake-package))
> --
> 2.55.0

Could you send an updated patch addressing those comments, please?

Best regards,

Julien.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  parent reply	other threads:[~2026-08-01 11:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29 15:26 [Buildroot] [PATCH 1/1] package/cpp-argparse: new package viper
2026-07-30 16:13 ` Konstantin Menyaev
2026-08-01 11:52 ` Julien Olivain via buildroot [this message]
2026-08-01 13:57   ` [Buildroot] Thanks for remarks, done viper
2026-08-01 14:04   ` viper
2026-08-01 14:09   ` [Buildroot] [PATCH v2 1/1] package/cpp-argparse: new package viper
2026-08-01 14:27     ` Julien Olivain via buildroot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=720f85aa2477fe660f66a45354973035@free.fr \
    --to=buildroot@buildroot.org \
    --cc=ju.o@free.fr \
    --cc=viper@landau.one \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox