-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixed memory view render bugs. #352
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks!
@@ -65,22 +66,22 @@ class QuadRend(ByteRend): | |||
__fmt_char__ = 'Q' | |||
__pad__ = True | |||
|
|||
def isAscii(bytez): | |||
bytez = bytez.split('\x00')[0] | |||
def isAscii(bytez: bytes): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is viv moving to py3 only? otherwise, i dont think this is valid py2 syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are. I'm planning to maintain a py2 branch for a little while (here: https://github.com/vivisect/vivisect/tree/v0.x.x-support) so people can move over. I've still got at least one release planned for that to deal with the issue you pointed out in #348 (the backport for that should be fairly simple). Any more releases depends on the size of feature itself. I'm not going to backport something like the PPC branch into a py2 release. We just don't have the manpower to do so on our end.
And since py2 is already EOL'd and pip support for it is cut in 21.0 (pypa/pip#6148), we really need to move away from it. That and we just don't have the manpower to maintain a py2 branch and py3 branch for overly long. python 3 just needs to happen since we've been dragging our feet on it for years at this point.
In terms of versioning, v1.0.0 will be where python3 support begins. v0.x.x will still be python2 support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I've got some README updates and things in flight, so I'll be sure to point that stuff out in my updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imho, this is a pretty major decision to be buried within this tiny PR. there are quite a few downstream projects that rely on viv (and are stuck using py2 for that reason), so understanding that and when viv will be py3 only from now on is important news.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the main breakage happened in the py cutover branch that was up for a couple months, so I'm at least not going to blame bat-serjo for that. Any py3 breakage (or weird leftover bugs from that) is my fault.
But ultimately, I agree that a more formal-ish notice should be put out, which is why I was in the process of throwing updates/transition/py2 branch notes into the README. It's the best I've got since #vivisect is kind of a graveyard, and I have no other decent way, that I know of at least, to inform people "Hey, this is happening". I was midway through that last week, until I got pulled off to do some contract work.
This is why I got stable releases up first, so I couple point people to those to use instead of using master. So I'll point those out in the updates at least so people have a stable live branch to live off as they do their own py3 transition.
The behavior before this PR was different when `va` was `None`: vivisect#352 Go back to the past, so that capa keeps working.
the memory canvas had issues with byte/uints. Fixed.