guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#69074] [PATCH] Add python-angr.


From: jgart
Subject: [bug#69074] [PATCH] Add python-angr.
Date: Mon, 08 Jul 2024 16:29:04 -0500

> I was under the impression that I fixed the relevant linker warnings. I
> didn't fix the capstone warnings as I only added a patch for capstone
> and didn't want to refactor the existing code as part of that (i.e. the
> warnings are not introduced by my changes). For python-angr I only get
> two "line is way too long"-warnings, both caused by long not easily
> breakable strings (e.g. the checksum).
>

Hi Soeren,

> Should I refactor the capstone package as part of this patchset?

Could you send just the capstone package in a separate new ticket and CC me?

I can review capstone separately. Once that ticket is resolved we can
update this ticket and continue the reviewing here for angr with less
patches for me to review all at once. My time is limited and I think
that this will allow us to progress on this issue.

Notice that I changed the package name. The upstream is called
*demangler and not *demangle. I also added a note as to why we are not
using the PyPI source. If not using the PyPI source we should add a
comment as to why not. We prefer PyPI whenever possible for `guix
import` tool reasons.

Can you send a v2 without python-itanium-demangler in a new patch
series?

I spent some time reviewing these patches. There's definitely a lot to
look at.

I applied python-itanium-demangler in this commit:

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=789c4037947d59a7143999269791bf75436fdccd

Another thing I noticed is that we have this ticket open for pwntools:

https://issues.guix.gnu.org/61431

-;; python-pwntools requires a -rc release of unicorn

The above line was removed but this patch series leaves pwntools broken.

I think we should resolve that here.

Also, if there are versions of Python packages that are specifically
needed for angr and no other packages depend on them then I think it
would be better practice to call them python-foo-for-angr instead of
leaving a comment and using the package name python-claripy. For
example, python-claripy-for-python-angr. We have similar packages in the
guix package collection that follow such a pattern. The latest version
of python-claripy is 9.2.109 and you're packaging 9.2.46 with the
top-level variable name.

Other python team members or guix contributors feel free to comment on
this if you have additional feedback.

-- 
all the best,
jgart





reply via email to

[Prev in Thread] Current Thread [Next in Thread]