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

tarfile add method bombs because lstat returns a uid of None #449

Closed
ut-adamc opened this issue Nov 7, 2018 · 7 comments
Closed

tarfile add method bombs because lstat returns a uid of None #449

ut-adamc opened this issue Nov 7, 2018 · 7 comments

Comments

@ut-adamc
Copy link

ut-adamc commented Nov 7, 2018

When you call tarfile's .add method, it calls lstat underneath the scenes. When using fake_filesystem_unittest, the lstat call returns an st_uid value of None. This causes tarfile to raise an exception.

The following example demonstrates the problem. I tested with version 3.5.

import os
import tarfile
import unittest

from pyfakefs import fake_filesystem_unittest as pyfakefs_unittest

class TestExample(pyfakefs_unittest.TestCase):
    def setUp(self):
        self.setUpPyfakefs()
        self.fakefs_dir = '/fakefs'
        self.fake_src_dir = os.path.join(self.fakefs_dir, 'src_dir')
        self.fake_src_file = os.path.join(self.fake_src_dir, 'src_file.txt')
        os.makedirs(self.fakefs_dir)
        os.makedirs(self.fake_src_dir)
        self.fs.create_file(self.fake_src_file)


    def test_tarfile(self):
        archive_filepath = os.path.join(self.fakefs_dir, 'myarchive.tar.bz2')
        with tarfile.open(archive_filepath, "w|bz2") as tf:
            tf.add(self.fake_src_dir)

if __name__ == '__main__':
    unittest.main()
@mrbean-bremen
Copy link
Member

Currently we do not support st_uid and st_gid, setting them to None, as documented in FakeFile.
The assumption is that the user sets them himself if needed - that should also be possible in your case.

I agree that this is inconvenient - the case just never came up before. Recently, we added the possibility to set the current user ID (via fake_filesystem.set_uid()), but this is not used for st_uid in created files, which it probably should. It could also make sense to add a similar set_gid() function, to set st_gid in new files.
And the default values should probably not be None, but a number, probably 1.
What do you think?

mrbean-bremen added a commit that referenced this issue Nov 8, 2018
@mrbean-bremen
Copy link
Member

@ut-adamc - I made the above-mentioned changes. st_uid and st_gid are now set to 1 by default, and you can change the current user/group id.
Please check if that helps with your problem (I can't reproduce this, as I'm currently working under Windows).

@ut-adamc
Copy link
Author

Thanks. It does get past that error. There is still another error, which I think is due to tarfile saving the prior value of the open function as "bltn_open" and then using that to open the tarfile.

I think for the tarfile module to work, you would also need to hook bltn_open.

@mrbean-bremen
Copy link
Member

This is the kind of thing that is difficult to hook generically. Using the modules_to_reload argument in setUpPyfakefs may help with that case - this is done automatically with thetempfile module, as it also uses a cached open.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Nov 15, 2018

I just looked at the tarfile source code, and the line you have been mentioning is:

from builtins import open as bltn_open

This is not as tricky as I thought - maybe we can handle this generically.
Anyway, setUpPyfakefs(modules_to_reload=[tarfile]) could also work, I'm interested to hear if that helps you.

@mrbean-bremen
Copy link
Member

@ut-adamc - I added code to patch this kind of import automatically. Can you please check if the code in master works for you?

@mrbean-bremen
Copy link
Member

This shall now be fixed in the released version - closing. Please reopen if this is not sufficient for you.

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