Skip to content

restore initial environment before processing each easystack item#4213

Merged
boegel merged 4 commits intoeasybuilders:developfrom
bedroge:bugfix/easystack_restore_env
Feb 14, 2023
Merged

restore initial environment before processing each easystack item#4213
boegel merged 4 commits intoeasybuilders:developfrom
bedroge:bugfix/easystack_restore_env

Conversation

@bedroge
Copy link
Copy Markdown
Contributor

@bedroge bedroge commented Feb 10, 2023 •
edited
Loading

Closes #4194 by explicitly restoring the environment at the end of build_and_install_software. This should prevent that the dependencies of a previous (set of) installation(s) are still loaded for the next one, e.g. when using an easystack file. Also added a test that verifies this.

Test output before applying the patch to main.py:

$ python3 -O -m test.framework.easystack
.......F..
======================================================================
FAIL: test_easystack_restore_env_after_each_build (__main__.EasyStackTest)
Test that the build environment is reset for each easystack item
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/bob/ebdev/easybuild-framework/test/framework/easystack.py", line 148, in test_easystack_restore_env_after_each_build
    self.assertFalse(regex.search(stdout), "Pattern '%s' should not be found in: %s" % (regex.pattern, stdout))
AssertionError: <re.Match object; span=(166129, 166280), match="WARNING Loaded modules detected: ['EasyBuild-deve> is not false : Pattern 'WARNING Loaded modules detected: \[.*gompi/2018.*\]\n' should not be found in:
<LOTS OF OUTPUT>
== 2023-02-10 13:46:59,710 easyblock.py:2248 WARNING Loaded modules detected: ['EasyBuild-develop', 'GCC/6.4.0-2.28', 'hwloc/1.11.8-GCC-6.4.0-2.28', 'OpenMPI/2.1.2-GCC-6.4.0-2.28', 'gompi/2018a']
<LOTS OF OUTPUT>
----------------------------------------------------------------------
Ran 10 tests in 2.169s

FAILED (failures=1)

After applying the patch:

$ python3 -O -m test.framework.easystack
..........
----------------------------------------------------------------------
Ran 10 tests in 2.096s

OK

@bedroge bedroge added bug fix easystack Issues and PRs related to easystack files labels Feb 10, 2023
@bedroge
Copy link
Copy Markdown
Contributor Author

bedroge commented Feb 10, 2023

Tests are failing because of:

ERROR: test_compiler_cache (test.framework.toolchain.ToolchainTest)
Test ccache
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/runner/736df0562413dad6f69227f3175ee31db19c4876/lib/python2.7/site-packages/test/framework/toolchain.py", line 2187, in test_compiler_cache
    self.assertTrue(os.path.samefile(os.environ['CCACHE_DIR'], ccache_dir))
  File "/opt/hostedtoolcache/Python/2.7.18/x64/lib/python2.7/UserDict.py", line 40, in __getitem__
    raise KeyError(key)
KeyError: 'CCACHE_DIR'

But I have no clue what's causing this, doesn't seem related to this PR?

@boegel boegel added this to the next release (4.7.1?) milestone Feb 13, 2023
@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 13, 2023

@bedroge That test is failing because in it we assume that the build environment does not get reset after the installation of easyconfigs, which the change you propose (which does make sense) is breaking...

@bedroge bedroge changed the title Restore the initial environment after each build with an easystack file Restore the initial environment after installation of all provided easyconfig files Feb 13, 2023
@boegel
restore environment before processing an easystack entry in process_e…
b172651 
…asystack (rather than restoring environment after installing a series of easyconfigs in build_and_install_software)
@boegel boegel mentioned this pull request Feb 13, 2023
Merged
@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 13, 2023

@bedroge The failing test is actually telling us that we're making a somewhat intrusive change.
It's not impossible that some people are actually relying on the build environment not being reset, for example when using EasyBuild as a library.

Here's a proposal for a different approach: bedroge#1

@bedroge
Merge pull request #1 from boegel/bugfix/easystack_restore_env
6c2d00f 
restore environment before processing an easystack entry in process_easystack
@bedroge
Copy link
Copy Markdown
Contributor Author

bedroge commented Feb 14, 2023

@bedroge The failing test is actually telling us that we're making a somewhat intrusive change. It's not impossible that some people are actually relying on the build environment not being reset, for example when using EasyBuild as a library.

Here's a proposal for a different approach: bedroge#1

Looks good, merged!

@boegel boegel changed the title Restore the initial environment after installation of all provided easyconfig files restore initial environment before processing each easystack item Feb 14, 2023
boegel
boegel approved these changes Feb 14, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@boegel boegel merged commit 3cc6e91 into easybuilders:develop Feb 14, 2023
@bedroge bedroge deleted the bugfix/easystack_restore_env branch February 14, 2023 15:26
@ocaisa ocaisa mentioned this pull request Dec 11, 2024
Merged
1 task
This was referenced Jan 9, 2026
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@boegel boegel boegel approved these changes

Assignees

No one assigned

Labels

bug fix easystack Issues and PRs related to easystack files

Projects

None yet

Milestone

4.7.1

Development

Successfully merging this pull request may close these issues.

Dependencies of first build are still loaded for all next builds with an easystack file

2 participants

@bedroge @boegel