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

Remove pipe-based git command line usage #2

Open
rowanj opened this Issue Sep 25, 2011 · 10 comments

Comments

Projects
None yet
4 participants
@rowanj
Owner

rowanj commented Sep 25, 2011

GitX should use libgit2 & objective-git instead of running the git command line tools and parsing the output.

The pipe APIs used are deprecated, and they interact poorly with debug tools.

@ghost ghost assigned rowanj Sep 25, 2011

@rowanj

This comment has been minimized.

Show comment
Hide comment
@rowanj

rowanj Dec 16, 2011

Owner

Xcode 4.3 beta is distributed as a plain "Xcode.app" bundle, so git isn't even accessible from the command line by default with Xcode installed, by the look of it. This bumps the priority of working on this, I'd like to have it done by the time Xcode 4.3 hits general release.

Owner

rowanj commented Dec 16, 2011

Xcode 4.3 beta is distributed as a plain "Xcode.app" bundle, so git isn't even accessible from the command line by default with Xcode installed, by the look of it. This bumps the priority of working on this, I'd like to have it done by the time Xcode 4.3 hits general release.

@rowanj

This comment has been minimized.

Show comment
Hide comment
@rowanj

rowanj Dec 21, 2011

Owner

To clarify the 'why' of this:

  • The git binary that is found is not always the same one that the user will get on the command line
  • Command responses are parsed from stdout and stderr of the binary. Not all cases are handled, and it is fragile to any message format changes or localizations.
  • It introduces a dependancy on an externally-installed program
  • Any bugs due to the version/configuration of the user's git program will be difficult to diagnose or reproduce
  • We want to be able to run out-of-the-box with current versions of Xcode, and it seems 4.3 is not installing command-line developer tools anymore
Owner

rowanj commented Dec 21, 2011

To clarify the 'why' of this:

  • The git binary that is found is not always the same one that the user will get on the command line
  • Command responses are parsed from stdout and stderr of the binary. Not all cases are handled, and it is fragile to any message format changes or localizations.
  • It introduces a dependancy on an externally-installed program
  • Any bugs due to the version/configuration of the user's git program will be difficult to diagnose or reproduce
  • We want to be able to run out-of-the-box with current versions of Xcode, and it seems 4.3 is not installing command-line developer tools anymore

rowanj added a commit that referenced this issue Apr 25, 2012

PBGitRepository: Use objective-git to look up shaForRef
Removes one count of command-line Git usage for issue #2.

Also removes the use of the ref cache in this instance. I strongly suspect the performance advantage is not significant, but it would be trivial to re-implement anyway, probably closer to the libgit2 objects that back them for quicker revalidation.

rowanj added a commit that referenced this issue Apr 25, 2012

rowanj added a commit that referenced this issue Apr 25, 2012

@rowanj

This comment has been minimized.

Show comment
Hide comment
@rowanj

rowanj Jan 22, 2013

Owner

We're going to be reliant on command-line git to some extent at least until libgit2 implements the git@ (ssh) transport.

Still, that shouldn't stop us from converting to libgit2 for local operations at least.

Owner

rowanj commented Jan 22, 2013

We're going to be reliant on command-line git to some extent at least until libgit2 implements the git@ (ssh) transport.

Still, that shouldn't stop us from converting to libgit2 for local operations at least.

@rowanj

This comment has been minimized.

Show comment
Hide comment
@rowanj

rowanj Jun 14, 2013

Owner

n.b. GitX now looks for git as xcrun git as well, for the case where Xcode is installed but the Xcode command-line tools aren't.

Owner

rowanj commented Jun 14, 2013

n.b. GitX now looks for git as xcrun git as well, for the case where Xcode is installed but the Xcode command-line tools aren't.

@jtbandes

This comment has been minimized.

Show comment
Hide comment
@jtbandes

jtbandes Jul 26, 2013

libgit2 is currently missing some features: libgit2/libgit2#1730

libgit2 is currently missing some features: libgit2/libgit2#1730

@rowanj

This comment has been minimized.

Show comment
Hide comment
@rowanj

rowanj Aug 14, 2013

Owner

@jtbandes true, but it does support pretty much everything that GitX has an interface for; and those things are coming.

I've merged a big hunk of commits to master for this recently, and would love any feedback people can add; esp. regarding loading performance or missing commits/branches.

The sort order for commits has also changed, I think for the better; but discussion is welcome.

Owner

rowanj commented Aug 14, 2013

@jtbandes true, but it does support pretty much everything that GitX has an interface for; and those things are coming.

I've merged a big hunk of commits to master for this recently, and would love any feedback people can add; esp. regarding loading performance or missing commits/branches.

The sort order for commits has also changed, I think for the better; but discussion is welcome.

rowanj added a commit that referenced this issue Aug 18, 2013

@rowanj

This comment has been minimized.

Show comment
Hide comment
@rowanj

rowanj Aug 22, 2013

Owner

Problem repo in #183 tested with acceptable performance.

Owner

rowanj commented Aug 22, 2013

Problem repo in #183 tested with acceptable performance.

rowanj added a commit that referenced this issue Sep 3, 2013

rowanj added a commit that referenced this issue Sep 3, 2013

rowanj added a commit that referenced this issue Sep 20, 2013

@sdegutis

This comment has been minimized.

Show comment
Hide comment
@sdegutis

sdegutis May 12, 2014

So apparently C is the new Unix? 😢

So apparently C is the new Unix? 😢

@capnslipp

This comment has been minimized.

Show comment
Hide comment
@capnslipp

capnslipp May 13, 2014

@sdegutis Considering the shared authors between the two, I'd say C is the old Unix.

@sdegutis Considering the shared authors between the two, I'd say C is the old Unix.

@rowanj

This comment has been minimized.

Show comment
Hide comment
@rowanj

rowanj May 16, 2014

Owner

@sdegutis If you're asking for the justification for these changes:

  • Parsing the command-line output is fragile; formats change, sort orders change, date representations are always a PITA. Profiling tools inject output into the stream, customised distributions, password agents, server messages and the like all combine to cause problems that can't be anticipated or diagnosed on test machines.
  • It's slow; and can't be meaningfully parallelised.
  • It introduces external dependencies beyond my control; bundling a version of Git introduces more problems when it interacts with the .gitconfig and repository format of the user's other tools.
  • Users commonly have multiple git versions installed (i.e. Xcode, homebrew)
  • Running shell commands from within the context of another program is a security risk
  • OS X sandboxing makes this unreliable, too; and sandboxing is a Very Good Idea for applications that by requirement access remote resources and run code in the user's security context
Owner

rowanj commented May 16, 2014

@sdegutis If you're asking for the justification for these changes:

  • Parsing the command-line output is fragile; formats change, sort orders change, date representations are always a PITA. Profiling tools inject output into the stream, customised distributions, password agents, server messages and the like all combine to cause problems that can't be anticipated or diagnosed on test machines.
  • It's slow; and can't be meaningfully parallelised.
  • It introduces external dependencies beyond my control; bundling a version of Git introduces more problems when it interacts with the .gitconfig and repository format of the user's other tools.
  • Users commonly have multiple git versions installed (i.e. Xcode, homebrew)
  • Running shell commands from within the context of another program is a security risk
  • OS X sandboxing makes this unreliable, too; and sandboxing is a Very Good Idea for applications that by requirement access remote resources and run code in the user's security context

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

PBGitRepository: Use objective-git to look up shaForRef
Removes one count of command-line Git usage for issue #2.

Also removes the use of the ref cache in this instance. I strongly suspect the performance advantage is not significant, but it would be trivial to re-implement anyway, probably closer to the libgit2 objects that back them for quicker revalidation.

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

samsonjs pushed a commit to samsonjs/gitx that referenced this issue Jul 15, 2016

ssp referenced this issue in gitx/gitx Dec 7, 2016

elia pushed a commit to elia/gitx that referenced this issue Jan 26, 2017

Merge pull request #2 from nanotech/f/binary-size-diffs
Display size changes for binary files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment