Page MenuHomeFreeBSD

security/openssl-devel: Create OpenSSL 1.1.0 Alpha port
ClosedPublic

Authored by brnrd on Feb 28 2016, 5:01 PM.

Details

Summary

Proposed commit log

security/openssl-devel: Create OpenSSL 1.1.0 Alpha port

 - Repo-copy of security/openssl
 - Remove unneeded USE_OPENSSL check
 - Add and normalize CONFLICTS
 - Put OPTIONS into GROUPS for ciphers, hashes, protocols and optimizations
 - Add more configurable ciphers, hashes and protocols
 - Sort <OPT>_DESC alphabetically
 - Reword <OPT>_DESC after grouping, add (comment)
 - Rewrite ${PORT_OPTIONS:M<OPT>} to <OPT>_<FEATURE>_* where possible
 - Rewrite ${PORT_OPTIONS:M<OPT>} to target-<OPT>-on where possible
 - Rewrite do-configure target to HAS_CONFIGURE/CONFIGURE_SCRIPT
 - Rewrite (regression-)test target to TEST_TARGET
 - Add NPN support patch from [1]
 - Remove 1.0.2 specific patches (Padlock and EVP_MD_CTX_FLAG_ONESHOT)
 - Disable obsolete, broken and obscure features

[1] https://github.com/openssl/openssl/pull/757

Reviewed_by:	feld (mentor), koobs (mentor)
Approved by:	(mentor)
Differential_Revision:	D5484
Test Plan
  • portlint -AC
  • poudriere testport

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
brnrd added inline comments.Feb 29 2016, 9:50 PM
security/openssl-devel/Makefile
5–6 ↗(On Diff #13853)

make -V PKGNAME
pkg version -t openssl-devel-1.1.0.a openssl-devel-1.1.0
Handbook Table 5.1
Good hint!
Had to resort to DISTNAME to silence a portlint warning on PORTVERSION and DISTVERSION

7 ↗(On Diff #13853)

Once 1.1 is released, the security/openssl port will take over until OpenSSL starts a new devel version. @dinoex already questioned if we want this port at all.
11 already has 1.0.2
10 has 1.0.1 (EoL end of this year)
9 has 0.9 (EoL end of LAST year)
The SHLIB versions have not updated between 1.0 and 1.1 so no need for a separate openssl11 port.

9 ↗(On Diff #13853)

Only one. And no info on mirroring process. Therefore must assume all mirrors are updated unsafely and we should rely on the SHA256 hash and tarball size.

14 ↗(On Diff #13853)

My bad.
Should I also update Created by? (line 1)

15 ↗(On Diff #13853)

Corrected

17–19 ↗(On Diff #13853)

Tested with make -DUSE_OPENSSL, obviously superfluous

21 ↗(On Diff #13853)

It does, and added first digit check (for lint).

25 ↗(On Diff #13853)

That's more correct. Broke indentation so shortened them.

OPTIMIZE or OPTIMISE... (en_US vs en_UK iirc) ? :D

56–67 ↗(On Diff #13853)

Fixed all the DESC items

108 ↗(On Diff #13853)

Using CONFIGURE_ARGS makes the Makefile more legible and a lot smaller.
There's no replacement for EC_EXTRACONFIG.
Other configure options are also named CONFIGURE (HAS_, _ARGS, _ENV, etc) so CONFIGURE is not necessarily tied to autotools

116–120 ↗(On Diff #13853)

Missed this one! Done.

164–165 ↗(On Diff #13853)

Is already a no-op, both branches are the same. Fixed accordingly

Replaced with HAS_CONFIGURE, CONFIGURE_SCRIPT, USE_PERL5=build,configure, moved Makefile patching to post-configure

brnrd updated this revision to Diff 13909.Feb 29 2016, 10:36 PM
brnrd edited edge metadata.
brnrd marked 14 inline comments as done.

Update after mentor comments

  • See review for an itemized list
brnrd updated this revision to Diff 13910.Feb 29 2016, 10:38 PM
brnrd edited edge metadata.

Unlearn muscle-memory... OpenSSL not Libre this time

koobs requested changes to this revision.Mar 1 2016, 11:30 PM
koobs edited edge metadata.

Please format commit log as:

- What was changed, why

In particular, the following entries are unclear:

- Utilize options fully
- Makefile cleanup
- Disable obsolete, broken and obscure features

Additionally the commit log message does not enumerate/describe all changes, including:

  • comment
  • conflicts
  • adding of options groups
  • changing of options descriptions
  • stripping of shared libraries/binaries

Commit log message should represent a standalone (without any additional information) description of what changed and why, and match the changeset

security/openssl-devel/Makefile
5 ↗(On Diff #13910)

Again why not DISTVERSIONSUFFIX=-pre3 here, precluding the need for DISTNAME override?

This revision now requires changes to proceed.Mar 1 2016, 11:30 PM
brnrd marked an inline comment as done.Mar 3 2016, 9:31 AM

Rewrite of commit message still pending.

security/openssl-devel/Makefile
5 ↗(On Diff #13910)

According to the Package Naming Examples an alpha suffix translates into .a and stings like alpha, beta and rc are not allowed. The OpenSSL project labels these -pre releases as alpha release.

I will change this into 1.1.0.p3 as per the -version.numbers specification and pkg version tests

The version string follows a dash (-) and is a period-separated list of integers and single lowercase alphabetics. In particular, it is not permissible to have another dash inside the version string. The only exception is the string pl (meaning “patchlevel”), which can be used only when there are no major and minor version numbers in the software. If the software version has strings like “alpha”, “beta”, “rc”, or “pre”, take the first letter and put it immediately after a period. If the version string continues after those names, the numbers follow the single alphabet without an extra period between them (for example, 1.0b2).

$ pkg version -t pkg version -t openssl-devel-1.1.0.p3 openssl-devel-1.1.0.b2
<
$ pkg version -t pkg version -t openssl-devel-1.1.0.p3 openssl-devel-1.1.0.r1`
<

DISTNAME would resolve to openssl-1.1.0.p3 so I had to resort to DISTVERSION= 1.1.0-pre3.

koobs added inline comments.Mar 3 2016, 9:53 AM
security/openssl-devel/Makefile
15 ↗(On Diff #13910)

I dont think the alpha designation is needed here. Just that its a different release series than security/openssl.

I was thinking

COMMENT= SSL and crypto library (1.1.x)

brnrd updated this revision to Diff 14009.Mar 3 2016, 9:53 AM
brnrd edited edge metadata.
brnrd marked an inline comment as done.

Fix port version, pkgname and distname

koobs requested changes to this revision.Mar 3 2016, 9:58 AM
koobs edited edge metadata.
koobs added inline comments.
security/openssl-devel/Makefile
5 ↗(On Diff #14009)

Let DISTVERSION do the correct PORTVERSION derivation automatically. This should be DISTVERSION=1.1.0-pre3

11 ↗(On Diff #14009)

This wont be needed when you use DISTVERSION above

This revision now requires changes to proceed.Mar 3 2016, 9:58 AM
brnrd updated this revision to Diff 14010.Mar 3 2016, 10:03 AM
brnrd edited edge metadata.

PORTVERSION is correctly derived from DISTVERSION automatically

brnrd updated this revision to Diff 14011.Mar 3 2016, 10:19 AM
brnrd edited edge metadata.

Remove unrelated change from security/Makefile

koobs requested changes to this revision.Mar 3 2016, 10:24 AM
koobs edited edge metadata.
koobs added inline comments.
security/openssl-devel/Makefile
25 ↗(On Diff #13853)

US

1 ↗(On Diff #14011)

Revert, this is based on (and copied from) another port. Its heritage is importante :)

10 ↗(On Diff #14011)

I believe this will make portlint cry (due to ordering). DISTVERSION should be placed where PORTVERSION normally is

13 ↗(On Diff #14011)

I think 1.1.x is better here, as the alpha will at some point become beta -> rc -> release, and we ought not have to change COMMENT each time.

102 ↗(On Diff #14011)

I don't believe reason for global -L${PREFIX}/lib was answered from previous review inline comment

109 ↗(On Diff #14011)

Don't need .if/.endif block surrounding entries when using options helpers

This revision now requires changes to proceed.Mar 3 2016, 10:24 AM
brnrd updated this object.Mar 3 2016, 10:36 AM
brnrd edited edge metadata.
brnrd marked 6 inline comments as done.Mar 3 2016, 10:53 AM
brnrd added inline comments.
security/openssl-devel/Makefile
10 ↗(On Diff #14011)

Fixed portlint FATAL by moving

13 ↗(On Diff #14011)

Agree!

18–27 ↗(On Diff #14011)

OptimiZe is US

102 ↗(On Diff #14011)

Without -L${PREFIX}/lib the Configure script will not correctly set EX_LIBS for compilation leading to issues during linking
EX_LIBS =-L/usr/local/lib vs EX_LIBS =
EX_LIBS is used in many places in the .in files.

109 ↗(On Diff #14011)

These are 2 separate tests. This is to make sure that 386 is added to CONFIGURE_ARCHS only if both the OPTION is set and the ARCH is 386. (386 vs I386))

brnrd updated this revision to Diff 14012.Mar 3 2016, 11:06 AM
brnrd edited edge metadata.
brnrd marked 5 inline comments as done.

Updates for latest comments

koobs added inline comments.Mar 3 2016, 11:12 AM
security/openssl-devel/Makefile
1 ↗(On Diff #14011)

Not done

10 ↗(On Diff #14011)

Not done, cant see this changed in the diff

13 ↗(On Diff #14011)

Not done, cant see this in the diff

18–27 ↗(On Diff #14011)

I would still use the shorter version OPTIMIZE (but your call)

102 ↗(On Diff #14011)

But what needs to be linked from LOCALBASE exactly?

Further, shouldn't USES=localbase be used if its required?

109 ↗(On Diff #14011)

Based on that explanation, I think this should be:

i386_OPTIONS_DEFINE=I386
...
I386_CONFIGURE_ON=386

Relevant snippet from bsd.options.mk:

  1. OPTIONS_DEFINE_${ARCH} - List of options this ports accept and are
  2. specific to ${ARCH}
koobs requested changes to this revision.Mar 3 2016, 11:14 AM
koobs edited edge metadata.
koobs added inline comments.
security/openssl-devel/Makefile
102–103 ↗(On Diff #14012)

Remove commented line, or fix

110–111 ↗(On Diff #14012)

See prior diff comment comment (re ARCH_OPTIONS_DEFINE)

This revision now requires changes to proceed.Mar 3 2016, 11:14 AM
brnrd updated this revision to Diff 14014.Mar 3 2016, 11:26 AM
brnrd edited edge metadata.

I386 option is only relevant for i386 architecture

brnrd updated this revision to Diff 14015.Mar 3 2016, 11:28 AM
brnrd edited edge metadata.

Remove commented line

brnrd marked 2 inline comments as done.Mar 3 2016, 11:37 AM

Comments addressed

brnrd updated this revision to Diff 14016.Mar 3 2016, 11:38 AM

PREFIX -> Localbase

brnrd marked 9 inline comments as done.Mar 3 2016, 12:47 PM

All done

security/openssl-devel/Makefile
18–28 ↗(On Diff #14016)

OK as is

brnrd updated this revision to Diff 14022.Mar 3 2016, 2:08 PM
brnrd marked an inline comment as done.

rename OPTIMIZATIONS to OPTIMIZE

koobs accepted this revision.Mar 3 2016, 2:11 PM
koobs edited edge metadata.

LGTM. Approved if portlint and poudriere still pass.

This revision is now accepted and ready to land.Mar 3 2016, 2:11 PM
This revision was automatically updated to reflect the committed changes.
brnrd marked an inline comment as done.