[proxy] github.com← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public

Conversation

Copy link
Contributor

aixtools commented Aug 8, 2018

the many year old issue was: test_search_cpp error on AIX (with xlc)

This patch fixes all (6) current test_distutils errors, when using xlc (aka not using gcc for AIX)

https://bugs.python.org/issue11191

aixtools changed the title bpo-11191: fix test_distutils for AIX bpo-11191: fix test_distutils for AIX with xlc Aug 8, 2018
Copy link
Contributor

embray commented Oct 3, 2018

Something I'm a little confused by about this issue: It seems like it's fixing assumptions made for gcc that don't hold for the xlc compiler. But is that a question of xlc for AIX, or xlc in general?

Copy link
Contributor Author

aixtools commented Oct 3, 2018

I have no access to XLC for Linux. However, as a gut-feeling - IBM SW developers tend to be very consistent, for better or worse, when it comes to cross-platform UI issues. "Support" really dislikes options that work differently on different platforms.

And, please note: there is only one "difference". The flag -E - to create .i files does not take a -o flag.

The change to " elif not cmd or cmd is None:" in init.py is fixes several tests.

Perhaps it will help review to have this broken into two PR that can be reviewed separately. If so, I would prefer to attach them both to this issue number.

Copy link
Contributor Author

aixtools commented Oct 3, 2018

More detail: This PR corrects 7 "errors", six are corrected by the change in init.py.

The more bulky change fixes the original issue re: search_cpp - which uses the combined gcc flags of -E and -o. XLC fails because it does not support the combination.

Your consideration is appreciated!

Details: 6 + 1 errors:

======================================================================
ERROR: test_run (distutils.tests.test_build_clib.BuildCLibTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/tests/test_build_clib.py", line 121, in test_run
    ccmd = missing_compiler_executable()
  File "/data/prj/python/git/python3-3.8/Lib/test/support/__init__.py", line 2758, in missing_compiler_executable
    if spawn.find_executable(cmd[0]) is None:
IndexError: list index out of range

======================================================================
ERROR: test_build_ext (distutils.tests.test_build_ext.BuildExtTestCase)

======================================================================
ERROR: test_get_outputs (distutils.tests.test_build_ext.BuildExtTestCase)

======================================================================
ERROR: test_build_ext (distutils.tests.test_build_ext.ParallelBuildExtTestCase)

======================================================================
ERROR: test_get_outputs (distutils.tests.test_build_ext.ParallelBuildExtTestCase)

======================================================================
ERROR: test_record_extensions (distutils.tests.test_install.InstallTestCase)

±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±±
======================================================================
ERROR: test_search_cpp (distutils.tests.test_config_cmd.ConfigTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/unixccompiler.py", line 107, in preprocess
    self.spawn(pp_args)
  File "/data/prj/python/git/python3-3.8/Lib/distutils/ccompiler.py", line 909, in spawn
    spawn(cmd, dry_run=self.dry_run)
  File "/data/prj/python/git/python3-3.8/Lib/distutils/spawn.py", line 36, in spawn
    _spawn_posix(cmd, search_path, dry_run=dry_run)
  File "/data/prj/python/git/python3-3.8/Lib/distutils/spawn.py", line 157, in _spawn_posix
    raise DistutilsExecError(
distutils.errors.DistutilsExecError: command 'xlc_r' failed with exit status 40

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/distutils/tests/test_config_cmd.py", line 49, in test_search_cpp
    match = cmd.search_cpp(pattern='xxx', body='/* xxx */')
  File "/data/prj/python/git/python3-3.8/Lib/distutils/command/config.py", line 201, in search_cpp
    src, out = self._preprocess(body, headers, include_dirs, lang)
  File "/data/prj/python/git/python3-3.8/Lib/distutils/command/config.py", line 124, in _preprocess
    self.compiler.preprocess(src, out, include_dirs=include_dirs)
  File "/data/prj/python/git/python3-3.8/Lib/distutils/unixccompiler.py", line 109, in preprocess
    raise CompileError(msg)
distutils.errors.CompileError: command 'xlc_r' failed with exit status 40

FAILED (errors=7, skipped=29)
test test_distutils failed


Copy link
Contributor

embray commented Oct 4, 2018

@aixtools

I have no access to XLC for Linux. However, as a gut-feeling - IBM SW developers tend to be very consistent, for better or worse, when it comes to cross-platform UI issues. "Support" really dislikes options that work differently on different platforms.

I suspect you are right, but it would be good to check. I do not have it either, but apparently IBM supplies a free trial version for Linux, so I might give that a try.

The reason I ask is that while it's true AIX is far and away the most likely place to encounter that compiler, the sys.platform checks make me uneasy, when instead (as in most cases) it would be better to be doing feature checks. It's also of course possible to run GCC on AIX. It looks like in your patch you account for that, at least in most cases. Perhaps, just as there is a self._is_gcc there should be a self._is_xlc if possible (perhaps it can even call the compiler and check the --version output?)

Of course there are cases where checking the platform to determine what flags to use does make sense, especially when it comes to linking. But for something like "xlc does not support -E -o <filename>" it might be better to check explicitly for the compiler in question, not the platform it's used on.

Copy link
Contributor

embray commented Oct 4, 2018

I can see about getting that trial version of XL C/C++ for Linux if it helps.

Copy link
Contributor Author

aixtools commented Oct 4, 2018

@aixtools

I have no access to XLC for Linux. However, as a gut-feeling - IBM SW developers tend to be very consistent, for better or worse, when it comes to cross-platform UI issues. "Support" really dislikes options that work differently on different platforms.

I suspect you are right, but it would be good to check. I do not have it either, but apparently IBM supplies a free trial version for Linux, so I might give that a try.

The reason I ask is that while it's true AIX is far and away the most likely place to encounter that compiler, the sys.platform checks make me uneasy, when instead (as in most cases) it would be better to be doing feature checks. It's also of course possible to run GCC on AIX. It looks like in your patch you account for that, at least in most cases. Perhaps, just as there is a self._is_gcc there should be a self._is_xlc if possible (perhaps it can even call the compiler and check the --version output?)

Of course there are cases where checking the platform to determine what flags to use does make sense, especially when it comes to linking. But for something like "xlc does not support -E -o <filename>" it might be better to check explicitly for the compiler in question, not the platform it's used on.

Perhaps, just as there is a self._is_gcc there should be a self._is_xlc

Maybe I can use that instead, rather than the additional method - exernalized (self.compiler that only calls self._compiler()) to get the compiler name.

This took forever to figure out.

The generic bottom-line is: it is known to work if self._isgcc is true, and unknown if it is not (versus known to not work if AIX and not self._isgcc).

Maybe it is more maintainable to respond as some tests do with "skip" when ((not "known to work") is True)

And, actually, just the documentation of xlc for Linux should answer the question. I'll look for the comment in AIX documentation (I hope it is there), and see if I can find the same in the Linux XLC documentation.

Copy link
Contributor Author

aixtools commented Oct 4, 2018

I can see about getting that trial version of XL C/C++ for Linux if it helps.

So, from the documentation:

AIX documentation - xlc compiler reference (v12) (PDF)

  • Unless -C is specified, comments are replaced in the preprocessed output by a
    single space character. New lines and #line directives are issued for comments that
    span multiple source lines.
  • The -E option overrides the -P, -o, and -qsyntaxonly options.

For Linux on Power version: https://www.ibm.com/support/knowledgecenter/SSXVZZ_16.1.0/com.ibm.xlcpp161.lelinux.doc/compiler_ref/opt_e_upper.html

Usage

Source files with unrecognized file name suffixes are treated and preprocessed as C files.

Unless -C is specified, comments are replaced in the preprocessed output by a single space character. New lines and #line directives are issued for comments that span multiple source lines.

The -E option overrides the -P and -fsyntax-only (-qsyntaxonly) options. The combination of -E -o stores the preprocessed result in the file specified by -o.

Answer seems to be same behavior for both versions.

Copy link
Contributor Author

aixtools commented Oct 5, 2018 via email

Copy link
Contributor Author

aixtools commented Oct 5, 2018 via email

Copy link
Contributor

ncoghlan commented Dec 26, 2018

I've updated the NEWS entry in the Test section to reflect just the bpo-11191 fix.

However, there is an additional change still in this PR that affects the actual distutils library behaviour:

Add the additional argument '-C' for xlc so comments are included in preprocessor output

I'm not clear how that relates to either the original issue in bpo-11191, or the issue that was split out as bpo-34897 (and is now covered by PR #9706)

Copy link
Contributor Author

aixtools commented Dec 26, 2018

Originally I had all the "things" I found while researching bpo-11191 in a single PR. Over time I felt it may be more understandable - and I hoped easier to get accepted - a PR that focused, as much as possible - on the original "ancient" issue - and moved the "newly discovered" but not yet in an issue - to a new issue and PR.

During my initial analysis of bpo-11191 I noticed that the comments were not being included, at all - so even before I had learned that the output could not be re-directed to a file I had to correct that comments would even be included in any output. My opinion is that is was also a an element of the cause, just not the complete solution.

Hope this helps.

If you prefer a new issue - I'll be happy to provide both a new issue and PR.

And thanks for updating the NEWS content!

Copy link
Contributor Author

Argh - had not noticed you had already removed the check_compiler function - and in tying to remove the file Lib/distutils/command/config.py from the PR reverted the changes you had added. So, back now to the "line deletion" on line 333. Sigh - and Happy New Year.

p.s. did remove the change that added the -C argument needed for when the test does become possible with a later (version 15 or 16?) of the xlc compiler.

ncoghlan merged commit ed57e13 into python:master Dec 28, 2018
aixtools deleted the bpo-11191 branch December 28, 2018 15:42
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.

6 participants