Skip to content

[cpuinfo,fbgemm,nnpack] update to latest source version and rename targets#17063

Merged
strega-nil-ms merged 13 commits intomicrosoft:masterfrom
luncliff:port/cpuinfo
May 5, 2021
Merged

[cpuinfo,fbgemm,nnpack] update to latest source version and rename targets#17063
strega-nil-ms merged 13 commits intomicrosoft:masterfrom
luncliff:port/cpuinfo

Conversation

@luncliff
Copy link
Copy Markdown
Contributor

@luncliff luncliff commented Apr 3, 2021 •
edited
Loading

What does your PR fix?

The previous patch cpuinfo-pull-22-868bd11.patch, which is from pytorch/cpuinfo#22 failed to apply.

Starting package 1/1: cpuinfo:x64-osx
Building package cpuinfo[core]:x64-osx...
-- Using cached /Users/user/vcpkg/downloads/cpuinfo-pull-22-868bd11.patch
-- Using cached /Users/user/vcpkg/downloads/pytorch-cpuinfo-5916273f79a21551890fd3d56fc5375a78d1598d.tar.gz
-- Extracting source /Users/user/vcpkg/downloads/pytorch-cpuinfo-5916273f79a21551890fd3d56fc5375a78d1598d.tar.gz
-- Applying patch /Users/user/vcpkg/downloads/cpuinfo-pull-22-868bd11.patch
CMake Error at scripts/cmake/z_vcpkg_apply_patches.cmake:57 (message):
  Applying patch failed: Checking patch CMakeLists.txt...

  error: while searching for:

  OPTION(CPUINFO_BUILD_MOCK_TESTS "Build cpuinfo mock tests" ON)

  OPTION(CPUINFO_BUILD_BENCHMARKS "Build cpuinfo micro-benchmarks" ON)

  

  # ---[ CMake options

  IF(CPUINFO_BUILD_UNIT_TESTS OR CPUINFO_BUILD_MOCK_TESTS)

    ENABLE_TESTING()

  

  error: patch failed: CMakeLists.txt:17

  error: CMakeLists.txt: patch does not apply

  Checking patch cmake/Config.cmake.in...

  Checking patch deps/clog/CMakeLists.txt...

Call Stack (most recent call first):
  scripts/cmake/vcpkg_extract_source_archive_ex.cmake:144 (z_vcpkg_apply_patches)
  scripts/cmake/vcpkg_from_github.cmake:154 (vcpkg_extract_source_archive_ex)
  ports/cpuinfo/portfile.cmake:11 (vcpkg_from_github)
  scripts/ports.cmake:142 (include)

Picked those differences and created a new patch file.
What a sad that PR is not merged yet ...

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

No change in triplet support. The current build check must be passed.

Target namespace changed

The package cpuinfo:... provides CMake targets:

    find_package(unofficial-cpuinfo CONFIG REQUIRED)
    target_link_libraries(main PRIVATE unofficial::cpuinfo::clog unofficial::cpuinfo::cpuinfo)

Does your PR follow the maintainer guide?

I think so.

@luncliff luncliff mentioned this pull request Apr 3, 2021
Merged
@luncliff luncliff changed the title [cpuinfo] update port to latest [cpuinfo] update to latest source version Apr 3, 2021
@JackBoosY JackBoosY self-assigned this Apr 5, 2021
@JackBoosY JackBoosY added the category:port-update The issue is with a library, which is requesting update new revision label Apr 5, 2021
JackBoosY
JackBoosY suggested changes Apr 7, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Since this is no the upstream changes, we should add prefix unofficial-.

@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Apr 7, 2021

Cool. There are some ports I worked to use this, but let me fix them after this

luncliff and others added 4 commits April 7, 2021 11:17
@luncliff @JackBoosY
Update ports/cpuinfo/fix-install.patch
6e91ea5 
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
@luncliff @JackBoosY
Update ports/cpuinfo/fix-install.patch
d8817d8 
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
@luncliff @JackBoosY
Update ports/cpuinfo/portfile.cmake
fd45b97 
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
@luncliff @JackBoosY
Update ports/cpuinfo/fix-install.patch
be6c706 
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Apr 7, 2021

I think you should remake the patch file.

@luncliff luncliff mentioned this pull request Apr 10, 2021
Merged
luncliff added 4 commits April 10, 2021 17:57
@luncliff
[cpuinfo] update port SHA
8dc3694
@luncliff
[cpuinfo] recover find_package
925dbab 
* fixup target path to `share/unofficial-cpuinfo`
* change IMPORTED target name to `unofficial::cpuinfo::cpuinfo`
@luncliff
[fbgemm] resolve failure after cpuinfo rename
5c3b233 
* change cpuinfo::cpuinfo to unofficial
@luncliff
[fbgemm] update port version
b76d735
@luncliff luncliff changed the title [cpuinfo] update to latest source version [cpuinfo,fbgemm] update to latest source version and rename targets Apr 10, 2021
@luncliff
Copy link
Copy Markdown
Contributor Author

luncliff commented Apr 10, 2021 •
edited
Loading

bcb6171: Merged master to resolve possible conflicts with #16344

@luncliff luncliff changed the title [cpuinfo,fbgemm] update to latest source version and rename targets [cpuinfo,fbgemm,nnpack] update to latest source version and rename targets Apr 10, 2021
@luncliff luncliff mentioned this pull request Apr 10, 2021
Merged
19 tasks
@JackBoosY JackBoosY 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 Apr 11, 2021
@strega-nil-ms
Copy link
Copy Markdown
Contributor

strega-nil-ms commented Apr 30, 2021

I think that that's incorrect; they're just corrections of upstream targets, so they should be under the upstream name, not unofficial-; @JackBoosY do you disagree?

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented May 2, 2021

@strega-nil-ms The upstream doesn't export any targets in the master branch, did I miss something?

@strega-nil-ms
Copy link
Copy Markdown
Contributor

strega-nil-ms commented May 5, 2021

@JackBoosY oh... yeah you're right, I totally misread that, you're right.

@strega-nil-ms strega-nil-ms merged commit 4ef97c2 into microsoft:master May 5, 2021
@strega-nil-ms
Copy link
Copy Markdown
Contributor

strega-nil-ms commented May 5, 2021

Thanks @luncliff :)

@luncliff luncliff deleted the port/cpuinfo branch May 5, 2021 20:52
@luncliff luncliff mentioned this pull request Apr 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@JackBoosY JackBoosY JackBoosY approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JackBoosY JackBoosY

Labels

category:port-update The issue is with a library, which is requesting update new revision 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 @JackBoosY @strega-nil-ms