Subject: [bulk] Adequate modelling of the report generation
To: NetBSD Packages Technical Discussion List <tech-pkg@NetBSD.org>
From: Roland Illig <rillig@NetBSD.org>
List: tech-pkg
Date: 12/05/2005 00:55:02
This is a multi-part message in MIME format.
--------------090401000506050407040702
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Hi pkgsrc bulk builders,

I disagree with the names of the variables that determine where the bulk 
build reports are generated. Therefore I have written a patch that 
models the situation in a way that I would find convenient to 
communicate to the user in the pkgsrc guide. I cannot say this of the 
current state.

First, the names of the variables that are currently in use are 
completely unintuitive and misleading:

- I cannot explain where the "x" in FTPx comes from.

- The default value of FTPURL does not specify a URL. Neither is the 
variable used like a URL.

- The FTP variable has nothing to do with FTP, as it specifies a local path.

- FTPHOST does not specify a host name, but rather a URL scheme, 
together with a host name.

Also, the comments in the build.conf-example are not really helpful.

I see the situation as follows. On some publicly available server, there 
is a directory that keeps the build logs. The individual build logs are 
saved in subdirectories of this directory. On the build machine, there 
is a similar directory containing the actual reports in subdirectories. 
It is likely that the reports from the build machine are copied as-is to 
the publicly available server. So my suggestion is to define the 
REPORTS_DIR, the location where the reports are created on the build 
machine, and the REPORTS_URL, where the reports will be made available. 
Each report is then created in the REPORT_BASEDIR in those directories.

This model directly covers the current practice and is easy to explain, 
so I would like it to replace the current model, which I am not willing 
to document in the pkgsrc guide or somewhere else.

The second file in the patch gives an impression of how the post-build 
program may look like after the changes. Note how the careful choice of 
variable names makes the code more readable. (Just look at the last 
chunk. Assuming you did not know how the variables have been defined, 
you might assume that a URL is appended to a host name, which makes no 
sense at all.)

I have not yet tested the patch, as it is only intended to give an 
impression on the change. Of course I will test it before actually 
committing it.

Roland

--------------090401000506050407040702
Content-Type: text/plain;
 name="bulk-report.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="bulk-report.patch"

Index: build.conf-example
===================================================================
RCS file: /cvsroot/pkgsrc/mk/bulk/build.conf-example,v
retrieving revision 1.32
diff -u -p -r1.32 build.conf-example
--- build.conf-example	3 Dec 2005 01:00:37 -0000	1.32
+++ build.conf-example	4 Dec 2005 23:29:09 -0000
@@ -61,11 +61,17 @@ ADMIN="you@some.whe.re"
 # Who the report is signed by
 ADMINSIG="-Your Name"
 
-# Some paths for output files and paths to log files
-FTPx=`date +%Y%m%d.%H%M`
-FTPURL="pub/NetBSD/pkgstat/${FTPx}"		# relative to ~ftp !
-FTP="/home/ftp/${FTPURL}"			# absolute base path
-FTPHOST="ftp://ftp.machi.ne"			# host for broken.html
+# The basename of the directory of the current bulk build.
+REPORT_BASEDIR=`date +%Y%m%d.%H%M`
+
+# The directory where the final reports are collected. The reports
+# themselves are saved in the subdirectory ${BUILD_ID}.
+#REPORTS_DIR="$HOME/bulk-logs"
+REPORTS_DIR="/home/ftp/pub/NetBSD/pkgstat"
+
+# The URL where the final reports will be available. The reports
+# themselves are expected to be available in the subdirectory ${BUILD_ID}.
+REPORTS_URL="ftp://ftp.machi.ne/pub/NetBSD/pkgstat"
 
 #
 # Uploading binary packages
Index: post-build
===================================================================
RCS file: /cvsroot/pkgsrc/mk/bulk/post-build,v
retrieving revision 1.62
diff -u -p -r1.62 post-build
--- post-build	3 Dec 2005 13:39:04 -0000	1.62
+++ post-build	4 Dec 2005 23:29:09 -0000
@@ -91,14 +91,20 @@ sub get_build_conf_vars(@) {
 
 get_build_conf_vars(
 	'ADMINSIG',		# "-Your Name"
-	'FTPURL',		# "pub/NetBSD/pkgstat/`date +%Y%m%d.%H%M`"
-	'FTP',			# "/disk1/ftp/${FTPURL}"
-	'FTPHOST',		# ftp://ftp.machi.ne/
+	'REPORT_BASEDIR',	# "`date +%Y%m%d.%H%M`"
+	'REPORTS_DIR',		# "/home/ftp/pub/NetBSD/pkgstat"
+	'REPORTS_URL',		# "ftp://ftp.NetBSD.org/pub/NetBSD/pkgstat"
 	'REPORT',		# "broken.html"
 	'USR_PKGSRC',		# "/usr/pkgsrc"
 	'osrev',		# `uname -r`
 );
 
+my $report_basedir	= $vars{"REPORT_BASEDIR"};
+my $reports_dir		= $vars{"REPORTS_DIR"};
+my $reports_url		= $vars{"REPORTS_URL"};
+my $report_dir		= "${reports_dir}/${report_basedir}";
+my $report_url		= "${reports_url}/${report_basedir}";
+
 my $reportf = basename($vars{REPORT});
 
 my $os = `uname -s`;
@@ -187,31 +193,28 @@ sub print_report_header() {
 	print("--------------------------------------------------------------\n");
 }
 
-my_system("mkdir", "-p", "--", $vars{FTP});
+my_system("mkdir", "-p", "--", $report_dir);
 
 # Copy over the output from the build process
 chdir($vars{"BULKFILESDIR"}) or pb_die($vars{"BULKFILESDIR"}, "Cannot change directory.");
-my_system("find . -name $vars{BROKENFILE} -print -o -name $vars{BROKENWRKLOG} -print | $vars{PAX} -r -w -X $vars{FTP}");
+my_system("find . -name $vars{BROKENFILE} -print -o -name $vars{BROKENWRKLOG} -print | $vars{PAX} -r -w -X $report_dir");
 
 # Copy over the cache files used during the build
 foreach my $f qw(BULK_DBFILE DEPENDSTREEFILE DEPENDSFILE SUPPORTSFILE INDEXFILE ORDERFILE) {
 	if (-f $vars{$f}) {
-		my_system("cp", "--", $vars{$f}, $vars{FTP});
+		my_system("cp", "--", $vars{$f}, $report_dir);
 	}
 }
 
-chdir($vars{FTP}) or pb_die($vars{"FTP"}, "Cannot change directory.");
+chdir($report_dir) or pb_die($report_dir, "Cannot change directory.");
 writeReport();
 
 #
 # Adjust "last" symlink
 #
-{
-	my ($base, $dir) = ($vars{FTP} =~ m|^(.*)/([^/]*)$|);
-
-	unlink("$base/last");
-	symlink($dir, "$base/last");
-}
+chdir($reports_dir) or pb_die($reports_dir, "Cannot change directory.");
+unlink("last");
+symlink($report_basedir, "last");
 
 #
 # Generate leftovers-$vars{MACHINE_ARCH}.html: files not deleted
@@ -219,7 +222,7 @@ writeReport();
 # and linked from leftovers-$vars{MACHINE_ARCH}.html
 #
 {
-	chdir($vars{FTP});
+	chdir($report_dir) or pb_die($report_dir, "Cannot change directory.");
 	my_system("mkdir", "-p", "leftovers-$vars{MACHINE_ARCH}");
 
 	# Find files since last build:
@@ -270,7 +273,7 @@ writeReport();
 EOOUT
 	foreach (@leftovers) {
 		chomp;
-		print OUT "<a href=\"$vars{FTPHOST}/$vars{FTPURL}/leftovers-$vars{MACHINE_ARCH}$_\">$_</a>\n";
+		print OUT "<a href=\"${report_url}/leftovers-$vars{MACHINE_ARCH}$_\">$_</a>\n";
 	}
 	print OUT <<EOOUT2;
 </pre>
@@ -369,7 +372,7 @@ EOF
 Packages not listed here resulted in a binary package. The build
 report, including logs of failed/not-packaged is available from:
 
-$vars{FTPHOST}/$vars{FTPURL}/$reportf
+${report_url}/${reportf}
 EOF
 	}
 

--------------090401000506050407040702--