[devel/mdb]: Fix build with clang 8.
ClosedPublic
Actions

Authored by jhb on Mar 15 2019, 11:00 PM.

Details

Reviewers

swills
jrm

Commits

rP499384: MFH: r497977
rP497977: Fix build with clang 8.

Summary

Workaround clang 8 compile issues. Note that this requires changes to the
kernel source tree in r345196.

While here, switch to using a date for the version number since there are
no real version numbers in the git repository. This doesn't use the 'g'
prefix since the older versions that already exist would sort after it.

PR: 236207

Test Plan

testport against a patched world with clang 8

Diff Detail

Repository

rP FreeBSD ports repository

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 23124
Build 22177: arc lint + arc unit

Event Timeline

jhb created this revision.Mar 15 2019, 11:00 PM

Herald added a subscriber: mat. · View Herald TranscriptMar 15 2019, 11:00 PM

Harbormaster completed remote builds in B23124: Diff 55126.Mar 15 2019, 11:00 PM

If this requires a specific src rev is there an OSVERSION that rev corresponds to so the port can test for it before it builds?

The patches check __FreeBSD_version already, so it should compile on everything except for 13-head in between clang 8 and r345196. It doesn't seem worth it to me to add an explicit BROKEN just for a small range of OSVERSION values on HEAD.

swills added a subscriber: swills.Apr 3 2019, 6:35 PM

You could get around the 'g' prefix issue with PORTEPOCH but if you don't want to do that I guess it's fine. LGTM.

This revision is now accepted and ready to land.Apr 3 2019, 6:40 PM

jrm added a subscriber: jrm.Apr 3 2019, 6:53 PM

jrm added inline comments.

devel/mdb/Makefile
19	Should be after the `LICENSE` block according to `Order of Variables in Port Makefiles` chapter in the Porter's Handbook.
30–32	Do you want to add a check to ensure the system is at `r345196`? I don't know if that is helpful, since it's sounds like it's the source tree that has to include that revision. +.if !exists(${SRC_BASE}/cddl/compat) \|\| !exists(${SRC_BASE}/sys/cddl/compat) \|\| ${OSVERSION} < 1300015 +IGNORE= requires full source tree with CDDL sources after r345196 .endif

jrm requested changes to this revision.Apr 3 2019, 6:53 PM

This revision now requires changes to proceed.Apr 3 2019, 6:53 PM

In D19602#424888, @swills wrote:

You could get around the 'g' prefix issue with PORTEPOCH but if you don't want to do that I guess it's fine. LGTM.

I had guessed that the PORTEPOCH cure was kind of worse than the 'g' prefix, but I wasn't really sure what the best practice for this is (aside from having not done it wrong in the first place)

jhb added inline comments.Apr 3 2019, 7:03 PM

devel/mdb/Makefile
30–32	No, that would break the build on 12.x and 11.x, etc. As I mentioned to Josh, it's only a narrow range of OSVERSIONS on 13 that is broken, so it didn't seem worth it to add explicit OSVERSION checks just for that range.