I'm filing this as a bug report since I couldn't use the module without hard coding the path to git (in my case /usr/local/git/bin/git) into the module. This is a pattern that other modules such as imagecache (and it's imagmagick/convert support) have. There's no reason it can't try to find the default on install and configure that, but it'd should really be able to be overridden by the user.

CommentFileSizeAuthor
#6 0001-feature-1001668-Path-to-git-binary-should-be-user-co-v3.patch11.17 KBmarvil07
#5 1001668-Path-to-git-binary-should-be-user-co-5.patch11.53 KBmikey_p
#4 0001-feature-1001668-Path-to-git-binary-should-be-user-co.patch9.47 KBmarvil07

Comments

marvil07’s picture

marvil07 commented
Issue tags: +git phase 2

tagging

mikey_p’s picture

mikey_p commented
Priority: Major » Critical

I'm elevating this to critical, most of the new contributors that I've tried to get setup are blocked on this, and it's very hard for them to even describe what's happening and pinpoint the actual error.

marvil07’s picture

marvil07 commented
Assigned: Unassigned » marvil07

let's try

marvil07’s picture

marvil07 commented
Category: bug » feature
Status: Active » Needs review
StatusFileSize
new 0001-feature-1001668-Path-to-git-binary-should-be-user-co.patch9.47 KB

Ok, here a patch :-)

My only two concerns ar:

  • Now that git binary is configurable, do we need to provide better feedback if the value set is not really a git binary?
  • Am I doing enough for validating malicious input on exec() calls?
mikey_p’s picture

mikey_p commented
StatusFileSize
new 1001668-Path-to-git-binary-should-be-user-co-5.patch11.53 KB

I added a little validation, I think generally, the escaping looks fine, and this patch works great for me.

marvil07’s picture

marvil07 commented
StatusFileSize
new 0001-feature-1001668-Path-to-git-binary-should-be-user-co-v3.patch11.17 KB

Thanks for the changes :-)

One little addition: Add special validation when using 'git'(default value for the git binary path) as git binary.

I think this is ready.

sdboyer’s picture

sdboyer commented
Issue tags: +git sprint 7

+1

sdboyer’s picture

sdboyer commented
Status: Needs review » Reviewed & tested by the community
marvil07’s picture

marvil07 commented
Title: Path to git binary should be user configurable » Configurable path for the git binary
Status: Reviewed & tested by the community » Fixed
Issue tags: -git sprint 7

After asking sdboyer opinion on IRC, I committed this.

mikey_p’s picture

mikey_p commented
Issue tags: +git sprint 7

Adding back issue tag

Status: Fixed » Closed (fixed)
Issue tags: -git phase 2, -git sprint 7

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +git phase 2, +git sprint 7

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.

  • Commit f77b5f4 on 6.x-2.x, fix-invalid-default-branches, fullsync-memory by marvil07: 
    feature #1001668 by marvil07, mikey_p: Configurable path for the git...