On Mon, Nov 16, 2009 at 4:02 AM, Dan McGee
+*\--print*:: + Only print the targets instead of performing the actual operation (sync, + remove or upgrade). Use '\--print-format' to specify how targets are + displayed. + +*\--print-format* <'format'>:: + Specify a printf-like format to control the output of the '\--print' + operation. The possible are attributes are : %n for pkgname, %v for pkgver, %l + for location, %r for repo and %s for size. The default format string is "%l", + which displays url with '-S', filename with '-U' and pkgname-pkgver with '-R'. +
* Isn't --print actually -p/--print? * Should the default format stuff actually be in the --print documentation (either as well, or completely move it there?). I feel like I would look there first. * You didn't kill the print-uri documentation.
done done done
+ /* set up the print operations */ + if(config->print) { + config->noconfirm = 1; + config->flags |= PM_TRANS_FLAG_NOCONFLICTS; + config->flags |= PM_TRANS_FLAG_NOLOCK; + /* Display only errors */ + config->logmask &= ~PM_LOG_WARNING; Man, I hate having to do this. We really should think about a stderr/stdout jihad.
So like we would display all warnings/errors to stderr and the useful stuff to stdout ? And it indeed sounds like it would be a lot of work.. It seems like pacman was not written having in mind it could be used in scripts/wrapper.
+ } + + #if defined(HAVE_GETEUID) && !defined(CYGWIN) /* check if we have sufficient permission for the requested operation */ if(myuid > 0 && needs_root()) { diff --git a/src/pacman/remove.c b/src/pacman/remove.c index b5119fa..06503d3 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -159,6 +159,12 @@ int pacman_remove(alpm_list_t *targets) }
/* Step 3: actually perform the removal */ + if(config->print) { /* --print */ + print_packages(alpm_trans_get_pkgs()); + /* we are done */ None of the comments here seem particularly necessary. (/*--print*/, /*we are done*/). Can you kill them?
done
- /* Display only errors with -Sp and -Sw operations */ - if((config->flags & PM_TRANS_FLAG_DOWNLOADONLY) || config->op_s_printuris) { + /* Display only errors with -Sw operations */ + if(config->flags & PM_TRANS_FLAG_DOWNLOADONLY) { Why do we do this for -Sw?
That question was not directly related to my patch but yeah, I have no idea why we do this for -Sw so I killed it.
/* Step 3: perform the installation */ + if(config->print) { /* --print */ + print_packages(alpm_trans_get_pkgs()); + /* we are done */ + trans_release(); + return(0);
Kill comments
done
+ /* %r : repo */ + if(strstr(temp,"%r")) { + const char *repo = "local"; + pmdb_t *db = alpm_pkg_get_db(pkg); + if(db) { + repo = alpm_db_get_name(db); + } + string = strreplace(temp, "%r", repo); + free(temp); + temp = string; + } + /* %s : size */ + if(strstr(temp,"%s")) { + char *size; + double mbsize = 0.0; + mbsize = pkg_get_size(pkg) / (1024.0 * 1024.0); + asprintf(&size, "%.2f", mbsize); + string = strreplace(temp, "%s", size); + free(size); + free(temp); + temp = string; + } This strreplace() stuff seems a bit naive, considering we could do a lot of replaces on the string. I'm not sure we need to worry about it but could be a place for some improvement.
Do you mean that we are walking through the same string many times ? I thought at some point we could have a multireplace function which could take a list of struct replace { char *old ; char *new; } but ... firstly it looked a bit overkill and secondly, I was worried that all the pkg/db accessors could be more costly than these strings operations. Though that could also be solved by doing a first pass on the string to see what format options we have, and then build the replace list with only the stuff we need. I agree the current code is quite naive but it does not affect performance so much that it makes the operation unusable. pacman -S kde gnome --print --printformat '%n %v %l %r %s' takes 0.7 second.
OK, and now for some usability testing.
$ ./src/pacman/pacman -Q <big list of packages> ... zope-interface 3.5.1-1 zvbi 0.2.33-1
$ ./src/pacman/pacman -Q --print error: no targets specified (use -h for help)
So two things here- I expected --print to work and it did not, and we already have the standard "pkgname <space> pkgver" output that our default print format uses. Do we want to invent a new one with the default --print for remove?
Oh wait, does --print even work for -Q? Now I'm all confused. :) $ ./src/pacman/pacman -Q --print pacman-git error: package "pacman-git" not found
$ ./src/pacman/pacman -Q pacman-git pacman-git 20091018-1
More unexpected behavior...
$ ./src/pacman/pacman -Ss --print pacman core/pacman 3.3.3-1 (base) A library-based package manager with dependency support core/pacman-mirrorlist 20090616-1 (base) [installed] Arch Linux mirror list for use by pacman extra/namcap 2.4-1 [installed] A Pacman package analyzer
I wanted a list of URLs for all packages that matched my pacman search, I did not get that...
-Dan
About that last part, I already replied one month ago :) Since the changes I just did were quite trivial, I will just push an updated version to my print branch.