Skip to content

Commit 3f1cdbf

Browse files
scheibelptgamblin
scheibelp
authored and
tgamblin
committed
Revert #2292: use frontend compiler for build deps (#2549)
The primary goal of #2292 was to use the frontend compiler to make build dependencies like cmake on HPC platforms. It turns out that while this works in some cases, it did not handle cases where a package was a link dependency of the root and of a build dependency (and could produce incorrect concretizations which would not build).
1 parent 3ce3165 commit 3f1cdbf

4 files changed

Lines changed: 24 additions & 90 deletions

File tree

‎lib/spack/spack/architecture.py‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -514,8 +514,3 @@ def sys_type():
514514
"""
515515
arch = Arch(platform(), 'default_os', 'default_target')
516516
return str(arch)
517-
518-
519-
@memoized
520-
def frontend_sys_type():
521-
return str(Arch(platform(), 'frontend', 'frontend'))

‎lib/spack/spack/concretize.py‎

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,8 @@ def concretize_architecture(self, spec):
251251
DAG has an architecture, then use the root otherwise use the defaults
252252
on the platform.
253253
"""
254-
root_arch = spec.link_root().architecture
255-
if spec.build_dep() and spec.disjoint_build_tree():
256-
sys_arch = spack.spec.ArchSpec(
257-
spack.architecture.frontend_sys_type())
258-
else:
259-
sys_arch = spack.spec.ArchSpec(spack.architecture.sys_type())
254+
root_arch = spec.root.architecture
255+
sys_arch = spack.spec.ArchSpec(spack.architecture.sys_type())
260256
spec_changed = False
261257

262258
if spec.architecture is None:
@@ -327,21 +323,14 @@ def _proper_compiler_style(cspec, aspec):
327323
spec.compiler in all_compilers):
328324
return False
329325

330-
if spec.compiler:
331-
other_spec = spec
332-
elif spec.build_dep() and spec.disjoint_build_tree():
333-
link_root = spec.link_root()
334-
build_subtree = list(link_root.traverse(direction='children'))
335-
candidates = list(x for x in build_subtree if x.compiler)
336-
other_spec = candidates[0] if candidates else link_root
337-
else:
338-
# Find another spec that has a compiler, or the root if none do.
339-
# Prefer compiler info from other specs which are not build deps.
340-
other_spec = (
341-
find_spec(spec, lambda x: x.compiler and not x.build_dep()) or
342-
spec.root)
326+
# Find the another spec that has a compiler, or the root if none do
327+
other_spec = spec if spec.compiler else find_spec(
328+
spec, lambda x: x.compiler)
343329

330+
if not other_spec:
331+
other_spec = spec.root
344332
other_compiler = other_spec.compiler
333+
assert(other_spec)
345334

346335
# Check if the compiler is already fully specified
347336
if other_compiler in all_compilers:
@@ -389,20 +378,16 @@ def concretize_compiler_flags(self, spec):
389378
# running.
390379
return True
391380

392-
def compiler_match(spec1, spec2):
393-
return ((spec1.compiler, spec1.architecture) ==
394-
(spec2.compiler, spec2.architecture))
395-
396381
ret = False
397382
for flag in spack.spec.FlagMap.valid_compiler_flags():
398383
try:
399384
nearest = next(p for p in spec.traverse(direction='parents')
400-
if ((p is not spec) and
401-
compiler_match(p, spec) and
385+
if ((p.compiler == spec.compiler and
386+
p is not spec) and
402387
flag in p.compiler_flags))
403-
if (flag not in spec.compiler_flags or
404-
(set(nearest.compiler_flags[flag]) -
405-
set(spec.compiler_flags[flag]))):
388+
if flag not in spec.compiler_flags or \
389+
not (sorted(spec.compiler_flags[flag]) >=
390+
sorted(nearest.compiler_flags[flag])):
406391
if flag in spec.compiler_flags:
407392
spec.compiler_flags[flag] = list(
408393
set(spec.compiler_flags[flag]) |
@@ -413,11 +398,10 @@ def compiler_match(spec1, spec2):
413398
ret = True
414399

415400
except StopIteration:
416-
if (compiler_match(spec.root, spec) and
417-
flag in spec.root.compiler_flags and
418-
((flag not in spec.compiler_flags) or
419-
(set(spec.root.compiler_flags[flag]) -
420-
set(spec.compiler_flags[flag])))):
401+
if (flag in spec.root.compiler_flags and
402+
((flag not in spec.compiler_flags) or
403+
sorted(spec.compiler_flags[flag]) !=
404+
sorted(spec.root.compiler_flags[flag]))):
421405
if flag in spec.compiler_flags:
422406
spec.compiler_flags[flag] = list(
423407
set(spec.compiler_flags[flag]) |
@@ -441,8 +425,10 @@ def compiler_match(spec1, spec2):
441425
if compiler.flags[flag] != []:
442426
ret = True
443427
else:
444-
if (set(compiler.flags[flag]) -
445-
set(spec.compiler_flags[flag])):
428+
if ((sorted(spec.compiler_flags[flag]) !=
429+
sorted(compiler.flags[flag])) and
430+
(not set(spec.compiler_flags[flag]) >=
431+
set(compiler.flags[flag]))):
446432
ret = True
447433
spec.compiler_flags[flag] = list(
448434
set(spec.compiler_flags[flag]) |

‎lib/spack/spack/spec.py‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,7 @@ def return_val(res):
11021102

11031103
successors = deps
11041104
if direction == 'parents':
1105-
successors = self.dependents_dict(deptype)
1105+
successors = self.dependents_dict() # TODO: deptype?
11061106

11071107
visited.add(key)
11081108
for name in sorted(successors):
@@ -1323,23 +1323,6 @@ def from_json(stream):
13231323
except Exception as e:
13241324
raise sjson.SpackJSONError("error parsing JSON spec:", str(e))
13251325

1326-
def build_dep(self):
1327-
# If this spec is the root, it will automatically be included in
1328-
# traverse
1329-
return not (self.root in
1330-
self.traverse(
1331-
deptype=('link', 'run'), direction='parents'))
1332-
1333-
def link_root(self):
1334-
parents = list(self.traverse(deptype=('link',), direction='parents',
1335-
order='pre'))
1336-
return parents[-1]
1337-
1338-
def disjoint_build_tree(self):
1339-
link_root = self.link_root()
1340-
build_subtree = list(link_root.traverse(direction='children'))
1341-
return all(x.build_dep() for x in build_subtree)
1342-
13431326
def _concretize_helper(self, presets=None, visited=None):
13441327
"""Recursive helper function for concretize().
13451328
This concretizes everything bottom-up. As things are
@@ -1358,8 +1341,8 @@ def _concretize_helper(self, presets=None, visited=None):
13581341

13591342
# Concretize deps first -- this is a bottom-up process.
13601343
for name in sorted(self._dependencies.keys()):
1361-
dep = self._dependencies[name]
1362-
changed |= dep.spec._concretize_helper(presets, visited)
1344+
changed |= self._dependencies[
1345+
name].spec._concretize_helper(presets, visited)
13631346

13641347
if self.name in presets:
13651348
changed |= self.constrain(presets[self.name])

‎lib/spack/spack/test/concretize.py‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -97,36 +97,6 @@ def test_concretize_variant(self):
9797
self.check_concretize('mpich debug=2')
9898
self.check_concretize('mpich')
9999

100-
def test_concretize_with_build_dep(self):
101-
# Set the target as the backend. Since the cmake build dependency is
102-
# not explicitly configured to target the backend it should target
103-
# the frontend (whatever compiler that is, it is different)
104-
spec = self.check_concretize('cmake-client platform=test target=be')
105-
client_compiler = spack.compilers.compiler_for_spec(
106-
spec.compiler, spec.architecture)
107-
cmake_spec = spec['cmake']
108-
cmake_compiler = spack.compilers.compiler_for_spec(
109-
cmake_spec.compiler, cmake_spec.architecture)
110-
self.assertTrue(client_compiler.operating_system !=
111-
cmake_compiler.operating_system)
112-
113-
def test_concretize_link_dep_of_build_dep(self):
114-
# The link dep of the build dep should use the same compiler as
115-
# the build dep, and both should be different from the root
116-
spec = self.check_concretize('dttop platform=test target=be')
117-
dttop_compiler = spack.compilers.compiler_for_spec(
118-
spec.compiler, spec.architecture)
119-
dtlink2_spec = spec['dtlink2']
120-
dtlink2_compiler = spack.compilers.compiler_for_spec(
121-
dtlink2_spec.compiler, dtlink2_spec.architecture)
122-
dtbuild1_spec = spec['dtbuild1']
123-
dtbuild1_compiler = spack.compilers.compiler_for_spec(
124-
dtbuild1_spec.compiler, dtbuild1_spec.architecture)
125-
self.assertTrue(dttop_compiler.operating_system !=
126-
dtlink2_compiler.operating_system)
127-
self.assertTrue(dtbuild1_compiler.operating_system ==
128-
dtlink2_compiler.operating_system)
129-
130100
def test_conretize_compiler_flags(self):
131101
self.check_concretize('mpich cppflags="-O3"')
132102

0 commit comments

Comments
 (0)