-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
uu.py calls os.path.chmod which doesn't exist #77868
Comments
Library file uu.py on at least 2.7 and 3.6 contains:
As far as I can tell, os.path.chmod does not exist, so this always raises AttributeError and becomes a no-op. Is suspect the proper fix is to remove the ".path" ? |
Looks as the proper fix to me. |
Would be nice to add a test. |
I've added a test and updated the PR. |
Is the uu module still maintained? Christian Heimes wants to remove the module: Xiang Zhang made a change in uu last year to add a new feature: new backtick optional parameter, bpo-30103. |
The email module uses it, so I would object to its being removed. It may not be used often (probably only when working with old email archives), but there's no good reason I can see to break something that currently works. |
I modified it for the feature, not maintain it. I remember at that time I get the feeling it's somewhat strange there are two APIs, with similar functionality resides in two modules, need to be updated. |
What is your use case, Poul-Henning? It looks like the module has never set the file mode, at least since it was added to Python in 1994. I suggest just remove the dead code and fix the documentation. At least you shouldn’t make this change in bug fix releases. It is just as likely to break people’s code as fix it. If you do want to change the behaviour, I suggest using “os.open” to set the mode when the file is created. Otherwise you risk errors and messing up the filesystem when using special files e.g. uu.decode(..., "/dev/null"). |
You cannot just remove the mode parameter without a deprecation period. Right now, the flag is documented: https://docs.python.org/dev/library/uu.html I agree with Martin that I am not comfortable to fix the bug in stable branches (2.7, 3.6, 3.7). I'm +0 to fix the uu module in master (future 3.8). If we fix the bug in master, we should document that mode is ignored in 2.7, 3.6 and 3.7 (update their documentation). |
I was just playing with it in a prototype and noticed that it didn't work. |
I concur with Martin. If this feature never worked, there is a risk of breaking user code when fix it. Let consider this as adding a new feature in 3.8. For older versions it should be documented that the mode of the output file is not set. And I agree that it would be more safe to use os.open(). Either call os.open() and pass the file descriptor to the builtin open() (but be careful to not leak it) or use the opener argument of open(). def opener(path, flags):
return os.open(path, flags, mode)
fp = open(out_file, 'wb', opener=opener) It would be worth also to use the 'xb' opening mode instead of 'wb'. |
But 'xb' should be used only if out_file is not specified. |
Is there really an use case which requires to set the permission of the file created by uu.decode()? It is already possible to call uu.decode() with an open file which has been created with the expected permission. Moreover, it's also possible to explicitly call chmod() *after* uu.decode(). I would suggest to deprecate the mode parameter in Python 3.7 and remove it from Python 3.8. |
Please note that the mode is not just a parameter, it is also a data field inside the encoded input. See: https://en.wikipedia.org/wiki/Uuencoding (search for "mode") |
Oh... right... "<mode> is the file's Unix file permissions as three octal digits (e.g. 644, 744). This is typically only significant to unix-like operating systems." Ok, in that case, it's maybe better to fix the bug instead of removing the parameter. |
Thank you for the report, Poul-Henning and thank you for the PR, Timo! |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: