Skip to content

Replace fcntl with cross-platform portalocker (fix win/powershell errors)#739

Merged
evantahler merged 7 commits into
mainfrom
no-fcntl
Jan 9, 2026
Merged

Replace fcntl with cross-platform portalocker (fix win/powershell errors)#739
evantahler merged 7 commits into
mainfrom
no-fcntl

Conversation

@evantahler

@evantahler evantahler commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

So even importing fcntl causes problems on windows. This PR replaces fcntl with portalocker. Tests all pass, so I think we are good.

ref: https://arcade-ai.slack.com/archives/C08K1SJ072S/p1767897850450239?thread_ts=1766186586.406019&cid=C08K1SJ072S

Screenshot 2026-01-08 at 2 57 46 PM

Closes ENGTOP-8


Note

Cross-platform file locking

  • Replace fcntl with portalocker in arcade_core/usage/identity.py (shared/exclusive locks); switch to atomic os.replace()
  • Add portalocker dependency and bump arcade-core to 4.2.1

Installation/CI

  • New GitHub Actions workflow test-install.yml runs install/CLI checks on macOS, Windows, and Linux for Python 3.10/3.12
  • Add tests/install/test_install.py and README to verify install, arcade CLI availability, and portalocker locking behavior

Written by Cursor Bugbot for commit 3fe98fb. This will update automatically on new commits. Configure here.

@evantahler evantahler changed the title Replace fcntl with cross-platform portalocker Replace fcntl with cross-platform portalocker (fix win/powershell errors) Jan 8, 2026
@codecov

codecov Bot commented Jan 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@evantahler

Copy link
Copy Markdown
Contributor Author

@evantahler evantahler marked this pull request as ready for review January 8, 2026 23:30
and logout identity rotation.
"""

import fcntl

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was the actual problem to solve for windows users

Comment on lines -94 to +92
os.rename(temp_path, self.usage_file_path)
# Use os.replace() for cross-platform atomic file replacement
# os.replace() is atomic on both Unix and Windows (Python 3.3+)
os.replace(temp_path, self.usage_file_path)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My new test taught me this

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.

TIL!

Comment on lines -94 to +92
os.rename(temp_path, self.usage_file_path)
# Use os.replace() for cross-platform atomic file replacement
# os.replace() is atomic on both Unix and Windows (Python 3.3+)
os.replace(temp_path, self.usage_file_path)

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.

TIL!

@evantahler evantahler merged commit c034046 into main Jan 9, 2026
20 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.

3 participants