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.

Sign up for GitHub

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

Jump to bottom

Toolchain arg to cmake to force 19.40 instead of changing system props #644

Merged
merged 3 commits into from
Dec 12, 2024
Merged

Toolchain arg to cmake to force 19.40 instead of changing system props #644

merged 3 commits into from
Dec 12, 2024

Conversation

mastercoms
Copy link
Contributor

I think using the toolchain argument is a much simpler and preferred solution to changing the system configuration for the VC tools. This is only relevant for Windows.

@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Dec 1, 2024 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: mastercoms / name: mcoms (62d9bc3, 651176e)
  • ✅ login: bernie-laberge / name: Bernard Laberge (7b47929)

bernie-laberge
bernie-laberge approved these changes Dec 3, 2024
View reviewed changes
Copy link
Contributor

@bernie-laberge bernie-laberge left a comment

Choose a reason for hiding this comment

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

Nice !
Thank you @mastercoms !

@mastercoms
Copy link
Contributor Author

mastercoms commented Dec 11, 2024 •
edited
Loading

Is this ready for a merge? I can update the documentation as well.

@bernie-laberge
Copy link
Contributor

Is this ready for a merge? I can update the documentation as well.

Hello @mastercoms !
I am currently testing your changes on a brand new Windows instance.
We'll take care of modifying the documentation to make this step only to be used as a last resort:
https://aswf-openrv.readthedocs.io/en/latest/build_system/config_windows.html#method-1-check-default-props-file

I will get back to you shortly with the result of the build on a fresh Windows instance.
Thank you so much for your contribution !

@bernie-laberge
Copy link
Contributor

FYI: This was a complete success.
RV built without errors with the Open RV contibutor’s RV
Using the latest Microsoft Visual Studio Community 17.12.3 and the latest msys.

Thanks to your PR @mastercoms , we’ll be able to make this step optional:
https://aswf-openrv.readthedocs.io/en/latest/build_system/config_windows.html#method-1-check-default-props-file

@bernie-laberge bernie-laberge enabled auto-merge (squash) December 11, 2024 23:04
@bernie-laberge bernie-laberge merged commit 82d3efd into AcademySoftwareFoundation:main Dec 12, 2024
10 checks passed
@mastercoms mastercoms deleted the patch-1 branch December 12, 2024 12:16
eloisebrosseau pushed a commit to eloisebrosseau/OpenRV that referenced this pull request Dec 12, 2024
@mastercoms @bernie-laberge @eloisebrosseau
Toolchain arg to cmake to force 19.40 instead of changing system props (
4033cdd 
AcademySoftwareFoundation#644)

I think using the toolchain argument is a much simpler and preferred
solution to changing the system configuration for the VC tools. This is
only relevant for Windows.

Signed-off-by: mcoms <[email protected]>
Co-authored-by: Bernard Laberge <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bernie-laberge bernie-laberge bernie-laberge approved these changes

@cedrik-fuoco-adsk cedrik-fuoco-adsk cedrik-fuoco-adsk approved these changes

@eloisebrosseau eloisebrosseau eloisebrosseau approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mastercoms @bernie-laberge @eloisebrosseau @cedrik-fuoco-adsk