GNU bug report logs - #27791
[PATCH] gnu: Add passmenu

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27791 in the body.
You can then email your comments to 27791 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jelle Licht <jlicht <at> fsfe.org>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add passmenu
Date: Sat, 22 Jul 2017 14:35:56 +0200

[Message part 1 (text/plain, inline)]

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

[Message part 2 (text/html, inline)]

[0001-gnu-Add-passmenu.patch (application/octet-stream, attachment)]

Message #8 received at 27791 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Jelle Licht <jlicht <at> fsfe.org>, 27791 <at> debbugs.gnu.org
Subject: Re: [bug#27791] [PATCH] gnu: Add passmenu
Date: Sat, 22 Jul 2017 15:00:32 +0200

[Message part 1 (text/plain, inline)]

Hi Jelle,

Jelle Licht <jlicht <at> 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!

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 27791 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 27791 <at> debbugs.gnu.org, Jelle Licht <jlicht <at> fsfe.org>
Subject: Re: [bug#27791] [PATCH] gnu: Add passmenu
Date: Mon, 16 Oct 2017 15:22:47 +0200

Hi Jelle,

Is anything holding this back?

  https://bugs.gnu.org/27791

TIA!  :-)

Ludo’.

Marius Bakke <mbakke <at> fastmail.com> skribis:

> Hi Jelle,
>
> Jelle Licht <jlicht <at> 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!

Message #14 received at 27791 <at> debbugs.gnu.org (full text, mbox):

From: Jelle Licht <jlicht <at> fsfe.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27791 <at> debbugs.gnu.org, Marius Bakke <mbakke <at> fastmail.com>,
 Jelle Licht <jlicht <at> fsfe.org>
Subject: Re: [bug#27791] [PATCH] gnu: Add passmenu
Date: Mon, 16 Oct 2017 16:05:22 +0200

Ludovic Courtès <ludo <at> 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 <at> fastmail.com> skribis:
>
>> Hi Jelle,
>>
>> Jelle Licht <jlicht <at> 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.

>>
>> 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.
>>
>> Adding 'xdotool' adds ~8MiB to the password-store closure size, so I
>> don't think we need a separate output either.
Fair enough.

>>
>> Thanks!

Thank you for the review.

Message #17 received at 27791 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Jelle Licht <jlicht <at> fsfe.org>, Ludovic Courtès
 <ludo <at> gnu.org>
Cc: 27791 <at> debbugs.gnu.org, Jelle Licht <jlicht <at> fsfe.org>
Subject: Re: [bug#27791] [PATCH] gnu: Add passmenu
Date: Tue, 17 Oct 2017 00:09:56 +0200

[Message part 1 (text/plain, inline)]

Jelle Licht <jlicht <at> fsfe.org> writes:

> Ludovic Courtès <ludo <at> 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 <at> fastmail.com> skribis:
>>
>>> Hi Jelle,
>>>
>>> Jelle Licht <jlicht <at> 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 :)

[signature.asc (application/pgp-signature, inline)]

Message #22 received at 27791-done <at> debbugs.gnu.org (full text, mbox):

From: Jelle Licht <jlicht <at> fsfe.org>
To: 27791-done <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Jelle Licht <jlicht <at> fsfe.org>, Marius Bakke <mbakke <at> fastmail.com>
Subject: Re: [bug#27791] [PATCH] gnu: Add passmenu
Date: Fri, 10 Nov 2017 14:50:08 +0100

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 <at> fastmail.com> writes:

> Jelle Licht <jlicht <at> fsfe.org> writes:
>
>> Ludovic Courtès <ludo <at> 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 <at> fastmail.com> skribis:
>>>
>>>> Hi Jelle,
>>>>
>>>> Jelle Licht <jlicht <at> 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 :)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.