[pacman-dev] [PATCH] New -Ru option
Hi! The most difficult thing was: to find out good names for the option and the newly introduced stuffs. So feel free to rename them. And if you like and apply the patch, please update pacman's manual and bash-completion. Note: If user wants to remove pkg1 and pkg2 with the same "prov" provision, then only one of them should be removed from the list. That's why inducer is not checked by depmiss_isin. But if there is many multiple provision in the original target list, human may be able to compute fewer package to remove from the target list, but I don't care (pathological case):-P Enjoy it, ngaba ---------------------- From d8a7ab05ba50197386b25e7fd879f342384370d4 Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Fri, 26 Oct 2007 18:33:20 +0200 Subject: [PATCH] New -Ru option With --unneeded option 'pacman -R' doesn't stop in case of dependency error; it removes the needed-dependency targets from the target-list instead. See also: http://archlinux.org/pipermail/pacman-dev/2007-October/009653.html The patch also adds a new inducer field to pmdepmissing_t which indicates the to-be-removed package which would cause a dependency break. This is needed, because miss->depend.name may be a provision. miss->depend will be useful in -R dependency error messages too. A new _alpm_pkgname_pkg_cmp(pkgname, pkg) function is also added, which retruns true iff pkg's name is pkgname. This is useful if you want to remove a package from pmpkg_t* list, and you want to search for package name. remove049.py tests -Ru. --- lib/libalpm/alpm.h | 4 +++- lib/libalpm/conflict.c | 2 +- lib/libalpm/deps.c | 41 ++++++++++++++++++++++++++++------------- lib/libalpm/deps.h | 3 ++- lib/libalpm/package.c | 5 +++++ lib/libalpm/package.h | 1 + lib/libalpm/remove.c | 15 +++++++++++++++ src/pacman/pacman.c | 3 +++ 8 files changed, 58 insertions(+), 16 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 2000db4..99dcd24 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -275,7 +275,8 @@ typedef enum _pmtransflag_t { PM_TRANS_FLAG_DOWNLOADONLY = 0x200, PM_TRANS_FLAG_NOSCRIPTLET = 0x400, PM_TRANS_FLAG_NOCONFLICTS = 0x800, - PM_TRANS_FLAG_PRINTURIS = 0x1000 + PM_TRANS_FLAG_PRINTURIS = 0x1000, + PM_TRANS_FLAG_UNNEEDED = 0x2000, } pmtransflag_t; /* Transaction Events */ @@ -375,6 +376,7 @@ alpm_list_t *alpm_checkdeps(pmdb_t *db, pmtranstype_t op, const char *alpm_miss_get_target(pmdepmissing_t *miss); pmdeptype_t alpm_miss_get_type(pmdepmissing_t *miss); pmdepend_t *alpm_miss_get_dep(pmdepmissing_t *miss); +const char *alpm_miss_get_inducer(pmdepmissing_t *miss); pmdepmod_t alpm_dep_get_mod(pmdepend_t *dep); const char *alpm_dep_get_name(pmdepend_t *dep); diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index d09c996..bf8aa80 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -76,7 +76,7 @@ static void add_conflict(alpm_list_t **baddeps, const char *pkg1, const char *pkg2) { pmdepmissing_t *miss = _alpm_depmiss_new(pkg1, PM_DEP_TYPE_CONFLICT, - PM_DEP_MOD_ANY, pkg2, NULL); + PM_DEP_MOD_ANY, pkg2, NULL, ""); if(miss && !_alpm_depmiss_isin(miss, *baddeps)) { *baddeps = alpm_list_add(*baddeps, miss); } else { diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 7f4fb0b..f8109d8 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -65,7 +65,7 @@ static void _alpm_graph_free(void *data) pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, pmdepmod_t depmod, const char *depname, - const char *depversion) + const char *depversion, const char *inducer) { pmdepmissing_t *miss; @@ -86,6 +86,7 @@ pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, } else { miss->depend.version[0] = 0; } + strncpy(miss->inducer, inducer, PKG_NAME_LEN); return(miss); } @@ -333,7 +335,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, _alpm_log(PM_LOG_DEBUG, "checkdeps: updated '%s' won't satisfy a dependency of '%s'\n", alpm_pkg_get_name(oldpkg), alpm_pkg_get_name(p)); miss = _alpm_depmiss_new(p->name, PM_DEP_TYPE_DEPEND, depend->mod, - depend->name, depend->version); + depend->name, depend->version, ""); if(!_alpm_depmiss_isin(miss, baddeps)) { baddeps = alpm_list_add(baddeps, miss); } else { @@ -382,7 +384,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, _alpm_log(PM_LOG_DEBUG, "missing dependency '%s' for package '%s'\n", depend->name, alpm_pkg_get_name(tp)); miss = _alpm_depmiss_new(alpm_pkg_get_name(tp), PM_DEP_TYPE_DEPEND, depend->mod, - depend->name, depend->version); + depend->name, depend->version, ""); if(!_alpm_depmiss_isin(miss, baddeps)) { baddeps = alpm_list_add(baddeps, miss); } else { @@ -437,7 +439,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, alpm_pkg_get_name(p), alpm_pkg_get_name(rmpkg)); miss = _alpm_depmiss_new(alpm_pkg_get_name(p), PM_DEP_TYPE_DEPEND, depend->mod, depend->name, - depend->version); + depend->version, alpm_pkg_get_name(rmpkg)); if(!_alpm_depmiss_isin(miss, baddeps)) { baddeps = alpm_list_add(baddeps, miss); } else { @@ -772,7 +774,19 @@ error: return(-1); } -const char SYMEXPORT *alpm_miss_get_target(pmdepmissing_t *miss) + +pmdeptype_t SYMEXPORT alpm_miss_get_type(pmdepmissing_t *miss) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(-1)); + ASSERT(miss != NULL, return(-1)); + + return miss->type; +} + +pmdepend_t SYMEXPORT *alpm_miss_get_dep(pmdepmissing_t *miss) { ALPM_LOG_FUNC; @@ -780,21 +794,21 @@ const char SYMEXPORT *alpm_miss_get_target(pmdepmissing_t *miss) ASSERT(handle != NULL, return(NULL)); ASSERT(miss != NULL, return(NULL)); - return miss->target; + return &miss->depend; } -pmdeptype_t SYMEXPORT alpm_miss_get_type(pmdepmissing_t *miss) +const char SYMEXPORT *alpm_miss_get_inducer(pmdepmissing_t *miss) { ALPM_LOG_FUNC; /* Sanity checks */ - ASSERT(handle != NULL, return(-1)); - ASSERT(miss != NULL, return(-1)); + ASSERT(handle != NULL, return(NULL)); + ASSERT(miss != NULL, return(NULL)); - return miss->type; + return miss->inducer; } -pmdepend_t SYMEXPORT *alpm_miss_get_dep(pmdepmissing_t *miss) +const char SYMEXPORT *alpm_miss_get_target(pmdepmissing_t *miss) { ALPM_LOG_FUNC; @@ -802,9 +816,10 @@ pmdepend_t SYMEXPORT *alpm_miss_get_dep(pmdepmissing_t *miss) ASSERT(handle != NULL, return(NULL)); ASSERT(miss != NULL, return(NULL)); - return &miss->depend; + return miss->target; } + pmdepmod_t SYMEXPORT alpm_dep_get_mod(pmdepend_t *dep) { ALPM_LOG_FUNC; diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 59b2630..6f01593 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -40,6 +40,7 @@ struct __pmdepmissing_t { char target[PKG_NAME_LEN]; pmdeptype_t type; pmdepend_t depend; + char inducer[PKG_NAME_LEN]; /* this is used in case of remove dependency error only */ }; /* Graphs */ @@ -53,7 +54,7 @@ struct __pmgraph_t { pmdepmissing_t *_alpm_depmiss_new(const char *target, pmdeptype_t type, pmdepmod_t depmod, const char *depname, - const char *depversion); + const char *depversion, const char *inducer); int _alpm_depmiss_isin(pmdepmissing_t *needle, alpm_list_t *haystack); alpm_list_t *_alpm_sortbydeps(alpm_list_t *targets, pmtranstype_t mode); alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 4f6f5a9..00e8332 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -803,6 +803,11 @@ int _alpm_pkg_cmp(const void *p1, const void *p2) return(strcmp(alpm_pkg_get_name(pk1), alpm_pkg_get_name(pk2))); } +int _alpm_pkgname_pkg_cmp(const void *pkgname, const void *package) +{ + return(strcmp(alpm_pkg_get_name((pmpkg_t *) package), (char *) pkgname)); +} + /* Parses the package description file for the current package * TODO: this should ALL be in a backend interface (be_files), we should * be dealing with the abstracted concepts only in this file diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h index daaeb36..d7032e6 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -94,6 +94,7 @@ pmpkg_t* _alpm_pkg_new(const char *name, const char *version); pmpkg_t *_alpm_pkg_dup(pmpkg_t *pkg); void _alpm_pkg_free(pmpkg_t *pkg); int _alpm_pkg_cmp(const void *p1, const void *p2); +int _alpm_pkgname_pkg_cmp(const void *pkgname, const void *package); int alpm_pkg_compare_versions(pmpkg_t *local_pkg, pmpkg_t *pkg); pmpkg_t *_alpm_pkg_load(const char *pkgfile, unsigned short full); pmpkg_t *_alpm_pkg_find(const char *needle, alpm_list_t *haystack); diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 0ed4dd0..3904a36 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -126,6 +126,21 @@ int _alpm_remove_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data) FREELIST(lp); lp = _alpm_checkdeps(db, trans->type, trans->packages); } + } else if (trans->flags & PM_TRANS_FLAG_UNNEEDED) { + while(lp != NULL) { + alpm_list_t *i; + for(i = lp; i; i = i->next) { + pmdepmissing_t *miss = (pmdepmissing_t *)i->data; + pmpkg_t *unneeded; + trans->packages = alpm_list_remove(trans->packages, miss->inducer, _alpm_pkgname_pkg_cmp, &unneeded); + if(unneeded) { + _alpm_log(PM_LOG_WARNING, "removing %s from the target-list\n", alpm_pkg_get_name(unneeded)); + alpm_pkg_free(unneeded); + } + } + FREELIST(lp); + lp = _alpm_checkdeps(db, trans->type, trans->packages); + } } else { if(data) { *data = lp; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 705edaf..199878f 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -95,6 +95,7 @@ static void usage(int op, char *myname) printf(_(" -k, --dbonly only remove database entry, do not remove files\n")); printf(_(" -n, --nosave remove configuration files as well\n")); printf(_(" -s, --recursive remove dependencies also (that won't break packages)\n")); + printf(_(" -u, --unneeded remove only unneeded packages (that won't break packages)\n")); } else if(op == PM_OP_UPGRADE) { if(config->flags & PM_TRANS_FLAG_FRESHEN) { printf("%s: %s {-F --freshen} [%s] <%s>\n", str_usg, myname, str_opt, str_file); @@ -277,6 +278,7 @@ static int parseargs(int argc, char *argv[]) {"orphans", no_argument, 0, 't'}, {"upgrades", no_argument, 0, 'u'}, {"sysupgrade", no_argument, 0, 'u'}, + {"unneeded", no_argument, 0, 'u'}, {"verbose", no_argument, 0, 'v'}, {"downloadonly", no_argument, 0, 'w'}, {"refresh", no_argument, 0, 'y'}, @@ -413,6 +415,7 @@ static int parseargs(int argc, char *argv[]) case 'u': config->op_s_upgrade = 1; config->op_q_upgrade = 1; + config->flags |= PM_TRANS_FLAG_UNNEEDED; break; case 'v': (config->verbose)++; break; case 'w': diff --git a/pactest/tests/remove049.py b/pactest/tests/remove049.py new file mode 100644 index 0000000..724f8da --- /dev/null +++ b/pactest/tests/remove049.py @@ -0,0 +1,19 @@ +self.description = "-Ru test" + +lp1 = pmpkg("pkg1") +lp1.requiredby = [ "pkg3" ] +self.addpkg2db("local", lp1) + +lp2 = pmpkg("pkg2") +self.addpkg2db("local", lp2) + +lp3 = pmpkg("pkg3") +lp3.depends = [ "pkg1" ] +self.addpkg2db("local", lp3) + +self.args = "-Ru pkg1 pkg2" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PKG_EXIST=pkg1") +self.addrule("!PKG_EXIST=pkg2") +self.addrule("PKG_EXIST=pkg3") \ No newline at end of file -- 1.5.3.4
+ printf(_(" -u, --unneeded remove only unneeded packages (that won't break packages)\n")); Hm. This description may not be correct ("remove unneeded packages only" sounds better for me). But I won't fix it now, because I want to motivate you to fix these "Hunglish" problems (I'm not sure on "target-list" neither...). You have probably realized already, that my English is far from perfect ;-), so I leave these works for you, sorry. Bye, ngaba
On 10/26/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
+ printf(_(" -u, --unneeded remove only unneeded packages (that won't break packages)\n")); Hm. This description may not be correct ("remove unneeded packages only" sounds better for me). But I won't fix it now, because I want to motivate you to fix these "Hunglish" problems (I'm not sure on "target-list" neither...). You have probably realized already, that my English is far from perfect ;-), so I leave these works for you, sorry.
Hah. "Hunglish" Just for the record... both sound a little "forced". I think it's actually the word "only" that is making it feel funny (to me). "remove unneeded packages" sounds better, but that's just my opinion. Regarding the patch... a couple of comments:
--- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -40,6 +40,7 @@ struct __pmdepmissing_t { char target[PKG_NAME_LEN]; pmdeptype_t type; pmdepend_t depend; + char inducer[PKG_NAME_LEN]; /* this is used in case of remove dependency error only */
Hrm. I don't know if I like the name "inducer" - could you explain what you meant with this so we could maybe use a clearer term?
+int _alpm_pkgname_pkg_cmp(const void *pkgname, const void *package)
I don't know if I'm a fan of this function here. Seems a bit excessive, BUT if it does have a lot of usages, could you submit this a separate (small) patch, just so we can push it in there? Other than that, it all looks sound. And I do like the -Ru feature, myself.
On 10/26/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
+ printf(_(" -u, --unneeded remove only unneeded packages (that won't break packages)\n")); Hm. This description may not be correct ("remove unneeded packages only" sounds better for me). But I won't fix it now, because I want to motivate you to fix these "Hunglish" problems (I'm not sure on "target-list" neither...). You have probably realized already, that my English is far from perfect ;-), so I leave these works for you, sorry.
Hah. "Hunglish"
Just for the record... both sound a little "forced". I think it's actually the word "only" that is making it feel funny (to me). "remove unneeded packages" sounds better, but that's just my opinion.
Regarding the patch... a couple of comments:
--- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -40,6 +40,7 @@ struct __pmdepmissing_t { char target[PKG_NAME_LEN]; pmdeptype_t type; pmdepend_t depend; + char inducer[PKG_NAME_LEN]; /* this is used in case of remove dependency error only */
Hrm. I don't know if I like the name "inducer" - could you explain what you meant with this so we could maybe use a clearer term?
OK. First of all, this is useful with "pacman -R" only.(*) If you do a "pacman -R foo", checkdeps will collect the broken dependencies (after the theoretical commit), for it will return with "bar" requires "prov" ("foo" provides "prov" here, and there is no other installed "prov"-provider). So 1. user get a not really informative error message; that's why I wrote in patch description: "miss->depend (EDIT: typo, I meant miss->inducer) will be useful in -R dependency error messages too." 2. -Ru needs this information too to figure out the "needed" packages. Of course we can live without inducer field (which indicates the "dependency-broker" package): we just checking the target list again; but that is simply needless, and using alpm_depcmp and such things in pacman/remove.c is not nice imho. Note: I'm not sure on (*), this can be useful with other commands, too: I hate these type of error messages: "pacman -S foo" ERROR: bar requires baz=1.0 (This can happen in the following case: resolvedeps pulls baz, since new version is needed for foo; but updating baz 1.0 to 1.1 would break the "baz=1.0" depend of bar <- resolvedeps doesn't resolve this.)
+int _alpm_pkgname_pkg_cmp(const void *pkgname, const void *package)
I don't know if I'm a fan of this function here. Seems a bit excessive, BUT if it does have a lot of usages, could you submit this a separate (small) patch, just so we can push it in there?
I can repeat myself: we can live without this. But IMHO this is nicer than get pkg from the list by name, then alpm_list_remove... or create a pseudo package with only pkgname filled in (we do the "same" thing twice). This can be used in every place where "removing package from target list" is needed (conflict resolving, for example). Or an alternative function: get pkg's alpm_list_node from the list by its name, and this can be removed by alpm_list_remove_node. But IMHO this is uglier.
Other than that, it all looks sound. And I do like the -Ru feature, myself.
---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
On 10/30/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
On 10/26/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
+ printf(_(" -u, --unneeded remove only unneeded packages (that won't break packages)\n")); Hm. This description may not be correct ("remove unneeded packages only" sounds better for me). But I won't fix it now, because I want to motivate you to fix these "Hunglish" problems (I'm not sure on "target-list" neither...). You have probably realized already, that my English is far from perfect ;-), so I leave these works for you, sorry.
Hah. "Hunglish"
Just for the record... both sound a little "forced". I think it's actually the word "only" that is making it feel funny (to me). "remove unneeded packages" sounds better, but that's just my opinion.
Regarding the patch... a couple of comments:
--- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -40,6 +40,7 @@ struct __pmdepmissing_t { char target[PKG_NAME_LEN]; pmdeptype_t type; pmdepend_t depend; + char inducer[PKG_NAME_LEN]; /* this is used in case of remove dependency error only */
Hrm. I don't know if I like the name "inducer" - could you explain what you meant with this so we could maybe use a clearer term?
OK. First of all, this is useful with "pacman -R" only.(*) If you do a "pacman -R foo", checkdeps will collect the broken dependencies (after the theoretical commit), for it will return with "bar" requires "prov" ("foo" provides "prov" here, and there is no other installed "prov"-provider). So 1. user get a not really informative error message; that's why I wrote in patch description: "miss->depend (EDIT: typo, I meant miss->inducer) will be useful in -R dependency error messages too." 2. -Ru needs this information too to figure out the "needed" packages. Of course we can live without inducer field (which indicates the "dependency-broker" package): we just checking the target list again; but that is simply needless, and using alpm_depcmp and such things in pacman/remove.c is not nice imho. Note: I'm not sure on (*), this can be useful with other commands, too: I hate these type of error messages: "pacman -S foo" ERROR: bar requires baz=1.0 (This can happen in the following case: resolvedeps pulls baz, since new version is needed for foo; but updating baz 1.0 to 1.1 would break the "baz=1.0" depend of bar <- resolvedeps doesn't resolve this.)
I'm actually just commenting on the variable name itself. Not the code. The variable name is unclear. I didn't really need all the rationalization behind it. I just think that seeing "dep->inducer" gives me no information because the variable name is confusing. Could you change this to be a little more clear and resubmit?
+int _alpm_pkgname_pkg_cmp(const void *pkgname, const void I can repeat myself: we can live without this. But IMHO this is nicer than get
Ok, in this case, could you resubmit without this function, to make things cleaner. We can add this later as another patch. (You might want to take a look at "git commit --amend"
I'm actually just commenting on the variable name itself. Not the code. The variable name is unclear. I didn't really need all the rationalization behind it.
Then you should suggest a better name; inducer was informative to me (Hunglish-man ;-). miss->reasonpkg will be OK? Bye, ngaba ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
On 10/31/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
I'm actually just commenting on the variable name itself. Not the code. The variable name is unclear. I didn't really need all the rationalization behind it.
Then you should suggest a better name; inducer was informative to me (Hunglish-man ;-). miss->reasonpkg will be OK? Bye, ngaba
Aaron- I guess I'm with Nagy here in that I don't know a much better name than "inducer" for what he is trying to do. Obviously comments in the code clarifying exactly what this means would be great, but can you think of a better variable name for "package that caused something to be removed"? -Dan
On Wed, Oct 31, 2007 at 08:39:05AM -0500, Dan McGee <dpmcgee@gmail.com> wrote:
Aaron- I guess I'm with Nagy here in that I don't know a much better name than "inducer" for what he is trying to do. Obviously comments in the code clarifying exactly what this means would be great, but can you think of a better variable name for "package that caused something to be removed"?
afaik inducer is not an English word nor an abbreviation. "indicator" of course still does not express the "package that caused something to be removed" but at least is an English word ;) - VMiklos
On 10/31/07, Miklos Vajna <vmiklos@frugalware.org> wrote:
On Wed, Oct 31, 2007 at 08:39:05AM -0500, Dan McGee <dpmcgee@gmail.com> wrote:
Aaron- I guess I'm with Nagy here in that I don't know a much better name than "inducer" for what he is trying to do. Obviously comments in the code clarifying exactly what this means would be great, but can you think of a better variable name for "package that caused something to be removed"?
afaik inducer is not an English word nor an abbreviation. "indicator" of course still does not express the "package that caused something to be removed" but at least is an English word ;)
- VMiklos
Woah, wait a second... did you just side with me? History in the making! 8) Let me explain where I'm coming from here. It's me being picky, sure, but I really feel that you should be able to "get" the idea of something by saying it out loud. miss->inducer => "the pmdemiss_t miss's inducer" doesn't mean much miss->causingpkg => "the pmdemiss_t miss's causing package" is clearer, but the variable name sucks removalpkg? parentpkg? I dunno. I just feel like saying the line outloud should have some sort of meaning. But then again, I am just being picky here, so, Dan, you're welcome to veto me here.
On 10/31/07, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On 10/31/07, Miklos Vajna <vmiklos@frugalware.org> wrote:
On Wed, Oct 31, 2007 at 08:39:05AM -0500, Dan McGee <dpmcgee@gmail.com> wrote:
Aaron- I guess I'm with Nagy here in that I don't know a much better name than "inducer" for what he is trying to do. Obviously comments in the code clarifying exactly what this means would be great, but can you think of a better variable name for "package that caused something to be removed"?
afaik inducer is not an English word nor an abbreviation. "indicator" of course still does not express the "package that caused something to be removed" but at least is an English word ;)
- VMiklos
Woah, wait a second... did you just side with me? History in the making! 8)
Let me explain where I'm coming from here. It's me being picky, sure, but I really feel that you should be able to "get" the idea of something by saying it out loud.
miss->inducer => "the pmdemiss_t miss's inducer" doesn't mean much miss->causingpkg => "the pmdemiss_t miss's causing package" is clearer, but the variable name sucks
removalpkg? parentpkg? I dunno. I just feel like saying the line outloud should have some sort of meaning. But then again, I am just being picky here, so, Dan, you're welcome to veto me here.
causingpkg actually sounds quite clear to me. Works for me! -Dan
causingpkg actually sounds quite clear to me. Works for me! I prefer bikeshed ;-P I will resend the patch with your preferred name on Monday, and split it as many parts as possible (I cut off alpm_pkgname_pkg_cmp and miss->causingpkg) to make you happy (and me unhappy) Bye, ngaba
---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
causingpkg actually sounds quite clear to me. Works for me! I prefer bikeshed ;-P I will resend the patch with your preferred name on Monday, and split it as many parts as possible (I cut off alpm_pkgname_pkg_cmp and miss->causingpkg) to make you happy (and me unhappy) Bye, ngaba
Well. I've changed my mind. First please fix this, because my patch depends on it: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009711.html I won't fix it, because I didn't get any feedback from the leaders (remove_all vs remove_first) so I simply let them fix; but alpm_list_remove is a ... now. (OK. I'm not saying anything) Bye, ngaba ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
On 11/5/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
causingpkg actually sounds quite clear to me. Works for me! I prefer bikeshed ;-P I will resend the patch with your preferred name on Monday, and split it as many parts as possible (I cut off alpm_pkgname_pkg_cmp and miss->causingpkg) to make you happy (and me unhappy) Bye, ngaba
Well. I've changed my mind. First please fix this, because my patch depends on it: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009711.html I won't fix it, because I didn't get any feedback from the leaders (remove_all vs remove_first) so I simply let them fix; but alpm_list_remove is a ... now. (OK. I'm not saying anything)
Counter-productivity is counterproductive! Yay!
After you read the subject, you can ignore this mail ;-) Mostly personal things and feelings about pacman and devels will follow: Nostalgie: 1. First of all, I enjoy hacking pacman. 2. When I first wanted to join to the development, I saw that pacman was dying: pointless flames can be read on the ML between archlinux and vmiklos and no real development... so I dropped the idea soon. 3. Then I wanted to speed-up pacman (IMHO I arrived here with this mail: http://www.archlinux.org/pipermail/pacman-dev/2007-February/007202.html); I realized that the main limiting factor is ldconfig, which I cannot do too much with. 4. When I read the code, I found _many_ bugs, I reported them here. I wasn't satisfied with deps.c at all (which is IMHO one of the most important things in a package manager). They irritated me so much, that I fixed them for _myself_. I also sent the patches here, to "support" the community (the patch-flow started here: http://www.archlinux.org/pipermail/pacman-dev/2007-April/008127.html, and probably you can remember my funny IV/C naming ;-) 5. You were ignorant (http://www.archlinux.org/pipermail/pacman-dev/2007-May/008238.html), but I didn't care: I used and enjoyed my patched versions and become silent about my patches. 6. Thanks to Xavier, my patches were applied later and I become motivated again. The "history" repeats itself: 4.' I am absolutely not satisfied with sync.c (which is one of the most important...) 5.' You are ignorant. But now I am disappointed about this. We are talking about ->inducer names, instead of real things (<-> when Dan committed my topo-sort algorithm (which was a hard-to-read patch, indeed), he simply renamed my "->son" to "->child" <- this is how things should go IMHO.). I tried to follow your rules (git patches...), but nothing changed. So now I decided to return to "my" rules: I will hack pacman, and I will use (and hopefully enjoy) my version. Probably I will sent my patches to you (<-totally needless); if you find them useful, use them. But I'm tired of fighting with you: the patch will give you all the needed information to you; you needn't use it. I simply don't care if you accept or reject them... I will use them. I won't have any pricks of conscience: I send my work to you, I cannot do more to make Arch-users life easier: the ball is on your side. Summary: I cannot find any other job, where I'm so unmotivated by "bosses" like here: http://www.archlinux.org/pipermail/pacman-dev/2007-September/009429.html http://www.archlinux.org/pipermail/pacman-dev/2007-October/009711.html http://www.archlinux.org/pipermail/pacman-dev/2007-September/009448.html If you are lazy (unable?) to make some decision or give any feedback and you are just waiting for ready-to-commit (without any modification?!) patches; I won't worry about the future of pacman neither. Hopefully I can find other projects where I can enjoy hacking again (mc, mplayer...) Bye, ngaba PS: Instead of these, I'm still optimistic (that's why I wrote this mail) -- but only you (Dan and Aaron) can tune up the development again: If you have not enough free time, I'm sure that there are some "specialists" around here who you can assign the to-be-reviewed stuff to (as I see, Xavier and I are the dependency/conflict "specialists" -- personally I don't really familiar in lower-lever stuffs (libarchive/libdownload...)). One more personal thing: When I arrived at this ML, I was pretty sure that Xavier is one of the main devels; he always red (and reads!) my patches, he had (and has) interesting questions/suggestions and he was the most active here... (to tell the truth, he was my motivator). So I'm sure that he can also speed-up the development. ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
participants (4)
-
Aaron Griffin
-
Dan McGee
-
Miklos Vajna
-
Nagy Gabor