Support for passing from-pr in an easyconfig-based Easystack file [WIP] #4052
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…without explicit '.eb' extension
…o top level item was specified in the YAML
…in easystack files. As discussed here easybuilders#4021 (comment)
…ck files based on the 'easyconfig' top level keyword
… det_easyconfig_paths. WIP, so still with lots of debugging print statements. It works for the top level EasyConfigs, but dependencies are resolved elsewhere I guess.
…ic --from-pr statements that may be included in easystack files. Note that this is really a first attempt, with lots of debugging prints still in there and a pretty horrible type of code duplication from easybuild/tools/options.py to easybuild/tools/robot.py to re-configure the robot_path build option. It's extremely ugly, but appears to work on a test EasyStack file that specifies some EasyConfig-specific --from-pr's... Now we need to decide if this is even the wanted behaviour...
casparvl
commented
Aug 3, 2022
| if opts_current_ec is not None: | ||
| _log.debug("Applying easyconfig-specific build options %s to %s" % (opts_current_ec, path)) | ||
| orig_build_options = update_build_options(opts_current_ec) | ||
| # TODO: we should probably also update the pr_path (?) since process_easyconfig calls EasyConfig(...) |
There was a problem hiding this comment.
We should probably update pr_path here as well, in a similar way as we do in resolve_dependencies (and we should probably grab that code snipped and make a function out of it, rather than duplicating that code everywhere)
casparvl
commented
Aug 3, 2022
| if custom_build_opts is not None: | ||
| _log.debug("Applying easyconfig specific options %s" % str(custom_build_opts)) | ||
| orig_build_opts = update_build_options(custom_build_opts) | ||
| # Re-determine robot-path from options (borrowed from easybuild/tools/options) |
There was a problem hiding this comment.
We should probably make a function out of this, rather than copying the code. That function could then be called whenever we need to calculate an updated robot_path (it could even update the build_option for us, but I'm not sure if we'd like that)
|
Closed in favor of #4057 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note that this PR builds on top of #4021 (because we need option parsing for EasyStack files, before we can do anything with the options...). Thus, that part might still change.
At this point in time, the behavior of this PR on the following example:
Is:
In this,
Yasm, is a dependency ofx265, which is a dependency ofFFmpeg. As we can see, all are properly resolved from the right tempdir.../files_pr15918/.... Essentially, this is as expected.In terms of the 'challenges' discussed in issue #4050, behaviour would/should now be that for
With
C.ebbeing a dependency of bothA.ebandB.eb, this should resolveC.ebto the version fromfrom_pr.