Skip to content
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.

Sign up for GitHub

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

Check root #163

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Check root #163

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
328a1b4
check if we're at root of git repo at configure
gforsyth Mar 8, 2017
eaaf727
add tests for in_git_repo
gforsyth Mar 8, 2017
b25ff66
add changelog entry
gforsyth Mar 8, 2017
b3757af
Merge branch 'master' into check_root
gforsyth Mar 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
Doctr Changelog
=================

Current
=======
- Error/warn user if ``doctr configure`` is run not at root of git repo
(:issue:`162`)

1.4.1 (2017-01-11)
==================
- Fix Travis API endpoint when checking if a repo exists. (:issue:`143`)
Expand Down
8 changes: 7 additions & 1 deletion doctr/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
from textwrap import dedent

from .local import (generate_GitHub_token, encrypt_variable, encrypt_file,
upload_GitHub_deploy_key, generate_ssh_key, check_repo_exists, GitHub_login)
upload_GitHub_deploy_key, generate_ssh_key,
check_repo_exists, GitHub_login, in_git_root)
from .travis import (setup_GitHub_push, commit_docs, push_docs,
get_current_repo, sync_from_log, find_sphinx_build_dir, run)
from . import __version__
Expand Down Expand Up @@ -176,6 +177,11 @@ def configure(args, parser):
parser.error("doctr appears to be running on Travis. Use "
"doctr configure --force to run anyway.")

if not args.force and not in_git_root():
parser.error("doctr should be run from the root directory "
"of your repository. Use doctr configure --force "
"to run anyway.")

if args.upload_key:
login_kwargs = GitHub_login()
else:
Expand Down
17 changes: 17 additions & 0 deletions doctr/local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,20 @@ def check_repo_exists(deploy_repo, service='github', *, auth=None, headers=None)
r.raise_for_status()

return r.json().get('private', False)

def in_git_root():
"""
Check if current working directory is root dir of git repo.
Return ``False`` if it isn't.
"""
repo = subprocess.run(['git', 'rev-parse', '--show-toplevel'],
Copy link
Member

Choose a reason for hiding this comment

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

Can't you just check if the current directory has a .git directory?

Copy link
Member

Choose a reason for hiding this comment

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

That wouldn't check if we are in a git repo at all, but I don't see the point have having the distinct error messages in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

My thought was that someone might want to run doctr in a subdirectory for some specific reason? I can't think of what that would be, but that was the thought.

Copy link
Member

Choose a reason for hiding this comment

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

Well it doesn't have to be at the root of the repo, unless I'm mistaken, as long as the paths to the key and the other arguments are given to it, relative to the current directory. It does need to be in the repo, as it calls git.

So I actually think it should only error when it is not in the git repo (it will error anyway, eventually, because git will complain as soon as doctr calls it), and warn, but not error when it isn't, just so it is easier for people to debug if they accidentally are cd'd to the wrong directory and are trying to figure out why doctr failed.

Copy link
Member

Choose a reason for hiding this comment

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

OTOH, maybe weird stuff will happen when checking out gh-pages if we aren't in the root (unless the directory we are in isn't tracked, or is in both branches).

stdout=subprocess.PIPE, stderr=subprocess.PIPE)
if repo.returncode == 0:
root_dir = repo.stdout.decode('utf-8').strip()
if os.getcwd() == root_dir:
return True
else:
return False
else:
raise RuntimeError('doctr should be run in the root directory of a '
'git repository')
31 changes: 30 additions & 1 deletion doctr/tests/test_local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import os
import tempfile
import subprocess

from ..local import check_repo_exists
from ..local import check_repo_exists, in_git_root

import pytest
from pytest import raises

TEST_TOKEN = os.environ.get('TESTING_TOKEN', None)
Expand Down Expand Up @@ -41,3 +44,29 @@ def test_travis_bad_repo():

def test_travis_repo_exists():
assert not check_repo_exists('drdoctr/doctr', service='travis')

@pytest.fixture(scope="module")
def test_repo():
temp_dir = tempfile.mkdtemp()
os.chdir(temp_dir)
subprocess.call(['git', 'init'])
return {'repo_dir': temp_dir}

@pytest.mark.parametrize('subdir, _bool', [
('a_subdir', False),
('.', True),
])
def test_in_git_root(subdir, _bool, test_repo):
os.chdir(test_repo['repo_dir'])
try:
os.mkdir(subdir)
except FileExistsError:
pass
os.chdir(subdir)
assert in_git_root() is _bool

def test_not_in_git_repo():
temp_dir = tempfile.mkdtemp()
os.chdir(temp_dir)
with raises(RuntimeError):
in_git_root()