[arch-projects] [dbscripts] [PATCH v2 2/2] Add reproducible archive of packages.

[Eli Schwartz]

On 1/9/19 8:00 AM, Florian Pritz wrote:
> On Tue, Jan 08, 2019 at 06:40:37PM -0500, Eli Schwartz via arch-projects <arch-projects at archlinux.org> wrote:
>> diff --git a/db-archive b/db-archive
>> new file mode 100755
>> index 00000000..5680b9de
>> --- /dev/null
>> +++ b/db-archive
>> @@ -0,0 +1,21 @@
>> +#!/bin/bash
>> +
>> +. "$(dirname "$(readlink -e "$0")")/config"
> 
> This uses $0 (see below).
> 
>> +
>> +if (( $# != 1 )); then
>> +	echo "usage: %s <pkgfile>" "${0##*/}"
>> +	exit 1
>> +fi
>> +
>> +if [[ -n ${ARCHIVEUSER} ]]; then
>> +	exec sudo -u "${ARCHIVEUSER}" bash "${BASH_SOURCE[0]}" "${@}"
> 
> This uses $BASH_SOURCE instead of $0 as used above. Is this intentional,
> if so why? I'd argue that this should also use $0, but maybe I'm missing
> something?

BASH_SOURCE explicitly refers to the file it was sourced/executed from,
it's like the __file__ macro other languages have.

$0 can be anything since programs can modify their argv0 freely, but by
default it is the toplevel script name, which is why it gets used so
much. It's still not technically correct, and I prefer to use BASH_SOURCE.

In short, this is a copy-paste error above.

>> +fi
>> +
>> +pkgfile=${1##*/}
>> +pkgname=${pkgfile%-*-*-*}
>> +archive_dir="${ARCHIVE_BASE}/packages/${pkgname:0:1}/${pkgname}"
>> +
>> +if [[ ! -f ${archive_dir}/${pkgfile} ]]; then
>> +	mkdir -p "${archive_dir}"
>> +	cp -np "${1}"{,.sig} "${archive_dir}/"
>> +fi
>> diff --git a/db-functions b/db-functions
>> index 7aeedced..b8a00b90 100644
>> --- a/db-functions
>> +++ b/db-functions
>> @@ -444,4 +447,24 @@ arch_repo_modify() {
>>  	REPO_MODIFIED=1
>>  }
>>  
>> +# Verify the existence of dependent packages needed by a given pkgfile
>> +# usage: check_reproducible pkgfile
>> +check_reproducible() {
>> +	local pkg dir pkgs=() pkgfile pkgfiles=()
>> +
>> +	mapfile -t pkgs < <(_grep_all_info "${1}" .BUILDINFO installed)
>> +
>> +	for pkg in "${pkgs[@]}"; do
>> +		local pkgname=${pkg%-*-*-*}
>> +		for dir in "${ARCHIVE_BASE}/packages/${pkgname:0:1}/${pkgname}" "${STAGING}"/**/; do
>> +			if pkgfile="$(getpkgfile "${dir}/${pkg}"${PKGEXTS} 2>/dev/null)"; then
>> +				pkgfiles+=("${pkgfile}")
>> +				continue 2
>> +			fi
>> +		done
>> +		error "could not find existing package for %s" "${pkg}"
> 
> 
> I imagine that I'd be confused if I ever saw this error. How about
> clarifying it like this? "could not find package for dependency %s in
> reproducibility archive or your staging directory"

Maybe "existing or staged package for dependency %s"?

>> +		return 1
>> +	done
>> +}
>> +
>>  . "$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")/db-functions-${VCS}"
>> diff --git a/db-update b/db-update
>> index 313fb999..04a29bf3 100755
>> --- a/db-update
>> +++ b/db-update
>> @@ -61,6 +61,9 @@ for repo in "${repos[@]}"; do
>>  			if ! check_builddir "${pkg}"; then
>>  				die "Package %s was not built in a chroot" "$repo/${pkg##*/}"
>>  			fi
>> +			if ! check_reproducible "${pkg}"; then
>> +				die "Package %s is not reproducible" "${pkg}"
> 
> Same as above. I'd suggest something like this:
> 
> "Package %s depends on packages that are missing in the reproducibility
> archive and your staging directory. Ensure that all dependencies either
> exist in the repositories or reproducibility archive already or that
> they are added together with the package in a single call to db-update."

The two errors will only be called together. I think expanding the
message when printing the missing dependency should be enough.

-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/arch-projects/attachments/20190109/129a23ed/attachment.asc>