From f45e66d4da4171ef8294f9d119f2dce6d0840c66 Mon Sep 17 00:00:00 2001 From: Erik Flodin Date: Sat, 2 Jan 2021 00:24:45 +0100 Subject: [PATCH] Rework clone Instead of doing work to find the default branch just to be able to set up the repository before doing a fetch, do a "git clone" and let git handle it. Use -c core.sharedrepository=0600 to get the same result as --shared=0600 passed to init. Use --separate-git-dir to get the git directory in $YADM_REPO. Use a temporary dir as work tree and remove it right after the clone is done. When the clone is done, iterate over all missing files in $YADM_WORK and perform a checkout. If local files exists that differ compared with the cloned ones the local files are left intact and the user is instructed to deal with the conflicts. --- test/test_clone.py | 42 +++--- test/test_default_remote_branch.py | 27 ---- test/test_unit_is_valid_branch_name.py | 40 ------ yadm | 169 ++++++++++--------------- 4 files changed, 81 insertions(+), 197 deletions(-) delete mode 100644 test/test_default_remote_branch.py delete mode 100644 test/test_unit_is_valid_branch_name.py diff --git a/test/test_clone.py b/test/test_clone.py index b024e9c..62e9c03 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -58,8 +58,8 @@ def test_clone( if not good_remote: # clone should fail assert run.failure - assert run.out != '' - assert 'Unable to fetch origin' in run.err + assert run.out == '' + assert 'Unable to clone the repository' in run.err assert not paths.repo.exists() elif repo_exists and not force: # can't overwrite data @@ -76,8 +76,7 @@ def test_clone( # ensure conflicts are handled properly if conflicts: assert 'NOTE' in run.out - assert 'Merging origin/master failed' in run.out - assert 'Conflicts preserved' in run.out + assert 'Local files with content that differs' in run.out # confirm correct Git origin run = runner( @@ -89,23 +88,16 @@ def test_clone( # ensure conflicts are really preserved if conflicts: - # test to see if the work tree is actually "clean" + # test that the conflicts are preserved in the work tree run = runner( command=yadm_cmd('status', '-uno', '--porcelain'), cwd=paths.work) assert run.success assert run.err == '' - assert run.out == '', 'worktree has unexpected changes' + assert str(ds1.tracked[0].path) in run.out - # test to see if the conflicts are stashed - run = runner(command=yadm_cmd('stash', 'list'), cwd=paths.work) - assert run.success - assert run.err == '' - assert 'Conflicts preserved' in run.out, 'conflicts not stashed' - - # verify content of the stashed conflicts - run = runner( - command=yadm_cmd('stash', 'show', '-p'), cwd=paths.work) + # verify content of the conflicts + run = runner(command=yadm_cmd('diff'), cwd=paths.work) assert run.success assert run.err == '' assert '\n+conflict' in run.out, 'conflicts not stashed' @@ -242,20 +234,20 @@ def test_clone_perms( f'initial private dir perms drwxrwxrwx.+.{private_type}', run.out) assert re.search( - f'pre-merge private dir perms drwxrwxrwx.+.{private_type}', + f'pre-checkout private dir perms drwxrwxrwx.+.{private_type}', run.out) assert re.search( - f'post-merge private dir perms drwxrwxrwx.+.{private_type}', + f'post-checkout private dir perms drwxrwxrwx.+.{private_type}', run.out) else: # private directories which are created, should be done prior to - # merging, and with secure permissions. + # checkout, and with secure permissions. assert 'initial private dir perms' not in run.out assert re.search( - f'pre-merge private dir perms drwx------.+.{private_type}', + f'pre-checkout private dir perms drwx------.+.{private_type}', run.out) assert re.search( - f'post-merge private dir perms drwx------.+.{private_type}', + f'post-checkout private dir perms drwx------.+.{private_type}', run.out) # standard perms still apply afterwards unless disabled with auto.perms @@ -297,8 +289,8 @@ def test_alternate_branch(runner, paths, yadm_cmd, repo_config, branch): if branch == 'invalid': assert run.failure - assert 'ERROR: Clone failed' in run.err - assert f"'origin/{branch}' does not exist in {remote_url}" in run.err + assert 'ERROR: Unable to clone the repository' in run.err + assert f"Remote branch {branch} not found in upstream" in run.err else: assert successful_clone(run, paths, repo_config) @@ -321,7 +313,6 @@ def test_alternate_branch(runner, paths, yadm_cmd, repo_config, branch): def successful_clone(run, paths, repo_config, expected_code=0): """Assert clone is successful""" assert run.code == expected_code - assert 'Initialized' in run.out assert oct(paths.repo.stat().mode).endswith('00'), 'Repo is not secured' assert repo_config('core.bare') == 'false' assert repo_config('status.showUntrackedFiles') == 'no' @@ -342,10 +333,11 @@ def remote(paths, ds1_repo_copy): def test_no_repo(runner, yadm_cmd, ): """Test cloning without specifying a repo""" - run = runner(command=yadm_cmd('clone')) + run = runner(command=yadm_cmd('clone', '-f')) assert run.failure assert run.out == '' - assert 'ERROR: No repository provided' in run.err + assert 'ERROR: Unable to clone the repository' in run.err + assert 'repository \'repo.git\' does not exist' in run.err def verify_head(paths, branch): diff --git a/test/test_default_remote_branch.py b/test/test_default_remote_branch.py deleted file mode 100644 index 6405417..0000000 --- a/test/test_default_remote_branch.py +++ /dev/null @@ -1,27 +0,0 @@ -"""Unit tests: _default_remote_branch()""" -import pytest - - -@pytest.mark.parametrize('condition', ['found', 'missing']) -def test(runner, paths, condition): - """Test _default_remote_branch()""" - test_branch = 'test/branch' - output = f'ref: refs/heads/{test_branch}\\tHEAD\\n' - if condition == 'missing': - output = 'output that is missing ref' - script = f""" - YADM_TEST=1 source {paths.pgm} - function git() {{ - printf '{output}'; - printf 'mock stderr\\n' 1>&2 - }} - _default_remote_branch URL - """ - print(condition) - run = runner(command=['bash'], inp=script) - assert run.success - assert run.err == '' - if condition == 'found': - assert run.out.strip() == test_branch - else: - assert run.out.strip() == 'master' diff --git a/test/test_unit_is_valid_branch_name.py b/test/test_unit_is_valid_branch_name.py deleted file mode 100644 index 9e5b6d1..0000000 --- a/test/test_unit_is_valid_branch_name.py +++ /dev/null @@ -1,40 +0,0 @@ -"""Unit tests: is_valid_branch_name""" -import pytest - -# Git branches do not allow: -# * path component that begins with "." -# * double dot -# * "~", "^", ":", "\", space -# * end with a "/" -# * end with ".lock" - - -@pytest.mark.parametrize( - 'branch, expected', [ - ('master', 'valid'), - ('path/branch', 'valid'), - ('path/.branch', 'invalid'), - ('path..branch', 'invalid'), - ('path~branch', 'invalid'), - ('path^branch', 'invalid'), - ('path:branch', 'invalid'), - ('path\\branch', 'invalid'), - ('path branch', 'invalid'), - ('path/branch/', 'invalid'), - ('branch.lock', 'invalid'), - ]) -def test_is_valid_branch_name(runner, yadm, branch, expected): - """Test function is_valid_branch_name()""" - - script = f""" - YADM_TEST=1 source {yadm} - if is_valid_branch_name "{branch}"; then - echo valid - else - echo invalid - fi - """ - run = runner(command=['bash'], inp=script) - assert run.success - assert run.err == '' - assert run.out.strip() == expected diff --git a/yadm b/yadm index 4cae9eb..e374329 100755 --- a/yadm +++ b/yadm @@ -127,7 +127,7 @@ function main() { ;; -l) # used by decrypt() DO_LIST="YES" - [ "$YADM_COMMAND" = "config" ] && YADM_ARGS+=("$1") + [[ "$YADM_COMMAND" =~ ^(clone|config)$ ]] && YADM_ARGS+=("$1") ;; -w) # used by init() and clone() if [[ ! "$2" =~ ^/ ]] ; then @@ -705,84 +705,73 @@ function clean() { } -function _default_remote_branch() { - local ls_remote - ls_remote=$("$GIT_PROGRAM" ls-remote -q --symref "$1" 2>/dev/null) - match="^ref:[[:blank:]]+refs/heads/([^[:blank:]]+)" - if [[ "$ls_remote" =~ $match ]] ; then - echo "${BASH_REMATCH[1]}" - else - echo master - fi -} - function clone() { DO_BOOTSTRAP=1 - local branch= - - local repo_url= + local -a args + local -i do_checkout=1 while [[ $# -gt 0 ]] ; do - key="$1" - case $key in - -b) - if ! is_valid_branch_name "$2"; then - error_out "You must provide a branch name when using '-b'" - fi - branch="$2" - shift - ;; + case "$1" in --bootstrap) # force bootstrap, without prompt DO_BOOTSTRAP=2 ;; --no-bootstrap) # prevent bootstrap, without prompt DO_BOOTSTRAP=3 ;; - *) # use first found argument as the URL - [ -z "$repo_url" ] && repo_url="$1" + --checkout) + do_checkout=1 + ;; + -n|--no-checkout) + do_checkout=0 + ;; + --bare|--mirror|--recurse-submodules*|--recursive|--separate-git-dir=*) + # ignore arguments without separate parameter + ;; + --separate-git-dir) + # ignore arguments with separate parameter + shift + ;; + *) + args+=("$1") ;; esac shift done - [ -z "$repo_url" ] && error_out "No repository provided" - - [ -z "$branch" ] && branch=$(_default_remote_branch "$repo_url") - [ -n "$DEBUG" ] && display_private_perms "initial" - # shellcheck disable=SC2119 - # clone will begin with a bare repo - init + # safety check, don't attempt to clone when the repo is already present + [ -d "$YADM_REPO" ] && [ -z "$FORCE" ] && + error_out "Git repo already exists. [$YADM_REPO]\nUse '-f' if you want to force it to be overwritten." - # configure local HEAD with the correct branch - printf 'ref: refs/heads/%s\n' "$branch" > "${YADM_REPO}/HEAD" - - # add the specified remote, and configure the repo to track origin/$branch - debug "Adding remote to new repo" - "$GIT_PROGRAM" remote add origin "$repo_url" - debug "Configuring new repo to track origin/${branch}" - "$GIT_PROGRAM" config "branch.${branch}.remote" origin - "$GIT_PROGRAM" config "branch.${branch}.merge" "refs/heads/${branch}" - - # fetch / merge (and possibly fallback to reset) - debug "Doing an initial fetch of the origin" - "$GIT_PROGRAM" fetch origin || { - debug "Removing repo after failed clone" + # remove existing if forcing the clone to happen anyway + [ -d "$YADM_REPO" ] && { + debug "Removing existing repo prior to clone" rm -rf "$YADM_REPO" - error_out "Unable to fetch origin $repo_url" } - debug "Verifying '${branch}' is a valid branch to merge" - [ -f "${YADM_REPO}/refs/remotes/origin/${branch}" ] || { - debug "Removing repo after failed clone" - rm -rf "$YADM_REPO" - error_out "Clone failed, 'origin/${branch}' does not exist in $repo_url" + + local wc + wc="$(mktemp -d)" || error_out "Unable to create temporary directory" + + # first clone without checkout + debug "Doing an initial clone of the repository" + (cd "$wc" && + "$GIT_PROGRAM" -c core.sharedrepository=0600 clone --no-checkout \ + --separate-git-dir="$YADM_REPO" "${args[@]}" repo.git) || { + debug "Removing repo after failed clone" + rm -rf "$YADM_REPO" "$wc" + error_out "Unable to clone the repository" } + rm -rf "$wc" + configure_repo + + # then reset the index as the --no-checkout flag makes the index empty + "$GIT_PROGRAM" reset --quiet -- . if [ "$YADM_WORK" = "$HOME" ]; then debug "Determining if repo tracks private directories" for private_dir in $(private_dirs all); do - found_log=$("$GIT_PROGRAM" log -n 1 "origin/${branch}" -- "$private_dir" 2>/dev/null) + found_log=$("$GIT_PROGRAM" log -n 1 -- "$private_dir" 2>/dev/null) if [ -n "$found_log" ]; then debug "Private directory $private_dir is tracked by repo" assert_private_dirs "$private_dir" @@ -790,51 +779,33 @@ function clone() { done fi - [ -n "$DEBUG" ] && display_private_perms "pre-merge" - debug "Doing an initial merge of origin/${branch}" - "$GIT_PROGRAM" merge "origin/${branch}" || { - debug "Merge failed, doing a reset and stashing conflicts." - "$GIT_PROGRAM" reset "origin/${branch}" - if cd "$YADM_WORK"; then # necessary because of a bug in Git - "$GIT_PROGRAM" -c user.name='yadm clone' -c user.email='yadm' stash save Conflicts preserved from yadm clone command 2>&1 - cat < /dev/null; then