Skip to content
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.

Sign up for GitHub

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

Jump to bottom

first look for patch in alt_location when it is specified #4093

Merged
merged 5 commits into from
Oct 17, 2022
Merged

first look for patch in alt_location when it is specified #4093

merged 5 commits into from
Oct 17, 2022

Conversation

akesandgren
Copy link
Contributor

When alt_location is specified we should not look in the directory of the easyconfig for patches before alt_location.

@akesandgren
first look in alt_location for patch when specified
0685188 
When alt_location is specified we should not look in the directory of
the easyconfig for patches before alt_location.
@akesandgren akesandgren added this to the next release (4.6.2?) milestone Oct 12, 2022
@akesandgren akesandgren requested a review from boegel October 12, 2022 13:52
@boegel
Copy link
Member

boegel commented Oct 12, 2022

This extends the impact of alt_location a bit, but I guess it makes sense: if alt_location is specified, it should be honored.

We should add some logging though, to make it clear that the directory where the easyconfig file is being excluded because alt_location is specified, to make this a bit more explicit?

@akesandgren
Copy link
Contributor Author

@boegel something like that then?

@akesandgren akesandgren mentioned this pull request Oct 12, 2022
Merged
boegel added 2 commits October 17, 2022 11:10
@boegel
always log a message in obtain_file to indicate whether or directory …
bb04613 
…in which easyconfig file is located is being consider to locate file
@boegel
enhance test_obtain_file to check that directory in which easyconfig …
dc7a61f 
…file is located is ignored when alt_location is used
@boegel boegel mentioned this pull request Oct 17, 2022
Merged
boegel
boegel requested changes Oct 17, 2022
View reviewed changes
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

And we should cover this in the test for obtain_file too.

See akesandgren#20

easybuild/framework/easyblock.py Show resolved Hide resolved
easybuild/framework/easyblock.py Outdated Show resolved Hide resolved
@akesandgren
Merge pull request #20 from boegel/fix-obtain_files-for-patch-with-al…
042d57a 
…t_location

always log a message in obtain_file to indicate whether or directory in which easyconfig file is located is being consider to locate file + enhance test_obtain_file
boegel
boegel approved these changes Oct 17, 2022
View reviewed changes
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel merged commit e276c51 into easybuilders:develop Oct 17, 2022
@akesandgren akesandgren deleted the fix-obtain_files-for-patch-with-alt_location branch October 17, 2022 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boegel boegel boegel approved these changes

Assignees
No one assigned
Labels
bug fix
Projects
None yet
Milestone
4.6.2
Development

Successfully merging this pull request may close these issues.
2 participants
@akesandgren @boegel