Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Several global constants lost in 5.2.1 #809

Closed
davidlbaird opened this issue Apr 11, 2023 · 10 comments
Closed

Several global constants lost in 5.2.1 #809

davidlbaird opened this issue Apr 11, 2023 · 10 comments

Comments

@davidlbaird
Copy link
Collaborator

Upon importing pyfakefs 5.2.1, we see numerous errors due to lost global constants in fake_filesystem.py.

No attribute 'PERM_ALL' 
No attribute 'PERM_DEF'
No attribute 'PERM_DEF_FILE'
No attribute 'PERM_EXE'
No attribute 'PERM_READ'
No attribute 'PERM_WRITE'
No attribute 'USER_ID'

Please re-define them, for backward compatibiltiy.

Thanks.

@davidlbaird
Copy link
Collaborator Author

Instead of USER_ID, I think a get_uid function should be added.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Apr 12, 2023

Thanks! Sorry for that, that had been due to the refactoring I made for 5.2.0 (to make pytype a bit faster...).
I think I had omitted the PERM_ constants intentionally (moved them to helpers), not being aware that they may be used from outside. Bad guess...
Same for USER_ID, which can be set using set_uid and get using os.getuid, but I guess get_uid would be a good idea for symmetry.
Will get at this tonight.

@davidlbaird
Copy link
Collaborator Author

getuid is conspicuously missing from FakeOsModule. I also found that simply setting USER_ID = helpers.USER_ID in fake_filesystem.py does not capture changes to the value after calling set_uid.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Apr 12, 2023

getuid is conspicuously missing from FakeOsModule

Hm, I was sure that is there, but you are right of course. Will add this.

I also found that simply setting USER_ID = helpers.USER_ID in fake_filesystem.py does not capture changes to the value after calling set_uid

Also true. This will only correctly work for constants, not for mtuable POD globals, as otherwise always the local copy is used. This can only be fixed by moving the globals back, or adapting the user code to use get_uid instead, after it is added (or os.getuid after it is fixed).

Probably have to move the globals back...

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Apr 12, 2023

Adding these back opens a can of worm regarding cyclic dependencies, so I'm not sure yet how to handle this properly without reverting the whole refactoring. I'll probably leave them for the moment and think about how to do this later...

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 12, 2023
- some if these had been accessed in user code,
  which caused a regression
- add convenience function get_uid()/get_gid()
- fixes pytest-dev#809
mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 12, 2023
- some if these had been accessed in user code,
  which caused a regression
- add convenience function get_uid()/get_gid()
- fixes pytest-dev#809
mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 12, 2023
- some if these had been accessed in user code,
  which caused a regression
- add convenience function get_uid()/get_gid()
- fixes pytest-dev#809
@davidlbaird
Copy link
Collaborator Author

davidlbaird commented Apr 12, 2023

Adding this also works, but perhaps accessing the value through os.getuid (and setting, too? look through the os module's functions) is optimal.

def __getattr__(name):
    if name == 'USER_ID':
        return helpers.USER_ID
    raise AttributeError("No attribute %r." % name)

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 12, 2023
- some if these had been accessed in user code,
  which caused a regression
- add convenience function get_uid()/get_gid()
- fixes pytest-dev#809
@mrbean-bremen
Copy link
Member

mrbean-bremen commented Apr 12, 2023

Adding this also works

Ah, thanks - I forgot about this! I had already used that elsewhere... This is read-only, there is no possibility for a module-level property setter as far as I know.
Yes, I will add this for backwards compatibilty to read the globals - setting them has to be done via set_uid, which was available and documented before. I will add this to the PR, and you may have a look at it.
Edit: Done.

Thanks for the help!

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Apr 12, 2023
- some if these had been accessed in user code,
  which caused a regression
- add convenience function get_uid()/get_gid()
- see pytest-dev#809
@mrbean-bremen
Copy link
Member

Please check if the changes are sufficient for you - if yes, I will merge them and probably make another patch release shortly.

@davidlbaird
Copy link
Collaborator Author

I looked at the changes, and they will work. Thanks!

mrbean-bremen added a commit that referenced this issue Apr 13, 2023
- some if these had been accessed in user code,
  which caused a regression
- add convenience function get_uid()/get_gid()
- see #809
@mrbean-bremen
Copy link
Member

Fixed in just released version 5.2.2, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants