Skip to content
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

[3.11] gh-101100: Clean up Doc/c-api/exceptions.rst and Doc/c-api/sys.rst (#114825) #115311

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

smontanaro
Copy link
Contributor

@smontanaro smontanaro commented Feb 11, 2024

sigh Let's try this again against the proper branch.


📚 Documentation preview 📚: https://cpython-previews--115311.org.readthedocs.build/

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news labels Feb 11, 2024
@AlexWaygood AlexWaygood changed the title [3.11] Clean up Doc/c-api/exceptions.rst and Doc/c-api/sys.rst #114825 (gh-101100) [3.11] gh-101100: Clean up Doc/c-api/exceptions.rst and Doc/c-api/sys.rst (#114825) Feb 11, 2024
@AlexWaygood
Copy link
Member

Very close ;)

The GitHub issue needs to be used as a prefix to the PR title, and the PR you're backporting needs to be a suffix in brackets to the PR title. Only then will Bedevere correctly recognise which PR you're trying to backport :)

I've fixed up the title for you :)

@smontanaro
Copy link
Contributor Author

@AlexWaygood While I have your attention, can you explain how to fix the target branch? I screwed up on this again, couldn't see how to change main to 3.11 and wound up simply closing the PR, then redoing the process.

@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 11, 2024

@AlexWaygood While I have your attention, can you explain how to fix the target branch? I screwed up on this again, couldn't see how to change main to 3.11 and wound up simply closing the PR, then redoing the process.

First click the "edit" button at the top:


9FB2269E-4E1D-4064-8589-B972FA76D7F5


Then toggle the "base" dropdown menu that appears here:


C252EC26-0964-4C7D-BB7C-1F3DD214230B


@smontanaro
Copy link
Contributor Author

Thanks! I thought "edit" only pertained to the title (given its placement).

@AlexWaygood
Copy link
Member

Thanks! I thought "edit" only pertained to the title (given its placement).

Yeah, definitely not an intuitive UI from github there :)

@serhiy-storchaka
Copy link
Member

What do you do to get such weird result?

Here is my recipe.

  • Install cherry_picker.
  • Run the command suggested by @miss-islington-app.
  • Resolve conflicts by editing files and add the edited files using git add, until git diff show nothing. You can also rename/move or remove files if needed. Do not commit!
  • Run cherry_picker --continue. It will open a browser page, you simply need to confirm the creation of a new PR. You can edit the title and message if needed, I do this if the title was split or formatting was broken. No need to change the branch or whatever, it should already be correct.
  • Enjoy.
  • If you make some mistakes during resolving conflicts, you can run cherry_picker --abort and start from the beginning (from runing the command suggested by @miss-islington-app).

It is better to have several workspaces for different versions (git worktree add ../cpython3.12 3.12, etc) and work with backports only in the corresponding directory. So you will not need to rebuild Python from scratch after changing branch.

Doc/c-api/sys.rst Outdated Show resolved Hide resolved
@smontanaro
Copy link
Contributor Author

There are several places where you manually need to get things right. I'm having trouble getting everything right throughout the full lifetime of a PR. I do have main, 3.12 & 3.11 work trees. That's where I switch to relevant branches for this stuff. So, when creating a PR against main, I work in my "cpython" sandbox, when backporting to 3.11 I use that sandbox, etc.

The other thing that's different is I simply don't work on Python day-in/day-out. If I did, I would have many more opportunities to practice this stuff and learn to make the workflow and tooling second nature. Most/all of what seasoned developers take for granted was put in place during the several (ten? fifteen?) years I wasn't actively working on Python. You had the chance to adapt incrementally as workflow changed and new tools were added. For me, it burst forth in full bloom when I returned to messing around a bit with Python after I retired. I still have the much more casual mental model of how the system worked in the "olden days"* ingrained into my mind. It's just taking time to adapt.

  • In the really olden days, @gvanrossum would drop a new release shar file on Usenet then announce he was going on holiday for a couple weeks. ;-)

@gvanrossum
Copy link
Member

Hi Skip! Here's my workflow (modern git users probably would balk at this, but it works for me).

I still mostly use a single working directory. In there I do git checkout 3.11 and then make some edits, commit, and git push me (where me is my "git remote" for my GitHub fork of cpython). That push prints a link I can copy/paste into the browser to create the PR. (Of course there's also testing etc.; ./configure and make clean followed by make -j will usually build everything.)

I suspect that your problem is that you're trying to backport something from main (or from 3.12?). For that you can use, instead of the "make some edits" step above, git cherry-pick XXXXXXX where XXXXXXX is the commit hash of the thing you're trying to backport. (You should be able to get that out of GitHub, or out of git log main or git log 3.12.) If there are merge conflicts it'll tell you and you can hopefully resolve those with your editor (after fixing, do git add <file> for the file you've merged, one file at a time, to keep control).

One more tip: there's something called ccache that can speed up builds when you switch branches a lot. It 's worth studying. Also, configure is much faster the second time if you add the -C flag (also a cache).

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Feb 11, 2024

I was just puzzled what was wrong in Skips environment or our tools that he continues to make the same mistake.

My only guess is that Skip did not use cherry_picker and made backports manually, using git cherry-pick. Am I right? Than such kind of errors would be common. I would make such errors several times per week. The cherry_picker tool saves not only time.

@smontanaro
Copy link
Contributor Author

@serhiy-storchaka Nope, I use the cherry-picker command suggested by Git. My most consistent problem with backports seems to be that I forgot to change main to whatever branch I'm trying to backport to.

I'm still confused a bit by the required contents of the title in the backport. The comments in the PR creation text area mention two different items (target branch and PR?), but when I look at the title of the PR against main, I see three items. Perhaps a concrete example in that comment would be useful.

Consider this PR's title:

gh-101100: Clean up Doc/c-api/exceptions.rst and Doc/c-api/sys.rst (#114825) #115311

There are three keys and a title. Despite my best effort, I seem to screw up transferring the relevant items to the proper slots when creating the new PR against the earlier branch. Maybe the comment should be amended to have a precise example (or construct an example from the current PR – it clearly knows what it wants where). That would avoid the mental translation step for novices like myself trying to figure out what bits go where.

@smontanaro
Copy link
Contributor Author

Also, one can't dismiss the possibility that I'm just an idiot. ;-)

@gvanrossum
Copy link
Member

gh-101100: Clean up Doc/c-api/exceptions.rst and Doc/c-api/sys.rst (#114825) #115311

This looks like you copied and pasted the PR title of gh-115311, but you left out the [3.11] prefix (which is part of the PR title, and required for backport commits), and you included the PR# of the PR you were looking at (which is not part of the title).

Here's what the keys mean:

  • The first key (gh-101100) refers to the issue that this PR addresses. It is required for cpython PRs unless they have the skip issue label. This is enforced by a CI test.

  • The second key ((#114825)) is the PR from which this was backported. This is actually part of the latter's commit title -- it is auto-generated by GitHub at the time a PR is merged. The core dev doing the merge has the freedom to change it to the preferred notation (i.e., gh-114825) but many core devs (myself included) usually don't bother.

The third key (#115311) is not actually part of the title, it's just being displayed by GitHub as the PR# of the commit you're looking at.

I hope this helps!

@hugovk
Copy link
Member

hugovk commented Feb 12, 2024

Nope, I use the cherry-picker command suggested by Git.

But I think you're using git cherry-pick directly and not this cherry_picker tool?

https://github.com/python/cherry-picker

My most consistent problem with backports seems to be that I forgot to change main to whatever branch I'm trying to backport to.

The tool takes care of this. It pops open the PR creation page and prefills the correct branch.

I'm still confused a bit by the required contents of the title in the backport.

The tool also prefills the title field.

See #114825 (comment) for an example command.

@serhiy-storchaka serhiy-storchaka merged commit 07fff60 into python:3.11 Feb 12, 2024
21 checks passed
@smontanaro
Copy link
Contributor Author

But I think you're using git cherry-pick directly and not this cherry_picker tool?

Nope:

% like cherry
/Users/skip/miniconda3/envs/python311a/bin/cherry_picker

My most consistent problem with backports seems to be that I forgot to change main to whatever branch I'm trying to backport to.

The tool takes care of this. It pops open the PR creation page and prefills the correct branch.

That seems to be where I'm going wrong. (No PR creation page was automatically opened.) Once the cherry picking is done and I've pushed the change to my fork, I go to GitHub, where it helpfully(?) tells me that pushed changes have been made and offers a handy green button to start the pull request creation process. That always starts with main as the target branch.

@hugovk
Copy link
Member

hugovk commented Feb 12, 2024

Nope:

% like cherry
/Users/skip/miniconda3/envs/python311a/bin/cherry_picker

👍

That seems to be where I'm going wrong. (No PR creation page was automatically opened.) Once the cherry picking is done and I've pushed the change to my fork, I go to GitHub, where it helpfully(?) tells me that pushed changes have been made and offers a handy green button to start the pull request creation process. That always starts with main as the target branch.

Aha! That handy green button is usually fine, but GitHub knows nothing of our intent to open a PR to another branch and defaults to the (surprise!) default branch: main.

cherry_picker attempts to open a special link that tells it which branch to target, using webbrowser.open_new_tab(url) (ref), I wonder why that's not working.

Anyway, it should also print it out the URL. Here's one of mine from today. First step:

cherry_picker 92483b21b30d451586c54dc4923665f7f7eedd7a 3.11
🐍 🍒 ⛏
Now backporting '92483b21b30d451586c54dc4923665f7f7eedd7a' into '3.11'
Error cherry-pick 92483b21b30d451586c54dc4923665f7f7eedd7a.
Auto-merging Doc/library/asyncio-protocol.rst
Auto-merging Doc/library/asyncio-subprocess.rst
Auto-merging Doc/library/msvcrt.rst
CONFLICT (content): Merge conflict in Doc/library/msvcrt.rst
Auto-merging Doc/library/multiprocessing.rst
CONFLICT (content): Merge conflict in Doc/library/multiprocessing.rst
Auto-merging Doc/library/subprocess.rst
Auto-merging Doc/whatsnew/2.6.rst
Auto-merging Doc/whatsnew/2.7.rst
Auto-merging Doc/whatsnew/3.1.rst
Auto-merging Doc/whatsnew/3.2.rst
error: could not apply 92483b21b3... gh-101100: Fix Sphinx warnings in `whatsnew/2.7.rst` and related (#115319)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".


Failed to cherry-pick 92483b21b30d451586c54dc4923665f7f7eedd7a into 3.11 ☹
... Stopping here.

To continue and resolve the conflict:
    $ cherry_picker --status  # to find out which files need attention
    # Fix the conflict
    $ cherry_picker --status  # should now say 'all conflict fixed'
    $ cherry_picker --continue

To abort the cherry-pick and cleanup:
    $ cherry_picker --abort

Then I resolved the conflicts manually, did a git add, then continued:

cherry_picker --continue
🐍 🍒 ⛏
Backport PR URL:
https://github.com/python/cpython/compare/3.11...hugovk:backport-92483b2-3.11?expand=1
branch backport-92483b2-3.11 has been deleted.

Backport PR:

[3.11] gh-101100: Fix Sphinx warnings in `whatsnew/2.7.rst` and related (GH-115319)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>.
(cherry picked from commit 92483b21b30d451586c54dc4923665f7f7eedd7a)


Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

The "Backport PR URL" has 3.11 in there to tell it to use that as the base branch.

If the URL isn't auto-opened, you can click or copy/paste it into your browser.

@smontanaro
Copy link
Contributor Author

cherry_picker attempts to open a special link that tells it which branch to target, using webbrowser.open_new_tab(url) (ref), I wonder why that's not working.

Just speculation, but maybe my BROWSER setting (open on a Mac) or that I don't use Safari (Brave instead)?

I've forked and cloned it. Will try using that version with the addition of a few prints to see if something turns up.

@smontanaro
Copy link
Contributor Author

Just speculation, but maybe my BROWSER setting (open on a Mac) or that I don't use Safari (Brave instead)?

That's not it. I run cherry_picker, then:

...
CONFLICT (content): Merge conflict in Doc/tools/.nitignore
error: could not apply 1b895914742... gh-101100: Fix dangling refs in bdb.rst (#114983)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

Failed to cherry-pick 1b895914742d20ccebd1b56b1b0936b7e00eb95e into 3.12 ☹
... Stopping here.

I go off to Emacs and fix the conflict, then go back to the shell. Seeing the hints from git cherry-pick, I am completely oblivious to the fact that I'm supposed to continue using cherry_picker for the rest of the process.

To continue and resolve the conflict:
    $ cherry_picker --status  # to find out which files need attention
    # Fix the conflict
    $ cherry_picker --status  # should now say 'all conflict fixed'
    $ cherry_picker --continue

To abort the cherry-pick and cleanup:
    $ cherry_picker --abort

So, again, pilot error.

@hugovk
Copy link
Member

hugovk commented Feb 12, 2024

Oh, now I can definitely see there's room for confusion here: cherry_picker wraps git cherry-pick, and Git is also printing out its own hints with its own git cherry-pick ... commands.

Maybe pilot error, but air traffic control isn't giving the clearest instructions...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants