[go: up one dir, main page]
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

[bit7z] new port #40889

Merged
merged 17 commits into from
Sep 24, 2024
Merged

[bit7z] new port #40889

merged 17 commits into from
Sep 24, 2024

Conversation

msclock
Copy link
Contributor
@msclock msclock commented Sep 10, 2024 •
edited by Cheney-W
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.

@msclock msclock marked this pull request as draft September 10, 2024 02:37
@msclock msclock marked this pull request as ready for review September 10, 2024 03:27
@Cheney-W Cheney-W added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Sep 10, 2024
@msclock msclock marked this pull request as draft September 10, 2024 07:09
@msclock
[bit7z] new port
0b39f0c 
Signed-off-by: msclock <msclock@qq.com>
@msclock msclock marked this pull request as ready for review September 10, 2024 08:55
@Cheney-W Cheney-W marked this pull request as draft September 10, 2024 10:58
@Cheney-W
Copy link
Contributor

Can these features of this port be enabled simultaneously?

msclock and others added 2 commits September 11, 2024 09:05
@msclock @Cheney-W
Update ports/bit7z/portfile.cmake
2af2436 
Co-authored-by: Cheney Wang <38240633+Cheney-W@users.noreply.github.com>
@msclock @Cheney-W
Update ports/bit7z/portfile.cmake
44b8a3c 
Co-authored-by: Cheney Wang <38240633+Cheney-W@users.noreply.github.com>
@msclock
Copy link
Contributor Author
msclock commented Sep 11, 2024 •
edited
Loading

Can these features of this port be enabled simultaneously?

@Cheney-W AFAIK, of cousre, all features in the vcpkg.json are from the bit7z build options

@msclock
Fix bit7z version
74d123a 
Signed-off-by: msclock <msclock@qq.com>
@msclock msclock marked this pull request as ready for review September 11, 2024 01:23
@msclock msclock marked this pull request as draft September 11, 2024 01:35
@msclock msclock marked this pull request as ready for review September 11, 2024 02:19
@dg0yt
Copy link
Contributor
dg0yt commented Sep 11, 2024 •
edited
Loading

Can these features of this port be enabled simultaneously?

all features in the vcpkg.json are from the bit7z build options

The origin is obvious, but this doesn't answer the question.

Some "features" belong to the triplet and toolchain: link-libcpp, static-runtime.

@msclock
Copy link
Contributor Author
msclock commented Sep 11, 2024 •
edited
Loading

Some "features" belong to the triplet and toolchain: link-libcpp, static-runtime.

@dg0yt Yep, it can customize ports in triplets or set flags in toolchains to do the same things. And it needs more comprehensive understand on the build options of bit7z. But for simplicity, I will remove them.

@msclock
Format
76833db 
Signed-off-by: msclock <msclock@qq.com>
@msclock
Fix versions
65a2bcd 
Signed-off-by: msclock <msclock@qq.com>
dg0yt
dg0yt suggested changes Sep 11, 2024
View reviewed changes
ports/bit7z/fix_install.patch Outdated Show resolved Hide resolved
ports/bit7z/fix_install.patch Outdated Show resolved Hide resolved
ports/bit7z/portfile.cmake Show resolved Hide resolved
ports/bit7z/vcpkg.json Outdated Show resolved Hide resolved
ports/bit7z/vcpkg.json Outdated Show resolved Hide resolved
ports/bit7z/vcpkg.json Outdated Show resolved Hide resolved
ports/bit7z/vcpkg.json Outdated Show resolved Hide resolved
ports/bit7z/fix_dependency.patch Outdated Show resolved Hide resolved
ports/bit7z/fix_dependency.patch Outdated Show resolved Hide resolved
@msclock msclock marked this pull request as draft September 12, 2024 01:12
@BillyONeal BillyONeal removed the info:reviewed Pull Request changes follow basic guidelines label Sep 19, 2024
@msclock msclock marked this pull request as draft September 20, 2024 03:20
@msclock
Readjust features and move its options to portfile
979ec3d 
Signed-off-by: msclock <msclock@qq.com>
@msclock
Fix versions
3ad6426 
Signed-off-by: msclock <msclock@qq.com>
@msclock msclock marked this pull request as ready for review September 23, 2024 00:56
@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label Sep 23, 2024
Cheney-W
Cheney-W previously approved these changes Sep 23, 2024
View reviewed changes
JavierMatosD
JavierMatosD requested changes Sep 23, 2024
View reviewed changes
Copy link
Contributor
@JavierMatosD JavierMatosD left a comment

Choose a reason for hiding this comment

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

LGTM with those nitpicks :)

Thank you for the contribution! Feel free to mark the PR "Ready for Review" once you have responded to my comments and/or made any relevant changes.

ports/bit7z/fix_dependency.patch Outdated Show resolved Hide resolved
ports/bit7z/fix_install.patch Outdated Show resolved Hide resolved
@JavierMatosD JavierMatosD marked this pull request as draft September 23, 2024 21:10
@msclock
Copy link
Contributor Author
msclock commented Sep 24, 2024

Can you explain why commenting our PUBLIC_HEADERS was needed?

@JavierMatosD I just make easiest changes I think, and are there better ways to handle it🤔

@msclock msclock marked this pull request as ready for review September 24, 2024 02:59
@msclock
Copy link
Contributor Author
msclock commented Sep 24, 2024

I have switched to set_target_properties with PUBLIC_HEADER to conform to upstream public headers. And it is more accurate in some way.

JavierMatosD
JavierMatosD requested changes Sep 24, 2024
View reviewed changes
ports/bit7z/fix_install.patch Outdated Show resolved Hide resolved
@JavierMatosD JavierMatosD marked this pull request as draft September 24, 2024 14:51
msclock and others added 2 commits September 25, 2024 00:38
@msclock @JavierMatosD
Update ports/bit7z/fix_install.patch
4e89746 
Co-authored-by: Javier Matos Denizac <javier.matosd@gmail.com>
@msclock
Fix patch and verions
5409218 
Signed-off-by: msclock <msclock@qq.com>
@msclock msclock marked this pull request as ready for review September 24, 2024 16:47
@JavierMatosD JavierMatosD merged commit 43b4ba9 into microsoft:master Sep 24, 2024
16 checks passed
@msclock msclock deleted the bit7z branch October 10, 2024 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JavierMatosD JavierMatosD JavierMatosD approved these changes

@rikyoz rikyoz rikyoz approved these changes

@Cheney-W Cheney-W Cheney-W left review comments

@dg0yt dg0yt Awaiting requested review from dg0yt

@BillyONeal BillyONeal Awaiting requested review from BillyONeal

Assignees

@Cheney-W Cheney-W

Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@msclock @Cheney-W @dg0yt @rikyoz @BillyONeal @JavierMatosD