clang path is not needed with mpich-3.2.1#8320
clang path is not needed with mpich-3.2.1#8320PDoakORNL wants to merge 12 commits intospack:developfrom
Conversation
| # see https://lists.mpich.org/pipermail/discuss/2016-May/004764.html | ||
| # and https://lists.mpich.org/pipermail/discuss/2016-June/004768.html | ||
| patch('mpich32_clang.patch', when='@3.2%clang') | ||
| patch('mpich32_clang.patch', when='@3.2.0%clang') |
There was a problem hiding this comment.
There is no version 3.2.0.
@tgamblin I think this is a bug in the concretizer. @3.2 should only apply to version 3.2, not to every version 3.2.X.
At the same time, I'm really confused, because I've always wanted to say depends_on('[email protected]'), but that's never worked because it would install exactly version 2.7.
So now I'm really confused how the concretizer works with regards to MAJOR.MINOR version numbers.
There was a problem hiding this comment.
My aim was to have 3.2 work and not have 3.2.1 try to patch and bomb. Empirically this worked on my setup.
There was a problem hiding this comment.
This change does prevent 3.2.1 from being patched, but it also prevents 3.2 from being patched.
There was a problem hiding this comment.
would changing the actual version:
version('3.2', 'f414cfa77099cd1fa1a5ae4e22db508a')
From:
3.2 to 3.2.0 help?
There was a problem hiding this comment.
It would, but then we would need to override url_for_version to get things to work. The underlying bug still remains.
currently fails
| return self in other or other in self | ||
| """self.version = x.y | ||
| other.version = a.b.c | ||
| self overlaps other iff c==0 |
There was a problem hiding this comment.
could you please add two (1.2. vs 1.2.0 and 1.2.0 vs 1.2) tests to lib/spack/spack/test/versions.py ?
There was a problem hiding this comment.
Ok that test opened more questions for me.
Let me state what I think the design principles involved here are:
-
Patches should be applied if a spec version overlaps a patch constraint not if a spec satisfies a patch constraint. One can imagine the mpich patch situation happening with some regularity.
-
Specs should for consistency with other package managers and ease of use the satisfies semantics. This will generally work as in a.b.c c's usually involve internal changes that can/will break patchs but do not change the public API.
Since overlap should have meaning different from satisfies the way the list ranges overlap seems counterintuitive based on the defnition of version overlap as symetric and specific based on the conversion of x.y to the normal form of x.y.0.0.0...
this assertion fails:
assert_no_overlap('1.2.1', ':1.2')
It does this because
assert_in('1.4.2', '1.2:1.4')
is true.
__contains__ uses the satisfies semantics 😞 but overlap for ranges depends on it.
By my logic:
1.2.1 is in 1.2: and not in :1.2
while 1.2.1 satisifies both :1.2 and 1.2:
But I would propse the fix for patches to simply remove the dependency of overlaps on __contains__.
Unfortunately even this breaks some other tests namely
assert_overlaps('1.6.5', ':1.6')
test_intersection_with_containment
test_union_with_containment
Discovered further inconsistency. Fix?
|
I don't know enough about the internals of the patching mechanism to review this. Maybe @scheibelp? |
|
Yes its still there on Centos Linux too. The question of d82d8c6 remains. While the behavior and documentation of overlaps vs. satisifies was inconsistent before, it is still inconsistent in this PR, although it does fix this issue. |
|
It seems it's a bit complex to fix the underlying issue, given this would it make sense to change the preferred version mpich to avoid the issue? |
|
The original commit 855ec48 was the hack solution. |
|
I'm running into the same problem trying to install mpich%clang on a Debian machine. Any chance this could be resolved? |
|
I'm over it. This is somewhat addressed by other PR's good enough. |
6 participants
Patch fails and would appear to be unneeded with with mpich-3.2.1