Skip to content

check for number of available sources when determining where to apply patches#1070

Merged
boegel merged 3 commits intoeasybuilders:developfrom
boegel:patches_no_sources
Oct 28, 2014
Merged

check for number of available sources when determining where to apply patches#1070
boegel merged 3 commits intoeasybuilders:developfrom
boegel:patches_no_sources

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Oct 23, 2014

This fixes a hard crash when patch files are listed with an empty list of source files:

ERROR: test_patch_step (__main__.EasyBlockTest)
Test patch step.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kehoste/work/easybuild-framework/test/framework/easyblock.py", line 529, in test_patch_step
    self.assertErrorRegex(EasyBuildError, '.*', eb.patch_step)
  File "test/framework/utilities.py", line 61, in assertErrorRegex
    call(*args, **kwargs)
  File "easybuild/framework/easyblock.py", line 1275, in patch_step
    beginpath = self.src[srcind]['finalpath']
IndexError: list index out of range

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 23, 2014

@stdweird, @wpoely86: please review

@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 23, 2014

Test PASSed.

stdweird
stdweird reviewed Oct 24, 2014
View reviewed changes
Comment thread easybuild/framework/easyblock.py Outdated
Copy link
Copy Markdown
Contributor

@stdweird stdweird Oct 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please cleanup the code above while you're at it
start with documenting the expected behaviour wrt source, sourcepath and copy (it's hard to see if the current code is just legacy patchwork or real intentional behaviour)

this is the current code:

srcind = patch.get('source',0)
srcpathsuffix = patch.get('sourcepath', patch.get('copy',''))
copy = 'copy' in patch and not ('sourcepath' in patch)

Copy link
Copy Markdown
Contributor

@stdweird stdweird Oct 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the lower level = patch.get('level', None) too

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 28, 2014

@stdweird: remarks fixed, please rereview

@stdweird
Copy link
Copy Markdown
Contributor

stdweird commented Oct 28, 2014

@boegel LGTM

@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 28, 2014

Test PASSed.

boegel added a commit that referenced this pull request Oct 28, 2014
@boegel
Merge pull request #1070 from boegel/patches_no_sources
576884f 
check for number of available sources when determining where to apply patches
@boegel boegel merged commit 576884f into easybuilders:develop Oct 28, 2014
@boegel boegel deleted the patches_no_sources branch October 28, 2014 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@boegel @hpcugentbot @stdweird