misc/rumprun/Makefile
16 ↗	(On Diff #37878)	Can't we use `USES=compiler` options instead of `USE_GCC` ?
25 ↗	(On Diff #37878)	Why did you have to split your includes as `.pre.mk` and `.post.mk`? Anything I have missed about here?

mmokhi added a reviewer: mmokhi.Jan 12 2018, 7:30 PM

fabian.freyer_physik.tu-berlin.de added inline comments.Jan 12 2018, 7:42 PM

misc/rumprun/Makefile
16 ↗	(On Diff #37878)	Unfortunately, no, it needs gcc.
25 ↗	(On Diff #37878)	You're right, I don't need to do this. I'll update the patch.

Merge .pre.mk .post.mk

fabian.freyer_physik.tu-berlin.de marked 2 inline comments as done.Jan 12 2018, 7:43 PM

yuri added a subscriber: yuri.Jan 12 2018, 10:33 PM

yuri added inline comments.

misc/rumprun/Makefile
16 ↗	(On Diff #37878)	Why does it need gcc? What isn't working with clang?

yuri added inline comments.Jan 12 2018, 10:36 PM

misc/rumprun/Makefile
5 ↗	(On Diff #37880)	The version should be `g20180105`.

yuri added inline comments.Jan 12 2018, 10:37 PM

misc/rumprun/Makefile
5 ↗	(On Diff #37880)	And it should be `DISTVERSION`, not `PORTVERSION`.

fabian.freyer_physik.tu-berlin.de added inline comments.Jan 13 2018, 3:16 AM

misc/rumprun/Makefile

5 ↗

(On Diff #37880)

Ah, I hadn't seen Example 5.14. Makes sense, I'll update the patch.

16 ↗

(On Diff #37878)

See https://github.com/rumpkernel/rumprun/blob/master/build-rr.sh#L305:

checktools ()
{

	# Check that a clang build is not attempted.
	[ -z "${BUILDRUMP_HAVE_LLVM}" ] \
	    || die rumprun does not yet support clang ${CC:+(\$CC: $CC)}

	delay=5

	# check that gcc is modern enough
	vers=$(${CC:-cc} -E -dM - < /dev/null | LANG=C awk '
	    /__GNUC__/ {version += 100*$3}
	    /__GNUC_MINOR__/ {version += $3}
	    END { print version; if (version) exit 0; exit 1; }') \
		|| unable to probe cc version
	if [ ${vers} -lt 406 ]; then
		die gcc is too old, need 4.6 or later. ${CC:+(\$CC: $CC)}
	elif [ ${vers} -lt 408 ]; then
		echo '>>'
		echo ">> WARNING: gcc is old. ${CC:+(\$CC: $CC)}"
		echo '>> Version 4.8 or later is recommended.'

Usse g prefix on DISTVERSION

fabian.freyer_physik.tu-berlin.de marked 2 inline comments as done.Jan 13 2018, 3:49 AM

fabian.freyer_physik.tu-berlin.de marked an inline comment as done.

Looks good to me.
If @yuri is also fine with it, I'll commit it.

About GCC i think it makes sense that it requires it since in netbsd the default is gcc AFAIK.
(however I'd go with fixing the build-with-Clang in long-term and upstream the fix).

Macro lgtm:

This revision is now accepted and ready to land.Jan 13 2018, 11:19 AM

I am fine, thanks!

Thanks a lot! Feel free to close PR #225117 when comitted.

Closed by commit rP458940: misc/rumprun: Add port to the tree (authored by mmokhi). · Explain WhyJan 13 2018, 8:36 PM

This revision was automatically updated to reflect the committed changes.

mat added inline comments.Jan 15 2018, 2:36 PM

head/misc/rumprun/Makefile
22	This is wrong. man pages should be installed in PREFIX/man, so the build or install script should be patched to put them at the right place.

@mat OH I see 🙈 (I'm so sorry that I didn't noticed that before 😅 🙏 )
We have another PR open, and in committing that I'll also fix this as well before committing.