conda-forge / staged-recipes

A place to submit conda recipes before they become fully fledged conda-forge feedstocks
https://conda-forge.org
BSD 3-Clause "New" or "Revised" License
697 stars 4.83k forks source link

Package vs2017_runtime #4474

Open minrk opened 6 years ago

minrk commented 6 years ago

It would be good to package the Visual Studio 2017 runtime, now that it's out. I imagine this should be mostly a copy of the vs2015_runtime with a few version numbers changed.

isuruf commented 6 years ago

I was under the impression that vs2015 and vs2017 runtime dlls are the same

minrk commented 6 years ago

Maybe they are! I might have gotten confused while trying to build something that required vs2017.

jakirkham commented 6 years ago

That was also my understanding, but maybe someone else can verify. @mingwandroid, do you know offhand if we will need to package these or not?

mingwandroid commented 6 years ago

They're not the same, just compatible from now on and delivered as part of Windows (10 and above).

jakirkham commented 6 years ago

Thanks for clarifying. So should we package them?

msarahan commented 6 years ago

I don't know for certain, but I think it's safe to treat them the same way we treat libstdc++ on linux: have a lower bound on the runtime corresponding to the version of the compiler used. Assume forwards compatibility (newer runtimes support code built with older compilers), but not backwards compatibility (older runtimes may not support code built with newer compilers).

I began to package these: https://github.com/AnacondaRecipes/aggregate/tree/master/vs2017 but we have not begun using this new compiler, and that recipe may need more work.

jakirkham commented 6 years ago

Sounds like the gist of this is PRs welcome. :)

msarahan commented 6 years ago

Please revisit our recipe. This commit in particular was essential to make things work: https://github.com/AnacondaRecipes/aggregate/commit/b7579119655fd18143f419c2b943830e9ce4d86d#diff-ae85109899cb22e14629b1fe648eba60

msarahan commented 6 years ago

PS: we made the runtime package name be vs2015_runtime. It has a different version number from the one for VS2015. We did this to get mutual exclusivity, because the filenames in the two are the same, and would clobber each other.

bencherian commented 6 years ago

@msarahan Do you know when the new 14.1 version of the vs2015_runtime be made available in the main Anaconda repository?

msarahan commented 6 years ago

We have been holding off on this because there have been show-stopper bugs in VS2017.

I have heard that there was a new release recently, so we should revisit it to see if it is workable now.

CC @mingwandroid

jakirkham commented 6 years ago

What sort of bugs?

mingwandroid commented 6 years ago

Bugs like this one preventing the Qt Project from supporting it: https://bugreports.qt.io/browse/QTBUG-59575

Some older ones from @SylvainCorlay (if you can share more details that'd be great): https://twitter.com/SylvainCorlay/status/973496978335297536 https://twitter.com/SylvainCorlay/status/972410040861777924

.. and a brand new one against the newest release (I think): https://twitter.com/SylvainCorlay/status/997072965798957058

At this point I would consider (or support any effort to investigate the feasibility of) switching to a UCRT-backended Clang on Windows. I know it'd be a load of work but this is a perfect case study as to why being able to fix your compilers is extremely important.

SylvainCorlay commented 6 years ago

The bug mentioned in https://twitter.com/SylvainCorlay/status/972410040861777924 with respect to the link errors was fixed in 15.7 (cf developercommunity.visualstudio.com/content/problem/208938/compilation-error-c2057-expected-constant-expressi.html)

Overall, we have had many issues with Visual C++ 2017 with every update. 2015 has been more robust.

SylvainCorlay commented 6 years ago

We document compiler bugs and implemented workaround in a section of the documentation of xtensor (where there are the most) and other QuantStack projects such as xwidgets, xtl, etc.

isuruf commented 6 years ago

At this point I would consider (or support any effort to investigate the feasibility of) switching to a UCRT-backended Clang on Windows.

+1 to using clang/clang-cl on windows.

I know it'd be a load of work but this is a perfect case study as to why being able to fix your compilers is extremely important.

What kind of work needs to be done?

mingwandroid commented 6 years ago

Well, nearly every package in existence that can be built on Windows assumes that when building on Windows you are using MSVC and if they don't assume MSVC then they might fall back to GCC.

if (MSVC)
  # MSVC specific stuff.
else ()
  # GCC specific stuff.
endif (MSVC)

.. check pretty much any sufficiently complex CMake based project.

Now I know Clang implements an MSVC frontend but I don't expect it to emulate everything (or even most things, pragmas, warning numbering (ones specific to MS's C++ standard library for example), flags, subtle behaviour gotchas etc). I know for many years that MS special C++ exception blocks stuff was not supported (as used in Google's sandbox library) but that's now fixed AFAIK.

Then you need to think about the base build tools and any fixes needed in there. Does CMake set MSVC when running Clang with the MSVC frontend? Does it set also CLANG so that workarounds can be implemented? Apart from the CMakeLists.txt files (and the numerous CMake share files for 'handling' specific libraries) are the (probably) thousands of lines of C++ code in CMake that handle MSVC correct for clang.

The even if all these issues are fixed and/or already ok, is Clang truly interoperable with MSVC? If not, if not, does everything need to be rebuilt?

And (if so and regardless), what to do with libc++ ignore it and use MSVC's C++ standard library? Is that even legally allowed? If we'd switch to libc++ how good are they about backwards compatibility and how Windows-ready are they?

I hope I've convinced you this is far from simple.

mingwandroid commented 6 years ago

Then there are the numerous packages where the build scripts for Windows are 'pre-baked' .vcxproj files probably full of hard-coded references to MSVC tooling.

bencherian commented 6 years ago

@mingwandroid @SylvainCorlay Do any of these bugs affect the actual runtime? If not this is really only an explanation for why you wouldn't distribute the compiler...there's nothing to prevent people from still using the older compiler package to build packages that require the runtime to be 14.0 or greater, right?

mingwandroid commented 6 years ago

The runtime is the UCRT so in theory you'd be OK, but the runtime comes with your computer anyway provided you are running Windows 10 so it's not hugely relevant.

isuruf commented 6 years ago

Thanks for the info. I'll look into details.

Here's some info that I know from my experience building OpenBLAS with clang-cl.exe

Then you need to think about the base build tools and any fixes needed in there. Does CMake set MSVC when running Clang with the MSVC frontend?

Yes, MSVC is defined.

Does it set also CLANG so that workarounds can be implemented?

CMAKE_<C/CXX>_COMPILER_ID is set to Clang.

Apart from the CMakeLists.txt files (and the numerous CMake share files for 'handling' specific libraries) are the (probably) thousands of lines of C++ code in CMake that handle MSVC correct for clang.

I think so. Note that clang.exe is not supported, but clang-cl.exe is.

And (if so and regardless), what to do with libc++ ignore it and use MSVC's C++ standard library? Is that even legally allowed? If we'd switch to libc++ how good are they about backwards compatibility and how Windows-ready are they?

I'd say ignore libc++ and use MSVC's C++ standard library. Don't know much about libc++ on windows.

I hope I've convinced you this is far from simple.

Of course. I don't think it's simple.

mingwandroid commented 6 years ago

Thanks for the details @isuruf one thing we cannot gloss over though:

I'd say ignore libc++ and use MSVC's C++ standard library.

Is that even legally allowed?

hmaarrfk commented 5 years ago

Has this been revisited lately? pytorch requires MSVC2017.

msarahan commented 5 years ago

https://github.com/AnacondaRecipes/aggregate/tree/master/vs2017

We use this on defaults with the usual c_compiler settings in CBC.yaml. https://github.com/AnacondaRecipes/aggregate/blob/master/conda_build_config.yaml

hmaarrfk commented 5 years ago

@ msarahan why is your cl version 19.xx.xxxxx and not 14.xx.xxxxx???

NVM, it is strange that the number are so similar, but the cl.exe in 14.xx.xxxx does report that it has version 19.xx

msarahan commented 5 years ago

Yep... There's some comments on that in https://github.com/AnacondaRecipes/aggregate/blob/master/vs2017/conda_build_config.yaml#L4

hmaarrfk commented 5 years ago

Is this a really special package for you guys? Do you have to essentially build it from a running windows machine?

msarahan commented 5 years ago

Yep... It has to be run on a machine that has the exact VS version specified by the recipe. It tries to validate that the correct runtimes are being packaged. Because it's a straight binary repack, it's important to be strict about that, or else the version numbers of the package would be meaningless.

hmaarrfk commented 5 years ago

Yeah, I guess I'm downloading it and trying to install it automatically in the recipe. It will likely be super fragile once windows decides to randomly update their installed version (which I think they did with me just now ....)

Seeing as this is the case, is there really a reason to package within conda-forge, or is using defaults' essentially the same.