Skip to content

Move two os functions to extras#188

Merged
slott56 merged 2 commits into
mainfrom
branch_177
Jul 2, 2026
Merged

Move two os functions to extras#188
slott56 merged 2 commits into
mainfrom
branch_177

Conversation

@slott56

@slott56 slott56 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

The os.getegid() and os.geteuid() are not available on all OS's. The group_access and user_access attributes of a file are not universally available.

This treats each extra feature as potential available or unavailable. It no longer lumps the extra features into a composite "all-or-none" change.

Closes #177.

This also removes PR #178 and #179.

The os.getegid() and os.geteuid() are not available on all OS's. The `group_access` and `user_access` attributes of a file are **not** universally available.

@ajkerrigan ajkerrigan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Windows fix sounds good, just a point inline about potentially avoiding a Linux regression

Comment thread src/celpy/__main__.py Outdated
"st_rdev": celtypes.IntType(status.st_rdev),
"st_gen": celtypes.IntType(status.st_gen), # type: ignore [attr-defined, unused-ignore]
"group_access": celtypes.BoolType(status.st_gid == os.getegid()),
"user_access": celtypes.BoolType(status.st_uid == os.geteuid()),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm wondering if these 2 are worth handling separately, since they are available on Linux but some other attributes in this block aren't. The upshot being that on my Ubuntu machine...

# Without this changeuv run python -m celpy -n 'stat("pyproject.toml").user_access'
true

# With this changeuv run python -m celpy -n 'stat("pyproject.toml").user_access'
ERROR: <input>:1:1 no such member in mapping: 'user_access'
    | stat("pyproject.toml").user_access
    | ^

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Super-helpful! Thanks! Important underlying principle: don't lump these functions together; each one is a unique, precious snowflake.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah that's even better 😎 . Comparing the full JSON of a stat("pyproject.toml") I see...

main --> 8e9a0cc

@@ -1,5 +1,4 @@
 {
-  "group_access": true,
   "kind": "f",
   "r": true,
   "setgid": false,
@@ -12,7 +11,6 @@
   "st_nlink": 1,
   "st_size": 4670,
   "sticky": false,
-  "user_access": true,
   "w": true,
   "x": false
 }

Your last change restores the 2 lost attributes and adds a few more. 8e9a0cc --> 8efc843

 {
+  "group_access": true,
   "kind": "f",
   "r": true,
   "setgid": false,
   "setuid": false,
   "st_atime": "2026-07-02T10:49:38Z",
+  "st_blksize": 4096,
+  "st_blocks": 16,
   "st_ctime": "2026-07-02T10:49:38Z",
   "st_dev": 64513,
   "st_ino": 55182052,
   "st_mtime": "2026-07-02T10:49:38Z",
   "st_nlink": 1,
+  "st_rdev": 0,
   "st_size": 4670,
   "sticky": false,
+  "user_access": true,
   "w": true,
   "x": false
 }

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good! Your comments make it clear I need Windows and Ubuntu Docker images to do more thorough testing.

Instead of an overall "all-or-nothing" test each extra feature individually to see if it's part of the current OS.
@slott56 slott56 requested a review from ajkerrigan July 2, 2026 16:10

@ajkerrigan ajkerrigan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, looks good 👍

@slott56 slott56 merged commit 9569e76 into main Jul 2, 2026
15 checks passed
@slott56 slott56 deleted the branch_177 branch July 2, 2026 20:30
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.

The __main__.py module breaks under Windows

2 participants