[proxy] web.archive.org← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public
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

bpo-35348: Fix platform.architecture() #11159

Merged
merged 2 commits into from Dec 17, 2018
Merged

Conversation

Copy link
Member

vstinner commented Dec 14, 2018

Fix platform.architecture(): add the -b option to the "file" command.

https://bugs.python.org/issue35348

Copy link
Member Author

vstinner commented Dec 14, 2018

I'm not sure if the "file" command supports the -b option on all platforms. It seems like platform used file -b in the past and nobody complained.

Copy link
Member Author

vstinner commented Dec 17, 2018

@serhiy-storchaka: you wrote PR #11160, but I'm confident that it will be fine to use "file -b -- executable" command. I propose to make this change in the master branch and leave stable branches (2.7 and 3.7) unchanged. If something goes wrong, we can easily revert this change.

Copy link
Member

serhiy-storchaka commented Dec 17, 2018

Could you then borrow other changes from #11160?

Make platform.architecture() parsing of "file" command output more
reliable:

* Add the "-b" option to the "file" command to omit the filename;
* Force the usage of the C locale;
* Search also the "shared object" pattern.

Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
Copy link
Member Author

vstinner commented Dec 17, 2018

I removed "--" from "file -b -- target". If target starts with "-", file will fail since the filename miss. Example:

$ file -b -b
Usage: file [-bcCdEhikLlNnprsvzZ0] [--apple] [--extension] [--mime-encoding]
            [--mime-type] [-e <testname>] [-F <separator>]  [-f <namefile>]
            [-m <magicfiles>] [-P <parameter=value>] <file> ...
       file -C [-m <magicfiles>]
       file [--help]

$ echo $?
1

This change should prevent portability issue.

Could you then borrow other changes from #11160?

Sure. I picked your changes into my PR and I credited you.

I replaced env={'LC_ALL': 'C'} with env=dict(os.environ, LC_ALL='C'). Few programs like to run in an (almost) empty environment.

I wasn't sure if you preferred to push your changes separately or not.

vstinner merged commit 0af9c33 into python:master Dec 17, 2018
vstinner deleted the platform_file branch Dec 17, 2018
Copy link
Member Author

vstinner commented Dec 17, 2018

I propose to not backport this fix. At least not the addition of the -b flag.

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 this pull request may close these issues.

None yet

4 participants