Bug 250461 - New port: databases/proxysql ProxySQL is a high performance proxy for MySQL and forks
Summary: New port: databases/proxysql ProxySQL is a high performance proxy for MySQL a...
Status: Closed Overcome By Events
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-19 13:47 UTC by Felipe Zipitria
Modified: 2020-10-19 18:17 UTC (History)
1 user (show)

See Also:


Attachments
New port diff (15.80 KB, patch)
2020-10-19 13:47 UTC, Felipe Zipitria
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Felipe Zipitria 2020-10-19 13:47:56 UTC
Created attachment 218887 [details]
New port diff

This port adds proxysql.

I've been discussing with the maintainers about their dependency usage:

- they compile all their own libraries, and from what I've commented, they had problems in the past with specific versions so Rene prefers to use their embedded ones
- there is the dependency on libinjection, that has a build dependency on python2. This is a real problem, because we are phasing out python2 in the near future.

The library recently passed from client9 to new maintainers in libinjection/libinjection. And then when they fix this dependency, we need to talk with upstream to include the new libinjection in their deps.

Right now, the good thing is that it is usable and keeps the spirit from upstream.
Comment 1 daniel.engberg.lists 2020-10-19 17:24:52 UTC
Python 2.x is going to be removed in less than 2 months (at the end of the year) so this probably isn't going get committed.

As for using vendor provided libs, this is more or less discouraged https://www.freebsd.org/doc/en/books/porters-handbook/book.html#bundled-libs and for a good reason. Looking at vendor repo there's a lot of _very_ old libs with known vulns. https://github.com/sysown/proxysql/tree/v2.0.15/deps

Mixing and matching different build frameworks is also less than ideal and can (probably will) lead to interesting failures.

Makefile: L6+L2+29 - Check section 5.4.3
https://www.freebsd.org/doc/en/books/porters-handbook/book.html#makefile-distfiles
Makefile: L7 - Remove, PORTREVISION starts at 0 and doesn't need to be set.
Makefile: L14 -  Use tab instead of space (style, consistency)
Makefile: L22 - Remove +
Makefile: L24 - For the sake of readability use alphabetical order
Makefile: L36-37 - No, read  section 6.27
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/book.html#users-and-groups
Makefile: L58-59 - Looking at L61-L62 just set them directly or just ${PORTNAME} if you insist on using a variable?
Makefile: L84 is definitely a no go, that needs to be fixed within the port.

Unless it greatly improves readability there's no need to define a variable and only use it once.

portlint will probably find a few issues too
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/book.html#testing-portlint
A poudriere run will also be very helpful.

I'm by far no expert but the mentioned issues above looks incorrect to me at first glance.
Comment 2 Felipe Zipitria 2020-10-19 18:17:13 UTC
Well, thanks for all the comments.

I guess this needs to be solved in upstream first ¯\_(ツ)_/¯.

Will retry when libinjection gets proper py3 support.