svn commit: r437089 - in head/devel: . hhdate hhdate/files

[Michael Gmelin]

On Tue, 28 Mar 2017 15:26:12 +0000
Alexey Dokuchaev <danfe at FreeBSD.org> wrote:

> On Tue, Mar 28, 2017 at 04:57:08PM +0200, Michael Gmelin wrote:
> > On Tue, 28 Mar 2017 14:27:01 +0000
> > Alexey Dokuchaev <danfe at FreeBSD.org> wrote:  
> > > ...
> > > Bogus formatting (one extra tab after knobs), would you please
> > > fix it?  
> > 
> > These are in to align to
> > 
> > TEST_TEST_TARGET=<tab>test, otherwise it would look like this:
> > 
> > TEST_USES=      compiler:c++11-lib
> > TEST_TEST_TARGET=test  
> 
> I'm not even sure why wrapping test-framework stuff in
> options-framework, was it because of TEST_USES (since TEST_DEPENDS
> was not really what you wanted)?

The idea is that "make test" won't fail in case "TEST" wasn't set.
c++11-lib is not a big deal, as these days we usually have clang as a
system compiler that provides this anyway. In other cases you might
pull in huge dependencies with the TEST knob though, so one might
decide to not use tests for this specific port (e.g. pulls in clang39
or some other huge library by other DEPENDS), so if you disable it and
still run make test (which we e.g. do for all ports when doing
production builds) the build would fail. Hence only providing a test
target if configured to do tests (the original version of the port
skeleton had much more elaborate tests and will get them back once the
maintainer contacted the author about a few unit tests that don't seem
to make sense).

> 
> > (replaced tabs by spaces for email)
> > 
> > Would this actually be preferred?  
> 
> Options-support (helper) knobs are indeed longer than usual and might
> require special care (esp. when mixed with shorter knobs) to get
> pretty alignment, and that's OK since they are usually grouped
> together after OPTIONS_DEFINE (or OPTIONS_DEFAULT -- since it also is
> overly long).
> 
> That said, there is no reason to double-tab standard width knobs that
> preceed OPTIONS_DEFINE just to align them with what goes below it.
> OPTIONS_DEFINE is a nice dividing line (esp. since it often gets quite
> long as well).
> 
> So, group longer option helper knobs and align within them, but don't
> aling the entire Makefile with them.

Makes sense, thank you.

> 
> > > Any reason why option helper target (post-extract-TEST-on) was not
> > > used here?  
> > 
> > Simply unaware it existed, so this would change to:
> > 
> > post-extract-TEST-on:
> >         ${INSTALL_DATA} ${FILESDIR}/Makefile.in ${WRKSRC}/Makefile
> >         ${INSTALL_DATA} ${FILESDIR}/test-minimal.cpp ...  
> 
> Right; now don't take my word for it, rehash PHB and
> `Mk/bsd.options.mk' and test changes thoroughly. :-)

:)

One slightly related question:
While working on this port skeleton I wondered if it wouldn't make
sense to chage Mk/Uses/compilers.mk to change the minimum default
version of llvm for c++14. Right now we use llvm36, but other ports
need newer versions and using the latest stable version would probably
stop us from building multiple versions of llvm when building even just
parts of the ports tree.

-m

-- 
Michael Gmelin