Bug 237315

Comment 1 Tobias Kortkamp 2019-05-01 07:09:34 UTC

Comment on attachment 203718 [details]
pichi.shar

Please add

# $FreeBSD$

to the first line then run portlint and fix everything it raises:

WARN: Makefile: [14]: USE_* seen before USES.  According to the porters-handbook, USES must appear first.
WARN: pkg-descr: includes lines that exceed 80 characters.
WARN: Makefile: [39]: use tab (not space) to make indentation
WARN: Makefile: [40]: use tab (not space) to make indentation
WARN: Makefile: [41]: use tab (not space) to make indentation
WARN: Makefile: [43]: use tab (not space) to make indentation
WARN: Makefile: [44]: use tab (not space) to make indentation
WARN: Makefile: [45]: use tab (not space) to make indentation
WARN: Makefile: [46]: use tab (not space) to make indentation
WARN: Makefile: [47]: use tab (not space) to make indentation
WARN: Makefile: [48]: use tab (not space) to make indentation
WARN: Makefile: [49]: use tab (not space) to make indentation
WARN: Makefile: [50]: use tab (not space) to make indentation
WARN: Makefile: COMMENT is not supposed to begin with 'A ', 'An ', or 'The '.
WARN: Makefile: Setting PORTREVISION to 0 is not necessary.
WARN: Makefile: extra item placed in the USES/USE_x section, for example, "PLIST_SUB".
WARN: Makefile: "BUILD_DEPENDS" has to appear earlier.
WARN: Makefile: "USES" has to appear earlier.
0 fatal errors and 18 warnings found.

Several of these items are formatting related, so please also try and
reformat the port like other ports are formatted. ports-mgmt/portfmt
can help you there.

XPLIST_SUB= VERSION=1.2.0

PLIST_SUB=	VERSION=${DISTVERSION}

XCMAKE_ARGS= -DCMAKE_CXX_FLAGS=NDEBUG -DVERSION=1.2.0

This looks bogus.  What is this supposed to do?

XINSTALL_TARGET= test install/strip

Hmm, this looks weird.  Note that we generally do not force our
users to run tests during regular builds.  Maybe add

TEST_TARGET=	test

instead?

X.if ${PORT_OPTIONS:MTLS}
XUSES+=  ssl
XSTATIC_BUILD_DEPENDS+=	openssl:security/openssl
XSTATIC_LIB_DEPENDS_OFF+=	libssl.so:security/openssl libcrypto.so:security/openssl
X.endif

This is wrong.  You can never depend on security/openssl directly
(users can select other versions of it and they are all mutually
exclusive to each other).  This should be something more like

.if ${PORT_OPTIONS:MTLS}
.if ${PORT_OPTIONS:MSTATIC}
USES+=	ssl:build
.else
USES+=	ssl
.endif
.endif

XTLS_DESC= Enable TLS support(requiring OpenSSL)

Add a space before '(' and s/requiring/requires/.

X	${INSTALL_DATA} ${WRKSRC}/include/pichi.h ${STAGEDIR}${PREFIX}/include/pichi.h

Hmm, would this still be necessary if the INSTALL_HEADERS cmake
option would be enabled instead of disabled?

Created attachment 204217 [details]
pichi.shar

Fix all errors and warnings complained by portlint.

Please ignore my previous coments. Not all comments you gave have fixed.

Created attachment 204223 [details]
pichi.shar

All comments fixed except:

1. There's 'WARN: Makefile: "BUILD_DEPENDS" has to appear earlier.' warning reported by portlint I don't know how to fix.
2. The installation of 'pichi.h' is necessary here because INSTALL_HEADERS option is going to install all C++ headers which are not mostly needed. Furthermore, this option has a bug, that not all headers are correctly installed, in version 1.2.0, and it's supposed to be fixed in the next version.

Comment 6 Tobias Kortkamp 2019-05-18 09:41:01 UTC

(In reply to Pichi from comment #5)
> Created attachment 204223 [details]
> pichi.shar
> 
> All comments fixed except:
> 
> 1. There's 'WARN: Makefile: "BUILD_DEPENDS" has to appear earlier.' warning
> reported by portlint I don't know how to fix.

Put it in its own block between LICENSE_FILE and USES.

See the Porter's Handbook:
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/porting-order.html

> 2. The installation of 'pichi.h' is necessary here because INSTALL_HEADERS
> option is going to install all C++ headers which are not mostly needed.
> Furthermore, this option has a bug, that not all headers are correctly
> installed, in version 1.2.0, and it's supposed to be fixed in the next
> version.

Ok.

XUSE_GITHUB=	yes
XGH_ACCOUNT=	pichi-router

This needs to go directly after USES.

X	${INSTALL_DATA} ${WRKSRC}/server/pichi.json.default ${STAGEDIR}${ETCDIR}/pichi.json.default

Is pichi.json.default something that users need to copy to pichi.json?
Maybe install it as pichi.json.sample and add @sample to it in pkg-plist
to do this on package install.

All the pichi_*() functions in files/pichi.in need to be moved to
just before run_rc_command at the end.

Comment 7 Tobias Kortkamp 2019-05-18 09:49:33 UTC

XUSE_RC_SUBR=	pichi

This needs to be moved into the USES block.

Test building this on FreeBSD 12.0 fails with:

FAILED: src/CMakeFiles/pichi_lib.dir/net/asio.cpp.o 
/usr/bin/c++  -DBOOST_ASIO_NO_DEPRECATED -Dpichi_lib_EXPORTS -I/wrkdirs/usr/ports/net/pichi/work/pichi-1.2.0/include -Iinclude -I/usr/local/include -Isrc -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -Os -DNDEBUG -fPIC   -std=c++17 -MD -MT src/CMakeFiles/pichi_lib.dir/net/asio.cpp.o -MF src/CMakeFiles/pichi_lib.dir/net/asio.cpp.o.d -o src/CMakeFiles/pichi_lib.dir/net/asio.cpp.o -c /wrkdirs/usr/ports/net/pichi/work/pichi-1.2.0/src/net/asio.cpp
In file included from /wrkdirs/usr/ports/net/pichi/work/pichi-1.2.0/src/net/asio.cpp:3:
In file included from /usr/local/include/boost/asio/connect.hpp:20:
In file included from /usr/local/include/boost/asio/basic_socket.hpp:21:
/usr/local/include/boost/asio/detail/io_object_impl.hpp:87:40: error: no member named 'get_executor' in 'boost::asio::execution_context'
      implementation_executor_(context.get_executor(),
                               ~~~~~~~ ^
/usr/local/include/boost/asio/ip/basic_resolver.hpp:118:7: note: in instantiation of function template specialization 'boost::asio::detail::io_object_impl<boost::asio::detail::resolver_service<boost::asio::ip::tcp>, boost::asio::executor>::io_object_impl<boost::asio::execution_context>' requested here
    : impl_(context)
      ^
/wrkdirs/usr/ports/net/pichi/work/pichi-1.2.0/src/net/asio.cpp:82:25: note: in instantiation of function template specialization 'boost::asio::ip::basic_resolver<boost::asio::ip::tcp, boost::asio::executor>::basic_resolver<boost::asio::execution_context>' requested here
                        tcp::resolver{s.get_executor().context()}.async_resolve(
                        ^
1 error generated.
ninja: build stopped: subcommand failed.
*** Error code 1

Stop.
make: stopped in /usr/ports/net/pichi
=>> Cleaning up wrkdir
===>  Cleaning for pichi-1.2.0
build of net/pichi | pichi-1.2.0 ended at Sat May 18 11:46:00 CEST 2019
build time: 00:00:52
!!! build failure encountered !!!

Full log: https://people.freebsd.org/~tobik/logs/pichi-1.2.0.log

About building failure, it seems that Boost.Asio changed its API from 1.70.0, which was released around releasing time of pichi 1.2.0. A patch file is going to be append to SHAR file later.

Created attachment 204497 [details]
pichi.shar

(In reply to Tobias Kortkamp from comment #6)

> Put it in its own block between LICENSE_FILE and USES.
> 
> See the Porter's Handbook:
> https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/porting-> order.html

Done. Thanks for your link.

> Is pichi.json.default something that users need to copy to pichi.json?
> Maybe install it as pichi.json.sample and add @sample to it in pkg-plist
> to do this on package install.

Yes and done.

> All the pichi_*() functions in files/pichi.in need to be moved to
> just before run_rc_command at the end.

Done.

> XUSE_RC_SUBR=	pichi
> 
> This needs to be moved into the USES block.

Done.

> Test building this on FreeBSD 12.0 fails with:
> 
> FAILED: src/CMakeFiles/pichi_lib.dir/net/asio.cpp.o 
> ...

It was caused by Boost.Asio 1.70.0 having been changed its API. Patch files attached are supposed to fix it.

Comment 10 commit-hook 2019-11-10 06:17:22 UTC

A commit references this bug:

Author: rodrigo
Date: Sun Nov 10 06:16:27 UTC 2019
New revision: 517183
URL: https://svnweb.freebsd.org/changeset/ports/517183

Log:
  New port: net/pichi An Application Layer Proxy controlled via RESTful APIs

  PR:		237315
  Submitted by:	Pichi <pichi@elude.in>

Changes:
  head/net/Makefile
  head/net/pichi/
  head/net/pichi/Makefile
  head/net/pichi/distinfo
  head/net/pichi/files/
  head/net/pichi/files/patch-CMakeLists.txt
  head/net/pichi/files/patch-cmake_ProcessOptions.cmake
  head/net/pichi/files/patch-include_config.h.in
  head/net/pichi/files/patch-src_net_asio.cpp
  head/net/pichi/files/pichi.in
  head/net/pichi/pkg-descr
  head/net/pichi/pkg-plist

Comment 11 Rodrigo Osorio 2019-11-10 06:22:34 UTC

Committed with minor changes, thanks.

- Add USE_LDCONFIG requested by portlint
- Add $FreeBSD$ in files/pichi.in