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: 108: Command line tools fon't run with Cygwin git #170

Closed
wants to merge 2 commits into from

Conversation

@kevinrushforth
Copy link
Member

kevinrushforth commented Sep 30, 2019

https://bugs.openjdk.java.net/browse/SKARA-108

This is a very preliminary "proof of concept" patch that allows git jcheck to work with git from Cygwin. It is incomplete, and is missing the necessary checks to allow it to continue working with native Windows git.

@edvbld @rwestberg - I submitted it this early to get your high-level feedback on whether this support is something that you have thought about already; if not, I'd like your feedback on the approach.

The idea would be to check whether we are are using a Cygwin version of git (this should be trivial), and if so, do the conversion between the Path strings and the input/output to the git command in all of the necessary places in GitRepository (that should be the only class that needs to be modified).


Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Download

$ git fetch https://git.openjdk.java.net/skara pull/170/head:pull/170
$ git checkout pull/170

@bridgekeeper
Copy link

bridgekeeper bot commented Sep 30, 2019

👋 Welcome back kcr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

@openjdk openjdk bot added the rfr label Sep 30, 2019
@mlbridge
Copy link

mlbridge bot commented Sep 30, 2019

Webrevs

@rwestberg
Copy link
Member

rwestberg commented Oct 2, 2019

Looks like a reasonable approach to me, perhaps our most prominent Windows user @JornVernee has considered something in this area.. I usually avoid installing cygwin git to avoid issues like this.

@JornVernee
Copy link
Member

JornVernee commented Oct 2, 2019

@rwestberg I'm happily using Windows native git :)

@edvbld
Copy link
Member

edvbld commented Oct 4, 2019

Hi Kevin,

thanks for looking into this and sorry for not responding earlier. I would like to understand your use case a bit more - why are you using Cygwin git over Git for Windows? I understand that you need to use Cygwin, but why not use the Windows native git binary in Cygwin (instead of Cygwin's version of git)?

Looking at your changes I can immediately see that adding support for the Cygwin version of git will have far-reaching consequences for almost all our code, so I would really like to understand the advantages of using Cygwin git before pursuing this further.

Thanks,
Erik

@rwestberg
Copy link
Member

rwestberg commented Oct 8, 2019

Please disregard this comment, investigating a failed email delivery.

@bridgekeeper
Copy link

bridgekeeper bot commented Feb 19, 2020

@kevinrushforth This pull request has been inactive for more than 3 weeks and will be automatically closed if another 3 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@kevinrushforth
Copy link
Member Author

kevinrushforth commented Feb 20, 2020

I'm withdrawing this PR. I still think having support for Cygwin git would be a good thing. I tried native Windows git a few months back and had a variety of usage issues, so I'm sticking with Cygwin git. I can live without formal support for the Skara command line tools on Cygwin git (my temporary patch is enough to allow git jcheck to work, so this isn't blocking me).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

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