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

[Eli Schwartz]

On 1/9/19 10:34 AM, Florian Pritz wrote:
> 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.

Well, db-update check doesn't necessarily know what check_reproducible()
cannot find, I guess we could exit directly in there, but...

Hmm, what about:

Package %s is not reproducible. Ensure that all dependencies are
available in the repositories or are added in the same db-update.

-- 
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/8d2947cb/attachment-0001.asc>