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

Duplicated newline in shebang of windows launcher #220

Closed
A2uria opened this issue Aug 23, 2024 · 3 comments · Fixed by #221
Closed

Duplicated newline in shebang of windows launcher #220

A2uria opened this issue Aug 23, 2024 · 3 comments · Fixed by #221

Comments

@A2uria
Copy link
Contributor

A2uria commented Aug 23, 2024

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'.

distlib/distlib/scripts.py

Lines 169 to 170 in 888c48b

if simple_shebang:
result = b'#!' + executable + post_interp + b'\n'

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

distlib/distlib/scripts.py

Lines 255 to 257 in 888c48b

linesep = os.linesep.encode('utf-8')
if not shebang.endswith(linesep):
shebang += linesep

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
Copy link
Collaborator

vsajip commented Aug 23, 2024

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

@A2uria
Copy link
Contributor Author

A2uria commented Aug 24, 2024

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 �[36;1mpython3 -c 'import os; print(os.name); print(os.linesep.encode())'�[0m
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().

distlib/distlib/scripts.py

Lines 172 to 174 in 888c48b

result = b'#!/bin/sh\n'
result += b"'''exec' " + executable + post_interp + b' "$0" "$@"\n'
result += b"' '''"

@A2uria
Copy link
Contributor Author

A2uria commented Aug 24, 2024

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'

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

Successfully merging a pull request may close this issue.

2 participants