Skip to content

Conversation

@methane
Copy link
Member

@methane methane commented Mar 29, 2019

On Unix, this doesn't have any effect.
On Windows, "r" mode will cause error on ftell.

https://bugs.python.org/issue20844

@methane
Copy link
Member Author

methane commented Mar 29, 2019

OK, AppVeyor failed on added test:

======================================================================
FAIL: test_issue20884 (test.test_cmd_line_script.CmdLineTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\cpython\lib\test\test_cmd_line_script.py", line 425, in test_issue20884
    rc, out, err = assert_python_ok(script_name)
  File "C:\projects\cpython\lib\test\support\script_helper.py", line 157, in assert_python_ok
    return _assert_python(True, *args, **env_vars)
  File "C:\projects\cpython\lib\test\support\script_helper.py", line 143, in _assert_python
    res.fail(cmd_line)
  File "C:\projects\cpython\lib\test\support\script_helper.py", line 70, in fail
    raise AssertionError("Process return code is %d\n"
AssertionError: Process return code is 1
command line: ['C:\\projects\\cpython\\PCbuild\\amd64\\python.exe', '-X', 'faulthandler', '-I', 'C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\tmpnejtdxss\\issue20884.py']
stdout:
---
---
stderr:
---
File "C:\Users\appveyor\AppData\Local\Temp\1\tmpnejtdxss\issue20884.py", line 1
SyntaxError: encoding problem: iso-8859-1
---
----------------------------------------------------------------------

Same fail on Azure x86 and amd64

@ghost
Copy link

ghost commented Mar 30, 2019

Should revert some of this change (git commit 815b41b) in issue20731?

Eryk Sun mentioned it in msg221134.

@ghost
Copy link

ghost commented Mar 31, 2019

Eryk Sun said in msg273110:

That would also entail documenting that PyRun_SimpleFileExFlags requires a FILE pointer that's opened in binary mode.

Not sure whether this advice is needed.

@methane
Copy link
Member Author

methane commented Apr 1, 2019

Should revert some of this change (git commit 815b41b) in issue20731?

I will backport this change to 3.7 branch. So I want to keep patch simple, safe, and no regression.
It will be solved more good way in the future. But I don't care it.

@methane methane merged commit 10654c1 into python:master Apr 1, 2019
@miss-islington
Copy link
Contributor

Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@methane methane deleted the use-rb branch April 1, 2019 09:35
@miss-islington
Copy link
Contributor

Sorry, @methane, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 10654c19b5e6efdf3c529ff9bf7bcab89bdca1c1 3.7

methane added a commit that referenced this pull request Apr 1, 2019
methane added a commit that referenced this pull request Apr 1, 2019
(cherry picked from commit 10654c1)

Co-authored-by: Inada Naoki <songofacandy@gmail.com>
@bedevere-bot
Copy link

GH-12647 is a backport of this pull request to the 3.7 branch.

methane added a commit that referenced this pull request Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OS-windows type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants