gnu: Add xed.

  • Done
  • quality assurance status badge
Details
2 participants
  • elaexuotee
  • Marius Bakke
Owner
unassigned
Submitted by
elaexuotee
Severity
normal
E
E
elaexuotee wrote on 28 May 2020 10:41
(address . guix-patches@gnu.org)
30SSS7KMH7STU.2VZS2NNFF6QOP@wilsonb.com
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.
Attachment: 0001-gnu-Add-xed.patch
Attachment: signature.asc
E
E
elaexuotee wrote on 30 May 2020 06:16
Re: gnu: Add xev.
(address . 41574@debbugs.gnu.org)
2QH9MCU253SH7.2JDM9LTPYXMEC@wilsonb.com
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.
Attachment: 0001-gnu-Add-xed.patch
Attachment: signature.asc
E
E
elaexuotee wrote on 3 Jun 2020 12:33
gnu: Add intel-xev.
(address . 41574@debbugs.gnu.org)
338KSVUXDSMM8.2HI3H62CJZAVU@wilsonb.com
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.
Attachment: 0001-gnu-Add-intel-xed.patch
Attachment: signature.asc
M
M
Marius Bakke wrote on 22 Jun 2020 22:49
878sgebzbx.fsf@gnu.org
elaexuotee--- via Guix-patches via <guix-patches@gnu.org> writes:

Toggle quote (21 lines)
> 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@wilsonb.com>
> Date: Thu, 28 May 2020 07:32:28 +0900
> Subject: [PATCH] gnu: Add intel-xed.
> To: guix-patches@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

[...]

Toggle quote (32 lines)
> @@ -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.

[...]

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

Toggle quote (2 lines)
> + (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?

[...]

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

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAl7xGWIACgkQoqBt8qM6
VPr2qwgAz/57Htntg3yLYdBPTQPmRv4l2L0wYrZf+qSn3cZFb/CthniyKjPUApS1
s/CB0XRf3EqmNizmn3aOeaStvupFylLUWBETpWBNpC/JqAhg02Bisv8hRoT5Wya/
kmZj601shRze2s9ikhqbWYkQjqfy5WqzpQgxjYydQM0MBU+FYoyiJCOKjAhR2W/O
bM3xEDrohPvmkPHCtUQ9SiJtrn0QlVaUjcsDBoq2loVNSicq8S7lfCz83OlKtKLH
+uEUku2rzR8rUYFylQKsMDTH6vVYQjm0/565qpx30pBT10le/NauZRWWz494YQP0
+H2Tfi3RIU20J7RPxjFpfvmeVxOyPw==
=MUeP
-----END PGP SIGNATURE-----

E
E
elaexuotee wrote on 23 Jun 2020 08:04
(name . Marius Bakke)(address . marius@gnu.org)(address . 41574@debbugs.gnu.org)
337PSS1AAVGN7.2MVZ1RZ09WTJA@wilsonb.com
Marius,

Thanks for taking a look at this.

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

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

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

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

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

Toggle quote (2 lines)
> Can you send an updated patch? Thanks!

Done!
Attachment: 0001-gnu-Add-intel-xed.patch
Attachment: signature.asc
M
M
Marius Bakke wrote on 24 Jun 2020 21:55
(address . elaexuotee@wilsonb.com)(address . 41574@debbugs.gnu.org)
87o8p86xyg.fsf@gnu.org
elaexuotee@wilsonb.com writes:

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

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

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

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

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

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

Toggle quote (8 lines)
> From a90ef5e79d863201d990d607c2926400654bfd9b Mon Sep 17 00:00:00 2001
> From: "B. Wilson" <elaexuotee@wilsonb.com>
> Date: Thu, 28 May 2020 07:32:28 +0900
> Subject: [PATCH] gnu: Add intel-xed.
> To: guix-patches@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.

[...]

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

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

Toggle quote (2 lines)
> + (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! :-)
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAl7zr5cACgkQoqBt8qM6
VPp37ggAyOAPt8USV0wXsWeqUWpbcmzS98ri6GHhpigX5Kbhq3gE3eSHGFp1XcFq
+mY2HzMmiUxpbfBTPHvVAJ4uSTKePzTqefS7b8IDdJ7H+ECVQIFvReaLysu6KmRX
RpWRcm7hb97MbBDkubKEOFBo58zndRsbT8cNbP4gq48K1fQeT8IkalM+XiSjGVFQ
i2j9fDVgsAJlv8F6qdhyDWjc8PHd97dpm3+PTAwIXrTB4mWmzkPNhImc27vyIb7J
Mq7FBeRP8lf2h501ycRnYt9V8ru8al8gH5vTKsv0p6gq5c7mQto26faC6MRE1dsv
oQ9RK+038YuFE+08+xAyVBiwL4dKqw==
=kbbT
-----END PGP SIGNATURE-----

M
M
Marius Bakke wrote on 24 Jun 2020 21:59
(address . elaexuotee@wilsonb.com)(address . 41574@debbugs.gnu.org)
87lfkc6xrb.fsf@gnu.org
Marius Bakke <marius@gnu.org> writes:

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

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAl7zsJkACgkQoqBt8qM6
VPpB4ggArySKnbY06R8zKf1YrSIb847HVk6MMsSLoXSw2lFEgoVY2WgtpvC8zJgc
6RI7ewRtb2B1MymE82RXLiKsgRFUOXSjXLrQkrrdR4VHmfDywNhkgHh7JZBE8De3
u4tHXEs2KoCq1vwgEoOKA3QVC5W+eMQjnvTqfcONUwW6pI4joQ4lGziMe7S1d+MR
i1/aC7HdxXuMwVUf95uLimTDR7DLaGg4qIEQiDDWarm1Ce7/LKeq3+OnH2Li7CT6
uLlsxjJ5mZiCFrhIk1SlP9cCvxxiJu7w3MGt9CSAephHZBiSq+WyF5loJAEv+5vo
UIrd7MpNqN/i+DJB3CO/mPjWF8rlZg==
=p5EG
-----END PGP SIGNATURE-----

E
E
elaexuotee wrote on 25 Jun 2020 04:50
(name . Marius Bakke)(address . marius@gnu.org)(address . 41574@debbugs.gnu.org)
3ILYVAJYYCJDV.3HZ7KJLE03IG5@wilsonb.com
Toggle quote (3 lines)
> 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.

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

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

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

Argh. Completely missed that. Nice catch.

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

Here you go!
Attachment: 0001-gnu-Add-intel-xed.patch
Attachment: signature.asc
M
M
Marius Bakke wrote on 21 Jul 2020 23:02
(address . elaexuotee@wilsonb.com)(address . 41574-done@debbugs.gnu.org)
87r1t4fuo3.fsf@gnu.org
elaexuotee@wilsonb.com writes:

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

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAl8XV/wACgkQoqBt8qM6
VPofvggAs866rYK3N3FKJ6O76kuMWKqslaibFF5925DSj1a/5OvXiHtbnbl1CFW2
M7hRvAL/MX1I2EKQ0Z23IUOJW4FTRLNE8vuMZzl2bxi5nERQyeeom0gdMfppS0q5
M7GUo7iUD2RS+ZXWYhnZ5BaDs/fHbzPu6HlLUphVKXSxBugcT6O+I8G2Z5d3WyyK
0ag4P+9MCl0sxcj54biQZg5d1HIioEMAfv+yRdy+4aEt57JXIaTYliuuvQDjm9wS
LnH9PP8eoOCpPgYxsR8NffLG5gH3/ymk2aJLFz9RYhfnF3e2bPzn4Fv5NjicyFr2
TVQTSozx9gQ6rDwzymHC7WyHNDu1+w==
=SjCI
-----END PGP SIGNATURE-----

Closed
E
E
elaexuotee wrote on 24 Jul 2020 08:56
(name . Marius Bakke)(address . marius@gnu.org)(address . 41574-done@debbugs.gnu.org)
2HEGN68DVIUDM.2UJZX2J00IYAA@wilsonb.com
Toggle quote (4 lines)
> Sorry for the delay, this patch got lost in my horrifying email queue.
>
> Applied now!

No worries. Thanks!
Attachment: signature.asc
Closed
?