feat: Detect MSVC settings in Conan 2.0 way #454

hwhsu1231 · 2022-10-25T11:29:21Z

According to the documentation, msvc will deprecate Visual Studio in Conan 2.X. Therefore, I improve conan_cmake_autodetect() function. And I already tests it locally. The following is the Test Log:

F:\GitRepo\cmake-conan>pytest tests.py
================================= test session starts =================================
platform win32 -- Python 3.10.7, pytest-7.1.3, pluggy-1.0.0
rootdir: F:\GitRepo\cmake-conan
collected 37 items

tests.py .................s...................                                   [100%]

====================== 36 passed, 1 skipped in 309.32s (0:05:09) ======================

hwhsu1231 · 2022-10-26T10:41:16Z

The only remained settings that _conan_detect_compiler() haven't detected for msvc compiler is compiler.update. Still thinking about how to detect it. Any idea?

But, it seems fine without specifying compiler.update for now.

hwhsu1231 · 2022-10-29T06:30:49Z

According to this issue from Kitware/CMake, it is recommended to use CMAKE_<LANG>_COMPILER_ARCHITECTURE_ID instead of MSVC_[C|CXX]_ARCHITECTURE_ID.

hwhsu1231 · 2022-10-31T07:48:40Z

I tried to follow the Conventional Commits specification. So I modified some commit messages.

hwhsu1231 · 2022-11-01T03:13:20Z

@czoido

How is this PR going? I hope this PR can be merged to develop branch as soon as possible since the conan_cmake_detect_msvc_runtime() function is required for detecting MSVC-based Clang compilers as well.

compiler.runtime
compiler.runtime_type

hwhsu1231 · 2022-11-01T14:46:20Z

@czoido

I suggest that we rename those internal functions which are used in _conan_detect_compiler macro with conan_detect_ prefix. That is:

conan_detect_unix_libcxx (from conan_cmake_detect_unix_libcxx)
conan_detect_msvc_version (from _get_msvc_ide_version)
conan_detect_msvc_runtime (from conan_cmake_detect_vs_runtime)

Later when implementing an internal function to detect compiler.runtime_version setting of Clang compilers, I will name it:

conan_detect_clang_runtime_version

What do you think?

hwhsu1231 · 2022-11-04T10:01:29Z

This PR should fix: #421

hwhsu1231 · 2022-11-12T16:28:55Z

@czoido

When will this PR be merged? What else do I need to improve?

Should I need to rename the internal functions myself, or will Conan Team do so?

conan_detect_unix_libcxx (from conan_cmake_detect_unix_libcxx)

conan_detect_msvc_version (from _get_msvc_ide_version)

conan_detect_msvc_runtime (from conan_cmake_detect_vs_runtime)

hwhsu1231 · 2022-11-15T03:25:48Z

I've rebased this branch from the latest develop branch.

hwhsu1231 · 2022-11-15T08:03:18Z

@czoido

Should I squash those commits into about 1~3 commits?

czoido · 2022-11-15T09:13:12Z

@czoido

Should I squash those commits into about 1~3 commits?

Hi @hwhsu1231,

No need to do anything on your side, we merge the PR's with squash.

Regarding the PR, we can't merge this as it is, because it will detect msvc instead of Visual Studio and will break users that use generators that don't support msvc. Maybe we can add an extra argument to conan_cmake_autodetect and make this opt-in. Something like:

conan_cmake_autodetect(CONAN_V2 ON)

hwhsu1231 · 2022-11-15T17:35:56Z

@czoido

I think maybe it is time for using conan_version() to differentiate the implementation between Conan 1.0 and Conan 2.0 since Visual Studio will be deprecated in Conan 2.0 but still required in Conan 1.0 for now. For example, using conan_version() in _conan_detect_compiler() macro like the following. What do you think?

elseif ("${CMAKE_${LANGUAGE}_COMPILER_ID}" STREQUAL "MSVC"
        OR ("${CMAKE_${LANGUAGE}_COMPILER_ID}" STREQUAL "Clang" 
            AND "${CMAKE_${LANGUAGE}_COMPILER_FRONTEND_VARIANT}" STREQUAL "MSVC" 
            AND "${CMAKE_${LANGUAGE}_SIMULATE_ID}" STREQUAL "MSVC"))

    conan_version(CONAN_VERSION)
    if (${CONAN_VERSION} VERSION_LESS "2.0.0" AND NOT CONAN_V2)
        # Conan 1.0 without CONAN_V2 enabled. 
        # using 'Visual Studio' compiler settings.
    else ()
        # Conan 2.0 or Conan 1.0 with CONAN_V2 enabled.
        # using 'msvc' compiler settings.
    endif ()
endif ()

If so, I think we should add back the following internal functions for using Visual Studio compiler settings:

_get_msvc_ide_version
conan_cmake_detect_vs_runtime

czoido · 2022-11-16T10:20:45Z

Hi @hwhsu1231,
The problem I see with that is that users won't be able to test with msvc using Conan 1.X, I think selecting the behaviour of the function between old and new would probably is a better option.

hwhsu1231 · 2022-11-16T12:31:03Z

@czoido

How about the following implementations?

In conan_cmake_autodetect():

Click to expand

function(conan_cmake_autodetect detected_settings)
    # Detect whether CONAN_V2 is specified with ON:
    # - If CONAN_V2 is specified with ON, then MODE_CONAN_V2 will be ON.
    # - Otherwise:
    #   - If current Conan version is 1.0, then set MODE_CONAN_V2 to OFF.
    #   - If current Conan version is 2.0, then set MODE_CONAN_V2 TO ON.
    set(autodetectOneValueArgs CONAN_V2)
    cmake_parse_arguments(MODE "" "${autodetectOneValueArgs}" "" ${ARGV})
    if (NOT MODE_CONAN_V2)
        conan_version(CONAN_VERSION)
        if (CONAN_VERSION VERSION_LESS "2.0.0")
            set(MODE_CONAN_V2 OFF)
        else ()
            set(MODE_CONAN_V2 ON)
        endif ()
    endif ()
    _conan_detect_build_type(${ARGV})
    _conan_check_system_name()
    _conan_check_language()
    _conan_detect_compiler(${ARGV})
    _collect_settings(collected_settings)
    set(${detected_settings} ${collected_settings} PARENT_SCOPE)
endfunction()

In MSVC condition statement of _conan_detect_compiler():

Click to expand

elseif ("${CMAKE_${LANGUAGE}_COMPILER_ID}" STREQUAL "MSVC"
        OR ("${CMAKE_${LANGUAGE}_COMPILER_ID}" STREQUAL "Clang" 
            AND "${CMAKE_${LANGUAGE}_COMPILER_FRONTEND_VARIANT}" STREQUAL "MSVC" 
            AND "${CMAKE_${LANGUAGE}_SIMULATE_ID}" STREQUAL "MSVC"))
    
    if (NOT MODE_CONAN_V2)
        ###########################################
        # Detect 'Visual Studio' compiler settings.
        ###########################################
    else ()
        ###########################################
        # Detect 'msvc' compiler settings.
        ###########################################
    endif ()
endif ()

Therefore, users can acquire:

Visual Studio compiler settings by default when using Conan 1.0.
msvc compiler settings with CONAN_V2 ON when using Conan 1.0.
msvc compiler settings by default when using Conan 2.0.

hwhsu1231 · 2022-11-16T17:45:48Z

@czoido

I made the above-mentioned implementations. How about now? Is it able to be merged?

hwhsu1231 · 2022-11-17T15:35:01Z

@czoido

I've rebased this branch from the latest develop branch.

hwhsu1231 · 2022-11-17T16:01:45Z

@czoido

About renaming internal functions used inside _conan_detect_compiler macro, I was thinking about removing cmake string and adding _ prefix from them. That is:

conan_cmake_detect_vs_version -> _conan_detect_vs_version
conan_cmake_detect_vs_runtime -> _conan_detect_vs_runtime
conan_cmake_detect_msvc_version -> _conan_detect_msvc_version
conan_cmake_detect_msvc_runtime -> _conan_detect_msvc_runtime
conan_cmake_detect_unix_libcxx -> _conan_detect_unix_libcxx

What do you think?

hwhsu1231 · 2022-11-24T07:17:44Z

@czoido

I've rebased this branch from the latest develop branch.

hwhsu1231 · 2022-12-03T02:11:35Z

@czoido - Execuse me. What else problems does this PR have?

hwhsu1231 · 2022-12-12T14:43:28Z

@czoido @memsharded

It's about one and a half month since I submit this PR.
I want to keep improving this repo.
Hope Conan Team can merge this PR as soon as possible.

Thanks.

prince-chrismc · 2022-12-12T16:30:52Z

As per the readme, this project is a secondary to the main client and I am sure after the 2.0 release there will be more attention to these great contributions.

Thanks for understanding ❤️

hwhsu1231 · 2022-12-18T18:01:48Z

@prince-chrismc

I understand that Conan Team is paying more attention on 2.0 release. However, since this PR is passed with the current CI/CD testing, I wonder what else does this PR need to be improved? If not, why not merge this PR? If some errors do happen thereafter, then we just submit another PR to fix it. Right?

memsharded · 2022-12-18T21:15:49Z

Hi @hwhsu1231

I understand that Conan Team is paying more attention on 2.0 release. However, since this PR is passed with the current CI/CD testing, I wonder what else does this PR need to be improved? If not, why not merge this PR?

Thanks very much for your contribution, but this is not how it works, not every green PR is merged, not even our own PRs, many of them also stay there long time, getting reviews, discussed, or just stuck until it becomes a higher priority. Things need to be reviewed, to see if they align with the design, with the maintainers view on the tool, consider possible side effects, future maintenance, etc, etc. Also this cmake-conan is not that thoroughly tested, so it better not rely only on CI but on reviews too. It is also true that large PRs also take more time to review.

If some errors do happen thereafter, then we just submit another PR to fix it. Right?

Some things might have a high risk of breaking, yes, things can be fixed later, but if possible it is better not to break in the first place.

* Add '_conan_detect_arch' macro. * Apply '_conan_detect_arch' in 'conan_cmake_settings'. * Apply '_conan_detect_arch' in 'conan_cmake_autodetect'. * Remove detecting 'arch' mechanism in detecting MSVC compiler.

hwhsu1231 · 2022-12-19T05:28:24Z

I pushed a new commit, which is aim to fix: #457

hwhsu1231 · 2022-12-26T05:21:36Z

I summarized the commits in this PR so far, so that Conan Team can squash them into a single commit if it's accepted.

feat: Detect MSVC settings in Conan 2.0 way (#454)

In conan.cmake:

* Rename internal functions/macros:

  * conan_cmake_detect_unix_libcxx() -> _conan_detect_unix_libcxx()
  * _get_msvc_ide_version()          -> _conan_detect_vs_version()
  * conan_cmake_detect_vs_runtime()  -> _conan_detect_vs_runtime()

* Add new internal functions/macros:

  * Add _conan_detect_msvc_runtime() function
  * Add _conan_detect_msvc_version() function
  * Add _conan_detect_arch() macro

* Modify conan_cmake_autodetect():

  * Apply _conan_detect_arch()
  * Remove detecting arch for MSVC compiler
  * Add optional CONAN_V2 arguments

* Apply _conan_detect_arch() in conan_cmake_settings()
* Add compiler.update and compiler.runtime_type in _collect_settings()

In tests.py:

* Add compiler. prefix in test_settings()
* Add = after {} in test_settings_removed_from_autodetect()

Closes: #421, #457

hwhsu1231 · 2023-02-06T07:10:27Z

@czoido @prince-chrismc @memsharded

Dear Conan Team,

How is the progress of this PR? It's almost about 3 months since this PR was labled PR: Under review. If there are some other situations required to consider in this PR, please let me know. Maybe I can handle them myself. I just hope this project can be improved.

Sincerely.

prince-chrismc · 2023-02-07T16:02:14Z

As you are aware, this auxiliary project is on hold until after Conan 2.0 is out. I am sure your contributions will be reviewed in the near future :)

hwhsu1231 · 2023-03-15T12:10:52Z

@memsharded @jcar87

Excuse me.

I saw that a new branch develop2 was created. Does that mean this Change/PR should be merged to that new branch?

If so, should I clsoe this PR and re-submit a new one to the develop2 branch?

memsharded · 2023-03-15T12:50:19Z

If so, should I clsoe this PR and re-submit a new one to the develop2 branch?

No, the develop2 is the one used for the experimentation of the new approaches for 2.0 for the team, please do not send PRs to it.

hwhsu1231 · 2023-03-17T13:22:09Z

@memsharded

Will Conan Team keep maintaining the develop branch?

CLAassistant · 2023-10-04T17:52:59Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

hwhsu1231 force-pushed the detect-msvc-settings-in-conan2 branch 2 times, most recently from 7903b94 to b4bffdf Compare October 31, 2022 07:43

hwhsu1231 changed the title ~~Detect MSVC settings in Conan 2.X way.~~ Detect MSVC settings in Conan 2.X way Oct 31, 2022

hwhsu1231 mentioned this pull request Nov 13, 2022

[bug] v0.18.0+ does not recognize Clang with GNU-like command-line on Windows #459

Open

hwhsu1231 force-pushed the detect-msvc-settings-in-conan2 branch from f274d04 to 4e512b4 Compare November 15, 2022 03:10

czoido added the PR: Under review label Nov 15, 2022

hwhsu1231 force-pushed the detect-msvc-settings-in-conan2 branch from 34a0d44 to c0634f4 Compare November 17, 2022 15:22

hwhsu1231 force-pushed the detect-msvc-settings-in-conan2 branch from 5d95329 to cbc0c8c Compare November 24, 2022 06:19

hwhsu1231 changed the title ~~Detect MSVC settings in Conan 2.X way~~ feat: Detect MSVC settings in Conan 2.0 way Dec 12, 2022

hwhsu1231 added 14 commits December 19, 2022 02:12

feat: Add back conan_cmake_detect_vs_version

2d9fb66

feat: Add back conan_cmake_detect_vs_runtime

0218e2f

chore: Add comments for 'msvc' compiler.version

dde1eda

chore: Move up conan_cmake_detect_msvc_runtime

341e8f4

chore: Modify some comments

2f53c9f

chore: Add comment for _conan_detect_build_type

7f7d4fb

chore: Add comment for _conan_check_system_name

596265a

chore: Adjust condition statement indents of MSVC

db8f8ae

feat: Add back old VS compiler detection

9f7446f

test: Fix test_settings_removed_from_autodetect

32321d9

feat: Add CONAN_V2 detecting mechanism

3115467

chore: Move up conan_cmake_detect_unix_libcxx

8e7ec0a

chore: Add comments for detecting compiler.libcxx

4718944

chore: Rename to _conan_detect_xxx

c144477

hwhsu1231 force-pushed the detect-msvc-settings-in-conan2 branch from 2ea6db2 to c144477 Compare December 18, 2022 19:04

feat: Add _conan_detect_arch macro

b6b59c6

* Add '_conan_detect_arch' macro. * Apply '_conan_detect_arch' in 'conan_cmake_settings'. * Apply '_conan_detect_arch' in 'conan_cmake_autodetect'. * Remove detecting 'arch' mechanism in detecting MSVC compiler.

aminya mentioned this pull request Jan 2, 2023

Update Conan.cmake to 0.18.1 aminya/project_options#184

Open

prince-chrismc removed the PR: Under review label Feb 7, 2023

hwhsu1231 mentioned this pull request Mar 7, 2023

[question] Why is this project on hold until after the 2.0 release? #447

Closed

hwhsu1231 mentioned this pull request Mar 16, 2023

Discussion of cmake-conan evolving towards Dependency Provider. #475

Open

aminya mentioned this pull request Jul 21, 2023

Support Conan 2 aminya/project_options#212

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Detect MSVC settings in Conan 2.0 way #454

feat: Detect MSVC settings in Conan 2.0 way #454

hwhsu1231 commented Oct 25, 2022

hwhsu1231 commented Oct 26, 2022

hwhsu1231 commented Oct 29, 2022

hwhsu1231 commented Oct 31, 2022 •

edited

Loading

hwhsu1231 commented Nov 1, 2022

hwhsu1231 commented Nov 1, 2022

hwhsu1231 commented Nov 4, 2022

hwhsu1231 commented Nov 12, 2022

hwhsu1231 commented Nov 15, 2022

hwhsu1231 commented Nov 15, 2022

czoido commented Nov 15, 2022

hwhsu1231 commented Nov 15, 2022 •

edited

Loading

czoido commented Nov 16, 2022

hwhsu1231 commented Nov 16, 2022 •

edited

Loading

hwhsu1231 commented Nov 16, 2022

hwhsu1231 commented Nov 17, 2022

hwhsu1231 commented Nov 17, 2022 •

edited

Loading

hwhsu1231 commented Nov 24, 2022

hwhsu1231 commented Dec 3, 2022

hwhsu1231 commented Dec 12, 2022

prince-chrismc commented Dec 12, 2022

hwhsu1231 commented Dec 18, 2022 •

edited

Loading

memsharded commented Dec 18, 2022

hwhsu1231 commented Dec 19, 2022

hwhsu1231 commented Dec 26, 2022 •

edited

Loading

hwhsu1231 commented Feb 6, 2023

prince-chrismc commented Feb 7, 2023

hwhsu1231 commented Mar 15, 2023 •

edited

Loading

memsharded commented Mar 15, 2023

hwhsu1231 commented Mar 17, 2023

CLAassistant commented Oct 4, 2023

feat: Detect MSVC settings in Conan 2.0 way #454

Are you sure you want to change the base?

feat: Detect MSVC settings in Conan 2.0 way #454

Conversation

hwhsu1231 commented Oct 25, 2022

hwhsu1231 commented Oct 26, 2022

hwhsu1231 commented Oct 29, 2022

hwhsu1231 commented Oct 31, 2022 • edited Loading

hwhsu1231 commented Nov 1, 2022

hwhsu1231 commented Nov 1, 2022

hwhsu1231 commented Nov 4, 2022

hwhsu1231 commented Nov 12, 2022

hwhsu1231 commented Nov 15, 2022

hwhsu1231 commented Nov 15, 2022

czoido commented Nov 15, 2022

hwhsu1231 commented Nov 15, 2022 • edited Loading

czoido commented Nov 16, 2022

hwhsu1231 commented Nov 16, 2022 • edited Loading

hwhsu1231 commented Nov 16, 2022

hwhsu1231 commented Nov 17, 2022

hwhsu1231 commented Nov 17, 2022 • edited Loading

hwhsu1231 commented Nov 24, 2022

hwhsu1231 commented Dec 3, 2022

hwhsu1231 commented Dec 12, 2022

prince-chrismc commented Dec 12, 2022

hwhsu1231 commented Dec 18, 2022 • edited Loading

memsharded commented Dec 18, 2022

hwhsu1231 commented Dec 19, 2022

hwhsu1231 commented Dec 26, 2022 • edited Loading

hwhsu1231 commented Feb 6, 2023

prince-chrismc commented Feb 7, 2023

hwhsu1231 commented Mar 15, 2023 • edited Loading

memsharded commented Mar 15, 2023

hwhsu1231 commented Mar 17, 2023

CLAassistant commented Oct 4, 2023

hwhsu1231 commented Oct 31, 2022 •

edited

Loading

hwhsu1231 commented Nov 15, 2022 •

edited

Loading

hwhsu1231 commented Nov 16, 2022 •

edited

Loading

hwhsu1231 commented Nov 17, 2022 •

edited

Loading

hwhsu1231 commented Dec 18, 2022 •

edited

Loading

hwhsu1231 commented Dec 26, 2022 •

edited

Loading

hwhsu1231 commented Mar 15, 2023 •

edited

Loading