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

[Windows] GH Actions #17205

Draft
wants to merge 190 commits into
base: develop2
Choose a base branch
from

Conversation

franramirez688
Copy link
Contributor

@franramirez688 franramirez688 commented Oct 23, 2024

Changelog: omit
Docs: We have to document the part regarding: compiler.update={{ (detect_api.detect_msvc_update(version) | int) % 10 }}

Related to #17094 #17054

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

This is big. Great work.
Just some minor concerns.

@@ -21,15 +21,17 @@ def test_profile_detect_compiler(self):
compiler.version={{detect_api.default_compiler_version(compiler, version)}}
compiler.runtime={{runtime}}
compiler.cppstd={{detect_api.default_cppstd(compiler, version)}}
compiler.update={{detect_api.detect_msvc_update(version)}}
# FIXME: check if we can do something better than this, we have a problem with updates
compiler.update={{ (detect_api.detect_msvc_update(version) | int) % 10 }}
Copy link
Member

Choose a reason for hiding this comment

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

Recall to update the docs with this

@@ -49,11 +49,11 @@ def test_msbuild_lib_2022():
assert "hello/0.1: _MSC_VER191" in client.out

# Create works
client.run("create . -s compiler.version=193")
client.run("create . -s compiler.version=194")
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure why this change.
previously neither 193 nor 194 were the default version, yet the test managed to build and pass. Why is it necessary to do this, maybe because 193 toolset is not installed anymore? The problem with this is that we would not test anymore that we can build compiler.version=193 with a VS>=17.10

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, let me try to install the v143 toolset too and check if we can make it work as it should.

test/integration/toolchains/microsoft/vcvars_test.py Outdated Show resolved Hide resolved
test/unittests/util/output_test.py Outdated Show resolved Hide resolved
@@ -411,6 +411,7 @@ def package_info(self):
assert "CONAN_NAME_OTHER" not in make_content


@pytest.mark.skipif(platform.system() == "Windows", reason="Test uses Unix-style paths not compatible with Windows filesystem.")
Copy link
Member

Choose a reason for hiding this comment

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

This is unexpected, at least some #FIXME (as it is not an xfail), better investigate a bit more?

["/GL"], ["/GL"], ["/LTCG"], ["/LTCG"]),
# ("msvc", "190", "dynamic", "14", "Release", [], [], [], [], []),
# ("msvc", "190", "dynamic", "14", "Release",
# ["TEST_DEFINITION1", "TEST_DEFINITION2=0", "TEST_DEFINITION3=", "TEST_DEFINITION4=TestPpdValue4",
Copy link
Member

Choose a reason for hiding this comment

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

At least a #FIXME, to recover this, even if 190 is not available, at least to test the defines and flags

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.

3 participants