FS#10630 - Making optdepends more visible to the end-user

Attached to Project: Pacman
Opened by Abhishek Dasgupta (abhidg) - Wednesday, 11 June 2008, 02:55 GMT
Last edited by Xavier (shining) - Sunday, 24 August 2008, 01:10 GMT
Task Type Feature Request
Category General
Status Closed
Assigned To Xavier (shining)
Architecture All
Severity Low
Priority Normal
Reported Version 3.1.4
Due in Version 3.2.1
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Summary and Info:

Currently, optdepends is an optional field in PKGBUILDs which lists
optional dependencies for a particular package along with the use of
that particular dependency. However optdepends is not exposed to the
end-user during installation of packages. It is visible on doing a query
, but ideally it should be printed on installation. Otherwise
duplication of work occurs when a packager puts in optdepends and has
to manually put in the deps in the .install file to be printed out.

It would be nice if pacman printed out the optdepends for a package
after installation prior to/after the .install file (if any) is
executed. Also a switch to install optdepends would be handy.
This task depends upon

Closed by  Xavier (shining)
Sunday, 24 August 2008, 01:10 GMT
Reason for closing:  Implemented
Additional comments about closing:  implemented with commit 96e023c7bd0b1
Comment by Xavier (shining) - Wednesday, 11 June 2008, 06:00 GMT
The printing on installation should be rather easy to do, and is indeed important because it is what would replace all these scriptlets which only prints optional dependencies. So this should be done for 3.2.

About a switch to install, I am not sure. It would be yet another switch (we have already many ;)) but also, what would it do, install all optdepends?
It seems like this feature might be more interesting to implement in a gui where the opt depends could be graphically selected or something.
Comment by Abhishek Dasgupta (abhidg) - Thursday, 12 June 2008, 07:42 GMT
I was thinking of a switch which would result in something like what
happens when one tries to install a group:

# pacman -Sz blueman
Optional dependencies for blueman
gnome-bluetooth: Send files over bluetooth
gnome-vfs-obexftp: To view OBEX ftp shares
btsco: For bluetooth audio

Install all of them (y/n)?

Selecting y will install all, and n will allow one-by-one choices.

I agree this is not as important as printing out the optdepends.
Comment by Xavier (shining) - Thursday, 12 June 2008, 07:56 GMT
Hm yeah, I see.
Anyway, I actually run into several problems when trying to do just the printing part.
It is easy to access the optdepends after the installation of one package in libalpm (libalpm/add.c), but the printing should be done from the frontend.
This could be done by some sort of callbacks, maybe another EVENT ? That looks very ugly though...

If the printing is done from the frontend, it can only be done after the transaction ends, that is, after every package is installed.
From the frontend, we can get a list of the packages with alpm_trans_get_pkg but this returns a list of pmsyncpkg.
And this pmsyncpkg contains a pmpkg which comes from the sync database!
And guess what, the sync databases don't have this OPTDEPENDS information, it only appears in the local database.

So currently, I am stuck.
Comment by Xavier (shining) - Wednesday, 18 June 2008, 14:04 GMT
So to make the second part more explicit, here is the patch I wrote, which does not work because sync dbs don't have OPTDEPENDS.
But maybe they should.
Comment by Xavier (shining) - Thursday, 24 July 2008, 09:29 GMT
This feature request deserves more attention and would help encouraging the usage of optdepends
http://www.archlinux.org/pipermail/arch-dev-public/2008-July/007136.html

Do we need to delay it to a later 3.2.x release or even 3.3?
Comment by Nagy Gabor (combo) - Wednesday, 30 July 2008, 21:23 GMT
"If the printing is done from the frontend, it can only be done after the transaction ends".
This is not true:
EVENT(trans, PM_TRANS_EVT_UPGRADE_DONE, newpkg, oldpkg);
So the front-end can do anything after each package upgrade.
Comment by Xavier (shining) - Wednesday, 30 July 2008, 21:34 GMT
Yeah, I mentioned the EVENT stuff in the first paragraph of the same comment.
So that's the way we need to go?
Comment by Nagy Gabor (combo) - Wednesday, 30 July 2008, 21:40 GMT
"So that's the way we need to go?"
IMHO yes.
Comment by Nagy Gabor (combo) - Wednesday, 30 July 2008, 22:34 GMT
Some reasons against this way:
If we want to do some hacks in order to automatically install optdepends we must hack this to back-end, because in this case the optdepends part should be handled in sync_trans_prepare part (where we have syncdb info only, so %OPTDEPENDS% db-record is needed!). Here the front-end cannot add any new targets to the transaction, only the back-end. (Creating transaction from front-end is very ugly, this process can be "recursive".)

So imho the first thing we should decide, whether alpm should deal with optdepends at all. If not, imho it shouldn't send any optdepend specific event, and front-end should do its task via UPGRADE_DONE callback. If yes, imho we _must_ put %OPTDEPENDS% to db; in this case back-end should ask optdepend specific question, so that solution is optimal.
Comment by Xavier (shining) - Thursday, 31 July 2008, 06:05 GMT
Oh wait, I indeed misunderstood what you were saying. You were not talking about a specific EVENT for optdepends, just printing optdepends after each package upgrade.
Yeah, that seems nice indeed, I did not think about that.

"So imho the first thing we should decide, whether alpm should deal with optdepends at all. If not, imho it shouldn't send any optdepend specific event, and front-end should do its task via UPGRADE_DONE callback. If yes, imho we _must_ put %OPTDEPENDS% to db; in this case back-end should ask optdepend specific question, so that solution is optimal."

Why not first doing it in the frontend, but also put %OPTDEPENDS% to the db, so that we can later decide to handle it in the backend. It would also allows us to see optdepends on -Si operation.
Comment by Nagy Gabor (combo) - Thursday, 31 July 2008, 09:59 GMT
Yes, %OTPDEPENDS% db-field for -Si is a valid point (currently -Si reports no optdepends for every package), from pacman side, this is implemented. (I guess, repo-add should be patched in a straightforward way.)
Comment by Xavier (shining) - Thursday, 31 July 2008, 14:39 GMT
It was very easy using UPGRADE_DONE indeed, great suggestion!

Patch attached, and a sample output :

checking package integrity...
(2/2) checking for file conflicts [#####################] 100%
(1/2) upgrading audacious-plugins [#####################] 100%
Optional dependencies for audacious-plugins
jack-audio-connection-kit: Jack output
curl>=7.16.2: Audioscrobbler plugin
musicbrainz: Audioscrobbler plugin
lirc-utils: Infra-red support
libmodplug: Modplug plugin
projectm: Projectm visualization plugin
sdl: Paranormal visualization plugin
neon: HTTP and Web-stream playback
wavpack: Wavpack input
fluidsynth: Fluidsynth amidi-plug backend
(2/2) upgrading alsa-lib [#####################] 100%
Optional dependencies for alsa-lib
python
Comment by Nagy Gabor (combo) - Thursday, 31 July 2008, 14:46 GMT
Argh, we did some parallel work.

Your patch is ~identical to mine, except that I used list_display(" ", optdepends, 1) instead of printf. (See ML for list_display patch. This is a bit ugly, because I used " " for displaying 4 spaces...)
Comment by Nagy Gabor (combo) - Thursday, 31 July 2008, 14:47 GMT
Wow, filespray don't allow many spaces here. Imagine _3_ spaces between "".
Comment by Nagy Gabor (combo) - Thursday, 31 July 2008, 14:49 GMT
Btw, I let repo-add work for you, because I don't really understand that script. (I could copy the DEPENDS part, but I don't want.)
Comment by Xavier (shining) - Thursday, 31 July 2008, 14:52 GMT
Hmm I see, that makes your list_display patch interesting indeed :)

I was looking at the repo-add part too, but I am not doing much except copying depends/conflicts/provides part.
Comment by Nagy Gabor (combo) - Thursday, 31 July 2008, 15:09 GMT
Use of list_display is not identical with your version, because list_display uses indentprint. So if a line is too long, your patch won't indent the "line-tail", mine will. (This also affects OptDepends printing after my list_display patch). I don't know which is better, I let the decision to you (reconstructing my patch from yours is left for exercise if needed (your variable names/strings are better imho) :-P)
Comment by Xavier (shining) - Thursday, 31 July 2008, 16:40 GMT
Just push your patch to your git repo just in case, so that I can also compare them :)
Comment by Xavier (shining) - Thursday, 31 July 2008, 17:35 GMT
No, don't worry. I just rebased my patch against your working branch, for using your list_display function, and I like it. Thanks!
Comment by Nagy Gabor (combo) - Thursday, 31 July 2008, 17:57 GMT
I didn't worry, I just don't like parallel works ;-) I am not sure that using list_display is better here than noindent:
I am not really satisfied with my indent neither (the same for -Qi Optdepends):
Original (no) indent:
foo: install foo to enab
le bar

New indent:
foo: install foo to
enable bar

Clearly, indent to : looks be the best, but I am lazy for doing that (and we have no clear definition to OptDepend format)
Comment by Xavier (shining) - Thursday, 31 July 2008, 17:58 GMT
Uh, repo-add code is broken, as we already noticed after the pacman 3.1 release and the "provision version" format we had introduced.
We then changed it to "provision=version" to avoid this problem, but the same problem still hurts us for the optdepends. And we can't really work around it here.
I went back to this old provision version bug report, and saw you proposed a fix to repo-add :
http://bugs.archlinux.org/task/9171?getfile=1756

I guess I did not pay much attention to it back then, because looking at it again now, it looks fine, and this code definitely needed to be fixed.
So after getting that fix, the repo-add part for optdepends should then be straightforward.
Comment by Nagy Gabor (combo) - Thursday, 31 July 2008, 18:01 GMT
Hm. Indeed. (In my previous comment you can imagine some hidden tabs ;-) Did I understand repo-add some months ago?! Interesting ;-)
Comment by Nagy Gabor (combo) - Thursday, 31 July 2008, 18:09 GMT
I hope we don't have a corner case, where "echo -e" can cause problems.
Comment by Xavier (shining) - Friday, 01 August 2008, 08:54 GMT
Hm actually, I don't clearly understand your old indent vs new indent. Maybe you could attach two files to preserve the format, so that we can see the difference.
Because in my test, your new indent looked ok to me, when I tested with pacman -Qi audacious-plugins in a small terminal.

About echo -e, I can't think about any cases where it could cause problem. We should not have escape characters anywhere.
Comment by Nagy Gabor (combo) - Friday, 01 August 2008, 10:41 GMT
OK. I attached an indent-file. This indent discussion is totally harmless, it is just an eye-candy ;-) [Currently we use list_display(,,1) for optdepends printing, so we can also implement a new function instead of linebreak param of list_display]

Btw, I am satisfied with both your and my patch atm.
Comment by Xavier (shining) - Friday, 01 August 2008, 11:35 GMT
Oh ok, you meant aligning to the second colon, not the first one :) I get it now.
Ok, I agree that the output with your patch is not the ideal one, but it is still very decent, and I find it much better than no indent at all.

And btw, I thought the same, having two list_display functions instead of a linebreak param.
But this situation is very similar to the yesno extra param I added. Maybe it would also be better here to provide two functions, like yesno and noyes.
In both cases, the two functions could be basic wrapper to the generic function with an extra parameter.
Comment by Xavier (shining) - Friday, 01 August 2008, 11:37 GMT
We are also getting way too off-topic here, we either need to continue this discussion on pacman-dev ML (when it works again), or privately :)
Comment by Gavin Bisesi (Daenyth) - Monday, 11 August 2008, 17:50 GMT
This is marked for 3.2.0
Is it implemented?
Comment by Xavier (shining) - Monday, 11 August 2008, 18:09 GMT
Unfortunately not.
The progress happened too late, right around the 3.2.0 release.
Then I hoped it could make it in 3.2.1, but I doubt Dan wants new features in minor release, so I will put this for 3.3 (even if it is a minor patch).
Comment by Xavier (shining) - Sunday, 24 August 2008, 01:09 GMT
Well actually, since this was a minor patch only affecting the frontend and also an useful feature, it was accepted for 3.2.1

Loading...