build: [WIP] Enable building C# projects on Windows with CMake #2929
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lokitoth
changed the title
jackgerrits
requested changes
Apr 8, 2021
| @@ -0,0 +1,171 @@ | |||
| # A collection of helpers to work with NuGet as required by VW projects. | |||
There was a problem hiding this comment.
Is this infrastructure usable for .Net core?
There was a problem hiding this comment.
The APIs, maybe, but for .NET Core the package management is driven through the dotnet command (the same applies for .NET Framework projects using the new SDK-project format). With that said, those work through the mechanism, and CMake has specific affordances for it: https://cmake.org/cmake/help/latest/prop_tgt/VS_PACKAGE_REFERENCES.html, so we may no longer need this infrastructure at all.
| @@ -356,6 +356,10 @@ if(BUILD_EXTERNAL_PARSER) | |||
| set(vw_all_sources ${vw_all_sources} ${external_parser_sources}) | |||
| endif() | |||
|
|
|||
| if (vw_BUILD_NETFX) | |||
| set(vw_all_sources ${vw_all_sources} spanning_tree.cc) | |||
There was a problem hiding this comment.
This is needed so that the object is present in the native-only lib, because we cannot import <future> into a /clr-compiled C++ object. There is a wrapper here in the CLI bindings, but if the regular spanning_tree object is missing from libvw linking fails. The Java bindings pull in spanning_tree.cc directly, and it is pulled in by the spanning_tree binary as well.
lokitoth
force-pushed
the
dev/Enable-FullWindowsCMakeBuild
branch
2 times, most recently
from
9d8ee34
to
25fd44c
Compare
|
Will be rebased, so please avoid merging with base branch through the GH UI |
|
I found a "helpful" MSBuild behaviour: When it runs a Task, unless explicitly opting out at the Task level (only available for users with the Exec task), it will parse the output for strings matching The short-term workaround is to disable More details here: https://stackoverflow.com/questions/48117302/does-the-msbuild-exec-task-search-stdout-for-the-string-error CMake mailing list discussion: |
* Flatten output structure (enable native dependencies to be copied SxS with managed) * Set up correct TargetFrameworkVersion * Enforce CMake version minimum when building .NET bindings * Unify WIN32-specific configuration in one CMake include
* This takes the generated sources of out the sourcetree
* Normalize .cmake file casing * Clarify option naming (NETFX => NET_FRAMEWORK) * Spelling and whitespace fixes
lokitoth
force-pushed
the
dev/Enable-FullWindowsCMakeBuild
branch
from
bbbb9ca
to
4885dc9
Compare
jackgerrits
approved these changes
May 6, 2021
jackgerrits
added a commit
to jackgerrits/vowpal_wabbit
that referenced
this pull request
May 8, 2021
* feat: Add cb_to_cb_adf reduction (VowpalWabbit#2680) This will 'translate' cb (non adf) input commands into their cb_adf counterparts Except when: - the model file loaded is from a version older than and not including 8.11.0 - user bypasses this translation step using '--cb_force_legacy' Breaks: predict format, output format of progressive validation. This is due to the input format of cb. --cb input format does not include 0 as a valid action, cb_adf uses 0 to represent the action 1. This will merge the behavior between the cb and cb_adf branch. Related files: cb_algs.cc, cb_explore.cc, cbify.cc, cb_to_cb_adf.cc * feat: [cb cb_adf] prog val print known label (VowpalWabbit#2957) * refactor: migrate active_cover learner init (VowpalWabbit#2960) * feat: consolidate math functions and add combinatorial related funcs (VowpalWabbit#2965) * feat: consolidate math functions and add combinatorial related funcs * formatting * feat: [cb_adf_explore] prog val print known label (VowpalWabbit#2961) * ci: Make LGTM ignore ext_libs directory (VowpalWabbit#2967) * test: Add tests for vw_math and fix one function (VowpalWabbit#2966) * test: Add tests for vw_math and fix one function * formatting * feat: add optional extra metrics (VowpalWabbit#2959) --extra_metrics filename Specify filename to write additional debug metrics to. Note: There is no fixed schema. Depends on specific reductions. related: VowpalWabbit#2805 * refactor: Remove unused foreach_feature func (VowpalWabbit#2973) * fix: fixes for headers not including things they need (VowpalWabbit#2974) * Fix includes * fix more headers * Fix more headers * formatting * undo changes * dont include on windows * formatting * Update mwt.h * mutex header * Fixing json parse issue when pdrop is 0 (VowpalWabbit#2977) Co-authored-by: olgavrou <olvrousg@microsoft.com> * fix: Avoid float to double promotion by using C++ version of math functions (VowpalWabbit#2979) * fix: use nullptr instead of NULL (VowpalWabbit#2980) * fix: Unify duplicated error code headers (VowpalWabbit#2978) * fix: Unify dulicated error code headers * formatting * refactor: Do not use cstyle casts (guideline 16) (VowpalWabbit#2981) * refactor: do not use c style casts * formatting * Add back cast * formatting * refactor: Support const iterators to feature groups (VowpalWabbit#2972) * Refactor feature iterators * Fix unique sort * fixup * Fix increments * Use ptr for audit() since it is optional * formatting * to_flatbuffer * fix: Add read_line_json_s function taking line length (VowpalWabbit#2968) This will be used in RLClientLib to enable the dotnet bindings to pass size information and fix a crash; the alternate fix would be to force a \0 terminator on the .NET side * Also deprecates un-sized call, as it safer to expect the caller to have run strlen, rather than assume it is safe to run. Co-authored-by: @jackgerrits * style: Give template types more descriptive names (VowpalWabbit#2984) * style: give template parameters more descriptive names * Formatting * Fix typos * update names * Fix formatting issues * build: [WIP] Enable building C# projects on Windows with CMake (VowpalWabbit#2929) * Set up .NET/CLI-compatible build environment on WIN32 * Flatten output structure (enable native dependencies to be copied SxS with managed) * Set up correct TargetFrameworkVersion * Enforce CMake version minimum when building .NET bindings * Unify WIN32-specific configuration in one CMake include * Get VW.Net Common library building under CMake * Get VW.Net managed C++ building under CMake * Get VW.Net managed wrapper building under CMake * Pull out NuGet package integration to CMake module * Get VW.Net JSON bindings building under CMake * Get VW.Net Parallel bindings building under CMake * Enable StrongName signing * Get VW.Net Console building under CMake * Get .NET UnitTests building under CMake * Integrate C# UnitTests into test_with_output * Fix issue with vw-unit-test CTest when output is redirected * Get VW.Net non-"friend" UnitTests building under CMake * Integrate C# non-"friend" UnitTests into test_with_output * Get .NET Simulator building under CMake * Update T4Template utility to run T4 tool on configure * This takes the generated sources of out the sourcetree * Update for PR comments * Normalize .cmake file casing * Clarify option naming (NETFX => NET_FRAMEWORK) * Spelling and whitespace fixes * Update target names to match pre-CMake targets * Fix warnings in search_generate.cc * Change Windows CMake pipeline to use VS generator * Fix formatting issues Co-authored-by: Jack Gerrits <jackgerrits@users.noreply.github.com> Co-authored-by: Jack Gerrits <jack@jackgerrits.com> * style: Use float instead of float ref (VowpalWabbit#2986) * perf: Use float instead of float ref * Fix formatting issues * Update skip * fix more merge issues * Fix win build issue Co-authored-by: Eduardo Salinas <edus@microsoft.com> Co-authored-by: Casey Irvine <44978150+cirvine-MSFT@users.noreply.github.com> Co-authored-by: olgavrou <olvrousg@microsoft.com> Co-authored-by: Jacob Alber <jacob.alber@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change ports the cs/ subdirectory to be configurable by CMake, and tested by CTest. It paves the way to reducing the number of build-systems by one, by standardizing on CMake, and simplifies native dependency resolution by unifying them in VcPkg / submodules on Windows.
Known remaining todos pre-merge:
Update nuget generation to support packing CMake-built assembliesin separate PR