search

BinaryAnalysisPlatform / qemu

Official QEMU mirror
Other
9 stars 12 forks source link

Tracewrap 8.1 #32

Closed Rot127 closed 9 months ago

Rot127 commented 1 year ago

Should be merge to tracerap-8.1 though.\, not stable-8.1

DMaroo commented 1 year ago

Should be merge to tracerap-8.1 though, not stable-8.1

I think you can do that. There is an option to change it on GitHub UI.

Rot127 commented 1 year ago

I think you can do that. There is an option to change it on GitHub UI.

Only have the option to select an already present branch.

Why can't we just have a shell script which does this? Why do we need Python? Other than the fact that it is easy to write, I don't see any other reason. But again it's just a one time effort, so we might as well not force people to run Python when this can be also done with bash.

Guess this is run during build. But nonetheless we should do it in another PR anyways.

Rot127 commented 1 year ago

I suggest to do at least the basic CI first

@XVilka Just added one. Could it be that you need to approve the run?

XVilka commented 1 year ago

I suggest to do at least the basic CI first

@XVilka Just added one. Could it be that you need to approve the run?

Probably Actions need to be enabled first. @ivg could you please do that?

Rot127 commented 1 year ago

Just figured that we won't catch build errors like the one fixed in https://github.com/BinaryAnalysisPlatform/qemu/pull/32/commits/d1fe736f3acc905b9a09f6bd493245ffa5ca4e3e if there is no code which uses tracewrap. So the CI build is only useful with an arch.

Rot127 commented 10 months ago

@ivg The tracing for Hexagon is almost done (if I do not find some edge case). But it relies on 8.1. If you find time in the next two weeks, I can open the PR for Hexagon.

ivg commented 10 months ago

@Rot127 sure, shoot it!

Rot127 commented 10 months ago

Alright. I'll wait for the merge of this so the Hexagon PR doesn't have 5k+ commits. Do you have any comments to this: https://github.com/BinaryAnalysisPlatform/qemu/issues/31?

ivg commented 10 months ago

I enabled the actions and left some comments on the action file. Please, don't hesitate to @-me as otherwise I will most likely miss any updates :)

Rot127 commented 10 months ago

Sure, I'll address them soon. Thanks

Rot127 commented 9 months ago

@ivg Please take a look again. Especially at the changes to the workflow. If you merge it, please don't forget to merge it into tracewrap-8.1

ivg commented 9 months ago

As a final question, do you prefer it to be rebased, squashed, or just merged with a merge commit?

Rot127 commented 9 months ago

@ivg I think squashed into tracewrap-8.1-base is fine.

XVilka commented 9 months ago

@ivg apparently @Rot127 asked to squash by mistake. We have fixed the history in stable-8.1 and tracewrap-8.1 branches, but now there is protected branch tracewrap-8.1-base that I cannot remove, could you please remove it?

https://github.com/BinaryAnalysisPlatform/qemu/branches

ivg commented 9 months ago

I can't also remove the protected branch (we should have the same permission btw), so I need to remove the protection before deleting it. I will remove the protection, delete it, and re-instantiate the protection back.