[pacman-dev] [PATCH 2/2] ensure package download sizes are invalidated when an error occurs

Nagy Gabor ngaba at bibl.u-szeged.hu
Sun May 16 19:17:17 EDT 2010


> Hi
> 
> On Sun, 2010-05-16 at 14:54 +0200, Nagy Gabor wrote:
> > The spirit of this patch is better, but it is still not perfect, because
> > of the -Sw (PM_TRANS_FLAG_DOWNLOADONLY) case. That return(0) can also be
> > changed to goto error, and I can see one more RET_ERR in the function.
> 
> See my other patch for the -Sw case. I seem to have a tendency to
> overlook the RET_ERR ones though...

Oh, I didn't see your other patch, sorry. :-) 
 
> > I just note that in case of error-free -S commit operation, this
> > code-part is needless/overkill, because the target pkgcache packages are
> > replaced in the integrity-check part by the pmpkg_t of pkg.tar.xz file
> > (line 949), and then every download_size is either 0 or unset.
> 
> Ok, good point, I didn't notice that. In that case I think we should
> move this back to where it was originally, but make sure only packages
> for the current repo get unset. This might be a good time to introduce a
> pmpkg_t parameter to the download callback as well.

Yes, I agree, it would be "nice" to refactor our download code. Then we could do
this unset stuff in a more clever way. In addition, we could finally
drop this ugly PM_TRANS_FLAG_DOWNLOADONLY flag, and allow the front-end
to do the downloading manually, if needed. (Fake -Sw trans_commit has
some other side effects, like stupid run_ldconfig implementation etc.) But this is not
at the top of my TODO list... But patches are welcome. :-) 

> > An other side note:
> > I have some bad feelings when I see many foo->download_size in this
> > codepart. We have a good access function for this. I know that atm all
> > download_sizes are filled in correctly at the end of sync_prepare, but
> > that may be changed later. But this direct access causes some nanosec
> > speedup, so I dunno. :-)
> 
> I agree, but I don't mind either way. Since PM_ERR_PKG_INVALID_NAME is
> checked in the commit I'm not sure we even need the pre-computation any
> more. I'll have a closer look when I get the chance.
 
Personally I preferred your "original" patch, but now I am fine with
this way. We have more important open bugs (I should not have
mentioned this at all). :-)

NG


More information about the pacman-dev mailing list