Merge ~smoser/cloud-init:cleanup/tools-run-centos-use-git-clone into cloud-init:master

[Scott Moser]

tools: run-centos: only collect .git dir.

I have lots of files in my checkout dir. tar <dir> causes
all those files to get copied into the container.

This instead collects only the .git dir and then checks out
the branch inside the container. That means a lot less
disk IO, given that .tox/ can be 300M+ while .git is just
about 40M.

using 'git clone' makes this work from a worktree.
I couldnt' figure out how to easily find "the real .git" if inside a worktree.

Figuring that out, and then just using 'tar -cf .git' would avoid the local tmpdir copy

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:bca132c1b66d7f9f53a2120b8155a984de4fd95f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/767/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/767/rebuild

review: Approve (continuous-integration)

[Joshua Powers (powersj)]

Did you push for the right branch? This looks like packaging of a new release.

[Scott Moser (smoser)]

Josh, yeah, I had wrong base branch.
that is fixed.

Also fixed, now we dont use a temp dir, so it is basically
  tar -cf .git | tar -xf

that means ~ 42M which is an improvement over the current solution.
Espeically when my .tox is now 316M.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:dd2a9f2c8e7b006e3dfee01e4ff5bde0e9e33613
https://jenkins.ubuntu.com/server/job/cloud-init-ci/768/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/768/rebuild

review: Approve (continuous-integration)

[Joshua Powers (powersj)]

LGTM and tested locally seems to make execution go faster. Thanks!

review: Approve

[Joshua Powers (powersj)]

thinking about this some more, this will not pull in uncommitted code. Do we want to have a message print a warning to the user that they have uncommitted code and it won't be pulled in?

[Scott Moser (smoser)]

You're correct that it will not bring in un-committed code. I can address
that. I dont think its all that big of a deal really, the git work flow is
generally "git commit"... git ammend. other parts of cloud-init also do
not pick up uncommitted changes.

On Mon, Feb 12, 2018 at 11:51 AM, Joshua Powers <email address hidden>
wrote:

> thinking about this some more, this will not pull in uncommitted code. Do
> we want to have a message print a warning to the user that they have
> uncommitted code and it won't be pulled in?
> --
> https://code.launchpad.net/~smoser/cloud-init/+git/cloud-
> init/+merge/337499
> You are the owner of ~smoser/cloud-init:cleanup/tools-run-centos-use-git-
> clone.
>

[Scott Moser (smoser)]

Josh, please see new commit. I think this is sane.

On Mon, Feb 12, 2018 at 12:03 PM, Scott Moser <email address hidden>
wrote:

> You're correct that it will not bring in un-committed code. I can address
> that. I dont think its all that big of a deal really, the git work flow is
> generally "git commit"... git ammend. other parts of cloud-init also do
> not pick up uncommitted changes.
>
>
> On Mon, Feb 12, 2018 at 11:51 AM, Joshua Powers <<email address hidden>
> >
> wrote:
>
> > thinking about this some more, this will not pull in uncommitted code. Do
> > we want to have a message print a warning to the user that they have
> > uncommitted code and it won't be pulled in?
> > --
> > https://code.launchpad.net/~smoser/cloud-init/+git/cloud-
> > init/+merge/337499
> > You are the owner of ~smoser/cloud-init:cleanup/
> tools-run-centos-use-git-
> > clone.
> >
>
> --
> https://code.launchpad.net/~smoser/cloud-init/+git/cloud-
> init/+merge/337499
> You are the owner of ~smoser/cloud-init:cleanup/tools-run-centos-use-git-
> clone.
>

[Joshua Powers (powersj)]

LGTM thanks for this!

review: Approve

diff --git a/tools/run-centos b/tools/run-centos
index d58ef3e..6ac6c11 100755
--- a/tools/run-centos
+++ b/tools/run-centos
@@ -23,6 +23,9 @@ Usage: ${0##*/} [ options ] version
 
     options:
       -a | --artifact keep .rpm artifacts
+           --dirty    apply local changes before running tests.
+                      If not provided, a clean checkout of branch is tested.
+                      Inside container, changes are in local-changes.diff.
       -k | --keep     keep container after tests
       -r | --rpm      build .rpm
       -s | --srpm     build .src.rpm
@@ -80,25 +83,84 @@ inside() {
 inject_cloud_init(){
     # take current cloud-init git dir and put it inside $name at
     # ~$user/cloud-init.
-    local name="$1" user="$2" top_d="" dname="" pstat=""
-    top_d=$(git rev-parse --show-toplevel) || {
-        errorrc "Failed to get git top level in $PWD";
+    local name="$1" user="$2" dirty="$3"
+    local changes="" top_d="" dname="cloud-init" pstat=""
+    local gitdir="" commitish=""
+    gitdir=$(git rev-parse --git-dir) || {
+        errorrc "Failed to get git dir in $PWD";
         return
     }
-    dname=$(basename "${top_d}") || return
-    debug 1 "collecting ${top_d} ($dname) into user $user in $name."
-    tar -C "${top_d}/.." -cpf - "$dname" |
+    local t=${gitdir%/*}
+    case "$t" in
+        */worktrees) 
+            if [ -f "${t%worktrees}/config" ]; then
+                gitdir="${t%worktrees}"
+            fi
+    esac
+
+    # attempt to get branch name.
+    commitish=$(git rev-parse --abbrev-ref HEAD) || {
+        errorrc "Failed git rev-parse --abbrev-ref HEAD"
+        return
+    }
+    if [ "$commitish" = "HEAD" ]; then
+        # detached head
+        commitish=$(git rev-parse HEAD) || {
+            errorrc "failed git rev-parse HEAD"
+            return
+        }
+    fi
+
+    local local_changes=false
+    if ! git diff --quiet "$commitish"; then
+        # there are local changes not committed.
+        local_changes=true
+        if [ "$dirty" = "false" ]; then
+            error "WARNING: You had uncommitted changes.  Those changes will "
+            error "be put into 'local-changes.diff' inside the container. "
+            error "To test these changes you must pass --dirty."
+        fi
+    fi
+
+    debug 1 "collecting ${gitdir} ($dname) into user $user in $name."
+    tar -C "${gitdir}" -cpf - . |
         inside_as "$name" "$user" sh -ec '
             dname=$1
+            commitish=$2
             rm -Rf "$dname"
+            mkdir -p $dname/.git
+            cd $dname/.git
             tar -xpf -
-            [ "$dname" = "cloud-init" ] || mv "$dname" cloud-init' \
-            extract "$dname"
+            cd ..
+            git config core.bare false
+            out=$(git checkout $commitish 2>&1) ||
+                { echo "failed git checkout $commitish: $out" 1>&2; exit 1; }
+            out=$(git checkout . 2>&1) ||
+                { echo "failed git checkout .: $out" 1>&2; exit 1; }
+            ' extract "$dname" "$commitish"
     [ "${PIPESTATUS[*]}" = "0 0" ] || {
-        error "Failed to push tarball of '$top_d' into $name" \
+        error "Failed to push tarball of '$gitdir' into $name" \
             " for user $user (dname=$dname)"
         return 1
     }
+
+    echo "local_changes=$local_changes dirty=$dirty"
+    if [ "$local_changes" = "true" ]; then
+        git diff "$commitish" |
+            inside_as "$name" "$user" sh -exc '
+                cd "$1"
+                if [ "$2" = "true" ]; then
+                    git apply
+                else
+                    cat > local-changes.diff
+                fi
+                ' insert_changes "$dname" "$dirty"
+        [ "${PIPESTATUS[*]}" = "0 0" ] || {
+            error "Failed to apply local changes."
+            return 1
+        }
+    fi
+
     return 0
 }
 
@@ -179,7 +241,7 @@ delete_container() {
 
 main() {
     local short_opts="ahkrsuv"
-    local long_opts="artifact,help,keep,rpm,srpm,unittest,verbose"
+    local long_opts="artifact,dirty,help,keep,rpm,srpm,unittest,verbose"
     local getopt_out=""
     getopt_out=$(getopt --name "${0##*/}" \
         --options "${short_opts}" --long "${long_opts}" -- "$@") &&
@@ -188,11 +250,13 @@ main() {
 
     local cur="" next=""
     local artifact="" keep="" rpm="" srpm="" unittest="" version=""
+    local dirty=false
 
     while [ $# -ne 0 ]; do
         cur="${1:-}"; next="${2:-}";
         case "$cur" in
             -a|--artifact) artifact=1;;
+               --dirty) dirty=true;;
             -h|--help) Usage ; exit 0;;
             -k|--keep) KEEP=true;;
             -r|--rpm) rpm=1;;
@@ -231,7 +295,7 @@ main() {
     inside "$name" useradd "$user"
 
     debug 1 "inserting cloud-init"
-    inject_cloud_init "$name" "$user" || {
+    inject_cloud_init "$name" "$user" "$dirty" || {
         errorrc "FAIL: injecting cloud-init into $name failed."
         return
     }
@@ -244,7 +308,7 @@ main() {
 
     local errors=0
     inside_as_cd "$name" "$user" "$cdir" \
-        sh -ec "git checkout .; git status" ||
+        sh -ec "git status" ||
             { errorrc "git checkout failed."; errors=$(($errors+1)); }
 
     if [ -n "$unittest" ]; then