[vowpal-wabbit] New port by jackgerrits · Pull Request #27323 · microsoft/vcpkg

jackgerrits · 2022-10-19T14:09:42Z

Describe the pull request

What does your PR fix?

This PR adds a port for the VowpalWabbit library
Which triplets are supported/not supported? Have you updated the [CI baseline]

I believe all should be supported except x86-windows. I am unsure about uwp though.
Does your PR follow the maintainer guide?

Yes
If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout ce99c947b64ea3c0e274e18cc3035ca727681089 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index be790ae..0dd4892 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7777,7 +7777,7 @@
       "port-version": 1
     },
     "vowpalwabbit": {
-      "baseline": "9.3.0",
+      "baseline": "9.5.0",
       "port-version": 0
     },
     "vs-yasm": {
diff --git a/versions/v-/vowpalwabbit.json b/versions/v-/vowpalwabbit.json
index deafc86..293d3c4 100644
--- a/versions/v-/vowpalwabbit.json
+++ b/versions/v-/vowpalwabbit.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "6b1205a5160128a40adf6714c5f9d6a38eaf1383",
+      "version": "9.5.0",
+      "port-version": 0
+    },
     {
       "git-tree": "6aac1bbc0bca774722db8ffd2b2b29c43676b5eb",
       "version": "9.3.0",

jackgerrits · 2022-10-19T14:19:33Z

I went ahead and marked arm64-windows as fail in the ci baseline file. If it's possible to see the log produced maybe it's an easy fix? I don't have access to a machine to test that configuration unfortunately though

LilyWangLL · 2022-10-20T06:57:10Z

scripts/ci.baseline.txt

 vectorclass:arm64-windows=fail
 vectorclass:arm-uwp=fail
+vowpalwabbit:arm64-windows=fail
+vowpalwabbit:x86-windows=fail


If this port doesn't support x86-windows, please add supports in vcpkg.json and remove this code from ci.baseline.txt.

LilyWangLL · 2022-10-20T07:33:58Z

Due to boost-random:x64-uwp build failed, so the uwp status of boost-math and this port on CI pipeline is cascade.

LilyWangLL · 2022-10-20T07:41:28Z

scripts/ci.baseline.txt

 v8:x64-uwp=fail
 vectorclass:arm64-windows=fail
 vectorclass:arm-uwp=fail
+vowpalwabbit:arm64-windows=fail


There are build error when build vowpalwabbit:arm64-windows, this port doesn't support arm64 too.
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.33.31629\include\xmmintrin.h(27): fatal error C1189: #error: This header is specific to X86, X64 and ARM64EC targets

Marked arm+windows unsupported

jackgerrits · 2022-10-20T13:07:53Z

Thanks for the comments! I have marked the port as unsupported for x86, uwp and windows+arm.

BillyONeal

This is great, thank you! Can you check/confirm/fix the usage text? Then this is ready to merge!

vowpalwabbit

New port checklist:

Review the code ✅
Check the name against https://repology.org/ ⚠️
No match, but there are Python and R bindings that appear to match.
Check the name against Bing/Google. (If the first results aren't C++ related, try "name C++")
First hit ✅
Check the source code for optional find_packages
GTest ✅, Threads ✅, Boost ✅, fmt ✅, spdlog ✅, RapidJson ✅, Boost ✅, Zlib ✅, Flatbuffers (Optional, but in vcpkg.json)
JNI, Java skipped by BUILD_JAVA == off by default
Boost, PythonXxx skipped by BUILD_PYTHON == off by default
Check that the versioning scheme matches what upstream says ✅
Check that the license in vcpkg.json matches what upstream says ✅
Check that the license installed by the port matches what upstream says ✅
Check that the source code comes from the upstream project's authoritative source. ✅
Check that the generated usage is accurate.

vowpalwabbit provides CMake targets:

    # this is heuristically generated, and may not be correct
    find_package(VowpalWabbit CONFIG REQUIRED)
    # note: 4 additional targets are not displayed.
    target_link_libraries(main PRIVATE VowpalWabbit::vw_io VowpalWabbit::vw_core VowpalWabbit::vw_common VowpalWabbit::vw_config)

This seems not right due to the "4 additional targets skipped" ? I translated the heuristically generated result to a file for you, can you confirm what a user is supposed to do there? Thanks!

Check for issues that are resolved. ✅

BillyONeal · 2022-10-22T04:28:50Z

(Blank usage would also be OK if that's what's appropriate)

jackgerrits · 2022-10-22T17:17:49Z

Thanks for the review!

Check the name against https://repology.org/ ⚠️
No match, but there are Python and R bindings that appear to match.

Thanks for checking this. You are right, the correct name in Repology is vowpal-wabbit. I will go ahead and update the port name to vowpal-wabbit to match.

Check that the generated usage is accurate.

It looks generally right, but I would like to go in and confirm myself what the 4 additional targets are prior to merging and remove any that potentially don't make sense.

I'll mention you here when I've resolved these two things.

jackgerrits · 2022-10-24T13:29:31Z

@BillyONeal I believe I've addressed the comments. A couple of the extra generated targets were for bins (that were previously deleted in the portfile), I added a patch to the port and created an issue in the upstream to address this (VowpalWabbit/vowpal_wabbit#4244).

I updated the usage file to explain the most common use case, those other targets are dependencies of the main one and are supporting libs.

BillyONeal · 2022-10-24T18:35:06Z

/azp run

azure-pipelines · 2022-10-24T18:35:15Z

Azure Pipelines successfully started running 1 pipeline(s).

BillyONeal · 2022-10-24T18:46:23Z

Thanks for the new port!

@dg0yt

@BillyONeal

* When @BillyONeal started being the on-call vcpkg maintainer on 2022-10-17, he started applying use of the "embedded VERSION" feature microsoft/vcpkg-tool#717 to PRs on merge. @dg0yt points out that this use should be accompanied by a call to vcpkg_minimum_required, in https://github.com/microsoft/vcpkg/pull/27594/files#r1010641672 This is an audit of everything merged in that time and whether it needs to gain that. #27561 No VERSION #27525 No VERSION #27554 Already has vcpkg_minimum_required #27536 No VERSION #27562 No VERSION #24914 Fixed here #27547 No VERSION #27502 No VERSION #27497 No VERSION #27317 No VERSION #27288 No VERSION #27509 No VERSION #27504 No VERSION #27514 No VERSION #27501 No VERSION #27495 No VERSION #27488 No VERSION #27499 No VERSION #27378 No VERSION #27376 Fixed here #27373 No VERSION #27045 No VERSION #27521 No VERSION #27453 No VERSION #27481 No VERSION #27511 No VERSION #27490 No VERSION #27510 No VERSION #27496 No VERSION #27503 No VERSION #27485 No VERSION #27484 No VERSION #27483 No VERSION #27459 No VERSION #27369 No VERSION #27489 No VERSION #26594 No VERSION #27465 No VERSION #27456 No VERSION #27425 No VERSION #27464 Fixed here #27406 No VERSION #27398 No VERSION #27240 No VERSION #27450 No VERSION #27463 No VERSION #27462 No VERSION #27448 No VERSION #27440 No VERSION #27435 No VERSION #27424 No VERSION #27414 No VERSION #27412 No VERSION #27380 No VERSION #27343 No VERSION #27342 No VERSION #27367 No VERSION #27226 No VERSION #27320 No VERSION #26923 No VERSION #27284 No VERSION #27433 No VERSION #27314 VERSION got *removed* #27335 No VERSION #27370 No VERSION #27324 No VERSION #27391 No VERSION #27388 No VERSION #27396 No VERSION #27404 No VERSION #27413 No VERSION #27417 No VERSION #27427 No VERSION #27428 No VERSION #27368 No VERSION #27307 No VERSION #27415 Fixed here. #27371 Fixed here. #27323 No VERSION #27352 No VERSION #27347 No VERSION #27366 No VERSION #27361 No VERSION #27359 No VERSION #27358 No VERSION #27355 No VERSION #27331 No VERSION #24615 No VERSION #27325 No VERSION #24861 No VERSION #27354 No VERSION #27346 No VERSION #27345 No VERSION #27218 No VERSION #27329 No VERSION #27326 No VERSION #27321 No VERSION #27312 No VERSION #27297 No VERSION #27336 No VERSION #27225 No VERSION #27339 No VERSION #27302 No VERSION #27295 No VERSION #27233 No VERSION #27313 No VERSION #27237 No VERSION #27250 No VERSION #27263 No VERSION #27266 No VERSION #27272 No VERSION #27287 No VERSION #27282 No VERSION #27294 No VERSION #27228 No VERSION #27163 No VERSION #26817 No VERSION #27286 No VERSION #27274 No VERSION #27276 No VERSION #27232 No VERSION #27221 No VERSION #27215 No VERSION #27166 No VERSION #27239 No VERSION #27246 No VERSION #27268 No VERSION #27259 No VERSION #27238 No VERSION #27224 No VERSION #27203 No VERSION #27124 No VERSION * Also add libcanberra

jackgerrits added 3 commits October 19, 2022 09:58

add vowpalwabbit port

28230a1

update baseline

f3d44f2

update version

c1271ce

github-actions bot requested changes Oct 19, 2022

View reviewed changes

update baseline after version change

20760f7

github-actions bot previously approved these changes Oct 19, 2022

View reviewed changes

mark arm64-windows as fail

e2acef4

jackgerrits dismissed github-actions[bot]’s stale review via e2acef4 October 19, 2022 14:18

github-actions bot previously approved these changes Oct 19, 2022

View reviewed changes

Update vowpalwabbit.json

c007269

jackgerrits dismissed github-actions[bot]’s stale review via c007269 October 19, 2022 14:26

github-actions bot previously approved these changes Oct 19, 2022

View reviewed changes

Update ci.baseline.txt

f577c3a

jackgerrits dismissed github-actions[bot]’s stale review via f577c3a October 19, 2022 14:27

Update ci.baseline.txt

27208a7

github-actions bot approved these changes Oct 19, 2022

View reviewed changes

github-actions bot previously approved these changes Oct 19, 2022

View reviewed changes

LilyWangLL self-assigned this Oct 20, 2022

LilyWangLL added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Oct 20, 2022

LilyWangLL reviewed Oct 20, 2022

View reviewed changes

LilyWangLL added the requires:author-response label Oct 20, 2022

LilyWangLL reviewed Oct 20, 2022

View reviewed changes

jackgerrits added 2 commits October 20, 2022 09:06

update supports field in manifest

684ba5b

run x-add-version

074871b

jackgerrits dismissed github-actions[bot]’s stale review via 074871b October 20, 2022 13:07

github-actions bot previously approved these changes Oct 20, 2022

View reviewed changes

Update ci.baseline.txt

9dba96d

LilyWangLL added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 21, 2022

BillyONeal added 2 commits October 21, 2022 21:03

Merge remote-tracking branch 'origin/master' into HEAD

9b6894c

Add usage and nitpick quotes.

64f2791

BillyONeal dismissed stale reviews from github-actions[bot] and LilyWangLL via 64f2791 October 22, 2022 04:23

BillyONeal requested changes Oct 22, 2022

View reviewed changes

BillyONeal added requires:author-response and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Oct 22, 2022

github-actions bot previously approved these changes Oct 22, 2022

View reviewed changes

jackgerrits added 2 commits October 24, 2022 09:24

rename port to vowpal-wabbit and add patch to remove bin targets

10b9efb

update versions, rename port

427e96d

jackgerrits dismissed github-actions[bot]’s stale review via 427e96d October 24, 2022 13:25

github-actions bot approved these changes Oct 24, 2022

View reviewed changes

jackgerrits mentioned this pull request Oct 24, 2022

Make it possible to build just library and not executables VowpalWabbit/vowpal_wabbit#4244

Closed

BillyONeal changed the title ~~[vowpalwabbit] New port~~ [vowpal-wabbit] New port Oct 24, 2022

BillyONeal added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Oct 24, 2022

BillyONeal approved these changes Oct 24, 2022

View reviewed changes

BillyONeal merged commit 0c7c52d into microsoft:master Oct 24, 2022

autoantwort mentioned this pull request Oct 27, 2022

[vowpal-wabbit] no absolute paths #27490

Merged

jackgerrits mentioned this pull request Oct 30, 2022

Create VCPKG package for VowpalWabbit VowpalWabbit/vowpal_wabbit#2228

Closed

BillyONeal mentioned this pull request Nov 1, 2022

Add vcpkg_minimum_required before using VERSION. #27598

Merged

Conversation

jackgerrits commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does your PR fix?

Which triplets are supported/not supported? Have you updated the [CI baseline]

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

jackgerrits commented Oct 19, 2022

Uh oh!

LilyWangLL Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

jackgerrits Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

LilyWangLL commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LilyWangLL Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

jackgerrits Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

jackgerrits commented Oct 20, 2022

Uh oh!

BillyONeal left a comment

Choose a reason for hiding this comment

Uh oh!

BillyONeal commented Oct 22, 2022

Uh oh!

jackgerrits commented Oct 22, 2022

Uh oh!

jackgerrits commented Oct 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BillyONeal commented Oct 24, 2022

Uh oh!

azure-pipelines bot commented Oct 24, 2022

Uh oh!

BillyONeal commented Oct 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jackgerrits commented Oct 19, 2022 •

edited

Loading

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

LilyWangLL commented Oct 20, 2022 •

edited

Loading

jackgerrits commented Oct 24, 2022 •

edited

Loading