On Tue, Mar 11, 2008 at 08:25:29PM -0500, Dan McGee wrote:
On Tue, Mar 11, 2008 at 7:59 AM, Chantry Xavier <shiningxc@gmail.com> 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.