[WIP] New package.py functions _has_make_target() and _has_ninja_target().#8604
[WIP] New package.py functions _has_make_target() and _has_ninja_target().#8604greenc-FNAL wants to merge 2 commits intospack:developfrom
Conversation
|
@adamjstewart I've marked this [WIP] due to the overlap with PR #8484 (one commit in common). I can separate them if you prefer, but then whichever one gets merged first, the other one will be non-trivial to merge. Let me know if you would like me to separate them. |
Split off from _if_make_target_execute() and _if_ninja_target_execute(), respectively. Intended to facilitate testing of other functionality, per spack#8484 (comment). spack#8484 (comment) was considered, but this word order maintains consistency with existing functions.
greenc-FNAL
force-pushed
the
feature/target-interrogation
branch
from
6557d23 to
656150b
Compare
|
Rebase against |
|
|
||
| m.cmake = Executable('cmake') | ||
| m.ctest = Executable('ctest') | ||
| m.ctest = MakeExecutable('ctest', jobs) |
There was a problem hiding this comment.
We should probably rename MakeExecutable to ParallelExecutable since most of these aren't make. We should also move ParallelExecutable to lib/spack/spack/utils/executable.py.
|
Also, is this a duplicate of #8484? We should either have a single PR that contains everything or two PRs that contain half of the things, not two PRs that contain everything. |
|
@adamjstewart This is not a duplicate of #8484. This one is marked as WIP because it builds on top of it: the changes overlap too much in terms of how diffs would apply otherwise. This PR provides the functions that were originally part of #8484 but using only this solution the PR as was caused problems for some recipes. The revised PR did not use these functions, but they were not the cause of the aforementioned problems (see comments there for details). However, you pointed out that these functions were useful in their own right, so I moved them to this PR. If you like, I can apply the requested change above to #8484 where it belongs, and then when that PR gets merged I can rebase this one. Let me know what you'd like me to do. |
|
@adamjstewart I'm not sure what the hold-up is on PR #8484: I thought the other commenters were happy with the current state. I will rebase it and ping again. |
|
I don't think there's anything preventing #8484 from being merged, but it requires someone more familiar with CMake than me to approve and merge. |
2 participants
Split off from _if_make_target_execute() and _if_ninja_target_execute(), respectively.
Intended to facilitate testing of other functionality, per #8484 (comment).
#8484 (comment) was considered, but this word order maintains consistency with existing functions.
Incorporates PR #8484 due to overlap.