-
-
Notifications
You must be signed in to change notification settings - Fork 13
Use GitPython instead of git command #198
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
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #198 +/- ##
==========================================
+ Coverage 98.44% 98.45% +0.01%
==========================================
Files 44 44
Lines 2118 2132 +14
==========================================
+ Hits 2085 2099 +14
Misses 33 33
Continue to review full report at Codecov.
|
| lml>=0.0.9 | ||
| appdirs==1.4.3 | ||
| crayons | ||
| GitPython==2.1.11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same version as in opensuse https://build.opensuse.org/package/view_file/devel:languages:python/python-GitPython/python-GitPython.spec?expand=1 👍
Looks to be missing from debian https://packages.debian.org/stretch/python-git , but that can bw fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry ... checked Debian again, and https://tracker.debian.org/pkg/python-git is the same library
| subprocess.check_call(["git", "submodule", "update"]) | ||
| os.chdir(current_working_dir) | ||
| reporter.report_info_message("checking out submodule") | ||
| repo.git.submodule("update", "--init") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should test this on vm without git installed, to be sure it doesnt depend on git
Maybe add a new git script in PATH on travis which just does exit 2.
There might be a way to avoid this , esp. if .gitmodules doesnt exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it depends on git still but provide better wrapping over git.
https://gitpython.readthedocs.io/en/stable/intro.html:
Requirements
Python 2.7 or newer
Git 1.7.0 or newer
It should also work with older versions, but it may be that some operations involving remotes will not work as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On this basis, do we still need this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
branch parameter can be specified with or without GitPython
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitPython should default to a pure python solution, so git cli is unnecessary for most operations. but this git submodules worries me - I suspect there is no pure python implementation of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, moban will definitely still depend on git, but here is the output without git installed.
> ~/.local/bin/moban -m mobanfile
Traceback (most recent call last):
File "/home/jayvdb/.local/lib/python3.6/site-packages/git/__init__.py", line 83, in <module>
refresh()
File "/home/jayvdb/.local/lib/python3.6/site-packages/git/__init__.py", line 73, in refresh
if not Git.refresh(path=path):
File "/home/jayvdb/.local/lib/python3.6/site-packages/git/cmd.py", line 290, in refresh
raise ImportError(err)
ImportError: Bad git executable.
The git executable must be specified in one of the following ways:
- be included in your $PATH
- be set via $GIT_PYTHON_GIT_EXECUTABLE
- explicitly set via git.refresh()
All git commands will error until this is rectified.
This initial warning can be silenced or aggravated in the future by setting the
$GIT_PYTHON_REFRESH environment variable. Use one of the following values:
- quiet|q|silence|s|none|n|0: for no warning or exception
- warn|w|warning|1: for a printed warning
- error|e|raise|r|2: for a raised exception
Example:
export GIT_PYTHON_REFRESH=quiet
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/jayvdb/.local/bin/moban", line 11, in <module>
load_entry_point('moban', 'console_scripts', 'moban')()
File "/home/jayvdb/projects/moban/moban/moban/main.py", line 33, in main
count = handle_moban_file(moban_file, options)
File "/home/jayvdb/projects/moban/moban/moban/main.py", line 145, in handle_moban_file
mobanfile.handle_moban_file_v1(moban_file_configurations, options)
File "/home/jayvdb/projects/moban/moban/moban/mobanfile.py", line 56, in handle_moban_file_v1
handle_requires(requires)
File "/home/jayvdb/projects/moban/moban/moban/mobanfile.py", line 188, in handle_requires
git_clone(git_repos)
File "/home/jayvdb/projects/moban/moban/moban/utils.py", line 136, in git_clone
from git import Repo
File "/home/jayvdb/.local/lib/python3.6/site-packages/git/__init__.py", line 85, in <module>
raise ImportError('Failed to initialize: {0}'.format(exc))
ImportError: Failed to initialize: Bad git executable.
The git executable must be specified in one of the following ways:
- be included in your $PATH
- be set via $GIT_PYTHON_GIT_EXECUTABLE
- explicitly set via git.refresh()
All git commands will error until this is rectified.
This initial warning can be silenced or aggravated in the future by setting the
$GIT_PYTHON_REFRESH environment variable. Use one of the following values:
- quiet|q|silence|s|none|n|0: for no warning or exception
- warn|w|warning|1: for a printed warning
- error|e|raise|r|2: for a raised exception
Example:
export GIT_PYTHON_REFRESH=quietThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite this, there are likely some operations which will be able to be done without git-core, especially any git db operations, so there will be less subprocesses, and/or we can do more intensive git queries on existing repos after they are cloned without incurring more subprocess costs.
And there is better error handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moban can alert user when git is indeed used, which is much more friendly. will include it in another PR.
| os.chdir(local_repo_folder) | ||
| subprocess.check_call(["git", "pull"]) | ||
| repo = Repo(local_repo_folder) | ||
| repo.git.pull() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs testing with a private repo.
#207 ssh not working (unrelated to this PR)
http private works. But it is using subprocesses. Anyways, this is definately better.
delivers #169