* [PATCH 0/6] osstest: create a local binary FreeBSD package repository
@ 2019-02-20 16:59 Roger Pau Monne
2019-02-20 16:59 ` [PATCH 1/6] osstest: introduce a helper to stash a whole directory Roger Pau Monne
` (5 more replies)
0 siblings, 6 replies; 35+ messages in thread
From: Roger Pau Monne @ 2019-02-20 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Hello,
In order to reliably run FreeBSD test with the development branch (aka
HEAD) osstest needs it's custom binary package repository that contains
the packages built against the specific version of FreeBSD under test.
FreeBSD HEAD doesn't have any ABI guarantees, and as such binary
packages are tied to the base system used to build them.
This series introduces a new job to the FreeBSD specific flight, that
builds a local binary package repository against the FreeBSD version
under test. Note that this repository only contains the dependencies
required to build Xen. The output of the package building job anointed
together with the installer as a pair, so they are used in conjunction.
Note that the package building job is (like the installer building job)
only used by the FReeBSD specific flight, the output however will be
consumed by other flights, just like the FreeBSD installer.
The runvar changes for a FreeBSD flight are:
+build-amd64-freebsd-packages all_host_os freebsd
+build-amd64-freebsd-packages all_hostflags PropEq:Firmware:bios:bios
+build-amd64-freebsd-packages arch amd64
+build-amd64-freebsd-packages freebsdbuildjob build-amd64-freebsd
+build-amd64-freebsd freebsdpackagesbuildjob 133315.build-amd64-freebsd-packages
+build-amd64-freebsd-again freebsdpackagesbuildjob build-amd64-freebsd-packages
+build-amd64-freebsd-packages freebsdpackagesbuildjob 133315.build-amd64-freebsd-packages
+build-amd64-xen-freebsd freebsdpackagesbuildjob build-amd64-freebsd-packages
+build-amd64-freebsd-packages host_hostflags arch-amd64,purpose-build
+build-amd64-freebsd-packages recipe_skipbuildprep true
+build-amd64-freebsd-packages recipe_testinstall true
+build-amd64-freebsd-packages revision_freebsdports
+build-amd64-freebsd-packages svnrevision_freebsdports 483590
+build-amd64-freebsd-packages svntree_freebsdports https://svn.FreeBSD.org/ports/head
+build-amd64-freebsd-packages tree_freebsdports git://github.com/freebsd/freebsd-ports.git
And for a xen-unstable flight:
+test-amd64-amd64-examine freebsdpackagesbuildjob 133315.build-amd64-freebsd-packages
The following links show the result of a successful FreeBSD flight
including the binary package repository build:
http://logs.test-lab.xenproject.org/osstest/logs/133321/
Note that before applying the series the output of the above flight
should be anointed, so follow up flights can used this output as the
initial seed:
$ ./mg-anoint anoint "freebsd build master amd64" 133321 build-amd64-freebsd \
"freebsd-packages build master amd64" 133321 build-amd64-freebsd-packages
I've pushed the patch series to my git repo:
git://xenbits.xen.org/people/royger/osstest.git freebsd-pkg
Thanks, Roger.
Roger Pau Monne (6):
osstest: introduce a helper to stash a whole directory
osstest: introduce a helper to create a weblink to a directory
osstest: allow to perform multiple anoints in the same transaction
osstest: introduce a helper to get the svn revision of a git commit
osstest: introduce a script to build a FreeBSD package repository
osstest: use a locally built pkg repository for FreeBSD
Osstest/TestSupport.pm | 32 +++++++-
ap-common | 6 ++
ap-fetch-version | 19 ++++-
cr-daily-branch | 92 ++++++++++++++-------
cri-common | 35 ++++++--
make-freebsd-flight | 14 ++--
mfi-common | 94 ++++++++++++++++-----
mg-anoint | 167 ++++++++++++++++++++------------------
sg-run-job | 9 +-
ts-build-prep-freebsd | 43 ++++++++++
ts-freebsd-build-packages | 145 +++++++++++++++++++++++++++++++++
ts-freebsd-host-install | 13 ---
12 files changed, 506 insertions(+), 163 deletions(-)
create mode 100755 ts-freebsd-build-packages
--
2.17.2 (Apple Git-113)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/6] osstest: introduce a helper to stash a whole directory
2019-02-20 16:59 [PATCH 0/6] osstest: create a local binary FreeBSD package repository Roger Pau Monne
@ 2019-02-20 16:59 ` Roger Pau Monne
2019-05-23 9:48 ` [Xen-devel] " Ian Jackson
2019-02-20 16:59 ` [PATCH 2/6] osstest: introduce a helper to create a weblink to a directory Roger Pau Monne
` (4 subsequent siblings)
5 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2019-02-20 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Without compressing it.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Osstest/TestSupport.pm | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 334cc2cb..c6da5ee9 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -30,6 +30,7 @@ use Osstest;
use Osstest::Logtailer;
use File::Copy;
use File::Basename;
+use File::Path;
use IO::Handle;
use Carp;
use Digest::SHA;
@@ -90,7 +91,7 @@ BEGIN {
dir_identify_vcs
build_url_vcs build_clone
built_stash built_stash_file built_stash_debugfile
- built_compress_stashed
+ built_compress_stashed built_stash_dir
hg_dir_revision git_dir_revision vcs_dir_revision
store_revision store_vcs_revision
git_massage_url
@@ -1695,6 +1696,22 @@ sub built_stash_file ($$$$;$) {
store_runvar("path_$item", $stashleaf);
}
+sub built_stash_dir ($$$$;$) {
+ my ($ho, $builddir, $item, $dname, $optional) = @_;
+ my $files = target_cmd_output($ho, <<END, 300);
+cd $builddir/$dname
+find -L . -type f
+END
+
+ foreach my $file (split /\n/, $files) {
+ $file =~ s,^./,,;
+ my $stashleaf = "$stash/build/$item/$file";
+
+ mkpath(dirname($stashleaf));
+ target_getfile($ho, 300, "$builddir/$dname/$file", $stashleaf);
+ }
+ store_runvar("path_$item", "build/$item");
+}
sub built_compress_stashed($) {
my ($path) = @_;
--
2.17.2 (Apple Git-113)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 2/6] osstest: introduce a helper to create a weblink to a directory
2019-02-20 16:59 [PATCH 0/6] osstest: create a local binary FreeBSD package repository Roger Pau Monne
2019-02-20 16:59 ` [PATCH 1/6] osstest: introduce a helper to stash a whole directory Roger Pau Monne
@ 2019-02-20 16:59 ` Roger Pau Monne
2019-05-23 9:57 ` [Xen-devel] " Ian Jackson
2019-02-20 16:59 ` [PATCH 3/6] osstest: allow to perform multiple anoints in the same transaction Roger Pau Monne
` (3 subsequent siblings)
5 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2019-02-20 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Osstest/TestSupport.pm | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index c6da5ee9..12427d11 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -125,6 +125,7 @@ BEGIN {
toolstack guest_create guest_create_paused
await_webspace_fetch_byleaf create_webfile
+ create_weblink
file_link_contents get_timeout
setup_netboot_di setup_netboot_local host_netboot_file
subst_netboot_template setup_netboot_memdisk
@@ -2739,6 +2740,18 @@ sub create_webfile ($$$) {
return $wf_url;
}
+sub create_weblink ($$$) {
+ my ($ho, $tail, $target) = @_;
+ my $wf_rhs= hostnamepath($ho)."_".$tail;
+ my $wf_common= $c{WebspaceCommon}.$wf_rhs;
+ my $wf_url= $c{WebspaceUrl}.$wf_common;
+ my $wf_file= $c{WebspaceFile}.$wf_common;
+
+ unlink $wf_file;
+ symlink $target, $wf_file or die "$wf_file $!";
+ return $wf_url;
+}
+
#---------- netboot handling ----------
sub file_link_contents ($$$) {
--
2.17.2 (Apple Git-113)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 3/6] osstest: allow to perform multiple anoints in the same transaction
2019-02-20 16:59 [PATCH 0/6] osstest: create a local binary FreeBSD package repository Roger Pau Monne
2019-02-20 16:59 ` [PATCH 1/6] osstest: introduce a helper to stash a whole directory Roger Pau Monne
2019-02-20 16:59 ` [PATCH 2/6] osstest: introduce a helper to create a weblink to a directory Roger Pau Monne
@ 2019-02-20 16:59 ` Roger Pau Monne
2019-05-23 10:00 ` [Xen-devel] " Ian Jackson
2019-02-20 16:59 ` [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit Roger Pau Monne
` (2 subsequent siblings)
5 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2019-02-20 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
In the same way allow to perform several fetches in the same retrieve
transaction.
Further patches will anoint a FreeBSD image and a binary ports tree
in the same transaction, and it's required to do it in the same
transaction in order to avoid inconsistencies when fetching them.
Note that most of the changes in this patch is code movement in order
to place the database accessors inside of a loop that iterates over
the input parameters.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
mg-anoint | 167 ++++++++++++++++++++++++++++--------------------------
1 file changed, 88 insertions(+), 79 deletions(-)
diff --git a/mg-anoint b/mg-anoint
index d09124b3..08447b8e 100755
--- a/mg-anoint
+++ b/mg-anoint
@@ -10,12 +10,12 @@
#
# ./mg-anoint destroy REFKEY
#
-# ./mg-anoint anoint [ANOINT-OPTION...] REFKEY FLIGHT JOB
+# ./mg-anoint anoint [ANOINT-OPTION...] REFKEY FLIGHT JOB [REFKEY FLIGHT JOB...]
# ANOINT-OPTIONs are:
# --allow-blessed=BLESSING,... default is from `prepare'
# --allow-job-status=STATUS,... default is only `pass'
#
-# ./mg-anoint retrieve [--tolerate-unprepared] REFKEY
+# ./mg-anoint retrieve [--tolerate-unprepared] REFKEY [REFKEY...]
# => FLIGHT JOB
# if nothing anointed yet, prints nothing and exits 0
# if anointment not prepared, fails
@@ -189,8 +189,7 @@ sub cmd_anoint {
die "unknown option $_ ?";
}
}
- die unless @ARGV==3;
- my ($refkey, $flight, $job) = @ARGV;
+ die unless @ARGV%3==0;
fail_unless_can_anoint();
prep_queries();
@@ -228,69 +227,74 @@ END
db_retry($dbh_tests, [], sub {
@o = ();
- $task_q->execute($refkey);
-
- # find the task row (ie, the anointment kind)
- my ($task, $refinfo) = $task_q->fetchrow_array();
- die "no such anointment kind \`$refkey' (no prepare?)\n"
- unless defined $task;
- my %params;
- foreach (split /\s+/, $refinfo) {
- die unless m/=/;
- $params{$`} = $';
- }
- my %blessings;
- $blessings{$_}++ foreach
- grep /./,
- (split /,/, $params{blessings}),
- (split /,/, $allow_blessed);
-
- my %jobstatus;
- $jobstatus{pass}++;
- $jobstatus{$_}++ foreach grep /./, split /,/, $allow_jobstatus;
-
- # check the to-be-anointed flight's blessing
- $newflight_q->execute($flight);
- my $frow = $newflight_q->fetchrow_hashref();
- die "flight $flight missing" unless $frow;
- die "flight $flight not started" unless defined $frow->{started};
-
- # check the job status
- $newjob_q->execute($flight, $job);
- my ($jstatus) = $newjob_q->fetchrow_array();
- die "job $flight.$job missing" unless defined $jstatus;
- die "job $flight.$job status $jstatus" unless $jobstatus{$jstatus};
-
- push @o, "flight $flight blessed $frow->{blessing}".
- " started ".show_abs_time($frow->{started});
-
- die "flight $flight blessing $frow->{blessing}".
- " (not $params{blessings} / $allow_blessed)"
- unless $blessings{ $frow->{blessing} };
-
- # check to-be-annointed flight is most recent
- $mostrecent_q->execute($task);
- my $mostrecent = $mostrecent_q->fetchrow_hashref();
- die "flight $flight not newer than $mostrecent->{flight}"
- unless $frow->{started} > ($mostrecent->{started} // 0);
-
- # expire old anointments
- $count_q->execute($task);
- my ($current) = $count_q->fetchrow_array();
- my $want_delete = ($current+1) - $params{keep};
- push @o, "anointment $refkey: currently $current anointed";
- if ($want_delete > 0) {
- $todelete_q->execute($task, $want_delete);
- while (my $d = $todelete_q->fetchrow_hashref()) {
- push @o, " expiring $d->{flight}.$d->{job} [/$d->{shareix}]".
- " started ".show_abs_time($d->{started});
- $delete_res_q->execute($task, $d->{flight}, $d->{shareix});
- }
- }
- # at last!
- $insert_q->execute($flight,$task,$task,$job);
- push @o, "anointed $flight.$job";
+ for (my $i=0; $i < @ARGV; $i+=3) {
+ my ($refkey, $flight, $job) = @ARGV[$i..$i+2];
+
+ $task_q->execute($refkey);
+
+ # find the task row (ie, the anointment kind)
+ my ($task, $refinfo) = $task_q->fetchrow_array();
+ die "no such anointment kind \`$refkey' (no prepare?)\n"
+ unless defined $task;
+ my %params;
+ foreach (split /\s+/, $refinfo) {
+ die unless m/=/;
+ $params{$`} = $';
+ }
+ my %blessings;
+ $blessings{$_}++ foreach
+ grep /./,
+ (split /,/, $params{blessings}),
+ (split /,/, $allow_blessed);
+
+ my %jobstatus;
+ $jobstatus{pass}++;
+ $jobstatus{$_}++ foreach grep /./, split /,/, $allow_jobstatus;
+
+ # check the to-be-anointed flight's blessing
+ $newflight_q->execute($flight);
+ my $frow = $newflight_q->fetchrow_hashref();
+ die "flight $flight missing" unless $frow;
+ die "flight $flight not started" unless defined $frow->{started};
+
+ # check the job status
+ $newjob_q->execute($flight, $job);
+ my ($jstatus) = $newjob_q->fetchrow_array();
+ die "job $flight.$job missing" unless defined $jstatus;
+ die "job $flight.$job status $jstatus" unless $jobstatus{$jstatus};
+
+ push @o, "flight $flight blessed $frow->{blessing}".
+ " started ".show_abs_time($frow->{started});
+
+ die "flight $flight blessing $frow->{blessing}".
+ " (not $params{blessings} / $allow_blessed)"
+ unless $blessings{ $frow->{blessing} };
+
+ # check to-be-annointed flight is most recent
+ $mostrecent_q->execute($task);
+ my $mostrecent = $mostrecent_q->fetchrow_hashref();
+ die "flight $flight not newer than $mostrecent->{flight}"
+ unless $frow->{started} > ($mostrecent->{started} // 0);
+
+ # expire old anointments
+ $count_q->execute($task);
+ my ($current) = $count_q->fetchrow_array();
+ my $want_delete = ($current+1) - $params{keep};
+ push @o, "anointment $refkey: currently $current anointed";
+ if ($want_delete > 0) {
+ $todelete_q->execute($task, $want_delete);
+ while (my $d = $todelete_q->fetchrow_hashref()) {
+ push @o, " expiring $d->{flight}.$d->{job} [/$d->{shareix}]".
+ " started ".show_abs_time($d->{started});
+ $delete_res_q->execute($task, $d->{flight}, $d->{shareix});
+ }
+ }
+
+ # at last!
+ $insert_q->execute($flight,$task,$task,$job);
+ push @o, "anointed $flight.$job";
+ }
});
pr_o();
}
@@ -301,26 +305,31 @@ sub cmd_retrieve {
shift @ARGV;
$tolerate_unprepared = 1;
}
- die unless @ARGV==1;
+ die unless @ARGV>=1;
die if $ARGV[0] =~ m/^-/;
- my ($refkey) = @ARGV;
empty_unless_can_anoint();
prep_queries();
db_retry($dbh_tests, [], sub {
@o = ();
- $task_q->execute($refkey);
- my ($task) = $task_q->fetchrow_array();
- die "no such anointment kind \`$refkey'"
- unless defined $task or $tolerate_unprepared;
-
- $mostrecent_q->execute($task);
- my $row = $mostrecent_q->fetchrow_hashref();
- if ($row) {
- push @o, "$row->{flight} $row->{job}";
- } else {
- print STDERR "warning: nothing anointed $refkey\n";
+
+ foreach my $refkey (@ARGV) {
+ $task_q->execute($refkey);
+ my ($task) = $task_q->fetchrow_array();
+ die "no such anointment kind \`$refkey'"
+ unless defined $task or $tolerate_unprepared;
+
+ $mostrecent_q->execute($task);
+ my $row = $mostrecent_q->fetchrow_hashref();
+ if ($row) {
+ push @o, "$row->{flight} $row->{job}";
+ } else {
+ # Push an error line so the number of output lines
+ # matches the number of REFKEYs requested.
+ push @o, "ERROR";
+ print STDERR "warning: nothing anointed $refkey\n";
+ }
}
});
pr_o();
--
2.17.2 (Apple Git-113)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit
2019-02-20 16:59 [PATCH 0/6] osstest: create a local binary FreeBSD package repository Roger Pau Monne
` (2 preceding siblings ...)
2019-02-20 16:59 ` [PATCH 3/6] osstest: allow to perform multiple anoints in the same transaction Roger Pau Monne
@ 2019-02-20 16:59 ` Roger Pau Monne
2019-05-23 10:03 ` [Xen-devel] " Ian Jackson
2019-02-20 17:00 ` [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository Roger Pau Monne
2019-02-20 17:00 ` [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD Roger Pau Monne
5 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2019-02-20 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
This only works when the svn revision is stored as a git note
with the format 'revision=<revision number>'.
Such conversion is required in order to bootstrap a FreeBSD system
without relying on external package repositories. FreeBSD base system
only contains a subversion client (no git client), and thus in order
to fetch the ports repository (that contain the external packages
build makefiles) svn must be used.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
cri-common | 35 +++++++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/cri-common b/cri-common
index 8d2d26cf..2655a9ac 100644
--- a/cri-common
+++ b/cri-common
@@ -38,12 +38,10 @@ besteffort_repo () {
cached_repo "$1" '[fetch=try]'
}
-repo_tree_rev_fetch_git () {
- local treename=$1
- local remoteurl=$2
- local remotetag=$3
- local localtag=$4
+repo_get_realurl () {
+ local remoteurl=$1
local proxy=`getconfig GitCacheProxy`
+
case $remoteurl in
$proxy*)
local realurl="$remoteurl" ;;
@@ -52,14 +50,35 @@ repo_tree_rev_fetch_git () {
*)
local realurl="$remoteurl" ;;
esac
- if ! test -d $repos/$treename; then
- git clone --bare $realurl $repos/$treename >&2
- fi
+
+ echo $realurl
+}
+
+repo_tree_rev_fetch_git () {
+ local treename=$1
+ local remoteurl=$2
+ local remotetag=$3
+ local localtag=$4
+ local realurl=`repo_get_realurl $remoteurl`
+
cd $repos/$treename
git fetch -f $realurl $remotetag:$localtag >&2
git rev-parse $localtag^0
}
+repo_tree_git2svn_rev () {
+ local treename=$1
+ local remoteurl=$2
+ local gitrev=$3
+ local realurl=`repo_get_realurl $remoteurl`
+
+ cd $repos/$treename
+ git fetch -f $realurl refs/notes/commits:refs/notes/commits >&2
+ git notes show $gitrev | \
+ sed -n 's/^.*revision=\([0-9][0-9]*\).*$/\1/p'
+}
+
+
select_prevxenbranch () {
prevxenbranch=`./cri-getprevxenbranch $xenbranch`
}
--
2.17.2 (Apple Git-113)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
2019-02-20 16:59 [PATCH 0/6] osstest: create a local binary FreeBSD package repository Roger Pau Monne
` (3 preceding siblings ...)
2019-02-20 16:59 ` [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit Roger Pau Monne
@ 2019-02-20 17:00 ` Roger Pau Monne
2019-05-23 10:19 ` [Xen-devel] " Ian Jackson
2019-05-23 10:38 ` [Xen-devel] " Ian Jackson
2019-02-20 17:00 ` [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD Roger Pau Monne
5 siblings, 2 replies; 35+ messages in thread
From: Roger Pau Monne @ 2019-02-20 17:00 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
The building of the binary packages itself is done with the poudriere
tool, that given a set of port names generates a binary package
repository with the requested packages and all it's dependencies.
Add a packages build job in the FreeBSD flight. Note that the binary
packages generated are tied to the base system used to build them when
not using a FreeBSD stable branch, stable branches guarantee ABI
stability.
Fetching the ports tree that contain the Makefiles to build the ports
is slightly more complicated than what would expected, since the
FreeBSD base system doesn't contain a git client the fetching is done
form the svn repository using the svnlite tool from the base system.
This is required in order to assure that bootstrapping the binary
repository doesn't depend on any external tools not found in the base
system.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
ap-common | 6 ++
ap-fetch-version | 19 +++--
cr-daily-branch | 57 +++++++++------
make-freebsd-flight | 8 ++-
sg-run-job | 9 ++-
ts-freebsd-build-packages | 145 ++++++++++++++++++++++++++++++++++++++
6 files changed, 215 insertions(+), 29 deletions(-)
create mode 100755 ts-freebsd-build-packages
diff --git a/ap-common b/ap-common
index 87df7948..8083b6c2 100644
--- a/ap-common
+++ b/ap-common
@@ -41,6 +41,11 @@
: ${PUSH_TREE_FREEBSD:=$XENBITS:/home/xen/git/freebsd.git}
: ${BASE_TREE_FREEBSD:=git://xenbits.xen.org/freebsd.git}
+: ${TREE_FREEBSD_PORTS:=git://github.com/freebsd/freebsd-ports.git}
+: ${TREE_FREEBSD_PORTS_SVN:=https://svn.FreeBSD.org/ports/head}
+: ${PUSH_TREE_FREEBSD_PORTS:=$XENBITS:/home/xen/git/freebsd-ports.git}
+: ${BASE_TREE_FREEBSD_PORTS:=git://xenbits.xen.org/freebsd-ports.git}
+
: ${TREE_LIBVIRT:=git://libvirt.org/libvirt.git}
: ${PUSH_TREE_LIBVIRT:=$XENBITS:/home/xen/git/libvirt.git}
: ${BASE_TREE_LIBVIRT:=git://xenbits.xen.org/libvirt.git}
@@ -88,6 +93,7 @@ fi
: ${LOCALREV_OVMF:=daily-cron.$branch}
: ${LOCALREV_XTF:=daily-cron.$branch}
: ${LOCALREV_FREEBSD:=daily-cron.$branch}
+: ${LOCALREV_FREEBSD_PORTS:=daily-cron.$branch}
: ${TREEBASE_LINUX_XCP:=http://hg.uk.xensource.com/carbon/trunk/linux-2.6.27}
diff --git a/ap-fetch-version b/ap-fetch-version
index 87725bf0..a0963cc5 100755
--- a/ap-fetch-version
+++ b/ap-fetch-version
@@ -108,11 +108,22 @@ ovmf)
;;
freebsd-*)
branchcore=${branch#freebsd-}
- if [ "x$branchcore" != "xmaster" ]; then
- branchcore="stable/$branchcore"
+ subbranch=${branchcore#*--}
+ branchcore=${branchcore%--"$subbranch"}
+
+ if [ "$subbranch" = "$branchcore" ]; then
+ if [ "$branchcore" != master ]; then
+ branchcore="stable/$branchcore"
+ fi
+ repo_tree_rev_fetch_git freebsd \
+ $TREE_FREEBSD $branchcore $LOCALREV_FREEBSD
+ else
+ if [ "$subbranch" != ports ]; then
+ exit 1
+ fi
+ repo_tree_rev_fetch_git freebsd-ports \
+ $TREE_FREEBSD_PORTS master $LOCALREV_FREEBSD_PORTS
fi
- repo_tree_rev_fetch_git freebsd \
- $TREE_FREEBSD $branchcore $LOCALREV_FREEBSD
;;
osstest)
if [ "x$OSSTEST_USE_HEAD" = "xy" ] ; then
diff --git a/cr-daily-branch b/cr-daily-branch
index 49b8ad8e..971f4c01 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -47,19 +47,28 @@ determine_version () {
local tversionvar=$1
local tbranch=$2
local treevarwhich=$3
- if [ "x$tbranch" = "x$branch" ] && [ "x$force_baseline" = x ]; then
- if [ "x$FORCE_REVISION" != x ]; then
- tversion="$FORCE_REVISION"
- else
- tversion=`$AP_FETCH_PFX ./ap-fetch-version "$tbranch"`
- fi
- determine_tree "$treevarwhich" "" _${treevarwhich}
- determine_tree "$treevarwhich" "" _${treevarwhich}_THIS
- else
+
+ case "$tbranch" in
+ "$branch"|"$branch"--*)
+ if [ "x$force_baseline" = x ]; then
+ if [ "x$FORCE_REVISION" != x ]; then
+ tversion="$FORCE_REVISION"
+ else
+ tversion=`$AP_FETCH_PFX ./ap-fetch-version "$tbranch"`
+ fi
+ determine_tree "$treevarwhich" "" _${treevarwhich}
+ determine_tree "$treevarwhich" "" _${treevarwhich}_THIS
+ eval "$tversionvar=$tversion"
+ return
+ fi
+ ;& # fallthrough
+ *)
tversion=`$AP_FETCH_PFX ./ap-fetch-version-baseline "$tbranch"`
determine_tree "$treevarwhich" BASE_ _${treevarwhich}
determine_tree "$treevarwhich" BASE_ _${treevarwhich}_THIS
- fi
+ ;;
+ esac
+
eval "$tversionvar=$tversion"
}
@@ -235,18 +244,24 @@ if [ "x$REVISION_LINUXFIRMWARE" = x ]; then
determine_version REVISION_LINUXFIRMWARE linuxfirmware LINUXFIRMWARE
export REVISION_LINUXFIRMWARE
fi
-if [ "x$REVISION_FREEBSD" = x ]; then
- case "$branch" in
- freebsd-*)
- determine_version REVISION_FREEBSD "$branch" FREEBSD
- ;;
- *)
- determine_version REVISION_FREEBSD freebsd-master FREEBSD
- ;;
- esac
-
+case "$branch" in
+freebsd-*)
+ [ "x$REVISION_FREEBSD" = x ] && \
+ determine_version REVISION_FREEBSD "$branch" FREEBSD && \
export REVISION_FREEBSD
-fi
+ [ "x$REVISION_FREEBSD_PORTS" = x ] && \
+ determine_version REVISION_FREEBSD_PORTS "$branch"--ports FREEBSD_PORTS && \
+ export REVISION_FREEBSD_PORTS
+ ;;
+*)
+ [ "x$REVISION_FREEBSD" = x ] && \
+ determine_version REVISION_FREEBSD freebsd-master FREEBSD && \
+ export REVISION_FREEBSD
+ [ "x$REVISION_FREEBSD_PORTS" = x ] && \
+ determine_version REVISION_FREEBSD_PORTS freebsd-master--ports FREEBSD_PORTS && \
+ export REVISION_FREEBSD_PORTS
+ ;;
+esac
case "$tree" in
xen)
diff --git a/make-freebsd-flight b/make-freebsd-flight
index d3c413b5..fc3d2d83 100755
--- a/make-freebsd-flight
+++ b/make-freebsd-flight
@@ -38,13 +38,15 @@ job_create_build_filter_callback () {
for arch in "$arches"; do
set_freebsd_runvars
-
create_freebsd_build_job build-$arch-freebsd
- # Create an identical job that's going to use the build output from
- # the previous one.
+ # Create a job to build the packages against the new world.
freebsd_runvars="$freebsd_runvars freebsdbuildjob=build-$arch-freebsd \
recipe_testinstall=true"
+ create_freebsd_pkg_build_job build-$arch-freebsd-packages
+
+ # Create an identical job that's going to use the build output from
+ # the previous one.
create_freebsd_build_job build-$arch-freebsd-again
# Create a Xen build job that's going to use the output from the first
diff --git a/sg-run-job b/sg-run-job
index 7d27f415..37123980 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -716,6 +716,7 @@ proc need-hosts/build-rumprun {} { return BUILD_LINUX }
proc need-hosts/build-xtf {} { return BUILD_LINUX }
proc need-hosts/build-freebsd {} { return BUILD_FREEBSD }
proc need-hosts/build-xen-freebsd {} { return BUILD_FREEBSD }
+proc need-hosts/build-pkg-freebsd {} { return BUILD_FREEBSD }
proc run-job/build {} {
run-ts . = ts-xen-build
@@ -746,6 +747,10 @@ proc run-job/build-freebsd {} {
run-ts . = ts-freebsd-build
}
+proc run-job/build-pkg-freebsd {} {
+ run-ts . = ts-freebsd-build-packages
+}
+
proc run-job/build-xen-freebsd {} {
run-ts . = ts-xen-build-freebsd + host
}
@@ -768,7 +773,9 @@ proc prepare-build-host-freebsd {} {
global jobinfo
if {[recipe-flag testinstall]} { set broken fail } { set broken broken }
run-ts $broken host-install(*) ts-freebsd-host-install
- run-ts . host-build-prep ts-build-prep-freebsd
+ if {![recipe-flag skipbuildprep]} {
+ run-ts . host-build-prep ts-build-prep-freebsd
+ }
}
proc need-hosts/coverity {} { return BUILD_LINUX }
diff --git a/ts-freebsd-build-packages b/ts-freebsd-build-packages
new file mode 100755
index 00000000..9202dd9f
--- /dev/null
+++ b/ts-freebsd-build-packages
@@ -0,0 +1,145 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2019 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 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 Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This script will generate a package repository for FreeBSD, as
+# described in the handbook:
+# https://www.freebsd.org/doc/handbook/ports-poudriere.html
+
+# Consumes the following input runvars:
+#
+# svnrevision_freebsdports: ports svn revision id to use.
+# svntree_freebsdports ports svn tree to fetch the source code from.
+#
+# Produces: a binary package repository to consume.
+#
+# Sets the following runvar:
+#
+# path_freebsdpackagesdist: points to the folder where the package tree is
+# stored.
+
+use strict qw(vars);
+use DBI;
+use POSIX;
+use URI;
+
+unshift @INC, qw(.);
+use Osstest;
+use Osstest::TestSupport;
+use Osstest::BuildSupport;
+
+tsreadconfig();
+
+selectbuildhost(\@ARGV);
+builddirsprops();
+
+sub checkout () {
+ my $u = URI->new($c{HttpProxy});
+ my $host = $u->host;
+ my $port = $u->port;
+ prepbuilddirs();
+
+ logm("Checkout ports tree from svn");
+ target_cmd_build($ho, 4000, $builddir, <<END);
+cd $builddir
+rm -rf ports
+# svn ignores HTTP_PROXY envvar
+svnlite checkout --config-option servers:global:http-proxy-host=$host \\
+ --config-option servers:global:http-proxy-port=$port \\
+ --trust-server-cert \\
+ $r{"svntree_freebsdports"} \\
+ -r $r{"svnrevision_freebsdports"} ports
+END
+}
+
+sub install_port ($$) {
+ my ($port, $time) = @_;
+ my $name = $port;
+
+ $name =~ s/\//-/;
+ buildcmd_stamped_logged_root($time, "ports/$port", "$name-install",
+ '', 'make install', '');
+}
+
+sub config_poudriere() {
+ target_cmd_root($ho, <<END, 30);
+set -ex
+mkdir -p $builddir/poudriere
+# Create the directory for the distfiles
+mkdir -p /usr/ports/distfiles
+cat << ENDP >> /usr/local/etc/poudriere.conf
+# Use the ZFS pool created by the installer
+ZPOOL=zroot
+# Setup the http proxy for the builder jail
+export HTTP_PROXY=$c{HttpProxy}
+# Use parallel make jobs
+ALLOW_MAKE_JOBS=yes
+# Store build results in builddir
+# NB: use realpath to workaround a bug in poudriere versions < 3.3.0:
+# https://github.com/freebsd/poudriere/issues/654
+POUDRIERE_DATA=`realpath $builddir/poudriere`
+ENDP
+# Register the local ports tree
+poudriere ports -c -m none -M $builddir/ports -p local
+END
+}
+
+sub create_jail() {
+ my $src_prefix = $r{"freebsd_distpath"} ||
+ get_stashed("path_freebsddist", $r{"freebsdbuildjob"});
+ my $dst_prefix = "/root/sets";
+
+ target_cmd_root($ho, "mkdir -p $dst_prefix", 30);
+ target_putfile_root($ho, 600, "$src_prefix/base.txz",
+ "$dst_prefix/base.txz");
+
+ # NB: create the jail using the same build as the host
+ target_cmd_root($ho, <<END, 1200);
+poudriere jail -c -j builder -v `uname -r` -m tar=$dst_prefix/base.txz
+END
+}
+
+sub build() {
+ my @packages = qw(devel/git devel/glib20 devel/pkgconf devel/yajl
+ devel/gmake x11/pixman textproc/markdown
+ devel/gettext lang/python devel/argp-standalone
+ archivers/lzo2 lang/gcc devel/binutils);
+
+ target_putfilecontents_root_stash($ho, 30, join("\n", @packages),
+ '~/package.list');
+
+ target_cmd_root($ho, <<END, 14400);
+poudriere bulk -j builder -p local -f ~/package.list
+END
+}
+
+checkout();
+
+logm("Build and install poudriere");
+install_port("ports-mgmt/poudriere", 3600);
+config_poudriere();
+
+logm("Create build jail");
+create_jail();
+
+logm("Build packages");
+build();
+
+logm("Stashing build output");
+built_stash_dir($ho, $builddir, "freebsdpackagesdist",
+ "poudriere/packages/builder-local");
+
+logm("FreeBSD packages built successful");
--
2.17.2 (Apple Git-113)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD
2019-02-20 16:59 [PATCH 0/6] osstest: create a local binary FreeBSD package repository Roger Pau Monne
` (4 preceding siblings ...)
2019-02-20 17:00 ` [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository Roger Pau Monne
@ 2019-02-20 17:00 ` Roger Pau Monne
2019-05-23 11:06 ` [Xen-devel] " Ian Jackson
5 siblings, 1 reply; 35+ messages in thread
From: Roger Pau Monne @ 2019-02-20 17:00 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
This removes the dependency on the official pkg repository, which is
dangerous when not testing stable branches, since the ABI of the
development branch is not stable, and thus it's easy for packages to
get out of sync, or for osstest to test an old FreeBSD version that
has an ABI different than the one used to build the official
packages.
The output of the package build test is tested together with the newly
built image, and if there are no regressions both are anointed in
lockstep in order to prevent temporary discrepancies between the
installer and the package repository.
Note that in order to bootstrap the first run it's possible to
manually set the package repository to use with an environment
variable:
- FREEBSD_PACKAGES_<arch>_BUILDJOB: points to the flight.job that
contains the binary package repository.
- FREEBSD_PACKAGES: points to a local directory that contains the
binary repository.
It's also possible to set the directory that contains the package
repository in the configuration file using FreeBSDPackages.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
cr-daily-branch | 35 +++++++++++----
make-freebsd-flight | 10 +++--
mfi-common | 94 +++++++++++++++++++++++++++++++----------
ts-build-prep-freebsd | 43 +++++++++++++++++++
ts-freebsd-host-install | 13 ------
5 files changed, 147 insertions(+), 48 deletions(-)
diff --git a/cr-daily-branch b/cr-daily-branch
index 971f4c01..28cbd61a 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -321,15 +321,26 @@ freebsd)
esac
IFS=$'\n'
+count=0
for anointed in \
- `./mg-anoint list-prepared "freebsd build $freebsd_branch *"`; do
+ `./mg-anoint list-prepared "freebsd* build $freebsd_branch *"`; do
# Retrieve previous successful FreeBSD build for each arch.
freebsd_arch=${anointed##* }
- freebsd_envvar="FREEBSD_${freebsd_arch^^}_BUILDJOB"
+ freebsd_name=${anointed%% *}
+ freebsd_name=${freebsd_name/-/_}
+ freebsd_envvar="${freebsd_name^^}_${freebsd_arch^^}_BUILDJOB"
if [ "x${!freebsd_envvar}" = "x" ]; then
- flight_job=`./mg-anoint retrieve "$anointed"`
- export ${freebsd_envvar}=${flight_job/ /.}
+ envvars[$count]="$freebsd_envvar"
+ refkeys[$count]="$anointed"
+ count=$((count+1))
+ fi
+done
+count=0
+for flight_job in `./mg-anoint retrieve ${refkeys[@]}`; do
+ if [ "$flight_job" != "ERROR" ]; then
+ export ${envvars[$count]}=${flight_job/ /.}
fi
+ count=$((count+1))
done
unset IFS
@@ -542,17 +553,23 @@ freebsd-*)
[ "x$OSSTEST_BLESSING" == "xreal" ]; then
IFS=$'\n'
for anointed in `./mg-anoint list-prepared \
- "freebsd build $freebsd_branch *"`; do
+ "freebsd* build $freebsd_branch *"`; do
# Update anointed versions
# NB: failure to update an anointed build for a specific arch
# should not be fatal, and it's not an issue if one of the
# arches gets slightly out of sync with the other ones.
freebsd_arch=${anointed##* }
- if ./mg-anoint anoint "$anointed" \
- $flight build-$freebsd_arch-freebsd; then
- echo "Anointed artifacts from build-$freebsd_arch-freebsd"
- fi
+ freebsd_name=${anointed%% *}
+ # Rely on the fact that the job suffix is the same as the
+ # anointment refkey. Ie:
+ # refkey: freebsd job: build-<arch>-freebsd
+ # refkey: freebsd-packages job: build-<arch>-freebsd-packages
+ anoint="$anoint \"$anointed\" $flight \
+ build-$freebsd_arch-$freebsd_name"
done
+ if ./mg-anoint anoint $anoint; then
+ echo "Anointed build artifacts from flight"
+ fi
unset IFS
fi
;;
diff --git a/make-freebsd-flight b/make-freebsd-flight
index fc3d2d83..0458a33b 100755
--- a/make-freebsd-flight
+++ b/make-freebsd-flight
@@ -45,12 +45,14 @@ for arch in "$arches"; do
recipe_testinstall=true"
create_freebsd_pkg_build_job build-$arch-freebsd-packages
- # Create an identical job that's going to use the build output from
- # the previous one.
+ # Create a build job that going to use the output of both the jobs
+ # above in order to test the newly built FreeBSD and packages
+ freebsd_runvars="$freebsd_runvars \
+ freebsdpackagesbuildjob=build-$arch-freebsd-packages"
create_freebsd_build_job build-$arch-freebsd-again
- # Create a Xen build job that's going to use the output from the first
- # FreeBSD build job.
+ # Create a Xen build job that's going to use the output from the
+ # FreeBSD build jobs.
create_xen_build_job build-$arch-xen-freebsd build-xen-freebsd \
host_hostflags=arch-$arch,purpose-build all_host_os=freebsd \
$freebsd_runvars
diff --git a/mfi-common b/mfi-common
index 83d3c713..12cde85f 100644
--- a/mfi-common
+++ b/mfi-common
@@ -156,7 +156,6 @@ set_freebsd_runvars () {
# 4. Look for an anointed build of FreeBSD `master' (Executive only)
#
local no_hostflags=$1
- local envvar="FREEBSD_${arch^^}_BUILDJOB"
if [ x$no_hostflags != xtrue ]; then
# osstest doesn't yet know how to install FreeBSD on UEFI hosts, so
@@ -164,27 +163,59 @@ set_freebsd_runvars () {
freebsd_runvars="all_hostflags,=PropEq:Firmware:bios:bios"
fi
- if [ -n "${!envvar}" ]; then
- freebsd_runvars="$freebsd_runvars freebsdbuildjob=${!envvar}"
- return
- fi
- if [ -n "$FREEBSD_DIST" ] && [ -n "$FREEBSD_VERSION" ]; then
- freebsd_runvars="$freebsd_runvars freebsd_distpath=$FREEBSD_DIST/$arch \
- freebsd_version=$FREEBSD_VERSION"
- return
- fi
- local distpath=`getconfig "FreeBSDDist"`
- if [ -n "$distpath" ]; then
- local version=`getconfig "FreeBSDVersion"`
- freebsd_runvars="$freebsd_runvars freebsd_distpath=$distpath/$arch \
- freebsd_version=$version"
- return
- fi
- local anointment="freebsd build master $arch"
- local flightjob=`./mg-anoint retrieve --tolerate-unprepared "$anointment"`
- if [ -n "$flightjob" ]; then
- freebsd_runvars="$freebsd_runvars freebsdbuildjob=${flightjob/ /.}"
- return
+ # Check if the packages are provided externally, or else assume they
+ # are provided by the same flight as the installer binaries.
+ local pkgpath=`getconfig "FreeBSDPackages"`
+ counter=0
+ IFS=$'\n'
+ for flightjob in `./mg-anoint retrieve --tolerate-unprepared \
+ "freebsd build master $arch" \
+ "freebsd-packages build master $arch"`; do
+ if [ $counter -eq 0 ]; then
+ # Anointed FreeBSD installer
+ local envvar="FREEBSD_${arch^^}_BUILDJOB"
+ local distpath=`getconfig "FreeBSDDist"`
+ if [ -n "${!envvar}" ]; then
+ freebsd_runvars="$freebsd_runvars \
+ freebsdbuildjob=${!envvar}"
+ elif [ -n "$FREEBSD_DIST" ] && [ -n "$FREEBSD_VERSION" ]; then
+ freebsd_runvars="$freebsd_runvars \
+ freebsd_distpath=$FREEBSD_DIST/$arch \
+ freebsd_version=$FREEBSD_VERSION"
+ elif [ -n "$distpath" ]; then
+ local version=`getconfig "FreeBSDVersion"`
+ freebsd_runvars="$freebsd_runvars \
+ freebsd_distpath=$distpath/$arch \
+ freebsd_version=$version"
+ elif [ "$flightjob" != "ERROR" ]; then
+ freebsd_runvars="$freebsd_runvars \
+ freebsdbuildjob=${flightjob/ /.}"
+ fi
+ elif [ $counter -eq 1 ]; then
+ # Anointed package repository
+ local envvar="FREEBSD_PACKAGES_${arch^^}_BUILDJOB"
+ local pkgpath=`getconfig "FreeBSDPackages"`
+ if [ -n "${!envvar}" ]; then
+ freebsd_runvars="$freebsd_runvars \
+ freebsdpackagesbuildjob=${!envvar}"
+ elif [ -n "$FREEBSD_PACKAGES" ]; then
+ freebsd_runvars="$freebsd_runvars \
+ freebsd_packages=$FREEBSD_PACKAGES/$arch"
+ elif [ -n "$pkgpath" ]; then
+ freebsd_runvars="$freebsd_runvars \
+ freebsd_packages=$pkgpath/$arch"
+ elif [ "$flightjob" != "ERROR" ]; then
+ freebsd_runvars="$freebsd_runvars \
+ freebsdpackagesbuildjob=${flightjob/ /.}"
+ fi
+ fi
+ counter=$((counter+1))
+ done
+ unset IFS
+
+ # Make sure we got exactly 2 results.
+ if [ $counter -ne 2 ]; then
+ exit 1
fi
}
@@ -202,6 +233,25 @@ create_freebsd_build_job () {
$freebsd_runvars
}
+create_freebsd_pkg_build_job () {
+ repos=`getrepos`
+ local name=$1
+ local svnrev=`repo_tree_git2svn_rev freebsd-ports $TREE_FREEBSD_PORTS \
+ $REVISION_FREEBSD_PORTS`
+
+ job_create_build $name build-pkg-freebsd \
+ arch=$arch \
+ $RUNVARS $BUILD_RUNVARS $BUILD_FREEBSD_RUNVARS \
+ $arch_runvars \
+ tree_freebsdports=$TREE_FREEBSD_PORTS \
+ revision_freebsdports=$REVISION_FREEBSD_PORTS \
+ svntree_freebsdports=$TREE_FREEBSD_PORTS_SVN \
+ svnrevision_freebsdports=$svnrev \
+ host_hostflags=arch-$arch,purpose-build \
+ all_host_os=freebsd \
+ $freebsd_runvars recipe_skipbuildprep=true
+}
+
create_xen_build_job () {
local name=$1; shift
local recipe=$1; shift
diff --git a/ts-build-prep-freebsd b/ts-build-prep-freebsd
index ef880503..ec428227 100755
--- a/ts-build-prep-freebsd
+++ b/ts-build-prep-freebsd
@@ -30,6 +30,48 @@ $whhost ||= 'host';
our $ho= selecthost($whhost);
exit 0 if $ho->{SharedReady};
+# NB: the packages are built as part of the build-<arch>-freebsd-again
+# job, which installs the host using the newly compiled FreeBSD version.
+our $path_packages = $r{"freebsd_packages"} ||
+ get_stashed("path_freebsdpackagesdist",
+ $r{"freebsdpackagesbuildjob"});
+
+sub bootstrap () {
+ # Create a host-specific link to the pkg binaries to use.
+ my $pkg_repo_url = create_weblink($ho, "packages", $path_packages);
+
+ target_cmd_root($ho, <<END, 600);
+set -ex
+# Set proxy for the pkg manager
+mkdir -p /usr/local/etc/
+cat << ENDPKG >> /usr/local/etc/pkg.conf
+pkg_env: { http_proxy = $c{HttpProxy} }
+default_always_yes: true
+assume_always_yes: true
+ENDPKG
+
+# Setup custom pkg repository
+mkdir -p /usr/local/etc/pkg/repos/
+cat << ENDREPO > /usr/local/etc/pkg/repos/osstest.conf
+osstest: {
+ url: "$pkg_repo_url",
+ enabled: yes,
+}
+ENDREPO
+# Disable default official FreeBSD mirror
+cat << ENDREPO > /usr/local/etc/pkg/repos/FreeBSD.conf
+FreeBSD: {
+ enabled: no,
+}
+ENDREPO
+
+# Bootstap the package manager
+export HTTP_PROXY=$c{HttpProxy}
+export ASSUME_ALWAYS_YES=yes
+pkg bootstrap
+END
+}
+
sub install_deps () {
my @packages = qw(git glib pkgconf yajl gmake pixman markdown gettext
python argp-standalone lzo2 git gcc binutils);
@@ -37,6 +79,7 @@ sub install_deps () {
target_install_packages($ho, @packages);
}
+bootstrap();
install_deps();
gitcache_setup($ho);
diff --git a/ts-freebsd-host-install b/ts-freebsd-host-install
index 3c3e9c34..e8977e95 100755
--- a/ts-freebsd-host-install
+++ b/ts-freebsd-host-install
@@ -194,19 +194,6 @@ sysrc sendmail_submit_enable=NO
sysrc sendmail_outbound_enable=NO
sysrc sendmail_msp_queue_enable=NO
-# Set proxy for the pkg manager
-mkdir -p /usr/local/etc/
-cat << ENDPKG >> /usr/local/etc/pkg.conf
-pkg_env: { http_proxy = $c{HttpProxy} }
-default_always_yes: true
-assume_always_yes: true
-ENDPKG
-
-# Bootstap the package manager
-export HTTP_PROXY=$c{HttpProxy}
-export ASSUME_ALWAYS_YES=yes
-pkg bootstrap
-
# Allow root user login and setup ssh keys
chsh -s /bin/sh root
echo 'PermitRootLogin yes' >> /etc/ssh/sshd_config
--
2.17.2 (Apple Git-113)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 1/6] osstest: introduce a helper to stash a whole directory
@ 2019-05-23 9:48 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 9:48 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 1/6] osstest: introduce a helper to stash a whole directory"):
> Without compressing it.
You've open-coded a recursive directory copy. Is rsync available on
$ho at this point ? I think maybe it could be ...
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 1/6] osstest: introduce a helper to stash a whole directory
@ 2019-05-23 9:48 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 9:48 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 1/6] osstest: introduce a helper to stash a whole directory"):
> Without compressing it.
You've open-coded a recursive directory copy. Is rsync available on
$ho at this point ? I think maybe it could be ...
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/6] osstest: introduce a helper to create a weblink to a directory
@ 2019-05-23 9:57 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 9:57 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 2/6] osstest: introduce a helper to create a weblink to a directory"):
> +sub create_weblink ($$$) {
> + my ($ho, $tail, $target) = @_;
> + my $wf_rhs= hostnamepath($ho)."_".$tail;
> + my $wf_common= $c{WebspaceCommon}.$wf_rhs;
> + my $wf_url= $c{WebspaceUrl}.$wf_common;
> + my $wf_file= $c{WebspaceFile}.$wf_common;
> +
> + unlink $wf_file;
> + symlink $target, $wf_file or die "$wf_file $!";
> + return $wf_url;
Most of this is the start of create_webfile. Can you factor that
out ?
I have three suggestions for the shape:
my ($wf_file, $wf_url) = prepare_create_webfile($ho, $tail);
symlink $target, $wf_file or die "$wf_file $!";
return $wf_url;
}
or split create_webfile into create_web_fsobject which takes a
subref, and the call to file_link_contents:
create_web_fsobject($ho, $tail, sub {
my ($wf_file) = @_;
symlink $target, $wf_file or die "$wf_file $!";
});
}
or the same but make passing a subref as $contents legal for
create_webfile,
sub create_webfile ($$$) {
my .... $contents_spec);
# $contents as for file_link_contents, or a subref
# which will be called as $contents_spec($wf_file,$wf_url);
...
if (ref($contents_spec) ne 'CODE') {
my $contents = $contents_spec;
$contents_spec = sub {
my ($wf_file) = @_;
file_link_contents($wf_file, $contents, "webspace-$wf_rhs");
};
}
Take your pick, or do something similar ?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 2/6] osstest: introduce a helper to create a weblink to a directory
@ 2019-05-23 9:57 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 9:57 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 2/6] osstest: introduce a helper to create a weblink to a directory"):
> +sub create_weblink ($$$) {
> + my ($ho, $tail, $target) = @_;
> + my $wf_rhs= hostnamepath($ho)."_".$tail;
> + my $wf_common= $c{WebspaceCommon}.$wf_rhs;
> + my $wf_url= $c{WebspaceUrl}.$wf_common;
> + my $wf_file= $c{WebspaceFile}.$wf_common;
> +
> + unlink $wf_file;
> + symlink $target, $wf_file or die "$wf_file $!";
> + return $wf_url;
Most of this is the start of create_webfile. Can you factor that
out ?
I have three suggestions for the shape:
my ($wf_file, $wf_url) = prepare_create_webfile($ho, $tail);
symlink $target, $wf_file or die "$wf_file $!";
return $wf_url;
}
or split create_webfile into create_web_fsobject which takes a
subref, and the call to file_link_contents:
create_web_fsobject($ho, $tail, sub {
my ($wf_file) = @_;
symlink $target, $wf_file or die "$wf_file $!";
});
}
or the same but make passing a subref as $contents legal for
create_webfile,
sub create_webfile ($$$) {
my .... $contents_spec);
# $contents as for file_link_contents, or a subref
# which will be called as $contents_spec($wf_file,$wf_url);
...
if (ref($contents_spec) ne 'CODE') {
my $contents = $contents_spec;
$contents_spec = sub {
my ($wf_file) = @_;
file_link_contents($wf_file, $contents, "webspace-$wf_rhs");
};
}
Take your pick, or do something similar ?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 3/6] osstest: allow to perform multiple anoints in the same transaction
@ 2019-05-23 10:00 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 10:00 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 3/6] osstest: allow to perform multiple anoints in the same transaction"):
> Note that most of the changes in this patch is code movement in order
> to place the database accessors inside of a loop that iterates over
> the input parameters.
Sorry to be *really* annoying, but is it possible to put the code
motion in a separate patch ? Move it into a subroutine, I guess.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 3/6] osstest: allow to perform multiple anoints in the same transaction
@ 2019-05-23 10:00 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 10:00 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 3/6] osstest: allow to perform multiple anoints in the same transaction"):
> Note that most of the changes in this patch is code movement in order
> to place the database accessors inside of a loop that iterates over
> the input parameters.
Sorry to be *really* annoying, but is it possible to put the code
motion in a separate patch ? Move it into a subroutine, I guess.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit
@ 2019-05-23 10:03 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 10:03 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit"):
> This only works when the svn revision is stored as a git note
> with the format 'revision=<revision number>'.
Wow. This is pretty ugly.
> Such conversion is required in order to bootstrap a FreeBSD system
> without relying on external package repositories. FreeBSD base system
> only contains a subversion client (no git client), and thus in order
> to fetch the ports repository (that contain the external packages
> build makefiles) svn must be used.
git notes have some different way of travelling than commits, don't
they ? Where is this git note coming from and how do we know it is
the right note, IYSWIM ?
Aside from that, please break the refactoring (in this case, the
breaking out of repo_get_realurl) into a separate NFC patch.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit
@ 2019-05-23 10:03 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 10:03 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit"):
> This only works when the svn revision is stored as a git note
> with the format 'revision=<revision number>'.
Wow. This is pretty ugly.
> Such conversion is required in order to bootstrap a FreeBSD system
> without relying on external package repositories. FreeBSD base system
> only contains a subversion client (no git client), and thus in order
> to fetch the ports repository (that contain the external packages
> build makefiles) svn must be used.
git notes have some different way of travelling than commits, don't
they ? Where is this git note coming from and how do we know it is
the right note, IYSWIM ?
Aside from that, please break the refactoring (in this case, the
breaking out of repo_get_realurl) into a separate NFC patch.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
@ 2019-05-23 10:19 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 10:19 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository"):
> The building of the binary packages itself is done with the poudriere
> tool, that given a set of port names generates a binary package
> repository with the requested packages and all it's dependencies.
...
> ap-common | 6 ++
> ap-fetch-version | 19 +++--
> cr-daily-branch | 57 +++++++++------
I want to talk separately about the cr-daily-branch and related
things.
Also, patches like this are usually more conveniently split up. That
way I can ack the less difficult parts separately.
> ts-freebsd-build-packages | 145 ++++++++++++++++++++++++++++++++++++++
> sg-run-job | 9 ++-
> make-freebsd-flight | 8 ++-
At the very least split these three out. But maybe you want to split
them into two or three.
I'll read these now...
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
@ 2019-05-23 10:19 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 10:19 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository"):
> The building of the binary packages itself is done with the poudriere
> tool, that given a set of port names generates a binary package
> repository with the requested packages and all it's dependencies.
...
> ap-common | 6 ++
> ap-fetch-version | 19 +++--
> cr-daily-branch | 57 +++++++++------
I want to talk separately about the cr-daily-branch and related
things.
Also, patches like this are usually more conveniently split up. That
way I can ack the less difficult parts separately.
> ts-freebsd-build-packages | 145 ++++++++++++++++++++++++++++++++++++++
> sg-run-job | 9 ++-
> make-freebsd-flight | 8 ++-
At the very least split these three out. But maybe you want to split
them into two or three.
I'll read these now...
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
@ 2019-05-23 10:38 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 10:38 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository"):
> diff --git a/make-freebsd-flight b/make-freebsd-flight
> index d3c413b5..fc3d2d83 100755
> --- a/make-freebsd-flight
> +++ b/make-freebsd-flight
> @@ -38,13 +38,15 @@ job_create_build_filter_callback () {
>
> for arch in "$arches"; do
> set_freebsd_runvars
> -
> create_freebsd_build_job build-$arch-freebsd
>
> - # Create an identical job that's going to use the build output from
> - # the previous one.
> + # Create a job to build the packages against the new world.
> freebsd_runvars="$freebsd_runvars freebsdbuildjob=build-$arch-freebsd \
> recipe_testinstall=true"
> + create_freebsd_pkg_build_job build-$arch-freebsd-packages
> +
> + # Create an identical job that's going to use the build output from
> + # the previous one.
> create_freebsd_build_job build-$arch-freebsd-again
>
> # Create a Xen build job that's going to use the output from the first
This looks OK.
> @@ -768,7 +773,9 @@ proc prepare-build-host-freebsd {} {
> global jobinfo
> if {[recipe-flag testinstall]} { set broken fail } { set broken broken }
> run-ts $broken host-install(*) ts-freebsd-host-install
> - run-ts . host-build-prep ts-build-prep-freebsd
> + if {![recipe-flag skipbuildprep]} {
> + run-ts . host-build-prep ts-build-prep-freebsd
What's this for ? Oh, I see.
I notice that none of your freebsd build jobs pass any share- hostflag
so they always use a fresh installation. Is that necessary ?
> proc need-hosts/coverity {} { return BUILD_LINUX }
> diff --git a/ts-freebsd-build-packages b/ts-freebsd-build-packages
> new file mode 100755
> index 00000000..9202dd9f
> --- /dev/null
> +++ b/ts-freebsd-build-packages
> @@ -0,0 +1,145 @@
> +#!/usr/bin/perl -w
> +# This is part of "osstest", an automated testing framework for Xen.
> +# Copyright (C) 2019 Citrix Inc.
...
> +# Consumes the following input runvars:
> +# svnrevision_freebsdports: ports svn revision id to use.
> +# svntree_freebsdports ports svn tree to fetch the source code from.
More regular in osstest terms would be
tree_freebsdports
revision_freebsdports
treevcs_freebsdports=svn
But I guess svn is sufficiently unlike what osstest expects out of a
vcs that this is not feasible, and it is better to do it this way.
> +sub checkout () {
> + my $u = URI->new($c{HttpProxy});
> + my $host = $u->host;
> + my $port = $u->port;
> + prepbuilddirs();
> +
> + logm("Checkout ports tree from svn");
> + target_cmd_build($ho, 4000, $builddir, <<END);
> +cd $builddir
> +rm -rf ports
> +# svn ignores HTTP_PROXY envvar
> +svnlite checkout --config-option servers:global:http-proxy-host=$host \\
> + --config-option servers:global:http-proxy-port=$port \\
> + --trust-server-cert \\
> + $r{"svntree_freebsdports"} \\
> + -r $r{"svnrevision_freebsdports"} ports
Will this work to cache the checkout ? All of this says http but I
assume it's really https ? Typically, https clients expect to do the
TLS themselves but I think you're using our squid mitm and that's what
"--trust-server-cert" is doing ?
Rather than "--trust-server-cert" which disables TLS's own mitm
protection it would be rather better to inject the osstest mitm squid
cert into the testbed, but that may be difficult, and the risk is only
from internal things between the build (test) box and the proxy.
> +sub create_jail() {
> + my $src_prefix = $r{"freebsd_distpath"} ||
> + get_stashed("path_freebsddist", $r{"freebsdbuildjob"});
> + my $dst_prefix = "/root/sets";
Do we need a jail for this ? We have a whole baremetal OS install
whose entire purpose is to do this build ...
> +logm("FreeBSD packages built successful");
^ly :-)
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
@ 2019-05-23 10:38 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 10:38 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository"):
> diff --git a/make-freebsd-flight b/make-freebsd-flight
> index d3c413b5..fc3d2d83 100755
> --- a/make-freebsd-flight
> +++ b/make-freebsd-flight
> @@ -38,13 +38,15 @@ job_create_build_filter_callback () {
>
> for arch in "$arches"; do
> set_freebsd_runvars
> -
> create_freebsd_build_job build-$arch-freebsd
>
> - # Create an identical job that's going to use the build output from
> - # the previous one.
> + # Create a job to build the packages against the new world.
> freebsd_runvars="$freebsd_runvars freebsdbuildjob=build-$arch-freebsd \
> recipe_testinstall=true"
> + create_freebsd_pkg_build_job build-$arch-freebsd-packages
> +
> + # Create an identical job that's going to use the build output from
> + # the previous one.
> create_freebsd_build_job build-$arch-freebsd-again
>
> # Create a Xen build job that's going to use the output from the first
This looks OK.
> @@ -768,7 +773,9 @@ proc prepare-build-host-freebsd {} {
> global jobinfo
> if {[recipe-flag testinstall]} { set broken fail } { set broken broken }
> run-ts $broken host-install(*) ts-freebsd-host-install
> - run-ts . host-build-prep ts-build-prep-freebsd
> + if {![recipe-flag skipbuildprep]} {
> + run-ts . host-build-prep ts-build-prep-freebsd
What's this for ? Oh, I see.
I notice that none of your freebsd build jobs pass any share- hostflag
so they always use a fresh installation. Is that necessary ?
> proc need-hosts/coverity {} { return BUILD_LINUX }
> diff --git a/ts-freebsd-build-packages b/ts-freebsd-build-packages
> new file mode 100755
> index 00000000..9202dd9f
> --- /dev/null
> +++ b/ts-freebsd-build-packages
> @@ -0,0 +1,145 @@
> +#!/usr/bin/perl -w
> +# This is part of "osstest", an automated testing framework for Xen.
> +# Copyright (C) 2019 Citrix Inc.
...
> +# Consumes the following input runvars:
> +# svnrevision_freebsdports: ports svn revision id to use.
> +# svntree_freebsdports ports svn tree to fetch the source code from.
More regular in osstest terms would be
tree_freebsdports
revision_freebsdports
treevcs_freebsdports=svn
But I guess svn is sufficiently unlike what osstest expects out of a
vcs that this is not feasible, and it is better to do it this way.
> +sub checkout () {
> + my $u = URI->new($c{HttpProxy});
> + my $host = $u->host;
> + my $port = $u->port;
> + prepbuilddirs();
> +
> + logm("Checkout ports tree from svn");
> + target_cmd_build($ho, 4000, $builddir, <<END);
> +cd $builddir
> +rm -rf ports
> +# svn ignores HTTP_PROXY envvar
> +svnlite checkout --config-option servers:global:http-proxy-host=$host \\
> + --config-option servers:global:http-proxy-port=$port \\
> + --trust-server-cert \\
> + $r{"svntree_freebsdports"} \\
> + -r $r{"svnrevision_freebsdports"} ports
Will this work to cache the checkout ? All of this says http but I
assume it's really https ? Typically, https clients expect to do the
TLS themselves but I think you're using our squid mitm and that's what
"--trust-server-cert" is doing ?
Rather than "--trust-server-cert" which disables TLS's own mitm
protection it would be rather better to inject the osstest mitm squid
cert into the testbed, but that may be difficult, and the risk is only
from internal things between the build (test) box and the proxy.
> +sub create_jail() {
> + my $src_prefix = $r{"freebsd_distpath"} ||
> + get_stashed("path_freebsddist", $r{"freebsdbuildjob"});
> + my $dst_prefix = "/root/sets";
Do we need a jail for this ? We have a whole baremetal OS install
whose entire purpose is to do this build ...
> +logm("FreeBSD packages built successful");
^ly :-)
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD
@ 2019-05-23 11:06 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 11:06 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD"):
> This removes the dependency on the official pkg repository, which is
> dangerous when not testing stable branches, since the ABI of the
> development branch is not stable, and thus it's easy for packages to
> get out of sync, or for osstest to test an old FreeBSD version that
> has an ABI different than the one used to build the official
> packages.
I realise this is a bit late to be saying this, but had you
considered making the packages build a different step in the same
job ? That might make a lot of this go away...
> IFS=$'\n'
> +count=0
> for anointed in \
> - `./mg-anoint list-prepared "freebsd build $freebsd_branch *"`; do
> + `./mg-anoint list-prepared "freebsd* build $freebsd_branch *"`; do
> # Retrieve previous successful FreeBSD build for each arch.
> freebsd_arch=${anointed##* }
> - freebsd_envvar="FREEBSD_${freebsd_arch^^}_BUILDJOB"
> + freebsd_name=${anointed%% *}
> + freebsd_name=${freebsd_name/-/_}
> + freebsd_envvar="${freebsd_name^^}_${freebsd_arch^^}_BUILDJOB"
> if [ "x${!freebsd_envvar}" = "x" ]; then
> - flight_job=`./mg-anoint retrieve "$anointed"`
> - export ${freebsd_envvar}=${flight_job/ /.}
> + envvars[$count]="$freebsd_envvar"
> + refkeys[$count]="$anointed"
> + count=$((count+1))
You don't need this counter. You can just say
envvars=()
...
envvars+=("$freebsd_envvar")
etc.
> + fi
> +done
> +count=0
> +for flight_job in `./mg-anoint retrieve ${refkeys[@]}`; do
> + if [ "$flight_job" != "ERROR" ]; then
> + export ${envvars[$count]}=${flight_job/ /.}
> fi
> + count=$((count+1))
I think you do need count here, if you do this as two loops. But:
Why not do this retrieve, and set the env vars, inside the first
loop ? I think that would avoid having to accumulate a data structure
full of information in shell variables at all (and shell is not very
good at this kind of thing).
> @@ -542,17 +553,23 @@ freebsd-*)
> [ "x$OSSTEST_BLESSING" == "xreal" ]; then
> IFS=$'\n'
> for anointed in `./mg-anoint list-prepared \
> - "freebsd build $freebsd_branch *"`; do
> + "freebsd* build $freebsd_branch *"`; do
> # Update anointed versions
> # NB: failure to update an anointed build for a specific arch
> # should not be fatal, and it's not an issue if one of the
> # arches gets slightly out of sync with the other ones.
> freebsd_arch=${anointed##* }
> - if ./mg-anoint anoint "$anointed" \
> - $flight build-$freebsd_arch-freebsd; then
> - echo "Anointed artifacts from build-$freebsd_arch-freebsd"
> - fi
> + freebsd_name=${anointed%% *}
> + # Rely on the fact that the job suffix is the same as the
> + # anointment refkey. Ie:
I don't think you mean "Rely on the fact". Rather, "by definition,
from the way the flight is constructed, the intended..." ?
> + # refkey: freebsd job: build-<arch>-freebsd
> + # refkey: freebsd-packages job: build-<arch>-freebsd-packages
> + anoint="$anoint \"$anointed\" $flight \
> + build-$freebsd_arch-$freebsd_name"
Maybe use an array variable for anount, and then you can avoid the
shell \" quoting.
> diff --git a/mfi-common b/mfi-common
> index 83d3c713..12cde85f 100644
> --- a/mfi-common
> +++ b/mfi-common
> @@ -156,7 +156,6 @@ set_freebsd_runvars () {
...
> + # Check if the packages are provided externally, or else assume they
> + # are provided by the same flight as the installer binaries.
> + local pkgpath=`getconfig "FreeBSDPackages"`
> + counter=0
> + IFS=$'\n'
> + for flightjob in `./mg-anoint retrieve --tolerate-unprepared \
> + "freebsd build master $arch" \
> + "freebsd-packages build master $arch"`; do
> + if [ $counter -eq 0 ]; then
> + # Anointed FreeBSD installer
I don't much like this code, but I'm having trouble saying what I
think should be done instead.
I don't much like the $counter -eq 0 approach. Maybe some of it
should go into a function ?
./mg-anoint retrieve ... >tmpfile
if freebsd_want_anointed <tmpfile '' Dist ...
but not sure what the function should be.
> + local envvar="FREEBSD_${arch^^}_BUILDJOB"
> + local distpath=`getconfig "FreeBSDDist"`
> + if [ -n "${!envvar}" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsdbuildjob=${!envvar}"
> + elif [ -n "$FREEBSD_DIST" ] && [ -n "$FREEBSD_VERSION" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsd_distpath=$FREEBSD_DIST/$arch \
> + freebsd_version=$FREEBSD_VERSION"
> + elif [ -n "$distpath" ]; then
> + local version=`getconfig "FreeBSDVersion"`
> + freebsd_runvars="$freebsd_runvars \
> + freebsd_distpath=$distpath/$arch \
> + freebsd_version=$version"
> + elif [ "$flightjob" != "ERROR" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsdbuildjob=${flightjob/ /.}"
There seems like a lot of repetition here. For example, FREEBSD_DIST
overrides FreeBSDDist but /$arch is appended in two places. Maybe
${FREEBSD_DIST- ... something ... } would be better ?
It is difficult to see the wood for the trees, particularly with the
constant repetition of
freebsd_runvars="$freebsd_runvars \
which could be avoided by having this fragment set a local variable
containing the things to be added.
> + elif [ $counter -eq 1 ]; then
> + # Anointed package repository
> + local envvar="FREEBSD_PACKAGES_${arch^^}_BUILDJOB"
> + local pkgpath=`getconfig "FreeBSDPackages"`
> + if [ -n "${!envvar}" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsdpackagesbuildjob=${!envvar}"
> + elif [ -n "$FREEBSD_PACKAGES" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsd_packages=$FREEBSD_PACKAGES/$arch"
> + elif [ -n "$pkgpath" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsd_packages=$pkgpath/$arch"
> + elif [ "$flightjob" != "ERROR" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsdpackagesbuildjob=${flightjob/ /.}"
This feels very similar to the code above, although it lacks the
special handling for the version.
The rest of this looks plausible.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD
@ 2019-05-23 11:06 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-23 11:06 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("[Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD"):
> This removes the dependency on the official pkg repository, which is
> dangerous when not testing stable branches, since the ABI of the
> development branch is not stable, and thus it's easy for packages to
> get out of sync, or for osstest to test an old FreeBSD version that
> has an ABI different than the one used to build the official
> packages.
I realise this is a bit late to be saying this, but had you
considered making the packages build a different step in the same
job ? That might make a lot of this go away...
> IFS=$'\n'
> +count=0
> for anointed in \
> - `./mg-anoint list-prepared "freebsd build $freebsd_branch *"`; do
> + `./mg-anoint list-prepared "freebsd* build $freebsd_branch *"`; do
> # Retrieve previous successful FreeBSD build for each arch.
> freebsd_arch=${anointed##* }
> - freebsd_envvar="FREEBSD_${freebsd_arch^^}_BUILDJOB"
> + freebsd_name=${anointed%% *}
> + freebsd_name=${freebsd_name/-/_}
> + freebsd_envvar="${freebsd_name^^}_${freebsd_arch^^}_BUILDJOB"
> if [ "x${!freebsd_envvar}" = "x" ]; then
> - flight_job=`./mg-anoint retrieve "$anointed"`
> - export ${freebsd_envvar}=${flight_job/ /.}
> + envvars[$count]="$freebsd_envvar"
> + refkeys[$count]="$anointed"
> + count=$((count+1))
You don't need this counter. You can just say
envvars=()
...
envvars+=("$freebsd_envvar")
etc.
> + fi
> +done
> +count=0
> +for flight_job in `./mg-anoint retrieve ${refkeys[@]}`; do
> + if [ "$flight_job" != "ERROR" ]; then
> + export ${envvars[$count]}=${flight_job/ /.}
> fi
> + count=$((count+1))
I think you do need count here, if you do this as two loops. But:
Why not do this retrieve, and set the env vars, inside the first
loop ? I think that would avoid having to accumulate a data structure
full of information in shell variables at all (and shell is not very
good at this kind of thing).
> @@ -542,17 +553,23 @@ freebsd-*)
> [ "x$OSSTEST_BLESSING" == "xreal" ]; then
> IFS=$'\n'
> for anointed in `./mg-anoint list-prepared \
> - "freebsd build $freebsd_branch *"`; do
> + "freebsd* build $freebsd_branch *"`; do
> # Update anointed versions
> # NB: failure to update an anointed build for a specific arch
> # should not be fatal, and it's not an issue if one of the
> # arches gets slightly out of sync with the other ones.
> freebsd_arch=${anointed##* }
> - if ./mg-anoint anoint "$anointed" \
> - $flight build-$freebsd_arch-freebsd; then
> - echo "Anointed artifacts from build-$freebsd_arch-freebsd"
> - fi
> + freebsd_name=${anointed%% *}
> + # Rely on the fact that the job suffix is the same as the
> + # anointment refkey. Ie:
I don't think you mean "Rely on the fact". Rather, "by definition,
from the way the flight is constructed, the intended..." ?
> + # refkey: freebsd job: build-<arch>-freebsd
> + # refkey: freebsd-packages job: build-<arch>-freebsd-packages
> + anoint="$anoint \"$anointed\" $flight \
> + build-$freebsd_arch-$freebsd_name"
Maybe use an array variable for anount, and then you can avoid the
shell \" quoting.
> diff --git a/mfi-common b/mfi-common
> index 83d3c713..12cde85f 100644
> --- a/mfi-common
> +++ b/mfi-common
> @@ -156,7 +156,6 @@ set_freebsd_runvars () {
...
> + # Check if the packages are provided externally, or else assume they
> + # are provided by the same flight as the installer binaries.
> + local pkgpath=`getconfig "FreeBSDPackages"`
> + counter=0
> + IFS=$'\n'
> + for flightjob in `./mg-anoint retrieve --tolerate-unprepared \
> + "freebsd build master $arch" \
> + "freebsd-packages build master $arch"`; do
> + if [ $counter -eq 0 ]; then
> + # Anointed FreeBSD installer
I don't much like this code, but I'm having trouble saying what I
think should be done instead.
I don't much like the $counter -eq 0 approach. Maybe some of it
should go into a function ?
./mg-anoint retrieve ... >tmpfile
if freebsd_want_anointed <tmpfile '' Dist ...
but not sure what the function should be.
> + local envvar="FREEBSD_${arch^^}_BUILDJOB"
> + local distpath=`getconfig "FreeBSDDist"`
> + if [ -n "${!envvar}" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsdbuildjob=${!envvar}"
> + elif [ -n "$FREEBSD_DIST" ] && [ -n "$FREEBSD_VERSION" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsd_distpath=$FREEBSD_DIST/$arch \
> + freebsd_version=$FREEBSD_VERSION"
> + elif [ -n "$distpath" ]; then
> + local version=`getconfig "FreeBSDVersion"`
> + freebsd_runvars="$freebsd_runvars \
> + freebsd_distpath=$distpath/$arch \
> + freebsd_version=$version"
> + elif [ "$flightjob" != "ERROR" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsdbuildjob=${flightjob/ /.}"
There seems like a lot of repetition here. For example, FREEBSD_DIST
overrides FreeBSDDist but /$arch is appended in two places. Maybe
${FREEBSD_DIST- ... something ... } would be better ?
It is difficult to see the wood for the trees, particularly with the
constant repetition of
freebsd_runvars="$freebsd_runvars \
which could be avoided by having this fragment set a local variable
containing the things to be added.
> + elif [ $counter -eq 1 ]; then
> + # Anointed package repository
> + local envvar="FREEBSD_PACKAGES_${arch^^}_BUILDJOB"
> + local pkgpath=`getconfig "FreeBSDPackages"`
> + if [ -n "${!envvar}" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsdpackagesbuildjob=${!envvar}"
> + elif [ -n "$FREEBSD_PACKAGES" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsd_packages=$FREEBSD_PACKAGES/$arch"
> + elif [ -n "$pkgpath" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsd_packages=$pkgpath/$arch"
> + elif [ "$flightjob" != "ERROR" ]; then
> + freebsd_runvars="$freebsd_runvars \
> + freebsdpackagesbuildjob=${flightjob/ /.}"
This feels very similar to the code above, although it lacks the
special handling for the version.
The rest of this looks plausible.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/6] osstest: introduce a helper to stash a whole directory
@ 2019-05-24 9:42 ` Roger Pau Monné
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monné @ 2019-05-24 9:42 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Thu, May 23, 2019 at 10:48:12AM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 1/6] osstest: introduce a helper to stash a whole directory"):
> > Without compressing it.
>
> You've open-coded a recursive directory copy. Is rsync available on
> $ho at this point ? I think maybe it could be ...
D'oh, yes, rsync could be made available on $ho at this point.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 1/6] osstest: introduce a helper to stash a whole directory
@ 2019-05-24 9:42 ` Roger Pau Monné
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monné @ 2019-05-24 9:42 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Thu, May 23, 2019 at 10:48:12AM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 1/6] osstest: introduce a helper to stash a whole directory"):
> > Without compressing it.
>
> You've open-coded a recursive directory copy. Is rsync available on
> $ho at this point ? I think maybe it could be ...
D'oh, yes, rsync could be made available on $ho at this point.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit
@ 2019-05-24 9:57 ` Roger Pau Monné
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monné @ 2019-05-24 9:57 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Thu, May 23, 2019 at 11:03:52AM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit"):
> > This only works when the svn revision is stored as a git note
> > with the format 'revision=<revision number>'.
>
> Wow. This is pretty ugly.
Indeed :(.
>
> > Such conversion is required in order to bootstrap a FreeBSD system
> > without relying on external package repositories. FreeBSD base system
> > only contains a subversion client (no git client), and thus in order
> > to fetch the ports repository (that contain the external packages
> > build makefiles) svn must be used.
>
> git notes have some different way of travelling than commits, don't
> they ? Where is this git note coming from and how do we know it is
> the right note, IYSWIM ?
I'm not an expert on this, but I think notes are always stored in a
separate branch on the same repo? In the FreeBSD case at least it's
git/refs/notes.
> Aside from that, please break the refactoring (in this case, the
> breaking out of repo_get_realurl) into a separate NFC patch.
Sure!
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit
@ 2019-05-24 9:57 ` Roger Pau Monné
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monné @ 2019-05-24 9:57 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Thu, May 23, 2019 at 11:03:52AM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit"):
> > This only works when the svn revision is stored as a git note
> > with the format 'revision=<revision number>'.
>
> Wow. This is pretty ugly.
Indeed :(.
>
> > Such conversion is required in order to bootstrap a FreeBSD system
> > without relying on external package repositories. FreeBSD base system
> > only contains a subversion client (no git client), and thus in order
> > to fetch the ports repository (that contain the external packages
> > build makefiles) svn must be used.
>
> git notes have some different way of travelling than commits, don't
> they ? Where is this git note coming from and how do we know it is
> the right note, IYSWIM ?
I'm not an expert on this, but I think notes are always stored in a
separate branch on the same repo? In the FreeBSD case at least it's
git/refs/notes.
> Aside from that, please break the refactoring (in this case, the
> breaking out of repo_get_realurl) into a separate NFC patch.
Sure!
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
@ 2019-05-24 10:13 ` Roger Pau Monné
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monné @ 2019-05-24 10:13 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Thu, May 23, 2019 at 11:38:57AM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository"):
> > diff --git a/make-freebsd-flight b/make-freebsd-flight
> > index d3c413b5..fc3d2d83 100755
> > --- a/make-freebsd-flight
> > +++ b/make-freebsd-flight
> > @@ -38,13 +38,15 @@ job_create_build_filter_callback () {
> >
> > for arch in "$arches"; do
> > set_freebsd_runvars
> > -
> > create_freebsd_build_job build-$arch-freebsd
> >
> > - # Create an identical job that's going to use the build output from
> > - # the previous one.
> > + # Create a job to build the packages against the new world.
> > freebsd_runvars="$freebsd_runvars freebsdbuildjob=build-$arch-freebsd \
> > recipe_testinstall=true"
> > + create_freebsd_pkg_build_job build-$arch-freebsd-packages
> > +
> > + # Create an identical job that's going to use the build output from
> > + # the previous one.
> > create_freebsd_build_job build-$arch-freebsd-again
> >
> > # Create a Xen build job that's going to use the output from the first
>
> This looks OK.
>
> > @@ -768,7 +773,9 @@ proc prepare-build-host-freebsd {} {
> > global jobinfo
> > if {[recipe-flag testinstall]} { set broken fail } { set broken broken }
> > run-ts $broken host-install(*) ts-freebsd-host-install
> > - run-ts . host-build-prep ts-build-prep-freebsd
> > + if {![recipe-flag skipbuildprep]} {
> > + run-ts . host-build-prep ts-build-prep-freebsd
>
> What's this for ? Oh, I see.
The job that creates the package repository cannot use build-prep
because the packages are not yet built.
> I notice that none of your freebsd build jobs pass any share- hostflag
> so they always use a fresh installation. Is that necessary ?
Hm, I don't think so. build-amd64-xen-freebsd and
build-amd64-freebsd-again could share a host. I need to take a look at
how to do this, I could send this as a separate fix for the existing
jobs.
> > proc need-hosts/coverity {} { return BUILD_LINUX }
> > diff --git a/ts-freebsd-build-packages b/ts-freebsd-build-packages
> > new file mode 100755
> > index 00000000..9202dd9f
> > --- /dev/null
> > +++ b/ts-freebsd-build-packages
> > @@ -0,0 +1,145 @@
> > +#!/usr/bin/perl -w
> > +# This is part of "osstest", an automated testing framework for Xen.
> > +# Copyright (C) 2019 Citrix Inc.
> ...
> > +# Consumes the following input runvars:
> > +# svnrevision_freebsdports: ports svn revision id to use.
> > +# svntree_freebsdports ports svn tree to fetch the source code from.
>
> More regular in osstest terms would be
> tree_freebsdports
> revision_freebsdports
> treevcs_freebsdports=svn
> But I guess svn is sufficiently unlike what osstest expects out of a
> vcs that this is not feasible, and it is better to do it this way.
I don't really have an opinion, I somehow assumed that using the same
format might interfere with things like bisection, so I've decided to
pass the git revision using tree_freebsdports &c and the svn revision
using the newly introduced flags.
> > +sub checkout () {
> > + my $u = URI->new($c{HttpProxy});
> > + my $host = $u->host;
> > + my $port = $u->port;
> > + prepbuilddirs();
> > +
> > + logm("Checkout ports tree from svn");
> > + target_cmd_build($ho, 4000, $builddir, <<END);
> > +cd $builddir
> > +rm -rf ports
> > +# svn ignores HTTP_PROXY envvar
> > +svnlite checkout --config-option servers:global:http-proxy-host=$host \\
> > + --config-option servers:global:http-proxy-port=$port \\
> > + --trust-server-cert \\
> > + $r{"svntree_freebsdports"} \\
> > + -r $r{"svnrevision_freebsdports"} ports
>
> Will this work to cache the checkout ?
I think so? Would https somehow prevent the caching?
> All of this says http but I
> assume it's really https ?
AFAIK svn uses the http-proxy options for both http and https.
> Typically, https clients expect to do the
> TLS themselves but I think you're using our squid mitm and that's what
> "--trust-server-cert" is doing ?
I can't really remember why I've added this option, but I'm quite
sure it was failing without it. As you say the proxy is acting as a
mitm, so that's likely why trust-server-cert is required.
> Rather than "--trust-server-cert" which disables TLS's own mitm
> protection it would be rather better to inject the osstest mitm squid
> cert into the testbed, but that may be difficult, and the risk is only
> from internal things between the build (test) box and the proxy.
I can look into this, but at the end of day this is all internal, so
I'm not sure there's a lot of risk here.
> > +sub create_jail() {
> > + my $src_prefix = $r{"freebsd_distpath"} ||
> > + get_stashed("path_freebsddist", $r{"freebsdbuildjob"});
> > + my $dst_prefix = "/root/sets";
>
> Do we need a jail for this ? We have a whole baremetal OS install
> whose entire purpose is to do this build ...
Yes, that's how the repository package builder (poudriere) works, it
requires a jail to do the package building. In our case it's not so
important, but I assume this is mostly done to always use a clean
install, so that currently installed packages on the system don't
interfere with package building.
> > +logm("FreeBSD packages built successful");
> ^ly :-)
Thanks.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
@ 2019-05-24 10:13 ` Roger Pau Monné
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monné @ 2019-05-24 10:13 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Thu, May 23, 2019 at 11:38:57AM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository"):
> > diff --git a/make-freebsd-flight b/make-freebsd-flight
> > index d3c413b5..fc3d2d83 100755
> > --- a/make-freebsd-flight
> > +++ b/make-freebsd-flight
> > @@ -38,13 +38,15 @@ job_create_build_filter_callback () {
> >
> > for arch in "$arches"; do
> > set_freebsd_runvars
> > -
> > create_freebsd_build_job build-$arch-freebsd
> >
> > - # Create an identical job that's going to use the build output from
> > - # the previous one.
> > + # Create a job to build the packages against the new world.
> > freebsd_runvars="$freebsd_runvars freebsdbuildjob=build-$arch-freebsd \
> > recipe_testinstall=true"
> > + create_freebsd_pkg_build_job build-$arch-freebsd-packages
> > +
> > + # Create an identical job that's going to use the build output from
> > + # the previous one.
> > create_freebsd_build_job build-$arch-freebsd-again
> >
> > # Create a Xen build job that's going to use the output from the first
>
> This looks OK.
>
> > @@ -768,7 +773,9 @@ proc prepare-build-host-freebsd {} {
> > global jobinfo
> > if {[recipe-flag testinstall]} { set broken fail } { set broken broken }
> > run-ts $broken host-install(*) ts-freebsd-host-install
> > - run-ts . host-build-prep ts-build-prep-freebsd
> > + if {![recipe-flag skipbuildprep]} {
> > + run-ts . host-build-prep ts-build-prep-freebsd
>
> What's this for ? Oh, I see.
The job that creates the package repository cannot use build-prep
because the packages are not yet built.
> I notice that none of your freebsd build jobs pass any share- hostflag
> so they always use a fresh installation. Is that necessary ?
Hm, I don't think so. build-amd64-xen-freebsd and
build-amd64-freebsd-again could share a host. I need to take a look at
how to do this, I could send this as a separate fix for the existing
jobs.
> > proc need-hosts/coverity {} { return BUILD_LINUX }
> > diff --git a/ts-freebsd-build-packages b/ts-freebsd-build-packages
> > new file mode 100755
> > index 00000000..9202dd9f
> > --- /dev/null
> > +++ b/ts-freebsd-build-packages
> > @@ -0,0 +1,145 @@
> > +#!/usr/bin/perl -w
> > +# This is part of "osstest", an automated testing framework for Xen.
> > +# Copyright (C) 2019 Citrix Inc.
> ...
> > +# Consumes the following input runvars:
> > +# svnrevision_freebsdports: ports svn revision id to use.
> > +# svntree_freebsdports ports svn tree to fetch the source code from.
>
> More regular in osstest terms would be
> tree_freebsdports
> revision_freebsdports
> treevcs_freebsdports=svn
> But I guess svn is sufficiently unlike what osstest expects out of a
> vcs that this is not feasible, and it is better to do it this way.
I don't really have an opinion, I somehow assumed that using the same
format might interfere with things like bisection, so I've decided to
pass the git revision using tree_freebsdports &c and the svn revision
using the newly introduced flags.
> > +sub checkout () {
> > + my $u = URI->new($c{HttpProxy});
> > + my $host = $u->host;
> > + my $port = $u->port;
> > + prepbuilddirs();
> > +
> > + logm("Checkout ports tree from svn");
> > + target_cmd_build($ho, 4000, $builddir, <<END);
> > +cd $builddir
> > +rm -rf ports
> > +# svn ignores HTTP_PROXY envvar
> > +svnlite checkout --config-option servers:global:http-proxy-host=$host \\
> > + --config-option servers:global:http-proxy-port=$port \\
> > + --trust-server-cert \\
> > + $r{"svntree_freebsdports"} \\
> > + -r $r{"svnrevision_freebsdports"} ports
>
> Will this work to cache the checkout ?
I think so? Would https somehow prevent the caching?
> All of this says http but I
> assume it's really https ?
AFAIK svn uses the http-proxy options for both http and https.
> Typically, https clients expect to do the
> TLS themselves but I think you're using our squid mitm and that's what
> "--trust-server-cert" is doing ?
I can't really remember why I've added this option, but I'm quite
sure it was failing without it. As you say the proxy is acting as a
mitm, so that's likely why trust-server-cert is required.
> Rather than "--trust-server-cert" which disables TLS's own mitm
> protection it would be rather better to inject the osstest mitm squid
> cert into the testbed, but that may be difficult, and the risk is only
> from internal things between the build (test) box and the proxy.
I can look into this, but at the end of day this is all internal, so
I'm not sure there's a lot of risk here.
> > +sub create_jail() {
> > + my $src_prefix = $r{"freebsd_distpath"} ||
> > + get_stashed("path_freebsddist", $r{"freebsdbuildjob"});
> > + my $dst_prefix = "/root/sets";
>
> Do we need a jail for this ? We have a whole baremetal OS install
> whose entire purpose is to do this build ...
Yes, that's how the repository package builder (poudriere) works, it
requires a jail to do the package building. In our case it's not so
important, but I assume this is mostly done to always use a clean
install, so that currently installed packages on the system don't
interfere with package building.
> > +logm("FreeBSD packages built successful");
> ^ly :-)
Thanks.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit
@ 2019-05-24 10:35 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-24 10:35 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit"):
> On Thu, May 23, 2019 at 11:03:52AM +0100, Ian Jackson wrote:
> > git notes have some different way of travelling than commits, don't
> > they ? Where is this git note coming from and how do we know it is
> > the right note, IYSWIM ?
>
> I'm not an expert on this, but I think notes are always stored in a
> separate branch on the same repo? In the FreeBSD case at least it's
> git/refs/notes.
OK, so what are git's rules for where it fetches notes from ? Which
notes does it look at ? (Do you understand why I am asking these
questions?)
> > Aside from that, please break the refactoring (in this case, the
> > breaking out of repo_get_realurl) into a separate NFC patch.
>
> Sure!
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit
@ 2019-05-24 10:35 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-24 10:35 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit"):
> On Thu, May 23, 2019 at 11:03:52AM +0100, Ian Jackson wrote:
> > git notes have some different way of travelling than commits, don't
> > they ? Where is this git note coming from and how do we know it is
> > the right note, IYSWIM ?
>
> I'm not an expert on this, but I think notes are always stored in a
> separate branch on the same repo? In the FreeBSD case at least it's
> git/refs/notes.
OK, so what are git's rules for where it fetches notes from ? Which
notes does it look at ? (Do you understand why I am asking these
questions?)
> > Aside from that, please break the refactoring (in this case, the
> > breaking out of repo_get_realurl) into a separate NFC patch.
>
> Sure!
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD
@ 2019-05-24 10:45 ` Roger Pau Monné
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monné @ 2019-05-24 10:45 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Thu, May 23, 2019 at 12:06:29PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD"):
> > This removes the dependency on the official pkg repository, which is
> > dangerous when not testing stable branches, since the ABI of the
> > development branch is not stable, and thus it's easy for packages to
> > get out of sync, or for osstest to test an old FreeBSD version that
> > has an ABI different than the one used to build the official
> > packages.
>
> I realise this is a bit late to be saying this, but had you
> considered making the packages build a different step in the same
> job ? That might make a lot of this go away...
Do you mean to build the packages in build-prep instead of relying on
having a custom binary repository?
I could do that, but it's going to take some time. Also doing the
package build in build-prep would require fetching the svn ports
repository for each build-prep instance.
IIRC the package building job takes a non-trivial amount of time (2-3h
IIRC), because it has to build gcc (for SeaBIOS) and python, perl...
> > IFS=$'\n'
> > +count=0
> > for anointed in \
> > - `./mg-anoint list-prepared "freebsd build $freebsd_branch *"`; do
> > + `./mg-anoint list-prepared "freebsd* build $freebsd_branch *"`; do
> > # Retrieve previous successful FreeBSD build for each arch.
> > freebsd_arch=${anointed##* }
> > - freebsd_envvar="FREEBSD_${freebsd_arch^^}_BUILDJOB"
> > + freebsd_name=${anointed%% *}
> > + freebsd_name=${freebsd_name/-/_}
> > + freebsd_envvar="${freebsd_name^^}_${freebsd_arch^^}_BUILDJOB"
> > if [ "x${!freebsd_envvar}" = "x" ]; then
> > - flight_job=`./mg-anoint retrieve "$anointed"`
> > - export ${freebsd_envvar}=${flight_job/ /.}
> > + envvars[$count]="$freebsd_envvar"
> > + refkeys[$count]="$anointed"
> > + count=$((count+1))
>
> You don't need this counter. You can just say
> envvars=()
> ...
> envvars+=("$freebsd_envvar")
Oh, thanks!
> > + fi
> > +done
> > +count=0
> > +for flight_job in `./mg-anoint retrieve ${refkeys[@]}`; do
> > + if [ "$flight_job" != "ERROR" ]; then
> > + export ${envvars[$count]}=${flight_job/ /.}
> > fi
> > + count=$((count+1))
>
> I think you do need count here, if you do this as two loops. But:
>
> Why not do this retrieve, and set the env vars, inside the first
> loop ? I think that would avoid having to accumulate a data structure
> full of information in shell variables at all (and shell is not very
> good at this kind of thing).
Yes, I think I can do it as you suggest.
> > @@ -542,17 +553,23 @@ freebsd-*)
> > [ "x$OSSTEST_BLESSING" == "xreal" ]; then
> > IFS=$'\n'
> > for anointed in `./mg-anoint list-prepared \
> > - "freebsd build $freebsd_branch *"`; do
> > + "freebsd* build $freebsd_branch *"`; do
> > # Update anointed versions
> > # NB: failure to update an anointed build for a specific arch
> > # should not be fatal, and it's not an issue if one of the
> > # arches gets slightly out of sync with the other ones.
> > freebsd_arch=${anointed##* }
> > - if ./mg-anoint anoint "$anointed" \
> > - $flight build-$freebsd_arch-freebsd; then
> > - echo "Anointed artifacts from build-$freebsd_arch-freebsd"
> > - fi
> > + freebsd_name=${anointed%% *}
> > + # Rely on the fact that the job suffix is the same as the
> > + # anointment refkey. Ie:
>
> I don't think you mean "Rely on the fact". Rather, "by definition,
> from the way the flight is constructed, the intended..." ?
>
> > + # refkey: freebsd job: build-<arch>-freebsd
> > + # refkey: freebsd-packages job: build-<arch>-freebsd-packages
> > + anoint="$anoint \"$anointed\" $flight \
> > + build-$freebsd_arch-$freebsd_name"
>
> Maybe use an array variable for anount, and then you can avoid the
> shell \" quoting.
Please bear with me, but can you elaborate on this?
> > diff --git a/mfi-common b/mfi-common
> > index 83d3c713..12cde85f 100644
> > --- a/mfi-common
> > +++ b/mfi-common
> > @@ -156,7 +156,6 @@ set_freebsd_runvars () {
> ...
> > + # Check if the packages are provided externally, or else assume they
> > + # are provided by the same flight as the installer binaries.
> > + local pkgpath=`getconfig "FreeBSDPackages"`
> > + counter=0
> > + IFS=$'\n'
> > + for flightjob in `./mg-anoint retrieve --tolerate-unprepared \
> > + "freebsd build master $arch" \
> > + "freebsd-packages build master $arch"`; do
> > + if [ $counter -eq 0 ]; then
> > + # Anointed FreeBSD installer
>
> I don't much like this code, but I'm having trouble saying what I
> think should be done instead.
>
> I don't much like the $counter -eq 0 approach. Maybe some of it
> should go into a function ?
> ./mg-anoint retrieve ... >tmpfile
> if freebsd_want_anointed <tmpfile '' Dist ...
> but not sure what the function should be.
Yes, the counter stuff is nasty. I guess I could change the output of
mg-anoint so it signals which output line belongs to which input
parameter, but there's always going to be multiple lines, and I wanted
to avoid having to change much of mg-anoint.
>
> > + local envvar="FREEBSD_${arch^^}_BUILDJOB"
> > + local distpath=`getconfig "FreeBSDDist"`
> > + if [ -n "${!envvar}" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsdbuildjob=${!envvar}"
> > + elif [ -n "$FREEBSD_DIST" ] && [ -n "$FREEBSD_VERSION" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsd_distpath=$FREEBSD_DIST/$arch \
> > + freebsd_version=$FREEBSD_VERSION"
> > + elif [ -n "$distpath" ]; then
> > + local version=`getconfig "FreeBSDVersion"`
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsd_distpath=$distpath/$arch \
> > + freebsd_version=$version"
> > + elif [ "$flightjob" != "ERROR" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsdbuildjob=${flightjob/ /.}"
>
> There seems like a lot of repetition here. For example, FREEBSD_DIST
> overrides FreeBSDDist but /$arch is appended in two places. Maybe
> ${FREEBSD_DIST- ... something ... } would be better ?
OK, let me try to remove some of the duplication here.
> It is difficult to see the wood for the trees, particularly with the
> constant repetition of
> freebsd_runvars="$freebsd_runvars \
> which could be avoided by having this fragment set a local variable
> containing the things to be added.
Hm, indeed. Using a local variable and then appending it when done
would make it clearer.
> > + elif [ $counter -eq 1 ]; then
> > + # Anointed package repository
> > + local envvar="FREEBSD_PACKAGES_${arch^^}_BUILDJOB"
> > + local pkgpath=`getconfig "FreeBSDPackages"`
> > + if [ -n "${!envvar}" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsdpackagesbuildjob=${!envvar}"
> > + elif [ -n "$FREEBSD_PACKAGES" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsd_packages=$FREEBSD_PACKAGES/$arch"
> > + elif [ -n "$pkgpath" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsd_packages=$pkgpath/$arch"
> > + elif [ "$flightjob" != "ERROR" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsdpackagesbuildjob=${flightjob/ /.}"
>
> This feels very similar to the code above, although it lacks the
> special handling for the version.
Maybe I can see about factoring some of this into a helper, but there
are slight differences in both if branches that I'm not sure can be
factored out.
Why I don't start by fixing the repetition of:
freebsd_runvars="$freebsd_runvars \... and we take it from there?
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD
@ 2019-05-24 10:45 ` Roger Pau Monné
0 siblings, 0 replies; 35+ messages in thread
From: Roger Pau Monné @ 2019-05-24 10:45 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Thu, May 23, 2019 at 12:06:29PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD"):
> > This removes the dependency on the official pkg repository, which is
> > dangerous when not testing stable branches, since the ABI of the
> > development branch is not stable, and thus it's easy for packages to
> > get out of sync, or for osstest to test an old FreeBSD version that
> > has an ABI different than the one used to build the official
> > packages.
>
> I realise this is a bit late to be saying this, but had you
> considered making the packages build a different step in the same
> job ? That might make a lot of this go away...
Do you mean to build the packages in build-prep instead of relying on
having a custom binary repository?
I could do that, but it's going to take some time. Also doing the
package build in build-prep would require fetching the svn ports
repository for each build-prep instance.
IIRC the package building job takes a non-trivial amount of time (2-3h
IIRC), because it has to build gcc (for SeaBIOS) and python, perl...
> > IFS=$'\n'
> > +count=0
> > for anointed in \
> > - `./mg-anoint list-prepared "freebsd build $freebsd_branch *"`; do
> > + `./mg-anoint list-prepared "freebsd* build $freebsd_branch *"`; do
> > # Retrieve previous successful FreeBSD build for each arch.
> > freebsd_arch=${anointed##* }
> > - freebsd_envvar="FREEBSD_${freebsd_arch^^}_BUILDJOB"
> > + freebsd_name=${anointed%% *}
> > + freebsd_name=${freebsd_name/-/_}
> > + freebsd_envvar="${freebsd_name^^}_${freebsd_arch^^}_BUILDJOB"
> > if [ "x${!freebsd_envvar}" = "x" ]; then
> > - flight_job=`./mg-anoint retrieve "$anointed"`
> > - export ${freebsd_envvar}=${flight_job/ /.}
> > + envvars[$count]="$freebsd_envvar"
> > + refkeys[$count]="$anointed"
> > + count=$((count+1))
>
> You don't need this counter. You can just say
> envvars=()
> ...
> envvars+=("$freebsd_envvar")
Oh, thanks!
> > + fi
> > +done
> > +count=0
> > +for flight_job in `./mg-anoint retrieve ${refkeys[@]}`; do
> > + if [ "$flight_job" != "ERROR" ]; then
> > + export ${envvars[$count]}=${flight_job/ /.}
> > fi
> > + count=$((count+1))
>
> I think you do need count here, if you do this as two loops. But:
>
> Why not do this retrieve, and set the env vars, inside the first
> loop ? I think that would avoid having to accumulate a data structure
> full of information in shell variables at all (and shell is not very
> good at this kind of thing).
Yes, I think I can do it as you suggest.
> > @@ -542,17 +553,23 @@ freebsd-*)
> > [ "x$OSSTEST_BLESSING" == "xreal" ]; then
> > IFS=$'\n'
> > for anointed in `./mg-anoint list-prepared \
> > - "freebsd build $freebsd_branch *"`; do
> > + "freebsd* build $freebsd_branch *"`; do
> > # Update anointed versions
> > # NB: failure to update an anointed build for a specific arch
> > # should not be fatal, and it's not an issue if one of the
> > # arches gets slightly out of sync with the other ones.
> > freebsd_arch=${anointed##* }
> > - if ./mg-anoint anoint "$anointed" \
> > - $flight build-$freebsd_arch-freebsd; then
> > - echo "Anointed artifacts from build-$freebsd_arch-freebsd"
> > - fi
> > + freebsd_name=${anointed%% *}
> > + # Rely on the fact that the job suffix is the same as the
> > + # anointment refkey. Ie:
>
> I don't think you mean "Rely on the fact". Rather, "by definition,
> from the way the flight is constructed, the intended..." ?
>
> > + # refkey: freebsd job: build-<arch>-freebsd
> > + # refkey: freebsd-packages job: build-<arch>-freebsd-packages
> > + anoint="$anoint \"$anointed\" $flight \
> > + build-$freebsd_arch-$freebsd_name"
>
> Maybe use an array variable for anount, and then you can avoid the
> shell \" quoting.
Please bear with me, but can you elaborate on this?
> > diff --git a/mfi-common b/mfi-common
> > index 83d3c713..12cde85f 100644
> > --- a/mfi-common
> > +++ b/mfi-common
> > @@ -156,7 +156,6 @@ set_freebsd_runvars () {
> ...
> > + # Check if the packages are provided externally, or else assume they
> > + # are provided by the same flight as the installer binaries.
> > + local pkgpath=`getconfig "FreeBSDPackages"`
> > + counter=0
> > + IFS=$'\n'
> > + for flightjob in `./mg-anoint retrieve --tolerate-unprepared \
> > + "freebsd build master $arch" \
> > + "freebsd-packages build master $arch"`; do
> > + if [ $counter -eq 0 ]; then
> > + # Anointed FreeBSD installer
>
> I don't much like this code, but I'm having trouble saying what I
> think should be done instead.
>
> I don't much like the $counter -eq 0 approach. Maybe some of it
> should go into a function ?
> ./mg-anoint retrieve ... >tmpfile
> if freebsd_want_anointed <tmpfile '' Dist ...
> but not sure what the function should be.
Yes, the counter stuff is nasty. I guess I could change the output of
mg-anoint so it signals which output line belongs to which input
parameter, but there's always going to be multiple lines, and I wanted
to avoid having to change much of mg-anoint.
>
> > + local envvar="FREEBSD_${arch^^}_BUILDJOB"
> > + local distpath=`getconfig "FreeBSDDist"`
> > + if [ -n "${!envvar}" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsdbuildjob=${!envvar}"
> > + elif [ -n "$FREEBSD_DIST" ] && [ -n "$FREEBSD_VERSION" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsd_distpath=$FREEBSD_DIST/$arch \
> > + freebsd_version=$FREEBSD_VERSION"
> > + elif [ -n "$distpath" ]; then
> > + local version=`getconfig "FreeBSDVersion"`
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsd_distpath=$distpath/$arch \
> > + freebsd_version=$version"
> > + elif [ "$flightjob" != "ERROR" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsdbuildjob=${flightjob/ /.}"
>
> There seems like a lot of repetition here. For example, FREEBSD_DIST
> overrides FreeBSDDist but /$arch is appended in two places. Maybe
> ${FREEBSD_DIST- ... something ... } would be better ?
OK, let me try to remove some of the duplication here.
> It is difficult to see the wood for the trees, particularly with the
> constant repetition of
> freebsd_runvars="$freebsd_runvars \
> which could be avoided by having this fragment set a local variable
> containing the things to be added.
Hm, indeed. Using a local variable and then appending it when done
would make it clearer.
> > + elif [ $counter -eq 1 ]; then
> > + # Anointed package repository
> > + local envvar="FREEBSD_PACKAGES_${arch^^}_BUILDJOB"
> > + local pkgpath=`getconfig "FreeBSDPackages"`
> > + if [ -n "${!envvar}" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsdpackagesbuildjob=${!envvar}"
> > + elif [ -n "$FREEBSD_PACKAGES" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsd_packages=$FREEBSD_PACKAGES/$arch"
> > + elif [ -n "$pkgpath" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsd_packages=$pkgpath/$arch"
> > + elif [ "$flightjob" != "ERROR" ]; then
> > + freebsd_runvars="$freebsd_runvars \
> > + freebsdpackagesbuildjob=${flightjob/ /.}"
>
> This feels very similar to the code above, although it lacks the
> special handling for the version.
Maybe I can see about factoring some of this into a helper, but there
are slight differences in both if branches that I'm not sure can be
factored out.
Why I don't start by fixing the repetition of:
freebsd_runvars="$freebsd_runvars \... and we take it from there?
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
@ 2019-05-24 11:21 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-24 11:21 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository"):
> On Thu, May 23, 2019 at 11:38:57AM +0100, Ian Jackson wrote:
> > I notice that none of your freebsd build jobs pass any share- hostflag
> > so they always use a fresh installation. Is that necessary ?
>
> Hm, I don't think so. build-amd64-xen-freebsd and
> build-amd64-freebsd-again could share a host. I need to take a look at
> how to do this, I could send this as a separate fix for the existing
> jobs.
Sure. It's achieved by putting share-SOMETHING in (any of) the
hostflags runvars. SOMETHING needs to include the value of every
setting which makes a difference, except the osstest revision (which
is added implictly). Jobs with identical SOMETHING can share.
> > > +# Consumes the following input runvars:
> > > +# svnrevision_freebsdports: ports svn revision id to use.
> > > +# svntree_freebsdports ports svn tree to fetch the source code from.
> >
> > More regular in osstest terms would be
> > tree_freebsdports
> > revision_freebsdports
> > treevcs_freebsdports=svn
> > But I guess svn is sufficiently unlike what osstest expects out of a
> > vcs that this is not feasible, and it is better to do it this way.
>
> I don't really have an opinion, I somehow assumed that using the same
> format might interfere with things like bisection, so I've decided to
> pass the git revision using tree_freebsdports &c and the svn revision
> using the newly introduced flags.
Well, svn is awkward to cache and to use and probably we couldn't get
the bisector to work with it, indeed. So err I guess I'm saying leave
it as it is.
> > > +sub create_jail() {
> > > + my $src_prefix = $r{"freebsd_distpath"} ||
> > > + get_stashed("path_freebsddist", $r{"freebsdbuildjob"});
> > > + my $dst_prefix = "/root/sets";
> >
> > Do we need a jail for this ? We have a whole baremetal OS install
> > whose entire purpose is to do this build ...
>
> Yes, that's how the repository package builder (poudriere) works, it
> requires a jail to do the package building. In our case it's not so
> important, but I assume this is mostly done to always use a clean
> install, so that currently installed packages on the system don't
> interfere with package building.
Fair enough.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository
@ 2019-05-24 11:21 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-24 11:21 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository"):
> On Thu, May 23, 2019 at 11:38:57AM +0100, Ian Jackson wrote:
> > I notice that none of your freebsd build jobs pass any share- hostflag
> > so they always use a fresh installation. Is that necessary ?
>
> Hm, I don't think so. build-amd64-xen-freebsd and
> build-amd64-freebsd-again could share a host. I need to take a look at
> how to do this, I could send this as a separate fix for the existing
> jobs.
Sure. It's achieved by putting share-SOMETHING in (any of) the
hostflags runvars. SOMETHING needs to include the value of every
setting which makes a difference, except the osstest revision (which
is added implictly). Jobs with identical SOMETHING can share.
> > > +# Consumes the following input runvars:
> > > +# svnrevision_freebsdports: ports svn revision id to use.
> > > +# svntree_freebsdports ports svn tree to fetch the source code from.
> >
> > More regular in osstest terms would be
> > tree_freebsdports
> > revision_freebsdports
> > treevcs_freebsdports=svn
> > But I guess svn is sufficiently unlike what osstest expects out of a
> > vcs that this is not feasible, and it is better to do it this way.
>
> I don't really have an opinion, I somehow assumed that using the same
> format might interfere with things like bisection, so I've decided to
> pass the git revision using tree_freebsdports &c and the svn revision
> using the newly introduced flags.
Well, svn is awkward to cache and to use and probably we couldn't get
the bisector to work with it, indeed. So err I guess I'm saying leave
it as it is.
> > > +sub create_jail() {
> > > + my $src_prefix = $r{"freebsd_distpath"} ||
> > > + get_stashed("path_freebsddist", $r{"freebsdbuildjob"});
> > > + my $dst_prefix = "/root/sets";
> >
> > Do we need a jail for this ? We have a whole baremetal OS install
> > whose entire purpose is to do this build ...
>
> Yes, that's how the repository package builder (poudriere) works, it
> requires a jail to do the package building. In our case it's not so
> important, but I assume this is mostly done to always use a clean
> install, so that currently installed packages on the system don't
> interfere with package building.
Fair enough.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD
@ 2019-05-24 17:26 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-24 17:26 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD"):
> On Thu, May 23, 2019 at 12:06:29PM +0100, Ian Jackson wrote:
> > I realise this is a bit late to be saying this, but had you
> > considered making the packages build a different step in the same
> > job ? That might make a lot of this go away...
>
> Do you mean to build the packages in build-prep instead of relying on
> having a custom binary repository?
No. Maybe I am confused. I thought your usual flight was
1 install anointed freebsd
2 build this freebsd
3+ build this package repo
4 install this freebsd (from step 2)
5 rebuild this freebsd (for testing that the build didn't break)
6 rebuild this package repo (")
7+ install this package repoo
8+ build xen
My question is why 2/3 and 5/6 are different jobs. If you made 2+3 a
single job (with 2 and 3 being separate steps) then there would only
need to be a single anointment.
> IIRC the package building job takes a non-trivial amount of time (2-3h
> IIRC), because it has to build gcc (for SeaBIOS) and python, perl...
You could make the package building job optional if you only want to
do it some of the time.
> > > + # refkey: freebsd job: build-<arch>-freebsd
> > > + # refkey: freebsd-packages job: build-<arch>-freebsd-packages
> > > + anoint="$anoint \"$anointed\" $flight \
> > > + build-$freebsd_arch-$freebsd_name"
> >
> > Maybe use an array variable for anount, and then you can avoid the
> > shell \" quoting.
>
> Please bear with me, but can you elaborate on this?
Roughly,
anoint=()
....
anoint+=("$anointed" $flight build-..)
...
./mg-anoint "${anoint[@]}"
Note that the \" \" construct has gone, because there is now no
additional layer of shell dequoting.
> > There seems like a lot of repetition here. For example, FREEBSD_DIST
> > overrides FreeBSDDist but /$arch is appended in two places. Maybe
> > ${FREEBSD_DIST- ... something ... } would be better ?
>
> OK, let me try to remove some of the duplication here.
Thanks.
Another suggestion I mentioned IRL which I wanted to write down was:
maybe have mg-anoint have a reporting mode where it prints something
suitable for shell `eval'.
> > This feels very similar to the code above, although it lacks the
> > special handling for the version.
>
> Maybe I can see about factoring some of this into a helper, but there
> are slight differences in both if branches that I'm not sure can be
> factored out.
Mmmm.
> Why I don't start by fixing the repetition of:
> freebsd_runvars="$freebsd_runvars \... and we take it from there?
Sure, let's see what you come up with.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD
@ 2019-05-24 17:26 ` Ian Jackson
0 siblings, 0 replies; 35+ messages in thread
From: Ian Jackson @ 2019-05-24 17:26 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel
Roger Pau Monne writes ("Re: [Xen-devel] [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD"):
> On Thu, May 23, 2019 at 12:06:29PM +0100, Ian Jackson wrote:
> > I realise this is a bit late to be saying this, but had you
> > considered making the packages build a different step in the same
> > job ? That might make a lot of this go away...
>
> Do you mean to build the packages in build-prep instead of relying on
> having a custom binary repository?
No. Maybe I am confused. I thought your usual flight was
1 install anointed freebsd
2 build this freebsd
3+ build this package repo
4 install this freebsd (from step 2)
5 rebuild this freebsd (for testing that the build didn't break)
6 rebuild this package repo (")
7+ install this package repoo
8+ build xen
My question is why 2/3 and 5/6 are different jobs. If you made 2+3 a
single job (with 2 and 3 being separate steps) then there would only
need to be a single anointment.
> IIRC the package building job takes a non-trivial amount of time (2-3h
> IIRC), because it has to build gcc (for SeaBIOS) and python, perl...
You could make the package building job optional if you only want to
do it some of the time.
> > > + # refkey: freebsd job: build-<arch>-freebsd
> > > + # refkey: freebsd-packages job: build-<arch>-freebsd-packages
> > > + anoint="$anoint \"$anointed\" $flight \
> > > + build-$freebsd_arch-$freebsd_name"
> >
> > Maybe use an array variable for anount, and then you can avoid the
> > shell \" quoting.
>
> Please bear with me, but can you elaborate on this?
Roughly,
anoint=()
....
anoint+=("$anointed" $flight build-..)
...
./mg-anoint "${anoint[@]}"
Note that the \" \" construct has gone, because there is now no
additional layer of shell dequoting.
> > There seems like a lot of repetition here. For example, FREEBSD_DIST
> > overrides FreeBSDDist but /$arch is appended in two places. Maybe
> > ${FREEBSD_DIST- ... something ... } would be better ?
>
> OK, let me try to remove some of the duplication here.
Thanks.
Another suggestion I mentioned IRL which I wanted to write down was:
maybe have mg-anoint have a reporting mode where it prints something
suitable for shell `eval'.
> > This feels very similar to the code above, although it lacks the
> > special handling for the version.
>
> Maybe I can see about factoring some of this into a helper, but there
> are slight differences in both if branches that I'm not sure can be
> factored out.
Mmmm.
> Why I don't start by fixing the repetition of:
> freebsd_runvars="$freebsd_runvars \... and we take it from there?
Sure, let's see what you come up with.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2019-05-24 17:27 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-20 16:59 [PATCH 0/6] osstest: create a local binary FreeBSD package repository Roger Pau Monne
2019-02-20 16:59 ` [PATCH 1/6] osstest: introduce a helper to stash a whole directory Roger Pau Monne
2019-05-23 9:48 ` Ian Jackson
2019-05-23 9:48 ` [Xen-devel] " Ian Jackson
2019-05-24 9:42 ` Roger Pau Monné
2019-05-24 9:42 ` [Xen-devel] " Roger Pau Monné
2019-02-20 16:59 ` [PATCH 2/6] osstest: introduce a helper to create a weblink to a directory Roger Pau Monne
2019-05-23 9:57 ` Ian Jackson
2019-05-23 9:57 ` [Xen-devel] " Ian Jackson
2019-02-20 16:59 ` [PATCH 3/6] osstest: allow to perform multiple anoints in the same transaction Roger Pau Monne
2019-05-23 10:00 ` Ian Jackson
2019-05-23 10:00 ` [Xen-devel] " Ian Jackson
2019-02-20 16:59 ` [PATCH 4/6] osstest: introduce a helper to get the svn revision of a git commit Roger Pau Monne
2019-05-23 10:03 ` Ian Jackson
2019-05-23 10:03 ` [Xen-devel] " Ian Jackson
2019-05-24 9:57 ` Roger Pau Monné
2019-05-24 9:57 ` [Xen-devel] " Roger Pau Monné
2019-05-24 10:35 ` Ian Jackson
2019-05-24 10:35 ` [Xen-devel] " Ian Jackson
2019-02-20 17:00 ` [PATCH 5/6] osstest: introduce a script to build a FreeBSD package repository Roger Pau Monne
2019-05-23 10:19 ` Ian Jackson
2019-05-23 10:19 ` [Xen-devel] " Ian Jackson
2019-05-23 10:38 ` Ian Jackson
2019-05-23 10:38 ` [Xen-devel] " Ian Jackson
2019-05-24 10:13 ` Roger Pau Monné
2019-05-24 10:13 ` [Xen-devel] " Roger Pau Monné
2019-05-24 11:21 ` Ian Jackson
2019-05-24 11:21 ` [Xen-devel] " Ian Jackson
2019-02-20 17:00 ` [PATCH 6/6] osstest: use a locally built pkg repository for FreeBSD Roger Pau Monne
2019-05-23 11:06 ` Ian Jackson
2019-05-23 11:06 ` [Xen-devel] " Ian Jackson
2019-05-24 10:45 ` Roger Pau Monné
2019-05-24 10:45 ` [Xen-devel] " Roger Pau Monné
2019-05-24 17:26 ` Ian Jackson
2019-05-24 17:26 ` [Xen-devel] " Ian Jackson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.