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

[openssl] Fix installation of openssl[core,tools] #31171

Merged
merged 1 commit into from
May 4, 2023
Merged

[openssl] Fix installation of openssl[core,tools] #31171

merged 1 commit into from
May 4, 2023

Conversation

flichtenheld
Copy link
Contributor

Do not try to install file that does not exist.

Fixes #31170

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@flichtenheld
openssl: Fix installation of openssl[core,tools]
e1757d5 
Do not try to install file that does not exist.

Fixes microsoft#31170

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
dg0yt
dg0yt reviewed Apr 28, 2023
View reviewed changes
ports/openssl/windows/portfile.cmake
Comment on lines 89 to +94
if("tools" IN_LIST FEATURES)
file(MAKE_DIRECTORY "${CURRENT_PACKAGES_DIR}/tools/${PORT}")
file(RENAME "${CURRENT_PACKAGES_DIR}/openssl.cnf" "${CURRENT_PACKAGES_DIR}/tools/${PORT}/openssl.cnf")
file(RENAME "${CURRENT_PACKAGES_DIR}/fipsmodule.cnf" "${CURRENT_PACKAGES_DIR}/tools/${PORT}/fipsmodule.cnf")
if("fips" IN_LIST FEATURES)
file(RENAME "${CURRENT_PACKAGES_DIR}/fipsmodule.cnf" "${CURRENT_PACKAGES_DIR}/tools/${PORT}/fipsmodule.cnf")
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

You are creating the next bug...
The fips-related lines must be moved out of the tools-related block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? Shouldn't this command be limited to when both tools and fips are enabled? The file is explicitly deleted when tools is not enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With my patch the file is installed into tools iff both tools and fips are enabled. It is deleted when fips is enabled but tools not, And it is never generated if fips is not enabled. Which of these cases do you want to have handled differently?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why should it be deleted at all for openssl[fips]?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why should it be deleted at all for openssl[fips]?

I see, it is about to be copied to the tools directory.

flichtenheld added a commit to OpenVPN/openvpn-build that referenced this pull request May 2, 2023
@flichtenheld
GHA: use vcpkg submodule without updating to latest
489b413 
Latest vcpkg openssl port broken right now.
See microsoft/vcpkg#31171

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
flichtenheld added a commit to OpenVPN/openvpn-build that referenced this pull request May 2, 2023
@flichtenheld
GHA: use vcpkg submodule without updating to latest
e86c2a5 
Latest vcpkg openssl port broken right now.
See microsoft/vcpkg#31171

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
@flichtenheld flichtenheld mentioned this pull request May 2, 2023
Merged
flichtenheld added a commit to OpenVPN/openvpn-build that referenced this pull request May 2, 2023
@flichtenheld
GHA: use vcpkg submodule without updating to latest
5b0ea84 
Latest vcpkg openssl port broken right now.
See microsoft/vcpkg#31171

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
@LilyWangLL LilyWangLL added the category:port-bug The issue is with a library, which is something the port should already support label May 4, 2023
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label May 4, 2023
@LilyWangLL LilyWangLL changed the title openssl: Fix installation of openssl[core,tools] [openssl] Fix installation of openssl[core,tools] May 4, 2023
@JavierMatosD JavierMatosD merged commit c102be8 into microsoft:master May 4, 2023
fwcd pushed a commit to fwcd/vcpkg that referenced this pull request Aug 21, 2023
@flichtenheld @fwcd
openssl: Fix installation of openssl[core,tools] (microsoft#31171)
4676422 
Do not try to install file that does not exist.

Fixes microsoft#31170

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dg0yt dg0yt dg0yt left review comments

@LilyWangLL LilyWangLL LilyWangLL approved these changes

Assignees

@LilyWangLL LilyWangLL

Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[openssl] 3.1.0#3 on Windows doesn't build with "tools" but without "fips"
4 participants
@flichtenheld @dg0yt @LilyWangLL @JavierMatosD