Hi On Mon, Nov 30, 2020 at 10:21 AM morganamilo <morganam...@archlinux.org> wrote: > > download_files never set ret on failiure, so even when downloading > fails, the transaction goes on to commit and error out. > > :: Retrieving packages... > python-packaging-20.4-4-any.pkg.tar.zst failed to download > error: failed retrieving file 'python-packaging-20.4-4-any.pkg.tar.zst' from > mirror.oldsql.cc : The requested URL returned error: 404 > warning: failed to retrieve some files > (1/1) checking keys in keyring > (1/1) checking package integrity > error: failed to commit transaction (wrong or NULL argument passed) > Errors occurred, no packages were upgraded. > > Also make the ret checking more consistent. > > diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c > index 601f1d69..5d8652a5 100644 > --- a/lib/libalpm/sync.c > +++ b/lib/libalpm/sync.c > @@ -769,7 +769,7 @@ static int download_files(alpm_handle_t *handle) > } > > ret = find_dl_candidates(handle, &files); > - if(ret) { > + if(ret != 0) { > goto finish; > }
"if(ret)" is semantically equivalent to "if(ret != 0)". Is the change really needed here? > > @@ -818,7 +818,9 @@ static int download_files(alpm_handle_t *handle) > > payloads = alpm_list_add(payloads, payload); > } > - if(_alpm_download(handle, payloads, cachedir) == -1) { > + > + ret = _alpm_download(handle, payloads, cachedir); > + if(ret != 0) { This part LGTM.