Opened 15 months ago

Closed 15 months ago

#30560 closed enhancement (fixed)

Make openssl an optional dependency of python3

Reported by: mkoeppe Owned by:
Priority: critical Milestone: sage-9.2
Component: build Keywords:
Cc: gh-mwageringel, slelievre, dimpase, mjo, jhpalmieri Merged in:
Authors: Matthias Koeppe Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 0c394ca (Commits, GitHub, GitLab) Commit: 0c394ca0885c02944fba5b3472c5bd7f9a3c274d
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

As discussed in #30556, if openssl is enabled using --enable-openssl and neither openssl nor python3 are available as system packages, this makes sure that openssl is built before python3.

If system openssl is found via #30557, then --enable-openssl does NOT trigger building openssl. Use --enable-openssl --without-system-openssl if this is desired.

Part of #21700 Meta-ticket: Declare "optional" dependencies of packages (a package manager's suggested/recommended packages)

Change History (15)

comment:1 Changed 15 months ago by mkoeppe

  • Summary changed from Build optional package openssl as part of TOOLCHAIN_DEPS to Build optional package openssl as part of SAGE_TOOLCHAIN

comment:2 Changed 15 months ago by mkoeppe

Best done with a change to the SAGE_SPKG_ENABLE macro

comment:3 Changed 15 months ago by mkoeppe

  • Priority changed from major to critical

comment:4 Changed 15 months ago by mkoeppe

  • Description modified (diff)

comment:5 Changed 15 months ago by mkoeppe

  • Branch set to u/mkoeppe/build_optional_package_openssl_as_part_of_sage_toolchain

comment:6 Changed 15 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to 0c394ca0885c02944fba5b3472c5bd7f9a3c274d
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from Build optional package openssl as part of SAGE_TOOLCHAIN to Make openssl an optional dependency of python3

Found a simpler solution than the previously discussed SAGE_TOOLCHAIN business. Needs review

New commits:

​0c394cabuild/pkgs/python3/dependencies: Make openssl a dependency if it is enabled

comment:7 Changed 15 months ago by mkoeppe

  • Description modified (diff)

comment:8 Changed 15 months ago by mkoeppe

  • Cc jhpalmieri added

comment:9 follow-ups: Changed 15 months ago by jhpalmieri

This looks like it works, and the change makes sense. I couldn't completely test, though, since the ssl module for Python always seems to get built.

To test, I did make distclean. Then 

$ ./configure --enable-openssl --with-system-python3=no
$ make

built openssl first and then python3, which is the point, right? Without this branch, these commands just build python3, not openssl.

Do I need to find a system on which Python's ssl module fails to build?

comment:10 in reply to: ↑ 9 ; follow-up: Changed 15 months ago by mkoeppe

Replying to jhpalmieri:

To test, I did make distclean. Then 

$ ./configure --enable-openssl --with-system-python3=no
$ make

built openssl first and then python3, which is the point, right?

Right.

Without this branch, these commands just build python3, not openssl.

I think without this branch it should also have built openssl, just not necessarily before python3.

comment:11 in reply to: ↑ 9 Changed 15 months ago by mkoeppe

Replying to jhpalmieri:

Do I need to find a system on which Python's ssl module fails to build?

I don't think that's necessary

comment:12 in reply to: ↑ 10 Changed 15 months ago by jhpalmieri

Replying to mkoeppe:

Replying to jhpalmieri:

To test, I did make distclean. Then 

$ ./configure --enable-openssl --with-system-python3=no
$ make

built openssl first and then python3, which is the point, right?

Right.

Without this branch, these commands just build python3, not openssl.

I think without this branch it should also have built openssl, just not necessarily before python3.

Sorry. I actually ran make python3, which is pretty convincing evidence that with this branch and with ./configure --enable-openssl, Sage believes that openssl is a dependency of python3.

comment:13 Changed 15 months ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

Good find with findstring, by the way.

comment:14 Changed 15 months ago by mkoeppe

Thanks!

comment:15 Changed 15 months ago by vbraun

  • Branch changed from u/mkoeppe/build_optional_package_openssl_as_part_of_sage_toolchain to 0c394ca0885c02944fba5b3472c5bd7f9a3c274d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.