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.

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

[Fastor] add new port Fastor #16587

Merged
merged 5 commits into from
Nov 26, 2021
Merged

Conversation

duanqn
Copy link
Contributor

@duanqn duanqn commented Mar 8, 2021

Describe the pull request

  • What does your PR fix? Fixes [New Port Request] Fastor #16532

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    It is a header-only library. I'm assuming it supports all triplets. CI baseline has been updated.

  • Does your PR follow the maintainer guide?
    Yes.

@Hoikas
Copy link
Contributor

Hoikas commented Mar 8, 2021

Since the upstream port provides a cmake config, IMO you should install it with cmake and fixup the targets.

HEAD_REF master
)

file(COPY ${SOURCE_PATH}/Fastor DESTINATION ${CURRENT_PACKAGES_DIR}/include/${PORT})
Copy link
Contributor

Choose a reason for hiding this comment

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

There is also CMakeLists.txt in the source code. So it would be better to use vcpkg_configure_cmake() and vcpkg_install_cmake() to build and install this port.

@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Mar 8, 2021
@NancyLi1013 NancyLi1013 self-assigned this Mar 8, 2021
@duanqn
Copy link
Contributor Author

duanqn commented Mar 8, 2021

Hi @Hoikas and @NancyLi1013,
I'm aware that Fastor supports CMake on master branch, but the latest release tag (V0.6.3) does not have CMake. That's why I can't use CMake there.
Any suggestions on how to solve this?

@Hoikas
Copy link
Contributor

Hoikas commented Mar 8, 2021

Hmm... Considering how helpful cmake targets are, it might be best to use the latest commit and set the port's version to today's date.

@duanqn
Copy link
Contributor Author

duanqn commented Mar 12, 2021

Is it possible to port the package without cmake for now? When that repo publishes a new tag we can write a new port with cmake.

@NancyLi1013
Copy link
Contributor

@duanqn

I agree with the proposal from @Hoikas. You can use the latest commit until the next tag is released which contains CMakeLists.txt.

@NancyLi1013 NancyLi1013 added the depends:upstream-changes Waiting on a change to the upstream project label Apr 16, 2021
@NancyLi1013
Copy link
Contributor

Convert to draft until the upstream changes. Please ping me if it is ready for review.

@NancyLi1013 NancyLi1013 marked this pull request as draft July 20, 2021 09:02
@NancyLi1013
Copy link
Contributor

Hi @duanqn

Could you please update to the latest commit that contains CMakeLists.txt for this PR?

@PhoebeHui PhoebeHui assigned JonLiu1993 and unassigned NancyLi1013 Nov 22, 2021
@JonLiu1993
Copy link
Member

@duanqn , Is this PR being worked on? Could you update pr based on nancy's suggestion?

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for fastor but no changes to version or port version.
-- Version: 0.6.3
-- Old SHA: ea69d44e4460ca4392581532c10e519c033fa3e1
-- New SHA: f9bce91121c3bf19d7b66bdef894cc5d5029e408
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

  • ports/fastor/portfile.cmake

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for fastor but no changes to version or port version.
-- Version: 2021-11-22
-- Old SHA: 5ba530e20d9adca2f9cda32b6685d8ee66242650
-- New SHA: 13267b1928216a11e9d09fc497290df5b5691144
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for fastor but no changes to version or port version.
-- Version: 2021-11-22
-- Old SHA: 5ba530e20d9adca2f9cda32b6685d8ee66242650
-- New SHA: 24b3fc4241eb82555225352893d2bd9dc14e17a1
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@duanqn duanqn marked this pull request as ready for review November 22, 2021 11:04
@duanqn
Copy link
Contributor Author

duanqn commented Nov 22, 2021

@JonLiu1993 Thanks for notifying me. Please see if the current port meets the requirement.

ports/fastor/portfile.cmake Outdated Show resolved Hide resolved
ports/fastor/portfile.cmake Outdated Show resolved Hide resolved
ports/fastor/portfile.cmake Outdated Show resolved Hide resolved
@JonLiu1993 JonLiu1993 removed the depends:upstream-changes Waiting on a change to the upstream project label Nov 23, 2021
Co-authored-by: JonLiu1993 <[email protected]>
duanqn and others added 2 commits November 23, 2021 15:08
Co-authored-by: JonLiu1993 <[email protected]>
Co-authored-by: JonLiu1993 <[email protected]>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for fastor but no changes to version or port version.
-- Version: 2021-11-22
-- Old SHA: 24b3fc4241eb82555225352893d2bd9dc14e17a1
-- New SHA: b01ca4e019955e95abc9338e07aaa076d989c8d3
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for fastor but no changes to version or port version.
-- Version: 2021-11-22
-- Old SHA: 24b3fc4241eb82555225352893d2bd9dc14e17a1
-- New SHA: 6437d1dc343d2b061f0c6414fce30b72b4e1e889
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for fastor but no changes to version or port version.
-- Version: 2021-11-22
-- Old SHA: 24b3fc4241eb82555225352893d2bd9dc14e17a1
-- New SHA: 9db673c173f5c27fc113f35837767d9d815e2976
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@JonLiu1993 JonLiu1993 added info:reviewed Pull Request changes follow basic guidelines and removed info:reviewed Pull Request changes follow basic guidelines labels Nov 23, 2021
@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Nov 25, 2021
@BillyONeal BillyONeal merged commit 2729e6e into microsoft:master Nov 26, 2021
@BillyONeal
Copy link
Member

Thanks for the new port!

@duanqn duanqn deleted the duanqn/fastor branch June 22, 2022 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

[New Port Request] Fastor
5 participants