Skip to content

Preserve sticky module requirements during purge - #693

Merged
xdelaruelle merged 4 commits into
envmodules:mainfrom
adrien-cotte:fix/sticky_prereq_purge
Sep 21, 2026
Merged

xdelaruelle merged 4 commits into
envmodules:mainfrom
adrien-cotte:fix/sticky_prereq_purge

Conversation

@adrien-cotte

@adrien-cotte adrien-cotte commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Purge skips sticky and super-sticky modules but still attempts to unload their non-sticky requirements. This reports an unrelated dependency error without explaining why the requirement remains loaded.

Preserve requirements needed by retained sticky modules and apply the sticky_purge reporting mode to their skipped unload.

Fixes #582

Assisted-by: Codex:gpt-6-astra

Comment thread tcl/report.tcl.in Outdated

@xdelaruelle xdelaruelle left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Testsuite should be fixed as CI jobs are failing.

Documentation should be adapted to mention this behavior change, especially in module.rst, changes.rst, the sticky/module-tags design docs and the sticky-modules cookbook.

Also good to check if this has an impact on other stuff mentioning sticky modules like stash-environment, initial-environment, conflict_unload, evaluation-errors. Check these design doc and adapt them if required.

@adrien-cotte
adrien-cotte force-pushed the fix/sticky_prereq_purge branch 2 times, most recently from 18f8719 to 4d70791 Compare September 9, 2026 12:45
@adrien-cotte
adrien-cotte force-pushed the fix/sticky_prereq_purge branch 2 times, most recently from 3885540 to d3c36a3 Compare September 10, 2026 08:10
@adrien-cotte
adrien-cotte requested review from xdelaruelle and removed request for xdelaruelle September 11, 2026 14:08
@adrien-cotte
adrien-cotte force-pushed the fix/sticky_prereq_purge branch from d3c36a3 to f330e14 Compare September 11, 2026 16:03
adrien-cotte and others added 4 commits September 20, 2026 20:03
Purge skips sticky and super-sticky modules but still attempts to unload
their non-sticky requirements. This reports an unrelated dependency error
without explaining why the requirement remains loaded.

Preserve requirements needed by retained sticky modules and apply the
sticky_purge reporting mode to their skipped unload.

Fixes envmodules#582

Assisted-by: Codex:gpt-6-astra
Signed-off-by: Adrien Cotte <adrien.cotte@asplus.fr>
On a forced purge, a sticky module required by a retained super-sticky
module got a "forced unload" warning immediately followed by the error
reporting its unload is skipped as a super-sticky requirement. The
module stayed loaded, but the sticky check reported the forced unload
before the requirement check retained the module.

Check whether a module is required by a retained sticky module as part
of the sticky check itself, prior reporting the forced unload of a
sticky module. Rely on the existing sticky module test to determine if
a sticky dependent is retained rather than duplicating this rule.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
The requirement check on purge only looked at dependent modules loaded
after the module to unload. A requirement loaded after its super-sticky
dependent, for instance reloaded after a forced unload, was purged and
left this dependent with a missing dependency, the very situation this
protection aims to avoid.

Look at dependent modules whatever their load order and drop the direct
dependent pre-check that relied on load order too. Also cover the abort
of the purge when the error raised for a retained requirement occurs
with abort_on_error configured for this sub-command.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Move the NEWS entry to the end of the 5.7.0 list, as entries follow the
order changes landed. Report in the cookbook example the output of the
env/base load, which auto-loads its requirements, and the auto-loaded
tag these requirements then carry in the module list. Also restore the
blank line removed from NEWS.rst, unrelated to this change.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
@xdelaruelle
xdelaruelle force-pushed the fix/sticky_prereq_purge branch from f330e14 to a61af73 Compare September 20, 2026 18:40

@xdelaruelle xdelaruelle left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

While reviewing I found two gaps in the new protection, so I rebased the branch on current main and pushed three follow-up commits on top of yours:

  • Keep sticky requirement of super-sticky module on forced purge. A requirement tagged sticky and needed by a retained super-sticky module got two contradictory messages on purge -f: "Unload of sticky module forced" immediately followed by "Unload of super-sticky module requirement skipped". The sticky check ran first and reported the forced unload before the requirement check retained the module. The requirement check now lives inside failOrSkipUnloadIfSticky, ahead of the forced-unload warning. It also reuses isModuleSticky for the dependent stickiness rule and drops the extra call in cmdModuleUnload.
  • Protect sticky module requirement whatever its load order on purge. The dependent lookup used the positional-order dependency hash, so a requirement loaded after its super-sticky dependent (for instance after module unload -f req then module load req) was purged and left the dependent with a missing prereq, which is the very situation #582 describes. The lookup now uses the non-positional-order hash. Tests cover both load orders, and the interplay with abort_on_error when the skipped requirement raises an error. A bullet in the sticky-modules design doc records this.
  • doc: fix NEWS placement and cookbook example. The NEWS entry moves to the end of the 5.7.0 list, as entries follow the order changes landed. The cookbook example now shows the output of module load env/base, whose requirements are auto-loaded, and the tags they carry in module list.

@codecov

codecov Bot commented Sep 20, 2026

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

@xdelaruelle
xdelaruelle merged commit cf1bb9d into envmodules:main Sep 21, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Silent purge fails when super-sticky module prerequisite isn't super-sticky

2 participants