pypa / distlib

A low-level library which implements some Python packaging standards (PEPs) and which could be used by third-party packaging tools to achieve interoperability.
http://distlib.readthedocs.io/
Other
48 stars 36 forks source link

Duplicated newline in shebang of windows launcher #220

Open A2uria opened 3 weeks ago

A2uria commented 3 weeks ago
0001a600: 2321 2243 3a5c 5072 6f67 7261 6d20 4669  #!"C:\Program Fi
0001a610: 6c65 735c 5079 7468 6f6e 3331 305c 7079  les\Python310\py
0001a620: 7468 6f6e 2e65 7865 220a 0d0a 504b 0304  thon.exe"...PK..
                                ^^^^^^^

ScriptMaker._build_shebang hardcoded a b'\n'.

https://github.com/pypa/distlib/blob/888c48b56886b03398646be1217508830427bd75/distlib/scripts.py#L169-L170

However, ScriptMaker._write_script will check for b'\r\n', which is non existant.

https://github.com/pypa/distlib/blob/888c48b56886b03398646be1217508830427bd75/distlib/scripts.py#L255-L257

Remove the hardcoded b'\n' and let ScriptMaker._write_script to generate newline for shebang is more universal but check for a single b'\n' on windows might be better.

vsajip commented 3 weeks ago

Or maybe just do the equivalent of result = b'#!' + executable + post_interp + os.linesep.encode('utf-8') in _build_shebang()?

A2uria commented 3 weeks ago

Or maybe just do the equivalent of result = b'#!' + executable + post_interp + os.linesep.encode('utf-8') in _build_shebang()?

os.linesep.encode('utf-8') is \r\n on windows but \n is enough.

Maybe we can ensure the shebang ends in \n. This should work on both windows and linux.

It seems that os.linesep is \n on macos, so it should work either.

``` 2024-08-24T02:07:18.2271430Z Current runner version: '2.319.1' 2024-08-24T02:07:18.2288270Z ##[group]Operating System 2024-08-24T02:07:18.2288710Z macOS 2024-08-24T02:07:18.2288980Z 14.6.1 2024-08-24T02:07:18.2289400Z 23G93 2024-08-24T02:07:18.2289670Z ##[endgroup] 2024-08-24T02:07:18.2289970Z ##[group]Runner Image 2024-08-24T02:07:18.2290340Z Image: macos-14-arm64 2024-08-24T02:07:18.2290670Z Version: 20240818.4 2024-08-24T02:07:18.2291530Z Included Software: https://github.com/actions/runner-images/blob/macos-14-arm64/20240818.4/images/macos/macos-14-arm64-Readme.md 2024-08-24T02:07:18.2292630Z Image Release: https://github.com/actions/runner-images/releases/tag/macos-14-arm64%2F20240818.4 2024-08-24T02:07:18.2293360Z ##[endgroup] 2024-08-24T02:07:18.2294020Z ##[group]Runner Image Provisioner 2024-08-24T02:07:18.2294480Z 2.0.374.1+4097a9592d27ce71de414581a65bffbda888dd1b 2024-08-24T02:07:18.2295020Z ##[endgroup] 2024-08-24T02:07:18.2303940Z ##[group]GITHUB_TOKEN Permissions 2024-08-24T02:07:18.2304880Z Contents: read 2024-08-24T02:07:18.2305210Z Metadata: read 2024-08-24T02:07:18.2305600Z Packages: read 2024-08-24T02:07:18.2305900Z ##[endgroup] 2024-08-24T02:07:18.2307520Z Secret source: Actions 2024-08-24T02:07:18.2307930Z Prepare workflow directory 2024-08-24T02:07:18.2721040Z Prepare all required actions 2024-08-24T02:07:18.2867370Z Complete job name: run 2024-08-24T02:07:18.3693490Z ##[group]Run python3 -c 'import os; print(os.name); print(os.linesep.encode())' 2024-08-24T02:07:18.3695380Z python3 -c 'import os; print(os.name); print(os.linesep.encode())' 2024-08-24T02:07:18.5044460Z shell: /bin/bash --noprofile --norc -e -o pipefail {0} 2024-08-24T02:07:18.5044940Z ##[endgroup] 2024-08-24T02:07:18.7285780Z posix 2024-08-24T02:07:18.7286400Z b'\n' 2024-08-24T02:07:18.7494070Z Cleaning up orphan processes ```
https://github.com/python/cpython/blob/556e8556849cb9df0666629b0f564b5dd203344c/Lib/os.py#L52-L54 https://github.com/python/cpython/blob/556e8556849cb9df0666629b0f564b5dd203344c/Modules/posixmodule.c#L536-L544

This should work.

if not shebang.endswith(b'\n'): 
    shebang += b'\n'

Or just hardcode a newline here in case of contrived shebang and remove the check, since _build_shebang() is always called before _write_script().

https://github.com/pypa/distlib/blob/888c48b56886b03398646be1217508830427bd75/distlib/scripts.py#L172-L174

A2uria commented 3 weeks ago

I suppose that _write_script() is private, so removing the check is ok since there is a hardcoded terminating newline in _build_shebang(). If this is not expected, the following code can be used instead.

if not shebang.endswith(b'\n'): 
    shebang += b'\n'