Subject: Re: [PATCH v6 8/8] gnu: Add imhex.

From: Sergio Pastor Pérez <[email-masked]>
Date: Wed, 19 Mar 2025 21:14:53 +0100

On 3/19/25 18:00, Sergio Pastor Pérez wrote:
> Changes from v5:
> * Outlined make-imhex-patterns.

Looks good.

> * Use keep lists rather than delete lists.

That's fine by me.

> * Unbundle jthread.

Thanks again for taking care of this.

> I haven't sorted the inputs yet, but IMHO they should be sorted.
> Same goes for the #:use-module lines, which would require sorting in an
> extra commit.

Yes. I think they should be sorted too, my bad.

> Would you like to explain this note?
> +        ;; NOTE: there is an issue with the way the test library is linked
> +        ;; with the output binaries. Tests are intrusive and should not be
> +        ;; shipped with the release, when the issue is fixed we could add a
> +        ;; phase which builds the package with the tests and runs the
> +        ;; testsuite and a second build phase which prepares the output
> +        ;; binary.
> IMHO it is the build system that should separate test-only code from installed
> code.

I added this note after the developers told me that, for the release 
build, we should not chose the cmake build type that includes the some 
extra teststing, because they affect the ImHex binary, I'm not sure if 
it was instrumentation code or something else that could hinder 
performance. To be honest, it's been a year since I made this note, so I 
don't remember the name of the `#:build-type` that we should not use. I 
think, the default one that the `cmake-build-system` chooses is fine, 
`RelWithDebInfo`.

> +(define-public imhex
 > [...]
> +      (inputs
> +       (list yara
> +             boost
> +             capstone
> +             curl
> +             edlib

I think this last patch series you've sent, v6, is missing the `edlib` 
patch so this won't build. Could you make sure before pushing that the 
`edlib` patch has been applied?

Thanks for the efforts!

Best regards,
Sergio.