GNU bug report logs - #41919
[patch] Add curlpp C++ bindings to curl package

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41919 in the body.
You can then email your comments to 41919 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
To: guix-patches <at> gnu.org
Subject: [patch] Add curlpp C++ bindings to curl package
Date: Wed, 17 Jun 2020 13:02:09 +0100

[Message part 1 (text/plain, inline)]

[0001-gnu-Add-curlpp-library.patch (text/x-patch, attachment)]

Message #8 received at 41919 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
Cc: 41919 <at> debbugs.gnu.org
Subject: Re: [bug#41919] [patch] Add curlpp C++ bindings to curl package
Date: Thu, 18 Jun 2020 14:43:12 +0200

Hi Dale,

Dale Mellor <guix-devel-0brg6b <at> rdmp.org> skribis:

> From 36c628cd27c2c6f281ec800767ba0e514fae13b1 Mon Sep 17 00:00:00 2001
> From: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
> Date: Wed, 17 Jun 2020 12:42:54 +0100
> Subject: [PATCH] gnu:  Add curlpp library.
>
> * gnu/packages/curl.scm (curlpp): New variable.

Nice!  A couple of comments:

[...]

> +(define-public  curlpp
> +  (package
> +   (name "curlpp")

Please pass the package through ./etc/indent-code.el:

  https://guix.gnu.org/manual/en/html_node/Formatting-Code.html

> +   (version "0.8.1")
> +   (source  (origin
> +             (method  git-fetch)
> +             (uri (git-reference
> +                   (url "https://github.com/jpbarrette/curlpp.git")
> +                   (commit "v0.8.1")))
> +             (sha256
> +              (base32 "1b0ylnnrhdax4kwjq64r1fk0i24n5ss6zfzf4hxwgslny01xiwrk"))))
> +   (build-system  cmake-build-system)
> +   (arguments  `(#:phases (modify-phases %standard-phases (delete 'check))))

To disable tests, use #:tests? #f.

However, we only disable tests when there’s a compelling reason to do
so, in which case there should be a comment indicating why they’re
disabled.

What’s the reason here?  Perhaps name-lookup errors?  (There’s no
networking in the isolated build environment.)

> +   (propagated-inputs `(("curl" ,curl)))

Do we need to propagate due to cURL headers being included in public
headers?

> +   (synopsis  "C++ wrapper around libcURL")
> +   (description
> +    "A free and easy-to-use client-side C++ URL transfer library,
> +supporting FTP, FTPS, HTTP, HTTPS, GOPHER, TELNET, DICT, FILE and LDAP. The
> +curlpp library supports HTTPS certificates, HTTP POST, HTTP PUT, FTP
> +uploading, kerberos, HTTP form based upload, proxies, cookies, user+password
> +authentication, file transfer resume, http proxy tunneling and more!  The
> +curlpp library is highly portable, it builds and works identically on numerous
> +platforms, including Solaris, NetBSD, FreeBSD, OpenBSD, Darwin, HPUX, IRIX,
> +AIX, Tru64, Linux, Windows, Amiga, OS/2, BeOs, Mac OS X, Ultrix, QNX, OpenVMS,
> +RISC OS, Novell NetWare, DOS and more... curlpp is free, thread-safe, IPv6
> +compatible, feature rich, well supported and fast.")

Please write full sentences and remove the list of supported operating
systems, which is not useful info in the context of Guix:

  https://guix.gnu.org/manual/en/html_node/Synopses-and-Descriptions.html

Thanks for the patch!

Ludo’.

Message #11 received at 41919 <at> debbugs.gnu.org (full text, mbox):

From: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
To: 41919 <at> debbugs.gnu.org
Subject: Re: [patch] Add curlpp C++ bindings to curl package
Date: Mon, 22 Jun 2020 00:16:09 +0100

[Message part 1 (text/plain, inline)]

On Thu, 2020-06-18 at 14:43 +0200, Ludovic Courtès wrote:
> Hi Dale,
> 
> Dale Mellor <guix-devel-0brg6b <at> rdmp.org> skribis:
> 
> > From 36c628cd27c2c6f281ec800767ba0e514fae13b1 Mon Sep 17 00:00:00 2001
> > From: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
> > Date: Wed, 17 Jun 2020 12:42:54 +0100
> > Subject: [PATCH] gnu:  Add curlpp library.
> > 
> > * gnu/packages/curl.scm (curlpp): New variable.
> 
> Nice!  A couple of comments:
> ...


Hi Ludo, I took all your comments on board; the attached patch replaces
the one I sent earlier.

Thanks,
Dale

[a.patch (text/x-patch, attachment)]

Message #14 received at 41919 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <marius <at> gnu.org>
To: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>, 41919 <at> debbugs.gnu.org
Subject: Re: [bug#41919] [patch] Add curlpp C++ bindings to curl package
Date: Wed, 24 Jun 2020 20:58:24 +0200

[Message part 1 (text/plain, inline)]

Hello Dale,

Thanks for the updated patch.  I will suggest another few improvements,
then I think it should be good to go.  :-)

Dale Mellor <guix-devel-0brg6b <at> rdmp.org> writes:

> From b1506404e27552a171fe535144b2d11b6aa67cfa Mon Sep 17 00:00:00 2001
> From: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
> Date: Sun, 21 Jun 2020 23:53:07 +0100
> Subject: [PATCH] gnu: Add curlpp library.
>
> * gnu/packages/curl.scm (curlpp): New variable.

[...]

> @@ -32,10 +33,12 @@
>    #:use-module (guix download)
>    #:use-module (guix git-download)
>    #:use-module (guix utils)
> +  #:use-module (guix build-system cmake)
>    #:use-module (guix build-system gnu)
>    #:use-module (guix build-system go)
>    #:use-module (gnu packages)
>    #:use-module (gnu packages compression)
> +  #:use-module (gnu packages gawk)

This import is unused.

>    #:use-module (gnu packages golang)
>    #:use-module (gnu packages guile)
>    #:use-module (gnu packages kerberos)
> @@ -258,3 +261,33 @@ not offer a replacement for libcurl.")
>  Guile to do client-side URL transfers, like requesting documents from HTTP or
>  FTP servers.  It is based on the curl library.")
>     (license license:gpl3+)))
> +
> +(define-public  curlpp
                 ^^
Extra space _____|

> +  (package
> +   (name "curlpp")
> +   (version "0.8.1")
> +   (source
> +    (origin
> +             (method  git-fetch)

What happened with the indentation here?  Also, there is an extra space
between 'method' and 'git-fetch'.

> +     (uri
> +      (git-reference

Place git-reference on the same line as uri.

> +                   (url "https://github.com/jpbarrette/curlpp.git")
> +                   (commit "v0.8.1")))

Use (string-append "v" version) here instead of hard-coding.  Also, you
should add a (file-name (git-file-name name version)) in this vicinity.

> +             (sha256
> +              (base32 "1b0ylnnrhdax4kwjq64r1fk0i24n5ss6zfzf4hxwgslny01xiwrk"))))
> +   (build-system  cmake-build-system)
                   ^^
Another extra space.

> +   ;; There are no build tests to be had.
> +   (arguments
> +    '(#:tests? #f))
> +   ;; The installed version needs the header files from the C library.
> +   (propagated-inputs
> +    `(("curl" ,curl)))
> +   (synopsis  "C++ wrapper around libcURL")
               ^^
Here too ______|

> +   (description
> +    "A free and easy-to-use client-side C++ URL transfer library,

Please use full sentences in the description.  In particular, start with
"This package provides a ..." instead of jumping to the "A ...".

> +supporting FTP, FTPS, HTTP, HTTPS, GOPHER, TELNET, DICT, FILE and LDAP; in
> +particular it supports HTTPS certificates, HTTP POST, HTTP PUT, FTP uploading,
> +kerberos, HTTP form based upload, proxies, cookies, user+password
> +authentication, file transfer resume, http proxy tunneling and more!")
> +   (home-page  "http://www.curlpp.org")
> +   (license  license:x11-style)))

There are extra spaces after 'home-page' and 'license'.  Also,
license:x11-style is a procedure that takes two arguments: an URI and
optionally a comment.  However reading the source files, it looks like
the standard "Expat" license, so you can use license:expat instead.

Can you send an updated patch?  TIA!

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 41919 <at> debbugs.gnu.org (full text, mbox):

From: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
To: Marius Bakke <marius <at> gnu.org>, 41919 <41919 <at> debbugs.gnu.org>
Subject: Re: [bug#41919] [patch] Add curlpp C++ bindings to curl package
Date: Sun, 28 Jun 2020 22:43:27 +0100

[Message part 1 (text/plain, inline)]

On Wed, 2020-06-24 at 20:58 +0200, Marius Bakke wrote:
> Hello Dale,
> 
> Thanks for the updated patch.  I will suggest another few improvements,
> then I think it should be good to go.  :-)


  All suggestions taken on board.

Thank you for checking over it,
Dale

[0001-gnu-Add-curlpp-library.patch (text/x-patch, attachment)]

Message #22 received at 41919-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
Cc: 41919 <41919-done <at> debbugs.gnu.org>, Marius Bakke <marius <at> gnu.org>
Subject: Re: [bug#41919] [patch] Add curlpp C++ bindings to curl package
Date: Fri, 03 Jul 2020 23:48:58 +0200

Hi Dale,

Dale Mellor <guix-devel-0brg6b <at> rdmp.org> skribis:

> From 16bff1a7c18ead5bb923f2b75e38371718660abb Mon Sep 17 00:00:00 2001
> From: Dale Mellor <guix-devel-0brg6b <at> rdmp.org>
> Date: Sun, 21 Jun 2020 23:53:07 +0100
> Subject: [PATCH] gnu: Add curlpp library.
>
> * gnu/packages/curl.scm (curlpp): New variable.

Applied, thanks!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.