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: Sören Tempel
Subject: [bug#69074] [PATCH] Add python-angr.
Date: Tue, 09 Jul 2024 18:35:43 +0200

Thanks for taking the time to look at the patchset in greater detail!
I really appreciate your feedback and have been learning a lot :)

jgart <jgart@dismail.de> wrote:
> Could you send just the capstone package in a separate new ticket and CC me?

Sure, see #72015. I can also submit each new package separately if that
easier to review for you, just let me know.

> My time is limited and I think that this will allow us to progress on
> this issue.

Sorry, I don't intend to waste your time. I thought this patchset was in
a good state already given that I already incorporated a lot of good
feedback from Troy, fixed all the linter warnings, made it pass on the
CI and also have been using this Guix package for a long time myself.
I am definitely committed and interested in improving it further.

> 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.

Thanks for pointing this out! I can add more comments. In most cases,
the PyPI source is missing files required to run the test suite.

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

Yes, will do so after the capstone patch from above is merged so that
I can also exclude that patch from the v2 of this patchset.

> I applied python-itanium-demangler in this commit:
>
> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=789c4037947d59a7143999269791bf75436fdccd

Thanks!

> 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.

python-pwntools does not build right now. Therefore, pwntools is already
in a broken state. As such, my patchset doesn't change the status quo in
this regard. My initial goal was to keep this patchset simple(r) by
focusing on angr and not including unrelated changes; hence, I didn't
touch pwntools 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.

claripy and the other packages added here can be used without angr (and
there is free software out there which does so), it's just that the
version needs to be in sync with python-angr. I would personally prefer
to stick to the PyPI name.

Sincerely,
Sören





reply via email to

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