Skip to content

Respect lock configuration for prefix locks - #579

Open
mathomp4 wants to merge 1 commit into
JCSDA:spack-stack-devfrom
GMAO-SI-Team:fix/disable-prefix-locks
Open

Respect lock configuration for prefix locks#579
mathomp4 wants to merge 1 commit into
JCSDA:spack-stack-devfrom
GMAO-SI-Team:fix/disable-prefix-locks

Conversation

@mathomp4

@mathomp4 mathomp4 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adapts the relevant portion of upstream Spack spack#52270 (de4dd3a422) to the older Spack revision currently pinned by spack-stack.

When Spack is run with locks disabled, this revision still creates enabled per-spec prefix locks and failure-tracker locks. On our shared NFS filesystem, those lock attempts can fail with:

OSError: [Errno 37] No locks available

This change propagates locks:enable to the prefix locker and failure tracker, allowing --disable-locks to disable these remaining installation-time locks.

Why this is a targeted backport

Upstream spack#52270 is broader than this patch: it changes 11 files, including stage, environment transaction, repository, and cache locks, along with tests. It also uses newer lock-configuration APIs that do not exist in the spack-stack-dev-based revision.

A direct cherry-pick would therefore be conflict-prone and would pull in a larger, insufficiently tested change set. This PR ports only the two call paths responsible for the observed package-install failures.

The preferred long-term solution is to update spack-stack's pinned Spack revision; this focused compatibility patch is needed until then.

Upstream reference

Testing

Used the patched Spack for batch installs on Discover with locks disabled, where the previous per-package lock failures occurred.

AI assistance

GPT-5.6 Terra assisted in identifying the incomplete lock-disable propagation and preparing this targeted backport.

@climbfuji

Copy link
Copy Markdown
Collaborator

style check is failing. Have they changed the line width in their upstream linter config?

@mathomp4

mathomp4 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

style check is failing. Have they changed the line width in their upstream linter config?

Oh. My bad. let me fix. Their fix came in another bigger PR where mine was smaller. Let me fix the style

@mathomp4
mathomp4 force-pushed the fix/disable-prefix-locks branch from ceb4cf5 to a7a7a46 Compare September 1, 2026 17:55
@mathomp4

mathomp4 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

Well, Terra pushed a change. Note I did ask it about cherry-picking from upstream but:

Not as one cherry-pick.

Upstream spack#52270 is broader than our two-file fix: it changes 11 files, including stage locks, environment transaction locks, repository locks, and binary/misc cache locks, plus tests. Its database.py / store.py API also differs from the older Spack revision we pin, so a direct cherry-pick is likely to conflict and would be more invasive than this targeted backport.

Our current change is the minimal, correctly adapted subset that fixes the actual failures we saw:

prefix_locker
failure_tracker

I’d keep PR #579 as-is, with an explicit note that it is a partial backport of upstream spack#52270 for the old pinned revision.

If we need full --disable-locks semantics everywhere, the better solution is to update the pinned Spack revision. A full manual backport of all 11 upstream changes would deserve its own change and testing.

@climbfuji climbfuji 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.

I don't see where the enable=True is eventually being used to turn on or off something, but my guess is that the receiver already had the enable option as an optional argument.

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.

2 participants