[PATCH] gnu: Add passmenu

  • Done
  • quality assurance status badge
Details
3 participants
  • Jelle Licht
  • Ludovic Courtès
  • Marius Bakke
Owner
unassigned
Submitted by
Jelle Licht
Severity
normal

Debbugs page

Jelle Licht wrote 8 years ago
(address . guix-patches@gnu.org)
CAPsKtfKfXFQUpZHPy_iv3Fq5oVtrty4+RRi2+mfYsk9MchrJUw@mail.gmail.com
Hello guix,

Attached is a patch to include passmenu, a dmenu interface to the pass
password store.

I was not quite sure how to structure this patch, as it basically installs
and wraps a shell script from the `password-store' sources. We could
instead include it as a separate output of our `password-store' package,
but I already had it like this in my GUIX_PACKAGE_PATH and I was not even
sure if that approach was in general preferable.

- Jelle
Attachment: file
Marius Bakke wrote 8 years ago
87zibw4oen.fsf@fastmail.com
Hi Jelle,

Jelle Licht <jlicht@fsfe.org> writes:

Toggle quote (11 lines)
> Hello guix,
>
> Attached is a patch to include passmenu, a dmenu interface to the pass
> password store.
>
> I was not quite sure how to structure this patch, as it basically installs
> and wraps a shell script from the `password-store' sources. We could
> instead include it as a separate output of our `password-store' package,
> but I already had it like this in my GUIX_PACKAGE_PATH and I was not even
> sure if that approach was in general preferable.

I don't think wrapping it with dmenu in PATH is necessary. Users of this
script are expected to have dmenu from before, and may want to use
another implementation (e.g. rofi), another version, etc.

Can you try to simply add a phase to the normal password-store package
that copies this file to out/bin? We can probably avoid the wrapper too
by giving it the full path to `xdotool`, e.g.:

(substitute "passmenu"
(("xdotool") (string-append (assoc-ref inputs "xdotool")
"/bin/xdotool")))

Adding 'xdotool' adds ~8MiB to the password-store closure size, so I
don't think we need a separate output either.

Thanks!
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAllzTHEACgkQoqBt8qM6
VPqCMggAlerDrLkGVmGF0/uo/4cGpAU4wn8qDuPPsYZLVsEIVzzrAIxk7j5qT+86
XiwHcLZW4siORg7P8HnhEVXjBob/7VG6eDRO82M/j3ujLtGr17QHs3Q9XHVbmo5D
6/SMYBSB+Wd7q6N+TXE0QmXt69/cyDvUAxHmi3EVIsR6c00EKv4zgSrgMxwwapfU
wrtlDHAKnoHhhxf8uNfV3CFyO2E/LDIt2znd+NflfA6+9D5EEgCnUBepuP/uXN7X
x8Q6vLQ1mywnHL4KbDxE/ZPecVP49hFrUsCjQ4+q3fi6VS5QYhc+Wd7yO55N05jt
mmHwUH8hFEA7L0tcQ7+pWPw1f3DTVg==
=MAXW
-----END PGP SIGNATURE-----

Ludovic Courtès wrote 7 years ago
(name . Marius Bakke)(address . mbakke@fastmail.com)
87tvyzcjmg.fsf@gnu.org
Hi Jelle,

Is anything holding this back?


TIA! :-)

Ludo’.

Marius Bakke <mbakke@fastmail.com> skribis:

Toggle quote (31 lines)
> Hi Jelle,
>
> Jelle Licht <jlicht@fsfe.org> writes:
>
>> Hello guix,
>>
>> Attached is a patch to include passmenu, a dmenu interface to the pass
>> password store.
>>
>> I was not quite sure how to structure this patch, as it basically installs
>> and wraps a shell script from the `password-store' sources. We could
>> instead include it as a separate output of our `password-store' package,
>> but I already had it like this in my GUIX_PACKAGE_PATH and I was not even
>> sure if that approach was in general preferable.
>
> I don't think wrapping it with dmenu in PATH is necessary. Users of this
> script are expected to have dmenu from before, and may want to use
> another implementation (e.g. rofi), another version, etc.
>
> Can you try to simply add a phase to the normal password-store package
> that copies this file to out/bin? We can probably avoid the wrapper too
> by giving it the full path to `xdotool`, e.g.:
>
> (substitute "passmenu"
> (("xdotool") (string-append (assoc-ref inputs "xdotool")
> "/bin/xdotool")))
>
> Adding 'xdotool' adds ~8MiB to the password-store closure size, so I
> don't think we need a separate output either.
>
> Thanks!
Jelle Licht wrote 7 years ago
(name . Ludovic Courtès)(address . ludo@gnu.org)
87shejmbml.fsf@fsfe.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (6 lines)
> Hi Jelle,
>
> Is anything holding this back?
>
> https://bugs.gnu.org/27791

It just fell through the cracks, thanks for reminding me :-).
I still needed to address some of Marius' concerns though...

Toggle quote (26 lines)
>
> TIA! :-)
>
> Ludo’.
>
> Marius Bakke <mbakke@fastmail.com> skribis:
>
>> Hi Jelle,
>>
>> Jelle Licht <jlicht@fsfe.org> writes:
>>
>>> Hello guix,
>>>
>>> Attached is a patch to include passmenu, a dmenu interface to the pass
>>> password store.
>>>
>>> I was not quite sure how to structure this patch, as it basically installs
>>> and wraps a shell script from the `password-store' sources. We could
>>> instead include it as a separate output of our `password-store' package,
>>> but I already had it like this in my GUIX_PACKAGE_PATH and I was not even
>>> sure if that approach was in general preferable.
>>
>> I don't think wrapping it with dmenu in PATH is necessary. Users of this
>> script are expected to have dmenu from before, and may want to use
>> another implementation (e.g. rofi), another version, etc.

While I agree with your general thoughts, wasn't guix supposed to
prevent this ad-hoc mishmash of software? If someone wants to use
another implementation (e.g. rofi), they could just create their own
package that inherits from `password-store' and overrides the "dmenu"
input. Case in point, I am not currently a user of dmenu (besides
indirectly through the passmenu script).

If people still see Marius' proposed solution as preferable, I am also
okay with that.

Toggle quote (9 lines)
>>
>> Can you try to simply add a phase to the normal password-store package
>> that copies this file to out/bin? We can probably avoid the wrapper too
>> by giving it the full path to `xdotool`, e.g.:
>>
>> (substitute "passmenu"
>> (("xdotool") (string-append (assoc-ref inputs "xdotool")
>> "/bin/xdotool")))

This seems nicer indeed.
Toggle quote (3 lines)
>>
>> Adding 'xdotool' adds ~8MiB to the password-store closure size, so I
>> don't think we need a separate output either.
Fair enough.

Toggle quote (3 lines)
>>
>> Thanks!

Thank you for the review.
Marius Bakke wrote 7 years ago
87efq27nij.fsf@fastmail.com
Jelle Licht <jlicht@fsfe.org> writes:

Toggle quote (44 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi Jelle,
>>
>> Is anything holding this back?
>>
>> https://bugs.gnu.org/27791
>
> It just fell through the cracks, thanks for reminding me :-).
> I still needed to address some of Marius' concerns though...
>
>>
>> TIA! :-)
>>
>> Ludo’.
>>
>> Marius Bakke <mbakke@fastmail.com> skribis:
>>
>>> Hi Jelle,
>>>
>>> Jelle Licht <jlicht@fsfe.org> writes:
>>>
>>>> Hello guix,
>>>>
>>>> Attached is a patch to include passmenu, a dmenu interface to the pass
>>>> password store.
>>>>
>>>> I was not quite sure how to structure this patch, as it basically installs
>>>> and wraps a shell script from the `password-store' sources. We could
>>>> instead include it as a separate output of our `password-store' package,
>>>> but I already had it like this in my GUIX_PACKAGE_PATH and I was not even
>>>> sure if that approach was in general preferable.
>>>
>>> I don't think wrapping it with dmenu in PATH is necessary. Users of this
>>> script are expected to have dmenu from before, and may want to use
>>> another implementation (e.g. rofi), another version, etc.
>
> While I agree with your general thoughts, wasn't guix supposed to
> prevent this ad-hoc mishmash of software? If someone wants to use
> another implementation (e.g. rofi), they could just create their own
> package that inherits from `password-store' and overrides the "dmenu"
> input. Case in point, I am not currently a user of dmenu (besides
> indirectly through the passmenu script).

In the "rofi" case it would be overriding dmenu and providing some extra
command-line arguments, but overall I agree with you and don't really
have a strong opinion. To my knowledge there is no established policy
for when to allow "impurities" (aka unqualified paths), but optional
dependencies often get a free pass.

I'm happy either way, so do what you think is best :)
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAlnlLjUACgkQoqBt8qM6
VPpIoQf9F3Wb2X/gS1dQ/Uq/QiQS1IS3n0NcAfTHW29+P+moAp2c/pQFbkQB/o/A
hJoX/+brzhLV+U096cfBcpWwWLxydWhyWhiGej40TZ2JZETZk1/oqCQYUrV2D5U9
g2Kz44XlWdQaUA6qnn88har69QG5bKRwKlTqeFolPhh00VGFaiXxHyVWps9GPtIx
TxrpK3GLmmVUrLqZy0O2IOl+EGvajEKKwnhF8fnqdMhzCrLEaUk7cvEjUc9nKLdj
o0Qnw2UowjkgsEKvHF4cnRIJtJRXTjoW+H41cy753ilksQi6KM9oYDitff2RUKc4
vtF8KmcblVLlHPDDncUy4iZqYfZRLw==
=WFwq
-----END PGP SIGNATURE-----

Jelle Licht wrote 7 years ago
(address . 27791-done@debbugs.gnu.org)
87r2t69ran.fsf@fsfe.org
In the end, I simply added dmenu to the list of `inputs' and replaced my
calls to `wrap-program' with a `substitute*' call. Pushed as 177475cfb5
to master.

Someone wanting to make use of an alternative to dmenu could just
inherit from `password-store' in order to override the relevant inputs
and/or phases. The closure size of password-store went from ~421MB to
~440MB, but as it is not a dependency of any non-password-store related
items, this should not be a big problem.

Thanks again for the guidance and reminders.

Marius Bakke <mbakke@fastmail.com> writes:

Toggle quote (53 lines)
> Jelle Licht <jlicht@fsfe.org> writes:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Hi Jelle,
>>>
>>> Is anything holding this back?
>>>
>>> https://bugs.gnu.org/27791
>>
>> It just fell through the cracks, thanks for reminding me :-).
>> I still needed to address some of Marius' concerns though...
>>
>>>
>>> TIA! :-)
>>>
>>> Ludo’.
>>>
>>> Marius Bakke <mbakke@fastmail.com> skribis:
>>>
>>>> Hi Jelle,
>>>>
>>>> Jelle Licht <jlicht@fsfe.org> writes:
>>>>
>>>>> Hello guix,
>>>>>
>>>>> Attached is a patch to include passmenu, a dmenu interface to the pass
>>>>> password store.
>>>>>
>>>>> I was not quite sure how to structure this patch, as it basically installs
>>>>> and wraps a shell script from the `password-store' sources. We could
>>>>> instead include it as a separate output of our `password-store' package,
>>>>> but I already had it like this in my GUIX_PACKAGE_PATH and I was not even
>>>>> sure if that approach was in general preferable.
>>>>
>>>> I don't think wrapping it with dmenu in PATH is necessary. Users of this
>>>> script are expected to have dmenu from before, and may want to use
>>>> another implementation (e.g. rofi), another version, etc.
>>
>> While I agree with your general thoughts, wasn't guix supposed to
>> prevent this ad-hoc mishmash of software? If someone wants to use
>> another implementation (e.g. rofi), they could just create their own
>> package that inherits from `password-store' and overrides the "dmenu"
>> input. Case in point, I am not currently a user of dmenu (besides
>> indirectly through the passmenu script).
>
> In the "rofi" case it would be overriding dmenu and providing some extra
> command-line arguments, but overall I agree with you and don't really
> have a strong opinion. To my knowledge there is no established policy
> for when to allow "impurities" (aka unqualified paths), but optional
> dependencies often get a free pass.
>
> I'm happy either way, so do what you think is best :)
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 27791@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 27791
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help