Bug 213956 - [New Port] devel/libtuntap: Library for configuring TUN or TAP devices in a portable manner
Summary: [New Port] devel/libtuntap: Library for configuring TUN or TAP devices in a p...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Jan Beich
URL:
Keywords: patch-ready
Depends on:
Blocks:
 
Reported: 2016-10-31 20:26 UTC by Mahdi Mokhtari
Modified: 2016-11-01 19:23 UTC (History)
0 users

See Also:


Attachments
Patch adds devel/libtuntap (4.17 KB, patch)
2016-10-31 20:26 UTC, Mahdi Mokhtari
no flags Details | Diff
Patch adds devel/libtuntap (with little comment fix) (4.22 KB, patch)
2016-10-31 20:31 UTC, Mahdi Mokhtari
no flags Details | Diff
Patch adds devel/libtuntap (After fixing the points jbeich@ pointed) (4.28 KB, patch)
2016-11-01 17:09 UTC, Mahdi Mokhtari
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mahdi Mokhtari freebsd_committer freebsd_triage 2016-10-31 20:26:32 UTC
Created attachment 176351 [details]
Patch adds devel/libtuntap

This port (libtuntap) is a library for configuring TUN or TAP devices in a portable way.
Comment 1 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-10-31 20:27:27 UTC
Portlint: Okay
Poudriere: (10.3/11) Okay

Returning to pool, so that a committer can take it.
Comment 2 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-10-31 20:31:07 UTC
Created attachment 176352 [details]
Patch adds devel/libtuntap (with little comment fix)

little comment fixing done on patch.
Just like before: QA is still Okay.
Comment 3 Jan Beich freebsd_committer freebsd_triage 2016-11-01 04:02:06 UTC
Comment on attachment 176352 [details]
Patch adds devel/libtuntap (with little comment fix)

CXX_WRPR=on build fails on FreeBSD 9.x (add USE_CXXSTD=c++11 to fix):

  /usr/local/bin/g++48   -DFreeBSD -DUnix -Dtuntap___EXPORTS -I. -I/usr/local/include -I/bindings/cpp -O2 -pipe -Wl,-rpath=/usr/local/lib/gcc48 -fno-strict-aliasing -Wl,-rpath=/usr/local/lib/gcc48 -O2 -pipe -Wl,-rpath=/usr/local/lib/gcc48 -fno-strict-aliasing -Wl,-rpath=/usr/local/lib/gcc48 -fPIC -o CMakeFiles/tuntap++.dir/bindings/cpp/tuntap++.cc.o -c bindings/cpp/tuntap++.cc
  In file included from bindings/cpp/tuntap++.cc:1:0:
  bindings/cpp/tuntap++.hh:16:22: warning: defaulted and deleted functions only available with -std=c++11 or -std=gnu++11 [enabled by default]
     tun(tun const &) = delete;
			^
  bindings/cpp/tuntap++.hh:17:36: warning: defaulted and deleted functions only available with -std=c++11 or -std=gnu++11 [enabled by default]
     tun & operator = (tun const &) = delete;
				      ^
  bindings/cpp/tuntap++.hh:18:11: error: expected ',' or '...' before '&&' token
     tun(tun &&);
	     ^
  bindings/cpp/tuntap++.hh:18:13: error: invalid constructor; you probably meant 'tuntap::tun (const tuntap::tun&)'
     tun(tun &&);
	       ^
  bindings/cpp/tuntap++.hh:44:22: warning: defaulted and deleted functions only available with -std=c++11 or -std=gnu++11 [enabled by default]
     tap(tap const &) = delete;
			^
  bindings/cpp/tuntap++.hh:45:36: warning: defaulted and deleted functions only available with -std=c++11 or -std=gnu++11 [enabled by default]
     tap & operator = (tap const &) = delete;
				      ^
  bindings/cpp/tuntap++.hh:46:11: error: expected ',' or '...' before '&&' token
     tap(tap &&);
	     ^
  bindings/cpp/tuntap++.hh:46:13: error: invalid constructor; you probably meant 'tuntap::tap (const tuntap::tap&)'
     tap(tap &&);
	       ^
  bindings/cpp/tuntap++.cc:20:14: error: expected ',' or '...' before '&&' token
   tun::tun(tun &&t)
		^
  bindings/cpp/tuntap++.cc:20:1: error: prototype for 'tuntap::tun::tun(tuntap::tun)' does not match any in class 'tuntap::tun'
   tun::tun(tun &&t)
   ^
  In file included from bindings/cpp/tuntap++.cc:1:0:
  bindings/cpp/tuntap++.hh:16:3: error: candidates are: tuntap::tun::tun(const tuntap::tun&)
     tun(tun const &) = delete;
     ^
  bindings/cpp/tuntap++.cc:9:1: error:                 tuntap::tun::tun()
   tun::tun()
   ^
  bindings/cpp/tuntap++.cc:97:14: error: expected ',' or '...' before '&&' token
   tap::tap(tap &&t)
		^
  bindings/cpp/tuntap++.cc:97:1: error: prototype for 'tuntap::tap::tap(tuntap::tap)' does not match any in class 'tuntap::tap'
   tap::tap(tap &&t)
   ^
  In file included from bindings/cpp/tuntap++.cc:1:0:
  bindings/cpp/tuntap++.hh:44:3: error: candidates are: tuntap::tap::tap(const tuntap::tap&)
     tap(tap const &) = delete;
     ^
  bindings/cpp/tuntap++.cc:86:1: error:                 tuntap::tap::tap()
   tap::tap()
   ^
  *** [CMakeFiles/tuntap++.dir/bindings/cpp/tuntap++.cc.o] Error code 1

> +PORTNAME=		libtuntap
            ^^^^^^^^^^^^ - 2 tabs
> +PORTVERSION=	2.1
               ^ - 1 tab

Inconsistent whitespace. Adjust your editor to tab width 8 (if not default) then reindent.

> +CATEGORIES=		devel
                        ^^^^^
Why not under net/ ? I don't think tun/tap devices can be used for non-networking purposes.

> +COMMENT=	Library for configuring TUN or TAP devices in a portable manner
                ^^^^^^^^^^^
Try to avoid redundant words for concise description e.g., "Library for" is already evident from the port name.

> +USE_GITHUB=	yes

Don't forget to populate WWW field in pkg-descr, pointing to the project page on GitHub.

> +GH_ACCOUNT=	m0khi

Repos on GitHub share commit objects for all forks. GH_ACCOUNT=LaKabane would work as well.

> +GH_TAGNAME=	6182ddf

Better use DISTVERSIONSUFFIX instead for better package version e.g.,

  $ git describe 6182ddf
  libtuntap-0.3-22-g6182ddf

turns into

  DISTVERSION=	0.3-22
  DISTVERSIONSUFFIX=	-g6182ddf

which is actually

  $ make -V PKGVERSION
  0.3.22

> +BUILD_DEPENDS=	cmake>=2.8:devel/cmake

cmake>=2.8 dates back to 2009-11-29. Do you expect the port to work on an even older tree? If not, drop the line.

> +OPTIONS_DEFINE+=		CXX_WRPR RGRSS_TEST

If you don't plan to have slave ports alter the value replace '+=' with '='.

> +RGRSS_TEST_DESC=		Build Regression tests for libtuntap

Better use common TEST_DESC (defined in Mk/bsd.options.desc.mk) instead i.e., rename the option. TEST is also enabled by default if you have DEVELOPER=yes in make.conf.

> +CMAKE_BUILD_TYPE=		Release

This is already the default value.

> +CXX_WRPR_CMAKE_ON=		-DENABLE_CXX=ON
> +RGRSS_TEST_CMAKE_ON=	-DENABLE_REGRESS=ON
                   ^^^
Convert to _CMAKE_BOOL option helper.

> +CXX_WRPR_USE+=			compiler:c++11-lib
               ^^
Most option helpers already append value, so '=' would work fine as well. I guess, it's just a typo, otherwise CXX_WRPR=on build would still use GCC 4.2.1 on FreeBSD 9.x systems.

> +.include <bsd.port.options.mk>
> +
> +check: build
> +.if ${PORT_OPTIONS:MRGRSS_TEST}
> +	@(cd ${WRKSRC} && ${SETENV} ${MAKE_ENV} ${MAKE} ${MAKE_FLAGS} ${MAKEFILE} ${MAKE_ARGS} test)
> +.endif

Convert to TEST_TARGET, see /usr/ports/CHANGES from 20150928. To hook poudriere you may also need

  # XXX https://github.com/freebsd/poudriere/pull/355
  pre-install-TEST-on: do-test

unless some tests are expected to fail e.g., due to lack of permissions.

> +CXX_WRPR_DESC=			Build CXX wrapper for libtuntap
> +CXX_WRPR_CMAKE_ON=		-DENABLE_CXX=ON
> +CXX_WRPR_USE+=			compiler:c++11-lib
[...]
> ++	if(ENABLE_CXX)
> ++		install(TARGETS tuntap++ DESTINATION lib)
> ++		install(TARGETS tuntap++-static DESTINATION lib)
> ++		install(FILES bindings/cpp/tuntap++.hh DESTINATION include)
> ++	endif(ENABLE_CXX)

Maybe move to a slave port instead. That way libtuntap consumers can be sure its C++ wrapper is always avaliable from a specific port. For example,

  # master Makefile
  COMMENT?=	Configure TUN or TAP devices in a portable manner
  ...
  OPTIONS_EXCLUDE?=               CXX_WRPR

  # master pkg-plist
  %%NO_CXX_WRPR%%include/tuntap.h
  ...

  # slave Makefile
  PKGNAMESUFFIX=  -c++

  COMMENT=        C++ wrapper for libtuntap       

  LIB_DEPENDS=    libtuntap.so:devel/libtuntap

  MASTERDIR=      ${.CURDIR}/../libtuntap

  OPTIONS_SLAVE=  CXX_WRPR
  OPTIONS_EXCLUDE=        # clear CXX_WRPR in master

  .include "${MASTERDIR}/Makefile"
Comment 4 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-11-01 17:05:44 UTC
(In reply to Jan Beich (mail not working) from comment #3)
> CXX_WRPR=on build fails on FreeBSD 9.x (add USE_CXXSTD=c++11 to fix):
Well, the `USE_CXXSTD=c++11` option didnt fix it, cause AFAIK FreeBSD 9.3's compiler do not understand `-std=c++11` option.
So, i marked port as broken for CXX_WRPR option on FreeBSD9.x

> Repos on GitHub share commit objects for all forks. GH_ACCOUNT=LaKabane would work as well.
Nice to know! i didn't know this :D

>   pre-install-TEST-on: do-test
Making test target as dependency of install is not necessary IMO, cause maybe someone wants to load device modules (eg, if_tap.ko) later. and they don't expect to see `make test` fails for this case.

All points are fixed.
Thanks for your suggestions :)
Comment 5 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-11-01 17:09:11 UTC
Created attachment 176381 [details]
Patch adds devel/libtuntap (After fixing the points jbeich@ pointed)

Portlint okay.
Poudriere (10.X and 11.X okay, 9.X without CXX okay, with CXX marked as broken)
Comment 6 commit-hook freebsd_committer freebsd_triage 2016-11-01 17:47:34 UTC
A commit references this bug:

Author: jbeich
Date: Tue Nov  1 17:46:53 UTC 2016
New revision: 425062
URL: https://svnweb.freebsd.org/changeset/ports/425062

Log:
  devel/libtuntap: add new port

  PR:		213956
  Submitted by:	Mahdi Mokhtari <mokhi64@gmail.com>

  libtuntap is a library for configuring TUN or TAP devices in a portable manner.

  TUN and TAP are virtual networking devices which allow
  userland applications to receive packets sent to it.

  The userland applications can also send their own packets to the devices
  and they will be forwarded to the kernel.

  https://github.com/LaKabane/libtuntap

Changes:
  head/devel/Makefile
  head/devel/libtuntap/
  head/devel/libtuntap/Makefile
  head/devel/libtuntap/distinfo
  head/devel/libtuntap/files/
  head/devel/libtuntap/files/patch-CMakeLists.txt
  head/devel/libtuntap/files/patch-bindings_cpp_tuntap++.cc
  head/devel/libtuntap/pkg-descr
  head/devel/libtuntap/pkg-plist
Comment 7 Jan Beich freebsd_committer freebsd_triage 2016-11-01 17:48:56 UTC
Thanks. Landed with minor changes.
Comment 8 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-11-01 19:23:47 UTC
(In reply to Jan Beich (mail not working) from comment #7)
Good job :)
Thanks.