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

_to_relative_path to support mixing slashes and backslashes #1961

Merged
merged 4 commits into from
Sep 14, 2024

Conversation

Andrej730
Copy link
Contributor

Working on Windows you sometime end up having some paths with backslashes (windows native) and some with slashes - this PR will resolve the issue using gitpython for those kind of cases (see example below). It will also fix the issues if paths contain redundant separators or "..".

import git

repo = git.Repo(r"C:\gittest")
repo.index.add(r"C:\gittest\1.txt")
# Traceback (most recent call last):
#   File "c:\second_test.py", line 5, in <module>
#     repo.index.add(r"C:/gittest/2.txt")
#   File "Python311\Lib\site-packages\git\index\base.py", line 879, in add
#     paths, entries = self._preprocess_add_items(items)
#                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "Python311\Lib\site-packages\git\index\base.py", line 672, in _preprocess_add_items
#     paths.append(self._to_relative_path(item))
#                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "Python311\Lib\site-packages\git\index\base.py", line 657, in _to_relative_path
#     raise ValueError("Absolute path %r is not in git repository at %r" % (path, self.repo.working_tree_dir))
# ValueError: Absolute path 'C:/gittest/2.txt' is not in git repository at 'C:\\gittest'

repo.index.add(r"C:/gittest/2.txt")

repo.index.commit("test")

Working on Windows you sometime end up having some paths with backslashes (windows native) and some with slashes - this PR will resolve the issue using gitpython for those kind of cases (see example below). It will also fix the issues if paths contain redundant separators or "..".

```
import git

repo = git.Repo(r"C:\gittest")
repo.index.add(r"C:\gittest\1.txt")
# Traceback (most recent call last):
#   File "c:\second_test.py", line 5, in <module>
#     repo.index.add(r"C:/gittest/2.txt")
#   File "Python311\Lib\site-packages\git\index\base.py", line 879, in add
#     paths, entries = self._preprocess_add_items(items)
#                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "Python311\Lib\site-packages\git\index\base.py", line 672, in _preprocess_add_items
#     paths.append(self._to_relative_path(item))
#                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#   File "Python311\Lib\site-packages\git\index\base.py", line 657, in _to_relative_path
#     raise ValueError("Absolute path %r is not in git repository at %r" % (path, self.repo.working_tree_dir))
# ValueError: Absolute path 'C:/gittest/2.txt' is not in git repository at 'C:\\gittest'

repo.index.add(r"C:/gittest/2.txt")

repo.index.commit("test")
```
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot - this looks like an improvement.

Could you also add a test that fails with the previous implementation?

Maybe as a question on the side: what prevents the caller from normalizing the input paths right away?

git/index/base.py Outdated Show resolved Hide resolved
@Andrej730
Copy link
Contributor Author

Could you also add a test that fails with the previous implementation?

Added.

Maybe as a question on the side: what prevents the caller from normalizing the input paths right away?

Well, normalizing the inputs is one error traceback back away indeed but usually when some method has a string-path argument user expects the method to handle any kind of paths.

@Byron
Copy link
Member

Byron commented Sep 14, 2024

Thanks for the test!

Once CI passes I think this can be merged.

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@Byron Byron merged commit 3470fb3 into gitpython-developers:main Sep 14, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants