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

[Libtomcrypt, libtommath] Add new port #8787

Closed
wants to merge 4 commits into from

Conversation

FredeJ
Copy link
Contributor

@FredeJ FredeJ commented Oct 29, 2019

I added libtommath and libtomcrypt.

I tried updating the solution file to be a newer version so it could be build with the vcpkg_build_msbuild command. However I was advised to use the existing makefile.msvc instead using nmake ( see libtom/libtomcrypt#517 ).

I made it build by inelegantly including ${CURRENT_PACKAGES_DIR}/.. - what is the solution for elegantly including the currently built libraries?

I ran into some issues with the approach, namely building a debug version (still not sure I'm doing that right) and building for x64 (unsolved as of yet). Additionally, I'm not completely sure how to go about building for linux and filtering correctly on the triplets.

Instead of just giving up and concluding that it's too much work to get non-cmake/msbuild packages to work with vcpkg I figured it would upload what I have now to get some feedback on how to proceed.

@msftclas
Copy link

msftclas commented Oct 29, 2019

CLA assistant check
All CLA requirements met.

@FredeJ FredeJ changed the title WIP: Libtomcrypt [Libtomcrypt] Add new port Oct 29, 2019
@grdowns grdowns requested review from grdowns and removed request for grdowns November 18, 2019 22:54
@grdowns grdowns self-assigned this Nov 18, 2019
@JackBoosY
Copy link
Contributor

/azp run

@LilyWangL
Copy link
Contributor

/azp run

@LilyWangL LilyWangL changed the title [Libtomcrypt] Add new port [Libtomcrypt, libtommath] Add new port Jan 19, 2020
@@ -0,0 +1,55 @@
include(vcpkg_common_functions)
Copy link
Contributor

Choose a reason for hiding this comment

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

include(vcpkg_common_functions) is deprecated. Please remove it.

@@ -0,0 +1,52 @@
include(vcpkg_common_functions)
Copy link
Contributor

Choose a reason for hiding this comment

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

include(vcpkg_common_functions) is deprecated. Please remove it.

@@ -0,0 +1,4 @@
Source: libtomcrypt
Version: 1.18.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add the Homepage for this port in the CONTROL file?

Homepage: https://github.com/libtom/libtomcrypt

@@ -0,0 +1,3 @@
Source: libtommath
Version: 1.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add the Homepage for this port in the CONTROL file?

Homepage: https://github.com/libtom/libtommath

@LilyWangL
Copy link
Contributor

If this port doesn't support some triplets, please so these steps as below:

  1. Add vcpkg_fail_port_install to portfile.cmake.
  2. Add the following code to VCPKG_PATH/scripts/ci.baseline.txt:
libtomcrypt:arm64-windows=fail
libtomcrypt:arm-uwp=fail
libtomcrypt:x64-windows-static=fail
libtommath:x64-osx
libtommath:x64-linux

@FredeJ
Copy link
Contributor Author

FredeJ commented Jan 20, 2020

I would love to just add support for triplets, but I have no idea how to do it - is there any chance that triplet support could be added to the https://github.com/microsoft/vcpkg/blob/master/docs/examples/packaging-github-repos.md guide? The only thing I can find guidance for is how to add a new triplet, not how to use existing.

@NancyLi1013
Copy link
Contributor

/azp run

@LilyWangL
Copy link
Contributor

Our CI testing system will test if the port can build successfully on the following triplets, you can click Details to check the build error log.

x86-windows
x64-windows
x64-windows-static
x64-uwp
x64-osx
x64-linux
arm-uwp
arm64-windows

Now the port build failed on some triplets as below. If this result is expected, please do the above steps:

libtomcrypt:arm64-windows=fail
libtomcrypt:arm-uwp=fail
libtomcrypt:x64-windows-static=fail
libtommath:x64-osx
libtommath:x64-linux

@NancyLi1013
Copy link
Contributor

/azp run

INSTALL
${SOURCE_PATH}/LICENSE
DESTINATION ${CURRENT_PACKAGES_DIR}/share/libtomcrypt/ RENAME copyright
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please update

file(
    INSTALL 
    ${SOURCE_PATH}/LICENSE
    DESTINATION ${CURRENT_PACKAGES_DIR}/share/libtomcrypt/ RENAME copyright
) 

as

file(
    INSTALL 
    ${SOURCE_PATH}/LICENSE
    DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT}  RENAME copyright
) 

?

file(INSTALL
${SOURCE_PATH}/LICENSE
DESTINATION ${CURRENT_PACKAGES_DIR}/share/libtommath/copyright
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please update

file(INSTALL
    ${SOURCE_PATH}/LICENSE
    DESTINATION ${CURRENT_PACKAGES_DIR}/share/libtommath/copyright
)

as

file(INSTALL
    ${SOURCE_PATH}/LICENSE
    DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT} RENAME copyright
)

?

@NancyLi1013
Copy link
Contributor

Hi @FredeJ
Could you please address the review suggestions and regressions?

@LilyWangL
Copy link
Contributor

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

@LilyWangL
Copy link
Contributor

Closing this PR since it seems that no progress is being made. Please reopen if work is still being done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants