Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

plumbing: fix pack commands for the file client on Windows #527

Merged
merged 1 commit into from
Aug 4, 2017

Conversation

strib
Copy link
Contributor

@strib strib commented Jul 31, 2017

The default git install on Windows doesn't come with commands for receive-pack and upload-pack in the default $PATH. Instead, use --exec-path to find pack executables in that case.

@mcuadros mcuadros requested a review from smola August 1, 2017 12:27
@smola
Copy link
Collaborator

smola commented Aug 1, 2017

@strib Thanks for your contribution!

As far as I know, git does have git-receive-pack and git-upload-pack binaries in Windows, but most installations do not update the PATH to contain the git exec path. I would prefer this to be implemented without build conditionals with the following logic:

  1. If command is in path (with LookPath), then use it.
  2. Otherwise, execute git --exec-path, which prints the git exec path (e.g. /usr/lib/git-core on most Linux distros) and then use it.

This should work on a wider range of scenarios, not only Windows, but also weird Linux or macOS setups.

@strib
Copy link
Contributor Author

strib commented Aug 1, 2017

Cool thanks, I'll look doing things that way as soon as I get a chance.

Are those appveyor failures my fault, or known failures?

@mcuadros
Copy link
Contributor

mcuadros commented Aug 2, 2017

@strib No the appveyor issues are being fixed #345

strib added a commit to strib/go-git that referenced this pull request Aug 2, 2017
@codecov
Copy link

codecov bot commented Aug 2, 2017

Codecov Report

Merging #527 into master will decrease coverage by 0.65%.
The diff coverage is 63.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #527      +/-   ##
==========================================
- Coverage   78.17%   77.51%   -0.66%     
==========================================
  Files         129      129              
  Lines        9749     9777      +28     
==========================================
- Hits         7621     7579      -42     
- Misses       1302     1380      +78     
+ Partials      826      818       -8
Impacted Files Coverage Δ
plumbing/transport/file/client.go 81.15% <63.33%> (-13.97%) ⬇️
plumbing/transport/ssh/common.go 20.54% <0%> (-45.21%) ⬇️
plumbing/transport/ssh/auth_method.go 33.33% <0%> (-24.77%) ⬇️

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 b29ccd9...c128f5d. Read the comment docs.

strib added a commit to strib/go-git that referenced this pull request Aug 2, 2017
strib added a commit to strib/go-git that referenced this pull request Aug 2, 2017
strib added a commit to strib/go-git that referenced this pull request Aug 2, 2017
strib added a commit to strib/go-git that referenced this pull request Aug 2, 2017
strib added a commit to strib/go-git that referenced this pull request Aug 2, 2017
@strib
Copy link
Contributor Author

strib commented Aug 2, 2017

@smola @mcuadros this is ready for review again. I even added a test, though I can't run it on Windows due to #345. (I don't think I caused the travis failure above either, but let me know if I'm wrong.)

Thanks!

@mcuadros
Copy link
Contributor

mcuadros commented Aug 3, 2017

Great job! Can you squash the commit? And then I will merge it. Thanks

@strib
Copy link
Contributor Author

strib commented Aug 3, 2017

@mcuadros done, thanks!

The default git install on Windows doesn't come with commands for
receive-pack and upload-pack in the default $PATH.  Instead, use
--exec-path to find pack executables in that case.
@mcuadros mcuadros merged commit 80421da into src-d:master Aug 4, 2017
@strib strib deleted the win-pack-cmds branch August 5, 2017 18:33
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.

3 participants