GNU bug report logs - #45825
[PATCH] gnu: Add efi_analyzer.

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 45825 in the body.
You can then email your comments to 45825 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: Vincent Legoll <vincent.legoll <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Vincent Legoll <vincent.legoll <at> gmail.com>
Subject: [PATCH] gnu: Add efi_analyzer.
Date: Tue, 12 Jan 2021 20:37:47 +0100

* gnu/packages/engineering.scm (efi_analyzer): New variable.
---
 gnu/packages/engineering.scm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gnu/packages/engineering.scm b/gnu/packages/engineering.scm
index 5166a4635c..1c001cbe0f 100644
--- a/gnu/packages/engineering.scm
+++ b/gnu/packages/engineering.scm
@@ -1580,6 +1580,33 @@ like relocation symbols.  It is able to deal with malformed binaries, making
 it suitable for security research and analysis.")
     (license license:lgpl3)))
 
+(define-public efi_analyzer
+  (let ((commit "77c9e3a67cd7c2fca48a4292dad25a5429872f95")
+        (revision "0"))
+    (package
+      (name "efi_analyzer")
+      (version (git-version "0.0.0" revision commit))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/xypron/efi_analyzer")
+                      (commit commit)))
+                (sha256
+                 (base32 "1izdkzybqyvzpzqz6kx4j7y47j6aa2dsdrychzgs65466x1a4br1"))
+                (file-name (string-append name "-" version "-checkout"))))
+      (build-system gnu-build-system)
+      (arguments
+       `(#:phases
+         (modify-phases %standard-phases
+           (delete 'configure))
+         #:make-flags (list "prefix=" (string-append "DESTDIR=" (assoc-ref %outputs "out")))))
+    (home-page "https://github.com/xypron/efi_analyzer")
+    (synopsis "Analyze EFI binary")
+    (description
+     "The EFI Analyzer can be used to check EFI binaries and to print out header
+and section information.")
+    (license license:bsd-2))))
+
 (define-public asco
   (package
     (name "asco")
-- 
2.30.0

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

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: Vincent Legoll <vincent.legoll <at> gmail.com>
Cc: 45825-done <at> debbugs.gnu.org, guix-patches <at> gnu.org
Subject: Re: [bug#45825] [PATCH] gnu: Add efi_analyzer.
Date: Tue, 12 Jan 2021 23:02:24 +0100

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

Vincent,

Vincent Legoll 写道：
> * gnu/packages/engineering.scm (efi_analyzer): New variable.

Neat!

>  gnu/packages/engineering.scm | 27 +++++++++++++++++++++++++++

I failed to see any connection to engineering and moved this to 
efi.scm.

Don't forget to add or update your copyright line.

> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url 
> "https://github.com/xypron/efi_analyzer")
> +                      (commit commit)))
> +                (sha256
> +                 (base32 
> "1izdkzybqyvzpzqz6kx4j7y47j6aa2dsdrychzgs65466x1a4br1"))

I added a newline after ‘source’ to give this 81-character line 
some breathing room.

URLs, snippets, ... can grow wide and a few columns can save a 
pointless string-append or newlines.

> +                (file-name (string-append name "-" version 
> "-checkout"))))

This can be done with the

 (file-name (git-file-name name version))

helper.

> +      (arguments
> +       `(#:phases
> +         (modify-phases %standard-phases
> +           (delete 'configure))

Not a comment on your patch, but I added a 
‘support-cross-compilation’ phase here that replaces the 
Makefile's hard-coded ‘gcc’ so you can inspect EFI binaries on 
your cross-compiled ARM box.

> +         #:make-flags (list "prefix=" (string-append "DESTDIR=" 
> (assoc-ref %outputs "out")))))

The other way 'round: set $prefix (or the BSD variant $PREFIX) to 
the destination, and never set DESTDIR unless you're working 
around a bug in a broken build system.

DESTDIR is for distributions/builds using a temporary ‘staging’ 
destination directory.  Guix doesn't.

> +    (description
> +     "The EFI Analyzer can be used to check EFI binaries and to 
> print out header
> +and section information.")

Slash empty ‘x can y’ (or ‘x allows the user to y’) filler with 
gleeful prejudice.

Pushed with these changes as 
373f808df229c12101301b766bd1810836b98481.

Thanks!

T G-R

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

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

From: Vincent Legoll <vincent.legoll <at> gmail.com>
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 45825-done <at> debbugs.gnu.org, Guix Patches <guix-patches <at> gnu.org>
Subject: Re: [bug#45825] [PATCH] gnu: Add efi_analyzer.
Date: Tue, 12 Jan 2021 23:17:58 +0100

Hello,

On Tue, Jan 12, 2021 at 11:02 PM Tobias Geerinckx-Rice <me <at> tobias.gr> wrote:
> >  gnu/packages/engineering.scm | 27 +++++++++++++++++++++++++++
>
> I failed to see any connection to engineering and moved this to
> efi.scm.

I searched a bit (debug.scm, etc.) but failed to find
the *really* obvious one :facepalm: I chose engineering
because another rev-eng tool (radare2) was here.

> Don't forget to add or update your copyright line.

I almost always forgot to do that.

> > +      (source (origin
> > +                (method git-fetch)
> > +                (uri (git-reference
> > +                      (url
> > "https://github.com/xypron/efi_analyzer")
> > +                      (commit commit)))
> > +                (sha256
> > +                 (base32
> > "1izdkzybqyvzpzqz6kx4j7y47j6aa2dsdrychzgs65466x1a4br1"))
>
> I added a newline after ‘source’ to give this 81-character line
> some breathing room.

This one I swear I ran the etc/indent.el script on, was it in
my brain only ?

> > +                (file-name (string-append name "-" version
> > "-checkout"))))
>
> This can be done with the
>
>   (file-name (git-file-name name version))
>
> helper.

Argh, another forgetting, you probably taught me that
a few times already.

> > +      (arguments
> > +       `(#:phases
> > +         (modify-phases %standard-phases
> > +           (delete 'configure))
>
> Not a comment on your patch, but I added a
> ‘support-cross-compilation’ phase here that replaces the
> Makefile's hard-coded ‘gcc’ so you can inspect EFI binaries on
> your cross-compiled ARM box.

I saw that, nice !

> > +         #:make-flags (list "prefix=" (string-append "DESTDIR="
> > (assoc-ref %outputs "out")))))
>
> The other way 'round: set $prefix (or the BSD variant $PREFIX) to
> the destination, and never set DESTDIR unless you're working
> around a bug in a broken build system.
>
> DESTDIR is for distributions/builds using a temporary ‘staging’
> destination directory.  Guix doesn't.

OK, I hesitated between the 2 options, and failed to choose the
right one :-))

> > +    (description
> > +     "The EFI Analyzer can be used to check EFI binaries and to
> > print out header
> > +and section information.")
>
> Slash empty ‘x can y’ (or ‘x allows the user to y’) filler with
> gleeful prejudice.

Just a copy-paste from upstream (should have proof read it, though)

Thanks a lot for your patience in bearing with my half-baked
submissions.

I think I'll owe you a beer (or two).

Tchuss

-- 
Vincent Legoll

Previous Next

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