Libguestfs - Aug 2016 - Re: [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 cmd
This 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>

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

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>