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