Page MenuHomeFreeBSD

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

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

Details

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb created this revision.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?

jhb added a comment.Apr 3 2019, 6:03 PM

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
swills accepted this revision.Apr 3 2019, 6:40 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 ↗(On Diff #55126)

Should be after the LICENSE block according to Order of Variables in Port Makefiles chapter in the Porter's Handbook.

30–32 ↗(On Diff #55126)

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
jhb added a comment.Apr 3 2019, 7:02 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.

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 ↗(On Diff #55126)

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.

jrm accepted this revision.Apr 3 2019, 7:08 PM

Sounds good. Sorry, I was unclear and didn't refresh before you replied to Josh.

This revision is now accepted and ready to land.Apr 3 2019, 7:08 PM
This revision was automatically updated to reflect the committed changes.