[bug#33876] [PATCH]:gnu: Add flat assembler

[Kei Kebreau]

Hi Guy,

Guy fleury <address@hidden> writes:

> From 061c6b7fd399a304d0a0fc089f0020b4f9fa368c Mon Sep 17 00:00:00 2001
> From: guy fleury iteriteka <address@hidden>
> Date: Wed, 26 Dec 2018 09:39:23 +0100
> Subject: [PATCH] gnu:Add flat assembler
>
> ---
>  gnu/packages/assembly.scm | 56 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/assembly.scm b/gnu/packages/assembly.scm
> index 763d183cf..34832bad2 100644
> --- a/gnu/packages/assembly.scm
> +++ b/gnu/packages/assembly.scm
> @@ -30,7 +30,9 @@
>    #:use-module (gnu packages perl)
>    #:use-module (gnu packages texinfo)
>    #:use-module (gnu packages python)
> -  #:use-module (gnu packages xml))
> +  #:use-module (gnu packages xml)
> +  #:use-module ((guix utils)
> +                #:select (%current-system)))
>  
>  (define-public nasm
>    (package
> @@ -122,3 +124,55 @@ abstracts over the target CPU by exposing a standardized 
> RISC instruction set
>  to the clients.")
>      (home-page "https://www.gnu.org/software/lightning/";)
>      (license license:gpl3+)))
> +
> +(define-public fasm
> +  (package
> +    (name "fasm")
> +    (version "1.73.04")

Since you've submitted this patch, the newest version has become
1.73.06. This is not a difficult change at all.

> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "https://flatassembler.net/fasm-";
> +                           version ".tgz"))
> +       (sha256
> +        (base32
> +         "0y0xkf9fzcm5gklhdi61wjpd1p8islpbcnkv5k16aqci3qsd0ia1"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure) ;;no configure script used
> +         (replace 'build
> +           (lambda _
> +             ;;source code are in this directory
> +             (chdir "source/Linux/")
> +             (if (string=? ,(%current-system) "x86_64-linux") ;;if x86_86

I believe you meant "if x86_64" in this comment? Anyway, it is not
necessary, as it is easy to see what is going on here.

> +                 ;;use pre-binarie in /root directory to build itself

Instead of "/root directory", I'd say "top-level directory", and
instead of "pre-binarie" we typically refer to these as "pre-compiled
binaries".

Also, we try to avoid relying on pre-compiled binaries to build
software, but it seems that fasm is strictly self-building.

> +                 (invoke "../../fasm.x64" "fasm.asm")
> +
> +                 (invoke "../../fasm" "fasm.asm")))) ;;the same with i686
> +
> +         (replace 'install
> +           (lambda _
> +             (let ((out (assoc-ref %outputs "out")))
> +               (begin
> +                 ;;delete pre-binaries
> +                 ;;this make sure not copy them
> +                 ;;instead of the one recentently build
> +                 (for-each delete-file '("../../fasm" "../../fasm.x64"))

You shouldn't need to remove these because the recently built fasm is
clearly in the current directory by itself.

> +                 (mkdir-p (string-append out "/bin"))
> +                 ;;copy the excutable recentently build in /source/Linux/fasm
> +                 ;;the excutable name is fasm
> +                 (copy-file "fasm" (string-append out "/bin/fasm"))))))

You can use the 'install-file' procedure here instead of
'copy-file'. This way you don't have to worry about naming the
file, the directory you install to is automatically created and your
intentions are clearer. See the difference:

(mkdir-p (string-append out "/bin"))
(copy-file "fasm" (string-append out "/bin/fasm"))

vs.

(install-file "fasm" (string-append out "/bin"))

Also please make sure that the install phase returns #t so Guix doesn't
show its "phase returned `#<unspecified>'" warning.

> +
> +         (delete 'check)))) ;;no test

Instead of deleting the check phase manually, you can use #f as an
argument to the keyword #:tests?. While you do that, also use #f as an
argument to the keyword #:strip-binaries? because strip reports that
fasm's binary doesn't have sections in it, so stripping the debug
section from the binary would have no effect.

(arguments
 `(#:tests? #f ; no tests
   #:strip-binaries? #f ; fasm has no sections.
   #:phases ...

> +    ;;support only intel x86 family processors
> +    (supported-systems '("x86_64-linux" "i686-linux"))
> +    (synopsis "Assembler for x86 processors")
> +    (description
> +     "FASM is a assembler that supports x86, and IA-64 Intel architectures.It
> +does multiple passes to optimize machine code.It have macro abilities and 
> focus on

We like to keep our line lengths below 80 characters, and it is easily
fixed in this case by moving "on" to the next line.

> +operating system portability.")
> +    (home-page "https://flatassembler.net/";)
> +    (license license:bsd-2)))
> +

Please reply with a patch with the above corrections. Thanks for
packaging this!