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.

Sign up for GitHub

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

Jump to bottom

Clean windows build and unify output paths #1599

Merged
merged 17 commits into from
Sep 22, 2018
Merged

Clean windows build and unify output paths #1599

merged 17 commits into from
Sep 22, 2018

Conversation

lokitoth
Copy link
Member

This simplifies the Windows build

  • Remove Azure projects from solution - they are not well-maintained, and need retargeting / rethinking
  • Remove Setup projects from solution - they require WiX to be installed, and this does not seem valuable to most people who will be building VW
  • Remove VisualLeakDetector projects - this requires VLD extension to be installed, and the projects do not build
  • Sets up an environment variable to inject vcpkg when invoking msbuild from the command line
  • Remove unused projects/configurations from solution
  • Reconcile core projects' output paths, and simplify project files
  • vw_dynamic renamed to vw_core - it is no longer a dynamic library (that is libvw), so the name was unclear
  • Fix malformed xml in comment warnings
  • Fix missing dll error in boost unit test-based project

rajan-chari
rajan-chari reviewed Sep 14, 2018
View reviewed changes
test/unit_test/unit_test.vcxproj Outdated
</ClCompile>
<PostBuildEvent>
<Command Condition="'$(Platform)'=='x64'">xcopy /Y ..\..\vowpalwabbit\packages\boost_unit_test_framework-vc140.1.63.0.0\lib\native\address-model-64\lib\*.dll $(OutDir)</Command>
<Command Condition="'$(Platform)'=='x86'">xcopy /Y ..\..\vowpalwabbit\packages\boost_unit_test_framework-vc140.1.63.0.0\lib\native\address-model-32\lib\*.dll $(OutDir)</Command>
Copy link
Member

Choose a reason for hiding this comment

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

I don't see platform x86 configured. I see Win32 and x64.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch

@JohnLangford
Copy link
Member

@lokitoth is this waiting on some updates from Rajan's comments?

@lokitoth
Remove Setup and Azure projects from solution
4cdea9f 
* The Azure projects are not well-maintained; they need retargetting / rethinking
* The Setup project requires WiX installed, and this does not seem valuable to most people building VW
@lokitoth
Remove Azure dependency from C# Binding unit tests
af6c8ce
@lokitoth
Enable command-line msbuild for vcpkg-dependent projects
116344c 
* This depends on VcpkgIntegration environment variable / project
   property set to the .targets file from msbuild integration:
      (VcPkgRoot)/scripts/buildsystems/msbuild/vcpkg.targets.

   If it is not present, the behaviour will be as before this change,
   demanding VS-level integration.
@lokitoth
Simplify and rename vs static library project
fc997c5 
* Rename vw_dynamic => vw_core
* Manually simplify the project and reconcile the outputs to
   $(SolutionDir)\out\<target\int>\...
* Update dependent projects
@lokitoth
Simplify vw executable project
2a8caed 
* Enabled multi-threaded compilation
* Also adds $(ProjectName) to int-path to avoid collisions
@lokitoth
Fix "malformed xml in xmldoc comment" warning
66d771b
@lokitoth
Simplify libvw project file
de1c717
@lokitoth
Fix typo in vw exe project
e712172
@lokitoth
Simplify c_test project
c99cdc4
@lokitoth
Simplify unit_test.vcxproj and unify output/intermediate paths
b37004d
@lokitoth
Add xcopy step to place boost unit test dll
cdddcb1
@lokitoth
Remove VisualLeakDetector-based projects
2e404a6 
They do not currently build, and require an installed component to work
properly.
@lokitoth
Simplify vw clr project and unify outputs
3b4a7b0
@lokitoth
Remove unused deploy_vw project from build
3a40657
@jackgerrits jackgerrits mentioned this pull request Sep 20, 2018
Merged
@lokitoth
Update for PR comments
dd7751d 
* Update unittest project to pull the right package under x86

None of these change the behaviour of the build after the previous set
of changes, just removes non-building projects:
* Delete superfluous vcsproj files (for VLD integration)
* Remove VLD package reference
@JohnLangford
Copy link
Member

What's the status of this one?

@rajan-chari
Copy link
Member

Ready to merge.

@JohnLangford JohnLangford merged commit 285620e into VowpalWabbit:master Sep 22, 2018
@lokitoth lokitoth deleted the Clean-WindowsBuild branch April 16, 2019 13:46
lokitoth added a commit to lokitoth/vowpal_wabbit that referenced this pull request Dec 20, 2019
@lokitoth @JohnLangford
Clean windows build and unify output paths (VowpalWabbit#1599)
7fcdb24 
* Remove Setup and Azure projects from solution

* The Azure projects are not well-maintained; they need retargetting / rethinking
* The Setup project requires WiX installed, and this does not seem valuable to most people building VW

* Remove Azure dependency from C# Binding unit tests

* Enable command-line msbuild for vcpkg-dependent projects

* This depends on VcpkgIntegration environment variable / project
   property set to the .targets file from msbuild integration:
      (VcPkgRoot)/scripts/buildsystems/msbuild/vcpkg.targets.

   If it is not present, the behaviour will be as before this change,
   demanding VS-level integration.

* Simplify and rename vs static library project

* Rename vw_dynamic => vw_core
* Manually simplify the project and reconcile the outputs to
   $(SolutionDir)\out\<target\int>\...
* Update dependent projects

* Simplify vw executable project

* Enabled multi-threaded compilation
* Also adds $(ProjectName) to int-path to avoid collisions

* Fix "malformed xml in xmldoc comment" warning

* Simplify libvw project file

* Fix typo in vw exe project

* Simplify c_test project

* Simplify unit_test.vcxproj and unify output/intermediate paths

* Add xcopy step to place boost unit test dll

* Remove VisualLeakDetector-based projects

They do not currently build, and require an installed component to work
properly.

* Simplify vw clr project and unify outputs

* Remove unused deploy_vw project from build

* Remove unused DebugLeakCheck configuration

* Simplify C# projects and unify outputs

* Update for PR comments

* Update unittest project to pull the right package under x86

None of these change the behaviour of the build after the previous set
of changes, just removes non-building projects:
* Delete superfluous vcsproj files (for VLD integration)
* Remove VLD package reference
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackgerrits jackgerrits jackgerrits left review comments

@rajan-chari rajan-chari rajan-chari left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lokitoth @JohnLangford @rajan-chari @jackgerrits