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@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
signature.asc
Description: OpenPGP digital signature
