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

["Sören Tempel" <soeren@soeren-tempel.net>]

Hi Troy,

First of all, thanks for the quick feedback. Much appreciated!

> Just a small note, I had to switch the order of python-pysmt and
> python-claripy to be able to apply this patch series. Somehow these are
> sent out of order.

Sorry about that! I guess because I send 15 emails at once, the MTA
queued some of them and then just didn't send them in the original order
(which it isn't required to). I don't think there is much I can do about
it on my end. I suppose `git-format-patch --numbered` would help?

> - python-mulpyplexer actually does have tests. The file mulpyplexer.py
>   itself is the test.

Good catch! I will just enable those [0].

> - python-rpyc seems to have an extensive test suite. Do all of them need
>   a network connection? It would be nicer to surgically remove the tests
>   that cannot be run.

Sure, we can enable parts of the test suite [1].

> - python-ailment skips both tests it runs. This might need to be looked
>   into or commented why this happens.

The first test is skipped because it needs python-pyvex, the second is
skipped because it needs python-angr. The first also indirectly depends
on angr because the VEX converter within ailment needs it. Therefore,
we cannot enable these tests as they would would result in a cyclic
dependency (python-angr <-> python-ailment). I can add a comment.

> - python-angr does not build for me, because a test is failing. I think
>   you might have some nondeterministic behaviour in your build:

The test performs benchmark using time.time() and expects a minimum
timespan to be satisfied [2]. Therefore, it depends on the current load
and the host CPU. Never failed for me, but probably good to disable it?

> - Would it make more sense to place python-archinfo and
>   python-keystone-engine under engineering.scm? The current placement
>   creates a direct import cycle engineering->python-xyz->engineering

Sure, I can move them accordingly.

> - The changes to local.mk and the patches are not mentioned in the
>   commit messages.

Good find, I will fix this in the next revision of this patchset.

P.S: Should I resend the whole patch series with the updates or should
I only resend the patches that changed due to the outlined updates? Also
let me know if I should send a revision immediately or if you want me to
wait for further feedback.

Greetings
Sören

[0]: https://github.com/nmeum/guix-symex/commit/c16f563502595bd9c17f7eb05a5c30bc7712006b
[1]: https://github.com/nmeum/guix-symex/commit/9e36fbccb633392c34216b2deab73667f7769cc1
[2]: https://github.com/angr/angr/blob/76da4fcb17efbf2373b2e7dd600b0ee578df3dd7/tests/storage/test_memory.py#L566