| Message ID | 20200604202236.25039-1-jsnow@redhat.com |
|---|---|
| Headers | show |
| Series | python: add mypy support to python/qemu | expand |
Am 04.06.2020 um 22:22 hat John Snow geschrieben: > Based-on: 20200604195252.20739-1-jsnow@redhat.com > > This series is extracted from my larger series that attempted to bundle > our python module as an installable module. These fixes don't require that, > so they are being sent first as I think there's less up for debate in here. > > This requires my "refactor shutdown" patch as a pre-requisite. You didn't like my previous R-b? Here's a new one. :-) Reviewed-by: Kevin Wolf <kwolf@redhat.com>
On 6/5/20 5:26 AM, Kevin Wolf wrote: > Am 04.06.2020 um 22:22 hat John Snow geschrieben: >> Based-on: 20200604195252.20739-1-jsnow@redhat.com >> >> This series is extracted from my larger series that attempted to bundle >> our python module as an installable module. These fixes don't require that, >> so they are being sent first as I think there's less up for debate in here. >> >> This requires my "refactor shutdown" patch as a pre-requisite. > > You didn't like my previous R-b? Here's a new one. :-) > > Reviewed-by: Kevin Wolf <kwolf@redhat.com> > I felt like I should address the feedback, and though I could have applied the R-B to patches I didn't change, it was ... faster to just re-send it. Serious question: How do you apply people's R-Bs to your patches? At the moment, it's pretty manually intensive for me. I use stgit and I pop all of the patches off (stg pop -n 100), and then one-at-a-time I `stg push; stg edit` and copy-paste the R-B into it. It's easier when I'm just pulling other people's patches from the ML, the patches tool handles it for me. It's updating my own patches that's a drag and prone to error. --js
Am 08.06.2020 um 17:19 hat John Snow geschrieben: > > > On 6/5/20 5:26 AM, Kevin Wolf wrote: > > Am 04.06.2020 um 22:22 hat John Snow geschrieben: > >> Based-on: 20200604195252.20739-1-jsnow@redhat.com > >> > >> This series is extracted from my larger series that attempted to bundle > >> our python module as an installable module. These fixes don't require that, > >> so they are being sent first as I think there's less up for debate in here. > >> > >> This requires my "refactor shutdown" patch as a pre-requisite. > > > > You didn't like my previous R-b? Here's a new one. :-) > > > > Reviewed-by: Kevin Wolf <kwolf@redhat.com> > > > > I felt like I should address the feedback, and though I could have > applied the R-B to patches I didn't change, it was ... faster to just > re-send it. > > Serious question: How do you apply people's R-Bs to your patches? At the > moment, it's pretty manually intensive for me. I use stgit and I pop all > of the patches off (stg pop -n 100), and then one-at-a-time I `stg push; > stg edit` and copy-paste the R-B into it. > > It's easier when I'm just pulling other people's patches from the ML, > the patches tool handles it for me. It's updating my own patches that's > a drag and prone to error. It's a manual process for me, too. Just that I don't use stgit, but 'git rebase -i'. Kevin
On 6/8/20 5:33 PM, Kevin Wolf wrote: > Am 08.06.2020 um 17:19 hat John Snow geschrieben: >> >> >> On 6/5/20 5:26 AM, Kevin Wolf wrote: >>> Am 04.06.2020 um 22:22 hat John Snow geschrieben: >>>> Based-on: 20200604195252.20739-1-jsnow@redhat.com >>>> >>>> This series is extracted from my larger series that attempted to bundle >>>> our python module as an installable module. These fixes don't require that, >>>> so they are being sent first as I think there's less up for debate in here. >>>> >>>> This requires my "refactor shutdown" patch as a pre-requisite. >>> >>> You didn't like my previous R-b? Here's a new one. :-) >>> >>> Reviewed-by: Kevin Wolf <kwolf@redhat.com> >>> >> >> I felt like I should address the feedback, and though I could have >> applied the R-B to patches I didn't change, it was ... faster to just >> re-send it. >> >> Serious question: How do you apply people's R-Bs to your patches? At the >> moment, it's pretty manually intensive for me. I use stgit and I pop all >> of the patches off (stg pop -n 100), and then one-at-a-time I `stg push; >> stg edit` and copy-paste the R-B into it. wget https://patchew.org/QEMU/${MSG_ID}/mbox git am mbox Where ${MSG_ID} is the Message-Id of the series cover letter. >> >> It's easier when I'm just pulling other people's patches from the ML, >> the patches tool handles it for me. It's updating my own patches that's >> a drag and prone to error. > > It's a manual process for me, too. Just that I don't use stgit, but > 'git rebase -i'. > > Kevin >
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > On 6/8/20 5:33 PM, Kevin Wolf wrote: >> Am 08.06.2020 um 17:19 hat John Snow geschrieben: >>> >>> >>> On 6/5/20 5:26 AM, Kevin Wolf wrote: >>>> Am 04.06.2020 um 22:22 hat John Snow geschrieben: >>>>> Based-on: 20200604195252.20739-1-jsnow@redhat.com >>>>> >>>>> This series is extracted from my larger series that attempted to bundle >>>>> our python module as an installable module. These fixes don't require that, >>>>> so they are being sent first as I think there's less up for debate in here. >>>>> >>>>> This requires my "refactor shutdown" patch as a pre-requisite. >>>> >>>> You didn't like my previous R-b? Here's a new one. :-) >>>> >>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com> >>>> >>> >>> I felt like I should address the feedback, and though I could have >>> applied the R-B to patches I didn't change, it was ... faster to just >>> re-send it. >>> >>> Serious question: How do you apply people's R-Bs to your patches? At the >>> moment, it's pretty manually intensive for me. I use stgit and I pop all >>> of the patches off (stg pop -n 100), and then one-at-a-time I `stg push; >>> stg edit` and copy-paste the R-B into it. > > wget https://patchew.org/QEMU/${MSG_ID}/mbox > git am mbox > > Where ${MSG_ID} is the Message-Id of the series cover letter. Patchew's awesomeness is still under-appreciated.
On 6/9/20 4:58 AM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@redhat.com> writes: > >> On 6/8/20 5:33 PM, Kevin Wolf wrote: >>> Am 08.06.2020 um 17:19 hat John Snow geschrieben: >>>> >>>> >>>> On 6/5/20 5:26 AM, Kevin Wolf wrote: >>>>> Am 04.06.2020 um 22:22 hat John Snow geschrieben: >>>>>> Based-on: 20200604195252.20739-1-jsnow@redhat.com >>>>>> >>>>>> This series is extracted from my larger series that attempted to bundle >>>>>> our python module as an installable module. These fixes don't require that, >>>>>> so they are being sent first as I think there's less up for debate in here. >>>>>> >>>>>> This requires my "refactor shutdown" patch as a pre-requisite. >>>>> >>>>> You didn't like my previous R-b? Here's a new one. :-) >>>>> >>>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com> >>>>> >>>> >>>> I felt like I should address the feedback, and though I could have >>>> applied the R-B to patches I didn't change, it was ... faster to just >>>> re-send it. >>>> >>>> Serious question: How do you apply people's R-Bs to your patches? At the >>>> moment, it's pretty manually intensive for me. I use stgit and I pop all >>>> of the patches off (stg pop -n 100), and then one-at-a-time I `stg push; >>>> stg edit` and copy-paste the R-B into it. >> >> wget https://patchew.org/QEMU/${MSG_ID}/mbox >> git am mbox >> >> Where ${MSG_ID} is the Message-Id of the series cover letter. > > Patchew's awesomeness is still under-appreciated. > Not for lack of appreciating patchew, but the problem with this workflow is if I have already made modifications to my patches locally, I can't use this to apply tags from upstream. It looks like I will continue to do this manually for the time being; but scripting the ability to "merge tags" from the list would be a cool trick. I'm not sure how to do it with git, though. Let's say I've got 16 patches and I've made modifications to some, but not all; so I have a branch with 16 patches ahead of origin/master. Does anyone have any cool tricks for being able to script: 1. Correlating a mailing list patch from e.g. patchew to a commit in my history, even if it's changed a little bit? (git-backport-diff uses patch names, that might be sufficient... Could use that as a starting point, at least.) 2. Obtaining the commit message of that patch? `git show -s --format=%B $SHA` ought to do it... 3. Editing that commit message? This I'm not sure about. I'd need to understand the tags on the upstream and downstream versions, merge them, and then re-write the message. Some magic with `git rebase -i` ? --js
On 6/16/20 7:58 PM, John Snow wrote: > > > On 6/9/20 4:58 AM, Markus Armbruster wrote: >> Philippe Mathieu-Daudé <philmd@redhat.com> writes: >> >>> On 6/8/20 5:33 PM, Kevin Wolf wrote: >>>> Am 08.06.2020 um 17:19 hat John Snow geschrieben: >>>>> >>>>> >>>>> On 6/5/20 5:26 AM, Kevin Wolf wrote: >>>>>> Am 04.06.2020 um 22:22 hat John Snow geschrieben: >>>>>>> Based-on: 20200604195252.20739-1-jsnow@redhat.com >>>>>>> >>>>>>> This series is extracted from my larger series that attempted to bundle >>>>>>> our python module as an installable module. These fixes don't require that, >>>>>>> so they are being sent first as I think there's less up for debate in here. >>>>>>> >>>>>>> This requires my "refactor shutdown" patch as a pre-requisite. >>>>>> >>>>>> You didn't like my previous R-b? Here's a new one. :-) >>>>>> >>>>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com> >>>>>> >>>>> >>>>> I felt like I should address the feedback, and though I could have >>>>> applied the R-B to patches I didn't change, it was ... faster to just >>>>> re-send it. >>>>> >>>>> Serious question: How do you apply people's R-Bs to your patches? At the >>>>> moment, it's pretty manually intensive for me. I use stgit and I pop all >>>>> of the patches off (stg pop -n 100), and then one-at-a-time I `stg push; >>>>> stg edit` and copy-paste the R-B into it. >>> >>> wget https://patchew.org/QEMU/${MSG_ID}/mbox >>> git am mbox >>> >>> Where ${MSG_ID} is the Message-Id of the series cover letter. >> >> Patchew's awesomeness is still under-appreciated. >> > > Not for lack of appreciating patchew, but the problem with this workflow > is if I have already made modifications to my patches locally, I can't > use this to apply tags from upstream. Does that mean you want to respin this series? Else you can consider it applied on python-next. > > It looks like I will continue to do this manually for the time being; > but scripting the ability to "merge tags" from the list would be a cool > trick. > > I'm not sure how to do it with git, though. Let's say I've got 16 > patches and I've made modifications to some, but not all; so I have a > branch with 16 patches ahead of origin/master. > > Does anyone have any cool tricks for being able to script: > > 1. Correlating a mailing list patch from e.g. patchew to a commit in my > history, even if it's changed a little bit? > > (git-backport-diff uses patch names, that might be sufficient... Could > use that as a starting point, at least.) > > 2. Obtaining the commit message of that patch? > `git show -s --format=%B $SHA` ought to do it... > > 3. Editing that commit message? This I'm not sure about. I'd need to > understand the tags on the upstream and downstream versions, merge them, > and then re-write the message. Some magic with `git rebase -i` ? > > --js >
On 16/06/20 19:58, John Snow wrote: > 1. Correlating a mailing list patch from e.g. patchew to a commit in my > history, even if it's changed a little bit? Use "git am --message-id"? > (git-backport-diff uses patch names, that might be sufficient... Could > use that as a starting point, at least.) > > 2. Obtaining the commit message of that patch? > `git show -s --format=%B $SHA` ought to do it... > > 3. Editing that commit message? This I'm not sure about. I'd need to > understand the tags on the upstream and downstream versions, merge them, > and then re-write the message. Some magic with `git rebase -i` ? "patchew apply" actually uses "git filter-branch --msg-filter" to add the tags after a successful "git am", so you can do something similar yourself. (Actually I have pending patches to patchew that switch it to server-side application of tags using the "mbox" URL that Philippe mentioned earlier, but they've been pending for quite some time now). To get the upstream tags you can use the Patchew REST API: $ MSGID=20200521153616.307100-1-stefanha@redhat.com $ curl -L https://patchew.org/api/v1/projects/by-name/QEMU/messages/$MSGID/ | jq -r '.tags[]' Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> So you'd have to take a commit, look for a Message-Id header, fetch the tags from above mentioned Patchew API URL and add the tags to the commit message. The commit message can be either emitted to stdout (and the script used with "git filter-branch)" or, for the faint of heart, the script could do a "git commit --amend" and you can use "git rebase -i --exec" to execute the script on all commits in a range. This script is for the latter option: #! /bin/bash BODY=$(git show -s --format=%B) MSGID=$(git interpret-trailers --parse <<< $BODY | sed -n 's/^Message-Id: <\(.*\)>/\1/ip') USER="$(git config user.name) <$(git config user.email)>" BODY=$(curl -L https://patchew.org/api/v1/projects/by-name/QEMU/messages/$MSGID/ | \ jq -r '.tags[]' | ( \ args=() while read x; do args+=(--trailer "$x") done git interpret-trailers \ --if-exists doNothing "${args[@]}" \ --if-exists replace --if-missing doNothing --trailer "Signed-off-by: $USER" <<< $BODY )) git commit --amend -m"$BODY" The script will also move your Signed-off-by line at the end of the commit message, this might be a problem if there is more than one such line and you want to keep them all. Paolo
On 6/16/20 11:33 PM, Philippe Mathieu-Daudé wrote: > Does that mean you want to respin this series? > Else you can consider it applied on python-next. No no - you're all set. Just taking the opportunity to discuss tooling that I find cumbersome at the moment. Thank you Philippe :) --js
On 6/4/20 10:22 PM, John Snow wrote: > Based-on: 20200604195252.20739-1-jsnow@redhat.com > > This series is extracted from my larger series that attempted to bundle > our python module as an installable module. These fixes don't require that, > so they are being sent first as I think there's less up for debate in here. > [...] > > John Snow (16): > python/qmp.py: Define common types > iotests.py: use qemu.qmp type aliases > python/qmp.py: re-absorb MonitorResponseError > python/qmp.py: Do not return None from cmd_obj > python/qmp.py: add casts to JSON deserialization > python/qmp.py: add QMPProtocolError > python/machine.py: Fix monitor address typing > python/machine.py: reorder __init__ > python/machine.py: Don't modify state in _base_args() > python/machine.py: Handle None events in events_wait > python/machine.py: use qmp.command > python/machine.py: Add _qmp access shim > python/machine.py: fix _popen access > python/qemu: make 'args' style arguments immutable > iotests.py: Adjust HMP kwargs typing > python/qemu: Add mypy type annotations > > python/qemu/accel.py | 8 +- > python/qemu/machine.py | 286 ++++++++++++++++++++-------------- > python/qemu/qmp.py | 111 +++++++++---- > python/qemu/qtest.py | 53 ++++--- > scripts/render_block_graph.py | 7 +- > tests/qemu-iotests/iotests.py | 11 +- > 6 files changed, 298 insertions(+), 178 deletions(-) > Thanks, applied to my python-next tree: https://gitlab.com/philmd/qemu/commits/python-next
On 6/17/20 1:18 PM, Philippe Mathieu-Daudé wrote: > On 6/4/20 10:22 PM, John Snow wrote: >> Based-on: 20200604195252.20739-1-jsnow@redhat.com >> >> This series is extracted from my larger series that attempted to bundle >> our python module as an installable module. These fixes don't require that, >> so they are being sent first as I think there's less up for debate in here. >> > [...] >> >> John Snow (16): >> python/qmp.py: Define common types >> iotests.py: use qemu.qmp type aliases >> python/qmp.py: re-absorb MonitorResponseError >> python/qmp.py: Do not return None from cmd_obj >> python/qmp.py: add casts to JSON deserialization >> python/qmp.py: add QMPProtocolError >> python/machine.py: Fix monitor address typing >> python/machine.py: reorder __init__ >> python/machine.py: Don't modify state in _base_args() >> python/machine.py: Handle None events in events_wait >> python/machine.py: use qmp.command >> python/machine.py: Add _qmp access shim >> python/machine.py: fix _popen access >> python/qemu: make 'args' style arguments immutable >> iotests.py: Adjust HMP kwargs typing >> python/qemu: Add mypy type annotations >> >> python/qemu/accel.py | 8 +- >> python/qemu/machine.py | 286 ++++++++++++++++++++-------------- >> python/qemu/qmp.py | 111 +++++++++---- >> python/qemu/qtest.py | 53 ++++--- >> scripts/render_block_graph.py | 7 +- >> tests/qemu-iotests/iotests.py | 11 +- >> 6 files changed, 298 insertions(+), 178 deletions(-) >> > > Thanks, applied to my python-next tree: > https://gitlab.com/philmd/qemu/commits/python-next > Awesome, thanks!
Based-on: 20200604195252.20739-1-jsnow@redhat.com This series is extracted from my larger series that attempted to bundle our python module as an installable module. These fixes don't require that, so they are being sent first as I think there's less up for debate in here. This requires my "refactor shutdown" patch as a pre-requisite. v3: 001/16:[----] [--] 'python/qmp.py: Define common types' 002/16:[----] [--] 'iotests.py: use qemu.qmp type aliases' 003/16:[----] [--] 'python/qmp.py: re-absorb MonitorResponseError' 004/16:[----] [--] 'python/qmp.py: Do not return None from cmd_obj' 005/16:[0002] [FC] 'python/qmp.py: add casts to JSON deserialization' 006/16:[----] [--] 'python/qmp.py: add QMPProtocolError' 007/16:[----] [-C] 'python/machine.py: Fix monitor address typing' 008/16:[----] [--] 'python/machine.py: reorder __init__' 009/16:[----] [-C] 'python/machine.py: Don't modify state in _base_args()' 010/16:[down] 'python/machine.py: Handle None events in events_wait' 011/16:[----] [--] 'python/machine.py: use qmp.command' 012/16:[----] [--] 'python/machine.py: Add _qmp access shim' 013/16:[----] [--] 'python/machine.py: fix _popen access' 014/16:[----] [--] 'python/qemu: make 'args' style arguments immutable' 015/16:[----] [--] 'iotests.py: Adjust HMP kwargs typing' 016/16:[0007] [FC] 'python/qemu: Add mypy type annotations' 005: Removed a cast, per Kevin Wolf's tip 010: Renamed with correct function name; Rewrote docstring and added comments 016: Use SocketAddrT instead of Union[Tuple[str,str],str] "v2": - This version supports iotests 297 - Many patches merged by Phil are removed - Replaces iotests.py type aliases with centralized ones (See patch 2) - Imports etc are reworked to use the non-installable package layout instead. (Mostly important for patch 3) Testing this out: - You'll need Python3.6+ - I encourage you to use a virtual environment! - You don't necessarily need these exact versions, but I didn't test the lower bounds, use older versions at your peril: - pylint==2.5.0 - mypy=0.770 - flake8=3.7.8 > cd ~/src/qemu/python/ > flake8 qemu > mypy --strict qemu > cd qemu > pylint *.py These should all 100% pass. --- Open RFC: What's the right way to hook this into make check to prevent regressions? John Snow (16): python/qmp.py: Define common types iotests.py: use qemu.qmp type aliases python/qmp.py: re-absorb MonitorResponseError python/qmp.py: Do not return None from cmd_obj python/qmp.py: add casts to JSON deserialization python/qmp.py: add QMPProtocolError python/machine.py: Fix monitor address typing python/machine.py: reorder __init__ python/machine.py: Don't modify state in _base_args() python/machine.py: Handle None events in events_wait python/machine.py: use qmp.command python/machine.py: Add _qmp access shim python/machine.py: fix _popen access python/qemu: make 'args' style arguments immutable iotests.py: Adjust HMP kwargs typing python/qemu: Add mypy type annotations python/qemu/accel.py | 8 +- python/qemu/machine.py | 286 ++++++++++++++++++++-------------- python/qemu/qmp.py | 111 +++++++++---- python/qemu/qtest.py | 53 ++++--- scripts/render_block_graph.py | 7 +- tests/qemu-iotests/iotests.py | 11 +- 6 files changed, 298 insertions(+), 178 deletions(-)