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

[Florian Pritz]

On Wed, Jan 09, 2019 at 09:49:26AM -0500, Eli Schwartz via arch-projects <arch-projects at archlinux.org> wrote:
> >> 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"?

"could not find existing or staged package for dependency %s" is fine by
me.

> 
> >> +		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.

I get that, but I think that a user that sees these two message may not
understand that a missing dependency is related to the package being
reproducible. To be honest, I actually expected db-update to run all
checks and show all errors at once instead of terminating after the
first one. I now know that this is not the case. I'm pretty sure that I
would have treat these two messages as separate errors and that I'd then
be confused as to what the "second error" is actually about.

I think that it may save a lot of time and confusion if this error
message is clear about what is wrong and how it can be fixed. Most of
the other error messages in db-update are rather clear about the actual
problem. Maybe not as clear as the message I propose here, but clearer
than "Package %s is not reproducible". Apart from that, does it really
hurt to have a more verbose error message? It will only be shown if
there is an actual error and it doesn't influence normal usage. I'd say
we can afford to be more verbose in that case.

If you still think that this message should not be made more verbose,
I'd argue that it should be removed entirely. If we have just the
message about a dependency not being found, it is quite clear to a user
what is wrong and how they could fix the error. I'd say that is much
less confusing than if there were a second message about reproducibility
that some people may or may not consider to be a different, additional
error as I've explained above.

Florian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.archlinux.org/pipermail/arch-projects/attachments/20190109/3686eac2/attachment.asc>