-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added support to 'download' sources from git #2555
Changes from 3 commits
0f9ce62
c657745
372b3c6
7baa951
75ef62c
5abd3b7
ed59ede
4ce646f
8ff3559
70c17b3
b8d9a4e
e2f07a0
ed74a75
8dbc954
51a4db3
e205640
b1eec8b
fa0389b
6af5a0c
9c578cd
e3ab8df
55f03be
a0ed138
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -348,6 +348,7 @@ def fetch_sources(self, sources=None, checksums=None): | |
extract_cmd = source.pop('extract_cmd', None) | ||
download_filename = source.pop('download_filename', None) | ||
source_urls = source.pop('source_urls', None) | ||
git_config = source.pop('git_config', {}) | ||
if source: | ||
raise EasyBuildError("Found one or more unexpected keys in 'sources' specification: %s", source) | ||
|
||
|
@@ -361,7 +362,7 @@ def fetch_sources(self, sources=None, checksums=None): | |
# check if the sources can be located | ||
force_download = build_option('force_download') in [FORCE_DOWNLOAD_ALL, FORCE_DOWNLOAD_SOURCES] | ||
path = self.obtain_file(filename, download_filename=download_filename, force_download=force_download, | ||
urls=source_urls) | ||
urls=source_urls, git_config=git_config) | ||
if path: | ||
self.log.debug('File %s found for source %s' % (path, filename)) | ||
self.src.append({ | ||
|
@@ -559,7 +560,8 @@ def fetch_extension_sources(self, skip_checksums=False): | |
|
||
return exts_sources | ||
|
||
def obtain_file(self, filename, extension=False, urls=None, download_filename=None, force_download=False): | ||
def obtain_file(self, filename, extension=False, urls=None, download_filename=None, force_download=False, | ||
git_config={}): | ||
""" | ||
Locate the file with the given name | ||
- searches in different subdirectories of source path | ||
|
@@ -568,6 +570,7 @@ def obtain_file(self, filename, extension=False, urls=None, download_filename=No | |
:param extension: indicates whether locations for extension sources should also be considered | ||
:param urls: list of source URLs where this file may be available | ||
:param download_filename: filename with which the file should be downloaded, and then renamed to <filename> | ||
:param git_config: dictionary to parametrize how to download the repository | ||
mboisson marked this conversation as resolved.
Show resolved
Hide resolved
mboisson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
:force_download: always try to download file, even if it's already available in source path | ||
""" | ||
srcpaths = source_paths() | ||
|
@@ -663,6 +666,52 @@ def obtain_file(self, filename, extension=False, urls=None, download_filename=No | |
if self.dry_run: | ||
self.dry_run_msg(" * %s found at %s", filename, foundfile) | ||
return foundfile | ||
elif git_config: | ||
# if a non-empty dictionary was provided, download from git and archive | ||
if not isinstance(git_config, dict): | ||
raise EasyBuildError("Found a non null git_config in 'sources', but value is not a dictionary") | ||
else: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for the else, the |
||
git_config = git_config.copy() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took this strategy from similar code above (line 347). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, now it clicks... It's done to avoid modifying the value that is passed into Better add a comment to explain that (on line 347 too, while you're at it). ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
tag = git_config.pop('tag', None) | ||
url = git_config.pop('url', None) | ||
repo_name = git_config.pop('repo_name', None) | ||
commit = git_config.pop('commit', None) | ||
recursive = git_config.pop('recursive', False) | ||
if git_config: | ||
raise EasyBuildError("Found one or more unexpected keys in 'git_config' specification: %s", | ||
git_config) | ||
if not repo_name: | ||
raise EasyBuildError("repo_name not specified in git_config parameter") | ||
if not tag and not commit: | ||
raise EasyBuildError("Neither tag nor commit found in git_config parameter") | ||
if tag and commit: | ||
raise EasyBuildError("Tag and commit are mutually exclusive in git_config parameter") | ||
if not url: | ||
raise EasyBuildError("url not specified in git_config parameter") | ||
if '.tar.gz' not in filename: | ||
raise EasyBuildError("git_config only supports filename ending in .tar.gz") | ||
|
||
targetdir = os.path.join(srcpaths[0], self.name.lower()[0], self.name) | ||
mkdir(targetdir, parents=True) | ||
targetpath = os.path.join(targetdir, filename) | ||
|
||
change_dir(targetdir) | ||
recursive = " --recursive " if recursive else "" | ||
if tag: | ||
cmd = "git clone --branch %s %s %s/%s.git " % (tag, recursive, url, repo_name) | ||
else: | ||
cmd = "git clone %s %s/%s.git" % (recursive, url, repo_name) | ||
(cmdstdouterr, ec) = run_cmd(cmd, log_all=True, log_ok=False, simple=False, regexp=False) | ||
if commit: | ||
change_dir(os.path.join(targetdir, repo_name)) | ||
recursive = " && git submodule update " if recursive else "" | ||
cmd = "git checkout %s %s " % (commit, recursive) | ||
(cmdstdouterr, ec) = run_cmd(cmd, log_all=True, log_ok=False, simple=False, regexp=False) | ||
change_dir(targetdir) | ||
cmd = "tar cfvz %s --exclude-vcs %s && rm -rf %s" % (targetpath, repo_name, repo_name) | ||
(cmdstdouterr, ec) = run_cmd(cmd, log_all=True, log_ok=False, simple=False, regexp=False) | ||
return targetpath | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mboisson Quick review: please move this to a dedicated function, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just this or the whole logic to download from git ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would create a function that takes elif git_config:
return new_function(git_config) (but find a better name than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. Moving it to filetools would require quite a few more parameters, because this bit of code uses a couple of variables internal to easyblock.py :
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, but that's only 2 parameters to add, that's OK.
cwd = change_dir(new_dir)
...
change_dir(cwd) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
else: | ||
# try and download source files from specified source URLs | ||
if urls: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continuation line over-indented for visual indent