Skip to content

Commit

Permalink
Fix wrong StageComposite keep override (spack#38938)
Browse files Browse the repository at this point in the history
`Stage(keep=True)` was ignored when put in a composite that doesn't
override the value.
  • Loading branch information
haampie authored Jul 17, 2023
1 parent 1fa60a6 commit f05837a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
19 changes: 17 additions & 2 deletions lib/spack/spack/stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import stat
import sys
import tempfile
from typing import Dict
from typing import Dict, Iterable

import llnl.util.lang
import llnl.util.tty as tty
Expand Down Expand Up @@ -742,14 +742,20 @@ def __init__(self):
]
)

@classmethod
def from_iterable(cls, iterable: Iterable[Stage]) -> "StageComposite":
"""Create a new composite from an iterable of stages."""
composite = cls()
composite.extend(iterable)
return composite

def __enter__(self):
for item in self:
item.__enter__()
return self

def __exit__(self, exc_type, exc_val, exc_tb):
for item in reversed(self):
item.keep = getattr(self, "keep", False)
item.__exit__(exc_type, exc_val, exc_tb)

#
Expand All @@ -771,6 +777,15 @@ def path(self):
def archive_file(self):
return self[0].archive_file

@property
def keep(self):
return self[0].keep

@keep.setter
def keep(self, value):
for item in self:
item.keep = value


class DIYStage:
"""
Expand Down
21 changes: 21 additions & 0 deletions lib/spack/spack/test/stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -890,3 +890,24 @@ def test_cannot_access(capsys):

captured = capsys.readouterr()
assert "Insufficient permissions" in str(captured)


def test_override_keep_in_composite_stage():
stage_1 = Stage("file:///does-not-exist", keep=True)
stage_2 = Stage("file:///does-not-exist", keep=False)
stage_3 = Stage("file:///does-not-exist", keep=True)
stages = spack.stage.StageComposite.from_iterable((stage_1, stage_2, stage_3))

# The getter for the composite stage just returns the value of the first stage
# its just there so we have a setter too.
assert stages.keep
assert stage_1.keep
assert not stage_2.keep
assert stage_3.keep

# This should override all stages
stages.keep = False
assert not stages.keep
assert not stage_1.keep
assert not stage_2.keep
assert not stage_3.keep

0 comments on commit f05837a

Please sign in to comment.