Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: call compilervars.sh within CMake helper (Intel C++) #6735

Merged
merged 1 commit into from May 20, 2020

Conversation

SSE4
Copy link
Contributor

@SSE4 SSE4 commented Mar 25, 2020

/cc @ohanar

related: #5699

NOTE: I only have a valid Linux license of Intel C++ compiler (they offer only Linux OSS), so Windows & Mac are untested. if someone can test them locally and report here, it would be nice.

heavily based on ohanar@7a62f3a

summary:

  • added few tools helpers for Intel C++ compiler: compilervars_command, compilervars_dict, compiler_vars (context manager), analogous to vcvars_command, vcvars_dict, vcvars
  • added function to locate Intel C++ compiler installation (needed to locate compilervars.sh/compilervars.bat)
  • extracted common part to get environment variables diff: tools.env.env_diff (from vcvars_dict function)
  • extracted tools.win.is_win64 as it repeats
  • CMake helper now calls compilervars.sh/compilervars.bat for certain generators
  • added test to check that

tested with Intel C++ compiler 19.1 (Intel Parallel Studio 2020, Open-Source edition, Linux)

my Conan profile:

[settings]
arch=x86_64
arch_build=x86_64
os=Linux
os_build=Linux
compiler=intel
compiler.version=19.1
compiler.base=gcc
compiler.base.libcxx=libstdc++11
compiler.base.version=7
[options]
[build_requires]
[env]
CC=/opt/intel/bin/icc
PATH=[u'/opt/intel/bin']
CXX=/opt/intel/bin/icc

was able to build several well-known projects from conan-center-index (with test package), such as:

  • zlib
  • boost
  • libxml2
  • libiconv
  • openssl
  • gtest

Changelog: Feature: Call compilervars.sh within CMake helper (Intel C++).
Docs: conan-io/docs#1716

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@SSE4 SSE4 force-pushed the intel_compiler_cmake branch from 602451e to 881733c Compare Mar 26, 2020
Copy link
Contributor

@ohanar ohanar left a comment

I don't have access to MacOS, but I did give things a try on my CentOS 6 and Windows 10 boxes with the following profiles:

[settings]
os=Linux
arch=x86_64
compiler=intel
compiler.version=19
compiler.base=gcc
compiler.base.version=8
compiler.base.libcxx=libstdc++
[options]
[build_requires]
[env]
CC=icc
CXX=icpc
[settings]
os=Windows
arch=x86_64
compiler=intel
compiler.version=19
compiler.base=Visual Studio
compiler.base.version=16
[options]
[build_requires]
[env]
CC=icl
CXX=icl

conans/client/tools/env.py Show resolved Hide resolved
conans/client/tools/intel.py Show resolved Hide resolved
conans/client/tools/intel.py Outdated Show resolved Hide resolved
conans/client/tools/intel.py Outdated Show resolved Hide resolved
conans/client/tools/intel.py Outdated Show resolved Hide resolved
conans/client/tools/env.py Show resolved Hide resolved
conans/client/tools/env.py Show resolved Hide resolved
@SSE4 SSE4 force-pushed the intel_compiler_cmake branch from 881733c to 9c6a64c Compare Mar 27, 2020
conans/client/tools/env.py Outdated Show resolved Hide resolved
conans/client/tools/env.py Show resolved Hide resolved
@SSE4 SSE4 force-pushed the intel_compiler_cmake branch 2 times, most recently from d470965 to dde7605 Compare Mar 28, 2020
ohanar
ohanar approved these changes Mar 30, 2020
Copy link
Contributor

@ohanar ohanar left a comment

Works well for me on both Windows and Linux.

@SSE4
Copy link
Contributor Author

SSE4 commented Apr 6, 2020

added code to handle spaces on Windows.
tested locally (on GTest package), works gracefully, with profile:

[settings]
arch=x86_64
arch_build=x86_64
os=Windows
os_build=Windows
compiler=intel
compiler.version=19.1
compiler.base=Visual Studio
compiler.base.version=16
compiler.base.runtime=MDd
build_type=Debug
[env]
CONAN_CMAKE_GENERATOR=Ninja
[build_requires]
ninja/1.9.0

danimtb
danimtb approved these changes May 6, 2020
@danimtb
Copy link
Member

danimtb commented May 6, 2020

please solve conflicts @SSE4

conans/client/tools/win.py Outdated Show resolved Hide resolved
@SSE4 SSE4 force-pushed the intel_compiler_cmake branch from fdb105f to be94923 Compare May 6, 2020
key_name = r'SOFTWARE\Wow6432Node\Microsoft\VisualStudio\SxS\VC7'
else:
key_name = r'HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\VisualStudio\SxS\VC7'
Copy link
Contributor Author

@SSE4 SSE4 May 6, 2020

Choose a reason for hiding this comment

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

also, there was a bug, but never noticed

@SSE4
Copy link
Contributor Author

SSE4 commented May 6, 2020

@danimtb any idea about?

======================================================================

FAIL: is_there_var_for_settings_previous_version_test (disabled_revisions.conans.test.functional.old.test_migrations.TestMigrations)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/tmp/source/be94/py27/disabled_revisions/conans/test/functional/old/test_migrations.py", line 34, in is_there_var_for_settings_previous_version_test

    "Introduce the previous settings.yml file in the 'migrations_settings.yml")

AssertionError: Introduce the previous settings.yml file in the 'migrations_settings.yml

@SSE4 SSE4 force-pushed the intel_compiler_cmake branch from be94923 to 3b1d72f Compare May 7, 2020
@SSE4
Copy link
Contributor Author

SSE4 commented May 7, 2020

@danimtb ignore the last comment, it appears like GitHub web interface didn't perform the right thing for the merge

danimtb
danimtb approved these changes May 7, 2020
@danimtb danimtb added this to the 1.26 milestone May 7, 2020
Copy link
Member

@memsharded memsharded left a comment

A couple of minor things, but a concern it might be breaking for clang in Windows.

conans/client/build/cmake.py Outdated Show resolved Hide resolved
conans/client/tools/intel.py Show resolved Hide resolved
Signed-off-by: SSE4 <tomskside@gmail.com>
@SSE4 SSE4 force-pushed the intel_compiler_cmake branch from 3b1d72f to 3ef8564 Compare May 20, 2020
Copy link
Member

@memsharded memsharded left a comment

Great, thanks!

@memsharded memsharded merged commit 841eb07 into conan-io:develop May 20, 2020
1 check passed
@memsharded
Copy link
Member

memsharded commented May 20, 2020

Thanks @ohanar for the contributions too.

@SSE4 it would need a Changelog and some docs, please prepare them for the release too. Many thanks!

@ohanar
Copy link
Contributor

ohanar commented Jul 24, 2020

I'm only now getting around to trying to replacing our in-house intel compiler support with these changes. Unfortunately, it seems like a few bugs made their way back in during the rebasing process -- maybe a few commits got missed?

I've made a pair of PRs restoring the correct behavior: #7370 and #7416.

@danimtb
Copy link
Member

danimtb commented Jul 27, 2020

Thanks for trying it out @ohanar! I have done a review and requested @SSE4's as he implemented the intel integration. Let's continue the conversation in those PRs.

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.

None yet

4 participants