Skip to content

BUG, DIST: Print os error message when the executable not exist#20353

Merged
rgommers merged 1 commit intonumpy:mainfrom
seiko2plus:issue_20335
Nov 14, 2021
Merged

BUG, DIST: Print os error message when the executable not exist#20353
rgommers merged 1 commit intonumpy:mainfrom
seiko2plus:issue_20335

Conversation

@seiko2plus
Copy link
Copy Markdown
Member

@seiko2plus seiko2plus commented Nov 12, 2021 •
edited
Loading

this patch is really important since most of the users aren't able to determine
the build error when the toolchain or the built environment missing
executable files of compiler, linker, assembler, etc.

related to #20335

@seiko2plus seiko2plus mentioned this pull request Nov 12, 2021
Closed
@rgommers
Copy link
Copy Markdown
Member

rgommers commented Nov 13, 2021

Doing some testing:

$ touch numpy/core/src/npysort/radixsort.cpp
$ export CXX=THIS_IS_NOT_A_CXX_COMPILER
$ python setup.py build_ext -i

gives:

compile options: '-DNPY_INTERNAL_BUILD=1 -DHAVE_NPY_CONFIG_H=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE=1 -D_LARGEFILE64_SOURCE=1 -DHAVE_CBLAS -Inumpy/core/src/common -Inumpy/core/src/umath -Inumpy/core/include -Inumpy/core/include/numpy -Ibuild/src.macosx-11.0-arm64-3.9/numpy/distutils/include -Inumpy/core/src/common -Inumpy/core/src -Inumpy/core -Inumpy/core/src/npymath -Inumpy/core/src/multiarray -Inumpy/core/src/umath -Inumpy/core/src/npysort -Inumpy/core/src/_simd -I/Users/rgommers/mambaforge/envs/numpy-dev/include/python3.9 -Inumpy/core/src/common -Inumpy/core/src/npymath -c'
extra options: '-std=c++11 -D__STDC_VERSION__=0 -fno-exceptions -fno-rtti'
THIS_IS_NOT_A_CXX_COMPILER: numpy/core/src/npysort/radixsort.cpp
[Errno 2] No such file or directory: 'THIS_IS_NOT_A_CXX_COMPILER'
error: Command "THIS_IS_NOT_A_CXX_COMPILER -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -fwrapv -O2 -Wall -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -isystem /Users/rgommers/mambaforge/envs/numpy-dev/include -arch arm64 -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -isystem /Users/rgommers/mambaforge/envs/numpy-dev/include -arch arm64 -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -isystem /Users/rgommers/mambaforge/envs/numpy-dev/include -D_FORTIFY_SOURCE=2 -isystem /Users/rgommers/mambaforge/envs/numpy-dev/include -ftrapping-math -DNPY_INTERNAL_BUILD=1 -DHAVE_NPY_CONFIG_H=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE=1 -D_LARGEFILE64_SOURCE=1 -DHAVE_CBLAS -Inumpy/core/src/common -Inumpy/core/src/umath -Inumpy/core/include -Inumpy/core/include/numpy -Ibuild/src.macosx-11.0-arm64-3.9/numpy/distutils/include -Inumpy/core/src/common -Inumpy/core/src -Inumpy/core -Inumpy/core/src/npymath -Inumpy/core/src/multiarray -Inumpy/core/src/umath -Inumpy/core/src/npysort -Inumpy/core/src/_simd -I/Users/rgommers/mambaforge/envs/numpy-dev/include/python3.9 -Inumpy/core/src/common -Inumpy/core/src/npymath -c numpy/core/src/npysort/radixsort.cpp -o build/temp.macosx-11.0-arm64-3.9/numpy/core/src/npysort/radixsort.o -MMD -MF build/temp.macosx-11.0-arm64-3.9/numpy/core/src/npysort/radixsort.o.d -std=c++11 -D__STDC_VERSION__=0 -fno-exceptions -fno-rtti" failed with exit status 2

########### EXT COMPILER OPTIMIZATION ###########
...
<many lines of output left - need to fix later>

So it changes exit status from 127 to 2. We expected 127:

% $CXX compiler; echo $?
zsh: command not found: THIS_IS_NOT_A_CXX_COMPILER
127

Maybe 2 is the result here because we're running this from Python rather than in a terminal directly? It does have a more informative (probably message):

[Errno 2] No such file or directory: 'THIS_IS_NOT_A_CXX_COMPILER'

That message is just hard to miss in the output, because you have to scroll up quite a bit and then there are no blank lines around the output. How about we add a few blank lines above and below raise DistutilsExecError @seiko2plus?

charris
charris reviewed Nov 13, 2021
View reviewed changes
@seiko2plus
Copy link
Copy Markdown
Member Author

seiko2plus commented Nov 13, 2021

@rgommers,

So it changes exit status from 127 to 2. We expected 127:

shell environment returns 127 instead of returning the last system call error number, since it is reserved for the subprocess.

So I thought it would be better to not follow this way in case of the build log is redirected to file instead of stdout to make it easier to trace but after reconsidering I think it's a bad idea since it may make the user mixup between parent and subprocess error numbers.

How about we add a few blank lines above and below raise DistutilsExecError @seiko2plus?

yes, it would be nice

@seiko2plus
BUG, DIST: Print os error message when the executable not exist
6d69886 
  this patch is really important since most of the users aren't able to determine
  the build error when the toolchain or the built environment missing
  executable files of compiler, linker, assembler, etc.
@seiko2plus
Copy link
Copy Markdown
Member Author

seiko2plus commented Nov 13, 2021 •
edited
Loading

errno reverted to 127 and blank lines added above and after the OS error.

export CC=NO_SI_SI
python setup.py build
....
build_src
building py_modules sources
building library "npymath" sources


[Errno 2] No such file or directory: 'NO_SI_SI'


Traceback (most recent call last):
  File "/home/seiko/work/repos/numpy/setup.py", line 450, in <module>
    setup_package()
  File "/home/seiko/work/repos/numpy/setup.py", line 442, in setup_package
    setup(**metadata)
  File "/home/seiko/work/repos/numpy/numpy/distutils

rgommers
rgommers approved these changes Nov 14, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @seiko2plus!

@rgommers rgommers merged commit 4978118 into numpy:main Nov 14, 2021
@rgommers rgommers added this to the 1.22.0 release milestone Nov 14, 2021
@lithomas1 lithomas1 mentioned this pull request Nov 16, 2021
Closed
3 tasks
@charris charris mentioned this pull request Nov 25, 2021
Merged
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Nov 25, 2021
@charris charris removed this from the 1.21.5 release milestone Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charris charris charris left review comments

@rgommers rgommers rgommers approved these changes

@tylerjereddy tylerjereddy Awaiting requested review from tylerjereddy

Assignees

No one assigned

Labels

00 - Bug component: numpy.distutils

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@seiko2plus @rgommers @charris