Skip to content

Force Python 3.x from env in shebang lines #18285

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.

Sign up for GitHub

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

Jump to bottom
Merged
akoeplinger merged 2 commits into from
Jan 7, 2020
Merged

Force Python 3.x from env in shebang lines #18285

akoeplinger merged 2 commits into from
Jan 7, 2020

Conversation

@NattyNarwhal
Copy link
Contributor

@NattyNarwhal NattyNarwhal commented Dec 23, 2019

Extracted from #18284

@NattyNarwhal
Copy link
Contributor Author

NattyNarwhal commented Dec 26, 2019

FWIW this is useful for FreeBSD (so ports system can replace shebang lines, but it has to be consistent) per @rootwyrm

CoffeeFlux
CoffeeFlux approved these changes Dec 30, 2019
View reviewed changes
Copy link
Contributor

@CoffeeFlux CoffeeFlux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine in theory, though the test failures are relevant and should be resolved. Looks like the results of subprocess.check_output in both outgoingPath and incomingPath need to be converted to a string since that function returns a byte array?

@CoffeeFlux
Copy link
Contributor

CoffeeFlux commented Jan 6, 2020

Apparently RHEL6 is Py2.x and supported until Nov. @marek-safar once the build failures are fixed, is this safe to merge given the state of py2 support overall or do we have to hold off?

@marek-safar
Copy link
Member

marek-safar commented Jan 7, 2020

Switching to python3 is fine and actually desirable going forward

akoeplinger
akoeplinger approved these changes Jan 7, 2020
View reviewed changes
Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ready to me.

@CoffeeFlux
Copy link
Contributor

CoffeeFlux commented Jan 7, 2020

@monojenkins build failed

@akoeplinger
Copy link
Member

akoeplinger commented Jan 7, 2020

The webassembly failures are unrelated, merging.

@akoeplinger akoeplinger merged commit 524a829 into mono:master Jan 7, 2020
ManickaP pushed a commit to ManickaP/runtime that referenced this pull request Jan 20, 2020
@NattyNarwhal @akoeplinger
Force Python 3.x from env in shebang lines (mono/mono#18285)
ef9df08 
* Fix byte/string incompatiblity in Python 3


Commit migrated from mono/mono@524a829
@lewurm
Copy link
Contributor

lewurm commented Apr 2, 2020

@monojenkins backport 2019-12

@monojenkins monojenkins mentioned this pull request Apr 2, 2020
Merged
UnityAlex pushed a commit to Unity-Technologies/mono that referenced this pull request Dec 13, 2022
@NattyNarwhal @UnityAlex
Force Python 3.x from env in shebang lines (mono#18285)
56c6880 
* Fix byte/string incompatiblity in Python 3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akoeplinger akoeplinger akoeplinger approved these changes

@BrzVlad BrzVlad Awaiting requested review from BrzVlad

@directhex directhex Awaiting requested review from directhex

@kg kg Awaiting requested review from kg

@kjpou1 kjpou1 Awaiting requested review from kjpou1

@lambdageek lambdageek Awaiting requested review from lambdageek

@lewing lewing Awaiting requested review from lewing

@lewurm lewurm Awaiting requested review from lewurm

@marek-safar marek-safar Awaiting requested review from marek-safar

@naricc naricc Awaiting requested review from naricc

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

@vargaz vargaz Awaiting requested review from vargaz

+1 more reviewer

@CoffeeFlux CoffeeFlux CoffeeFlux approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@NattyNarwhal @CoffeeFlux @marek-safar @akoeplinger @lewurm