bpo-35348: Fix platform.architecture() by vstinner · Pull Request #11159 · python/cpython · GitHub
Skip to content

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

Merged
vstinner merged 2 commits into
python:masterfrom
vstinner:platform_file
Dec 17, 2018
Merged

bpo-35348: Fix platform.architecture()#11159
vstinner merged 2 commits into
python:masterfrom
vstinner:platform_file

Conversation

@vstinner

@vstinner vstinner commented Dec 14, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

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

https://bugs.python.org/issue35348

@vstinner

vstinner commented Dec 14, 2018

Copy link
Copy Markdown
Member Author

@vstinner vstinner mentioned this pull request Dec 14, 2018
Closed
@vstinner

vstinner commented Dec 17, 2018

Copy link
Copy Markdown
Member Author

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

@serhiy-storchaka

serhiy-storchaka commented Dec 17, 2018

Copy link
Copy Markdown
Member

Could you then borrow other changes from #11160?

@vstinner @serhiy-storchaka
bpo-35348: Fix platform.architecture()
2ab40aa 
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>
@vstinner

vstinner commented Dec 17, 2018

Copy link
Copy Markdown
Member Author

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
vstinner merged commit 0af9c33 into python:master Dec 17, 2018
@vstinner
vstinner deleted the platform_file branch December 17, 2018 17:47
@vstinner

vstinner commented Dec 17, 2018

Copy link
Copy Markdown
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@jcea jcea Awaiting requested review from jcea

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot