* [bug#41578] [PATCH] gnu: Add opendoas.
@ 2020-05-28 15:35 Morgan.J.Smith
2020-05-28 17:57 ` Tobias Geerinckx-Rice via Guix-patches via
0 siblings, 1 reply; 4+ messages in thread
From: Morgan.J.Smith @ 2020-05-28 15:35 UTC (permalink / raw)
To: 41578; +Cc: Morgan Smith
From: Morgan Smith <Morgan.J.Smith@outlook.com>
* gnu/packages/admin.scm (opendoas): New variable.
---
gnu/packages/admin.scm | 51 ++++++++++++++++++++++++++++++++++++++++++
gnu/system.scm | 1 +
2 files changed, 52 insertions(+)
diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index b0a43d9644..594ec62c1d 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -1389,6 +1389,57 @@ commands and their arguments.")
;; See <http://www.sudo.ws/sudo/license.html>.
(license license:x11)))
+(define-public opendoas
+ (package
+ (name "opendoas")
+ (version "6.6.1")
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://github.com/Duncaen/OpenDoas.git")
+ (commit (string-append "v" version))))
+ (file-name (git-file-name name version))
+ (sha256
+ (base32
+ "07kkc5729p654jrgfsc8zyhiwicgmq38yacmwfvay2b3gmy728zn"))))
+ (build-system gnu-build-system)
+ (arguments (let* ((target (%current-target-system))
+ (compiler (if target
+ (string-append target "-gcc")
+ "gcc")))
+ `(#:phases
+ (modify-phases %standard-phases
+ ;; We replace the configure phase in order to remove all the
+ ;; default flags. The configure script doesn't accept most
+ ;; of the default flags
+ (replace 'configure
+ (lambda* (#:key configure-flags #:allow-other-keys)
+ ;; The configure script can only be told which
+ ;; compiler to use through environment variables
+ (setenv "CC" ,compiler)
+ (apply invoke "./configure" configure-flags)))
+ (add-before 'install 'fix-makefile
+ (lambda* (#:key outputs #:allow-other-keys)
+ ;; We can't chown to root as the chroot doesn't have
+ ;; this user. Also the store is owned by root so this
+ ;; isn't necessary.
+ (substitute* "bsd.prog.mk"
+ (("^\tchown.*$") "")))))
+ #:configure-flags (list (string-append "--prefix=" %output)
+ (string-append "--target=" (or ,target ""))
+ "--with-timestamp")
+ ;; Compiler choice is not carried over from the configure script
+ #:make-flags (list (string-append "CC=" ,compiler))
+ ;; There are no tests provided
+ #:tests? #f)))
+ (native-inputs `(("bison" ,bison)))
+ (home-page "https://github.com/Duncaen/OpenDoas")
+ (synopsis "Portable version of OpenBSD's doas command")
+ (description "Doas is a minimal replacement for the venerable sudo. It was
+initially written by Ted Unangst of the OpenBSD project to provide 95% of the
+features of sudo with a fraction of the codebase.")
+ (license license:isc)))
+
(define-public wpa-supplicant-minimal
(package
(name "wpa-supplicant-minimal")
diff --git a/gnu/system.scm b/gnu/system.scm
index d929187695..d5fd0979a1 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -896,6 +896,7 @@ use 'plain-file' instead~%")
(file-append inetutils "/bin/ping6")
(file-append sudo "/bin/sudo")
(file-append sudo "/bin/sudoedit")
+ (file-append opendoas "/bin/doas")
(file-append fuse "/bin/fusermount")
;; To allow mounts with the "user" option, "mount" and "umount" must
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [bug#41578] [PATCH] gnu: Add opendoas.
2020-05-28 15:35 [bug#41578] [PATCH] gnu: Add opendoas Morgan.J.Smith
@ 2020-05-28 17:57 ` Tobias Geerinckx-Rice via Guix-patches via
2020-05-28 18:51 ` Morgan Smith
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Geerinckx-Rice via Guix-patches via @ 2020-05-28 17:57 UTC (permalink / raw)
To: Morgan.J.Smith; +Cc: 41578
[-- Attachment #1: Type: text/plain, Size: 5173 bytes --]
Morgan,
Morgan.J.Smith@outlook.com 写道：
> * gnu/packages/admin.scm (opendoas): New variable.
Thank you! It looks good to me. I've queued it locally with the
(minor) changes below, but will wait a few days for
<https://issues.guix.gnu.org/41579> to land if that's all right
with you. I also need to test it as a proper setuid programme.
> +(define-public opendoas
> + (package
> + (name "opendoas")
> + (version "6.6.1")
> + (source (origin
> + (method git-fetch)
> + (uri (git-reference
> + (url
> "https://github.com/Duncaen/OpenDoas.git")
> + (commit (string-append "v" version))))
> + (file-name (git-file-name name version))
> + (sha256
> + (base32
> +
> "07kkc5729p654jrgfsc8zyhiwicgmq38yacmwfvay2b3gmy728zn"))))
> + (build-system gnu-build-system)
> + (arguments (let* ((target (%current-target-system))
I've added a newline after ‘arguments’ to give your phases (and
helpful comments) some room to breathe and keep lines from
exceeding 80 characters. It's mainly a matter of preference.
Since I'm reviewing this so you're stuck with my preference.
> + (compiler (if target
> + (string-append target
> "-gcc")
> + "gcc")))
> + `(#:phases
> + (modify-phases %standard-phases
> + ;; We replace the configure phase in order
> to remove all the
> + ;; default flags. The configure script
> doesn't accept most
> + ;; of the default flags
I shortened this to the last sentence and added a full stop here…
> + (replace 'configure
> + (lambda* (#:key configure-flags
> #:allow-other-keys)
> + ;; The configure script can only be
> told which
> + ;; compiler to use through environment
> variables
…and here. ;;-style comments are full sentences, unlike ;-ones.
> + (add-before 'install 'fix-makefile
> + (lambda* (#:key outputs
> #:allow-other-keys)
> + ;; We can't chown to root as the
> chroot doesn't have
> + ;; this user. Also the store is owned
> by root so this
> + ;; isn't necessary.
All true, but so common a change in Guix that it's not worth a
comment.
> + (substitute* "bsd.prog.mk"
> + (("^\tchown.*$") "")))))
Phases need to end in truth so I've added a #t here. We get away
without one in the previous phase because INVOKE itself is
guaranteed to return #t.
> + #:configure-flags (list (string-append
> "--prefix=" %output)
> + (string-append
> "--target=" (or ,target ""))
It didn't look to me like this was used for anything, and quoting
Morgan on IRC:
<butterypancake> ya, the configure script really doesn't do a damn
thing with target. But in the future it might save someone some
time.
> + ;; Compiler choice is not carried over from
> the configure script.
> + #:make-flags (list (string-append "CC="
> ,compiler))
I agree that it's nice to save future maintainers the trouble of
retracing your steps but don't like the idea of sleeper code.
I'll keep them as comments.
> + ;; There are no tests provided
> + #:tests? #f)))
Changed to the equivalent but more conventional
#:tests? #f))) ; no test suite
> + (native-inputs `(("bison" ,bison)))
Added a trivial newline before `.
> + (home-page "https://github.com/Duncaen/OpenDoas")
> + (synopsis "Portable version of OpenBSD's doas command")
> + (description "Doas is a minimal replacement for the
> venerable sudo. It was
> +initially written by Ted Unangst of the OpenBSD project to
> provide 95% of the
> +features of sudo with a fraction of the codebase.")
Thanks for including a multi-line description! Won't stop me from
trying to expand it some more.
> + (license license:isc)))
Not surprisingly, libbsd/ is under a 3-clause BSD licence. I
added it.
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -896,6 +896,7 @@ use 'plain-file' instead~%")
> (file-append inetutils "/bin/ping6")
> (file-append sudo "/bin/sudo")
> (file-append sudo "/bin/sudoedit")
> + (file-append opendoas "/bin/doas")
> (file-append fuse "/bin/fusermount")
>
> ;; To allow mounts with the "user" option, "mount"
> and "umount" must
This would be a separate patch. However, this would install doas
on almost all systems. I think the default list should contain
only the minimal defaults, and I don't see doas being a must-have
%base-package any time soon.
Kind regards,
T G-R
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* [bug#41578] [PATCH] gnu: Add opendoas.
2020-05-28 17:57 ` Tobias Geerinckx-Rice via Guix-patches via
@ 2020-05-28 18:51 ` Morgan Smith
2020-05-30 23:57 ` bug#41578: " Tobias Geerinckx-Rice via Guix-patches via
0 siblings, 1 reply; 4+ messages in thread
From: Morgan Smith @ 2020-05-28 18:51 UTC (permalink / raw)
To: Tobias Geerinckx-Rice; +Cc: 41578
Thanks so much for the review Tobias!
I went back to figure out exactly what --target did in the configure
script. Now there is a bunch of logic that doesn't go anywhere so at
first glance it doesn't look like this, but I'm fairly certain that all
it does is check if it's Linux. If it is Linux, than it will add some
cflags in the configure script. These flags never make their way out of
the configure script. Honestly, the configure script is so bad I'm
tempted to make a pull request.
I'm a little sad I didn't get torn apart more for technical reasons. I
learned that phases must end in #t and a few style points. Maybe next
time I'll throw in some terrible mistakes.
Also I realize that I totally forgot to put the copyright notice in!
Could you throw
";;; Copyright © 2020 Morgan Smith <Morgan.J.Smith@outlook.com>"
in there somewhere? I'm excited to get my name in this project!
Thanks,
Morgan
On 2020-05-28 13:57, Tobias Geerinckx-Rice wrote:
> Morgan,
>
> Morgan.J.Smith@outlook.com 写道：
>> * gnu/packages/admin.scm (opendoas): New variable.
>
> Thank you! It looks good to me. I've queued it locally with the
> (minor) changes below, but will wait a few days for
> <https://issues.guix.gnu.org/41579> to land if that's all right with
> you. I also need to test it as a proper setuid programme.
>
>> +(define-public opendoas
>> + (package
>> + (name "opendoas")
>> + (version "6.6.1")
>> + (source (origin
>> + (method git-fetch)
>> + (uri (git-reference
>> + (url "https://github.com/Duncaen/OpenDoas.git")
>> + (commit (string-append "v" version))))
>> + (file-name (git-file-name name version))
>> + (sha256
>> + (base32
>> + "07kkc5729p654jrgfsc8zyhiwicgmq38yacmwfvay2b3gmy728zn"))))
>> + (build-system gnu-build-system)
>> + (arguments (let* ((target (%current-target-system))
>
> I've added a newline after ‘arguments’ to give your phases (and helpful
> comments) some room to breathe and keep lines from exceeding 80
> characters. It's mainly a matter of preference. Since I'm reviewing
> this so you're stuck with my preference.
>
>> + (compiler (if target
>> + (string-append target "-gcc")
>> + "gcc")))
>> + `(#:phases
>> + (modify-phases %standard-phases
>> + ;; We replace the configure phase in order to
>> remove all the
>> + ;; default flags. The configure script doesn't
>> accept most
>> + ;; of the default flags
>
> I shortened this to the last sentence and added a full stop here…
>
>> + (replace 'configure
>> + (lambda* (#:key configure-flags
>> #:allow-other-keys)
>> + ;; The configure script can only be told which
>> + ;; compiler to use through environment
>> variables
>
> …and here. ;;-style comments are full sentences, unlike ;-ones.
>
>> + (add-before 'install 'fix-makefile
>> + (lambda* (#:key outputs #:allow-other-keys)
>> + ;; We can't chown to root as the chroot
>> doesn't have
>> + ;; this user. Also the store is owned by
>> root so this
>> + ;; isn't necessary.
>
> All true, but so common a change in Guix that it's not worth a comment.
>
>> + (substitute* "bsd.prog.mk"
>> + (("^\tchown.*$") "")))))
>
> Phases need to end in truth so I've added a #t here. We get away
> without one in the previous phase because INVOKE itself is guaranteed to
> return #t.
>
>> + #:configure-flags (list (string-append "--prefix="
>> %output)
>> + (string-append "--target="
>> (or ,target ""))
>
> It didn't look to me like this was used for anything, and quoting Morgan
> on IRC:
>
> <butterypancake> ya, the configure script really doesn't do a damn thing
> with target. But in the future it might save someone some time.
>
>> + ;; Compiler choice is not carried over from the
>> configure script.
>> + #:make-flags (list (string-append "CC=" ,compiler))
>
> I agree that it's nice to save future maintainers the trouble of
> retracing your steps but don't like the idea of sleeper code. I'll keep
> them as comments.
>
>> + ;; There are no tests provided
>> + #:tests? #f)))
>
> Changed to the equivalent but more conventional
>
> #:tests? #f))) ; no test suite
>
>> + (native-inputs `(("bison" ,bison)))
>
> Added a trivial newline before `.
>
>> + (home-page "https://github.com/Duncaen/OpenDoas")
>> + (synopsis "Portable version of OpenBSD's doas command")
>> + (description "Doas is a minimal replacement for the venerable
>> sudo. It was
>> +initially written by Ted Unangst of the OpenBSD project to provide
>> 95% of the
>> +features of sudo with a fraction of the codebase.")
>
> Thanks for including a multi-line description! Won't stop me from
> trying to expand it some more.
>
>> + (license license:isc)))
>
> Not surprisingly, libbsd/ is under a 3-clause BSD licence. I added it.
>
>> --- a/gnu/system.scm
>> +++ b/gnu/system.scm
>> @@ -896,6 +896,7 @@ use 'plain-file' instead~%")
>> (file-append inetutils "/bin/ping6")
>> (file-append sudo "/bin/sudo")
>> (file-append sudo "/bin/sudoedit")
>> + (file-append opendoas "/bin/doas")
>> (file-append fuse "/bin/fusermount")
>>
>> ;; To allow mounts with the "user" option, "mount"
>> and "umount" must
>
> This would be a separate patch. However, this would install doas on
> almost all systems. I think the default list should contain only the
> minimal defaults, and I don't see doas being a must-have %base-package
> any time soon.
>
> Kind regards,
>
> T G-R
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-30 23:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-28 15:35 [bug#41578] [PATCH] gnu: Add opendoas Morgan.J.Smith
2020-05-28 17:57 ` Tobias Geerinckx-Rice via Guix-patches via
2020-05-28 18:51 ` Morgan Smith
2020-05-30 23:57 ` bug#41578: " Tobias Geerinckx-Rice via Guix-patches via
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).