On Sat, 24 Jun 2017 13:26:48 -0300
Adonay Felipe Nogueira <adfeno@openmailbox.org> wrote:

> Here follows a series of patches, it's part of the attempt to package
> GNU Ring. However, these contributions don't need custom patches from
> Savir Faire Linux. The only doubt is in regards to the need to have a
> variant of msgpack that uses C++ 11 instead, and have a variant of
> opendht that makes use of such msgpack variant instead.

I've just noticed that no one has taken a look at this yet! Not quite
sure how that has happened. Here are my comments.

The first general comment is that these patches don't quite apply now,
so it would be useful to send another set which do apply cleanly.

Starting with the kashmir package.

> +     (source (origin
> +	      (method url-fetch)
> +	      (uri (string-append "https://github.com/Corvusoft/"
> name
> +				  "-dependency/archive/" commit
> ".tar.gz"))
> +	      (file-name (string-append name "-" version ".tar.gz"))
> +	      (sha256
> +	       (base32
> +		"11hbh3d7w74gng9qgz3hdrzvxjgil1pymk2bzxv1ajh9vp60xxs9"))))

Looks like this patch may have got caught up in the recent hash changes
from GitHub. I had to change the sha256 before this would build.

The directory structure in this package looks a little odd. The Debian
package has kashmir-dependency/kashmir as include/kashmir, which might
be a little more usual.

guix lint also reports some issues:

/home/chris/.config/guix/latest/gnu/packages/cpp.scm:36:4: kashmir@0-1.2f3913f: tabulation on line 39, column 0
+ more tab issues
/home/chris/.config/guix/latest/gnu/packages/cpp.scm:36:4: kashmir@0-1.2f3913f: tabulation on line 71, column 0
/home/chris/.config/guix/latest/gnu/packages/cpp.scm:36:4: kashmir@0-1.2f3913f: line 72 is way too long (98 characters)
/home/chris/.config/guix/latest/gnu/packages/cpp.scm:36:4: kashmir@0-1.2f3913f: line 73 is way too long (176 characters)


As for the crypto++ package.

guix lint reports lots of tabs.

> +		       (add-after 'build 'build-shared
> +				  (lambda _
> +				    ;; By default, only the static
> +				    ;; library is built.
> +				    (zero?
> +				     (system* "make" "shared"))))

I think you could get rid of this phase by adding "shared" to the
#:make-flags.

I'm not sure why the binary is has .exe in the name? Even
the inbuilt help calls it cryptest:

  Unrecognized command. Run "cryptest h" to obtain usage information.

I have no idea what this does, but running it in a container fails, as
it seems to be looking for data in the doc output:

  → guix environment --container --ad-hoc crypto++:bin -- cryptest.exe
  CryptoPP::Exception caught: FileStore: error opening file for reading:
 TestData/usage.dat


On to the opendht update.

> +  (let ((release "1.3.3")
> +	(revision 5))

I'm not why this revision is being used?

Also, with the msgpack-c++-11 package, I built both msgpack and
msgpack-c++-11 and diffed the contents with diffoscope, and they are
practically identical as far as I can see. I'm guessing there was
supposed to be a difference?


Last but not least, for the argon2 package, for which I don't have any
questions or suggestions. Ignoring the tabs, I think it looks fine.

I hope this helps Adonay :)