Libguestfs - Jun 2020 - Re: New Rust bindings for nbdkit

The existing Rust bindings for nbdkit aren't very idiomatic Rust, and they
are missing a lot of features.  So I've rewritten them.  The new bindings
aren't backwards compatible, but I doubt that's a problem.  Most likely,
nobody has tried to use them yet, since the crate hasn't even published to
crates.io.  Please review the attached patch.
-Alan

On Thu, Jun 11, 2020 at 04:19:08PM -0600, alan somers
wrote:
> The existing Rust bindings for nbdkit aren't very idiomatic Rust, and
they
> are missing a lot of features.  So I've rewritten them.  The new
bindings
> aren't backwards compatible, but I doubt that's a problem.  Most
likely,
> nobody has tried to use them yet, since the crate hasn't even published
to
> crates.io.  Please review the attached patch.
> -Alan
Thanks Alan, and sorry for the delayed reply.  It just happened that
this email arrived before a long weekend.

First a note that it was a bit of a manual process for me to apply
this because I think it was prepared using "git diff"(?)  If you use
"git format-patch" or (better) "git send-email" for future
patches
then that's much easier.  However I was able to apply and review it.

I have pushed it, but added a few changes which I will summarise below:

 * Some lines had trailing whitespace, removed.  These are only
   allowed in POD where it is used for verbatim sections.

 * In VERSION docs we usually refer only to stable (even-numbered)
   releases, so I replaced 1.21.9 -> 1.22.

 * Remove plugins/rust/Cargo.toml from .gitignore.

 * Several newly added files were missing from EXTRA_DIST
   (make && make dist && make maintainer-check-extra-dist
   will find these in future).

This will create small rebase problems for you if you've made further
changes, but hopefully nothing that isn't easily fixable.

Other issues:

 * The license removed this clause:

   -// * Neither the name of Red Hat nor the names of its contributors may be
   -// used to endorse or promote products derived from this software without
   -// specific prior written permission.

   I believe this removal simply makes the license even more
   permissive, so that's fine.  However I will check with our legal
   people.  Also you should add license headers to the new files
   plugins/rust/tests/*.rs.  Essentially every file should have a
   license, and correct licensing is very important to us.

 * Although the build works (or doesn't break), make check doesn't
   appear to run the tests.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Mon, Jun 15, 2020 at 7:22 AM Richard W.M. Jones <rjones@redhat.com>
wrote:
> On Thu, Jun 11, 2020 at 04:19:08PM -0600, alan somers wrote:
> > The existing Rust bindings for nbdkit aren't very idiomatic Rust,
and
> they
> > are missing a lot of features.  So I've rewritten them.  The new
bindings
> > aren't backwards compatible, but I doubt that's a problem. 
Most likely,
> > nobody has tried to use them yet, since the crate hasn't even
published
> to
> > crates.io.  Please review the attached patch.
> > -Alan
>
> Thanks Alan, and sorry for the delayed reply.  It just happened that
> this email arrived before a long weekend.
>
> First a note that it was a bit of a manual process for me to apply
> this because I think it was prepared using "git diff"(?)  If you
use
> "git format-patch" or (better) "git send-email" for
future patches
> then that's much easier.  However I was able to apply and review it.
>
> I have pushed it, but added a few changes which I will summarise below:
>
>  * Some lines had trailing whitespace, removed.  These are only
>    allowed in POD where it is used for verbatim sections.
>
>  * In VERSION docs we usually refer only to stable (even-numbered)
>    releases, so I replaced 1.21.9 -> 1.22.
>
>  * Remove plugins/rust/Cargo.toml from .gitignore.
>
>  * Several newly added files were missing from EXTRA_DIST
>    (make && make dist && make maintainer-check-extra-dist
>    will find these in future).
>
> This will create small rebase problems for you if you've made further
> changes, but hopefully nothing that isn't easily fixable.
>
> Other issues:
>
>  * The license removed this clause:
>
>    -// * Neither the name of Red Hat nor the names of its contributors may
> be
>    -// used to endorse or promote products derived from this software
> without
>    -// specific prior written permission.
>
>    I believe this removal simply makes the license even more
>    permissive, so that's fine.  However I will check with our legal
>    people.  Also you should add license headers to the new files
>    plugins/rust/tests/*.rs.  Essentially every file should have a
>    license, and correct licensing is very important to us.
>
>  * Although the build works (or doesn't break), make check doesn't
>    appear to run the tests.
>
> Rich.
>
Ok, I can make those changes.  Speaking of "make check", what about
CI?  It
would be pretty easy to add it to this project.  Is there a reason you
haven't done it already?
-Alan

On Mon, Jun 15, 2020 at 02:22:32PM +0100, Richard W.M. Jones
wrote:
> On Thu, Jun 11, 2020 at 04:19:08PM -0600, alan somers wrote:
> > The existing Rust bindings for nbdkit aren't very idiomatic Rust,
and they
> > are missing a lot of features.  So I've rewritten them.  The new
bindings
> > aren't backwards compatible, but I doubt that's a problem. 
Most likely,
> > nobody has tried to use them yet, since the crate hasn't even
published to
> > crates.io.  Please review the attached patch.
> > -Alan
> Other issues:
> 
>  * The license removed this clause:
> 
>    -// * Neither the name of Red Hat nor the names of its contributors may
be
>    -// used to endorse or promote products derived from this software
without
>    -// specific prior written permission.
> 
>    I believe this removal simply makes the license even more
>    permissive, so that's fine.  However I will check with our legal
>    people.  Also you should add license headers to the new files
>    plugins/rust/tests/*.rs.  Essentially every file should have a
>    license, and correct licensing is very important to us.
This change is replacing 3-clause BSD with 2-clause BSD. Shouldn't cuase
any actual difference for consumers, but seems like a needless change to
be making.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|