Upgrade and fix clang-format-vs

[Hans Wennborg via cfe-commits]

On Mon, Nov 28, 2016 at 11:11 AM, Antonio Maiorano <amaiorano at gmail.com> wrote:
>> It's built with the script in utils/release/build_llvm_package.bat
> which I run manually on my machine once every few weeks.
>
> Okay, that's good news. So the simplest path to success would be to require
> the user to either pass the path to CMake via an arg like -DNUGET_EXE_PATH,
> or if it's not defined, to assume it's already in PATH. This is the most
> future-proof solution as it will work with future versions of VS (2017 RC
> just came out).
>
> I can still look into whether a vsix built with VS 2015 references will
> continue to work in older versions of VS, but even if this works, I feel
> like it's a temporary solution at best. There are other advantages to using
> NuGet here: it would allow us to more easily pin/upgrade which assemblies we
> want to use over time.
>
> If you're okay with it, I'll make the changes necessary to use
> -DNUGET_EXE_PATH, if defined, otherwise assume it's on PATH. This should be
> a simple change at this point.

That sounds good to me. There are already a bunch of prerequisites for
building the plugin, so adding this one doesn't seem unreasonable.
Especially since it seems it will simplify things to the point that
they might even work elsewhere than my own machine :-)


> On Mon, 28 Nov 2016 at 13:59 Hans Wennborg <hans at chromium.org> wrote:
>>
>> On Mon, Nov 28, 2016 at 10:46 AM, Antonio Maiorano <amaiorano at gmail.com>
>> wrote:
>> > Okay, I'll see if upgrading to the 2015 assemblies would allow the VSIX
>> > to
>> > keep working in older versions of VS.
>> >
>> > Still waiting on an answer to this question:
>> >
>> >> In either case, though, I must ask: how is the offical vsix that's
>> >> available on http://llvm.org/builds/ get built? Is it part of an
>> >> automated
>> >> Clang build, or is it built and uploaded manually? If it's automated,
>> >> then
>> >> having to download and point to nuget.exe won't work.
>>
>> It's built with the script in utils/release/build_llvm_package.bat
>> which I run manually on my machine once every few weeks.
>>
>>
>> > On Mon, 28 Nov 2016 at 13:04 Hans Wennborg <hans at chromium.org> wrote:
>> >>
>> >> On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano <amaiorano at gmail.com>
>> >> wrote:
>> >> > Ah, no, that's not what I meant. The required referenced assemblies
>> >> > are
>> >> > versions that are normally installed with VS 2010.
>> >> >
>> >> > The first time I worked on this, I had upgraded the referenced
>> >> > assemblies to
>> >> > the ones that ship with VS 2015, but then there was question of
>> >> > whether
>> >> > or
>> >> > not the VSIX would continue to work with VS 2010/2012/2013. I'm not
>> >> > sure
>> >> > if
>> >> > it would work, but I guess I can try to figure that out.
>> >>
>> >> Let me know if you figure this one out. It sounds like it would
>> >> simplify things a lot.
>> >>
>> >> > In any case, what I discovered is that the "right" way to do things
>> >> > to
>> >> > make
>> >> > sure your extension compiles in future versions of VS is to use NuGet
>> >> > to
>> >> > automatically pull in the required assemblies, or to check them in
>> >> > and
>> >> > reference them directly. The former would be better except for the
>> >> > problem
>> >> > of CLI builds as I described in my earlier email.
>> >> >
>> >> >
>> >> >
>> >> > On Fri, 25 Nov 2016 at 21:47 Zachary Turner <zturner at google.com>
>> >> > wrote:
>> >> >>
>> >> >> Sorry, i think I misunderstood the original option 1. I interpreted
>> >> >> it
>> >> >> as
>> >> >> just committing changes to the vsix manifest to reference a specific
>> >> >> version
>> >> >> of the assembly which we assume to be present since it should be
>> >> >> automatically installed with vs 2015. Is this not possible? Can't we
>> >> >> just
>> >> >> point the manifest to the version installed with vs?
>> >> >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano
>> >> >> <amaiorano at gmail.com>
>> >> >> wrote:
>> >> >>>
>> >> >>> Hi again,
>> >> >>>
>> >> >>> I've made the changes so that the required assemblies are
>> >> >>> committed,
>> >> >>> so
>> >> >>> now we can build the clang-format-vsix with just VS 2015. Since the
>> >> >>> patch
>> >> >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if
>> >> >>> you'd
>> >> >>> rather I attach it, let me know):
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
>> >> >>>
>> >> >>> Note that it's a git patch set, for which I followed the
>> >> >>> instructions
>> >> >>> here.
>> >> >>>
>> >> >>> Thanks.
>> >> >>>
>> >> >>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano <amaiorano at gmail.com>
>> >> >>> wrote:
>> >> >>>>
>> >> >>>> Okay, that's fine, I'll go for that and if all looks good, will
>> >> >>>> attach a
>> >> >>>> patch.
>> >> >>>>
>> >> >>>> Thanks.
>> >> >>>>
>> >> >>>> On Thu, 24 Nov 2016 at 15:09 Zachary Turner <zturner at google.com>
>> >> >>>> wrote:
>> >> >>>>>
>> >> >>>>> I would use the first solution. We lock ourselves to specific
>> >> >>>>> versions
>> >> >>>>> of vs, so i think it's fine to do the same with the assemblies
>> >> >>>>> and
>> >> >>>>> deal with
>> >> >>>>> it only when we upgrade
>> >> >>>>> On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano
>> >> >>>>> <amaiorano at gmail.com>
>> >> >>>>> wrote:
>> >> >>>>>>
>> >> >>>>>> Hi Hans,
>> >> >>>>>>
>> >> >>>>>> I saw that on September 15th, you checked in a change:
>> >> >>>>>> clang-format
>> >> >>>>>> VS
>> >> >>>>>> plugin: upgrade the project files to VS2015.
>> >> >>>>>>
>> >> >>>>>> When I open the latest version of ClangFormat.sln on a machine
>> >> >>>>>> that
>> >> >>>>>> has only VS 2015, it doesn't build. The reason is that some of
>> >> >>>>>> the
>> >> >>>>>> referenced assemblies are from VS 2010.
>> >> >>>>>>
>> >> >>>>>> <Reference Include="Microsoft.VisualStudio.CoreUtility,
>> >> >>>>>> Version=10.0.0.0, Culture=neutral,
>> >> >>>>>> PublicKeyToken=b03f5f7f11d50a3a,
>> >> >>>>>> processorArchitecture=MSIL" /> <Reference
>> >> >>>>>> Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0,
>> >> >>>>>> Culture=neutral,
>> >> >>>>>> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>> >> >>>>>>
>> >> >>>>>> What happens is that when building, these specific assemblies
>> >> >>>>>> are
>> >> >>>>>> not
>> >> >>>>>> found. I suspect you have VS 2010 installed on your machine,
>> >> >>>>>> which
>> >> >>>>>> is why
>> >> >>>>>> you don't get these build errors.
>> >> >>>>>>
>> >> >>>>>> The recommended way to handle this is to make use of NuGet to
>> >> >>>>>> have
>> >> >>>>>> it
>> >> >>>>>> automatically download the required assemblies. I have made the
>> >> >>>>>> changes
>> >> >>>>>> locally to get this working, and it works great when building
>> >> >>>>>> ClangFormat.sln from within Visual Studio; however, building
>> >> >>>>>> from
>> >> >>>>>> the CLI
>> >> >>>>>> doesn't work out of the box because you must explicitly run
>> >> >>>>>> 'nuget.exe
>> >> >>>>>> restore ClangFormat.sln' before running msbuild (or devenv.exe
>> >> >>>>>> in
>> >> >>>>>> our case).
>> >> >>>>>> The problem is that nuget.exe isn't something that's easily
>> >> >>>>>> found/accessible
>> >> >>>>>> on Windows, even once installed as an extension in VS. This is a
>> >> >>>>>> known
>> >> >>>>>> problem and the current best solution requires downloading and
>> >> >>>>>> making
>> >> >>>>>> nuget.exe available in PATH.
>> >> >>>>>>
>> >> >>>>>> So now i'm faced with figuring out how best to solve this so
>> >> >>>>>> that
>> >> >>>>>> the
>> >> >>>>>> custom build step in this CMakeLists.txt that runs devenv
>> >> >>>>>> doesn't
>> >> >>>>>> fail:
>> >> >>>>>>
>> >> >>>>>> devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build
>> >> >>>>>> Release
>> >> >>>>>>
>> >> >>>>>> There are a few options:
>> >> >>>>>>
>> >> >>>>>> 1) Forget NuGet and just commit the referenced assemblies. This
>> >> >>>>>> is
>> >> >>>>>> the
>> >> >>>>>> simplest solution, but is more annoying to manage if we want to
>> >> >>>>>> upgrade the
>> >> >>>>>> versions of these assemblies later.
>> >> >>>>>>
>> >> >>>>>> 2) Commit nuget.exe to the repo and just use it. This is simple
>> >> >>>>>> enough, but I'm not sure how people feel about committing
>> >> >>>>>> binaries,
>> >> >>>>>> and it
>> >> >>>>>> would be frozen at whatever version we commit.
>> >> >>>>>>
>> >> >>>>>> 3) Do some form of wget to grab the latest nuget.exe from
>> >> >>>>>> "https://nuget.org/nuget.exe" in CMake and invoke it. I'm not
>> >> >>>>>> yet
>> >> >>>>>> sure
>> >> >>>>>> what's the simplest way to do this. Powershell could be used,
>> >> >>>>>> but
>> >> >>>>>> there are
>> >> >>>>>> security annoyances to deal with.
>> >> >>>>>>
>> >> >>>>>> That's all I can come up with so far. Would love to hear from
>> >> >>>>>> you
>> >> >>>>>> guys
>> >> >>>>>> if you have any ideas or opinions on this. If you want I can
>> >> >>>>>> send
>> >> >>>>>> you a
>> >> >>>>>> patch of what I've got so far if that helps.
>> >> >>>>>>
>> >> >>>>>> Thanks,
>> >> >>>>>>
>> >> >>>>>> Antonio Maiorano
>> >> >>>>>>
>> >> >>>>>>
>> >> >>>>>>
>> >> >>>>>> On Thu, 15 Sep 2016 at 19:35 Antonio Maiorano
>> >> >>>>>> <amaiorano at gmail.com>
>> >> >>>>>> wrote:
>> >> >>>>>>>
>> >> >>>>>>> Sorry I haven't had a chance to get back to this. Things got
>> >> >>>>>>> busy
>> >> >>>>>>> at
>> >> >>>>>>> work. I do plan to get back to this as I'm hoping to add some
>> >> >>>>>>> features to
>> >> >>>>>>> this extension :)
>> >> >>>>>>> On Thu, Sep 15, 2016 at 7:31 PM Zachary Turner
>> >> >>>>>>> <zturner at google.com>
>> >> >>>>>>> wrote:
>> >> >>>>>>>>
>> >> >>>>>>>> Strange.  FWIW you can dump all the variables that are present
>> >> >>>>>>>> in
>> >> >>>>>>>> your environment.  You need to go to Tools -> Options ->
>> >> >>>>>>>> Projects
>> >> >>>>>>>> and
>> >> >>>>>>>> Solutions -> Build and Run and choose either Normal, Detailed,
>> >> >>>>>>>> or
>> >> >>>>>>>> Diagnostic
>> >> >>>>>>>> for the MSBuild project build output verbosity.  Then in the
>> >> >>>>>>>> output window
>> >> >>>>>>>> you will get a ton of spam, some of which is the set of all
>> >> >>>>>>>> MSBuild
>> >> >>>>>>>> variables you can take advantage of.
>> >> >>>>>>>>
>> >> >>>>>>>> On Thu, Sep 15, 2016 at 4:25 PM Hans Wennborg
>> >> >>>>>>>> <hans at chromium.org>
>> >> >>>>>>>> wrote:
>> >> >>>>>>>>>
>> >> >>>>>>>>> When I first opened the solution in VS it prompted me to
>> >> >>>>>>>>> install
>> >> >>>>>>>>> it
>> >> >>>>>>>>> and I did.
>> >> >>>>>>>>>
>> >> >>>>>>>>> On Thu, Sep 15, 2016 at 4:17 PM, Zachary Turner
>> >> >>>>>>>>> <zturner at google.com> wrote:
>> >> >>>>>>>>> > You may need to install the Visual Studio SDK.  Did you do
>> >> >>>>>>>>> > that
>> >> >>>>>>>>> > when you
>> >> >>>>>>>>> > initially installed VS 2015?
>> >> >>>>>>>>> >
>> >> >>>>>>>>> > On Thu, Sep 15, 2016 at 4:15 PM Hans Wennborg
>> >> >>>>>>>>> > <hans at chromium.org>
>> >> >>>>>>>>> > wrote:
>> >> >>>>>>>>> >>
>> >> >>>>>>>>> >> Well, on my machine $(SDKToolsDir) doesn't work :-( I
>> >> >>>>>>>>> >> suspect
>> >> >>>>>>>>> >> the file
>> >> >>>>>>>>> >> will need manual tweaking by whoever is trying to build
>> >> >>>>>>>>> >> the
>> >> >>>>>>>>> >> plugin.
>> >> >>>>>>>>> >>
>> >> >>>>>>>>> >> Anyway, I've updated the solution to build with VS2015 in
>> >> >>>>>>>>> >> r281648 and
>> >> >>>>>>>>> >> confirmed that it can still be used with older VS versions
>> >> >>>>>>>>> >> too.
>> >> >>>>>>>>> >>
>> >> >>>>>>>>> >> Cheers,
>> >> >>>>>>>>> >> Hans
>> >> >>>>>>>>> >>
>> >> >>>>>>>>> >> On Thu, Aug 18, 2016 at 7:11 PM, Zachary Turner
>> >> >>>>>>>>> >> <zturner at google.com>
>> >> >>>>>>>>> >> wrote:
>> >> >>>>>>>>> >> > The key.snk is generated when you build, the problem is
>> >> >>>>>>>>> >> > the
>> >> >>>>>>>>> >> > csproj file
>> >> >>>>>>>>> >> > hardcodes the directory to the sdk instead of using the
>> >> >>>>>>>>> >> > appropriate
>> >> >>>>>>>>> >> > project
>> >> >>>>>>>>> >> > system variable like $(SDKToolsDir)
>> >> >>>>>>>>> >> >
>> >> >>>>>>>>> >> > On Thu, Aug 18, 2016 at 7:09 PM Zachary Turner
>> >> >>>>>>>>> >> > <zturner at google.com>
>> >> >>>>>>>>> >> > wrote:
>> >> >>>>>>>>> >> >>
>> >> >>>>>>>>> >> >> Llvm doesn't support vs2012 anymore, as long as it
>> >> >>>>>>>>> >> >> supports
>> >> >>>>>>>>> >> >> vs2013 it's
>> >> >>>>>>>>> >> >> fine
>> >> >>>>>>>>> >> >> On Thu, Aug 18, 2016 at 7:07 PM Antonio Maiorano
>> >> >>>>>>>>> >> >> <amaiorano at gmail.com>
>> >> >>>>>>>>> >> >> wrote:
>> >> >>>>>>>>> >> >>>
>> >> >>>>>>>>> >> >>> Hi,
>> >> >>>>>>>>> >> >>>
>> >> >>>>>>>>> >> >>> What I meant by upgrade was simply making it build in
>> >> >>>>>>>>> >> >>> VS
>> >> >>>>>>>>> >> >>> 2015.
>> >> >>>>>>>>> >> >>> However,
>> >> >>>>>>>>> >> >>> you bring up a valid point about making sure the
>> >> >>>>>>>>> >> >>> extension
>> >> >>>>>>>>> >> >>> will
>> >> >>>>>>>>> >> >>> continue to
>> >> >>>>>>>>> >> >>> work in VS 2012. I will look into that. Like those
>> >> >>>>>>>>> >> >>> references that go
>> >> >>>>>>>>> >> >>> from
>> >> >>>>>>>>> >> >>> 10 to 14 that point out; I wonder if instead I should
>> >> >>>>>>>>> >> >>> be
>> >> >>>>>>>>> >> >>> able to bring
>> >> >>>>>>>>> >> >>> in
>> >> >>>>>>>>> >> >>> those version 10 assemblies via NuGet. I'll take a
>> >> >>>>>>>>> >> >>> closer
>> >> >>>>>>>>> >> >>> look.
>> >> >>>>>>>>> >> >>>
>> >> >>>>>>>>> >> >>> Part of my change, however, seems to imply that the
>> >> >>>>>>>>> >> >>> extension (vsix)
>> >> >>>>>>>>> >> >>> project would not build correctly even in VS 2012. For
>> >> >>>>>>>>> >> >>> instance, the
>> >> >>>>>>>>> >> >>> missing
>> >> >>>>>>>>> >> >>> Key.snk file. I don't have VS 2012 installed at the
>> >> >>>>>>>>> >> >>> moment,
>> >> >>>>>>>>> >> >>> so I
>> >> >>>>>>>>> >> >>> cannot
>> >> >>>>>>>>> >> >>> validate.
>> >> >>>>>>>>> >> >>>
>> >> >>>>>>>>> >> >>> Thanks,
>> >> >>>>>>>>> >> >>>
>> >> >>>>>>>>> >> >>> Antonio
>> >> >>>>>>>>> >> >>>
>> >> >>>>>>>>> >> >>>
>> >> >>>>>>>>> >> >>>
>> >> >>>>>>>>> >> >>> On Thu, 18 Aug 2016 at 19:38 Hans Wennborg
>> >> >>>>>>>>> >> >>> <hans at chromium.org> wrote:
>> >> >>>>>>>>> >> >>>>
>> >> >>>>>>>>> >> >>>> Hi Antonio,
>> >> >>>>>>>>> >> >>>>
>> >> >>>>>>>>> >> >>>> On Wed, Aug 17, 2016 at 8:15 AM, Antonio Maiorano via
>> >> >>>>>>>>> >> >>>> cfe-commits
>> >> >>>>>>>>> >> >>>> <cfe-commits at lists.llvm.org> wrote:
>> >> >>>>>>>>> >> >>>> > This patch for clang-format-vs includes the
>> >> >>>>>>>>> >> >>>> > following:
>> >> >>>>>>>>> >> >>>> >
>> >> >>>>>>>>> >> >>>> > - Upgrade to VS 2015, including .NET framework
>> >> >>>>>>>>> >> >>>> > upgrade
>> >> >>>>>>>>> >> >>>> > from 4.0 to
>> >> >>>>>>>>> >> >>>> > 4.5, and
>> >> >>>>>>>>> >> >>>> > upgrading Microsoft.VisualStudio references to v14
>> >> >>>>>>>>> >> >>>> > versions
>> >> >>>>>>>>> >> >>>> > - Fix build by removing dependency on "Key.snk"
>> >> >>>>>>>>> >> >>>> > file
>> >> >>>>>>>>> >> >>>> > which was
>> >> >>>>>>>>> >> >>>> > never
>> >> >>>>>>>>> >> >>>> > checked
>> >> >>>>>>>>> >> >>>> > in (and not really required anyway)
>> >> >>>>>>>>> >> >>>> > - Add ".vs" directory to svn ignore (new folder
>> >> >>>>>>>>> >> >>>> > that
>> >> >>>>>>>>> >> >>>> > VS
>> >> >>>>>>>>> >> >>>> > 2015
>> >> >>>>>>>>> >> >>>> > creates
>> >> >>>>>>>>> >> >>>> > for
>> >> >>>>>>>>> >> >>>> > user settings)
>> >> >>>>>>>>> >> >>>>
>> >> >>>>>>>>> >> >>>> "What does "Upgrade to VS 2015 mean? Adding support
>> >> >>>>>>>>> >> >>>> for
>> >> >>>>>>>>> >> >>>> running the
>> >> >>>>>>>>> >> >>>> plugin in VS2015, or does it mean requiring VS2015
>> >> >>>>>>>>> >> >>>> for
>> >> >>>>>>>>> >> >>>> building?
>> >> >>>>>>>>> >> >>>>
>> >> >>>>>>>>> >> >>>> +zturner: I thought the plugin already worked in VS
>> >> >>>>>>>>> >> >>>> 2015?
>> >> >>>>>>>>> >> >>>>
>> >> >>>>>>>>> >> >>>> I mostly just build the plugin without knowing
>> >> >>>>>>>>> >> >>>> exactly
>> >> >>>>>>>>> >> >>>> how
>> >> >>>>>>>>> >> >>>> this stuff
>> >> >>>>>>>>> >> >>>> works, but looking at the patch I'm worried that
>> >> >>>>>>>>> >> >>>> you're
>> >> >>>>>>>>> >> >>>> increasing
>> >> >>>>>>>>> >> >>>> the
>> >> >>>>>>>>> >> >>>> required version for building it? I see a bunch of
>> >> >>>>>>>>> >> >>>> values
>> >> >>>>>>>>> >> >>>> going from
>> >> >>>>>>>>> >> >>>> 10 (VS 2012) to 14 (VS 2015).
>> >> >>>>>>>>> >> >>>>
>> >> >>>>>>>>> >> >>>> Thanks,
>> >> >>>>>>>>> >> >>>> Hans