Add option for importing a course from a named branch of a git repo

lms/djangoapps/dashboard/git_import.py

@@ -39,7 +39,9 @@ class GitImportError(Exception):
     CANNOT_PULL = _('git clone or pull failed!')
     XML_IMPORT_FAILED = _('Unable to run import command.')
     UNSUPPORTED_STORE = _('The underlying module store does not support import.')
-
+    REMOTE_BRANCH_MISSING = _('The specified remote branch is not available.')
+    CANNOT_BRANCH = _('Unable to switch to specified branch. Please check '
+                      'your branch name.')
 
 def cmd_log(cmd, cwd):
     """
@@ -54,7 +56,60 @@ def cmd_log(cmd, cwd):
     return output
 
 
-def add_repo(repo, rdir_in):
+def switch_branch(branch, rdir):
+    """
+    This will determine how to change the branch of the repo, and then
+    use the appropriate git commands to do so.
+
+    Raises an appropriate GitImportError exception if there is any issues with changing
+    branches.
+    """
+    # Get the latest remote
+    try:
+        cmd_log(['git', 'fetch', ], rdir)
+    except subprocess.CalledProcessError as ex:
+        log.exception('Unable to fetch remote: %r', ex.output)
+        raise GitImportError(GitImportError.CANNOT_BRANCH)
+
+    # Check if the branch is available from the remote.
+    cmd = ['git', 'ls-remote', 'origin', '-h', 'refs/heads/{0}'.format(branch), ]
+    try:
+        output = cmd_log(cmd, rdir)
+    except subprocess.CalledProcessError as ex:
+        log.exception('Getting a list of remote branches failed: %r', ex.output)
+        raise GitImportError(GitImportError.CANNOT_BRANCH)
+    if not branch in output:
+        raise GitImportError(GitImportError.REMOTE_BRANCH_MISSING)
+    # Check it the remote branch has already been made locally
+    cmd = ['git', 'branch', '-a', ]
+    try:
+        output = cmd_log(cmd, rdir)
+    except subprocess.CalledProcessError as ex:
+        log.exception('Getting a list of local branches failed: %r', ex.output)
+        raise GitImportError(GitImportError.CANNOT_BRANCH)
+    branches = []
+    for line in output.split('\n'):
+        branches.append(line.strip())
+
+    if branch not in branches:
+        # Checkout with -b since it is remote only
+        cmd = ['git', 'checkout', '--force', '--track',
+               '-b', branch, 'origin/{0}'.format(branch), ]
+        try:
+            cmd_log(cmd, rdir)
+        except subprocess.CalledProcessError as ex:
+            log.exception('Unable to checkout remote branch: %r', ex.output)
+            raise GitImportError(GitImportError.CANNOT_BRANCH)
+    # Go ahead and reset hard to the newest version of the branch now that we know
+    # it is local.
+    try:
+        cmd_log(['git', 'reset', '--hard', 'origin/{0}'.format(branch), ], rdir)
+    except subprocess.CalledProcessError as ex:
+        log.exception('Unable to reset to branch: %r', ex.output)
+        raise GitImportError(GitImportError.CANNOT_BRANCH)
+
+
+def add_repo(repo, rdir_in, branch):
     """This will add a git repo into the mongo modulestore"""
     # pylint: disable=R0915
 
@@ -102,6 +157,9 @@ def add_repo(repo, rdir_in):
         log.exception('Error running git pull: %r', ex.output)
         raise GitImportError(GitImportError.CANNOT_PULL)
 
+    if branch:
+        switch_branch(branch, rdirp)
+
     # get commit id
     cmd = ['git', 'log', '-1', '--format=%H', ]
     try:

lms/djangoapps/dashboard/management/commands/git_add_course.py

@@ -25,8 +25,10 @@ class Command(BaseCommand):
     Pull a git repo and import into the mongo based content database.
     """
 
-    help = _('Import the specified git repository into the '
-             'modulestore and directory')
+    help = ('Usage: '
+            'git_add_course repository_url [directory to check out into] [repository_branch] '
+            '\n{0}'.format(_('Import the specified git repository and optional branch into the '
+                             'modulestore and optionally specified directory.')))
 
     def handle(self, *args, **options):
         """Check inputs and run the command"""
@@ -38,16 +40,19 @@ class Command(BaseCommand):
             raise CommandError('This script requires at least one argument, '
                                'the git URL')
 
-        if len(args) > 2:
-            raise CommandError('This script requires no more than two '
+        if len(args) > 3:
+            raise CommandError('This script requires no more than three '
                                'arguments')
 
         rdir_arg = None
+        branch = None
 
         if len(args) > 1:
             rdir_arg = args[1]
+        if len(args) > 2:
+            branch = args[2]
 
         try:
-            dashboard.git_import.add_repo(args[0], rdir_arg)
+            dashboard.git_import.add_repo(args[0], rdir_arg, branch)
         except GitImportError as ex:
             raise CommandError(str(ex))

lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py

@@ -2,11 +2,12 @@
 Provide tests for git_add_course management command.
 """
 
-import unittest
+import logging
 import os
 import shutil
 import StringIO
 import subprocess
+import unittest
 
 from django.conf import settings
 from django.core.management import call_command
@@ -14,6 +15,9 @@ from django.core.management.base import CommandError
 from django.test.utils import override_settings
 
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
+from xmodule.contentstore.django import contentstore
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 import dashboard.git_import as git_import
 from dashboard.git_import import GitImportError
@@ -39,6 +43,9 @@ class TestGitAddCourse(ModuleStoreTestCase):
     """
 
     TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git'
+    TEST_COURSE = 'MITx/edx4edx/edx4edx'
+    TEST_BRANCH = 'testing_do_not_delete'
+    TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx'
 
     def assertCommandFailureRegexp(self, regex, *args):
         """
@@ -56,16 +63,15 @@ class TestGitAddCourse(ModuleStoreTestCase):
         self.assertCommandFailureRegexp(
             'This script requires at least one argument, the git URL')
         self.assertCommandFailureRegexp(
-            'This script requires no more than two arguments',
-            'blah', 'blah', 'blah')
+            'This script requires no more than three arguments',
+            'blah', 'blah', 'blah', 'blah')
         self.assertCommandFailureRegexp(
             'Repo was not added, check log output for details',
             'blah')
         # Test successful import from command
-        try:
+        if not os.path.isdir(getattr(settings, 'GIT_REPO_DIR')):
             os.mkdir(getattr(settings, 'GIT_REPO_DIR'))
-        except OSError:
-            pass
+        self.addCleanup(shutil.rmtree, getattr(settings, 'GIT_REPO_DIR'))
 
         # Make a course dir that will be replaced with a symlink
         # while we are at it.
@@ -74,24 +80,28 @@ class TestGitAddCourse(ModuleStoreTestCase):
 
         call_command('git_add_course', self.TEST_REPO,
                      getattr(settings, 'GIT_REPO_DIR') / 'edx4edx_lite')
-        if os.path.isdir(getattr(settings, 'GIT_REPO_DIR')):
-            shutil.rmtree(getattr(settings, 'GIT_REPO_DIR'))
+
+        # Test with all three args (branch)
+        call_command('git_add_course', self.TEST_REPO,
+                     getattr(settings, 'GIT_REPO_DIR') / 'edx4edx_lite',
+                     self.TEST_BRANCH)
+
 
     def test_add_repo(self):
         """
         Various exit path tests for test_add_repo
         """
         with self.assertRaisesRegexp(GitImportError, GitImportError.NO_DIR):
-            git_import.add_repo(self.TEST_REPO, None)
+            git_import.add_repo(self.TEST_REPO, None, None)
 
         os.mkdir(getattr(settings, 'GIT_REPO_DIR'))
         self.addCleanup(shutil.rmtree, getattr(settings, 'GIT_REPO_DIR'))
 
         with self.assertRaisesRegexp(GitImportError, GitImportError.URL_BAD):
-            git_import.add_repo('foo', None)
+            git_import.add_repo('foo', None, None)
 
         with self.assertRaisesRegexp(GitImportError, GitImportError.CANNOT_PULL):
-            git_import.add_repo('file:///foobar.git', None)
+            git_import.add_repo('file:///foobar.git', None, None)
 
         # Test git repo that exists, but is "broken"
         bare_repo = os.path.abspath('{0}/{1}'.format(settings.TEST_ROOT, 'bare.git'))
@@ -101,7 +111,7 @@ class TestGitAddCourse(ModuleStoreTestCase):
                                 cwd=bare_repo)
 
         with self.assertRaisesRegexp(GitImportError, GitImportError.BAD_REPO):
-            git_import.add_repo('file://{0}'.format(bare_repo), None)
+            git_import.add_repo('file://{0}'.format(bare_repo), None, None)
 
     def test_detached_repo(self):
         """
@@ -114,9 +124,89 @@ class TestGitAddCourse(ModuleStoreTestCase):
         except OSError:
             pass
         self.addCleanup(shutil.rmtree, repo_dir)
-        git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite')
+        git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', None)
         subprocess.check_output(['git', 'checkout', 'HEAD~2', ],
                                 stderr=subprocess.STDOUT,
                                 cwd=repo_dir / 'edx4edx_lite')
         with self.assertRaisesRegexp(GitImportError, GitImportError.CANNOT_PULL):
-            git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite')
+            git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', None)
+
+    def test_branching(self):
+        """
+        Exercise branching code of import
+        """
+        repo_dir = getattr(settings, 'GIT_REPO_DIR')
+        # Test successful import from command
+        if not os.path.isdir(repo_dir):
+            os.mkdir(repo_dir)
+        self.addCleanup(shutil.rmtree, repo_dir)
+
+        # Checkout non existent branch
+        with self.assertRaisesRegexp(GitImportError, GitImportError.REMOTE_BRANCH_MISSING):
+            git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', 'asdfasdfasdf')
+
+        # Checkout new branch
+        git_import.add_repo(self.TEST_REPO,
+                            repo_dir / 'edx4edx_lite',
+                            self.TEST_BRANCH)
+        def_ms = modulestore()
+        # Validate that it is different than master
+        self.assertIsNotNone(def_ms.get_course(self.TEST_BRANCH_COURSE))
+
+        # Delete to test branching back to master
+        delete_course(def_ms, contentstore(),
+                      def_ms.get_course(self.TEST_BRANCH_COURSE).location,
+                      True)
+        self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE))
+        git_import.add_repo(self.TEST_REPO,
+                            repo_dir / 'edx4edx_lite',
+                            'master')
+        self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE))
+        self.assertIsNotNone(def_ms.get_course(self.TEST_COURSE))
+
+    def test_branch_exceptions(self):
+        """
+        This wil create conditions to exercise bad paths in the switch_branch function.
+        """
+        # create bare repo that we can mess with and attempt an import
+        bare_repo = os.path.abspath('{0}/{1}'.format(settings.TEST_ROOT, 'bare.git'))
+        os.mkdir(bare_repo)
+        self.addCleanup(shutil.rmtree, bare_repo)
+        subprocess.check_output(['git', '--bare', 'init', ], stderr=subprocess.STDOUT,
+                                cwd=bare_repo)
+
+        # Build repo dir
+        repo_dir = getattr(settings, 'GIT_REPO_DIR')
+        if not os.path.isdir(repo_dir):
+            os.mkdir(repo_dir)
+        self.addCleanup(shutil.rmtree, repo_dir)
+
+        rdir = '{0}/bare'.format(repo_dir)
+        with self.assertRaisesRegexp(GitImportError, GitImportError.BAD_REPO):
+            git_import.add_repo('file://{0}'.format(bare_repo), None, None)
+
+        # Get logger for checking strings in logs
+        output = StringIO.StringIO()
+        test_log_handler = logging.StreamHandler(output)
+        test_log_handler.setLevel(logging.DEBUG)
+        glog = git_import.log
+        glog.addHandler(test_log_handler)
+
+        # Move remote so fetch fails
+        shutil.move(bare_repo, '{0}/not_bare.git'.format(settings.TEST_ROOT))
+        try:
+            git_import.switch_branch('master', rdir)
+        except GitImportError:
+            self.assertIn('Unable to fetch remote', output.getvalue())
+        shutil.move('{0}/not_bare.git'.format(settings.TEST_ROOT), bare_repo)
+        output.truncate(0)
+
+        # Replace origin with a different remote
+        subprocess.check_output(
+            ['git', 'remote', 'rename', 'origin', 'blah', ],
+            stderr=subprocess.STDOUT, cwd=rdir
+        )
+        try:
+            git_import.switch_branch('master', rdir)
+        except GitImportError:
+            self.assertIn('Getting a list of remote branches failed', output.getvalue())

lms/djangoapps/dashboard/sysadmin.py

@@ -272,7 +272,7 @@ class Users(SysadminDashboardView):
             'msg': self.msg,
             'djangopid': os.getpid(),
             'modeflag': {'users': 'active-section'},
-            'mitx_version': getattr(settings, 'VERSION_STRING', ''),
+            'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''),
         }
         return render_to_response(self.template_name, context)
 
@@ -316,7 +316,7 @@ class Users(SysadminDashboardView):
             'msg': self.msg,
             'djangopid': os.getpid(),
             'modeflag': {'users': 'active-section'},
-            'mitx_version': getattr(settings, 'VERSION_STRING', ''),
+            'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''),
         }
         return render_to_response(self.template_name, context)
 
@@ -348,7 +348,7 @@ class Courses(SysadminDashboardView):
 
         return info
 
-    def get_course_from_git(self, gitloc, datatable):
+    def get_course_from_git(self, gitloc, branch, datatable):
         """This downloads and runs the checks for importing a course in git"""
 
         if not (gitloc.endswith('.git') or gitloc.startswith('http:') or
@@ -357,11 +357,11 @@ class Courses(SysadminDashboardView):
                      "and be a valid url")
 
         if self.is_using_mongo:
-            return self.import_mongo_course(gitloc)
+            return self.import_mongo_course(gitloc, branch)
 
-        return self.import_xml_course(gitloc, datatable)
+        return self.import_xml_course(gitloc, branch, datatable)
 
-    def import_mongo_course(self, gitloc):
+    def import_mongo_course(self, gitloc, branch):
         """
         Imports course using management command and captures logging output
         at debug level for display in template
@@ -390,7 +390,7 @@ class Courses(SysadminDashboardView):
 
         error_msg = ''
         try:
-            git_import.add_repo(gitloc, None)
+            git_import.add_repo(gitloc, None, branch)
         except GitImportError as ex:
             error_msg = str(ex)
         ret = output.getvalue()
@@ -411,7 +411,7 @@ class Courses(SysadminDashboardView):
         msg += "<pre>{0}</pre>".format(escape(ret))
         return msg
 
-    def import_xml_course(self, gitloc, datatable):
+    def import_xml_course(self, gitloc, branch, datatable):
         """Imports a git course into the XMLModuleStore"""
 
         msg = u''
@@ -436,13 +436,23 @@ class Courses(SysadminDashboardView):
             cmd_output = escape(
                 subprocess.check_output(cmd, stderr=subprocess.STDOUT, cwd=cwd)
             )
-        except subprocess.CalledProcessError:
-            return _('Unable to clone or pull repository. Please check your url.')
+        except subprocess.CalledProcessError as ex:
+            log.exception('Git pull or clone output was: %r', ex.output)
+            return _('Unable to clone or pull repository. Please check '
+                     'your url. Output was: {0!r}'.format(ex.output))
 
         msg += u'<pre>{0}</pre>'.format(cmd_output)
         if not os.path.exists(gdir):
             msg += _('Failed to clone repository to {0}').format(gdir)
             return msg
+        # Change branch if specified
+        if branch:
+            try:
+                git_import.switch_branch(branch, gdir)
+            except GitImportError as ex:
+                return str(ex)
+            msg += u'<p>{0}: {1}</p>'.format(_('Successfully switched to branch'), branch)
+
         self.def_ms.try_load_course(os.path.abspath(gdir))
         errlog = self.def_ms.errored_courses.get(cdir, '')
         if errlog:
@@ -494,7 +504,7 @@ class Courses(SysadminDashboardView):
             'msg': self.msg,
             'djangopid': os.getpid(),
             'modeflag': {'courses': 'active-section'},
-            'mitx_version': getattr(settings, 'VERSION_STRING', ''),
+            'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''),
         }
         return render_to_response(self.template_name, context)
 
@@ -511,8 +521,9 @@ class Courses(SysadminDashboardView):
         courses = self.get_courses()
         if action == 'add_course':
             gitloc = request.POST.get('repo_location', '').strip().replace(' ', '').replace(';', '')
+            branch = request.POST.get('repo_branch', '').strip().replace(' ', '').replace(';', '')
             datatable = self.make_datatable()
-            self.msg += self.get_course_from_git(gitloc, datatable)
+            self.msg += self.get_course_from_git(gitloc, branch, datatable)
 
         elif action == 'del_course':
             course_id = request.POST.get('course_id', '').strip()
@@ -563,7 +574,7 @@ class Courses(SysadminDashboardView):
             'msg': self.msg,
             'djangopid': os.getpid(),
             'modeflag': {'courses': 'active-section'},
-            'mitx_version': getattr(settings, 'VERSION_STRING', ''),
+            'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''),
         }
         return render_to_response(self.template_name, context)
 
@@ -602,7 +613,7 @@ class Staffing(SysadminDashboardView):
             'msg': self.msg,
             'djangopid': os.getpid(),
             'modeflag': {'staffing': 'active-section'},
-            'mitx_version': getattr(settings, 'VERSION_STRING', ''),
+            'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''),
         }
         return render_to_response(self.template_name, context)
 

lms/djangoapps/dashboard/tests/test_sysadmin.py

@@ -45,6 +45,10 @@ class SysadminBaseTestCase(ModuleStoreTestCase):
     Base class with common methods used in XML and Mongo tests
     """
 
+    TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git'
+    TEST_BRANCH = 'testing_do_not_delete'
+    TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx'
+
     def setUp(self):
         """Setup test case by adding primary user."""
         super(SysadminBaseTestCase, self).setUp()
@@ -58,11 +62,12 @@ class SysadminBaseTestCase(ModuleStoreTestCase):
         GlobalStaff().add_users(self.user)
         self.client.login(username=self.user.username, password='foo')
 
-    def _add_edx4edx(self):
+    def _add_edx4edx(self, branch=None):
         """Adds the edx4edx sample course"""
-        return self.client.post(reverse('sysadmin_courses'), {
-            'repo_location': 'https://github.com/mitocw/edx4edx_lite.git',
-            'action': 'add_course', })
+        post_dict = {'repo_location': self.TEST_REPO, 'action': 'add_course', }
+        if branch:
+            post_dict['repo_branch'] = branch
+        return self.client.post(reverse('sysadmin_courses'), post_dict)
 
     def _rm_edx4edx(self):
         """Deletes the sample course from the XML store"""
@@ -301,11 +306,24 @@ class TestSysadmin(SysadminBaseTestCase):
         self.assertIsNotNone(course)
 
         # Delete a course
-        response = self._rm_edx4edx()
+        self._rm_edx4edx()
         course = def_ms.courses.get('{0}/edx4edx_lite'.format(
             os.path.abspath(settings.DATA_DIR)), None)
         self.assertIsNone(course)
 
+        # Load a bad git branch
+        response = self._add_edx4edx('asdfasdfasdf')
+        self.assertIn(GitImportError.REMOTE_BRANCH_MISSING,
+                      response.content.decode('utf-8'))
+
+        # Load a course from a git branch
+        self._add_edx4edx(self.TEST_BRANCH)
+        course = def_ms.courses.get('{0}/edx4edx_lite'.format(
+            os.path.abspath(settings.DATA_DIR)), None)
+        self.assertIsNotNone(course)
+        self.assertIn(self.TEST_BRANCH_COURSE, course.location.course_id)
+        self._rm_edx4edx()
+
         # Try and delete a non-existent course
         response = self.client.post(reverse('sysadmin_courses'),
                                     {'course_id': 'foobar/foo/blah',

lms/templates/sysadmin_dashboard.html

@@ -126,10 +126,16 @@ textarea {
   <ul class="list-input">
 	<li class="field text">
 	  <label for="repo_location">
-		${_('Repo location')}:
+		${_('Repo Location')}:
 	  </label>
 	  <input type="text" name="repo_location" style="width:60%" />
 	</li>
+	<li class="field text">
+	  <label for="repo_location">
+		${_('Repo Branch (optional)')}:
+	  </label>
+	  <input type="text" name="repo_branch" style="width:60%" />
+	</li>
   </ul>
   <div class="form-actions">
 	<button type="submit" name="action" value="add_course">${_('Load new course from github')}</button>
@@ -201,6 +207,6 @@ textarea {
 
   </section>
   <div style="text-align:right; float: right"><span id="djangopid">${_('Django PID')}: ${djangopid}</span>
-  | <span id="mitxver">${_('Platform Version')}: ${mitx_version}</span></div>
+  | <span id="edxver">${_('Platform Version')}: ${edx_platform_version}</span></div>
 </div>
 </section>