Skip to content

pybind/*/setup.py: fix clang detection with ccache#35336

Merged
tchaikov merged 1 commit intoceph:masterfrom
tchaikov:wip-clang-cache
Jun 1, 2020
Merged

pybind/*/setup.py: fix clang detection with ccache#35336
tchaikov merged 1 commit intoceph:masterfrom
tchaikov:wip-clang-cache

Conversation

@tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Jun 1, 2020

Signed-off-by: Samuel Just sjust@redhat.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

Signed-off-by: Samuel Just <sjust@redhat.com>
@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 1, 2020

otherwise we have

master/build/src/pybind/rados/rados.c -o /home/jenkins-build/build/workspace/ceph-perf-crimson/ceph-master/build/lib/cython_modules/temp.linux-x86_64-3.6/home/jenkins-build/build/workspace/ceph-perf-crimson/ceph-master/build/src/pybind/rados/rados.o -Wno-unused-result -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC -fwrapv
clang-8: error: unknown argument: '-fstack-clash-protection'

@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 1, 2020

Copy link
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

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

Looks good! This bug could have affected GCC builds as well. I'm curious whether there is any significant difference because of parameter omission.

extra_compile_args=filter_unsupported_flags(
compiler.compiler[0],
distutils.sysconfig.get_config_var('CFLAGS').split()),
compiler.compiler[1:] + distutils.sysconfig.get_config_var('CFLAGS').split()),
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -72,7 +72,7 @@ def get_python_flags(libs):
libraries=libs + py_libs,
extra_compile_args=filter_unsupported_flags(
Copy link
Contributor

Choose a reason for hiding this comment

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

ACK: -fstack-clash-protection came from the new_compiler() call.
filter_unsupported_flags() just squeezes some compiler options if it detects clang.

def filter_unsupported_flags(compiler, flags):
    args = takewhile(lambda argv: not argv.startswith('-'), [compiler] + flags)
    if any('clang' in arg for arg in args):
        return list(filterfalse(lambda f:
                                f in ('-mcet',
                                      '-fstack-clash-protection',
                                      '-fno-var-tracking-assignments',
                                      '-Wno-deprecated-register',
                                      '-Wno-gnu-designator') or
                                f.startswith('-fcf-protection'),
                                flags))
    else:
        return flags

@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 1, 2020

@tchaikov tchaikov merged commit ff74736 into ceph:master Jun 1, 2020
@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 1, 2020

@athanatos sorry for stealing your commits and merging it without your blessings. i really want to test and merge ceph/ceph-build#1576 sooner.

@tchaikov tchaikov deleted the wip-clang-cache branch June 1, 2020 14:46
@athanatos
Copy link
Contributor

athanatos commented Jun 1, 2020 via email

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants