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

2017-07-07 Thread Roger Pau Monne
On Thu, Jul 06, 2017 at 04:12:08PM +0100, Ian Jackson wrote:
> 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.

I've changed it to:

sub package_install_cmd {
my ($norec) = @_;
my @hostflags = get_hostflags('host');
my @cmd;

if (grep /^freebsd\b/i, @hostflags) {
push @cmd, qw(lockf /var/run/osstest-pkg-lock pkg-static install));
} else {
push @cmd, qw(DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y
  with-lock-ex -w /var/lock/osstest-apt apt-get);
if ($norec)
push @cmd, qw(--no-install-recommends);
push @cmd, qw(-y install);
}

return @cmd;
}
sub target_install_packages {
my ($ho, @packages) = @_;
my @cmd = package_install_cmd();

target_cmd_root($ho,"@cmd @packages", 3000);
}
sub target_install_packages_norec {
my ($ho, @packages) = @_;
my @cmd = package_install_cmd(1);

target_cmd_root($ho,"@cmd @packages", 3000);
}

(will test shortly). Let me know if that looks better.

Thanks, Roger.

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


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

2017-07-06 Thread Ian Jackson
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


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

2017-07-06 Thread Roger Pau Monne
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.

Signed-off-by: Roger Pau Monné 
---
Changes since v3:
 - New in this version.
---
 Osstest/TestSupport.pm | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 5a9a6f8b..94ace09c 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -529,13 +529,30 @@ sub target_run_apt {
 "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y \\
 with-lock-ex -w /var/lock/osstest-apt apt-get @aptopts", 3000);
 }
+sub target_run_pkg {
+my ($ho, @aptopts) = @_;
+target_cmd_root($ho,"lockf /var/run/osstest-pkg-lock pkg-static @aptopts",
+3000);
+}
 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) {
+target_run_pkg($ho, qw(install), @packages);
+} else {
+target_run_apt($ho, qw(-y install), @packages);
+}
 }
 sub target_install_packages_norec {
 my ($ho, @packages) = @_;
-target_run_apt($ho, qw(--no-install-recommends -y install), @packages);
+my @hostflags = get_hostflags('host');
+
+if (grep /^freebsd/i, @hostflags) {
+target_run_pkg($ho, qw(install), @packages);
+} else {
+target_run_apt($ho, qw(--no-install-recommends -y install), @packages);
+}
 }
 
 sub target_somefile_getleaf ($$$) {
-- 
2.11.0 (Apple Git-81)


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