Modify

Opened 6 years ago

Closed 5 years ago

#3512 closed defect (worksforme)

On Win32 environment gitplugin cannot execute git.

Reported by: jangsuwon@… Owned by: hvr
Priority: high Component: GitPlugin
Severity: major Keywords:
Cc: thdz.x@… Trac Release: 0.11

Description

On Win32 environment gitplugin cannot execute git.

I tried running trac, git(msysgit) and gitplugin, but it doesn't work. That is because os.popen3() function does not take the parameter as a list. Git in cygwin doesn't have a problem such like this because it is emulation of unix.

So minor platform dependent fix is needed.

Attachments (2)

PyGIT_popen3_syntax_fix.patch (513 bytes) - added by rctay89@… 6 years ago.
fixes #3512
PyGIT_popen3_as_subprocess.patch (1.1 KB) - added by brentl 5 years ago.
Copied from #4227 -- really sorry for my incompetence…

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by anonymous

  • Trac Release changed from 0.10 to 0.11

comment:2 Changed 6 years ago by anonymous

  • Cc thdz.x@… added

comment:3 Changed 6 years ago by macke@…

The (later?) PyGIT at pypi (http://pypi.python.org/pypi/pygit) uses subprocess, so it might be more compatible. Need to fework git_fs.py though.

comment:4 Changed 6 years ago by rctay89@…

  • Severity changed from major to blocker

this isn't platform-dependent, it's a syntax error in python that affects all platforms.

source:/gitplugin/0.11/tracext/git/PyGIT.py@HEAD#L45

to fix,

        return os.popen3(" ".join(cmd)) # (input, output, error)

Changed 6 years ago by rctay89@…

fixes #3512

comment:5 Changed 5 years ago by thatch

  • Severity changed from blocker to major

rctay89, your patch works only if there are no spaces. Also os.popen3 does accept a list on unixes. Reducing severity since it's back to a platform issue.

comment:6 Changed 5 years ago by brentl

I have just hacked up this file is a preliminary conversion of PyGIT.py to use subprocess.Popen in place of os.popen3.

I have applied it on my home Trac system (0.12dev) and it appears to be working OK, but I don't have the in-depth knowledge to affirm that it's correct, so by all means criticise it.

I plan to test it on the full git source tree, but my resync is taking a while, so more later.

Changed 5 years ago by brentl

Copied from #4227 -- really sorry for my incompetence...

comment:7 Changed 5 years ago by brentl

After I jumped in here with both flat feet, I saw that #4227 has this fix already, and much better than mine. I couldn't delete my attachment, so I've overwritten it with that one.

Please accept my apologies for the noise.

comment:8 Changed 5 years ago by hvr

  • Resolution set to worksforme
  • Status changed from new to closed

should be fixed in current 0.11 branch, please test and confirm

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hvr. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.