Fix get_branch() to work if the current branch shares a name with a tag #5912
Conversation
cjerdonek
force-pushed
the
improve-get-branch
branch
from
343f23b to
abb7baa
Compare
cjerdonek
added
C: vcs
|
@di Since you've written or reviewed PR's that involve using Git in different ways, I thought you might be a good person to take a look at this PR. |
cjerdonek
force-pushed
the
improve-get-branch
branch
from
abb7baa to
9eb07fa
Compare
|
@pradyunsg Would you be able to review this PR? The main change is to replace a use of Also, the new |
|
Hello! I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the |
|
Hey @cjerdonek! I'm back on OSS for a bit now; any chance you could update this PR? |
|
@cjerdonek Please rebase it. |
|
Hi @pradyunsg, sure, I should be able to get to it within the next couple days or so. |
cjerdonek
force-pushed
the
improve-get-branch
branch
from
9eb07fa to
1f22370
Compare
|
@pradyunsg This is now updated. |
cjerdonek
force-pushed
the
improve-get-branch
branch
from
1f22370 to
1e903ea
Compare
|
@pradyunsg The tests are passing now. |
src/pip/_internal/utils/misc.py
Outdated
| @@ -653,6 +653,7 @@ def call_subprocess( | |||
| show_stdout=True, # type: bool | |||
| cwd=None, # type: Optional[str] | |||
| on_returncode='raise', # type: str | |||
| returncodes=None, # type: Optional[Iterable[int]] | |||
There was a problem hiding this comment.
maybe accepted_returncodes or acceptable_returncodes ?
There was a problem hiding this comment.
@xavfernandez Thanks! To confirm, do you think these names are okay even if they are in addition to zero?
src/pip/_internal/vcs/git.py
Outdated
| args = ['rev-parse', '--abbrev-ref', 'HEAD'] | ||
| output = self.run_command(args, show_stdout=False, cwd=location) | ||
| branch = output.strip() | ||
| # The -q causes the command to exit with status code 1 instead of |
There was a problem hiding this comment.
Maybe The -q causes the command to exit with status code 1 and empty output instead of ?
There was a problem hiding this comment.
@xavfernandez Just to clarify to you, the stdout is empty either way. It's just the exit code that changes (and a message to stderr is suppressed). But will be good to add that info in the comments in any case.
|
@xavfernandez Thanks for the review! I tried addressing your comments. |
cjerdonek
force-pushed
the
improve-get-branch
branch
from
5914356 to
929c958
Compare
| @@ -17,7 +17,7 @@ | |||
|
|
|||
| if MYPY_CHECK_RUNNING: | |||
| from typing import ( # noqa: F401 | |||
| Dict, Optional, Tuple, List, Type, Any, Mapping, Text | |||
| Any, Dict, Iterable, List, Mapping, Optional, Text, Tuple, Type | |||
There was a problem hiding this comment.
This has to be automated. I'll have to check how to do this.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
lock
bot
added
the
auto-locked
This PR tightens up a helper function to address an edge case revealed by issue #5867.
The helper function returns the name of the branch that
HEADis at, ifHEADis at a branch. However, this didn't work ifHEADis at a branch whose name also matches the name of a tag.I found that the cleanest way to do this was to use a Git command that exits with status code 1 in the case of a detached
HEAD, so I added a new argument tocall_subprocess()that lets one indicate additional return codes that are okay (i.e. in addition to 0).This new argument will also help on another issue that can benefit from a similar Git command that returns 0 or 1.