Merge pull request #3206 from migueldiascosta/include_easyblocks_from_pr

add support for --include-easyblocks-from-pr (REVIEW)

easybuild/tools/github.py

@@ -85,6 +85,7 @@
 GITHUB_API_URL = 'https://api.github.com'
 GITHUB_DIR_TYPE = u'dir'
 GITHUB_EB_MAIN = 'easybuilders'
+GITHUB_EASYBLOCKS_REPO = 'easybuild-easyblocks'
 GITHUB_EASYCONFIGS_REPO = 'easybuild-easyconfigs'
 GITHUB_DEVELOP_BRANCH = 'develop'
 GITHUB_FILE_TYPE = u'file'
@@ -369,13 +370,32 @@ def download_repo(repo=GITHUB_EASYCONFIGS_REPO, branch='master', account=GITHUB_
     return extracted_path
 
 
+def fetch_easyblocks_from_pr(pr, path=None, github_user=None):
+    """Fetch patched easyconfig files for a particular PR."""
+    return fetch_files_from_pr(pr, path, github_user, github_repo=GITHUB_EASYBLOCKS_REPO)
+
+
 def fetch_easyconfigs_from_pr(pr, path=None, github_user=None):
     """Fetch patched easyconfig files for a particular PR."""
+    return fetch_files_from_pr(pr, path, github_user, github_repo=GITHUB_EASYCONFIGS_REPO)
+
+
+def fetch_files_from_pr(pr, path=None, github_user=None, github_repo=None):
+    """Fetch patched files for a particular PR."""
 
     if github_user is None:
         github_user = build_option('github_user')
+
+    if github_repo is None:
+        github_repo = GITHUB_EASYCONFIGS_REPO
+
     if path is None:
-        path = build_option('pr_path')
+        if github_repo == GITHUB_EASYCONFIGS_REPO:
+            path = build_option('pr_path')
+        elif github_repo == GITHUB_EASYBLOCKS_REPO:
+            path = os.path.join(tempfile.gettempdir(), 'ebs_pr%s' % pr)
+        else:
+            raise EasyBuildError("Unknown repo: %s" % github_repo)
 
     if path is None:
         path = tempfile.mkdtemp()
@@ -384,9 +404,17 @@ def fetch_easyconfigs_from_pr(pr, path=None, github_user=None):
         mkdir(path, parents=True)
 
     github_account = build_option('pr_target_account')
-    github_repo = GITHUB_EASYCONFIGS_REPO
 
-    _log.debug("Fetching easyconfigs from %s/%s PR #%s into %s", github_account, github_repo, pr, path)
+    if github_repo == GITHUB_EASYCONFIGS_REPO:
+        easyfiles = 'easyconfigs'
+    elif github_repo == GITHUB_EASYBLOCKS_REPO:
+        easyfiles = 'easyblocks'
+    else:
+        raise EasyBuildError("Don't know how to fetch files from repo %s", github_repo)
+
+    subdir = os.path.join('easybuild', easyfiles)
+
+    _log.debug("Fetching %s from %s/%s PR #%s into %s", easyfiles, github_account, github_repo, pr, path)
     pr_data, _ = fetch_pr_data(pr, github_account, github_repo, github_user)
 
     pr_merged = pr_data['merged']
@@ -429,12 +457,12 @@ def fetch_easyconfigs_from_pr(pr, path=None, github_user=None):
     if final_path is None:
 
         if pr_closed:
-            print_warning("Using easyconfigs from closed PR #%s" % pr)
+            print_warning("Using %s from closed PR #%s" % (easyfiles, pr))
 
         # obtain most recent version of patched files
-        for patched_file in patched_files:
+        for patched_file in [f for f in patched_files if subdir in f]:
             # path to patch file, incl. subdir it is in
-            fn = os.path.sep.join(patched_file.split(os.path.sep)[-3:])
+            fn = patched_file.split(subdir)[1].strip(os.path.sep)
             sha = pr_data['head']['sha']
             full_url = URL_SEPARATOR.join([GITHUB_RAW, github_account, github_repo, sha, patched_file])
             _log.info("Downloading %s from %s", fn, full_url)
@@ -444,21 +472,21 @@ def fetch_easyconfigs_from_pr(pr, path=None, github_user=None):
 
     # symlink directories into expected place if they're not there yet
     if final_path != path:
-        dirpath = os.path.join(final_path, 'easybuild', 'easyconfigs')
+        dirpath = os.path.join(final_path, subdir)
         for eb_dir in os.listdir(dirpath):
             symlink(os.path.join(dirpath, eb_dir), os.path.join(path, os.path.basename(eb_dir)))
 
     # sanity check: make sure all patched files are downloaded
-    ec_files = []
-    for patched_file in [f for f in patched_files if not f.startswith('test/')]:
-        fn = os.path.sep.join(patched_file.split(os.path.sep)[-3:])
+    files = []
+    for patched_file in [f for f in patched_files if subdir in f]:
+        fn = patched_file.split(easyfiles)[1].strip(os.path.sep)
         full_path = os.path.join(path, fn)
         if os.path.exists(full_path):
-            ec_files.append(full_path)
+            files.append(full_path)
         else:
             raise EasyBuildError("Couldn't find path to patched file %s", full_path)
 
-    return ec_files
+    return files
 
 
 def create_gist(txt, fn, descr=None, github_user=None, github_token=None):

easybuild/tools/include.py

@@ -31,6 +31,7 @@
 import os
 import re
 import sys
+import tempfile
 
 from easybuild.base import fancylogger
 from easybuild.tools.build_log import EasyBuildError
@@ -147,14 +148,17 @@ def is_software_specific_easyblock(module):
 
 def include_easyblocks(tmpdir, paths):
     """Include generic and software-specific easyblocks found in specified locations."""
-    easyblocks_path = os.path.join(tmpdir, 'included-easyblocks')
+    easyblocks_path = tempfile.mkdtemp(dir=tmpdir, prefix='included-easyblocks-')
 
     set_up_eb_package(easyblocks_path, 'easybuild.easyblocks',
                       subpkgs=['generic'], pkg_init_body=EASYBLOCKS_PKG_INIT_BODY)
 
     easyblocks_dir = os.path.join(easyblocks_path, 'easybuild', 'easyblocks')
 
-    allpaths = [p for p in expand_glob_paths(paths) if os.path.basename(p) != '__init__.py']
+    allpaths = [p for p in expand_glob_paths(paths)
+                if os.path.basename(p).endswith('.py') and
+                os.path.basename(p) != '__init__.py']
+
     for easyblock_module in allpaths:
         filename = os.path.basename(easyblock_module)
 

easybuild/tools/options.py

@@ -80,7 +80,7 @@
 from easybuild.tools.github import GITHUB_PR_DIRECTION_DESC, GITHUB_PR_ORDER_CREATED, GITHUB_PR_STATE_OPEN
 from easybuild.tools.github import GITHUB_PR_STATES, GITHUB_PR_ORDERS, GITHUB_PR_DIRECTIONS
 from easybuild.tools.github import HAVE_GITHUB_API, HAVE_KEYRING, VALID_CLOSE_PR_REASONS
-from easybuild.tools.github import fetch_github_token
+from easybuild.tools.github import fetch_easyblocks_from_pr, fetch_github_token
 from easybuild.tools.hooks import KNOWN_HOOKS
 from easybuild.tools.include import include_easyblocks, include_module_naming_schemes, include_toolchains
 from easybuild.tools.job.backend import avail_job_backends
@@ -601,6 +601,8 @@ def github_options(self):
             'git-working-dirs-path': ("Path to Git working directories for EasyBuild repositories", str, 'store', None),
             'github-user': ("GitHub username", str, 'store', None),
             'github-org': ("GitHub organization", str, 'store', None),
+            'include-easyblocks-from-pr': ("Include easyblocks from specified PR", int, 'store', None,
+                                           {'metavar': 'PR#'}),
             'install-github-token': ("Install GitHub token (requires --github-user)", None, 'store_true', False),
             'close-pr': ("Close pull request", int, 'store', None, {'metavar': 'PR#'}),
             'close-pr-msg': ("Custom close message for pull request closed with --close-pr; ", str, 'store', None),
@@ -934,7 +936,7 @@ def _postprocess_checks(self):
         """Check whether (combination of) configuration options make sense."""
 
         # fail early if required dependencies for functionality requiring using GitHub API are not available:
-        if self.options.from_pr or self.options.upload_test_report:
+        if self.options.from_pr or self.options.include_easyblocks_from_pr or self.options.upload_test_report:
             if not HAVE_GITHUB_API:
                 raise EasyBuildError("Required support for using GitHub API is not available (see warnings)")
 
@@ -1052,8 +1054,8 @@ def _postprocess_list_avail(self):
         if self.options.avail_easyconfig_licenses:
             msg += avail_easyconfig_licenses(self.options.output_format)
 
-        # dump available easyblocks
-        if self.options.list_easyblocks:
+        # dump available easyblocks (unless including easyblocks from pr, in which case it will be done later)
+        if self.options.list_easyblocks and not self.options.include_easyblocks_from_pr:
             msg += list_easyblocks(self.options.list_easyblocks, self.options.output_format)
 
         # dump known toolchains
@@ -1097,7 +1099,8 @@ def _postprocess_list_avail(self):
             print(msg)
 
         # cleanup tmpdir and exit
-        cleanup_and_exit(self.tmpdir)
+        if not self.options.include_easyblocks_from_pr:
+            cleanup_and_exit(self.tmpdir)
 
     def avail_repositories(self):
         """Show list of known repository types."""
@@ -1414,6 +1417,29 @@ def set_up_configuration(args=None, logfile=None, testing=False, silent=False):
     init(options, config_options_dict)
     init_build_options(build_options=build_options, cmdline_options=options)
 
+    # done here instead of in _postprocess_include because github integration requires build_options to be initialized
+    if eb_go.options.include_easyblocks_from_pr:
+        easyblocks_from_pr = fetch_easyblocks_from_pr(eb_go.options.include_easyblocks_from_pr)
+
+        if eb_go.options.include_easyblocks:
+            # make sure we're not including the same easyblock twice
+            included_from_pr = set([os.path.basename(eb) for eb in easyblocks_from_pr])
+            included_from_file = set([os.path.basename(eb) for eb in eb_go.options.include_easyblocks])
+            included_twice = included_from_pr & included_from_file
+            if included_twice:
+                raise EasyBuildError("Multiple inclusion of %s, check your --include-easyblocks options",
+                                     ','.join(included_twice))
+
+        include_easyblocks(eb_go.options.tmpdir, easyblocks_from_pr)
+
+        if eb_go.options.list_easyblocks:
+            msg = list_easyblocks(eb_go.options.list_easyblocks, eb_go.options.output_format)
+            if eb_go.options.unittest_file:
+                log.info(msg)
+            else:
+                print(msg)
+            cleanup_and_exit(tmpdir)
+
     check_python_version()
 
     # move directory containing fake vsc namespace into temporary directory used for this session

test/framework/github.py

@@ -245,6 +245,33 @@ def test_close_pr(self):
         for pattern in patterns:
             self.assertTrue(pattern in stdout, "Pattern '%s' found in: %s" % (pattern, stdout))
 
+    def test_fetch_easyblocks_from_pr(self):
+        """Test fetch_easyblocks_from_pr function."""
+        if self.skip_github_tests:
+            print("Skipping test_fetch_easyblocks_from_pr, no GitHub token available?")
+            return
+
+        init_config(build_options={
+            'pr_target_account': gh.GITHUB_EB_MAIN,
+        })
+
+        # PR with new easyblock plus non-easyblock file
+        all_ebs_pr1964 = ['lammps.py']
+
+        # PR with changed easyblock
+        all_ebs_pr1967 = ['siesta.py']
+
+        # PR with more than one easyblock
+        all_ebs_pr1949 = ['configuremake.py', 'rpackage.py']
+
+        for pr, all_ebs in [(1964, all_ebs_pr1964), (1967, all_ebs_pr1967), (1949, all_ebs_pr1949)]:
+            try:
+                tmpdir = os.path.join(self.test_prefix, 'pr%s' % pr)
+                eb_files = gh.fetch_easyblocks_from_pr(pr, path=tmpdir, github_user=GITHUB_TEST_ACCOUNT)
+                self.assertEqual(sorted(all_ebs), sorted([os.path.basename(f) for f in eb_files]))
+            except URLError as err:
+                print("Ignoring URLError '%s' in test_fetch_easyblocks_from_pr" % err)
+
     def test_fetch_easyconfigs_from_pr(self):
         """Test fetch_easyconfigs_from_pr function."""
         if self.skip_github_tests:

test/framework/options.py

@@ -2471,7 +2471,8 @@ def test_xxx_include_easyblocks(self):
         self.eb_main(args, logfile=dummylogfn, raise_error=True)
         logtxt = read_file(self.logfile)
 
-        path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks', 'easybuild', 'easyblocks', 'foo.py')
+        path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks',
+                                    'foo.py')
         foo_regex = re.compile(r"^\|-- EB_foo \(easybuild.easyblocks.foo @ %s\)" % path_pattern, re.M)
         self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt))
 
@@ -2514,7 +2515,7 @@ def test_xxx_include_generic_easyblocks(self):
         self.eb_main(args, logfile=dummylogfn, raise_error=True)
         logtxt = read_file(self.logfile)
 
-        path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks', 'easybuild', 'easyblocks',
+        path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks',
                                     'generic', 'foobar.py')
         foo_regex = re.compile(r"^\|-- FooBar \(easybuild.easyblocks.generic.foobar @ %s\)" % path_pattern, re.M)
         self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt))
@@ -2552,7 +2553,7 @@ def test_xxx_include_generic_easyblocks(self):
         logtxt = read_file(self.logfile)
 
         mod_pattern = 'easybuild.easyblocks.generic.generictest'
-        path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks', 'easybuild', 'easyblocks',
+        path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks',
                                     'generic', 'generictest.py')
         foo_regex = re.compile(r"^\|-- GenericTest \(%s @ %s\)" % (mod_pattern, path_pattern), re.M)
         self.assertTrue(foo_regex.search(logtxt), "Pattern '%s' found in: %s" % (foo_regex.pattern, logtxt))
@@ -2563,6 +2564,113 @@ def test_xxx_include_generic_easyblocks(self):
         # 'undo' import of foo easyblock
         del sys.modules['easybuild.easyblocks.generic.generictest']
 
+    # must be run after test for --list-easyblocks, hence the '_xxx_'
+    # cleaning up the imported easyblocks is quite difficult...
+    def test_xxx_include_easyblocks_from_pr(self):
+        """Test --include-easyblocks-from-pr."""
+        if self.github_token is None:
+            print("Skipping test_preview_pr, no GitHub token available?")
+            return
+
+        orig_local_sys_path = sys.path[:]
+        fd, dummylogfn = tempfile.mkstemp(prefix='easybuild-dummy', suffix='.log')
+        os.close(fd)
+
+        # clear log
+        write_file(self.logfile, '')
+
+        # include extra test easyblock
+        foo_txt = '\n'.join([
+            'from easybuild.framework.easyblock import EasyBlock',
+            'class EB_foo(EasyBlock):',
+            '   pass',
+            ''
+        ])
+        write_file(os.path.join(self.test_prefix, 'foo.py'), foo_txt)
+
+        args = [
+            '--include-easyblocks=%s/*.py' % self.test_prefix,  # this shouldn't interfere
+            '--include-easyblocks-from-pr=1915',  # a PR for CMakeMake easyblock
+            '--list-easyblocks=detailed',
+            '--unittest-file=%s' % self.logfile,
+            '--github-user=%s' % GITHUB_TEST_ACCOUNT,
+        ]
+        self.eb_main(args, logfile=dummylogfn, raise_error=True)
+        logtxt = read_file(self.logfile)
+
+        # easyblock included from pr is found
+        path_pattern = os.path.join(self.test_prefix, '.*', 'included-easyblocks-.*', 'easybuild', 'easyblocks')
+        cmm_pattern = os.path.join(path_pattern, 'generic', 'cmakemake.py')
+        cmm_regex = re.compile(r"\|-- CMakeMake \(easybuild.easyblocks.generic.cmakemake @ %s\)" % cmm_pattern, re.M)
+        self.assertTrue(cmm_regex.search(logtxt), "Pattern '%s' found in: %s" % (cmm_regex.pattern, logtxt))
+
+        # easyblock is found via get_easyblock_class
+        klass = get_easyblock_class('CMakeMake')
+        self.assertTrue(issubclass(klass, EasyBlock), "%s is an EasyBlock derivative class" % klass)
+
+        # 'undo' import of easyblocks
+        del sys.modules['easybuild.easyblocks.foo']
+        del sys.modules['easybuild.easyblocks.generic.cmakemake']
+        os.remove(os.path.join(self.test_prefix, 'foo.py'))
+        sys.path = orig_local_sys_path
+        import easybuild.easyblocks
+        reload(easybuild.easyblocks)
+        import easybuild.easyblocks.generic
+        reload(easybuild.easyblocks.generic)
+
+        # include test cmakemake easyblock
+        cmm_txt = '\n'.join([
+            'from easybuild.framework.easyblock import EasyBlock',
+            'class CMakeMake(EasyBlock):',
+            '   pass',
+            ''
+        ])
+        write_file(os.path.join(self.test_prefix, 'cmakemake.py'), cmm_txt)
+
+        # including the same easyblock twice should fail
+        args = [
+            '--include-easyblocks=%s/cmakemake.py' % self.test_prefix,
+            '--include-easyblocks-from-pr=1915',
+            '--list-easyblocks=detailed',
+            '--unittest-file=%s' % self.logfile,
+            '--github-user=%s' % GITHUB_TEST_ACCOUNT,
+        ]
+        self.assertErrorRegex(EasyBuildError,
+                              "Multiple inclusion of cmakemake.py, check your --include-easyblocks options",
+                              self.eb_main, args, raise_error=True)
+
+        os.remove(os.path.join(self.test_prefix, 'cmakemake.py'))
+
+        # clear log
+        write_file(self.logfile, '')
+
+        args = [
+            '--from-pr=9979',  # PR for CMake easyconfig
+            '--include-easyblocks-from-pr=1936',  # PR for EB_CMake easyblock
+            '--unittest-file=%s' % self.logfile,
+            '--github-user=%s' % GITHUB_TEST_ACCOUNT,
+            '--extended-dry-run',
+        ]
+        self.eb_main(args, logfile=dummylogfn, raise_error=True)
+        logtxt = read_file(self.logfile)
+
+        # easyconfig from pr is found
+        ec_pattern = os.path.join(self.test_prefix, '.*', 'files_pr9979', 'c', 'CMake',
+                                  'CMake-3.16.4-GCCcore-9.2.0.eb')
+        ec_regex = re.compile(r"Parsing easyconfig file %s" % ec_pattern, re.M)
+        self.assertTrue(ec_regex.search(logtxt), "Pattern '%s' found in: %s" % (ec_regex.pattern, logtxt))
+
+        # easyblock included from pr is found
+        eb_regex = re.compile(r"Successfully obtained EB_CMake class instance from easybuild.easyblocks.cmake", re.M)
+        self.assertTrue(eb_regex.search(logtxt), "Pattern '%s' found in: %s" % (eb_regex.pattern, logtxt))
+
+        # easyblock is found via get_easyblock_class
+        klass = get_easyblock_class('EB_CMake')
+        self.assertTrue(issubclass(klass, EasyBlock), "%s is an EasyBlock derivative class" % klass)
+
+        # 'undo' import of easyblocks
+        del sys.modules['easybuild.easyblocks.cmake']
+
     def mk_eb_test_cmd(self, args):
         """Construct test command for 'eb' with given options."""