[pacman-dev] -Sc and shared cache

older
[pacman-dev] [GIT] The official...

Xavier

14 Feb 2008 14 Feb '08

11:24 p.m.

This was already discussed here : http://www.archlinux.org/pipermail/pacman-dev/2008-January/010940.html And then more recently in bbs : http://bbs.archlinux.org/viewtopic.php?id=43809 The guy there started to write a little script on his own, and that made me realize that the behavior he wanted is actually pretty close to the current -Sc code. Only a small part needs to be changed, most of the code can be re-used. But instead of only keeping the packages which are in the local database, we would keep the packages which are in a sync database. http://bbs.archlinux.org/viewtopic.php?pid=330487#p330487 So I hacked a patch together. It's not really usable yet, because it's not configurable , only the second behavior will be used. It's only a proof of concept for now. (and anyway, it only prints the package that it wants to remove, it doesn't actually remove them, for testing purpose) I just wanted to know what others think about proposing two different behaviors for -Sc, and how the user could choose between both (maybe an additional flag, but which?).

Attachments:

second-Sc-behavior.diff (text/x-patch — 1.9 KB)

Show replies by date

Roman Kyrylych

14 Feb 14 Feb

11:40 p.m.

2008/2/15, Xavier :

...

This was already discussed here : http://www.archlinux.org/pipermail/pacman-dev/2008-January/010940.html

And then more recently in bbs : http://bbs.archlinux.org/viewtopic.php?id=43809

The guy there started to write a little script on his own, and that made me realize that the behavior he wanted is actually pretty close to the current -Sc code. Only a small part needs to be changed, most of the code can be re-used. But instead of only keeping the packages which are in the local database, we would keep the packages which are in a sync database. http://bbs.archlinux.org/viewtopic.php?pid=330487#p330487

So I hacked a patch together. It's not really usable yet, because it's not configurable , only the second behavior will be used. It's only a proof of concept for now. (and anyway, it only prints the package that it wants to remove, it doesn't actually remove them, for testing purpose) I just wanted to know what others think about proposing two different behaviors for -Sc, and how the user could choose between both (maybe an additional flag, but which?).

Great work! +111 from me for keeping the current behaviour of -Sc as the default and bring back the old -Sc behaviour available with some extra flag. I think it can be pacman.conf-only, so it can be long-named (e.g. KeepNotInstalledButUpToDatePackagesInCache ;-P) I think users will prefer only one behaviour for all -Sc operations anyway. -- Roman Kyrylych (Роман Кирилич)

Xavier

15 Feb 15 Feb

12:38 a.m.

On Fri, Feb 15, 2008 at 12:40:31AM +0200, Roman Kyrylych wrote:

...

Great work! +111 from me for keeping the current behaviour of -Sc as the default and bring back the old -Sc behaviour available with some extra flag. I think it can be pacman.conf-only, so it can be long-named (e.g. KeepNotInstalledButUpToDatePackagesInCache ;-P) I think users will prefer only one behaviour for all -Sc operations anyway.

That makes sense. Your suggestion is a bit long though :)

Chantry Xavier

16 Feb 16 Feb

4:06 p.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared. When this option is enabled, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache. Ref : http://www.archlinux.org/pipermail/pacman-dev/2008-February/011140.html Signed-off-by: Chantry Xavier --- doc/pacman.8.txt | 3 ++ doc/pacman.conf.5.txt | 8 ++++++ src/pacman/conf.h | 9 +++++-- src/pacman/pacman.c | 3 ++ src/pacman/sync.c | 58 +++++++++++++++++++++++++++++++++--------------- 5 files changed, 60 insertions(+), 21 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index f6eb69c..611fa3e 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -241,6 +241,9 @@ Sync Options[[SO]] packages that are no longer installed; use two to remove all packages from the cache. In both cases, you will have a yes or no option to remove packages and/or unused downloaded databases. ++ +If you use a network shared cache, see the 'SharedPkgCache' option in +linkman:pacman.conf[5]. *-e, \--dependsonly*:: Install all dependencies of a package, but not the specified package diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index e8f7454..c1b76e8 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -131,6 +131,14 @@ Options than the percent of each individual download target. The progress bar is still based solely on the current file download. +*SharedPkgCache*:: + If set, the '-Sc' operation will clean outdated packages (not present in + any sync database), rather than packages that are no longer installed + (not present in the local database). + This is especially useful when the package cache is shared among multiple + machines, where the local databases are usually different, but the sync + databases used could be the same. + Repository Sections ------------------- Each repository section defines a section name and at least one location where diff --git a/src/pacman/conf.h b/src/pacman/conf.h index f804f56..a54a9f1 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -67,9 +67,12 @@ typedef struct __config_t { unsigned short chomp; /* I Love Candy! */ unsigned short usecolor; /* enable colorful output */ unsigned short showsize; /* show individual package sizes */ - unsigned short totaldownload; /* When downloading, display the amount - downloaded, rate, ETA, and percent - downloaded of the total download list */ + /* When downloading, display the amount downloaded, rate, ETA, and percent + * downloaded of the total download list */ + unsigned short totaldownload; + /* if the cache is shared, -Sc will clean outdated packages rather than + * packages that are no longer installed */ + unsigned short sharedpkgcache; } config_t; /* Operations */ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 377ea3f..7753833 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -638,6 +638,9 @@ static int _parseconfig(const char *file, const char *givensection, } else if(strcmp(key, "TotalDownload") == 0 || strcmp(upperkey, "TOTALDOWNLOAD") == 0) { config->totaldownload = 1; pm_printf(PM_LOG_DEBUG, "config: totaldownload\n"); + } else if(strcmp(key, "SharedPkgCache") == 0 || strcmp(upperkey, "SHAREDPKGCACHE") == 0) { + config->sharedpkgcache = 1; + pm_printf(PM_LOG_DEBUG, "config: usedelta\n"); } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 27218d6..cf11710 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -125,13 +125,17 @@ static int sync_cleancache(int level) /* incomplete cleanup */ DIR *dir; struct dirent *ent; - /* Let's vastly improve the way this is done. Before, we went by package - * name. Instead, let's only keep packages we have installed. Open up each - * package and see if it has an entry in the local DB; if not, delete it. - */ + /* Open up each package and see if it has an entry in the local DB; if not, + * delete it. */ printf(_("Cache directory: %s\n"), cachedir); - if(!yesno(_("Do you want to remove uninstalled packages from cache? [Y/n] "))) { - return(0); + if(config->sharedpkgcache) { + if(!yesno(_("Do you want to remove outdated packages from cache? [Y/n] "))) { + return(0); + } + } else { + if(!yesno(_("Do you want to remove uninstalled packages from cache? [Y/n] "))) { + return(0); + } } printf(_("removing old packages from cache... ")); @@ -145,13 +149,14 @@ static int sync_cleancache(int level) /* step through the directory one file at a time */ while((ent = readdir(dir)) != NULL) { char path[PATH_MAX]; - pmpkg_t *localpkg = NULL, *dbpkg = NULL; + int delete = 1; + pmpkg_t *localpkg = NULL, *pkg = NULL; if(!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { continue; } /* build the full filepath */ - snprintf(path, PATH_MAX, "%s/%s", cachedir, ent->d_name); + snprintf(path, PATH_MAX, "%s%s", cachedir, ent->d_name); /* attempt to load the package, skip file on failures as we may have * files here that aren't valid packages. we also don't need a full @@ -159,19 +164,36 @@ static int sync_cleancache(int level) if(alpm_pkg_load(path, 0, &localpkg) != 0 || localpkg == NULL) { continue; } - /* check if this package is in the local DB */ - dbpkg = alpm_db_get_pkg(db_local, alpm_pkg_get_name(localpkg)); - if(dbpkg == NULL) { - /* delete package, not present in local DB */ - unlink(path); - } else if(alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), - alpm_pkg_get_version(dbpkg)) != 0) { - /* delete package, it was found but version differs */ - unlink(path); + if(config->sharedpkgcache) { + /* check if this package is in a sync DB */ + alpm_list_t *sync_dbs = alpm_option_get_syncdbs(); + alpm_list_t *j; + + for(j = sync_dbs; j; j = alpm_list_next(j)) { + pmdb_t *db = alpm_list_getdata(j); + pkg = alpm_db_get_pkg(db, alpm_pkg_get_name(localpkg)); + if(pkg != NULL && alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), + alpm_pkg_get_version(pkg)) == 0) { + /* package was found in a sync DB and version matches, keep it */ + delete = 0; + break; + } + } + } else { + /* check if this package is in the local DB */ + pkg = alpm_db_get_pkg(db_local, alpm_pkg_get_name(localpkg)); + if(pkg != NULL && alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), + alpm_pkg_get_version(pkg)) == 0) { + /* package was found in local DB and version matches, keep it */ + delete = 0; + } } - /* else version was the same, so keep the package */ /* free the local file package */ alpm_pkg_free(localpkg); + + if(delete) { + unlink(path); + } } printf(_("done.\n")); } else { -- 1.5.4

Roman Kyrylych

17 Feb 17 Feb

4:37 p.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

2008/2/16, Chantry Xavier :

...

As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared.

When this option is enabled, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache.

Great name! :-) BTW, I've just realized that there might be a problem with "no longer in any sync db" that, I think was not in 3.0's -Sc: if SystemA has core,extra,community enabled and SystemB only core&extra then doing -Sc on SystemB will cleanup community packages from the shared cache. I don't know if it's worth to fix this by using a more complicated (filename comparison, AFAIR) aproach like in 3.0. :-/ -- Roman Kyrylych (Роман Кирилич)

Xavier

4:46 p.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

On Sun, Feb 17, 2008 at 05:37:13PM +0200, Roman Kyrylych wrote:

...

2008/2/16, Chantry Xavier :

...
As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared.

When this option is enabled, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache.

Great name! :-)

BTW, I've just realized that there might be a problem with "no longer in any sync db" that, I think was not in 3.0's -Sc: if SystemA has core,extra,community enabled and SystemB only core&extra then doing -Sc on SystemB will cleanup community packages from the shared cache. I don't know if it's worth to fix this by using a more complicated (filename comparison, AFAIR) aproach like in 3.0. :-/

Indeed, that's a problem, but don't know if it's a big one. I would think it's rather common to use the same set of sync db on a set of machines sharing pkg cache. And that approach is indeed much simpler. I mentioned this in the man page, that this option is rather meant for setup where same sync dbs are used.

Roman Kyrylych

5:14 p.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

2008/2/17, Xavier :

...

On Sun, Feb 17, 2008 at 05:37:13PM +0200, Roman Kyrylych wrote:

...
2008/2/16, Chantry Xavier :

...
As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared.

When this option is enabled, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache.

Great name! :-)

BTW, I've just realized that there might be a problem with "no longer in any sync db" that, I think was not in 3.0's -Sc: if SystemA has core,extra,community enabled and SystemB only core&extra then doing -Sc on SystemB will cleanup community packages from the shared cache. I don't know if it's worth to fix this by using a more complicated (filename comparison, AFAIR) aproach like in 3.0. :-/

Indeed, that's a problem, but don't know if it's a big one. I would think it's rather common to use the same set of sync db on a set of machines sharing pkg cache. And that approach is indeed much simpler.

I mentioned this in the man page, that this option is rather meant for setup where same sync dbs are used.

Ah, OK, didn't read that. :-) Then we can forget about this corner case, it can be worked around by cache cleanup scripts based on filename parsing (available on BBS and ML). -- Roman Kyrylych (Роман Кирилич)

Mark Constable

6:44 p.m.

New subject: [pacman-dev] Another clean repo script

On 18 Feb 2008 02:14, Roman Kyrylych wrote:

...

... Then we can forget about this corner case, it can be worked around by cache cleanup scripts based on filename parsing (available on BBS and ML).

Along these lines this could be useful for someone searching through the list archives. It just removes old duplicates. I need it to clean up my local custom repo when I move $PKGDEST contents into it before rebuilding the repo database. This construct has so far caught every package name I've thrown at it... pkgname=$(echo $FILENAME|sed 's/^$.*[^-][^0-9]$-[0-9].*/\1/') mpkg_cleanrepo() { [ -n "$1" ] && WORK=$1 || WORK=/var/cache/pacman/pkg [ -d $WORK ] && cd $WORK || { echo "ERROR: $WORK does not exist" && return } local TMP=($(echo *.pkg*)) # to prevent set -e failing [ ${#TMP[@]} -gt 1 ] && FILES=$(/bin/ls -v *.pkg*) || return BEFORE_DU=$(du -sb|cut -f1) for FILE in $FILES; do NAME=$(echo $FILE|sed 's/^$.*[^-][^0-9]$-[0-9].*/\1/') if [ "$PREV_NAME" = "$NAME" ]; then # msg "$PREV_FILE\n" 0 "REMOVED" 31 sudo rm $PREV_FILE CNT_RM=$(($CNT_RM+1)) else CNT_NOTRM=$(($CNT_NOTRM+1)) fi PREV_FILE=$FILE PREV_NAME=$NAME done AFTER_DU=$(du -sb|cut -f1) DIFF_DU=$(($BEFORE_DU-$AFTER_DU)) CURRENT_DU=$(echo $((AFTER_DU/1000000))|sed ':a;s/\B[0-9]\{3\}\>/,&/;ta') REMOVED_DU=$(echo $((DIFF_DU/1000000))|sed ':a;s/\B[0-9]\{3\}\>/,&/;ta') if [ $CNT_RM -gt 0 ]; then # msg "$(pwd)\n" 33 "Package Path:" 31 printf "%-24s %4d %8sM\n" "Packages removed" $CNT_RM $REMOVED_DU printf "%-24s %4d %8sM\n" "Remaining packages" $CNT_NOTRM $CURRENT_DU fi } --markc

Dan McGee

18 Feb 18 Feb

7:37 a.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

On Feb 17, 2008 10:14 AM, Roman Kyrylych wrote:

...

2008/2/17, Xavier :

...
On Sun, Feb 17, 2008 at 05:37:13PM +0200, Roman Kyrylych wrote:

...
2008/2/16, Chantry Xavier :

...
As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared.

When this option is enabled, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache.

Great name! :-)

BTW, I've just realized that there might be a problem with "no longer in any sync db" that, I think was not in 3.0's -Sc: if SystemA has core,extra,community enabled and SystemB only core&extra then doing -Sc on SystemB will cleanup community packages from the shared cache. I don't know if it's worth to fix this by using a more complicated (filename comparison, AFAIR) aproach like in 3.0. :-/

Indeed, that's a problem, but don't know if it's a big one. I would think it's rather common to use the same set of sync db on a set of machines sharing pkg cache. And that approach is indeed much simpler.

I mentioned this in the man page, that this option is rather meant for setup where same sync dbs are used.

Ah, OK, didn't read that. :-) Then we can forget about this corner case, it can be worked around by cache cleanup scripts based on filename parsing (available on BBS and ML).

And now we are back to the whole reason why I think this is a pointless problem to solve. :P Once you venture into the shared cache realm, I just don't see a need for pacman to hand-hold you. So we have Xav and Roman in support of this, and me against it. I don't want to say "majority rules" here, but I would like to get a few more voices on this one. Aaron? -Dan

Roman Kyrylych

9:06 a.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

2008/2/18, Dan McGee :

...

On Feb 17, 2008 10:14 AM, Roman Kyrylych wrote:

...
2008/2/17, Xavier :

...
On Sun, Feb 17, 2008 at 05:37:13PM +0200, Roman Kyrylych wrote:

...
2008/2/16, Chantry Xavier :

...
As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared.

When this option is enabled, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache.

Great name! :-)

BTW, I've just realized that there might be a problem with "no longer in any sync db" that, I think was not in 3.0's -Sc: if SystemA has core,extra,community enabled and SystemB only core&extra then doing -Sc on SystemB will cleanup community packages from the shared cache. I don't know if it's worth to fix this by using a more complicated (filename comparison, AFAIR) aproach like in 3.0. :-/

Indeed, that's a problem, but don't know if it's a big one. I would think it's rather common to use the same set of sync db on a set of machines sharing pkg cache. And that approach is indeed much simpler.

I mentioned this in the man page, that this option is rather meant for setup where same sync dbs are used.

Ah, OK, didn't read that. :-) Then we can forget about this corner case, it can be worked around by cache cleanup scripts based on filename parsing (available on BBS and ML).

And now we are back to the whole reason why I think this is a pointless problem to solve. :P

Not pointless at all, Xavier's solution will work on majority of configurations with a shared cache (that have the same set of repos enabled).

...

Once you venture into the shared cache realm, I just don't see a need for pacman to hand-hold you.

It's useful for users that don't use a shared cache, but want to keep non-installed but up-to-date packages in cache (e.g. me :-P) - there are some valid reasons for doing this.

...

So we have Xav and Roman in support of this, and me against it. I don't want to say "majority rules" here, but I would like to get a few more voices on this one. Aaron?

-- Roman Kyrylych (Роман Кирилич)

Xavier

11:05 a.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

On Mon, Feb 18, 2008 at 10:06:35AM +0200, Roman Kyrylych wrote:

...

Not pointless at all, Xavier's solution will work on majority of configurations with a shared cache (that have the same set of repos enabled).

It's useful for users that don't use a shared cache, but want to keep non-installed but up-to-date packages in cache (e.g. me :-P) - there are some valid reasons for doing this.

Ah. Then the SharedPkgCache name is not so great :/ I also thought of "CleanOutdated".

Roman Kyrylych

9:47 p.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

2008/2/18, Xavier :

...

On Mon, Feb 18, 2008 at 10:06:35AM +0200, Roman Kyrylych wrote:

...
Not pointless at all, Xavier's solution will work on majority of configurations with a shared cache (that have the same set of repos enabled).

It's useful for users that don't use a shared cache, but want to keep non-installed but up-to-date packages in cache (e.g. me :-P) - there are some valid reasons for doing this.

Ah. Then the SharedPkgCache name is not so great :/

...

I also thought of "CleanOutdated". I don't understand that ( == sounds confusing ) :-/ Unfortunately I cannot think of a better name, I think SharedPkgCache is fine until some really good and short name will be invented.

-- Roman Kyrylych (Роман Кирилич)

Nagy Gabor

19 Feb 19 Feb

5:03 p.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

...

On Mon, Feb 18, 2008 at 10:06:35AM +0200, Roman Kyrylych wrote:

...
Not pointless at all, Xavier's solution will work on majority of configurations with a shared cache (that have the same set of repos enabled).

It's useful for users that don't use a shared cache, but want to keep non-installed but up-to-date packages in cache (e.g. me :-P) - there are some valid reasons for doing this.

Ah. Then the SharedPkgCache name is not so great :/ I also thought of "CleanOutdated".

+1 ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/

Dan McGee

11 Mar 11 Mar

1:44 a.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

2008/2/19 Nagy Gabor :

...

...
On Mon, Feb 18, 2008 at 10:06:35AM +0200, Roman Kyrylych wrote:

...
Not pointless at all, Xavier's solution will work on majority of configurations with a shared cache (that have the same set of repos enabled).

It's useful for users that don't use a shared cache, but want to keep non-installed but up-to-date packages in cache (e.g. me :-P) - there are some valid reasons for doing this.

Ah. Then the SharedPkgCache name is not so great :/ I also thought of "CleanOutdated".

+1

OK, after thinking about this, how about something like CleanPackages = Installed (default if nothing specified, look at localdb) CleanPackages = UpToDate ('shared package cache', look at syncdb) Problem is those specs are reverse from how they sound- we are keeping and not cleaning what was specified. I'm saying do it this way rather than a boolean option so when some smart guy on the list comes up with a date-based cleaning solution or something, we can also accommodate for that. Hmm: CleanMethod = {KeepInstalled, KeepCurrent, KeepRecent} ? -Dan

Xavier

1:49 p.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

On Mon, Mar 10, 2008 at 07:44:23PM -0500, Dan McGee wrote:

...

OK, after thinking about this, how about something like

CleanPackages = Installed (default if nothing specified, look at localdb) CleanPackages = UpToDate ('shared package cache', look at syncdb)

Problem is those specs are reverse from how they sound- we are keeping and not cleaning what was specified.

I'm saying do it this way rather than a boolean option so when some smart guy on the list comes up with a date-based cleaning solution or something, we can also accommodate for that.

Hmm: CleanMethod = {KeepInstalled, KeepCurrent, KeepRecent} ?

That sounds nicer, but this makes the implementation more complicated. Firstly, we now need some error handling when the specified method doesn't exist. And secondly, we can't just store a boolean anymore. So either we store the string directly, and then it's a pita if we need to do the string comparisons several times, especially with the insensitive comparison mess. Or we could just use an integer, but then it's not clear what it means. So I added the following enum in conf.h : +/* clean method */ +enum { + PM_CLEAN_KEEPINST = 0, /* default */ + PM_CLEAN_KEEPCUR +}; But I still find your way better especially because the names are much clearer, and it's also extensible indeed. So I will attach the updated patch, but it will need to be reviewed more carefully with the above two slight issues. :)

Chantry Xavier

1:59 p.m.

New subject: [pacman-dev] [PATCH] Add new CleanMethod option.

As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared. This option has two possible values : KeepInstalled and KeepCurrent With KeepCurrent, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache. Ref : http://www.archlinux.org/pipermail/pacman-dev/2008-February/011140.html Signed-off-by: Chantry Xavier --- doc/pacman.8.txt | 3 ++ doc/pacman.conf.5.txt | 9 +++++++ src/pacman/conf.h | 13 +++++++-- src/pacman/pacman.c | 12 +++++++++ src/pacman/sync.c | 64 +++++++++++++++++++++++++++++++++++------------- 5 files changed, 80 insertions(+), 21 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 8235875..7c45ce9 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -244,6 +244,9 @@ Sync Options[[SO]] packages that are no longer installed; use two to remove all packages from the cache. In both cases, you will have a yes or no option to remove packages and/or unused downloaded databases. ++ +If you use a network shared cache, see the 'CleanMethod' option in +linkman:pacman.conf[5]. *-e, \--dependsonly*:: Install all dependencies of a package, but not the specified package diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index e8f7454..8a229f1 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -114,6 +114,15 @@ Options 'index.php', then you would not want the 'index.html' file to be extracted from the 'apache' package. +*CleanMethod =* KeepInstalled | KeepCurrent:: + If set to `KeepInstalled` (the default), the '-Sc' operation will clean + packages that are no longer installed (not present in the local database). + If set to `KeepCurrent`, '-Sc' will clean outdated packages (not present in + any sync database). + The second behavior is especially useful when the package cache is shared + among multiple machines, where the local databases are usually different, but + the sync databases used could be the same. + *UseSyslog*:: Log action messages through syslog(). This will insert log entries into ``/var/log/messages'' or equivalent. diff --git a/src/pacman/conf.h b/src/pacman/conf.h index d53db08..48ddba1 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -66,9 +66,10 @@ typedef struct __config_t { unsigned short chomp; /* I Love Candy! */ unsigned short usecolor; /* enable colorful output */ unsigned short showsize; /* show individual package sizes */ - unsigned short totaldownload; /* When downloading, display the amount - downloaded, rate, ETA, and percent - downloaded of the total download list */ + /* When downloading, display the amount downloaded, rate, ETA, and percent + * downloaded of the total download list */ + unsigned short totaldownload; + unsigned short cleanmethod; /* select -Sc behavior */ } config_t; /* Operations */ @@ -81,6 +82,12 @@ enum { PM_OP_DEPTEST }; +/* clean method */ +enum { + PM_CLEAN_KEEPINST = 0, /* default */ + PM_CLEAN_KEEPCUR +}; + /* global config variable */ extern config_t *config; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index f87db27..80f7d2d 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -699,6 +699,18 @@ static int _parseconfig(const char *file, const char *givensection, } else if (strcmp(key, "XferCommand") == 0 || strcmp(upperkey, "XFERCOMMAND") == 0) { alpm_option_set_xfercommand(ptr); pm_printf(PM_LOG_DEBUG, "config: xfercommand: %s\n", ptr); + } else if (strcmp(key, "CleanMethod") == 0 || strcmp(upperkey, "CLEANMETHOD") == 0) { + char *upperptr = strtoupper(strdup(ptr)); + if (strcmp(ptr, "KeepInstalled") == 0 || strcmp(upperptr, "KEEPINSTALLED") == 0) { + config->cleanmethod = PM_CLEAN_KEEPINST; + } else if (strcmp(ptr, "KeepCurrent") == 0 || strcmp(upperptr, "KEEPCURRENT") == 0) { + config->cleanmethod = PM_CLEAN_KEEPCUR; + } else { + pm_printf(PM_LOG_ERROR, _("invalid value for 'CleanMethod' : '%s'\n"), ptr); + return(1); + } + pm_printf(PM_LOG_DEBUG, "config: cleanmethod: %s\n", ptr); + free(upperptr); } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); diff --git a/src/pacman/sync.c b/src/pacman/sync.c index f2d8f4b..ffa6b61 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -133,13 +133,20 @@ static int sync_cleancache(int level) /* incomplete cleanup */ DIR *dir; struct dirent *ent; - /* Let's vastly improve the way this is done. Before, we went by package - * name. Instead, let's only keep packages we have installed. Open up each - * package and see if it has an entry in the local DB; if not, delete it. - */ + /* Open up each package and see if it has an entry in the local DB; if not, + * delete it. */ printf(_("Cache directory: %s\n"), cachedir); - if(!yesno(1, _("Do you want to remove uninstalled packages from cache?"))) { - return(0); + if(config->cleanmethod == PM_CLEAN_KEEPCUR) { + if(!yesno(1, _("Do you want to remove outdated packages from cache?"))) { + return(0); + } + } else if(config->cleanmethod == PM_CLEAN_KEEPINST) { + if(!yesno(1, _("Do you want to remove uninstalled packages from cache?"))) { + return(0); + } + } else { + /* this should not happen : the config parsing doesn't set any other value */ + return(1); } printf(_("removing old packages from cache... ")); @@ -153,13 +160,14 @@ static int sync_cleancache(int level) /* step through the directory one file at a time */ while((ent = readdir(dir)) != NULL) { char path[PATH_MAX]; - pmpkg_t *localpkg = NULL, *dbpkg = NULL; + int delete = 1; + pmpkg_t *localpkg = NULL, *pkg = NULL; if(!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { continue; } /* build the full filepath */ - snprintf(path, PATH_MAX, "%s/%s", cachedir, ent->d_name); + snprintf(path, PATH_MAX, "%s%s", cachedir, ent->d_name); /* attempt to load the package, skip file on failures as we may have * files here that aren't valid packages. we also don't need a full @@ -167,19 +175,39 @@ static int sync_cleancache(int level) if(alpm_pkg_load(path, 0, &localpkg) != 0 || localpkg == NULL) { continue; } - /* check if this package is in the local DB */ - dbpkg = alpm_db_get_pkg(db_local, alpm_pkg_get_name(localpkg)); - if(dbpkg == NULL) { - /* delete package, not present in local DB */ - unlink(path); - } else if(alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), - alpm_pkg_get_version(dbpkg)) != 0) { - /* delete package, it was found but version differs */ - unlink(path); + if(config->cleanmethod == PM_CLEAN_KEEPCUR) { + /* check if this package is in a sync DB */ + alpm_list_t *sync_dbs = alpm_option_get_syncdbs(); + alpm_list_t *j; + + for(j = sync_dbs; j; j = alpm_list_next(j)) { + pmdb_t *db = alpm_list_getdata(j); + pkg = alpm_db_get_pkg(db, alpm_pkg_get_name(localpkg)); + if(pkg != NULL && alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), + alpm_pkg_get_version(pkg)) == 0) { + /* package was found in a sync DB and version matches, keep it */ + delete = 0; + break; + } + } + } else if(config->cleanmethod == PM_CLEAN_KEEPINST) { + /* check if this package is in the local DB */ + pkg = alpm_db_get_pkg(db_local, alpm_pkg_get_name(localpkg)); + if(pkg != NULL && alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), + alpm_pkg_get_version(pkg)) == 0) { + /* package was found in local DB and version matches, keep it */ + delete = 0; + } + } else { + /* this should not happen : the config parsing doesn't set any other value */ + delete = 0; } - /* else version was the same, so keep the package */ /* free the local file package */ alpm_pkg_free(localpkg); + + if(delete) { + unlink(path); + } } printf(_("done.\n")); } else { -- 1.5.4.2

Dan McGee

12 Mar 12 Mar

2:25 a.m.

New subject: [pacman-dev] [PATCH] Add new CleanMethod option.

On Tue, Mar 11, 2008 at 7:59 AM, Chantry Xavier wrote:

...

As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared.

This option has two possible values : KeepInstalled and KeepCurrent With KeepCurrent, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache.

Ref : http://www.archlinux.org/pipermail/pacman-dev/2008-February/011140.html

Signed-off-by: Chantry Xavier --- doc/pacman.8.txt | 3 ++ doc/pacman.conf.5.txt | 9 +++++++ src/pacman/conf.h | 13 +++++++-- src/pacman/pacman.c | 12 +++++++++ src/pacman/sync.c | 64 +++++++++++++++++++++++++++++++++++------------- 5 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 8235875..7c45ce9 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -244,6 +244,9 @@ Sync Options[[SO]] packages that are no longer installed; use two to remove all packages from the cache. In both cases, you will have a yes or no option to remove packages and/or unused downloaded databases. ++ +If you use a network shared cache, see the 'CleanMethod' option in +linkman:pacman.conf[5].

*-e, \--dependsonly*:: Install all dependencies of a package, but not the specified package diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index e8f7454..8a229f1 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -114,6 +114,15 @@ Options 'index.php', then you would not want the 'index.html' file to be extracted from the 'apache' package.

+*CleanMethod =* KeepInstalled | KeepCurrent:: + If set to `KeepInstalled` (the default), the '-Sc' operation will clean + packages that are no longer installed (not present in the local database). + If set to `KeepCurrent`, '-Sc' will clean outdated packages (not present in + any sync database). + The second behavior is especially useful when the package cache is shared + among multiple machines, where the local databases are usually different, but + the sync databases used could be the same. Small English nag- I would just say "the sync databases in use are the same". Drop "especially" too, its really just a space filler. :)

...

*UseSyslog*:: Log action messages through syslog(). This will insert log entries into ``/var/log/messages'' or equivalent. Off-topic: who actually uses this? Forgot it even existed.

...

diff --git a/src/pacman/conf.h b/src/pacman/conf.h index d53db08..48ddba1 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -66,9 +66,10 @@ typedef struct __config_t { unsigned short chomp; /* I Love Candy! */ unsigned short usecolor; /* enable colorful output */ unsigned short showsize; /* show individual package sizes */ - unsigned short totaldownload; /* When downloading, display the amount - downloaded, rate, ETA, and percent - downloaded of the total download list */ + /* When downloading, display the amount downloaded, rate, ETA, and percent + * downloaded of the total download list */ + unsigned short totaldownload; + unsigned short cleanmethod; /* select -Sc behavior */ } config_t;

/* Operations */ @@ -81,6 +82,12 @@ enum { PM_OP_DEPTEST };

+/* clean method */ +enum { + PM_CLEAN_KEEPINST = 0, /* default */ + PM_CLEAN_KEEPCUR +}; + /* global config variable */ extern config_t *config;

diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index f87db27..80f7d2d 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -699,6 +699,18 @@ static int _parseconfig(const char *file, const char *givensection, } else if (strcmp(key, "XferCommand") == 0 || strcmp(upperkey, "XFERCOMMAND") == 0) { alpm_option_set_xfercommand(ptr); pm_printf(PM_LOG_DEBUG, "config: xfercommand: %s\n", ptr); + } else if (strcmp(key, "CleanMethod") == 0 || strcmp(upperkey, "CLEANMETHOD") == 0) { + char *upperptr = strtoupper(strdup(ptr)); + if (strcmp(ptr, "KeepInstalled") == 0 || strcmp(upperptr, "KEEPINSTALLED") == 0) { + config->cleanmethod = PM_CLEAN_KEEPINST; + } else if (strcmp(ptr, "KeepCurrent") == 0 || strcmp(upperptr, "KEEPCURRENT") == 0) { + config->cleanmethod = PM_CLEAN_KEEPCUR; + } else { + pm_printf(PM_LOG_ERROR, _("invalid value for 'CleanMethod' : '%s'\n"), ptr); + return(1); + } + pm_printf(PM_LOG_DEBUG, "config: cleanmethod: %s\n", ptr); + free(upperptr); I see why wanting to get rid of case-insensitivity would be nice.

...

} else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); diff --git a/src/pacman/sync.c b/src/pacman/sync.c index f2d8f4b..ffa6b61 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -133,13 +133,20 @@ static int sync_cleancache(int level) /* incomplete cleanup */ DIR *dir; struct dirent *ent; - /* Let's vastly improve the way this is done. Before, we went by package - * name. Instead, let's only keep packages we have installed. Open up each - * package and see if it has an entry in the local DB; if not, delete it. - */ + /* Open up each package and see if it has an entry in the local DB; if not, + * delete it. */

You simplified the comment, but still left "local" here. Is that misleading now?

...

printf(_("Cache directory: %s\n"), cachedir); - if(!yesno(1, _("Do you want to remove uninstalled packages from cache?"))) { - return(0); + if(config->cleanmethod == PM_CLEAN_KEEPCUR) { + if(!yesno(1, _("Do you want to remove outdated packages from cache?"))) { + return(0); + } + } else if(config->cleanmethod == PM_CLEAN_KEEPINST) { + if(!yesno(1, _("Do you want to remove uninstalled packages from cache?"))) { + return(0); + } + } else { + /* this should not happen : the config parsing doesn't set any other value */ + return(1);

Good, and I like the unreachable sanity check, as it should prevent any "doh!" moments in the future. However, can you do the if/else if statements in the order of the enum? This will also place the default option first, which seems logical to me. Switch statment instead of if? Not sure, but worth considering. gcc can do a little more static code checking in that case (like see if you are missing values from your enum in your case statements).

...

} printf(_("removing old packages from cache... "));

@@ -153,13 +160,14 @@ static int sync_cleancache(int level) /* step through the directory one file at a time */ while((ent = readdir(dir)) != NULL) { char path[PATH_MAX]; - pmpkg_t *localpkg = NULL, *dbpkg = NULL; + int delete = 1; + pmpkg_t *localpkg = NULL, *pkg = NULL;

if(!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { continue; } /* build the full filepath */ - snprintf(path, PATH_MAX, "%s/%s", cachedir, ent->d_name); + snprintf(path, PATH_MAX, "%s%s", cachedir, ent->d_name);

/* attempt to load the package, skip file on failures as we may have * files here that aren't valid packages. we also don't need a full @@ -167,19 +175,39 @@ static int sync_cleancache(int level) if(alpm_pkg_load(path, 0, &localpkg) != 0 || localpkg == NULL) { continue; } - /* check if this package is in the local DB */ - dbpkg = alpm_db_get_pkg(db_local, alpm_pkg_get_name(localpkg)); - if(dbpkg == NULL) { - /* delete package, not present in local DB */ - unlink(path); - } else if(alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), - alpm_pkg_get_version(dbpkg)) != 0) { - /* delete package, it was found but version differs */ - unlink(path); + if(config->cleanmethod == PM_CLEAN_KEEPCUR) { + /* check if this package is in a sync DB */ + alpm_list_t *sync_dbs = alpm_option_get_syncdbs(); + alpm_list_t *j; + + for(j = sync_dbs; j; j = alpm_list_next(j)) { + pmdb_t *db = alpm_list_getdata(j); + pkg = alpm_db_get_pkg(db, alpm_pkg_get_name(localpkg)); + if(pkg != NULL && alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), + alpm_pkg_get_version(pkg)) == 0) { + /* package was found in a sync DB and version matches, keep it */ + delete = 0; + break; + } + } + } else if(config->cleanmethod == PM_CLEAN_KEEPINST) { + /* check if this package is in the local DB */ + pkg = alpm_db_get_pkg(db_local, alpm_pkg_get_name(localpkg)); + if(pkg != NULL && alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), + alpm_pkg_get_version(pkg)) == 0) { + /* package was found in local DB and version matches, keep it */ + delete = 0; + } + } else { + /* this should not happen : the config parsing doesn't set any other value */ + delete = 0;

Switch order here too please.

...

} - /* else version was the same, so keep the package */ /* free the local file package */ alpm_pkg_free(localpkg); + + if(delete) { + unlink(path); + } } printf(_("done.\n")); } else {

If you address my comments, I'll pull this in, good work. -Dan

Xavier

10:52 a.m.

New subject: [pacman-dev] [PATCH] Add new CleanMethod option.

On Tue, Mar 11, 2008 at 08:25:29PM -0500, Dan McGee wrote:

...

On Tue, Mar 11, 2008 at 7:59 AM, Chantry Xavier wrote:

...
diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index e8f7454..8a229f1 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -114,6 +114,15 @@ Options 'index.php', then you would not want the 'index.html' file to be extracted from the 'apache' package.

+*CleanMethod =* KeepInstalled | KeepCurrent:: + If set to `KeepInstalled` (the default), the '-Sc' operation will clean + packages that are no longer installed (not present in the local database). + If set to `KeepCurrent`, '-Sc' will clean outdated packages (not present in + any sync database). + The second behavior is especially useful when the package cache is shared + among multiple machines, where the local databases are usually different, but + the sync databases used could be the same. Small English nag- I would just say "the sync databases in use are the same". Drop "especially" too, its really just a space filler. :)

Dropped "especially", and s/used/in use/

...

...
*UseSyslog*:: Log action messages through syslog(). This will insert log entries into ``/var/log/messages'' or equivalent. Off-topic: who actually uses this? Forgot it even existed.

Ahah, I didnt know it existed at all until I saw pacman.conf Nagy's patch that added options. I don't really see the point either :)

...

...
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index f87db27..80f7d2d 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -699,6 +699,18 @@ static int _parseconfig(const char *file, const char *givensection, } else if (strcmp(key, "XferCommand") == 0 || strcmp(upperkey, "XFERCOMMAND") == 0) { alpm_option_set_xfercommand(ptr); pm_printf(PM_LOG_DEBUG, "config: xfercommand: %s\n", ptr); + } else if (strcmp(key, "CleanMethod") == 0 || strcmp(upperkey, "CLEANMETHOD") == 0) { + char *upperptr = strtoupper(strdup(ptr)); + if (strcmp(ptr, "KeepInstalled") == 0 || strcmp(upperptr, "KEEPINSTALLED") == 0) { + config->cleanmethod = PM_CLEAN_KEEPINST; + } else if (strcmp(ptr, "KeepCurrent") == 0 || strcmp(upperptr, "KEEPCURRENT") == 0) { + config->cleanmethod = PM_CLEAN_KEEPCUR; + } else { + pm_printf(PM_LOG_ERROR, _("invalid value for 'CleanMethod' : '%s'\n"), ptr); + return(1); + } + pm_printf(PM_LOG_DEBUG, "config: cleanmethod: %s\n", ptr); + free(upperptr); I see why wanting to get rid of case-insensitivity would be nice.

...

yay..

...

...
} else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); diff --git a/src/pacman/sync.c b/src/pacman/sync.c index f2d8f4b..ffa6b61 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -133,13 +133,20 @@ static int sync_cleancache(int level) /* incomplete cleanup */ DIR *dir; struct dirent *ent; - /* Let's vastly improve the way this is done. Before, we went by package - * name. Instead, let's only keep packages we have installed. Open up each - * package and see if it has an entry in the local DB; if not, delete it. - */ + /* Open up each package and see if it has an entry in the local DB; if not, + * delete it. */

You simplified the comment, but still left "local" here. Is that misleading now?

Uh yes, it is, I will rephrase that.

...

...
printf(_("Cache directory: %s\n"), cachedir); - if(!yesno(1, _("Do you want to remove uninstalled packages from cache?"))) { - return(0); + if(config->cleanmethod == PM_CLEAN_KEEPCUR) { + if(!yesno(1, _("Do you want to remove outdated packages from cache?"))) { + return(0); + } + } else if(config->cleanmethod == PM_CLEAN_KEEPINST) { + if(!yesno(1, _("Do you want to remove uninstalled packages from cache?"))) { + return(0); + } + } else { + /* this should not happen : the config parsing doesn't set any other value */ + return(1);

Good, and I like the unreachable sanity check, as it should prevent any "doh!" moments in the future. However, can you do the if/else if statements in the order of the enum? This will also place the default option first, which seems logical to me. Switch statment instead of if? Not sure, but worth considering. gcc can do a little more static code checking in that case (like see if you are missing values from your enum in your case statements).

Yes, keeping the same order makes sense, and using switch looks nicer too, I will do it in both places.

Chantry Xavier

10:56 a.m.

New subject: [pacman-dev] [PATCH] Add new CleanMethod option.

As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared. This option has two possible values : KeepInstalled and KeepCurrent With KeepCurrent, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in the local db. The resulting behavior should be better for shared cache. Ref : http://www.archlinux.org/pipermail/pacman-dev/2008-February/011140.html Signed-off-by: Chantry Xavier --- doc/pacman.8.txt | 3 ++ doc/pacman.conf.5.txt | 9 ++++++ src/pacman/conf.h | 13 +++++++-- src/pacman/pacman.c | 12 ++++++++ src/pacman/sync.c | 69 ++++++++++++++++++++++++++++++++++++------------- 5 files changed, 85 insertions(+), 21 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index bbb25c6..2f3e118 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -244,6 +244,9 @@ Sync Options[[SO]] packages that are no longer installed; use two to remove all packages from the cache. In both cases, you will have a yes or no option to remove packages and/or unused downloaded databases. ++ +If you use a network shared cache, see the 'CleanMethod' option in +linkman:pacman.conf[5]. *-e, \--dependsonly*:: Install all dependencies of a package, but not the specified package diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index e8f7454..484e2c7 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -114,6 +114,15 @@ Options 'index.php', then you would not want the 'index.html' file to be extracted from the 'apache' package. +*CleanMethod =* KeepInstalled | KeepCurrent:: + If set to `KeepInstalled` (the default), the '-Sc' operation will clean + packages that are no longer installed (not present in the local database). + If set to `KeepCurrent`, '-Sc' will clean outdated packages (not present in + any sync database). + The second behavior is useful when the package cache is shared among + multiple machines, where the local databases are usually different, but the + sync databases in use could be the same. + *UseSyslog*:: Log action messages through syslog(). This will insert log entries into ``/var/log/messages'' or equivalent. diff --git a/src/pacman/conf.h b/src/pacman/conf.h index d53db08..48ddba1 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -66,9 +66,10 @@ typedef struct __config_t { unsigned short chomp; /* I Love Candy! */ unsigned short usecolor; /* enable colorful output */ unsigned short showsize; /* show individual package sizes */ - unsigned short totaldownload; /* When downloading, display the amount - downloaded, rate, ETA, and percent - downloaded of the total download list */ + /* When downloading, display the amount downloaded, rate, ETA, and percent + * downloaded of the total download list */ + unsigned short totaldownload; + unsigned short cleanmethod; /* select -Sc behavior */ } config_t; /* Operations */ @@ -81,6 +82,12 @@ enum { PM_OP_DEPTEST }; +/* clean method */ +enum { + PM_CLEAN_KEEPINST = 0, /* default */ + PM_CLEAN_KEEPCUR +}; + /* global config variable */ extern config_t *config; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 579474c..f05a82c 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -704,6 +704,18 @@ static int _parseconfig(const char *file, const char *givensection, } else if (strcmp(key, "XferCommand") == 0 || strcmp(upperkey, "XFERCOMMAND") == 0) { alpm_option_set_xfercommand(ptr); pm_printf(PM_LOG_DEBUG, "config: xfercommand: %s\n", ptr); + } else if (strcmp(key, "CleanMethod") == 0 || strcmp(upperkey, "CLEANMETHOD") == 0) { + char *upperptr = strtoupper(strdup(ptr)); + if (strcmp(ptr, "KeepInstalled") == 0 || strcmp(upperptr, "KEEPINSTALLED") == 0) { + config->cleanmethod = PM_CLEAN_KEEPINST; + } else if (strcmp(ptr, "KeepCurrent") == 0 || strcmp(upperptr, "KEEPCURRENT") == 0) { + config->cleanmethod = PM_CLEAN_KEEPCUR; + } else { + pm_printf(PM_LOG_ERROR, _("invalid value for 'CleanMethod' : '%s'\n"), ptr); + return(1); + } + pm_printf(PM_LOG_DEBUG, "config: cleanmethod: %s\n", ptr); + free(upperptr); } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' not recognized.\n"), file, linenum, key); diff --git a/src/pacman/sync.c b/src/pacman/sync.c index f2d8f4b..460933a 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -133,13 +133,23 @@ static int sync_cleancache(int level) /* incomplete cleanup */ DIR *dir; struct dirent *ent; - /* Let's vastly improve the way this is done. Before, we went by package - * name. Instead, let's only keep packages we have installed. Open up each - * package and see if it has an entry in the local DB; if not, delete it. - */ + /* Open up each package and see if it should be deleted, + * depending on the clean method used */ printf(_("Cache directory: %s\n"), cachedir); - if(!yesno(1, _("Do you want to remove uninstalled packages from cache?"))) { - return(0); + switch(config->cleanmethod) { + case PM_CLEAN_KEEPINST: + if(!yesno(1, _("Do you want to remove uninstalled packages from cache?"))) { + return(0); + } + break; + case PM_CLEAN_KEEPCUR: + if(!yesno(1, _("Do you want to remove outdated packages from cache?"))) { + return(0); + } + break; + default: + /* this should not happen : the config parsing doesn't set any other value */ + return(1); } printf(_("removing old packages from cache... ")); @@ -153,13 +163,16 @@ static int sync_cleancache(int level) /* step through the directory one file at a time */ while((ent = readdir(dir)) != NULL) { char path[PATH_MAX]; - pmpkg_t *localpkg = NULL, *dbpkg = NULL; + int delete = 1; + pmpkg_t *localpkg = NULL, *pkg = NULL; + alpm_list_t *sync_dbs = alpm_option_get_syncdbs(); + alpm_list_t *j; if(!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { continue; } /* build the full filepath */ - snprintf(path, PATH_MAX, "%s/%s", cachedir, ent->d_name); + snprintf(path, PATH_MAX, "%s%s", cachedir, ent->d_name); /* attempt to load the package, skip file on failures as we may have * files here that aren't valid packages. we also don't need a full @@ -167,19 +180,39 @@ static int sync_cleancache(int level) if(alpm_pkg_load(path, 0, &localpkg) != 0 || localpkg == NULL) { continue; } - /* check if this package is in the local DB */ - dbpkg = alpm_db_get_pkg(db_local, alpm_pkg_get_name(localpkg)); - if(dbpkg == NULL) { - /* delete package, not present in local DB */ - unlink(path); - } else if(alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), - alpm_pkg_get_version(dbpkg)) != 0) { - /* delete package, it was found but version differs */ - unlink(path); + switch(config->cleanmethod) { + case PM_CLEAN_KEEPINST: + /* check if this package is in the local DB */ + pkg = alpm_db_get_pkg(db_local, alpm_pkg_get_name(localpkg)); + if(pkg != NULL && alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), + alpm_pkg_get_version(pkg)) == 0) { + /* package was found in local DB and version matches, keep it */ + delete = 0; + } + break; + case PM_CLEAN_KEEPCUR: + /* check if this package is in a sync DB */ + for(j = sync_dbs; j && delete; j = alpm_list_next(j)) { + pmdb_t *db = alpm_list_getdata(j); + pkg = alpm_db_get_pkg(db, alpm_pkg_get_name(localpkg)); + if(pkg != NULL && alpm_pkg_vercmp(alpm_pkg_get_version(localpkg), + alpm_pkg_get_version(pkg)) == 0) { + /* package was found in a sync DB and version matches, keep it */ + delete = 0; + } + } + break; + default: + /* this should not happen : the config parsing doesn't set any other value */ + delete = 0; + break; } - /* else version was the same, so keep the package */ /* free the local file package */ alpm_pkg_free(localpkg); + + if(delete) { + unlink(path); + } } printf(_("done.\n")); } else { -- 1.5.4.2

Nagy Gabor

19 Feb 19 Feb

4:59 p.m.

New subject: [pacman-dev] [PATCH] Add new SharedPkgCache option.

...

...
2008/2/17, Xavier :

...
On Sun, Feb 17, 2008 at 05:37:13PM +0200, Roman Kyrylych wrote:

...
2008/2/16, Chantry Xavier :

...
As it was already mentioned several times, the new -Sc behavior in 3.1 is great, but only when the package cache is not shared.

When this option is enabled, -Sc will clean packages that are no longer available in any sync db, rather than packages that are no longer in

...
...
...
...
local db. The resulting behavior should be better for shared cache.

Great name! :-)

BTW, I've just realized that there might be a problem with "no longer in any sync db" that, I think was not in 3.0's -Sc: if SystemA has core,extra,community enabled and SystemB only core&extra then doing -Sc on SystemB will cleanup community packages from the shared cache. I don't know if it's worth to fix this by using a more complicated (filename comparison, AFAIR) aproach like in 3.0. :-/

Indeed, that's a problem, but don't know if it's a big one. I would

On Feb 17, 2008 10:14 AM, Roman Kyrylych wrote: the think

...
...
it's rather common to use the same set of sync db on a set of machines sharing pkg cache. And that approach is indeed much simpler.

I mentioned this in the man page, that this option is rather meant for setup where same sync dbs are used.

Ah, OK, didn't read that. :-) Then we can forget about this corner case, it can be worked around by cache cleanup scripts based on filename parsing (available on BBS and ML).

And now we are back to the whole reason why I think this is a pointless problem to solve. :P

Once you venture into the shared cache realm, I just don't see a need for pacman to hand-hold you.

So we have Xav and Roman in support of this, and me against it. I don't want to say "majority rules" here, but I would like to get a few more voices on this one. Aaron?

-Dan

Well, actually I like SharedPkgCache (== old behaviour) better. Caching installed packages is not interesting to me, because I can use re-pacman if I need that. [And if I downloaded newer but not yet installed package, usually I don't want to remove that] Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/

5900

Age (days ago)

5927

Last active (days ago)

List overview

Download

19 comments

6 participants

participants (6)

Chantry Xavier
Dan McGee
Mark Constable
Nagy Gabor
Roman Kyrylych
Xavier