Lacking abstraction #25
Comments
|
Hi Damien! Thanks for the feedback :D In many ways you are right, Helm System Packages lacks a good abstraction. Ensued the current code base: it was the easiest and most efficient to Get Things Done™. Now would be a good time for a massive refactoring, be it's a bit harder than it seems at first glance: if you give it a deeper look you'll soon notice that much of the code is very similar up to some tiny annoying details that make everything quite hard to factor. The core issue here is that we need to parse the command outputs and they are all massively different. Regarding https://gitlab.com/jabranham/system-packages/: it only deals with commands, not with output parsing, so it's much easier to make a uniform abstraction. We could re-use system-packages.el in this package, but it would add a depedency for not much benefit: about 3-5 commands are needed per package and that's it. The rest of the job must be done by us. If you've got some free time, it would be fantastic if you could contribute a dnf interface. It's pretty easy once you got the dnf output parsed. I suggest you start from Let me know if you are down for it! Cheers! |
|
Please see #26 for a prototype. Initial thoughts on what is lacking:
|
|
|
Pierre Neidhardt <[email protected]> writes:
Some actions don't even map exactly across the various package
managers (e.g. some actions don't exist in `brew`).
Instead of using :action slot use :action-transformer, like this you can
use only one set of actions for all.
…--
Thierry
|
|
Didn't know about |
|
I just remembered one strong argument why modules are not more factorized: simply because it's very hard to test and maintain by a single person. Testing a foreign package manager usually requires virtual machines, which can be costly and cumbersome to setup. This is why we must be conservative and make sure every module works independently from the rest. That said, we can work on factorization and move the modules to the new abstraction one by one. |
|
I've committed a draft of a package manager abstraction. See #27. |
|
#27 has been merged with some modifications. |
|
Great job, thank you. Some more abstractions:
(defvar helm-system-packages-guix
(helm-system-packages-manager-create
:name "guix"
:actions '(
:info #'helm-system-packages-guix-info))) |
|
I agree but my whole point is about abstracting away from helm for 2 reasons:
Not quite. Look at my PR #26. You will see a
you are right. This is always the case when you abstract things away. Moreover, this is probably the only way to implement the above (abstracting away from helm to get the selected packages as parameter).
you could provide a
this could be done at the level of (helm-system-packages-add-manager-action "dnf" '("Cleanup dnf database" . ...)) |
|
Damien Cassou <[email protected]> writes:
> The [...] block is rather idiomatic in Helm
I agree but my whole point is about abstracting away from helm for 2 reasons:
- maybe tomorrow I would like to use ivy instead;
So why developing a helm package? Once you have removed all the helm
related code you endup with simple completing-read's without the benefit
of all the helm features not present in other packages, also the "helm-"
prefix of your package name have non sense once you have removed all
helm related code.
…--
Thierry
|
|
@DamienCassou: I had initially misunderstood what you wanted to abstract. That said:
Conclusion: It will take a significant effort to factor Helm aside and I think it makes little sense to go down that road unless someone is really interested in making an Ivy interface.
OK, it's not much but yes, we can abstract that away.
Why separating actions at all? Isn't it simpler to provide a list of functions instead? This is what the current implementation does. Note that "actions" do not have a Helm-specific type, it's just an alist of
OK. |
that was not my point, sorry. My point was that if modules depend less on helm and stay focused on the package manager they support, the developers life will be simpler when developing a new module or when the completion interface changes.
you may, but this approach has some drawbacks:
This is more code to copy-paste which means it will be harder to maintain every module. |
|
Some of the Helm specific code found in modules would not be shorter with an abstraction. A good example is actions: with an abstraction, you'd need to provide a list of functions, which is what Helm does anyways. Adding an abstraction layer here would be duplicating code. I thought about using shared keybindings but in practice they are too different I'm afraid. I'm not sure what you have in mind when it comes to abstracting the actions. Could you provide an example? |
I provided one above. Here is it again: (defvar helm-system-packages-guix
(helm-system-packages-manager-create
:name "guix"
:actions '(
:info #'helm-system-packages-guix-info)
:extra-actions: helm-system-packages-guix-extra-actions)) ; <- if needed |
|
Yes, but I don't understand what
(defvar helm-system-packages-guix
(helm-system-packages-manager-create
:name "guix"
:actions '(
:info #'helm-system-packages-guix-info)
:extra-actions: helm-system-packages-guix-extra-actions)) ; <- if needed
improves over
(defvar helm-system-packages-guix
(helm-system-packages-manager-create
:name "guix"
:actions (list helm-system-package-guix-actions helm-system-packages-default-actions))
with `helm-system-packages-default-actions` being anything that can be factored.
|
|
This looks good as well. Can you please tell me how |
|
Hmm, looks like I wrote too fast, I don't see anything that can be factored :( Magic names with So if we don't go by magic names, then, regardless of the interface, the action-specific functions will have to be glued to the interface at some point. So I'm not sure what your suggestion simplifies. Currently: Your suggestion: |
I think it simplifies 2 things:
I feel that I'm repeating myself a lot. If you are not convinced by above arguments, I suggest we agree to disagree and move on :-). |
|
OK, I understand what you mean. Sorry for the previous confusion. I've had a similar architecture in mind at the beginning, but that was without counting on the existing code and the friction that arises when maintaining the various modules. Since I've been using only The road we are going down now is to move What we would ideally need is a test suite embedding various pre-recorded outputs (info, search, etc.) so that the modules can be safely worked on without having the actual package manager at hand. From there, we could safely change the interface to something more data-centric as you suggest. Any idea on how to produce those output recordings without access to the package manager? |
no need to apologize. It's at least half my fault :-).
that would be a really good start. If you want to go the integration test route, you could also test your package with several docker images, each built with a different OS. Gitlab has a CI supporting that by default.
you can easily get access to any package manager you want through virtualization. If you give me a list of package-manager commands, I will execute them for one or two package managers. |
|
Absolutely, but I don't have the resources to virtualize anything at the moment.
You can find the list of commands by looking up the "process-file" and
"call-process" functions.
Continuous integration would also be welcome of course.
|
|
I found another reason for abstraction: if modules are kept independent from helm, |
|
At the moment it should already be possible to run helm-system-packages over dnf
and guix separately.
As to displaying both of them at the same time... I guess this would work in
different sources. Is that what you were thinking? If so, I think it's a good idea.
|
|
Pierre Neidhardt <[email protected]> writes:
As to displaying both of them at the same time... I guess this would work in
different sources. Is that what you were thinking? If so, I think it's a good idea.
you could get a list of all packages from both managers, each one in a
separate helm section (I don't know Helm's vocabulary, sorry). The
package is not there yet, but it could become reasonable with the
refactorings I suggest.
…--
Damien Cassou
http://damiencassou.seasidehosting.st
"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill
|
|
Yes, we talk about the same thing: "source" is a section in Helm's parlance.
|
|
@DamienCassou: It's been a while! In the meantime I've worked on a similar universal package manager abstraction for Nyxt, this time starting from scratch. The abstraction should be much better, plus it has support for functional package managers like Guix and Nix. https://nyxt.atlas.engineer/article/release-2-pre-release-4.org |
|
Good job! |
I've just found your blog and this package. I was excited. When I saw there was no support for dnf I started writing one starting from the support for dpkg. Then I realized that it was a lot of code to duplicate from dpkg even though I was expecting to mostly only implement a list of strings of command line arguments (in the same way as https://gitlab.com/jabranham/system-packages/).
Why is there so much stuff to implement? Is there any abstraction missing? Why isn't helm-system-packages depending on system-packages?
The text was updated successfully, but these errors were encountered: