Skip to content

[foxi] create a new port#21786

Merged
BillyONeal merged 6 commits intomicrosoft:masterfrom
luncliff:port/foxi
Dec 3, 2021
Merged

[foxi] create a new port#21786
BillyONeal merged 6 commits intomicrosoft:masterfrom
luncliff:port/foxi

Conversation

@luncliff
Copy link
Copy Markdown
Contributor

@luncliff luncliff commented Dec 1, 2021 •
edited
Loading

What does your PR fix?

Build & install foxi_loader target for #17199 (comment)
Referenced PyTorch's Bazel config for https://github.com/houseroad/foxi.

third_party/foxi.BUILD is like the following.

load("@rules_cc//cc:defs.bzl", "cc_library")

cc_library(
    name = "foxi",
    srcs = [
        "foxi/onnxifi_loader.c",
    ],
    hdrs = glob([
        "foxi/*.h",
    ]),
    includes = [
        ".",
    ],
    linkstatic = 1,
    visibility = ["//visibility:public"],
)

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

The source is quite simple. Targets the following 2 triplets, but windows triplets will be able to build this.

  • x64-linux
  • x64-osx

Does your PR follow the maintainer guide?

I think this project is abnormal case. It contains multiple build targets, but they are not used.
#17199 (comment)

We can consider nesting the source files into the buildtree of #17199 and pick only what it needs.
But doing so requires 2 times of vcpkg_from_github.

# ports/libtorch/portfile.cmake ...

vcpkg_from_github(
    OUT_SOURCE_PATH SOURCE_PATH
    REPO pytorch/pytorch
    REF v1.10.0
    SHA512 92b70e6170a7f173c4a9cb29f6cec6dfa598587aa9cf6a620ec861b95da6ea555cbc7285914c0dab6cfc8af320fad4999be4a788acc1f15140664a67ad9dc35d
    HEAD_REF master
    PATCHES
        fix-cmake.patch
        fix-sources.patch
)

# All we need is `foxi_loader.c` and several header files. Nest them in the libtorch?
vcpkg_from_github(
    OUT_SOURCE_PATH FOXI_SOURCE_PATH
    REPO houseroad/foxi
    REF c278588e34e535f0bb8f00df3880d26928038cad
    SHA512 ad42cfd70e40ba0f0a9187b34ae9e3bd361c8c0038669f4c1591c4f7421d12ad93f76f42b33c2575eea1a3ddb3ff781da2895cdc636df5b60422598f450203c7
    PATCHES
        fix-cmakelists.patch
)
file(REMOVE_RECURSE "${SOURCE_PATH}/third_party/foxi")
file(COPY "${FOXI_SOURCE_PATH}" DESTINATION "${SOURCE_PATH}/third_party/foxi")

But someday in the future, the https://github.com/houseroad/foxi might be updated, so I'd suggest creating this tiny port.

@luncliff luncliff mentioned this pull request Dec 1, 2021
Merged
19 tasks
@JonLiu1993 JonLiu1993 self-assigned this Dec 1, 2021
@JonLiu1993 JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Dec 1, 2021
@luncliff
[foxi] disable UWP because of build failure
88530f9 
* LOAD_LIBRARY_SEARCH_DEFAULT_DIRS undefined
@JonLiu1993 JonLiu1993 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Dec 1, 2021
@BillyONeal BillyONeal merged commit 2647956 into microsoft:master Dec 3, 2021
@BillyONeal
Copy link
Copy Markdown
Member

BillyONeal commented Dec 3, 2021

Thanks!

@luncliff luncliff deleted the port/foxi branch December 21, 2021 18:11
luncliff added a commit to luncliff/vcpkg-registry that referenced this pull request Dec 21, 2021
@luncliff
[libtorch] update patch/dependency
0d5d42c 
* use foxi from microsoft/vcpkg#21786
* brought patches from microsoft/vcpkg#17199
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BillyONeal BillyONeal BillyONeal approved these changes

@github-actions github-actions[bot] github-actions[bot] approved these changes

+1 more reviewer

@JonLiu1993 JonLiu1993 JonLiu1993 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JonLiu1993 JonLiu1993

Labels

category:new-port The issue is requesting a new library to be added; consider making a PR! info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@luncliff @BillyONeal @JonLiu1993