Message ID | 20170706144227.36580-10-roger.pau@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Roger Pau Monne writes ("[PATCH v4 09/16] osstest: introduce a FreeBSD build script"): > In order to generate the FreeBSD installer image and the install > media. > +sub install_deps () { > + target_install_packages($ho, 'git'); Please say + target_install_packages($ho, qw(git)); > + logm("Checkout the FreeBSD source tree"); > + build_clone($ho, 'freebsd', $builddir, 'freebsd', ); You have a spurious ", " at the end there. In general, I notice that you sometimes add comments like this: # Reverse the neutron polarity neutron_polarity_op(--reverse); I won't insist on you removing any but in general I thought I'd say they aren't IMO particularly useful. > +sub build_release($$$) { > + my ($target, $prefix, $time) = @_; > + > + buildcmd_stamped_logged_root($time, 'freebsd', "release-$target", > + $prefix, <<END, ''); > +make -C release $target > +END Does this not want $makeflags ? Mostly, this would be a -j. > + # Build process as documented in the handbook: > + # https://www.freebsd.org/doc/handbook/updating-src.html _This_ is a really good comment :-). > +# Create a temporary fstab with the root dir > +echo '/dev/ufs/FreeBSD_Install / ufs rw 1 1' > etc/fstab It's quite noticeable that there is a lot of code here that perhaps ought to be in some FreeBSD component. (This is not a criticism of your osstest submission.) > + my $srcversion = target_cmd_output_root($ho, <<END, 30); > +awk '/^\\\#define[[:space:]]*__FreeBSD_version/ { print \$3 }' \\ > + $builddir/freebsd/sys/sys/param.h | cut -c1-2 > +END Cor. Might it be better to use target_getfile and get_filecontents, and use a perl regexp ? Your current approach means that: * if nothing matchines, you do not detect an error * you are unable to check that the putative version number has a plausible syntax * if something goes wrong, the param.h that you're parsing does not end up conveniently in a log (although I guess it's probably in build/ somewhere). > + store_runvar("freebsd_buildversion", "$srcversion"); > + > + # Set path_freebsddist to point to the build output folder Seems to be a unicode nonbreaking space after the # ! Ian.
On Thu, Jul 06, 2017 at 04:25:24PM +0100, Ian Jackson wrote: > Roger Pau Monne writes ("[PATCH v4 09/16] osstest: introduce a FreeBSD build script"): > You have a spurious ", " at the end there. > > In general, I notice that you sometimes add comments like this: > > # Reverse the neutron polarity > neutron_polarity_op(--reverse); Right, it's too verbose maybe... I've added the logm to have some kind of references to what's going on, but really it's a mess to find them between so much output (the size of the build log file is ~100MB IIRC). > I won't insist on you removing any but in general I thought I'd say > they aren't IMO particularly useful. > > > +sub build_release($$$) { > > + my ($target, $prefix, $time) = @_; > > + > > + buildcmd_stamped_logged_root($time, 'freebsd', "release-$target", > > + $prefix, <<END, ''); > > +make -C release $target > > +END > > Does this not want $makeflags ? Mostly, this would be a -j. No, those targets are (like) install targets, and will fail if called with -j. > > + # Build process as documented in the handbook: > > + # https://www.freebsd.org/doc/handbook/updating-src.html > > _This_ is a really good comment :-). > > > +# Create a temporary fstab with the root dir > > +echo '/dev/ufs/FreeBSD_Install / ufs rw 1 1' > etc/fstab > > It's quite noticeable that there is a lot of code here that perhaps > ought to be in some FreeBSD component. (This is not a criticism of > your osstest submission.) Nods, I agree. There's a GSoC project currently ongoing to integrate this functionality into the FreeBSD build process itself. > > + my $srcversion = target_cmd_output_root($ho, <<END, 30); > > +awk '/^\\\#define[[:space:]]*__FreeBSD_version/ { print \$3 }' \\ > > + $builddir/freebsd/sys/sys/param.h | cut -c1-2 > > +END > > Cor. Might it be better to use target_getfile and get_filecontents, > and use a perl regexp ? This line is basically the same used by the FreeBSD Makefile to get the version number, that's why I've used it, but I don't like it, I think it's fragile to regexp like that. I've changed this to: my $srcversion = target_cmd_output_root($ho, <<END, 30); set -e cd $builddir/freebsd eval `make buildenvvars` test -n "\$SRCRELDATE" echo "\$SRCRELDATE" | cut -c1-2 END Which I think it's more bullet-proof. > > + store_runvar("freebsd_buildversion", "$srcversion"); > > + > > + # Set path_freebsddist to point to the build output folder > > Seems to be a unicode nonbreaking space after the # ! Ups. Thanks, Roger.
Roger Pau Monne writes ("Re: [PATCH v4 09/16] osstest: introduce a FreeBSD build script"): > On Thu, Jul 06, 2017 at 04:25:24PM +0100, Ian Jackson wrote: > > Roger Pau Monne writes ("[PATCH v4 09/16] osstest: introduce a FreeBSD build script"): > > You have a spurious ", " at the end there. > > > > In general, I notice that you sometimes add comments like this: > > > > # Reverse the neutron polarity > > neutron_polarity_op(--reverse); > > Right, it's too verbose maybe... I've added the logm to have some kind > of references to what's going on, but really it's a mess to find them > between so much output (the size of the build log file is ~100MB > IIRC). Oh, yes, I approve of the logm's. > > Does this not want $makeflags ? Mostly, this would be a -j. > > No, those targets are (like) install targets, and will fail if called > with -j. Oh, OK. > > > + my $srcversion = target_cmd_output_root($ho, <<END, 30); > > > +awk '/^\\\#define[[:space:]]*__FreeBSD_version/ { print \$3 }' \\ > > > + $builddir/freebsd/sys/sys/param.h | cut -c1-2 > > > +END > > > > Cor. Might it be better to use target_getfile and get_filecontents, > > and use a perl regexp ? > > This line is basically the same used by the FreeBSD Makefile to get > the version number, that's why I've used it, but I don't like it, I > think it's fragile to regexp like that. I've changed this to: > > my $srcversion = target_cmd_output_root($ho, <<END, 30); > set -e > cd $builddir/freebsd > eval `make buildenvvars` > test -n "\$SRCRELDATE" > echo "\$SRCRELDATE" | cut -c1-2 > END > > Which I think it's more bullet-proof. LGTM. Although I have to say, evaling the output of make seems brave. Does BSD make only ever print blather to stderr, then ? > > > + store_runvar("freebsd_buildversion", "$srcversion"); > > > + > > > + # Set path_freebsddist to point to the build output folder > > > > Seems to be a unicode nonbreaking space after the # ! > > Ups. Ian.
On Thu, Jul 06, 2017 at 06:31:02PM +0100, Ian Jackson wrote: > Roger Pau Monne writes ("Re: [PATCH v4 09/16] osstest: introduce a FreeBSD build script"): > > On Thu, Jul 06, 2017 at 04:25:24PM +0100, Ian Jackson wrote: > > > Roger Pau Monne writes ("[PATCH v4 09/16] osstest: introduce a FreeBSD build script"): > > my $srcversion = target_cmd_output_root($ho, <<END, 30); > > set -e > > cd $builddir/freebsd > > eval `make buildenvvars` > > test -n "\$SRCRELDATE" > > echo "\$SRCRELDATE" | cut -c1-2 > > END > > > > Which I think it's more bullet-proof. > > LGTM. > > Although I have to say, evaling the output of make seems brave. > Does BSD make only ever print blather to stderr, then ? That's a special target that's designed to work like this, I assume this one is guaranteed to print everything else to stderr (and exit with != 0). I've changed the last line to: expr "\$SRCRELDATE" / 100000 So that when FreeBSD reaches version 100 (in about ~200years), this will still work. Roger.
Roger Pau Monne writes ("Re: [PATCH v4 09/16] osstest: introduce a FreeBSD build script"): > On Thu, Jul 06, 2017 at 06:31:02PM +0100, Ian Jackson wrote: > > Although I have to say, evaling the output of make seems brave. > > Does BSD make only ever print blather to stderr, then ? > > That's a special target that's designed to work like this, I assume > this one is guaranteed to print everything else to stderr (and exit > with != 0). FE > I've changed the last line to: > > expr "\$SRCRELDATE" / 100000 > > So that when FreeBSD reaches version 100 (in about ~200years), this > will still work. :-) Ian.
diff --git a/ts-freebsd-build b/ts-freebsd-build new file mode 100755 index 00000000..d64d85f5 --- /dev/null +++ b/ts-freebsd-build @@ -0,0 +1,245 @@ +#!/usr/bin/perl -w +# This is part of "osstest", an automated testing framework for Xen. +# Copyright (C) 2017 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/>. + +# Consumes the following input runvars: +# +# revision_freebsd: FreeBSD commit ID to generate the install media from. +# tree_freebsd: FreeBSD git tree to fetch the source code from. +# +# Produces the following output: +# +# Compressed install sets: kernel.txz, base.txz +# Compressed debug symbols for the kernel: kernel-dbg.txz +# Manifest file (checkums): MANIFEST +# Installer image: install.img +# +# Sets the following runvar: +# +# path_freebsddist: points to the folder where the above files are stored. +# freebsd_buildversion: version of FreeBSD built. + +use strict qw(vars); +use DBI; +use POSIX; + +unshift @INC, qw(.); +use Osstest; +use Osstest::TestSupport; +use Osstest::BuildSupport; + +tsreadconfig(); + +selectbuildhost(\@ARGV); +builddirsprops(); + +sub install_deps () { + target_install_packages($ho, 'git'); +} + +sub checkout () { + prepbuilddirs(); + + # Remove the directory as root, there might be files owned by root + target_cmd_build_root($ho, 300, $builddir, <<'END'); +# chflags will return error if the directory doesn't exist +chflags -fR noschg freebsd || true +rm -rf freebsd +END + + logm("Checkout the FreeBSD source tree"); + build_clone($ho, 'freebsd', $builddir, 'freebsd', ); +} + +sub build_target($$$) { + my ($target, $prefix, $time) = @_; + + buildcmd_stamped_logged($time, 'freebsd', $target, + $prefix, <<END, ''); +make $makeflags $target +END +} + +sub build_release($$$) { + my ($target, $prefix, $time) = @_; + + buildcmd_stamped_logged_root($time, 'freebsd', "release-$target", + $prefix, <<END, ''); +make -C release $target +END +} + +sub build () { + my $authkeys = authorized_keys(); + my $target = "bootonly"; + my $prefix = <<END; +export MAKEOBJDIRPREFIX=$builddir/obj +export TARGET=$r{arch} +END + + # Build process as documented in the handbook: + # https://www.freebsd.org/doc/handbook/updating-src.html + + logm("Cleaning up previous builds"); + build_target('cleanworld', $prefix, 300); + + logm("Building world"); + build_target('buildworld', $prefix, 25200); + + logm("Building kernel"); + build_target('buildkernel', $prefix, 3600); + + # NB: the steps below need to be done as root or the permissions + # of the files won't be properly set (and the target will fail). + logm("Creating the install sets"); + build_release('ftp', $prefix, 3600); + + logm("Populating the installer image"); + build_release($target, $prefix, 3600); + + logm("Placing ssh host keys"); + foreach my $file (<$c{OverlayLocal}/etc/ssh/ssh_host_*_key*>) { + target_putfile_root($ho, 30, $file, + "$builddir/freebsd/release/$target/etc/ssh/"); + } + + logm("Configuring the installer image"); + target_cmd_build_root($ho, 30, $builddir, <<END.<<'END'); +authkeys="$authkeys" +bauds="$c{Baud}" +cd freebsd/release/$target +END +# Enable sshd by default +sysrc -f etc/rc.conf sshd_enable=YES + +# Allow root login and copy the keys +echo 'PermitRootLogin yes' >> etc/ssh/sshd_config +mkdir -p root/.ssh +cat << ENDKEYS > root/.ssh/authorized_keys +$authkeys +ENDKEYS + +# Set host keys permissions +chown root:wheel etc/ssh/ssh_host_*_key* +chmod 0600 etc/ssh/ssh_host_*_key +chmod 0644 etc/ssh/ssh_host_*_key.pub + +# Setup serial console output for stage1 +printf "%s" "-h -S$bauds" >> boot.config +cat << ENDBOOT >> boot/loader.conf +# Serial console configuration +boot_serial="YES" +comconsole_speed="$bauds" +console="comconsole" +boot_verbose="YES" +beastie_disable="YES" + +# mfs boot parameters +mfs_load="YES" +mfs_type="mfs_root" +mfs_name="/mfsroot" +vfs.root.mountfrom="ufs:/dev/ufs/FreeBSD_Install" +ENDBOOT + +# Enable DHCP on all network interfaces +sysrc -f etc/rc.conf ifconfig_DEFAULT=DHCP + +# Remove the local script that launches the installer by default +rm -rf etc/rc.local + +# Create a temporary fstab with the root dir +echo '/dev/ufs/FreeBSD_Install / ufs rw 1 1' > etc/fstab + +# Remove the linked resolv.conf +rm -rf etc/resolv.conf +END + + logm("Create the installer"); + target_cmd_build_root($ho, 900, $builddir, <<END.<<'END'); +target="freebsd/release/$target" +output="install.img" +END +mkdir -p $output.tmp + +# Do some pruning +rm -rf $target/usr/share/man +rm -rf $target/usr/share/examples +rm -rf $target/usr/share/doc +rm -rf $target/usr/share/dtrace + +# Create a mfs root image +makefs -b 10% -B little -o label=FreeBSD_Install $output.tmp/mfsroot $target +# Compress image +gzip $output.tmp/mfsroot + +# Copy boot to the staging dir +cp -r $target/boot $output.tmp/ +cp $target/boot.config $output.tmp/ + +# The loader doesn't need any modules in order to boot into the mfsroot. +# The rest of the modules can be loaded from the mfs root itself. +rm -f $output.tmp/boot/kernel/*.ko + +# Compress the kernel +gzip $output.tmp/boot/kernel/kernel + +makefs -B little $output.part $output.tmp + +# Make the image bootable +mkimg -s gpt -b $target/boot/pmbr -p efi:=$target/boot/boot1.efifat \ + -p freebsd-boot:=$target/boot/gptboot -p freebsd-ufs:=$output.part \ + -p freebsd-swap::1M -o $output + +rm $output.part +rm -rf $output.tmp +END +} + +sub stash () { + my @sets = qw(MANIFEST base.txz kernel.txz); + my @symbols = qw(kernel-dbg.txz); + + + logm("Stashing FreeBSD build output"); + foreach my $set (@sets) { + built_stash_file($ho, $builddir, $set, + "freebsd/release/ftp/$set", 0); + } + foreach my $symbol (@symbols) { + built_stash_debugfile($ho, $builddir, $symbol, + "freebsd/release/ftp/$symbol", 0); + } + built_stash_file($ho, $builddir, "install.img", "install.img", 0); + + + my $srcversion = target_cmd_output_root($ho, <<END, 30); +awk '/^\\\#define[[:space:]]*__FreeBSD_version/ { print \$3 }' \\ + $builddir/freebsd/sys/sys/param.h | cut -c1-2 +END + store_runvar("freebsd_buildversion", "$srcversion"); + + # Set path_freebsddist to point to the build output folder + # in order to make ts-build-check happy. + store_runvar("path_freebsddist", "build/"); +} + +install_deps(); +checkout(); +build(); +stash(); + +logm("FreeBSD build successful"); +
In order to generate the FreeBSD installer image and the install media. The install sets are the vanilla ones generated by the 'ftp' release target. The installer image is handcrafted based on the filesystem created by the 'bootonly' target, which is then populated with the ssh host keys, and setup in order to use the serial console. The other difference from upstream FreeBSD installer images is that the one built by osstest uses a ramdisk instead of relying on the installer media to be somehow attached, either on a CD or USB drive. This is required in order to boot the image from pxelinux (where no CD or USB is actually attached to the host, and everything is fetched from tftp). Due to the nature of the FreeBSD build, the outputs are different from what osstest expects from a buildjob, more specifically path_freebsddist points to a folder that contains the several outputs form this buildjob. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v3: - Introduce two helpers to perform the build process. - Install packages using target_install_packages. Changes since v2: - Increase build target timeouts. - Use sysrc instead echo to set rc.conf options. Changes since v1: - Remove the ts-build-check FreeBSD hack. - Use pkg-static instead of pkg. - Introduce buildcmd_stamped_logged_root and target_cmd_build_root. - Use target_cmd_build_root and buildcmd_stamped_logged_root in the ts-freebsd-build script. - Fix the script snippets to use <<END.<<'END' in order to avoid escaping the shell variables. - Set path_freebsddist runvar to point to the folder where the build files are stashed. - Add a comment at the top of the file describing what runvars are consumed/produced by the build script. --- ts-freebsd-build | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100755 ts-freebsd-build