FATAL: ports/ftp/lftp/pkg-plist: [28]: this port installs .desktop files. Please add `desktop-file-utils` to USES. WARN: Makefile: [63]: use of != in assignments is almost never a good thing to do. Try to avoid using them. See http://lists.freebsd.org/pipermail/freebsd-ports/2008-July/049777.html for some helpful hints on what to do instead.
Created attachment 246033 [details] [PATCH 1/2] ftp/lftp: lint to pass sanity checks: add desktop-file-utils to USES
Created attachment 246034 [details] [PATCH 2/2] ftp/lftp: lint to pass sanity checks: avoid use of != in assignments
Created attachment 246100 [details] [PATCH 3/3] ftp/lftp: Lint with portclippy(1) and portfmt(1)
Hello Andrey, Thanks for your contribution. Despite portlint warnings, lftp does not need 'USES=desktop-file-utils', because it has no MimeType entry in its .desktop file, see: See: https://docs.freebsd.org/en/books/porters-handbook/book/#updating-desktop-database so I'll skip patch 1. Patch 3 does not apply cleanly. Could you update it please ?
Patch 2 seems a bit odd to me as what we want to do is executing the ldd command to check for libthr.so presence. We cannot remove the '!=' here. Also, you are assigning the string to a new variable, while we check for the old variable later in the code, see: .if !empty(NEED_PTHREAD) LDFLAGS+= -pthread .endif Best regards, Ganael.
Thank you for reviewing. On second patch: sorry for not stating upfront my reason behind it. The portlint(1) warning mentioned this issue discussed on mailing list: https://lists.freebsd.org/pipermail/freebsd-ports/2008-July/049777.html I understood it as just another form of setting variable value to the output of commands chain. Is it indeed so? Do you think this may be applicable here? I am ready to cancel the patches would you not like them. Best regards, Andrey
On third patch: I'm waiting for your answer about the second one to make it properly.
(In reply to Andrey Korobkov from comment #6) Thanks for the link, that point is interesting. The aim of this advice is to run processes only when necessary. It probably mostly applies to Mk/* files as they are embedded with other ports. In the case of ftp/lftp, we are talking about a *leaf* Makefile, with a lower incidence. Moreover, moving the execution to the 'if' statement (for example) would not improve things as it is not located in a specific target. The process would be executed at the same time it is currently with the '!=' assignment. I would prefer leaving the call as is, to facilitate readability/maintainability of the port. Could you update patch 3 so that I can take a look at it please ?
Created attachment 246217 [details] [PATCH] ftp:/lftp: Lint with portclippy(1) and portfmt(1) Of course. Here it is.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=a822421ac76be8f974de51d35470eab65610332f commit a822421ac76be8f974de51d35470eab65610332f Author: Andrey Korobkov <alster@vinterdalen.se> AuthorDate: 2023-11-10 11:32:45 +0000 Commit: Ganael LAPLANCHE <martymac@FreeBSD.org> CommitDate: 2023-11-10 11:32:45 +0000 ftp/lftp: lint port Re-order variables and fix COMMENT PR: 274839 Reported by: Andrey Korobkov <alster@vinterdalen.se> ftp/lftp/Makefile | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-)
Patch committed, thanks Andrey!