Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

I've attached a patch that works as discussed. When running CMake
with -DBUILD_CLANG_FORMAT_VS_PLUGIN=ON, it will look for nuget.exe in PATH,
or you can pass in DNUGET_EXE_PATH=C:\nuget, for e.g.


On Mon, 28 Nov 2016 at 14:31 Antonio Maiorano <amaiorano at gmail.com> wrote:

> Great, I'll get this working soon and attach a new patch :)
>
> On Mon, 28 Nov 2016 at 14:27 Hans Wennborg <hans at chromium.org> wrote:
>
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161129/0a649626/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-VS2015-build-of-clang-format-vsix-by-using-NuGet.patch
Type: application/octet-stream
Size: 14740 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161129/0a649626/attachment-0001.obj>