[PATCH libguestfs] daemon: Omit 'file -S' option on older distros that lack support
OpenSUSE LEAP 15 lacks support for this option, so test for it before using it. See-also: https://listman.redhat.com/archives/libguestfs/2023-September/032613.html Report-by: Olaf Hering Fixes: commit 23986d3c4f4d1f9cbac44cc743d3e6af721e4237 --- daemon/Makefile.am | 2 ++ daemon/file.ml | 10 ++++++++-- daemon/file_helper.ml | 29 +++++++++++++++++++++++++++++ daemon/file_helper.mli | 19 +++++++++++++++++++ daemon/filearch.ml | 5 ++++- 5 files changed, 62 insertions(+), 3 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index bb2e58d014..01c0f6416c 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -280,6 +280,7 @@ SOURCES_MLI = \ devsparts.mli \ file.mli \ filearch.mli \ + file_helper.mli \ findfs.mli \ inspect.mli \ inspect_fs.mli \ @@ -321,6 +322,7 @@ SOURCES_ML = \ btrfs.ml \ cryptsetup.ml \ devsparts.ml \ + file_helper.ml \ file.ml \ filearch.ml \ isoinfo.ml \ diff --git a/daemon/file.ml b/daemon/file.ml index 1f87b190c1..f0ef181938 100644 --- a/daemon/file.ml +++ b/daemon/file.ml @@ -43,7 +43,10 @@ let file path = | S_SOCK -> "socket" | S_REG -> (* Regular file, so now run [file] on it. *) - let out = command "file" ["-zSb"; Sysroot.sysroot_path path] in + let file_options = + sprintf "-z%sb" + (if File_helper.file_has_S_option () then "S" else "") in + let out = command "file" [file_options; Sysroot.sysroot_path path] in (* We need to remove the trailing \n from output of file(1). * @@ -54,6 +57,9 @@ let file path = String.trimr out ) else (* it's a device *) ( - let out = command "file" ["-zSbsL"; path] in + let file_options = + sprintf "-z%sbsL" + (if File_helper.file_has_S_option () then "S" else "") in + let out = command "file" [file_options; path] in String.trimr out ) diff --git a/daemon/file_helper.ml b/daemon/file_helper.ml new file mode 100644 index 0000000000..f8c4bcbe56 --- /dev/null +++ b/daemon/file_helper.ml @@ -0,0 +1,29 @@ +(* guestfs-inspection + * Copyright (C) 2009-2023 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +open Std_utils + +(* Does [file] support the [-S] / [--no-sandbox] option + * (not on OpenSUSE LEAP 15). + *) +let file_has_S_option = lazy ( + let out = Utils.command "file" ["file"; "--help"] in + String.find out "--no-sandbox" >= 0 + +) +let file_has_S_option () = Lazy.force file_has_S_option diff --git a/daemon/file_helper.mli b/daemon/file_helper.mli new file mode 100644 index 0000000000..a644cf6de2 --- /dev/null +++ b/daemon/file_helper.mli @@ -0,0 +1,19 @@ +(* guestfs-inspection + * Copyright (C) 2009-2023 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +val file_has_S_option : unit -> bool diff --git a/daemon/filearch.ml b/daemon/filearch.ml index 7c858129db..cf784f18a2 100644 --- a/daemon/filearch.ml +++ b/daemon/filearch.ml @@ -128,7 +128,10 @@ and cpio_arch magic orig_path path = | bin :: bins -> let bin_path = tmpdir // bin in if is_regular_file bin_path then ( - let out = command "file" ["-zSb"; bin_path] in + let file_options = + sprintf "-z%sb" + (if File_helper.file_has_S_option () then "S" else "") in + let out = command "file" [file_options; bin_path] in file_architecture_of_magic out orig_path bin_path ) else -- 2.41.0
On 9/21/23 13:42, Richard W.M. Jones wrote:
OpenSUSE LEAP 15 lacks support for this option, so test for it before using it.
See-also: https://listman.redhat.com/archives/libguestfs/2023-September/032613.html Report-by: Olaf Hering Fixes: commit 23986d3c4f4d1f9cbac44cc743d3e6af721e4237 --- daemon/Makefile.am | 2 ++ daemon/file.ml | 10 ++++++++-- daemon/file_helper.ml | 29 +++++++++++++++++++++++++++++ daemon/file_helper.mli | 19 +++++++++++++++++++ daemon/filearch.ml | 5 ++++- 5 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/daemon/Makefile.am b/daemon/Makefile.am index bb2e58d014..01c0f6416c 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -280,6 +280,7 @@ SOURCES_MLI = \ devsparts.mli \ file.mli \ filearch.mli \ + file_helper.mli \ findfs.mli \ inspect.mli \ inspect_fs.mli \ @@ -321,6 +322,7 @@ SOURCES_ML = \ btrfs.ml \ cryptsetup.ml \ devsparts.ml \ + file_helper.ml \ file.ml \ filearch.ml \ isoinfo.ml \ diff --git a/daemon/file.ml b/daemon/file.ml index 1f87b190c1..f0ef181938 100644 --- a/daemon/file.ml +++ b/daemon/file.ml @@ -43,7 +43,10 @@ let file path = | S_SOCK -> "socket" | S_REG -> (* Regular file, so now run [file] on it. *) - let out = command "file" ["-zSb"; Sysroot.sysroot_path path] in + let file_options = + sprintf "-z%sb" + (if File_helper.file_has_S_option () then "S" else "") in + let out = command "file" [file_options; Sysroot.sysroot_path path] in
(* We need to remove the trailing \n from output of file(1). *
This still open-codes "-z" and "-b" in multiple places, plus the file_has_S_option() call; I thought of a somewhat thicker wrapper (hiding all the details / options behind the "file" and "file-architecture" APIs). But, this works as well of course.
@@ -54,6 +57,9 @@ let file path = String.trimr out ) else (* it's a device *) ( - let out = command "file" ["-zSbsL"; path] in + let file_options = + sprintf "-z%sbsL" + (if File_helper.file_has_S_option () then "S" else "") in + let out = command "file" [file_options; path] in String.trimr out ) diff --git a/daemon/file_helper.ml b/daemon/file_helper.ml new file mode 100644 index 0000000000..f8c4bcbe56 --- /dev/null +++ b/daemon/file_helper.ml @@ -0,0 +1,29 @@ +(* guestfs-inspection
"guestfs-inspection" -- strange, but I see it in a bunch of other places.
+ * Copyright (C) 2009-2023 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +open Std_utils + +(* Does [file] support the [-S] / [--no-sandbox] option + * (not on OpenSUSE LEAP 15). + *) +let file_has_S_option = lazy ( + let out = Utils.command "file" ["file"; "--help"] in
I think this executes "file file --help", which happens to work, but the intent is probably just "file --help". (IOW, IMO this should be Utils.command "file" ["--help"] .)
+ String.find out "--no-sandbox" >= 0 + +) +let file_has_S_option () = Lazy.force file_has_S_option diff --git a/daemon/file_helper.mli b/daemon/file_helper.mli new file mode 100644 index 0000000000..a644cf6de2 --- /dev/null +++ b/daemon/file_helper.mli @@ -0,0 +1,19 @@ +(* guestfs-inspection + * Copyright (C) 2009-2023 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +val file_has_S_option : unit -> bool diff --git a/daemon/filearch.ml b/daemon/filearch.ml index 7c858129db..cf784f18a2 100644 --- a/daemon/filearch.ml +++ b/daemon/filearch.ml @@ -128,7 +128,10 @@ and cpio_arch magic orig_path path = | bin :: bins -> let bin_path = tmpdir // bin in if is_regular_file bin_path then ( - let out = command "file" ["-zSb"; bin_path] in + let file_options = + sprintf "-z%sb" + (if File_helper.file_has_S_option () then "S" else "") in + let out = command "file" [file_options; bin_path] in file_architecture_of_magic out orig_path bin_path ) else
Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo
On Thu, Sep 21, 2023 at 03:56:05PM +0200, Laszlo Ersek wrote: ...
+open Std_utils + +(* Does [file] support the [-S] / [--no-sandbox] option + * (not on OpenSUSE LEAP 15). + *) +let file_has_S_option = lazy ( + let out = Utils.command "file" ["file"; "--help"] in
I think this executes "file file --help", which happens to work, but the intent is probably just "file --help". (IOW, IMO this should be
Utils.command "file" ["--help"]
.)
Indeed it does. BTW I did check this patch by running: $ make && LIBGUESTFS_DEBUG=1 LIBGUESTFS_TRACE=1 make -C tests/ check TESTS=c-api/tests and examining tests/c-api/tests.log, and that shows that file --help (or rather, "file file --help") is being run exactly once, and the -S option was still being included in the parameters.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks, I'll tidy up and fix the things you mentioned. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
This is upstream in c95d8c4cf64142. Also added a few commits to fix the Ruby tests which had broken because of a recent change to Ruby. I'll cut a development release soon. 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
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones