msvc: Allow building with Visual Studio Preview versions #1711

sankhesh · 2023-09-27T17:11:49Z

This change allows compiling ycmd against latest Visual Studio Preview versions.

Tested against Visual Studio 2022 Community version 17.8.0 Preview 2.0.

This change is

bstaletic · 2023-09-27T19:36:57Z

Thanks for the pull request.
While I am not at all against this, I would like to know what -prerelease does.
Apologies, but I do not have a Windows machine handy.

puremourning

Reviewable status: 0 of 2 LGTMs obtained (waiting on @sankhesh)

build.py line 129 at r1 (raw file):

      print( "Calling vswhere -latest -installationVersion" )
    latest_full_v = subprocess.check_output(
      [ VSWHERE_EXE, '-latest', '-prerelease', '-property', 'installationVersion' ]

Please can you explain canonically what this does? We don’t want to always use prerelease versions. In fact I’d rather not support them at all as we will never test with them. I’m happy to make it work by chance or by opt-in, but not by default.

sankhesh · 2023-09-27T20:35:27Z

With -prerelease, vswhere also returns any prereleases/preview versions along with the release versions.
See doc: https://github.com/microsoft/vswhere/blob/f791b32483204ad3b0ff3cdce57a0c85ee119a8f/src/vswhere.lib/vswhere.lib.rc#L80

The current code defaults to the latest without user opt-in/choice if there are multiple versions. This change just extends it further to allow latest Preview versions as well. Note that without this change, if I just have VS Preview version installed (with valid MSVC compiler), ycmd build fails with the error that MSVC could not be found.

puremourning · 2023-09-27T20:45:07Z

With -prerelease, vswhere also returns any prereleases/preview versions along with the release versions.

See doc: https://github.com/microsoft/vswhere/blob/f791b32483204ad3b0ff3cdce57a0c85ee119a8f/src/vswhere.lib/vswhere.lib.rc#L80

The current code defaults to the latest without user opt-in/choice if there are multiple versions. This change just extends it further to allow latest Preview versions as well. Note that without this change, if I just have VS Preview version installed (with valid MSVC compiler), ycmd build fails with the error that MSVC could not be found.

If I have both a released and pre-release version, which is picked?

puremourning

Reviewable status: 0 of 2 LGTMs obtained (waiting on @sankhesh)

a discussion (no related file):
Flake8 error means ci is broken. Line too long.

sankhesh · 2023-10-03T13:22:15Z

If I have both a released and pre-release version, which is picked?

I am not an expert but IMHO, because of the -latest flag, the later of the two versions will be picked i.e. between VS2019 Preview and VS 2022, the latter will be chosen.

Flake8 error means ci is broken. Line too long.

Thanks, I'll fix that.

bstaletic

I am in favour of this. MS takes ages between two non-preview releases and I know that some people do use latest preview only. Plus MS likes to write blog posts about how latest preview versions are the bestest.

@sankhesh Would you mind using reviewable for the review. It helps keep discussion threads in order and has some nice features.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @sankhesh)

sankhesh

Sure 👍

Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @bstaletic and @puremourning)

a discussion (no related file):

Previously, puremourning (Ben Jackson) wrote…

Flake8 error means ci is broken. Line too long.

Done.

build.py line 129 at r1 (raw file):

Previously, puremourning (Ben Jackson) wrote…

Please can you explain canonically what this does? We don’t want to always use prerelease versions. In fact I’d rather not support them at all as we will never test with them. I’m happy to make it work by chance or by opt-in, but not by default.

Done

sankhesh · 2023-10-06T20:32:55Z

Hmm.. I don't know why that CI is failing.

bstaletic · 2023-10-06T21:14:24Z

It happens. Windows jobs are a little flakey.

codecov · 2023-10-06T21:58:44Z

Codecov Report

Merging #1711 (aba1b1d) into master (e755af6) will decrease coverage by 0.01%.
Report is 16 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1711      +/-   ##
==========================================
- Coverage   95.42%   95.41%   -0.01%     
==========================================
  Files          83       83              
  Lines        8123     8131       +8     
  Branches      164      165       +1     
==========================================
+ Hits         7751     7758       +7     
  Misses        322      322              
- Partials       50       51       +1

sankhesh · 2023-10-13T15:57:17Z

@bstaletic @puremourning Good to merge?

bstaletic · 2023-10-13T16:49:19Z

I am ok with this pull request, but @puremourning said he would prefer this to be opt-in.

I’m happy to make it work by chance or by opt-in, but not by default.

So could you add an option like --preview-msvc (or smothing, naming is hard) to the argument parser in build.py?

This flag allows compiling against the latest MSVC preview version. By default, this is disabled; meaning only the latest stable MSVC release is used.

sankhesh · 2023-12-01T20:33:27Z

@bstaletic @puremourning The latest commit adds the --preview-msvc option.

puremourning

thanks!

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (and 1 stale) (waiting on @sankhesh)

sankhesh

Reviewable status: complete! 2 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)

sankhesh

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! 2 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)

mergify · 2023-12-22T17:08:50Z

Thanks for sending a PR!

mergify · 2023-12-22T17:08:51Z

Thanks for sending a PR!

sankhesh · 2023-12-22T18:07:07Z

@bstaletic @puremourning Anything else needed here?

puremourning · 2023-12-22T18:08:37Z

@bstaletic @puremourning Anything else needed here?

Well, CI is failing, so we will need to resolve that before merging.

mergify · 2023-12-22T18:38:25Z

Thanks for sending a PR!

puremourning requested changes Sep 27, 2023

View reviewed changes

sankhesh force-pushed the msvc_preview branch from 9c53173 to f086c3f Compare October 3, 2023 13:26

bstaletic approved these changes Oct 3, 2023

View reviewed changes

msvc: Allow building with Visual Studio Preview versions

0c109c4

sankhesh force-pushed the msvc_preview branch from f086c3f to 0c109c4 Compare October 3, 2023 19:33

sankhesh commented Oct 3, 2023

View reviewed changes

build: Add a --preview-msvc flag

aba1b1d

This flag allows compiling against the latest MSVC preview version. By default, this is disabled; meaning only the latest stable MSVC release is used.

sankhesh force-pushed the msvc_preview branch from 1c991f8 to aba1b1d Compare December 1, 2023 20:33

puremourning approved these changes Dec 12, 2023

View reviewed changes

puremourning added the Ship It! Manual override to merge a PR by maintainer label Dec 12, 2023

sankhesh commented Dec 12, 2023

View reviewed changes

Merge branch 'master' into msvc_preview

c7b4485

mergify bot merged commit f4cfdb8 into ycm-core:master Dec 22, 2023
14 of 16 checks passed

sankhesh deleted the msvc_preview branch December 22, 2023 19:12

bstaletic mentioned this pull request Dec 28, 2023

Add instructions for use with .NET 6.0 ycm-core/YouCompleteMe#4205

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

msvc: Allow building with Visual Studio Preview versions #1711

msvc: Allow building with Visual Studio Preview versions #1711

sankhesh commented Sep 27, 2023 •

edited

Loading

bstaletic commented Sep 27, 2023

puremourning left a comment

sankhesh commented Sep 27, 2023

puremourning commented Sep 27, 2023

puremourning left a comment

sankhesh commented Oct 3, 2023

bstaletic left a comment

sankhesh left a comment

sankhesh commented Oct 6, 2023

bstaletic commented Oct 6, 2023

codecov bot commented Oct 6, 2023 •

edited

Loading

sankhesh commented Oct 13, 2023

bstaletic commented Oct 13, 2023

sankhesh commented Dec 1, 2023

puremourning left a comment

sankhesh left a comment

sankhesh left a comment

mergify bot commented Dec 22, 2023

mergify bot commented Dec 22, 2023

sankhesh commented Dec 22, 2023

puremourning commented Dec 22, 2023

mergify bot commented Dec 22, 2023

msvc: Allow building with Visual Studio Preview versions #1711

msvc: Allow building with Visual Studio Preview versions #1711

Conversation

sankhesh commented Sep 27, 2023 • edited Loading

bstaletic commented Sep 27, 2023

puremourning left a comment

Choose a reason for hiding this comment

sankhesh commented Sep 27, 2023

puremourning commented Sep 27, 2023

puremourning left a comment

Choose a reason for hiding this comment

sankhesh commented Oct 3, 2023

bstaletic left a comment

Choose a reason for hiding this comment

sankhesh left a comment

Choose a reason for hiding this comment

sankhesh commented Oct 6, 2023

bstaletic commented Oct 6, 2023

codecov bot commented Oct 6, 2023 • edited Loading

Codecov Report

sankhesh commented Oct 13, 2023

bstaletic commented Oct 13, 2023

sankhesh commented Dec 1, 2023

puremourning left a comment

Choose a reason for hiding this comment

sankhesh left a comment

Choose a reason for hiding this comment

sankhesh left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 22, 2023

mergify bot commented Dec 22, 2023

sankhesh commented Dec 22, 2023

puremourning commented Dec 22, 2023

mergify bot commented Dec 22, 2023

sankhesh commented Sep 27, 2023 •

edited

Loading

codecov bot commented Oct 6, 2023 •

edited

Loading