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.

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.

On Mon, Jun 15, 2020 at 8:04 AM Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > 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. My reasoning was that the 2-clause license is preferred for new code (at least in every other community where I've active), and this plugin is new code. But I can add the 3rd clause back if libguestfs likes it.

On Mon, Jun 15, 2020 at 8:04 AM Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > 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 > My reasoning was that the 2-clause license is preferred for new code (at least in every other community where I've active), and this plugin is new code. But I can add the 3rd clause back if libguestfs likes it.