From 426d648681261edacd395e722b5d969b1eee018c Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Dec 2015 06:09:38 -0800 Subject: [PATCH] In Drydock, don't reset current branch to point at unrelated commit Summary: Fixes T10037. When we're building commit `aabbccdd`, we currently do this to check it out: git reset --hard aabbccdd However, this has an undesirable side effect of moving the current branch pointer to point at `aabbccdd`. The current branch pointer may be some totally different branch which `aabbccdd` is not part of, so this is confusing and misleading. Instead, use `git reset --hard HEAD` to get the primary effect we want (destroying staged changes) and then `git checkout aabbccdd` to checkout the commit in a detached HEAD state. Test Plan: - Ran a build (a commit-focused operation) successfully. - Verified working copy was pointed at a detached HEAD afterward: ``` builder@sbuild001:/var/drydock/workingcopy-167/repo/git-test-ii$ git status HEAD detached at ffc7635 nothing to commit, working directory clean ``` - Ran a land (a branch-foused operation) successfully. - Verified working copy was pointed at a branch afterward: ``` builder@sbuild001:/core/data/drydock/workingcopy-168/repo/git-test$ git status On branch master Your branch is up-to-date with 'origin/master'. nothing to commit, working directory clean ``` Reviewers: chad Reviewed By: chad Subscribers: yelirekim Maniphest Tasks: T10037 Differential Revision: https://secure.phabricator.com/D14850 --- .../DrydockWorkingCopyBlueprintImplementation.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php index f39353a8f4..d60a65580b 100644 --- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php @@ -249,11 +249,15 @@ final class DrydockWorkingCopyBlueprintImplementation $ref = idx($spec, 'ref'); + // Reset things first, in case previous builds left anything staged or + // dirty. + $cmd[] = 'git reset --hard HEAD'; + if ($commit !== null) { - $cmd[] = 'git reset --hard %s'; + $cmd[] = 'git checkout %s --'; $arg[] = $commit; } else if ($branch !== null) { - $cmd[] = 'git checkout %s'; + $cmd[] = 'git checkout %s --'; $arg[] = $branch; $cmd[] = 'git reset --hard origin/%s'; @@ -267,13 +271,8 @@ final class DrydockWorkingCopyBlueprintImplementation $arg[] = $ref_ref; $arg[] = $ref_ref; - $cmd[] = 'git checkout %s'; + $cmd[] = 'git checkout %s --'; $arg[] = $ref_ref; - - $cmd[] = 'git reset --hard %s'; - $arg[] = $ref_ref; - } else { - $cmd[] = 'git reset --hard HEAD'; } $cmd = implode(' && ', $cmd);