Re: [PATCH cygport] Add initial support for SOURCE_DATE_EPOCH

[Jon Turney <jon.turney@dronecode.org.uk>]

On 28/08/2023 16:12, Christian Franke via Cygwin-apps wrote:
> Christian Franke wrote:
>> A small step towards reproducible packaging...
>>

Thanks very much for this. Sorry for taking so long to look at it.

A few questions and suggestions interspersed

[...]
> 
> -- 
> Regards,
> Christian
> 
> 
> 0001-Add-initial-support-for-SOURCE_DATE_EPOCH.patch
> 
>  From 73dde4d2dabb74b7b9ee40655204f84e1d4086d6 Mon Sep 17 00:00:00 2001
> From: Christian Franke<christian.franke@t-online.de>
> Date: Mon, 28 Aug 2023 16:24:36 +0200
> Subject: [PATCH] Add initial support for SOURCE_DATE_EPOCH
> 
> If specified, set the header timestamps of executables and
> patch files to SOURCE_DATE_EPOCH.
> Suppress the header timestamp of .gz files.
> Instruct tar to avoid more recent modification times and
> to sort all entries by name.
> ---
>   bin/cygport.in           | 17 +++++++++++++++--
>   lib/pkg_pkg.cygpart      | 20 ++++++++++++++++++--
>   lib/src_postinst.cygpart | 22 +++++++++++++++++++---
>   3 files changed, 52 insertions(+), 7 deletions(-)
> 
> diff --git a/bin/cygport.in b/bin/cygport.in
> index 3f89ac67..e2fe1785 100755
> --- a/bin/cygport.in
> +++ b/bin/cygport.in
> @@ -231,8 +231,9 @@ source ${_privlibdir}/check_funcs.cygpart
>   ###
>   
>   # check now for all mandatory programs
> -for _myprog in bzip2 cat chmod cp diff diffstat dos2unix file find gawk grep gzip \
> -               install ln mkdir mv patch rm rsync sed sort tar xargs which xz
> +for _myprog in bzip2 cat chmod cp date diff diffstat dos2unix file find gawk grep \
> +               gzip install ln mkdir mv patch rm rsync sed sort tar touch which \
> +               xargs xz
>   do
>   	if ! check_prog ${_myprog}
>   	then
> @@ -490,6 +491,18 @@ do
>   done
>   unset restrict
>   
> +if [ "${SOURCE_DATE_EPOCH+y}" = "y" ]
> +then
> +	if [ -n "$(echo "${SOURCE_DATE_EPOCH}" | sed -e 's/^$/X/' -e 's/[0-9]//g')" ]
> +	then
> +		error "Malformed SOURCE_DATE_EPOCH: '${SOURCE_DATE_EPOCH}'"

This error message should perhaps say what a well-formed 
SOURCE_DATE_EPOCH looks like: an integer number of seconds since the 
unix epoch?

> +	fi
> +	case $(peflags --version 2>/dev/null | sed -n '1s/^.* //p') in
> +		4.6.[6-9]|4.[7-9]*|[5-9]*) ;;
> +		*) error "SOURCE_DATE_EPOCH requires peflags 4.6.6 or later"
> +	esac
> +fi
> +
>   
>   ################################################################################
>   #
> diff --git a/lib/pkg_pkg.cygpart b/lib/pkg_pkg.cygpart
> index 2a2bb663..4e6a7cd2 100644
> --- a/lib/pkg_pkg.cygpart
> +++ b/lib/pkg_pkg.cygpart
> @@ -42,7 +42,7 @@ TAR_COMPRESSION_EXT="${TAR_COMPRESSION_EXT:-xz}"
>   #****
>   
>   __tar() {
> -	local TAR_COMPRESSION_OPT;
> +	local TAR_COMPRESSION_OPT TAR_SOURCE_DATE_OPTS;
>   
>   	# We could use --auto-compress, but this also constrains the extension
>   	# to the currently valid set. We could probe if tar supports the
> @@ -65,7 +65,14 @@ __tar() {
>   			error "tar option for TAR_COMPRESSION_EXT='${TAR_COMPRESSION_EXT}' unknown"
>   			;;
>   	esac
> -	tar ${TAR_COMPRESSION_OPT} --owner=Guest:501 --group=None:513 -cvf "$@"
> +
> +	if [ -n "${SOURCE_DATE_EPOCH}" ]
> +	then
> +		# Ensure reproducible sort order and last modification times <= SOURCE_DATE_EPOCH
> +		TAR_SOURCE_DATE_OPTS="--sort=name --mtime=@${SOURCE_DATE_EPOCH} --clamp-mtime"

I'm slightly concerned that maybe this is masking problems elsewhere, at 
least when making the source archive: In 2eb7c0eb I started making an 
effort so that if the "source inputs" have fixed, upstream timestamps, 
we'll preserve those in the output the source package.

(Obviously that's not always going to be the case, e.g. where cygport 
patches are from a local git checkout, so maybe that's not a real 
problem...)

> +	fi
> +
> +	tar ${TAR_COMPRESSION_OPT} ${TAR_SOURCE_DATE_OPTS} --owner=Guest:501 --group=None:513 -cvf "$@"
>   }
>   
>   __pkg_binpkg() {
> @@ -319,6 +326,7 @@ __pkg_diff() {
>   	local difflevel;
>   	local exclude;
>   	local optional_patchfiles;
> +	local source_date;
>   
>   	default_excludes="CYGWIN-PATCHES aclocal.m4~ aclocal.m4t autom4te.cache
>   		config.cache config.guess config.log config.status config.sub
> @@ -498,6 +506,14 @@ __pkg_diff() {
>   
>   	sed -b -e '/^diff -u/d' -i ${optional_patchfiles} ${patchdir}/${src_patchfile};
>   
> +	if [ -n "${SOURCE_DATE_EPOCH}" ]
> +	then
> +		# Ensure that the timestamp comment of the new file is reproducible

"timestamp comments in the generated patch files" or suchlike?

> +		source_date=$(date -d @"${SOURCE_DATE_EPOCH}" -u +'%Y-%m-%d %H:%M:%S.000000000 +0000')
> +		sed -b -e  "s|^\(+++ [^\t]*\t\).*\$|\1${source_date}|" \
> +			-i ${optional_patchfiles} ${patchdir}/${src_patchfile}
> +	fi
> +
>   	diffstat -p${difflevel} ${optional_patchfiles} ${patchdir}/${src_patchfile};
>   }
>   
> diff --git a/lib/src_postinst.cygpart b/lib/src_postinst.cygpart
> index dd947311..53eaa71a 100644
> --- a/lib/src_postinst.cygpart
> +++ b/lib/src_postinst.cygpart
> @@ -41,7 +41,7 @@ __prep_fonts_dir() {
>   	for catalogue in ${D}${cataloguedir}/*
>   	do
>   		fontdir=$(readlink -f ${catalogue})
> -		find ${D}${fontdir} -name '*.pcf' -exec gzip -q '{}' +
> +		find ${D}${fontdir} -name '*.pcf' -exec gzip -q ${SOURCE_DATE_EPOCH:+-n} '{}' +
>   		mkfontscale ${D}${fontdir}
>   		mkfontdir ${D}${fontdir}
>   	done
> @@ -775,7 +775,7 @@ __prepman() {
>   		while read -d $'\0' manpage
>   		do
>   			echo "        ${manpage##*/}";
> -			gzip -q "${manpage}";
> +			gzip -q ${SOURCE_DATE_EPOCH:+-n} "${manpage}";
>   		done
>   	fi
>   }
> @@ -819,7 +819,7 @@ __prepinfo() {
>   		while read -d $'\0' infopage
>   		do
>   			echo "        ${infopage##*/}";
> -			gzip -q "${infopage}";
> +			gzip -q ${SOURCE_DATE_EPOCH:+-n} "${infopage}";
>   		done
>   	fi
>   }
> @@ -989,6 +989,12 @@ __prepstrip_one() {
>   
>   	objdump=${objcopy/copy/dump}
>   
> +	if [ -n "${SOURCE_DATE_EPOCH}" ]
> +	then
> +		# Let objcopy preserve the timestamps

I found this comment confusing.

"Do not embed timestamps in archives"

Why to we need "--preserve-dates"? The input file presumably has a 
timestamp when the build actually took place, which we're going to clamp 
to SOURCE_DATE_EPOCH when we make the archive?

It seems like maybe "--enable-deterministic-archives" should be in 
LDFLAGS, if the build produces static or unstripped archives?

> +		objcopy+=" --enable-deterministic-archives --preserve-dates"
> +	fi
> +
>   	# Static libraries should not be fully stripped, but we can
>   	# still provide split debuginfo if desired
>   	case "${exe}" in
> @@ -1074,6 +1080,16 @@ __prepstrip_one() {
>   	# keep sticky bit if present
>   	chmod u+w,a+x "${exe}";
>   
> +	if [ -n "${SOURCE_DATE_EPOCH}" ]
> +	then
> +		case "${exe}" in
> +		*.exe|*.dll|*.so|*.so.*|*.oct|*.mex|*.cmxs)
> +			# Ensure PE header timestamp is reproducible and checksum is correct
> +			# objcopy later inherits the timestamp to debug info and stripped file
> +			peflags --checksum=1 --timestamp=${SOURCE_DATE_EPOCH} ${exe} >/dev/null ;;
> +		esac
> +	fi
> +
>   	if defined_CYGPORT_RESTRICT_debuginfo_
>   	then
>   		${objcopy} --strip-all "${exe}";