Issue2534
Created on 2016-11-28.10:57:59 by pguermo, last changed 2017-03-28.05:27:11 by zyasoft.
Messages | |||
---|---|---|---|
msg10992 (view) | Author: Guermonprez Patrick (pguermo) | Date: 2016-11-28.10:57:58 | |
Hello, I'm migrating from Jython 2.2.1 to 2.7.0 and I face a problem with the method os.getlogin. It was working fine in 2.2.1 but using 2.7.0, it returns either an exception, either a wrong login. user = os.getlogin() at org.python.core.PyString.<init>(PyString.java:62) at org.python.core.PyString.<init>(PyString.java:70) at org.python.modules.posix.PosixModule.getlogin(PosixModule.java:510) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:497) java.lang.IllegalArgumentException: java.lang.IllegalArgumentException: Cannot create PyString from null Thanks, Patrick |
|||
msg11100 (view) | Author: James Mudd (jamesmudd) | Date: 2017-02-19.20:00:54 | |
I have been having a look at this issue as I am also seeing this. But I think it is OS dependent. It seems to work ok on RHEL6 but it is failing on Ubuntu 16.04 64 bit. Jython 2.7.1b3 (, Feb 19 2017, 19:57:56) [Java HotSpot(TM) 64-Bit Server VM (Oracle Corporation)] on java1.8.0_121 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.getlogin() Traceback (most recent call last): File "<stdin>", line 1, in <module> at org.python.core.PyString.<init>(PyString.java:62) at org.python.core.PyString.<init>(PyString.java:70) at org.python.modules.posix.PosixModule.getlogin(PosixModule.java:528) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) java.lang.IllegalArgumentException: java.lang.IllegalArgumentException: Cannot create PyString from null >>> I was running a just built version of jython master. Could you provide some more details of the OS and java you were using. |
|||
msg11102 (view) | Author: James Mudd (jamesmudd) | Date: 2017-02-20.14:06:11 | |
I have also tested this now on RHEL6 and it works ok Jython 2.7.1b3 (, Feb 20 2017, 12:05:03) [Java HotSpot(TM) 64-Bit Server VM (Oracle Corporation)] on java1.8.0_101 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.getlogin() 'james' >>> So i'm sure this is something to do with the jffi calls to the OS. Would be interesting to test other platforms, I will try Windows when I get a chance |
|||
msg11103 (view) | Author: Stefan Richthofer (stefan.richthofer) | Date: 2017-02-20.15:44:58 | |
I see this too, but it even fails in CPython for me: Python 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.getlogin() Traceback (most recent call last): File "<stdin>", line 1, in <module> OSError: [Errno 2] No such file or directory >>> Did you check if os.getlogin works in CPython? Then I found the following: https://mail.python.org/pipermail/python-bugs-list/2002-July/012691.html (Yes it is rather old, but it seems to cover the error I observe.) So, given that os.getlogin is supposed to be a thin wrapper for https://linux.die.net/man/3/getlogin (?) Jython's behavior might actually be right if that call fails due to other reasons. Only the error message might better be more sophisticated. That said, it would be trivial to build some fallbacks into PosixModule.getlogin, e.g.: public static PyObject getlogin() { String login = posix.getlogin(); if (login == null) { login = System.getenv("LOGNAME"); } if (login == null) { login = System.getenv("USER"); } if (login == null) { login = System.getProperty("user.name"); } return new PyString(login); } originally: public static PyObject getlogin() { return new PyString(posix.getlogin()); } (posix is JNR's posix-wrapper) However os.getlogin shall only work on terminal/live console if at all, which would not hold then any more. So code using this to detect console-mode would break (bad way for detection anyway?). So maybe we should better keep it that way, like a thin wrapper and apply such fallbacks on Python-level, i.e. os.getenv('LOGNAME'), os.getenv('USER'), os.getenv('USERNAME') (windows) I am actually undecided, because I favor portable wrappers and guess these fallbacks could prevent failures in naive real-world existing code. Opinions? |
|||
msg11104 (view) | Author: James Mudd (jamesmudd) | Date: 2017-02-20.21:09:37 | |
Your right it fails for me in CPython as well in the same way on Ubuntu 16.04 64 bit Python 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.getlogin() Traceback (most recent call last): File "<stdin>", line 1, in <module> OSError: [Errno 2] No such file or directory >>> This is discussed in the Python docs https://docs.python.org/2/library/os.html#os.getlogin where they suggest using the environment variable 'LOGNAME' or "pwd.getpwuid(os.getuid())[0]" both appear to work correctly in Jython, so maybe these are more reliable approaches which should be used instead. Jython 2.7.1b3 (, Feb 19 2017, 19:57:56) [Java HotSpot(TM) 64-Bit Server VM (Oracle Corporation)] on java1.8.0_121 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.getenv('LOGNAME') 'james' >>> import pwd >>> pwd.getpwuid(os.getuid())[0] 'james' >>> So I think this issue should be closed as the Jython behaviour seems consistent with CPython? I don't really like the idea of adding the fallbacks to PosixModule think it's quite clean as it is. |
|||
msg11105 (view) | Author: Stefan Richthofer (stefan.richthofer) | Date: 2017-02-20.21:49:46 | |
Maybe you're right. Let me make one more point for the fallbacks: One could say that Jython's platform is Java and so the wrapper can be a bit more portable. On the other hand in strict sense the posix module shouldn't be available then anyway: Java isn't posix, is it? I looked into JNR, which backs the posix module and they also have the option to use a purely Java-backed posix. So in that sense Java actually is posix. Its getlogin wrapper is in turn bakced by getlogin() in https://github.com/jnr/jnr-posix/blob/master/src/main/java/jnr/posix/JavaLibCHelper.java, which plainly uses System.getProperty("user.name"). If we changed private static final POSIX posix = POSIXFactory.getPOSIX(posixHandler, true); into private static final POSIX posix = POSIXFactory.getJavaPOSIX(posixHandler, true); this would force using the Java backend, yielding the same behavior as the proposed fallbacks. However, compared to this I would still prefer the fallbacks, because System.getenv("LOGNAME")/System.getenv("USER") is said to be more reliable than os.user property. All in all I'd say we'd better include the fallbacks if there is some (major?) framework affected by this. If it's 'only' enduser code the endusers should better move on to a more suitable solution as suggested in https://docs.python.org/2/library/os.html#os.getlogin. Patrick: What is your view on this? I also wonder how this can have worked in Jython 2.2.1. Maybe PosixModule has been rewritten since then and they had the System.property variant in old days. Finally, I don't like the error message. That could be improved. Downside here is that CPython's variant isn't ideal either, so I see no point in reproducing that one. I suppose they also just put through some original cause that wasn't especially caught. Anyway, maybe we could still improve on that point? Suggestions? |
|||
msg11113 (view) | Author: James Mudd (jamesmudd) | Date: 2017-02-22.19:46:48 | |
I like the idea of making the platform pure Java, but then I agree Jython shouldn't implement anything that you can do within the JVM. (By default I really like JyNI though! Hoping to get some more time to look at it soon) Had another idea for a solution for this maybe we should just return None if the OS call fails I did this in https://github.com/jythontools/jython/pull/56 It's actually easy as getlogin() already returns PyObject. I'm not sure its a great solution though what do you think? It avoids the error message issue and is quite faithful to https://linux.die.net/man/3/getlogin If this was adopted it would still be down to the user code to handle the None in some way but the error would be gone. I don't think the "private static final POSIX posix = POSIXFactory.getJavaPOSIX(posixHandler, true);" would be useful almost all the methods in PosixModule disappear in this case because of the "@Hide(value=OS.NT, posixImpl = PosixImpl.JAVA)" annotation. Also the github repo is a bit behind the mercurial one I have another issue for that http://bugs.jython.org/issue2556 |
|||
msg11116 (view) | Author: Stefan Richthofer (stefan.richthofer) | Date: 2017-02-23.16:28:44 | |
Your're right: I overlooked that @Hide also rules out PosixImpl.JAVA. Regarding None I'm not sure how that benefits the user: He/she can as well catch the exception. IMO it would be more convenient to let at least the exception type match that of CPyton so the except-statement can be easily written in a portable way. So given that CPython happens to raise OSError we should do so as well. I'd suggest to let the error message point user directly to the better workaround, e.g. something like: "getlogin failed. It is recommended to rather use os.getenv('LOGNAME')/os.getenv('USER') on posix or os.getenv('USERNAME') on Windows." |
|||
msg11118 (view) | Author: James Mudd (jamesmudd) | Date: 2017-02-23.20:26:16 | |
I agree making the Jython behaviour consistent with CPython is probably the best. So throw OSError but with a more helpful messages. I have a pull request to do this https://github.com/jythontools/jython/pull/57 I haven't handled the Windows case as the getlogin method is hidden on Windows by the annotation. |
|||
msg11122 (view) | Author: Stefan Richthofer (stefan.richthofer) | Date: 2017-02-24.11:31:16 | |
Fixed via PR by James Mudd as of https://hg.python.org/jython/rev/07724b0547e0. Note that I marked Resolution as "invalid" to display that "fix" in this case means that we only improved the error-message. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2017-03-28 05:27:11 | zyasoft | set | status: pending -> closed |
2017-02-24 11:31:16 | stefan.richthofer | set | status: open -> pending messages: + msg11122 priority: normal assignee: stefan.richthofer milestone: Jython 2.7.1 keywords: + console resolution: invalid |
2017-02-23 20:26:17 | jamesmudd | set | messages: + msg11118 |
2017-02-23 16:28:45 | stefan.richthofer | set | messages: + msg11116 |
2017-02-22 19:46:48 | jamesmudd | set | messages: + msg11113 |
2017-02-20 21:49:46 | stefan.richthofer | set | messages: + msg11105 |
2017-02-20 21:09:37 | jamesmudd | set | messages: + msg11104 |
2017-02-20 15:44:59 | stefan.richthofer | set | nosy:
+ stefan.richthofer, zyasoft messages: + msg11103 |
2017-02-20 14:06:12 | jamesmudd | set | messages: + msg11102 |
2017-02-19 20:00:55 | jamesmudd | set | nosy:
+ jamesmudd messages: + msg11100 |
2016-11-28 10:58:20 | pguermo | set | title: os.getlogin() returns a wrong user or returns an exceptions -> os.getlogin() returns a wrong user or returns an exception |
2016-11-28 10:58:13 | pguermo | set | title: os.getlogin() returns a wrong user or return an exception -> os.getlogin() returns a wrong user or returns an exceptions |
2016-11-28 10:57:59 | pguermo | create |
Supported by Python Software Foundation,
Powered by Roundup