Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert Windows to use vckpg instead of nuget #277

Merged
merged 3 commits into from
Apr 4, 2018
Merged

Conversation

hughbe
Copy link
Contributor

@hughbe hughbe commented Apr 2, 2018

This is better as we don't need to maintain our own dependency on hexchat's GTK and we can rely on the vcpkg community

@qmfrederik FYI

@hughbe
Copy link
Contributor Author

hughbe commented Apr 2, 2018

@filipnavara If this PR is accepted, this should fix the WIn32 issue you mentioned in #269

@filipnavara
Copy link
Contributor

There was already Pango in the HexChat package, so it was not a deal-breaker, but I definitely prefer the VCPKG approach.

vcpkg integrate install
git clone https://github.com/Microsoft/vcpkg.git /cygdrive/c/Program\ Files\ \(x86\)/
/cygdrive/c/Program\ Files\ \(x86\)/bootstrap-vcpkg.bat
/cygdrive/c/Program\ Files\ \(x86\)/vcpkg integrate install
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not writable directory, so it's not going to work even if you get all the path quoting right.

Copy link
Contributor

@filipnavara filipnavara Apr 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably want something more like

git clone https://github.com/Microsoft/vcpkg.git
vcpkg/bootstrap-vcpkg.bat
vcpkg/vcpkg integrate install

(not sure about the 'admin on first-use' requirement, but it's possible to use other integration methods)

@hughbe
Copy link
Contributor Author

hughbe commented Apr 2, 2018

@akoeplinger does this require vcpkg to be installed on the mono CI? I'm not sure whether I can do it from the script:

./.jenkins-windows.sh: line 6: vcpkg/bootstrap-vcpkg.bat: Permission denied

@qmfrederik
Copy link
Contributor

@hughbe Yes, vcpkg is the way to go! The build scripts look a lot nicer now 😄 .

@@ -87,28 +87,17 @@
<PropertyGroup Label="UserMacros" />
<ItemDefinitionGroup>
<ClCompile>
<AdditionalIncludeDirectories>$(ProjectDir)\..;$(ProjectDir)\..\gtk\$(Platform)\release\include;$(ProjectDir)\..\gtk\$(Platform)\release\include\glib-2.0;$(ProjectDir)\..\gtk\$(Platform)\release\lib\glib-2.0\include;$(ProjectDir)\..\gtk\$(Platform)\release\include\cairo;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<PreprocessorDefinitions>HAVE_LIBGIF;HAVE_LIBJPEG;HAVE_LIBTIFF;HAVE_LIBPNG;HAVE_FCFINI;_WINDLL;WIN32;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<PreprocessorDefinitions>HAVE_LIBGIF;HAVE_LIBJPEG;HAVE_LIBTIFF;HAVE_LIBPNG;_WINDLL;WIN32;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did HAVE_FCFINI go, btw?

@ras0219-msft
Copy link

We'd also be happy to accept a PR in vcpkg for libgdiplus so we can make sure dependency updates don't break you (or at a minimum notify you) :)

@akoeplinger
Copy link
Member

@hughbe hm, I don't know why it fails that way. If I run the command manually as the same user via RDP it works. I'll need to dig in why :)

@filipnavara
Copy link
Contributor

@monojenkins build

@filipnavara
Copy link
Contributor

filipnavara commented Apr 4, 2018

I am not surprised that it fails. The "vcpkg integrate install" command installs system-wide MSBuild scripts.

@hughbe
Copy link
Contributor Author

hughbe commented Apr 4, 2018

Maybe we need to call it just once on the machine (not from the script) to install it?

@akoeplinger
Copy link
Member

@filipnavara it doesn't fail at vcpkg integrate install, it fails right away for some reason.

@hughbe we could do that but since the Windows builders are dynamically provisioned Azure VMs we'd need to do it at provision time. And since vcpkg compiles all the packages that takes quite some time so we'll probably need to use the caching feature instead.

@akoeplinger
Copy link
Member

Ok, the issue was some weird interaction with the cygwin bash shell calling the Windows batch command.

Moving everything to powershell seemed to work. Build without caching (i.e. compiling all the libs) takes about 30mins now which is OK for now I think, the Travis build takes longer anyway.

Thanks!

@akoeplinger akoeplinger merged commit 6b96852 into mono:master Apr 4, 2018
@hughbe hughbe deleted the vcpkg branch April 4, 2018 16:28
hughbe added a commit to hughbe/libgdiplus that referenced this pull request Apr 10, 2018
This is better as we don't need to maintain our own dependency on hexchat's GTK and we can rely on the vcpkg community
hughbe added a commit to hughbe/libgdiplus that referenced this pull request Apr 16, 2018
This is better as we don't need to maintain our own dependency on hexchat's GTK and we can rely on the vcpkg community
hughbe added a commit to hughbe/libgdiplus that referenced this pull request Apr 16, 2018
This is better as we don't need to maintain our own dependency on hexchat's GTK and we can rely on the vcpkg community
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants