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: Troy Figiel
Subject: [bug#69074] [PATCH] Add python-angr.
Date: Tue, 13 Feb 2024 12:52:41 +0100

Hi Sören,

First off, thank you for the patches! I will have a second look myself
today or tomorrow, as I was not familiar with angr and I would love to
try it out / play around with it myself.

On 2024-02-13 10:53, Sören Tempel wrote:
> 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?
> 

No worries, I'm just mentioning it for other reviewers/committers, as I
don't have commit rights myself.

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

Yes, I would cover this with a comment regarding the cyclical
dependency. In general, I prefer failing tests to be "surgically
removed" or clearly commented, since it acts as an entry point for
future developers.

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

I've had the same problem with some Go packages. The default test
timeout is set to 10 minutes and due to this, I cannot build these
packages locally. My laptop used to be considered fast :-) It seems QA
passes fine though.

I would be in favour of removing benchmark tests, but a second opinion
would be good.

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

I think it would be good to wait for feedback from others and
incorporate everything into a second patch set.

Best wishes,

Troy





reply via email to

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