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

[cld3] New Port #33312

Merged
merged 39 commits into from
Jan 25, 2024
Merged

[cld3] New Port #33312

merged 39 commits into from
Jan 25, 2024

Conversation

rozdaybeda
Copy link
Contributor

@rozdaybeda rozdaybeda commented Aug 22, 2023 •
edited
Loading

  • Changes comply with the maintainer guide
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@LilyWangLL LilyWangLL self-assigned this Aug 23, 2023
@LilyWangLL LilyWangLL added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Aug 23, 2023
@LilyWangLL LilyWangLL marked this pull request as draft August 25, 2023 07:01
@LilyWangLL
Copy link
Contributor

Convert this PR to draft since there are CI pipeline errors need to fix. Please ping us if this PR is ready for review again.

@LilyWangLL
Copy link
Contributor

Pinging @rozdaybeda for response. Is work still being done for this PR?

@rozdaybeda rozdaybeda marked this pull request as ready for review September 9, 2023 20:15
LilyWangLL
LilyWangLL requested changes Sep 11, 2023
View reviewed changes
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
LilyWangLL
LilyWangLL requested changes Sep 11, 2023
View reviewed changes
ports/cld3/vcpkg.json Show resolved Hide resolved
ports/cld3/portfile.cmake Show resolved Hide resolved
LilyWangLL
LilyWangLL previously approved these changes Sep 12, 2023
View reviewed changes
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Sep 12, 2023
dg0yt
dg0yt reviewed Sep 12, 2023
View reviewed changes
ports/cld3/fix-install.patch Outdated Show resolved Hide resolved
ports/cld3/portfile.cmake Outdated Show resolved Hide resolved
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
@LilyWangLL LilyWangLL removed the info:reviewed Pull Request changes follow basic guidelines label Sep 12, 2023
dg0yt
dg0yt reviewed Sep 13, 2023
View reviewed changes
ports/cld3/portfile.cmake Outdated Show resolved Hide resolved
ports/cld3/fix-install.patch Outdated Show resolved Hide resolved
@rozdaybeda rozdaybeda marked this pull request as draft September 14, 2023 11:27
@rozdaybeda rozdaybeda marked this pull request as ready for review September 15, 2023 08:10
@rozdaybeda rozdaybeda marked this pull request as draft September 15, 2023 08:12
@rozdaybeda rozdaybeda marked this pull request as ready for review September 15, 2023 12:50
LilyWangLL
LilyWangLL requested changes Sep 28, 2023
View reviewed changes
ports/cld3/portfile.cmake Outdated Show resolved Hide resolved
LilyWangLL
LilyWangLL requested changes Oct 8, 2023
View reviewed changes
ports/cld3/vcpkg.json Outdated Show resolved Hide resolved
@LilyWangLL
Copy link
Contributor

https://github.com/google/cld3/blob/b48dc46512566f5a2d41118c8c1116c4f96dc661/CMakeLists.txt#L61-L69

Are the following tools useful?
image

@rozdaybeda
Copy link
Contributor Author

https://github.com/google/cld3/blob/b48dc46512566f5a2d41118c8c1116c4f96dc661/CMakeLists.txt#L61-L69

Are the following tools useful? image

Not really, remove them from the build?

@LilyWangLL
Copy link
Contributor

Not really, remove them from the build?

These tools are installed to the temp folder, I just want confirm do these tools need to installed to packages and installed folder.

@rozdaybeda
Copy link
Contributor Author

Not really, remove them from the build?

These tools are installed to the temp folder, I just want confirm do these tools need to installed to packages and installed folder.

I don't think they are needed

LilyWangLL
LilyWangLL previously approved these changes Oct 10, 2023
View reviewed changes
@rozdaybeda rozdaybeda marked this pull request as ready for review November 16, 2023 09:52
@data-queue
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@data-queue
Copy link
Contributor

I'm not sure why the license/cla check is still waiting, when it was passing before

@dg0yt
Copy link
Contributor

dg0yt commented Nov 18, 2023

I'm not sure why the license/cla check is still waiting, when it was passing before

It is not AZP but GHA. Close+reopen might re-trigger it.

dg0yt
dg0yt suggested changes Nov 18, 2023
View reviewed changes
ports/cld3/portfile.cmake Outdated Show resolved Hide resolved
ports/cld3/portfile.cmake Outdated Show resolved Hide resolved
ports/cld3/fix-build.patch Outdated Show resolved Hide resolved
ports/cld3/portfile.cmake Outdated Show resolved Hide resolved
@LilyWangLL LilyWangLL removed the info:reviewed Pull Request changes follow basic guidelines label Nov 20, 2023
@LilyWangLL LilyWangLL marked this pull request as draft November 20, 2023 08:02
@rozdaybeda rozdaybeda marked this pull request as ready for review November 25, 2023 17:23
@LilyWangLL LilyWangLL marked this pull request as draft December 15, 2023 06:23
@BillyONeal
Copy link
Member

/azp run

@BillyONeal
Copy link
Member

Sorry I broke the build lab Sunday night

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rozdaybeda rozdaybeda marked this pull request as ready for review December 27, 2023 23:11
@JavierMatosD JavierMatosD merged commit fd675b7 into microsoft:master Jan 25, 2024
15 checks passed
@dg0yt
Copy link
Contributor

dg0yt commented Jan 26, 2024

@JavierMatosD This new port doesn't meet the guidelines: It adds CMake config which is not from upstream (AFAICS), but which lacks the unofficial prefix. I thought this would be checked by the official reviewers? This should be fixed immediately.

@JavierMatosD
Copy link
Contributor

@JavierMatosD This new port doesn't meet the guidelines: It adds CMake config which is not from upstream (AFAICS), but which lacks the unofficial prefix. I thought this would be checked by the official reviewers? This should be fixed immediately.

You are correct. I missed that. I'll open a PR with the fix shortly. Thanks for the heads up!

@JavierMatosD JavierMatosD mentioned this pull request Jan 26, 2024
Merged
TomKatom pushed a commit to TomKatom/vcpkg that referenced this pull request Feb 23, 2024
@rozdaybeda @LilyWangLL @TomKatom
[cld3] New Port (microsoft#33312)
5132ce2 
* [cld3] new port

* [cld3] fix port version

* update a path to the license

* use vcpkg_install_cmake

* remove deprecated functions

* disable arm test

* update sha

* ci.baseline.txt

* Update scripts/ci.baseline.txt

Co-authored-by: Lily Wang <[email protected]>

* Update ports/cld3/vcpkg.json

Co-authored-by: Lily Wang <[email protected]>

* Update ports/cld3/portfile.cmake

Co-authored-by: Lily Wang <[email protected]>

* fix version

* scripts/ci.baseline.txt

* update patch

* c++ 11 version

* disable 64-osx

* more patches

* fix linux/Win32 static only

* anable other platforms

* fix build

* x64-osx

* ci.baseline.txt

* Revert "ci.baseline.txt"

This reverts commit ade1499.

* Update ports/cld3/portfile.cmake

Co-authored-by: Lily Wang <[email protected]>

* fix version

* remove unused vcpkg-cmake-config

* move install headers to portfile

* remove ABI version

* add LF to ci.baseline.txt

* scripts/ci.baseline.txt

* disable unit tests

* fix

* move COMPILER_MSVC to port file

* fix version

* add line break

* refactoring

* uncomment -D_GLIBCXX_USE_CXX11_ABI=0

* add cld3Config.cmake.in

* improve cld3Config.cmake.in

---------

Co-authored-by: Alexander ROZDAYBEDA <[email protected]>
Co-authored-by: Lily Wang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BillyONeal BillyONeal BillyONeal left review comments

@JavierMatosD JavierMatosD JavierMatosD left review comments

@LilyWangLL LilyWangLL LilyWangLL left review comments

@data-queue data-queue data-queue left review comments

@dg0yt dg0yt dg0yt requested changes

Assignees

@LilyWangLL LilyWangLL

Labels
category:new-port The issue is requesting a new library to be added; consider making a PR!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rozdaybeda @LilyWangLL @BillyONeal @data-queue @dg0yt @JavierMatosD