Questions about cherry-picker from Windows never-venv user

[terryjreedy]

Reading the readme.rst displayed on https://github.com/python/core-workflow/tree/master/cherry_picker, I have questions and comments that I hope lead to improved instructions. I am on Windows and have never used venv.

A. The instructions are for *nix/bash and will not work as given on Windows. Has cherry-picker been successfully run on Windows?

Comments on individual setup lines:
1,2: okay.

3. Replace the non-existent 'python3' with 'py -3.6'. (Unlike 'python3', this is guaranteed to run 3.6 or fail.) I gather that this creates a venv directory called 'venv', which I presume is git-ignored.

4. Replace 'source ...' with 'venv\Scripts\activate.bat'. The venv doc says that 'activate' is not necessary but add the 'binary' directory (venv/bin or venv/Scripts) to the the path. It appears to also change the shell prompt.

5. This does not have the (venv) prompt. Is it just missing? Or is there a missing deactivate command? In other words, is line 7 supposed to be executed in or out of the venv. If the latter, should it not be moved up before line 3?

C. Am I correct in thinking that the setup lines (other than activate) are 1-time only, or do any have to be repeated to do further cherry-picks?

D. How does one cherry-pick multiple PRs for an issue into one backport PR? This should usually be much easier than doing multiple backports, as the backport for each dependent PR needs to wait for the previous one to be tested and merged.

[Mariatta]

Thanks for these questions, @terryjreedy
I only have access to a Mac OS machine, I'll have to rely on Windows users to report problems and help improve the instructions.

I don't have experience using venv in Windows, perhaps others can help answer these questions?

Am I correct in thinking that the setup lines (other than activate) are 1-time only, or do any have to be repeated to do further cherry-picks?

Correct. Setup lines are 1 time only. You should activate venv before using cherry_picker.py

How does one cherry-pick multiple PRs for an issue into one backport PR? This should usually be much easier than doing multiple backports, as the backport for each dependent PR needs to wait for the previous one to be tested and merged.

Use --no-push option, do additional git cherry-pick -x, and --continue option when you're ready to create the backport PR

example:

cherry_picker <firstcommithash> 3.6 --no-push
git cherry-pick -x <secondcommithash>
git cherry-pick -x <thirdcommithash>
# test / resolve conflict as necessary
cherry_picker --continue

I guess it should be mentioned in the readme that when using --no-push it can only backport one branch at a time. if multiple branches are passed, only the most recent maintenance branch will be backported.

eg if you do

cherry_picker <commithash> 2.7 3.6 3.5 --no-push

It only creates the backport branch for 3.6.

[brettcannon]

@terryjreedy

3. Yes
4. Yes
   5 Initially left out for brevity, but it looks like someone updated the README to have the prompt

[terryjreedy]

https://github.com/python/core-workflow/blob/master/cherry_picker/readme.rst still has line 7 "$ git remote... with (venv) missing. Anyway, I now know to not deactivate before entering this.

I am trying, initially, to backport PR 1641 of https://bugs.python.org/issue30303 to 3.6. I called the clone 'back' instead of 'core-workflow.

f:\dev>git clone https://github.com/python/core-workflow.git back
Cloning into 'back'...
f:\dev>cd back

f:\dev\back>py -3.6 -m venv venv

f:\dev\back>venv\Scripts\activate.bat

(venv) f:\dev\back>python -m pip install wheel
...
Successfully installed wheel-0.29.0

(venv) f:\dev\back>python -m pip install --upgrade .
Processing f:\dev\back
...
Successfully installed appdirs-1.4.2 cherry-picker-0.0.0 click-6.7 colorama-0.3.9 coverage-4.3.4 packaging-16.8 py-1.4.33 pyparsing-2.1.10 pytest-3.0.7 pytest-cov-2.4.0 pytest-mock-1.6.0 requests-2.13.0 setuptools-36.0.1 six-1.10.0

(venv) f:\dev\back>git remote add upstream https://github.com/python/cpython.git

At step 8, cherry_picker, I got an exception.

(venv) f:\dev\back>cherry_picker 295304d412700cc6621bb592109fa42249a9dcdb 3.6
🐍 🍒 ⛏
Traceback (most recent call last):
File "f:\dev\back\venv\Scripts\cherry_picker-script.py", line 11, in
load_entry_point('cherry-picker==0.0.0', 'console_scripts', 'cherry_picker')()
File "f:\dev\back\venv\lib\site-packages\click\core.py", line 722, in call
return self.main(*args, **kwargs)
File "f:\dev\back\venv\lib\site-packages\click\core.py", line 697, in main
rv = self.invoke(ctx)
File "f:\dev\back\venv\lib\site-packages\click\core.py", line 895, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "f:\dev\back\venv\lib\site-packages\click\core.py", line 535, in invoke
return callback(*args, **kwargs)
File "f:\dev\back\venv\lib\site-packages\cherry_picker\cherry_picker.py", line 281, in cherry_pick_cli
os.chdir(os.path.join(current_dir, 'cpython'))
FileNotFoundError: [WinError 2] The system cannot find the file specified: 'f:\dev\back\cpython'

I have no idea why there should be a cpython here. Here is part of what there is.

(venv) f:\dev\back>dir
Volume in drive F is Fdrive
Volume Serial Number is EEF7-AA3D
Directory of f:\dev\back
06/08/2017 07:46 PM

.
06/08/2017 07:46 PM ..
06/08/2017 07:45 PM 1,106 .gitignore
06/08/2017 07:45 PM 166 .travis.yml
06/08/2017 07:45 PM cherry_picker
06/08/2017 07:45 PM 11,356 LICENSE
06/08/2017 07:45 PM 459 README.md
06/08/2017 07:45 PM 28 setup.cfg
06/08/2017 07:45 PM 960 setup.py
06/08/2017 07:47 PM venv
6 File(s) 14,075 bytes
4 Dir(s) 1,561,688,424,448 bytes free

(venv) f:\dev\back>dir venv
Volume in drive F is Fdrive
Volume Serial Number is EEF7-AA3D
Directory of f:\dev\back\venv
06/08/2017 07:47 PM

.
06/08/2017 07:47 PM ..
06/08/2017 07:46 PM Include
06/08/2017 07:46 PM Lib
06/08/2017 07:47 PM 60 pip-selfcheck.json
06/08/2017 07:46 PM 84 pyvenv.cfg
06/08/2017 07:48 PM Scripts
2 File(s) 144 bytes
5 Dir(s) 1,561,688,424,448 bytes free

(venv) f:\dev\back>dir cherry_picker
Volume in drive F is Fdrive
Volume Serial Number is EEF7-AA3D
Directory of f:\dev\back\cherry_picker
06/08/2017 07:45 PM

.
06/08/2017 07:45 PM ..
06/08/2017 07:45 PM 11,631 cherry_picker.py
06/08/2017 07:45 PM 6,739 readme.rst
06/08/2017 07:45 PM 166 requirements.txt
06/08/2017 07:45 PM 3,930 test.py
06/08/2017 07:45 PM 0 init.py
06/08/2017 07:45 PM 93 main.py
6 File(s) 22,559 bytes
2 Dir(s) 1,561,688,424,448 bytes free

As near as I can tell, I followed readme.

[Mariatta]

Sorry for all the confusion with cherry_picker.

f:\dev\back>git remote add upstream https://github.com/python/cpython.git

Adding the upstream should be done in the cloned cpython directory, not in the core-workflow(back) directory.
That part of the instruction is to ensure that there is an upstream remote that points to python/cpython repo.

After activating the venv, cd to the cloned cpython repo.
Then run cherry_picker from cpython directory.

Here's an asciicast showing how I set up cherry_picker and then backport a PR.

Hope this helps.

[terryjreedy]

So the readme needs a patch ;-). Anyone running cherry_picker should already have an upstream remote in their local clone, so I would just remove that command and add a cd command.

For ease of repetitive typing, my local clone of remote 'cpython' is '3x', not 'cpython'.
(venv) f:\dev\3x>cherry_picker 295304d412700cc6621bb592109fa42249a9dcdb 3.6
seems to have worked, resulting in python/cpython#2018 .
I will try a 3-pr cherry-pick for bp0-30290 tomorrow.

[matrixise]

@terryjreedy there is a line about the upstream repo in the readme.rst, https://github.com/python/core-workflow/tree/master/cherry_picker#id2

or I didn't understand your requirement.

[Mariatta]

Yes, if everyone follows CPython devguide then they would have origin/upstream setup already :)

However, people are still free to choose their own workflow, for example @ncoghlan uses origin/pr instead of origin/upstream, and he encountered hiccups when using cherry_picker the first time.

The readme can definitely be improved, especially with instructions for Windows users.
+1 to adding cd to cpython as part of setup instruction.

[terryjreedy]

Yeah, for me, its 'cd ../3x', not 'cd/cpython' ;-). When I have a bit more experience, I will post here the Setup instructions that I wish I had been able to read.

[terryjreedy]

Next attempt failed.
(venv) f:\dev\3x>cherry_picker --continue
🐍 🍒 ⛏
Traceback (most recent call last):
File "f:\dev\back\venv\Scripts\cherry_picker-script.py", line 11, in
load_entry_point('cherry-picker==0.0.0', 'console_scripts', 'cherry_picker')()
File "f:\dev\back\venv\lib\site-packages\click\core.py", line 722, in call
return self.main(*args, **kwargs)
File "f:\dev\back\venv\lib\site-packages\click\core.py", line 697, in main
rv = self.invoke(ctx)
File "f:\dev\back\venv\lib\site-packages\click\core.py", line 895, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "f:\dev\back\venv\lib\site-packages\click\core.py", line 535, in invoke
return callback(*args, **kwargs)
File "f:\dev\back\venv\lib\site-packages\cherry_picker\cherry_picker.py", line 294, in cherry_pick_cli
cherry_picker.continue_cherry_pick()
File "f:\dev\back\venv\lib\site-packages\cherry_picker\cherry_picker.py", line 229, in continue_cherry_pick
cherry_pick_branch = get_current_branch()
File "f:\dev\back\venv\lib\site-packages\cherry_picker\cherry_picker.py", line 315, in get_current_branch
output = subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT)
File "C:\Programs\Python36\lib\subprocess.py", line 336, in check_output
**kwargs).stdout
File "C:\Programs\Python36\lib\subprocess.py", line 418, in run
output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command 'git symbolic-ref HEAD | sed 's!refs/heads/!!'' returned non-zero exit status 255.

Aside from this, I realized that cherry_picker is flawed in that it is trying to work with multiple version branches in one worktree, whereas one should have separate worktrees for each version branch one works with. The code for each version must be tested with binary for that version, which should live in the worktree for that version. A backport for 3.6 should be prepared in the 3.6 worktree, etc.

Upon seeing
(venv) f:\dev\3x>git status
On branch backport-054e091-3.6
Your branch is ahead of 'upstream/3.6' by 4 commits.
(use "git push" to publish your local commits)
nothing to commit, working tree clean

I decided to try making a PR. The result is the horribly wrong
python/cpython#2068
Someone else make one similar, so there must be a deterministic wrong way ;-).

[Mariatta]

I edited your PR by changing the base branch from master to 3.6, that seems to look better now.
I guess the error you encountered here made it unable to detect which branch you're in, so it selected the master branch.
But I haven't yet figure out why you're seeing this error 😞

[terryjreedy]

I followed "What this will do:" in the readme.
f:\dev\36>git checkout -b backport-bpo-30290-3.6 upstream/3.6
f:\dev\36>git cherry-pick -x 054e09147aaa6f61aca6cd40c7bf7ce6dc54a04b
f:\dev\36>git cherry-pick -x 5a346d5dbc1f0f70eca706a8ba19f7645bf17837
f:\dev\36>git cherry-pick -x eca7da0f13c78013b924fe7306f3e2e59c0af40b
f:\dev\36>git push origin backport-bpo-30290-3.6
Counting objects: 20, done.
Delta compression using up to 12 threads.
Compressing objects: 100% (16/16), done.
Writing objects: 100% (20/20), 5.36 KiB | 0 bytes/s, done.
Total 20 (delta 16), reused 5 (delta 4)
remote: Resolving deltas: 100% (16/16), completed with 5 local objects.
To https://github.com/terryjreedy/cpython

• [new branch] backport-bpo-30290-3.6 -> backport-bpo-30290-3.6

I then created python/cpython#2070
Do we care about the branch name, as opposed to the PR title?
Is the title OK?

f:\dev\36>git status
On branch backport-bpo-30290-3.6
Your branch is ahead of 'upstream/3.6' by 3 commits.
(use "git push" to publish your local commits)

Since I already pushed to origin, I presume this is suggesting git push (to upstream), which is the last thing I should do.

f:\dev\36>git status
On branch 3.6
Your branch is ahead of 'origin/3.6' by 20 commits.

How do I get the worktree synchronized with origin without pushing to origin?

[ncoghlan]

As far as the testing question goes: one of the assumptions of cherry_picker.py is that you'll be relying on pre-merge CI to test the backports, rather than testing them locally. Hence why it's able to just use your main working directory - it's only performing git operations, no actual builds, so it won't overwrite any previously built object files or other artifacts.

That does mean there are certain cases where it won't be appropriate (e.g. those which need to regenerate some checked in files), but it's sufficient for most documentation and pure Python changes, and even most C level changes (as long as you're not modifying inputs to Argument Clinic).

[terryjreedy]

The assumption if a backport merges and CI passes, then the result is okay does not always work. For instance, bpo-30303 has two patches to idlelib's textview and test_textview. Among other things, python/cpython#1499 added class ButtonClickTextViewTest(unittest.TestCase); python/cpython#1641 changed the name to class ButtonClickTest(unittest.TestCase). Mariatta prepared python/cpython#1916 for 1499, which I merged, and python/cpython#1866 for 1641. Because the latter was prepared without 1916 merged, the diff was quite difference; the name change became a full class addition. When I pulled 1866 into a local branch after merging 1916, it merged okay but test_textview had two copies of the class with the two different names.

At minimum, checking out 3.6 and then master in the master (3.7) worktree touches a large number of .c files. They then get uselessly re-compiled on the next rebuild, wasting yet more of my time. In my view, checkouts in each version worktree should only be for feature branches for that version and never other version branches.

[ncoghlan]

@terryjreedy Aye, that's the point I was making - it's up to the developer doing the backport to decide if we need to test the backport locally before submitting the PR.

Sometimes we're going to make mistakes (e.g. I merged a PR and backport recently that updated some Argument Clinic inputs without regenerating the affected outputs: https://bugs.python.org/issue19180#msg295524), but those mistakes then become a hint that our pre-merge CI checks could stand be improved: #118

As we iterate on those improvements, we'll eventually get to a point where our only direct involvement in backports is for us to set the label requesting the backport, and then to check the resulting PR looks sensible.

[ncoghlan]

For the "every file gets touched" problem, that sounds unusual, as I would expect the file modification dates to get set back to what they were on the original branch. If git behaves differently on Windows by default, that's useful information, and potentially a config setting folks may want to adjust if they plan to use cherry_picker.py (assuming it's exposed as a config setting).

[terryjreedy]

So far, cherry_picker has failed for me more often than not. I mentioned the problem with --continue above. For pr 2089, the problem was that there was no merge conflict to stop the commit, to allow editing. And cherry_picker, unlike git cherrypick, lacks a no-commit option. It needs one. No-push is too late. (When I did the backport without cherry_picker, I made pr 2091, but that has its own problem. I will ask about the problem with backport 2090 on core mentorship.)

[Mariatta]

@terryjreedy does this command line work on windows?
git symbolic-ref HEAD | sed 's!refs\/heads\/!!'

It's used in cherry_picker.py, and it seems to be failing according to your log above.

[zware]

Without trying it, the answer is maybe, if cherry_picker is run from Git Bash. Otherwise, sed is not available on Windows.

[Mariatta]

Thanks @zware
Any suggestion on how to make this work on Windows? 😃
(I still don't have access to any Windows machine)

[pfmoore]

I'd be inclined to rewrite it using Python (which I assume we are allowed to expect to be available?) There's no guaranteed-available equivalent to sed on Windows that I know of, and while git comes with an implementation of sed, it's not guaranteed to be on PATH and finding it would be tricky (you'd have to locate the git command on PATH and look relative to that, which would be easiest with Python, at which point you're back to why not just rewrite the pipeline in Python? Something like

out = subprocess.run(['git', 'symbolic-ref', 'HEAD'], check=True, stdout=PIPE).stdout
out = out.replace('refs/heads/', '')
print(out)

[terryjreedy]

What Zach said. Git bash requires some adjustment. It runs in a symbolic MINGW64 directory, and one has to know to turn drive letters to directory names, as in /f/dev/3x. Then shortcut keys for cut/copy/paste do not work. When I retyped your command in bash, the result was 'master' in /3x and '3.6' in /3x. So 'git checkout ' would do the right thing in each directory. I concur with Paul Moore that cherry-picker.py should be written in Python as much as possible.

Having successfully done over 5 backports Saturday without venv, I wonder why bother with it. If cherry_picker were pip installed in installed 3.6 (which one must have anyway), then 'python3/py -3.6 -m cherry_picker' would be the way to run it. If cherry_picker were a package, so that the above ran cherry_picker/__main__, then user data could be stored in the cherry_picker directory. For me, it could learn or be told that my 3.6 workspace is f:\dev\36 and cd there for 3.6 backports.

Most of the backports I did were partial 3.6 backports of idlelib changes in other people's 3.7 PRs. So I learned something about how to do them and how to cleanup and what steps might be automated. Ask if you want to add no-commit and continue after edit to cherry_picker.

[brettcannon]

The whole point of the venv suggestion is to simply avoid installing it globally. It's always optional but it can simplify things (e.g. on Linux they are cracking down on people installing globally to the system Python install and so a venv is important in that instance).

[brettcannon]

So at this point, @terryjreedy , are you just after a comment saying that the venv is optional?

[Mariatta]

In #123 I replaced the command line git symbolic-ref HEAD | sed 's!refs\/heads\/!!' with git rev-parse --abbrev-ref HEAD

This is used to get the name of the current git branch.

[terryjreedy]

I have gotten the information I need to continue working on IDLE. Thank you.

The next thing I would like is an option to request an automated backport. I know when the merge will work, and a passing test extremely likely: any backport of a idlelib code patch (there remains only one code line difference between 3.6 and 3.7). I know not to request an automated news backport (merges can be wrong even if there is no conflict). Partial backports are rare.

[brettcannon]

@terryjreedy The dream is to get it so once a PR is merged that the backport PR is automatically done for you based on the "needs backport to *" labels on the merged PR.