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

[openssl] Update to 1.1.1i #15298

Merged
merged 1 commit into from
Jan 11, 2021
Merged

[openssl] Update to 1.1.1i #15298

merged 1 commit into from
Jan 11, 2021

Conversation

lebdron
Copy link
Contributor

@lebdron lebdron commented Dec 25, 2020

  • Update openssl to 1.1.1i.
  • Add support for arm64-osx triplet.

@JackBoosY JackBoosY added the category:port-update The issue is with a library, which is requesting update new revision label Dec 28, 2020
@NancyLi1013 NancyLi1013 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Dec 28, 2020
@@ -25,7 +25,7 @@ elseif(CMAKE_SYSTEM_NAME STREQUAL "iOS")
# disable that makes linkage error (e.g. require stderr usage)
list(APPEND DISABLES no-stdio no-ui no-asm)
elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(PLATFORM darwin64-x86_64-cc)
set(PLATFORM darwin64-${CMAKE_OSX_ARCHITECTURES}-cc)
Copy link
Contributor

Choose a reason for hiding this comment

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

@Neumann-A Could you please help review?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://cmake.org/cmake/help/v3.19/variable/CMAKE_OSX_ARCHITECTURES.html

seems ok from the description but I am not an osx guy so I don't know which values CMAKE_OSX_ARCHITECTURES actually has

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for your help @Neumann-A.

Copy link
Member

Choose a reason for hiding this comment

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

If I recall correctly CMAKE_OSX_ARCHITECTURES can contain multiple values, e.g: x86_64;arm64 when building universal (a.k.a. fat) binaries. But to be fair, we don't handle those properly in vcpkg (see lines 166-196 in our toolchain).

I'd prefer if we used old-and-trusty if-else instead of trying to do something clever.

@NancyLi1013
Copy link
Contributor

Hi @lebdron

Thanks for this PR.

Could you please look into the failures?

cppcms failed on x86-windows and x64-windows

[302/431] cmd.exe /C "cd /D D:\buildtrees\cppcms\x86-windows-dbg && D:\installed\x86-windows\tools\python3\python.exe D:/buildtrees/cppcms/src/20a4ad93d4-26c9d29282.clean/bin/cppcms_tmpl_cc -o D:/buildtrees/cppcms/x86-windows-dbg/tc_skin.cpp D:/buildtrees/cppcms/src/20a4ad93d4-26c9d29282.clean/tests/tc_skin.tmpl"
FAILED: tc_skin.cpp 
cmd.exe /C "cd /D D:\buildtrees\cppcms\x86-windows-dbg && D:\installed\x86-windows\tools\python3\python.exe D:/buildtrees/cppcms/src/20a4ad93d4-26c9d29282.clean/bin/cppcms_tmpl_cc -o D:/buildtrees/cppcms/x86-windows-dbg/tc_skin.cpp D:/buildtrees/cppcms/src/20a4ad93d4-26c9d29282.clean/tests/tc_skin.tmpl"
Traceback (most recent call last):
  File "D:\buildtrees\cppcms\src\20a4ad93d4-26c9d29282.clean\bin\cppcms_tmpl_cc", line 14, in <module>
    import StringIO
ModuleNotFoundError: No module named 'StringIO'

open62541 failed on x64-windows-static:

FAILED: src_generated/open62541/transport_generated.c src_generated/open62541/transport_generated.h src_generated/open62541/transport_generated_handling.h src_generated/open62541/transport_generated_encoding_binary.h 
cmd.exe /C "cd /D D:\buildtrees\open62541\x64-windows-static-dbg && D:\installed\x64-windows-static\tools\python3\python.exe D:/buildtrees/open62541/src/26be47a0cb-7b2df1ce14.clean/tools/generate_datatypes.py --namespace=1 --selected-types=D:/buildtrees/open62541/src/26be47a0cb-7b2df1ce14.clean/tools/schema/datatypes_transport.txt --type-bsd=D:/buildtrees/open62541/src/26be47a0cb-7b2df1ce14.clean/tools/schema/Opc.Ua.Types.bsd --type-bsd=D:/buildtrees/open62541/src/26be47a0cb-7b2df1ce14.clean/tools/schema/Custom.Opc.Ua.Transport.bsd --type-csv=D:/buildtrees/open62541/src/26be47a0cb-7b2df1ce14.clean/tools/schema/NodeIds.csv --no-builtin --internal D:/buildtrees/open62541/x64-windows-static-dbg/src_generated/open62541/transport"
Traceback (most recent call last):
  File "D:\installed\x64-windows-static\tools\python3\lib\xml\etree\ElementTree.py", line 1527, in __init__
    from xml.parsers import expat
  File "D:\installed\x64-windows-static\tools\python3\lib\xml\parsers\expat.py", line 4, in <module>
    from pyexpat import *
ModuleNotFoundError: No module named 'pyexpat'

@lebdron
Copy link
Contributor Author

lebdron commented Dec 28, 2020

Hello @NancyLi1013

Could you rerun the pipelines please? Those issues do not really look related to OpenSSL. Thanks!

@NancyLi1013
Copy link
Contributor

@lebdron

I noticed you have submit a new commit to retrigger CI.

Since cppcms and open62541 have the dependency of openssl. It might be something related with the changes in this PR.

Let's see the latest build results.

@Hoikas
Copy link
Contributor

Hoikas commented Dec 31, 2020

The CI failures seem to be related to the wrong Python executable being used - I've submitted a fix in #15404.

@lebdron
Copy link
Contributor Author

lebdron commented Jan 6, 2021

Requires #15473 now. With poppler, the problem appears when curl is installed with vcpkg.

@lebdron
Copy link
Contributor Author

lebdron commented Jan 6, 2021

@Neumann-A could you check the change mentioned please? and also poppler build fails because of this policy issue https://github.com/microsoft/vcpkg/pull/10715/files#r404183663 . Could you tell what is the suggested fix?

@NancyLi1013
Copy link
Contributor

@lebdron

Could you please merge master to this branch to retrigger CI? Since some problems caused by python3 has been solved now.

@lebdron
Copy link
Contributor Author

lebdron commented Jan 7, 2021

@NancyLi1013 Done! CI passed 😌

@NancyLi1013
Copy link
Contributor

LGTM now except for CMAKE_OSX_ARCHITECTURES. Since I'm not familiar with this.

Thanks for your PR @lebdron.

@vicroms

Could you please help review this point? Please help confirm if CMAKE_OSX_ARCHITECTURES is proper here.

Thanks.

@NancyLi1013 NancyLi1013 added requires:discussion info:reviewed Pull Request changes follow basic guidelines labels Jan 8, 2021
@@ -25,7 +25,7 @@ elseif(CMAKE_SYSTEM_NAME STREQUAL "iOS")
# disable that makes linkage error (e.g. require stderr usage)
list(APPEND DISABLES no-stdio no-ui no-asm)
elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
set(PLATFORM darwin64-x86_64-cc)
set(PLATFORM darwin64-${CMAKE_OSX_ARCHITECTURES}-cc)
Copy link
Member

Choose a reason for hiding this comment

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

If I recall correctly CMAKE_OSX_ARCHITECTURES can contain multiple values, e.g: x86_64;arm64 when building universal (a.k.a. fat) binaries. But to be fair, we don't handle those properly in vcpkg (see lines 166-196 in our toolchain).

I'd prefer if we used old-and-trusty if-else instead of trying to do something clever.

@vicroms vicroms removed the info:reviewed Pull Request changes follow basic guidelines label Jan 8, 2021
Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
@lebdron lebdron requested a review from vicroms January 8, 2021 20:06
@vicroms
Copy link
Member

vicroms commented Jan 8, 2021

Thanks for the PR!
Waiting for CI results to merge.

@vicroms
Copy link
Member

vicroms commented Jan 11, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vicroms vicroms merged commit 1329385 into microsoft:master Jan 11, 2021
@vicroms
Copy link
Member

vicroms commented Jan 11, 2021

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants