GNU bug report logs - #41574
gnu: Add xed.

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 41574 in the body.
You can then email your comments to 41574 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: elaexuotee <at> wilsonb.com
To: guix-patches <at> gnu.org
Subject: gnu: Add xed.
Date: Thu, 28 May 2020 17:41:52 +0900

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

This patch packages up Intel's X86 Encoder and Decoder library and associated
cli tool "examples."

A few things of note:

1) The build uses Intel's custom Python build tool `mbuild' so we have to
   manually handle the main build phases. We may need to add explicit options
   to the build script invocation so that build variables (e.g. CFLAGS etc.)
   propogate correctly. These don't look to be set in the environment, so what
   variables should we pick be picking up and from where?

2) The group of tests under `tests/tests-avx512pf' seems to be failing. A user
   on the irc channel also cross-checked for me and confirmed the same. This
   program isn't actually *executing* the avx instructions, so I don't think
   the failing test are specific to the executing cpu. Anyway, I opted to leave
   this test in the source commented out.

3) The commands provided by the `out' output are pretty poorly documented and
   have dumb names. I suspose this is becase the utilities are branded as just
   "examples" of using the library. Anyway, this is a case where the only
   reasonable documentation is the source code, so I provide that for the
   utilities in the `doc' output.

4) Finally, the `devel' output supplies the library and headers proper.

5) The package name `xed' potentially collides with the package from
   http://xed.sourceforge.net/. We don't currently have the latter yet, but I
   mention this just in case there is a good way to proactively handle this up
   front.

Thoughts? I threw this together just because I wanted it myself but figured
it's worth sharing.

[0001-gnu-Add-xed.patch (text/x-patch, attachment)]

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

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

From: elaexuotee <at> wilsonb.com
To: 41574 <at> debbugs.gnu.org
Subject: Re: gnu: Add xev.
Date: Sat, 30 May 2020 13:16:34 +0900

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

Updated patch to fix three issues:

1) Change output name `doc' to `src' and install under src/<name>-<version>/,
2) Change output name `devel' to `lib', and
3) Delete extraneous *.py files from `lib' output.

However, in the course of the above, I ran into an issue with a
non-deterministic build. For now I'm sharing the current state of the patch but
am looking into fixing the determinism.

[0001-gnu-Add-xed.patch (text/x-patch, attachment)]

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

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

From: elaexuotee <at> wilsonb.com
To: 41574 <at> debbugs.gnu.org
Subject: gnu: Add intel-xev.
Date: Wed, 03 Jun 2020 19:33:25 +0900

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

This patch makes two main changes:

1) Fixes upstream's source of non-determinism!
2) Renames packages from `xed` to `intel-xed`,

along with a few other minor improvements.

[0001-gnu-Add-intel-xed.patch (text/x-patch, attachment)]

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

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

From: Marius Bakke <marius <at> gnu.org>
To: elaexuotee <at> wilsonb.com, 41574 <at> debbugs.gnu.org
Subject: Re: [bug#41574] gnu: Add intel-xev.
Date: Mon, 22 Jun 2020 22:49:38 +0200

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

elaexuotee--- via Guix-patches via <guix-patches <at> gnu.org> writes:

> This patch makes two main changes:
>
> 1) Fixes upstream's source of non-determinism!
> 2) Renames packages from `xed` to `intel-xed`,
>
> along with a few other minor improvements.
>
> From 4e0d690a702fbfc983cf2d981d4f07f1eb79ede3 Mon Sep 17 00:00:00 2001
> From: "B. Wilson" <elaexuotee <at> wilsonb.com>
> Date: Thu, 28 May 2020 07:32:28 +0900
> Subject: [PATCH] gnu: Add intel-xed.
> To: guix-patches <at> gnu.org
>
> * gnu/packages/assembly.scm (intel-xed): New variable.
> ---
>  gnu/local.mk                                  |   1 +
>  gnu/packages/assembly.scm                     | 105 +++++++++++++++++-
>  .../intel-xed-fix-nondeterminism.patch        | 100 +++++++++++++++++
>  3 files changed, 202 insertions(+), 4 deletions(-)
>  create mode 100644 gnu/packages/patches/intel-xed-fix-nondeterminism.patch

[...]

> @@ -149,14 +151,14 @@ to the clients.")
>  (define-public fasm
>    (package
>      (name "fasm")
> -    (version "1.73.24")
> +    (version "1.73.22")
>      (source
>       (origin
>         (method url-fetch)
>         (uri (string-append "https://flatassembler.net/fasm-"
>                             version ".tgz"))
>         (sha256
> -        (base32 "142vxhs8mh8isvlzq7ir0asmqda410phzxmk9gk9b43dldskkj7k"))))
> +        (base32 "1pb0rcfdsb0h89khjjrbikz5wjdllavj3ajim0rcyh7x12xr1hw5"))))
>      (build-system gnu-build-system)
>      (arguments
>       `(#:tests? #f                      ; no tests exist
> @@ -347,14 +349,14 @@ Supported architectures are:
>  (define-public xa
>    (package
>      (name "xa")
> -    (version "2.3.11")
> +    (version "2.3.10")
>      (source (origin
>                (method url-fetch)
>                (uri (string-append "https://www.floodgap.com/retrotech/xa"
>                                    "/dists/xa-" version ".tar.gz"))
>                (sha256
>                 (base32
> -                "0b81r7mvzqxgnbbmhixcnrf9nc72v1nqaw19k67221g3k561dwij"))))
> +                "0y5sd247g11jfk5msxy91hz2nhpy7smj125dzfyfhjsjnqk5nyw6"))))

Were these two downgrades intended?  I'm assuming no, since the new
package don't appear to use them.

[...]

> +(define-public intel-xed
> +  (package
> +    (name "intel-xed")
> +    (version "11.2.0")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/intelxed/xed.git")
> +             (commit "40125558530137444b4ee6fd26b445bfa105b543")))
> +       (sha256 (base32 "1jffayski2gpd54vaska7fmiwnnia8v3cka4nfyzjgl8xsky9v2s"))
> +       (file-name (git-file-name name version))
> +       (patches (search-patches "intel-xed-fix-nondeterminism.patch"))))
> +    (build-system gnu-build-system)
> +    (native-inputs
> +     `(("python-2" ,python-2)
> +       ("python-3" ,python-3)

Does it work to use 'python-wrapper' instead of providing both Python 2
and Python 3 here?

> +    (outputs '("out" "lib" "src"))

Is the src output used for other things than documentation?  If not, I
think we can drop it and let users do 'guix build --source intel-xed'
instead.  The description should be modified accordingly.

Apart from this the package LGTM.  Probably it should have:

  (supported-systems '("x86_64-linux" "i686-linux"))

too?

[...]

> diff --git a/gnu/packages/patches/intel-xed-fix-nondeterminism.patch b/gnu/packages/patches/intel-xed-fix-nondeterminism.patch
> new file mode 100644
> index 0000000000..657f7e979d
> --- /dev/null
> +++ b/gnu/packages/patches/intel-xed-fix-nondeterminism.patch
> @@ -0,0 +1,100 @@
> +diff --git a/pysrc/ild_codegen.py b/pysrc/ild_codegen.py

Can you add a short description at the top of the patch file explaining
what it does any why?

Can you send an updated patch?  Thanks!

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

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

From: elaexuotee <at> wilsonb.com
To: Marius Bakke <marius <at> gnu.org>
Cc: 41574 <at> debbugs.gnu.org
Subject: Re: [bug#41574] gnu: Add intel-xev.
Date: Tue, 23 Jun 2020 15:04:06 +0900

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

Marius,

Thanks for taking a look at this.

> Were these two downgrades intended?  I'm assuming no, since the new
> package don't appear to use them.

Definitely not. Looks like I was sloppy with a local rebase. Thanks for
catching this.

> Does it work to use 'python-wrapper' instead of providing both Python 2
> and Python 3 here?

Beautiful; 'python-wrapper' is exactly what I was looking for.

> Is the src output used for other things than documentation?  If not, I
> think we can drop it and let users do 'guix build --source intel-xed'
> instead.  The description should be modified accordingly.

Sounds emminently reasonable to me.

> Apart from this the package LGTM.  Probably it should have:
> 
>   (supported-systems '("x86_64-linux" "i686-linux"))
> 
> too?

I'm not so sure, actually. The tool and library simply facilitate translating
to/from x86 opcodes, but as far as I can tell it doesn't actually *execute* any
architecture-specific instructions.

> Can you add a short description at the top of the patch file explaining
> what it does any why?

Oh, neat. I didn't know this was possible.

> Can you send an updated patch?  Thanks!

Done!

[0001-gnu-Add-intel-xed.patch (text/x-patch, attachment)]

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

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

From: Marius Bakke <marius <at> gnu.org>
To: elaexuotee <at> wilsonb.com
Cc: 41574 <at> debbugs.gnu.org
Subject: Re: [bug#41574] gnu: Add intel-xev.
Date: Wed, 24 Jun 2020 21:55:03 +0200

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

elaexuotee <at> wilsonb.com writes:

> Marius,
>
> Thanks for taking a look at this.
>
>> Were these two downgrades intended?  I'm assuming no, since the new
>> package don't appear to use them.
>
> Definitely not. Looks like I was sloppy with a local rebase. Thanks for
> catching this.

OK.

>> Does it work to use 'python-wrapper' instead of providing both Python 2
>> and Python 3 here?
>
> Beautiful; 'python-wrapper' is exactly what I was looking for.

Great.

>> Is the src output used for other things than documentation?  If not, I
>> think we can drop it and let users do 'guix build --source intel-xed'
>> instead.  The description should be modified accordingly.
>
> Sounds emminently reasonable to me.

The new patch still has a "src" output, even though it does not seem to
use it.

>> Apart from this the package LGTM.  Probably it should have:
>> 
>>   (supported-systems '("x86_64-linux" "i686-linux"))
>> 
>> too?
>
> I'm not so sure, actually. The tool and library simply facilitate translating
> to/from x86 opcodes, but as far as I can tell it doesn't actually *execute* any
> architecture-specific instructions.

OK.

>> Can you add a short description at the top of the patch file explaining
>> what it does any why?
>
> Oh, neat. I didn't know this was possible.

Nice job on the comment.  :-)

>> Can you send an updated patch?  Thanks!
>
> Done!

Looks like I missed a couple of things in the first round, sorry about
that.  Here it comes...

> From a90ef5e79d863201d990d607c2926400654bfd9b Mon Sep 17 00:00:00 2001
> From: "B. Wilson" <elaexuotee <at> wilsonb.com>
> Date: Thu, 28 May 2020 07:32:28 +0900
> Subject: [PATCH] gnu: Add intel-xed.
> To: guix-patches <at> gnu.org
>
> * gnu/packages/assembly.scm (intel-xed): New variable.

Please also mention the new patch and change to local.mk in the commit
message.

[...]

> +(define-public intel-xed
> +  (package
> +    (name "intel-xed")
> +    (version "11.2.0")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/intelxed/xed.git")
> +             (commit "40125558530137444b4ee6fd26b445bfa105b543")))

Use the "11.2.0" tag here instead of the commit hash.

> +       (sha256 (base32 "1jffayski2gpd54vaska7fmiwnnia8v3cka4nfyzjgl8xsky9v2s"))
> +       (file-name (git-file-name name version))
> +       (patches (search-patches "intel-xed-fix-nondeterminism.patch"))))
> +    (build-system gnu-build-system)
> +    (native-inputs
> +     `(("python-wrapper" ,python-wrapper)
> +       ("tcsh" ,tcsh)
> +       ("mbuild"
> +        ,(let ((name "mbuild")
> +               (version "0.2496"))
> +           (origin
> +             (method git-fetch)
> +             (uri (git-reference
> +                   (url "https://github.com/intelxed/mbuild.git")
> +                   (commit "5304b94361fccd830c0e2417535a866b79c1c297")))
> +             (sha256
> +              (base32
> +               "0r3avc3035aklqxcnc14rlmmwpj3jp09vbcbwynhvvmcp8srl7dl"))
> +             (file-name (git-file-name name version)))))))

Can you add a comment about where you got that version number from?
Also, would it make sense to package this separately?  Can be done later
though.

> +    (outputs '("out" "lib" "src"))

As mentioned before, the 'src' output can go.

Apart from these minor issues, I think it's good to go.  \o/

Can you send an updated patch?  TIA!  :-)

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

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

From: Marius Bakke <marius <at> gnu.org>
To: elaexuotee <at> wilsonb.com
Cc: 41574 <at> debbugs.gnu.org
Subject: Re: [bug#41574] gnu: Add intel-xev.
Date: Wed, 24 Jun 2020 21:59:20 +0200

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

Marius Bakke <marius <at> gnu.org> writes:

>> +(define-public intel-xed
>> +  (package
>> +    (name "intel-xed")
>> +    (version "11.2.0")
>> +    (source
>> +     (origin
>> +       (method git-fetch)
>> +       (uri (git-reference
>> +             (url "https://github.com/intelxed/xed.git")
>> +             (commit "40125558530137444b4ee6fd26b445bfa105b543")))
>
> Use the "11.2.0" tag here instead of the commit hash.

Actually, I meant the 'version' variable here, as in (commit version).

Patch overload!

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

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

From: elaexuotee <at> wilsonb.com
To: Marius Bakke <marius <at> gnu.org>
Cc: 41574 <at> debbugs.gnu.org
Subject: Re: [bug#41574] gnu: Add intel-xev.
Date: Thu, 25 Jun 2020 11:50:48 +0900

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

> Please also mention the new patch and change to local.mk in the commit
> message.

Looks like I also forgot to mention the patch itself. Thanks.

> Use the "11.2.0" tag here instead of the commit hash.

Done. As you mentioned in a follow-up email, I replaced the commit string with
the `version' token which happily happens to match the name of the version tag
we want.

> Can you add a comment about where you got that version number from?
> Also, would it make sense to package this separately?  Can be done later
> though.

I went ahead and added a comment explaining both of these decisions. The reason
for including mbuild in this way instead of packaging it separately is simply
that it seems to be a home-grown build system just for XED (I have no idea
why). There are other projects called "mbuild" and I didn't want to pollute the
package namespace unnecessarily.

> > +    (outputs '("out" "lib" "src"))
> 
> As mentioned before, the 'src' output can go.

Argh. Completely missed that. Nice catch.

> Apart from these minor issues, I think it's good to go.  \o/
> 
> Can you send an updated patch?  TIA!  :-)

Here you go!

[0001-gnu-Add-intel-xed.patch (text/x-patch, attachment)]

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

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

From: Marius Bakke <marius <at> gnu.org>
To: elaexuotee <at> wilsonb.com
Cc: 41574-done <at> debbugs.gnu.org
Subject: Re: [bug#41574] gnu: Add intel-xev.
Date: Tue, 21 Jul 2020 23:02:52 +0200

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

elaexuotee <at> wilsonb.com writes:

>> Apart from these minor issues, I think it's good to go.  \o/
>> 
>> Can you send an updated patch?  TIA!  :-)
>
> Here you go!

Sorry for the delay, this patch got lost in my horrifying email queue.

Applied now!

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

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

From: elaexuotee <at> wilsonb.com
To: Marius Bakke <marius <at> gnu.org>
Cc: 41574-done <at> debbugs.gnu.org
Subject: Re: [bug#41574] gnu: Add intel-xev.
Date: Fri, 24 Jul 2020 15:56:33 +0900

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

> Sorry for the delay, this patch got lost in my horrifying email queue.
> 
> Applied now!

No worries. Thanks!

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

Previous Next

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