Tomáš Golembiovský
2016-Aug-09 11:55 UTC
Re: [Libguestfs] [PATCH 2/8] v2v: add basic support for the "deb" package manager
On Mon, 8 Aug 2016 18:38:49 +0200 Pino Toscano <ptoscano@redhat.com> wrote:> Implement the 'remove', 'file_list_of_package', and 'file_owner' methods > of the Linux module for the "deb" package manager (dpkg basically, on > Debian and derived distributions). > > Also allow it for the main conversion code. > --- > v2v/convert_linux.ml | 2 +- > v2v/linux.ml | 23 +++++++++++++++++++++++ > 2 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml > index 4b1ce99..65796d6 100644 > --- a/v2v/convert_linux.ml > +++ b/v2v/convert_linux.ml > @@ -79,7 +79,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > | "sles" | "suse-based" | "opensuse" -> `SUSE_family > | _ -> assert false in > > - assert (inspect.i_package_format = "rpm"); > + assert (inspect.i_package_format = "rpm" || inspect.i_package_format = "deb"); > > (* We use Augeas for inspection and conversion, so initialize it early. *) > Linux.augeas_init g; > diff --git a/v2v/linux.ml b/v2v/linux.ml > index ed639c1..5713f64 100644 > --- a/v2v/linux.ml > +++ b/v2v/linux.ml > @@ -48,6 +48,10 @@ and do_remove g inspect packages > assert (List.length packages > 0); > let package_format = inspect.i_package_format in > match package_format with > + | "deb" -> > + let cmd = [ "dpkg"; "--purge" ] @ packages in > + let cmd = Array.of_list cmd in > + ignore (g#command cmd); > | "rpm" -> > let cmd = [ "rpm"; "-e" ] @ packages in > let cmd = Array.of_list cmd in > @@ -61,6 +65,12 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app > let package_format = inspect.i_package_format in > > match package_format with > + | "deb" -> > + let cmd = [| "dpkg"; "-L"; app.G.app2_name |] in > + debug "%s" (String.concat " " (Array.to_list cmd)); > + let files = g#command_lines cmd in > + let files = Array.to_list files in > + List.sort compare files > | "rpm" -> > (* Since RPM allows multiple packages installed with the same > * name, always check the full ENVR here (RHBZ#1161250). > @@ -98,6 +108,19 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app > let rec file_owner (g : G.guestfs) inspect path > let package_format = inspect.i_package_format in > match package_format with > + | "deb" -> > + let cmd = [| "dpkg"; "-S"; path |] in > + debug "%s" (String.concat " " (Array.to_list cmd)); > + let lines > + try g#command_lines cmdThis is not good. dpkg-query command behaves differently from rpm. First, the returned packages are all on one line separated by commas. What I came up with is this: let cmd = [| "dpkg-query"; "-S"; path |] in debug "%s" (String.concat " " (Array.to_list cmd)); (try let pkg = g#command cmd in (* What we get is a string of form "pkg1, pkg2:arch, pkg3: /path" *) let len = String.length pkg in let rec loop i if i >= len then (* This is fishy. Internal bug? *) error (f_"internal error: file_owner: failed to process string '%s'") pkg else if pkg.[i] = ' ' && (pkg.[i-1] = ':' || pkg.[i-1] = ',') then String.sub pkg 0 (i-1) else loop (i+1) in loop 1 with Guestfs.Error msg as exn -> if String.find msg "no path found matching pattern" >= 0 then raise Not_found else raise exn ) The other concern is, whether we should escape the argument somehow. The fact is that dpkg-query accepts a glob expression, not just a simple path. If you provide it with a pattern you will get all the matching paths, each on a single line (prefixed by the package list). This might not be an issue. Depends on how we use the method. Thoughts?> + with Guestfs.Error msg as exn -> > + if String.find msg "no path found matching pattern" >= 0 then > + raise Not_found > + else > + raise exn in > + if Array.length lines = 0 then > + error (f_"internal error: file_owner: dpkg command returned no output"); > + lines.(0) > | "rpm" -> > (* Although it is possible in RPM for multiple packages to own > * a file, this deliberately only returns one package.-- Tomáš Golembiovský <tgolembi@redhat.com>
Pino Toscano
2016-Aug-09 13:24 UTC
Re: [Libguestfs] [PATCH 2/8] v2v: add basic support for the "deb" package manager
On Tuesday, 9 August 2016 13:55:36 CEST Tomáš Golembiovský wrote:> On Mon, 8 Aug 2016 18:38:49 +0200 > Pino Toscano <ptoscano@redhat.com> wrote: > > > Implement the 'remove', 'file_list_of_package', and 'file_owner' methods > > of the Linux module for the "deb" package manager (dpkg basically, on > > Debian and derived distributions). > > > > Also allow it for the main conversion code. > > --- > > v2v/convert_linux.ml | 2 +- > > v2v/linux.ml | 23 +++++++++++++++++++++++ > > 2 files changed, 24 insertions(+), 1 deletion(-) > > > > diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml > > index 4b1ce99..65796d6 100644 > > --- a/v2v/convert_linux.ml > > +++ b/v2v/convert_linux.ml > > @@ -79,7 +79,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > > | "sles" | "suse-based" | "opensuse" -> `SUSE_family > > | _ -> assert false in > > > > - assert (inspect.i_package_format = "rpm"); > > + assert (inspect.i_package_format = "rpm" || inspect.i_package_format = "deb"); > > > > (* We use Augeas for inspection and conversion, so initialize it early. *) > > Linux.augeas_init g; > > diff --git a/v2v/linux.ml b/v2v/linux.ml > > index ed639c1..5713f64 100644 > > --- a/v2v/linux.ml > > +++ b/v2v/linux.ml > > @@ -48,6 +48,10 @@ and do_remove g inspect packages > > assert (List.length packages > 0); > > let package_format = inspect.i_package_format in > > match package_format with > > + | "deb" -> > > + let cmd = [ "dpkg"; "--purge" ] @ packages in > > + let cmd = Array.of_list cmd in > > + ignore (g#command cmd); > > | "rpm" -> > > let cmd = [ "rpm"; "-e" ] @ packages in > > let cmd = Array.of_list cmd in > > @@ -61,6 +65,12 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app > > let package_format = inspect.i_package_format in > > > > match package_format with > > + | "deb" -> > > + let cmd = [| "dpkg"; "-L"; app.G.app2_name |] in > > + debug "%s" (String.concat " " (Array.to_list cmd)); > > + let files = g#command_lines cmd in > > + let files = Array.to_list files in > > + List.sort compare files > > | "rpm" -> > > (* Since RPM allows multiple packages installed with the same > > * name, always check the full ENVR here (RHBZ#1161250). > > @@ -98,6 +108,19 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app > > let rec file_owner (g : G.guestfs) inspect path > > let package_format = inspect.i_package_format in > > match package_format with > > + | "deb" -> > > + let cmd = [| "dpkg"; "-S"; path |] in > > + debug "%s" (String.concat " " (Array.to_list cmd)); > > + let lines > > + try g#command_lines cmd > > This is not good. dpkg-query command behaves differently from rpm.AFAIR, dpkg doesn't allow a file to be owned by different packages (and even if it does, it will complain hard when the file is not exactly the same in all the packages). This is not what happens with directories, which can be owned by every package (and indeed usually every debian package owns all the directories, / included, that contain the shipped files). There could be also multiarch to take into account, but that's something that could be dealt with later.> First, the returned packages are all on one line separated by commas. > What I came up with is this: > > let cmd = [| "dpkg-query"; "-S"; path |] in > debug "%s" (String.concat " " (Array.to_list cmd)); > (try > let pkg = g#command cmd in > (* What we get is a string of form "pkg1, pkg2:arch, pkg3: /path" *) > let len = String.length pkg in > let rec loop i > if i >= len then > (* This is fishy. Internal bug? *) > error (f_"internal error: file_owner: failed to process string '%s'") pkg > else if pkg.[i] = ' ' && > (pkg.[i-1] = ':' || pkg.[i-1] = ',') then > String.sub pkg 0 (i-1) > else loop (i+1) > in loop 1 > with Guestfs.Error msg as exn -> > if String.find msg "no path found matching pattern" >= 0 then > raise Not_found > else > raise exn > ) > > The other concern is, whether we should escape the argument somehow. The > fact is that dpkg-query accepts a glob expression, not just a simple > path. If you provide it with a pattern you will get all the matching > paths, each on a single line (prefixed by the package list).rpm seems to accept glob expression too -- OTOH we seem to provide only package names to Linux.remove calls (which were read from the package manager itself). -- Pino Toscano
Tomáš Golembiovský
2016-Aug-09 14:43 UTC
Re: [Libguestfs] [PATCH 2/8] v2v: add basic support for the "deb" package manager
On Tue, 09 Aug 2016 15:24:21 +0200 Pino Toscano <ptoscano@redhat.com> wrote:> On Tuesday, 9 August 2016 13:55:36 CEST Tomáš Golembiovský wrote: > > On Mon, 8 Aug 2016 18:38:49 +0200 > > Pino Toscano <ptoscano@redhat.com> wrote: > > > > > Implement the 'remove', 'file_list_of_package', and 'file_owner' methods > > > of the Linux module for the "deb" package manager (dpkg basically, on > > > Debian and derived distributions). > > > > > > Also allow it for the main conversion code. > > > --- > > > v2v/convert_linux.ml | 2 +- > > > v2v/linux.ml | 23 +++++++++++++++++++++++ > > > 2 files changed, 24 insertions(+), 1 deletion(-) > > > > > > diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml > > > index 4b1ce99..65796d6 100644 > > > --- a/v2v/convert_linux.ml > > > +++ b/v2v/convert_linux.ml > > > @@ -79,7 +79,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > > > | "sles" | "suse-based" | "opensuse" -> `SUSE_family > > > | _ -> assert false in > > > > > > - assert (inspect.i_package_format = "rpm"); > > > + assert (inspect.i_package_format = "rpm" || inspect.i_package_format = "deb"); > > > > > > (* We use Augeas for inspection and conversion, so initialize it early. *) > > > Linux.augeas_init g; > > > diff --git a/v2v/linux.ml b/v2v/linux.ml > > > index ed639c1..5713f64 100644 > > > --- a/v2v/linux.ml > > > +++ b/v2v/linux.ml > > > @@ -48,6 +48,10 @@ and do_remove g inspect packages > > > assert (List.length packages > 0); > > > let package_format = inspect.i_package_format in > > > match package_format with > > > + | "deb" -> > > > + let cmd = [ "dpkg"; "--purge" ] @ packages in > > > + let cmd = Array.of_list cmd in > > > + ignore (g#command cmd); > > > | "rpm" -> > > > let cmd = [ "rpm"; "-e" ] @ packages in > > > let cmd = Array.of_list cmd in > > > @@ -61,6 +65,12 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app > > > let package_format = inspect.i_package_format in > > > > > > match package_format with > > > + | "deb" -> > > > + let cmd = [| "dpkg"; "-L"; app.G.app2_name |] in > > > + debug "%s" (String.concat " " (Array.to_list cmd)); > > > + let files = g#command_lines cmd in > > > + let files = Array.to_list files in > > > + List.sort compare files > > > | "rpm" -> > > > (* Since RPM allows multiple packages installed with the same > > > * name, always check the full ENVR here (RHBZ#1161250). > > > @@ -98,6 +108,19 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app > > > let rec file_owner (g : G.guestfs) inspect path > > > let package_format = inspect.i_package_format in > > > match package_format with > > > + | "deb" -> > > > + let cmd = [| "dpkg"; "-S"; path |] in > > > + debug "%s" (String.concat " " (Array.to_list cmd)); > > > + let lines > > > + try g#command_lines cmd > > > > This is not good. dpkg-query command behaves differently from rpm. > > AFAIR, dpkg doesn't allow a file to be owned by different packages > (and even if it does, it will complain hard when the file is not exactly > the same in all the packages).Good point. I take it then that the function should focus only on files (as the name suggests).> This is not what happens with directories, which can be owned by every > package (and indeed usually every debian package owns all the > directories, / included, that contain the shipped files). > > There could be also multiarch to take into account, but that's something > that could be dealt with later.Still the fact remains, that the returned string contains not just a package name, but also the path itself. I.e: "package: /some/file"> > > First, the returned packages are all on one line separated by commas. > > What I came up with is this: > > > > let cmd = [| "dpkg-query"; "-S"; path |] in > > debug "%s" (String.concat " " (Array.to_list cmd)); > > (try > > let pkg = g#command cmd in > > (* What we get is a string of form "pkg1, pkg2:arch, pkg3: /path" *) > > let len = String.length pkg in > > let rec loop i > > if i >= len then > > (* This is fishy. Internal bug? *) > > error (f_"internal error: file_owner: failed to process string '%s'") pkg > > else if pkg.[i] = ' ' && > > (pkg.[i-1] = ':' || pkg.[i-1] = ',') then > > String.sub pkg 0 (i-1) > > else loop (i+1) > > in loop 1 > > with Guestfs.Error msg as exn -> > > if String.find msg "no path found matching pattern" >= 0 then > > raise Not_found > > else > > raise exn > > ) > > > > The other concern is, whether we should escape the argument somehow. The > > fact is that dpkg-query accepts a glob expression, not just a simple > > path. If you provide it with a pattern you will get all the matching > > paths, each on a single line (prefixed by the package list). > > rpm seems to accept glob expression too -- OTOH we seem to provide only > package names to Linux.remove calls (which were read from the package > manager itself). > > -- > Pino Toscano-- Tomáš Golembiovský <tgolembi@redhat.com>
Apparently Analagous Threads
- [PATCH 2/8] v2v: add basic support for the "deb" package manager
- Re: [PATCH 2/8] v2v: add basic support for the "deb" package manager
- Re: [PATCH 2/8] v2v: add basic support for the "deb" package manager
- [PATCH 2/7] v2v: add basic support for the "deb" package manager
- [PATCH] v2v: Fix RPM file owned test (RHBZ#1503958).