Re: [PATCH v4 08/16] osstest: add support for the FreeBSD package manager

[Ian Jackson <ian.jackson@eu.citrix.com>]

Roger Pau Monne writes ("[PATCH v4 08/16] osstest: add support for the FreeBSD package manager"):
> FreeBSD support is added to target_install_packages and
> target_install_packages_norec, although there's no equivalent to the
> --no-install-recommends in the FreeBSD package manager.

LGTM, just a couple of quibbles:

> +sub target_run_pkg {
> +    my ($ho, @aptopts) = @_;
> +    target_cmd_root($ho,"lockf /var/run/osstest-pkg-lock pkg-static @aptopts",

This line is rather long.

> +}
>  sub target_install_packages {
>      my ($ho, @packages) = @_;
> -    target_run_apt($ho, qw(-y install), @packages);
> +    my @hostflags = get_hostflags('host');
> +
> +    if (grep /^freebsd/i, @hostflags) {

I would prefer

  +    if (grep /^freebsd\b/i, @hostflags) {
                         ^^

> +        target_run_pkg($ho, qw(install), @packages);
> +    } else {
> +        target_run_apt($ho, qw(-y install), @packages);
> +    }

Also, target_install_packages and target_install_packages_norec are
rather too similar for my taste.  If you can think of a better way of
doing this please suggest one.  If not, then it's tolerable as it is.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel