[pacman-dev] Move -Sw to front-end
Hi! After moving -Sp to front-end I decided to move -Sw as well, thus we would reach the following goal: alpm_trans_commit represents a _real_ transaction, ie. its successful run will cause a localdb change. It would make our code much more understandable, for example we could simply move ldconfig call to trans_commit (this would fix the duplicated ldconfig run issue on -Su %REPLACES%) and this would be helpful in my refresh_pkgcache branch etc. The reason for not providing a patch, I couldn't decide how to make this "download part" public. (This should be factorised from sync_commit). I put some ideas/opinions here, some of them may be conflicting. I appreciate any feedback. 1. Pre-condition: download callback stuffs are attached to handle, not to trans. Cool, "nice" transaction independent download stuff is possible. 2. First I thought something like this: alpm_download_packages(alpm_list_t *packages, ...) This seems the easiest to implement (cut from sync_commit). However, if we want transaction-independent function, we should do pmsyncpkg_t -> pmpkg_t conversion always. (Ugly) 3. Then I started to like the following: alpm_download_package(pmpkg_t *package) We have a problem with this, in this case if we want to keep the current "download all packages then check all integrity" order we should make something like alpm_check_integrity public (no, too complicated). Alternatively, we could check_integrity in download_package. This is also logical, but changes the current behaviour (order). Either 2. or 3. seems easy first. But there is a very minor thing, I want to fix, which makes our life more difficult: delta_path and download_size (of pmpkg_t) 4. First I thought, that these shouldn't belong to pmpkg_t, because they are not "constant" package properties like packager etc. These depend on the actual state of the download cache. I started to like the idea of moving them to pmsyncpkg_t instead. (Btw, alpm_pkg_download_size is in sync.c) But this just broke my plan to make transaction independent download stuff, so this idea was dropped. 5. These two fields are computed by compute_download_size as a last step of sync_prepare. If front-end is allowed to download some packages manually, then we should update these fields automatically. (these fields are used by determining the to-be-downloaded files) Even if we drop "-Sw in front-end" idea, it would nice, if these (or at least the public download_size) fields were "on-demand" stuffs, like other fields of pmpkg_t (new infolevel?, or download_size = -1 for "not yet computed"?): not hardwired sync_prepare computing, and automatically update the field if needed. (Because the front-end is allowed to request download_size field between sync_addtarget and sync_prepare etc.) However, if we want to update these fields after successful download, we should completely restructure the current download stuff (atm we just collect files then download them, and forget about packages):-/ I may mixed many things here, and I may misunderstand something (this delta stuff made things quite complicated in sync.c) So I decided to rework this download part, but I am unsure about directions. Bye
On Tue, Aug 26, 2008 at 4:28 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi!
After moving -Sp to front-end I decided to move -Sw as well, thus we would reach the following goal: alpm_trans_commit represents a _real_ transaction, ie. its successful run will cause a localdb change.
It would make our code much more understandable, for example we could simply move ldconfig call to trans_commit (this would fix the duplicated ldconfig run issue on -Su %REPLACES%) and this would be helpful in my refresh_pkgcache branch etc.
The reason for not providing a patch, I couldn't decide how to make this "download part" public. (This should be factorised from sync_commit). I put some ideas/opinions here, some of them may be conflicting. I appreciate any feedback.
1. Pre-condition: download callback stuffs are attached to handle, not to trans. Cool, "nice" transaction independent download stuff is possible. 2. First I thought something like this: alpm_download_packages(alpm_list_t *packages, ...) This seems the easiest to implement (cut from sync_commit). However, if we want transaction-independent function, we should do pmsyncpkg_t -> pmpkg_t conversion always. (Ugly) 3. Then I started to like the following: alpm_download_package(pmpkg_t *package) We have a problem with this, in this case if we want to keep the current "download all packages then check all integrity" order we should make something like alpm_check_integrity public (no, too complicated). Alternatively, we could check_integrity in download_package. This is also logical, but changes the current behaviour (order).
I don't understand why you need to do ugly conversions for 2 but not for 3. About the integrity order, I don't think it matters, both ways are fine to me. If you want to check it from frontend, what about this : alpm_pkg_checkmd5sum
Either 2. or 3. seems easy first. But there is a very minor thing, I want to fix, which makes our life more difficult:
delta_path and download_size (of pmpkg_t)
4. First I thought, that these shouldn't belong to pmpkg_t, because they are not "constant" package properties like packager etc. These depend on the actual state of the download cache. I started to like the idea of moving them to pmsyncpkg_t instead. (Btw, alpm_pkg_download_size is in sync.c) But this just broke my plan to make transaction independent download stuff, so this idea was dropped.
delta_path and dowload_size are indeed special fields. I don't know if it is really a problem, and if there are nicer ways to handle this.
5. These two fields are computed by compute_download_size as a last step of sync_prepare. If front-end is allowed to download some packages manually, then we should update these fields automatically. (these fields are used by determining the to-be-downloaded files) Even if we drop "-Sw in front-end" idea, it would nice, if these (or at least the public download_size) fields were "on-demand" stuffs, like other fields of pmpkg_t (new infolevel?, or download_size = -1 for "not yet computed"?): not hardwired sync_prepare computing, and automatically update the field if needed. (Because the front-end is allowed to request download_size field between sync_addtarget and sync_prepare etc.) However, if we want to update these fields after successful download, we should completely restructure the current download stuff (atm we just collect files then download them, and forget about packages):-/
I am definitely not proud of this hardwired sync_prepare computing. About on-demand access, is that possible? As you said, these two fields depend on the filecache. How do you detect that the filecache has changed? So even if you have some on-demand access to both fields, I believe you still need to update them each time the filecache is affected. Maybe this is doable if each functions causing write to the filecache also update these two fields. So maybe a download(pkg) function could handle that? And I guess that function should take care of delta too : if an old package is present in filecache, then compute delta path, download delta files, and apply them. Otherwise just download the package?
I may mixed many things here, and I may misunderstand something (this delta stuff made things quite complicated in sync.c)
I agree, this complicates things quite a lot for no benefit (at least for now).
2. First I thought something like this: alpm_download_packages(alpm_list_t *packages, ...) This seems the easiest to implement (cut from sync_commit). However, if we want transaction-independent function, we should do pmsyncpkg_t -> pmpkg_t conversion always. (Ugly) 3. Then I started to like the following: alpm_download_package(pmpkg_t *package) We have a problem with this, in this case if we want to keep the current "download all packages then check all integrity" order we should make something like alpm_check_integrity public (no, too complicated). Alternatively, we could check_integrity in download_package. This is also logical, but changes the current behaviour (order).
I don't understand why you need to do ugly conversions for 2 but not for 3.
2. you have to build/free a new list 3. you can call on "spkg->pkg" for each sync package This is not much difference, indeed. Btw, almost all alpm functions require pmpkg_t*, resolvedeps, checkdeps, checkconflicts... So "build/free a new list" is already done in many places.
I am definitely not proud of this hardwired sync_prepare computing. About on-demand access, is that possible? As you said, these two fields depend on the filecache. How do you detect that the filecache has changed?
I assume that filecache is modified via alpm. (This is partly true, because -Sc is implemented in the front-end!) This is closer to reality than the current hardwired stuff.
So even if you have some on-demand access to both fields, I believe you still need to update them each time the filecache is affected. Maybe this is doable if each functions causing write to the filecache also update these two fields. So maybe a download(pkg) function could handle that?
Yes I thought that download function should handle that. I thought about a very primitive delta_path handling as a starting point. (If all delta downloads were successful, clear delta_path, if one of them was unsuccessful, set delta_path (and download_size) to "not computed")
And I guess that function should take care of delta too : if an old package is present in filecache, then compute delta path, download delta files, and apply them. Otherwise just download the package?
I may mixed many things here, and I may misunderstand something (this delta stuff made things quite complicated in sync.c)
I agree, this complicates things quite a lot for no benefit (at least for now).
And it is difficult to test them... I still don't know what to do;-) I even don't know whether you support the primary "goal": cleaner trans_commit. Bye
On Tue, Aug 26, 2008 at 6:26 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
2. you have to build/free a new list 3. you can call on "spkg->pkg" for each sync package This is not much difference, indeed.
Btw, almost all alpm functions require pmpkg_t*, resolvedeps, checkdeps, checkconflicts... So "build/free a new list" is already done in many places.
So that is what I thought. Using a list is fine too.
I assume that filecache is modified via alpm. (This is partly true, because -Sc is implemented in the front-end!) This is closer to reality than the current hardwired stuff.
Agreed.
Yes I thought that download function should handle that. I thought about a very primitive delta_path handling as a starting point. (If all delta downloads were successful, clear delta_path, if one of them was unsuccessful, set delta_path (and download_size) to "not computed")
Hmm... If one of them was unsuccessful, maybe it should just revert to the normal non-delta case?
I may mixed many things here, and I may misunderstand something (this delta stuff made things quite complicated in sync.c)
I agree, this complicates things quite a lot for no benefit (at least for now).
And it is difficult to test them...
True. What Nathan did might help : http://www.archlinux.org/pipermail/pacman-dev/2007-December/010476.html
I still don't know what to do;-) I even don't know whether you support the primary "goal": cleaner trans_commit.
Of course, I always support cleaner anything.
participants (2)
-
Nagy Gabor
-
Xavier