Skip to content

Conversation

@chfw
Copy link
Member

@chfw chfw commented Jan 28, 2019

delivers #169

@codecov-io
Copy link

codecov-io commented Jan 28, 2019

Codecov Report

Merging #198 into dev will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
tests/test_utils.py 94.91% <100%> (+0.85%) ⬆️
moban/utils.py 97.58% <100%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab77657...aad375e. Read the comment docs.

@chfw chfw requested review from CLiu13 and jayvdb January 28, 2019 22:47
lml>=0.0.9
appdirs==1.4.3
crayons
GitPython==2.1.11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

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")
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member Author

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?

Copy link
Member Author

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

Copy link
Member

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.

Copy link
Member

@jayvdb jayvdb Jan 30, 2019

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=quiet

Copy link
Member

@jayvdb jayvdb Jan 30, 2019

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.

Copy link
Member Author

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()
Copy link
Member

@jayvdb jayvdb Jan 29, 2019

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.

@chfw chfw merged commit aa06adb into dev Jan 30, 2019
@chfw chfw deleted the git-python branch February 16, 2019 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants