search

ycm-core / ycmd

A code-completion & code-comprehension server
https://ycm-core.github.io/ycmd/
GNU General Public License v3.0
1.69k stars 764 forks source link

msvc: Allow building with Visual Studio Preview versions #1711

Closed sankhesh closed 8 months ago

sankhesh commented 11 months ago

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 Reviewable

bstaletic commented 11 months ago

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.

sankhesh commented 11 months ago

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 commented 11 months ago

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?

sankhesh commented 11 months ago

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.

sankhesh commented 11 months ago

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

bstaletic commented 11 months ago

It happens. Windows jobs are a little flakey.

codecov[bot] commented 11 months ago

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 ```diff @@ 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 commented 11 months ago

@bstaletic @puremourning Good to merge?

bstaletic commented 11 months ago

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?

sankhesh commented 9 months ago

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

mergify[bot] commented 8 months ago

Thanks for sending a PR!

mergify[bot] commented 8 months ago

Thanks for sending a PR!

sankhesh commented 8 months ago

@bstaletic @puremourning Anything else needed here?

puremourning commented 8 months ago

@bstaletic @puremourning Anything else needed here?

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

mergify[bot] commented 8 months ago

Thanks for sending a PR!