[PATCH] gnu: Add opendoas.

  • Done
  • quality assurance status badge
Details
2 participants
  • Morgan.J.Smith
  • Tobias Geerinckx-Rice
Owner
unassigned
Submitted by
Morgan.J.Smith
Severity
normal

Debbugs page

Morgan.J.Smith wrote 5 years ago
(address . guix-patches@gnu.org)(name . Morgan Smith)(address . Morgan.J.Smith@outlook.com)
DM5PR1001MB210525690A0A78B56DD33822C58E0@DM5PR1001MB2105.namprd10.prod.outlook.com
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(+)

Toggle diff (76 lines)
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
Tobias Geerinckx-Rice wrote 5 years ago
(address . Morgan.J.Smith@outlook.com)
875zcg3pu8.fsf@nckx
Morgan,

Morgan.J.Smith@outlook.com 写道：
Toggle quote (2 lines)
> * 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.

Toggle quote (18 lines)
> +(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.

Toggle quote (12 lines)
> + (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…

Toggle quote (8 lines)
> + (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.

Toggle quote (9 lines)
> + (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.

Toggle quote (3 lines)
> + (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.

Toggle quote (5 lines)
> + #: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.

Toggle quote (5 lines)
> + ;; 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.

Toggle quote (3 lines)
> + ;; There are no tests provided
> + #:tests? #f)))

Changed to the equivalent but more conventional

#:tests? #f))) ; no test suite

Toggle quote (2 lines)
> + (native-inputs `(("bison" ,bison)))

Added a trivial newline before `.

Toggle quote (8 lines)
> + (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.

Toggle quote (2 lines)
> + (license license:isc)))

Not surprisingly, libbsd/ is under a 3-clause BSD licence. I
added it.

Toggle quote (12 lines)
> --- 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
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQT12iAyS4c9C3o4dnINsP+IT1VteQUCXs/7bwAKCRANsP+IT1Vt
eTPcAQDYdGslHS5kjbGlUFnIn91e6PaL1o7ZXa64OKl3txZqDQEAqoAQQQ9pUWr/
gNuPToBQiaqCyBj6x30jhoMfVUGFSQY=
=PZ67
-----END PGP SIGNATURE-----

Morgan Smith wrote 5 years ago
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
DM5PR1001MB2105D8359F1169986D89FBD9C58E0@DM5PR1001MB2105.namprd10.prod.outlook.com
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:
Toggle quote (135 lines)
> 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
Tobias Geerinckx-Rice wrote 5 years ago
(name . Morgan Smith)(address . Morgan.J.Smith@outlook.com)(address . 41578-done@debbugs.gnu.org)
87k10tyo0d.fsf@nckx
Morgan,

Morgan Smith 写道：
Toggle quote (6 lines)
> 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.

Don't give up!

Toggle quote (6 lines)
> 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!

Well, Marius stole my thunder with your other patch, but welcome
aboard!

Kind regards,

T G-R
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQT12iAyS4c9C3o4dnINsP+IT1VteQUCXtLy8gAKCRANsP+IT1Vt
eUvaAP9Hm7aBcR1qlXuDIN+JEFO1juBrQycKhGdNoG7jAsJqdQEA7Gp9Uh6Vwt8n
/nkki1yRuFBWgHCQUu6MOGIP1km1Wg0=
=LDLB
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 41578
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