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

WIP: use `cygpath.exe` to convert Windows paths and respect mount-points #639

Closed
wants to merge 6 commits into from

Conversation

@ankostis
Copy link
Contributor

@ankostis ankostis commented Jul 4, 2017

Currently the /cygdrive/ mount-point is hard-coded in GitPython's conversions git/utils.py since version 2.1.3 that Cygwin support was added. This PR purports to use Cygwin's cygpath -w | -u utility for path conversions.
An added benefit is support out-of-the-box for MSYS2 which is a Cygwin clone based on the excellent pacman package manager that (at last) mounts Windows-drives under root(/).

It is still a WorkInProgress (WIP) because although all travis TCs pass on the POSIX part, appveyor now fails on all non-Cygwin builds; prior to this PR, it was only Cygwin builds that were failing in a couple of TCs: https://ci.appveyor.com/project/ankostis/gitpython/build/1.0.443).

@ankostis ankostis self-assigned this Jul 4, 2017
ankostis added a commit to JRCSTU/allinone that referenced this pull request Jul 4, 2017
@codecov-io
Copy link

@codecov-io codecov-io commented Jul 11, 2017

Codecov Report

Merging #639 into master will decrease coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #639      +/-   ##
==========================================
- Coverage   92.57%   92.48%   -0.09%     
==========================================
  Files          61       61              
  Lines        9968     9985      +17     
==========================================
+ Hits         9228     9235       +7     
- Misses        740      750      +10
Impacted Files Coverage Δ
git/objects/submodule/util.py 75.47% <0%> (-1.72%) ⬇️
git/test/test_repo.py 95.1% <0%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf8dc25...7fd4154. Read the comment docs.

@Byron
Copy link
Member

@Byron Byron commented Sep 28, 2017

@ankostis I happen to be on a merge-spree right now and wonder if this one is ready. Also I know you could just merge yourself when ready, I am just wondering :). Thank you.

@ankostis
Copy link
Contributor Author

@ankostis ankostis commented Sep 28, 2017

Sorry, I cannot respond confidently.
I have imported this as hotfix in my code without any side-effect working on MSYS2 & Cygwin (less confident) for the last 2-3 months.

But still not all TCs pass, but have no time to fix them.
I would wait for them to be fixed; we already have 2 "worktree" TCs failing in Cygwin, so better not make it any worse than that.

Apologies for me not being available.

@Byron Byron closed this Jan 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.