also include patch files in --new-pr and --update-pr

easybuild/framework/easyconfig/easyconfig.py

@@ -60,7 +60,7 @@
 from easybuild.toolchains.gcccore import GCCcore
 from easybuild.tools.build_log import EasyBuildError
 from easybuild.tools.config import build_option, get_module_naming_scheme
-from easybuild.tools.filetools import decode_class_name, encode_class_name, mkdir, read_file, write_file
+from easybuild.tools.filetools import copy_file, decode_class_name, encode_class_name, mkdir, read_file, write_file
 from easybuild.tools.module_naming_scheme import DEVEL_MODULE_SUFFIX
 from easybuild.tools.module_naming_scheme.utilities import avail_module_naming_schemes, det_full_ec_version
 from easybuild.tools.module_naming_scheme.utilities import det_hidden_modname, is_valid_module_name
@@ -1365,55 +1365,84 @@ def robot_find_minimal_toolchain_of_dependency(dep, modtool, parent_tc=None):
     return minimal_toolchain
 
 
+def det_location_for(path, target_dir, soft_name, target_file):
+    """
+    Determine path to easyconfigs directory for specified software name, using specified target file name.
+
+    :param path: path of file to copy
+    :param target_dir: (parent) target directory, should contain easybuild/easyconfigs subdirectory
+    :param soft_name: software name (to determine location to copy to)
+    :param target_file: target file name
+    :return: full path to the right location
+    """
+    subdir = os.path.join('easybuild', 'easyconfigs')
+
+    if os.path.exists(os.path.join(target_dir, subdir)):
+        letter = soft_name.lower()[0]
+        if letter not in [chr(i) for i in range(ord('a'), ord('z') + 1)]:
+            raise EasyBuildError("Don't know which letter subdir to use for software name %s", soft_name)
+
+        target_path = os.path.join('easybuild', 'easyconfigs', letter, soft_name, target_file)
+        _log.debug("Target path for %s: %s", path, target_path)
+
+        target_path = os.path.join(target_dir, target_path)
+
+    else:
+        raise EasyBuildError("Subdirectory %s not found in %s", subdir, target_dir)
+
+    return target_path
+
+
 def copy_easyconfigs(paths, target_dir):
     """
     Copy easyconfig files to specified directory, in the 'right' location and using the filename expected by robot.
 
-    @paths: list of paths to copy to git working dir
-    @target_dir: target directory
-    @return: dict with useful information on copied easyconfig files (corresponding EasyConfig instances, paths, status)
+    :param paths: list of paths to copy to git working dir
+    :param target_dir: target directory
+    :return: dict with useful information on copied easyconfig files (corresponding EasyConfig instances, paths, status)
     """
     file_info = {
         'ecs': [],
         'paths_in_repo': [],
         'new': [],
     }
 
-    a_to_z = [chr(i) for i in range(ord('a'), ord('z') + 1)]
-    subdir = os.path.join('easybuild', 'easyconfigs')
+    for path in paths:
+        ecs = process_easyconfig(path, validate=False)
+        if len(ecs) == 1:
+            file_info['ecs'].append(ecs[0]['ec'])
 
-    if os.path.exists(os.path.join(target_dir, subdir)):
-        for path in paths:
-            ecs = process_easyconfig(path, validate=False)
-            if len(ecs) == 1:
-                file_info['ecs'].append(ecs[0]['ec'])
-                name = file_info['ecs'][-1].name
-                ec_filename = '%s-%s.eb' % (name, det_full_ec_version(file_info['ecs'][-1]))
+            soft_name = file_info['ecs'][-1].name
+            ec_filename = '%s-%s.eb' % (soft_name, det_full_ec_version(file_info['ecs'][-1]))
 
-                letter = name.lower()[0]
-                if letter not in a_to_z:
-                    raise EasyBuildError("Don't know which letter subdir to use for %s", name)
+            target_path = det_location_for(path, target_dir, soft_name, ec_filename)
 
-                target_path = os.path.join(subdir, letter, name, ec_filename)
-                _log.debug("Target path for %s: %s", path, target_path)
+            copy_file(path, target_path)
+            file_info['paths_in_repo'].append(target_path)
+            file_info['new'].append(os.path.exists(target_path))
 
-                full_target_path = os.path.join(target_dir, target_path)
-                try:
-                    file_info['new'].append(not os.path.exists(full_target_path))
+        else:
+            raise EasyBuildError("Multiple EasyConfig instances obtained from easyconfig file %s", path)
 
-                    mkdir(os.path.dirname(full_target_path), parents=True)
-                    shutil.copy2(path, full_target_path)
-                    _log.info("%s copied to %s", path, full_target_path)
-                except OSError as err:
-                    raise EasyBuildError("Failed to copy %s to %s: %s", path, target_path, err)
+    return file_info
 
-                file_info['paths_in_repo'].append(target_path)
-            else:
-                raise EasyBuildError("Multiple EasyConfig instances obtained from easyconfig file %s", path)
-    else:
-        raise EasyBuildError("Subdirectory %s not found in %s", subdir, target_dir)
 
-    return file_info
+def copy_patch_files(patch_specs, target_dir):
+    """
+    Copy patch files to specified directory, in the 'right' location according to the software name they relate to.
+
+    :param patch_specs: list of tuples with patch file location and name of software they are for
+    :param target_dir: target directory
+    """
+    patched_files = {
+        'paths_in_repo': [],
+    }
+    for patch_path, soft_name in patch_specs:
+        target_path = det_location_for(patch_path, target_dir, soft_name, os.path.basename(patch_path))
+        copy_file(patch_path, target_path)
+        patched_files['paths_in_repo'].append(target_path)
+
+    return patched_files
 
 
 class ActiveMNS(object):

easybuild/tools/filetools.py

@@ -593,6 +593,12 @@ def extract_cmd(filepath, overwrite=False):
     return cmd_tmpl % {'filepath': filepath, 'target': target}
 
 
+def is_patch_file(path):
+    """Determine whether file at specified path is a patch file (based on +++ and --- lines being present)."""
+    txt = read_file(path)
+    return bool(re.search(r'^\+{3}\s', txt, re.M) and re.search(r'^-{3}\s', txt, re.M))
+
+
 def det_patched_files(path=None, txt=None, omit_ab_prefix=False, github=False, filter_deleted=False):
     """
     Determine list of patched files from a patch.
@@ -1324,3 +1330,18 @@ def find_flexlm_license(custom_env_vars=None, lic_specs=None):
     _log.info("Found valid license specs via %s: %s", via_msg, valid_lic_specs)
 
     return (valid_lic_specs, lic_env_var)
+
+
+def copy_file(path, target_path):
+    """
+    Copy a file from path to target_path
+    :param path: the original filepath
+    :param target_path: path to copy the file to
+    """
+
+    try:
+        mkdir(os.path.dirname(target_path), parents=True)
+        shutil.copy2(path, target_path)
+        _log.info("%s copied to %s", path, target_path)
+    except OSError as err:
+        raise EasyBuildError("Failed to copy %s to %s: %s", path, target_path, err)

easybuild/tools/github.py

@@ -47,11 +47,13 @@
 from vsc.utils import fancylogger
 from vsc.utils.missing import nub
 
-from easybuild.framework.easyconfig.easyconfig import copy_easyconfigs
+from easybuild.framework.easyconfig.easyconfig import copy_easyconfigs, copy_patch_files, process_easyconfig
+from easybuild.framework.easyconfig.format.one import EB_FORMAT_EXTENSION
+from easybuild.framework.easyconfig.format.yeb import YEB_FORMAT_EXTENSION
 from easybuild.tools.build_log import EasyBuildError, print_msg
 from easybuild.tools.config import build_option
 from easybuild.tools.filetools import apply_patch, det_patched_files, download_file, extract_file
-from easybuild.tools.filetools import mkdir, read_file, which, write_file
+from easybuild.tools.filetools import is_patch_file, mkdir, read_file, which, write_file
 from easybuild.tools.systemtools import UNKNOWN, get_tool_version
 from easybuild.tools.utilities import only_if_module_is_available
 
@@ -627,6 +629,22 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco
 
     _log.debug("git status: %s", git_repo.git.status())
 
+    # seperate easyconfigs and patch files
+    ec_paths = [p for p in existing_paths if not is_patch_file(p)]
+    patch_paths = [p for p in existing_paths if p not in ec_paths]
+
+    # copy easyconfig files to right place
+    target_dir = os.path.join(git_working_dir, pr_target_repo)
+    print_msg("copying easyconfigs to %s..." % target_dir)
+    file_info = copy_easyconfigs(ec_paths, target_dir)
+
+    # figure out to which software name patches relate, and copy them to the right place
+    if patch_paths:
+        patch_specs = det_patch_specs(patch_paths, file_info)
+
+        print_msg("copying patch files to %s..." % target_dir)
+        patch_info = copy_patch_files(patch_specs, target_dir)
+
     # determine path to files to delete (if any)
     deleted_paths = []
     for fn in delete_files:
@@ -641,17 +659,14 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco
             else:
                 raise EasyBuildError("Path doesn't exist or file to delete isn't found in target branch: %s", fn)
 
-    # copy edited/added files to right place
-    file_info = copy_easyconfigs(existing_paths, repo_path)
-
-    if existing_paths:
-        name_version = file_info['ecs'][0].name + string.translate(file_info['ecs'][0].version, None, '-.')
-    else:
-        name_version = os.path.basename(delete_files[0])
-
     # checkout target branch
     if pr_branch is None:
-        pr_branch = '%s_new_pr_%s' % (time.strftime("%Y%m%d%H%M%S"), name_version)
+        if ec_paths:
+            label = file_info['ecs'][0].name + string.translate(file_info['ecs'][0].version, None, '-.')
+        else:
+            label = ''.join(random.choice(string.letters) for _ in range(10))
+        pr_branch = '%s_new_pr_%s' % (time.strftime("%Y%m%d%H%M%S"), label)
+
 
     # create branch to commit to and push;
     # use force to avoid errors if branch already exists (OK since this is a local temporary copy of the repo)
@@ -662,6 +677,10 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco
     _log.debug("Staging all %d new/modified easyconfigs", len(file_info['paths_in_repo']))
     git_repo.index.add(file_info['paths_in_repo'])
 
+    if patch_paths:
+        _log.debug("Staging all %d new/modified patch files", len(patch_info['paths_in_repo']))
+        git_repo.index.add(patch_info['paths_in_repo'])
+
     # stage deleted files
     if deleted_paths:
         git_repo.index.remove(deleted_paths)
@@ -716,6 +735,89 @@ def _easyconfigs_pr_common(paths, start_branch=None, pr_branch=None, target_acco
     return file_info, deleted_paths, git_repo, pr_branch, diff_stat
 
 
+def det_patch_specs(patch_paths, file_info):
+    """ Determine software names for patch files """
+    print_msg("determining software names for patch files...")
+    patch_specs = []
+    for patch_path in patch_paths:
+        soft_name = None
+        patch_file = os.path.basename(patch_path)
+
+        # consider patch lists of easyconfigs being provided
+        for ec in file_info['ecs']:
+            if patch_file in ec['patches']:
+                soft_name = ec.name
+                break
+
+        if soft_name:
+            patch_specs.append((patch_path, soft_name))
+        else:
+            # fall back on scanning all eb files for patches
+            print "Matching easyconfig for %s not found on the first try:" % patch_path,
+            print "scanning all easyconfigs to determine where patch file belongs (this may take a while)..."
+            soft_name = find_software_name_for_patch(patch_file)
+            if soft_name:
+                patch_specs.append((patch_path, soft_name))
+            else:
+                # still nothing found
+                raise EasyBuildError("Failed to determine software name to which patch file %s relates", patch_path)
+
+    return patch_specs
+
+
+def find_software_name_for_patch(patch_name):
+    """
+    Scan all easyconfigs in the robot path(s) to determine which software a patch file belongs to
+
+    :param patch_name: name of the patch file
+    :return: name of the software that this patch file belongs to (if found)
+    """
+
+    def is_patch_for(patch_name, ec):
+        """Check whether specified patch matches any patch in the provided EasyConfig instance."""
+        res = False
+        for patch in ec['patches']:
+            if isinstance(patch, (tuple, list)):
+                patch = patch[0]
+            if patch == patch_name:
+                res = True
+                break
+
+        return res
+
+    robot_paths = build_option('robot_path')
+    soft_name = None
+
+    all_ecs = []
+    for robot_path in robot_paths:
+        for (dirpath, _, filenames) in os.walk(robot_path):
+            for fn in filenames:
+                if fn != 'TEMPLATE.eb':
+                    path = os.path.join(dirpath, fn)
+                    rawtxt = read_file(path)
+                    if 'patches' in rawtxt:
+                        all_ecs.append(path)
+
+    nr_of_ecs = len(all_ecs)
+    for idx, path in enumerate(all_ecs):
+        if soft_name:
+            break
+        rawtxt = read_file(path)
+        try:
+            ecs = process_easyconfig(path, validate=False)
+            for ec in ecs:
+                if is_patch_for(patch_name, ec['ec']):
+                    soft_name = ec['ec']['name']
+                    break
+        except EasyBuildError as err:
+            _log.debug("Ignoring easyconfig %s that fails to parse: %s", path, err)
+        sys.stdout.write('\r%s of %s easyconfigs checked' % (idx+1, nr_of_ecs))
+        sys.stdout.flush()
+
+    sys.stdout.write('\n')
+    return soft_name
+
+
 @only_if_module_is_available('git', pkgname='GitPython')
 def new_pr(paths, title=None, descr=None, commit_msg=None):
     """Open new pull request using specified files."""
@@ -752,6 +854,7 @@ def new_pr(paths, title=None, descr=None, commit_msg=None):
     classes_counted = sorted([(classes.count(c), c) for c in nub(classes)])
     class_label = ','.join([tc for (cnt, tc) in classes_counted if cnt == classes_counted[-1][0]])
 
+
     if title is None:
         if file_info['ecs'] and all(file_info['new']) and not deleted_paths:
             # mention software name/version in PR title (only first 3)
@@ -762,7 +865,6 @@ def new_pr(paths, title=None, descr=None, commit_msg=None):
                 main_title = ', '.join(names_and_versions[:3] + ['...'])
 
             title = "{%s}[%s] %s" % (class_label, toolchain_label, main_title)
-
         else:
             raise EasyBuildError("Don't know how to make a PR title for this PR. "
                                  "Please include a title (use --pr-title)")

test/framework/filetools.py

@@ -672,6 +672,12 @@ def test_find_flexlm_license(self):
         del os.environ['LM_LICENSE_FILE']
         self.assertEqual(ft.find_flexlm_license(lic_specs=[None]), ([], None))
 
+    def test_is_patch_file(self):
+        """Test for is_patch_file() function."""
+        testdir = os.path.dirname(os.path.abspath(__file__))
+        self.assertFalse(ft.is_patch_file(os.path.join(testdir, 'easyconfigs', 'toy-0.0.eb')))
+        self.assertTrue(ft.is_patch_file(os.path.join(testdir, 'sandbox', 'sources', 'toy', 'toy-0.0_typo.patch')))
+
     def test_is_alt_pypi_url(self):
         """Test is_alt_pypi_url() function."""
         url = 'https://pypi.python.org/packages/source/e/easybuild/easybuild-2.7.0.tar.gz'
@@ -709,6 +715,16 @@ def test_apply_patch(self):
         # trying the patch again should fail
         self.assertErrorRegex(EasyBuildError, "Couldn't apply patch file", ft.apply_patch, toy_patch, path)
 
+    def test_copy_file(self):
+        """ Test copy_file """
+        testdir = os.path.dirname(os.path.abspath(__file__))
+        tmpdir = self.test_prefix
+        to_copy = os.path.join(testdir, 'easyconfigs', 'toy-0.0.eb')
+        target_path = os.path.join(tmpdir, 'toy-0.0.eb')
+        ft.copy_file(to_copy, target_path)
+        self.assertTrue(os.path.exists(target_path))
+        self.assertTrue(ft.read_file(to_copy) == ft.read_file(target_path))
+
 
 def suite():
     """ returns all the testcases in this module """