Skip to content

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

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

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

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Dec 14, 2018

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

https://bugs.python.org/issue35348

@vstinner
Copy link
Member Author

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.

@vstinner
Copy link
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
Copy link
Member

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>
@vstinner
Copy link
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
Copy link
Member Author

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.

4 participants