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

Breakage with Python 3.7: 'str' object has no attribute 'decode' #13

Closed
hhoffstaette opened this issue Jul 22, 2018 · 13 comments
Closed

Breakage with Python 3.7: 'str' object has no attribute 'decode' #13

hhoffstaette opened this issue Jul 22, 2018 · 13 comments

Comments

@hhoffstaette
Copy link

@hhoffstaette hhoffstaette commented Jul 22, 2018

Hooray, it's Python time 💩

Tinkering around with the examples and everything works fine with python 3.6:

PYTHONPATH=/tmp/python-btrfs/build/lib/ sudo python3.6 fs_info.py /mnt/backup
max_id 1 num_devices 1 fsid d163af2f-6e03-4972-bfd6-30c68b6ed312 nodesize 16384 sectorsize 4096 clone_alignment 4096
devid 1 uuid 01d2fa5e-0994-4d29-add7-8596122817ca bytes_used 1348686839808 total_bytes 4000785960960 path /dev/sdc1
devid 1 write_errs 0 read_errs 0 flush_errs 0 generation_errs 0 corruption_errs 0

Let's try the new hotness:

PYTHONPATH=/tmp/python-btrfs/build/lib/ sudo python3.7 fs_info.py /mnt/backup
Traceback (most recent call last):
  File "fs_info.py", line 3, in <module>
    import btrfs
  File "/tmp/python-btrfs/examples/btrfs/__init__.py", line 23, in <module>
    from btrfs.ctree import FileSystem  # noqa
  File "/tmp/python-btrfs/examples/btrfs/ctree.py", line 355, in <module>
    import btrfs.ioctl  # noqa
  File "/tmp/python-btrfs/examples/btrfs/ioctl.py", line 215, in <module>
    ioctl_search_key.format.decode(), 4096 - ioctl_search_key.size))
AttributeError: 'str' object has no attribute 'decode'

I dug through the Python 3.7 Changelog and think the relevant change is here:
"The struct.Struct.format type is now str instead of bytes."

Reading the bug discussion it seems this was actually a bug in <3.7, and the new behaviour is "correct". Not sure how to handle this so that it works with any version, maybe just check the type.

@knorrie
Copy link
Owner

@knorrie knorrie commented Jul 22, 2018

Sigh... I guess so... More if statements, or a wrapper function. Thanks for testing!

knorrie added a commit that referenced this issue Aug 12, 2018
https://docs.python.org/3/whatsnew/3.7.html#changes-in-the-python-api
  "The struct.Struct.format type is now str instead of bytes."

So, put in a wrapper function to handle both scenarios.

Thanks Holger Hoffstätte for reporting the issue.

Github: Fixes: #13
@knorrie
Copy link
Owner

@knorrie knorrie commented Aug 12, 2018

Hi, can you please test the develop branch? I put a little wrapper function in that looks at the type of the result of struct.format.

@hhoffstaette
Copy link
Author

@hhoffstaette hhoffstaette commented Aug 12, 2018

Looks good, now works with 3.7 as well:

PYTHONPATH=/tmp/python-btrfs/build/lib/ sudo python3.7 examples/fs_info.py /mnt/backup 
max_id 1 num_devices 1 fsid d163af2f-6e03-4972-bfd6-30c68b6ed312 nodesize 16384 sectorsize 4096 clone_alignment 4096
devid 1 uuid 01d2fa5e-0994-4d29-add7-8596122817ca bytes_used 1370161676288 total_bytes 4000785960960 path /dev/sdc1
devid 1 write_errs 0 read_errs 0 flush_errs 0 generation_errs 0 corruption_errs 0

Verified that it also still works with 3.6, giving the same result. Thanks!

@knorrie
Copy link
Owner

@knorrie knorrie commented Aug 12, 2018

Thanks. Now go do less boring things than fs_info ;-]

I'm finishing up the big usage / wasted space refactoring, and will probably do a release then, maybe even including the first part of the Turorial.

knorrie added a commit that referenced this issue Aug 14, 2018
https://docs.python.org/3/whatsnew/3.7.html#changes-in-the-python-api
  "The struct.Struct.format type is now str instead of bytes."

So, put in a wrapper function to handle both scenarios.

Thanks Holger Hoffstätte for reporting the issue.

Github: Fixes: #13
@jorti
Copy link
Contributor

@jorti jorti commented Oct 20, 2018

With this commit I still fail to import the module:

$ python3
Python 3.7.0 (default, Aug 30 2018, 14:32:33) 
[GCC 8.2.1 20180801 (Red Hat 8.2.1-2)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import btrfs
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.7/site-packages/btrfs/__init__.py", line 23, in <module>
    from btrfs.ctree import FileSystem  # noqa
  File "/usr/lib/python3.7/site-packages/btrfs/ctree.py", line 360, in <module>
    import btrfs.ioctl  # noqa
  File "/usr/lib/python3.7/site-packages/btrfs/ioctl.py", line 215, in <module>
    btrfs.ctree.struct_format(ioctl_search_key), 4096 - ioctl_search_key.size))
AttributeError: module 'btrfs' has no attribute 'ctree'
@knorrie
Copy link
Owner

@knorrie knorrie commented Oct 20, 2018

This is not the original problem, it's some circular import things go wrong.

Which code are you exactly running here? Which git commit? I can't reproduce this. I also have python3.7 here now to test with.

@jorti
Copy link
Contributor

@jorti jorti commented Oct 22, 2018

I'm applying the patch over the v9 release. Is it intended only to master?

@knorrie knorrie closed this in baa63d9 Oct 22, 2018
@knorrie
Copy link
Owner

@knorrie knorrie commented Oct 22, 2018

Ah, yes, then you also need the commit 'Work around circular imports'. To fix this right now for you, I reorganized my develop branch, moved the fixes to the beginning and merged those two commits to master, which means they won't change anymore.

I can also release this as 9.1 if you want.

@jorti
Copy link
Contributor

@jorti jorti commented Oct 22, 2018

A 9.1 would be great. I'm the maintainer of the Fedora package and an official release would be preferred.

Thanks!

@knorrie
Copy link
Owner

@knorrie knorrie commented Oct 22, 2018

Sure, done. I also prepared the debian packages for unstable, so that they're in before python 3.7 moves from debian experimental to unstable.

@antonagestam
Copy link

@antonagestam antonagestam commented Dec 3, 2018

@knorrie Pushing 9.1 to PyPI would be super appreciated if you find the time!

@knorrie
Copy link
Owner

@knorrie knorrie commented Dec 3, 2018

Ahem, yes. Thanks for the reminder. Will do tonight.

@knorrie
Copy link
Owner

@knorrie knorrie commented Dec 3, 2018

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.