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

Alter rev-parse syntax to support msys git #2668

Closed
wants to merge 1 commit into from

Conversation

donkopotamus
Copy link
Contributor

On windows, msys and cygwin versions of git need braces to be escaped when they are invoked via subprocess. (This appears to be an unfortunate side effect of the machinery that converts windows paths to posix-like paths for the executable, and is usually hidden when executing from an msys based shell).

If they are not escaped, then rev^{commit] becomes a meaningless rev^commit to an msys git, resulting in an error.

In contrast, we do not want to escape the braces if passing rev^{commit} to any other form of git. We can avoid having to decide whether escaping is needed by using the alternative notation ^0.

As noted in the rev-parse documentation

A suffix ^ followed by an object type name enclosed in brace pair means dereference the object at <rev> recursively ...
<rev>^0 is a short-hand for <rev>^{commit}.

Pull Request Check List

Resolves: #2667

  • Added tests for changed code. No additional tests should be necessary
  • Updated documentation for changed code. No additional documentation necessary

On windows, msys and cygwin versions of git need braces to be escaped when they
are invoked via subprocess.  (This appears to be is a side effect of the machinery
that converts windows paths to posix-like paths for the executable).

If they are not escaped, then "rev^{commit]" becomes a meaningless "rev^commit"
to an msys git, resulting in an error.

In contrast, we do not want to escape the braces if passing "rev^{commit}" to any
other form of git.  We can avoid having to decide whether escaping is needed by
using the alternative notation "^0".
Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Thanks a lot for your contribution.

Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please rebase onto master to resolve the conflict?

@donkopotamus
Copy link
Contributor Author

@finswimmer Rebasing is no help here, all the relevant code has moved to the poetry-core repo! Have reopened this as python-poetry/poetry-core#69

@donkopotamus donkopotamus deleted the patch-2 branch August 24, 2020 11:05
Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot resolve git branch names using Windows MSYS installations of git
2 participants