On Mon, Jun 26, 2023 at 08:02:08AM +0000, Tage Johansson wrote:
diff --git a/generator/generator.ml b/generator/generator.ml
index c73824e..18856f5 100644
--- a/generator/generator.ml
+++ b/generator/generator.ml
@@ -61,3 +61,5 @@ let () =
output_to "golang/closures.go" GoLang.generate_golang_closures_go;
output_to "golang/wrappers.go" GoLang.generate_golang_wrappers_go;
output_to "golang/wrappers.h" GoLang.generate_golang_wrappers_h;
+
+ output_to ~rustfmt:true "rust/src/bindings.rs" Rust.generate_rust_bindings;
diff --git a/generator/utils.ml b/generator/utils.ml
index 3a96929..3a1cc9f 100644
--- a/generator/utils.ml
+++ b/generator/utils.ml
@@ -413,7 +413,7 @@ let files_equal n1 n2 =
| 1 -> false
| i -> failwithf "%s: failed with error code %d" cmd i
-let output_to filename k =
+let output_to ?(rustfmt = false) filename k =
I feel the addition of ~rustfmt might even be a separate patch, and as
discussed previously it could be a generic "~format" parameter taking
various values like RustFmt | GoFmt | Indent. It would allow this to
go upstream sooner.
lineno := 1; col := 0;
let filename_new = filename ^ ".new" in
let c = open_out filename_new in
@@ -422,6 +422,13 @@ let output_to filename k =
close_out c;
chan := NoOutput;
+ if rustfmt then
+ (match system (sprintf "rustfmt %s" filename_new) with
+ | WEXITED 0 -> ()
+ | WEXITED i -> failwith (sprintf "Rustfmt failed with exit code %d"
i)
+ | _ -> failwith "Rustfmt was killed or stopped by a signal.");
+
+
Try to avoid adding extra whitespace lines such as here.
(* Is the new file different from the current file? *)
if Sys.file_exists filename && files_equal filename filename_new then
unlink filename_new (* same, so skip it *)
diff --git a/generator/utils.mli b/generator/utils.mli
index b4a2525..7489fe0 100644
--- a/generator/utils.mli
+++ b/generator/utils.mli
@@ -50,7 +50,8 @@ val files_equal : string -> string -> bool
val generate_header : ?extra_sources:string list -> comment_style -> unit
-val output_to : string -> (unit -> 'a) -> unit
+(** Redirect stdout to a file. If `rustfmt` is true, will format the text with rustfmt.
*)
And this line is too long.
+EXTRA_DIST = \
+ $(generator_built) \
+ .gitignore \
+ Cargo.toml \
+ src/lib.rs \
+ src/error.rs \
+ src/handle.rs \
+ src/types.rs \
+ src/utils.rs \
+ libnbd-sys/Cargo.toml \
+ libnbd-sys/build.rs \
+ libnbd-sys/wrapper.h \
+ libnbd-sys/src/lib.rs \
+ $(NULL)
I think I asked last time why there are two copies of some files, in
this case src/lib.rs. Are they the same file?
As this is just a file in the tarball, it's not too concerning. Maybe
Rust has a good reason for it, although I've not seen it need this
before.
If the file was duplicated in git (which I don't think it is) then
it'd be more serious and we'd reject the change.
+if HAVE_RUST
+
+all-local: $(source_files)
+ rm -f libnbd-sys/libnbd_version.t
+ $(abs_top_builddir)/run echo $(VERSION) > libnbd-sys/libnbd_version.t
+ mv libnbd-sys/libnbd_version.t libnbd-sys/libnbd_version
Yes, this is a safer way to generate files atomically from 'make'.
diff --git a/rust/cargo_test/src/lib.rs b/rust/cargo_test/src/lib.rs
new file mode 100644
index 0000000..a5cbb84
--- /dev/null
+++ b/rust/cargo_test/src/lib.rs
@@ -0,0 +1,31 @@
+// nbd client library in userspace
+// Copyright Tage Johansson
+//
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+/// A dummy test function which adds one to an 32-bit integer.
+pub fn add_one(i: i32) -> i32 {
+ i + 1
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn test_add_one() {
+ assert_eq!(add_one(42), 43);
+ }
+}
So I see it's a different file. I'm not sure why it's needed though.
[I only reviewed down to this point]
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW