> 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