From e74253d2def66bc9927f5e8122867b35b315215a Mon Sep 17 00:00:00 2001 From: DanielChabrowski Date: Sun, 3 Mar 2019 01:35:53 +0100 Subject: [PATCH 1/9] Allow shallow cloning --- pre_commit/store.py | 62 ++++++++++++++++++++++++++++++++++----------- testing/util.py | 5 ++++ tests/store_test.py | 38 +++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/pre_commit/store.py b/pre_commit/store.py index 8301ecad..9fa48127 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -10,6 +10,7 @@ import tempfile import pre_commit.constants as C from pre_commit import file_lock from pre_commit import git +from pre_commit.util import CalledProcessError from pre_commit.util import clean_path_on_failure from pre_commit.util import cmd_output from pre_commit.util import resource_text @@ -121,10 +122,7 @@ class Store(object): return result logger.info('Initializing environment for {}.'.format(repo)) - - directory = tempfile.mkdtemp(prefix='repo', dir=self.directory) - with clean_path_on_failure(directory): - make_strategy(directory) + directory = make_strategy() # Update our db with the created repo with self.connect() as db: @@ -134,19 +132,50 @@ class Store(object): ) return directory + def _perform_safe_clone(self, clone_strategy): + directory = tempfile.mkdtemp(prefix='repo', dir=self.directory) + with clean_path_on_failure(directory): + clone_strategy(directory) + return directory + + def _complete_clone(self, repo, ref, directory): + """Perform a complete clone of a repository and its submodules """ + env = git.no_git_env() + + cmd = ('git', 'clone', '--no-checkout', repo, directory) + cmd_output(*cmd, env=env) + + def _git_cmd(*args): + return cmd_output('git', *args, cwd=directory, env=env) + + _git_cmd('reset', ref, '--hard') + _git_cmd('submodule', 'update', '--init', '--recursive') + + def _shallow_clone(self, repo, ref, directory): + """Perform a shallow clone of a repository and its submodules """ + env = git.no_git_env() + + def _git_cmd(*args): + return cmd_output('git', *args, cwd=directory, env=env) + + _git_cmd('init', '.') + _git_cmd('remote', 'add', 'origin', repo) + _git_cmd('fetch', 'origin', ref, '--depth=1') + _git_cmd('checkout', ref) + _git_cmd('submodule', 'update', '--init', '--recursive', '--depth=1') + def clone(self, repo, ref, deps=()): """Clone the given url and checkout the specific ref.""" - def clone_strategy(directory): - env = git.no_git_env() - cmd = ('git', 'clone', '--no-checkout', repo, directory) - cmd_output(*cmd, env=env) - - def _git_cmd(*args): - return cmd_output('git', *args, cwd=directory, env=env) - - _git_cmd('reset', ref, '--hard') - _git_cmd('submodule', 'update', '--init', '--recursive') + def clone_strategy(): + try: + def shallow_clone(directory): + self._shallow_clone(repo, ref, directory) + return self._perform_safe_clone(shallow_clone) + except CalledProcessError: + def complete_clone(directory): + self._complete_clone(repo, ref, directory) + return self._perform_safe_clone(complete_clone) return self._new_repo(repo, ref, deps, clone_strategy) @@ -173,8 +202,11 @@ class Store(object): _git_cmd('add', '.') git.commit(repo=directory) + def make_strategy(): + return self._perform_safe_clone(make_local_strategy) + return self._new_repo( - 'local', C.LOCAL_REPO_VERSION, deps, make_local_strategy, + 'local', C.LOCAL_REPO_VERSION, deps, make_strategy, ) def _create_config_table_if_not_exists(self, db): diff --git a/testing/util.py b/testing/util.py index 15696730..f4dda0a9 100644 --- a/testing/util.py +++ b/testing/util.py @@ -142,3 +142,8 @@ def git_commit(*args, **kwargs): if msg is not None: # allow skipping `-m` with `msg=None` cmd += ('-m', msg) return fn(*cmd, **kwargs) + + +def git_ref_count(repo): + _, out, _ = cmd_output('git', 'rev-list', '--all', '--count', cwd=repo) + return int(out.split()[0]) diff --git a/tests/store_test.py b/tests/store_test.py index 238343fd..c3de6891 100644 --- a/tests/store_test.py +++ b/tests/store_test.py @@ -12,9 +12,11 @@ import six from pre_commit import git from pre_commit.store import _get_default_directory from pre_commit.store import Store +from pre_commit.util import CalledProcessError from testing.fixtures import git_dir from testing.util import cwd from testing.util import git_commit +from testing.util import git_ref_count def test_our_session_fixture_works(): @@ -81,6 +83,7 @@ def test_clone(store, tempdir_factory, log_info_mock): assert dirname.startswith('repo') # Should be checked out to the rev we specified assert git.head_rev(ret) == rev + assert git_ref_count(ret) == 1 # Assert there's an entry in the sqlite db for this assert store.select_all_repos() == [(path, rev, ret)] @@ -111,6 +114,41 @@ def test_clone_when_repo_already_exists(store): assert store.clone('fake_repo', 'fake_ref') == 'fake_path' +def test_clone_shallow_failure_fallback_to_complete( + store, tempdir_factory, + log_info_mock, +): + path = git_dir(tempdir_factory) + with cwd(path): + git_commit() + rev = git.head_rev(path) + git_commit() + + # Force shallow clone failure + def fake_shallow_clone(self, *args, **kwargs): + raise CalledProcessError(None, None, None) + store._shallow_clone = fake_shallow_clone + + ret = store.clone(path, rev) + + # Should have printed some stuff + assert log_info_mock.call_args_list[0][0][0].startswith( + 'Initializing environment for ', + ) + + # Should return a directory inside of the store + assert os.path.exists(ret) + assert ret.startswith(store.directory) + # Directory should start with `repo` + _, dirname = os.path.split(ret) + assert dirname.startswith('repo') + # Should be checked out to the rev we specified + assert git.head_rev(ret) == rev + + # Assert there's an entry in the sqlite db for this + assert store.select_all_repos() == [(path, rev, ret)] + + def test_create_when_directory_exists_but_not_db(store): # In versions <= 0.3.5, there was no sqlite db causing a need for # backward compatibility From b920f3cc6bacc0fafa0aef3edf817ea0f88bc46b Mon Sep 17 00:00:00 2001 From: DanielChabrowski Date: Sat, 9 Mar 2019 22:59:56 +0100 Subject: [PATCH 2/9] Reuse the directory for cloning --- pre_commit/store.py | 67 ++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 38 deletions(-) diff --git a/pre_commit/store.py b/pre_commit/store.py index 9fa48127..943c5a8d 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -122,7 +122,10 @@ class Store(object): return result logger.info('Initializing environment for {}.'.format(repo)) - directory = make_strategy() + + directory = tempfile.mkdtemp(prefix='repo', dir=self.directory) + with clean_path_on_failure(directory): + make_strategy(directory) # Update our db with the created repo with self.connect() as db: @@ -132,50 +135,41 @@ class Store(object): ) return directory - def _perform_safe_clone(self, clone_strategy): - directory = tempfile.mkdtemp(prefix='repo', dir=self.directory) - with clean_path_on_failure(directory): - clone_strategy(directory) - return directory - - def _complete_clone(self, repo, ref, directory): + def _complete_clone(self, ref, git_cmd): """Perform a complete clone of a repository and its submodules """ - env = git.no_git_env() - cmd = ('git', 'clone', '--no-checkout', repo, directory) - cmd_output(*cmd, env=env) + git_cmd('fetch', 'origin') + git_cmd('checkout', ref) + git_cmd('submodule', 'update', '--init', '--recursive') - def _git_cmd(*args): - return cmd_output('git', *args, cwd=directory, env=env) - - _git_cmd('reset', ref, '--hard') - _git_cmd('submodule', 'update', '--init', '--recursive') - - def _shallow_clone(self, repo, ref, directory): + def _shallow_clone(self, ref, protocol_version, git_cmd): """Perform a shallow clone of a repository and its submodules """ - env = git.no_git_env() - def _git_cmd(*args): - return cmd_output('git', *args, cwd=directory, env=env) - - _git_cmd('init', '.') - _git_cmd('remote', 'add', 'origin', repo) - _git_cmd('fetch', 'origin', ref, '--depth=1') - _git_cmd('checkout', ref) - _git_cmd('submodule', 'update', '--init', '--recursive', '--depth=1') + git_config = 'protocol.version={}'.format(protocol_version) + git_cmd('-c', git_config, 'fetch', 'origin', ref, '--depth=1') + git_cmd('checkout', ref) + git_cmd('-c', git_config, 'submodule', 'update', '--init', + '--recursive', '--depth=1') def clone(self, repo, ref, deps=()): """Clone the given url and checkout the specific ref.""" - def clone_strategy(): + def clone_strategy(directory): + env = git.no_git_env() + + def _git_cmd(*args): + cmd_output('git', *args, cwd=directory, env=env) + + _git_cmd('init', '.') + _git_cmd('remote', 'add', 'origin', repo) + try: - def shallow_clone(directory): - self._shallow_clone(repo, ref, directory) - return self._perform_safe_clone(shallow_clone) + self._shallow_clone(ref, 2, _git_cmd) except CalledProcessError: - def complete_clone(directory): - self._complete_clone(repo, ref, directory) - return self._perform_safe_clone(complete_clone) + try: + self._shallow_clone(ref, 1, _git_cmd) + except CalledProcessError: + self._complete_clone(ref, _git_cmd) return self._new_repo(repo, ref, deps, clone_strategy) @@ -202,11 +196,8 @@ class Store(object): _git_cmd('add', '.') git.commit(repo=directory) - def make_strategy(): - return self._perform_safe_clone(make_local_strategy) - return self._new_repo( - 'local', C.LOCAL_REPO_VERSION, deps, make_strategy, + 'local', C.LOCAL_REPO_VERSION, deps, make_local_strategy, ) def _create_config_table_if_not_exists(self, db): From 960bcc96141c2440923145603e61a3fc11d23e0e Mon Sep 17 00:00:00 2001 From: DanielChabrowski Date: Sat, 9 Mar 2019 23:56:37 +0100 Subject: [PATCH 3/9] Fix relative path repos --- pre_commit/store.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pre_commit/store.py b/pre_commit/store.py index 943c5a8d..75fbceb0 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -154,6 +154,9 @@ class Store(object): def clone(self, repo, ref, deps=()): """Clone the given url and checkout the specific ref.""" + if os.path.isdir(repo): + repo = os.path.abspath(repo) + def clone_strategy(directory): env = git.no_git_env() From 3cb35e8679b2e8c09398953b19bd063ceabfc665 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Thu, 14 Mar 2019 18:20:30 -0700 Subject: [PATCH 4/9] Revert "Merge pull request #949 from asottile/npm_install_git" This reverts commit a4c1a701bcd70a4a27b4bd0d9832a447c782daa9, reversing changes made to 889124b5ca31d51f8849a8aaca70b3cfaa742de5. --- pre_commit/languages/node.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index e7962cce..b313bf5b 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -62,11 +62,10 @@ def install_environment(prefix, version, additional_dependencies): cmd.extend(['-n', version]) cmd_output(*cmd) - dep = 'git+file:///{}'.format(prefix.prefix_dir) with in_env(prefix, version): helpers.run_setup_cmd( prefix, - ('npm', 'install', '-g', dep) + additional_dependencies, + ('npm', 'install', '-g', '.') + additional_dependencies, ) From d71a75fea2ebd3416353f2e2bf9d9c6139501ad3 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Thu, 14 Mar 2019 18:31:57 -0700 Subject: [PATCH 5/9] Run `npm install` before `npm install -g` --- pre_commit/languages/node.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index b313bf5b..aac1c591 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -63,6 +63,9 @@ def install_environment(prefix, version, additional_dependencies): cmd_output(*cmd) with in_env(prefix, version): + # https://npm.community/t/npm-install-g-git-vs-git-clone-cd-npm-install-g/5449 + # install as if we installed from git + helpers.run_setup_cmd(prefix, ('npm', 'install')) helpers.run_setup_cmd( prefix, ('npm', 'install', '-g', '.') + additional_dependencies, From ec2e15f086aab3510a1509650de9191819d551b1 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Thu, 14 Mar 2019 18:32:27 -0700 Subject: [PATCH 6/9] pre-commit run --all-files --- pre_commit/store.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pre_commit/store.py b/pre_commit/store.py index 75fbceb0..7a85d03e 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -148,8 +148,10 @@ class Store(object): git_config = 'protocol.version={}'.format(protocol_version) git_cmd('-c', git_config, 'fetch', 'origin', ref, '--depth=1') git_cmd('checkout', ref) - git_cmd('-c', git_config, 'submodule', 'update', '--init', - '--recursive', '--depth=1') + git_cmd( + '-c', git_config, 'submodule', 'update', '--init', + '--recursive', '--depth=1', + ) def clone(self, repo, ref, deps=()): """Clone the given url and checkout the specific ref.""" From e748da2abe1b5916847af3380dfb9b633a4b171c Mon Sep 17 00:00:00 2001 From: DanielChabrowski Date: Fri, 15 Mar 2019 23:25:04 +0100 Subject: [PATCH 7/9] Remove clone depth check --- testing/util.py | 5 ----- tests/store_test.py | 2 -- 2 files changed, 7 deletions(-) diff --git a/testing/util.py b/testing/util.py index f4dda0a9..15696730 100644 --- a/testing/util.py +++ b/testing/util.py @@ -142,8 +142,3 @@ def git_commit(*args, **kwargs): if msg is not None: # allow skipping `-m` with `msg=None` cmd += ('-m', msg) return fn(*cmd, **kwargs) - - -def git_ref_count(repo): - _, out, _ = cmd_output('git', 'rev-list', '--all', '--count', cwd=repo) - return int(out.split()[0]) diff --git a/tests/store_test.py b/tests/store_test.py index c3de6891..66217588 100644 --- a/tests/store_test.py +++ b/tests/store_test.py @@ -16,7 +16,6 @@ from pre_commit.util import CalledProcessError from testing.fixtures import git_dir from testing.util import cwd from testing.util import git_commit -from testing.util import git_ref_count def test_our_session_fixture_works(): @@ -83,7 +82,6 @@ def test_clone(store, tempdir_factory, log_info_mock): assert dirname.startswith('repo') # Should be checked out to the rev we specified assert git.head_rev(ret) == rev - assert git_ref_count(ret) == 1 # Assert there's an entry in the sqlite db for this assert store.select_all_repos() == [(path, rev, ret)] From a170e60daac3ac5a39e334ab4d34c43c762e6f25 Mon Sep 17 00:00:00 2001 From: DanielChabrowski Date: Fri, 15 Mar 2019 23:46:35 +0100 Subject: [PATCH 8/9] Remove protocol.version 1 shallow cloning --- pre_commit/store.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pre_commit/store.py b/pre_commit/store.py index 7a85d03e..09116861 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -142,10 +142,10 @@ class Store(object): git_cmd('checkout', ref) git_cmd('submodule', 'update', '--init', '--recursive') - def _shallow_clone(self, ref, protocol_version, git_cmd): + def _shallow_clone(self, ref, git_cmd): """Perform a shallow clone of a repository and its submodules """ - git_config = 'protocol.version={}'.format(protocol_version) + git_config = 'protocol.version=2' git_cmd('-c', git_config, 'fetch', 'origin', ref, '--depth=1') git_cmd('checkout', ref) git_cmd( @@ -169,12 +169,9 @@ class Store(object): _git_cmd('remote', 'add', 'origin', repo) try: - self._shallow_clone(ref, 2, _git_cmd) + self._shallow_clone(ref, _git_cmd) except CalledProcessError: - try: - self._shallow_clone(ref, 1, _git_cmd) - except CalledProcessError: - self._complete_clone(ref, _git_cmd) + self._complete_clone(ref, _git_cmd) return self._new_repo(repo, ref, deps, clone_strategy) From ab1df034182b3a699dcf05ec5c8f7a8eba7c8fae Mon Sep 17 00:00:00 2001 From: DanielChabrowski Date: Sat, 16 Mar 2019 00:16:39 +0100 Subject: [PATCH 9/9] Ignore shallow clone coverage on appveyor Appveyor uses old version of git so shallow clone always fails and lines 150-151 are not executed. --- pre_commit/store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pre_commit/store.py b/pre_commit/store.py index 09116861..93a9cab3 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -142,7 +142,7 @@ class Store(object): git_cmd('checkout', ref) git_cmd('submodule', 'update', '--init', '--recursive') - def _shallow_clone(self, ref, git_cmd): + def _shallow_clone(self, ref, git_cmd): # pragma: windows no cover """Perform a shallow clone of a repository and its submodules """ git_config = 'protocol.version=2'