Bug 270706 - Mk/Uses/nodejs.mk: fix '.if empty()' usage
Summary: Mk/Uses/nodejs.mk: fix '.if empty()' usage
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: Luca Pizzamiglio
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-04-08 15:56 UTC by John Hein
Modified: 2023-04-09 23:59 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (pizzamig)


Attachments
[patch] fix nodejs_ARGS test in Mk/Uses/nodejs.mk (539 bytes, patch)
2023-04-08 16:02 UTC, John Hein
jcfyecrayz: maintainer-approval? (sunpoet)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Hein 2023-04-08 15:56:47 UTC
Mk/Uses/nodejs.mk currently has the following:

.  if !empty(${nodejs_ARGS:Nbuild:Nrun:Nlts:Ncurrent:N14:N16:N18:N19})
IGNORE=     USES=nodejs has invalid arguments ${nodejs_ARGS}.
.  endif


'empty' should take an expression that is a variable or variable with modifiers - such as empty(FOO) or empty(FOO:Nsomething).  It should not normally take the expansion of the expression - such as empty(${FOO}) or empty(${FOO:Nsomething}) - unless the expansion is a desired variable expression to be tested for emptiness itself.

For nodejs.mk it wants to check if the nodejs_ARGS variable (with modifiers) is empty, so it should be:

.  if !empty(nodejs_ARGS:Nbuild:Nrun:Nlts:Ncurrent:N14:N16:N18:N19)
IGNORE=     USES=nodejs has invalid arguments ${nodejs_ARGS}.
.  endif


But there's another error, namely the :N modifiers should include a test for 'env'.  It appears omission of that was just an oversight.  So:

.  if !empty(nodejs_ARGS:Nbuild:Nenv:Nrun:Nlts:Ncurrent:N14:N16:N18:N19)
IGNORE=     USES=nodejs has invalid arguments ${nodejs_ARGS}.
.  endif


Locally this mistake was noticed after www/yarn added USES=nodejs:env, and it just so happened that the environment had 'env' defined in the environment.  So that triggered the following incorrect error:

% env env=x make -C www/yarn extract
===>  yarn-1.22.18_1 USES=nodejs has invalid arguments env..
*** Error code 1


After fixing the 'empty()' expression as described above, that same command does not failure.

If you only fix the missing :Nenv without removing the incorrect expansion of nodejs_ARGS and modifiers, then the invalid arguments test does not catch the invalid argument as it should:

% make -C www/yarn extract USES=metaport\ nodejs:inval
===> Fetching all distfiles required by yarn-1.22.18_1 for building
===>  Extracting for yarn-1.22.18_1
Comment 1 John Hein 2023-04-08 16:02:26 UTC
Created attachment 241355 [details]
[patch] fix nodejs_ARGS test in Mk/Uses/nodejs.mk

The attached patch fixes the incorrect expansion of nodejs_ARGS in the empty() test.  It also adds the missing :Nenv modifier in that test.
Comment 2 commit-hook freebsd_committer freebsd_triage 2023-04-09 13:45:34 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=c98bdc21c65e97a62c9290d38e002ec99d8905bc

commit c98bdc21c65e97a62c9290d38e002ec99d8905bc
Author:     John Hein <jcfyecrayz@liamekaens.com>
AuthorDate: 2023-04-09 13:35:48 +0000
Commit:     Po-Chuan Hsieh <sunpoet@FreeBSD.org>
CommitDate: 2023-04-09 13:37:36 +0000

    Mk/Uses/nodejs.mk: Fix nodejs_ARGS check

    - Fix empty() usage
    - Add missing valid USES=nodejs:env

    PR:             270706

 Mk/Uses/nodejs.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 3 Po-Chuan Hsieh freebsd_committer freebsd_triage 2023-04-09 13:45:57 UTC
Committed. Thanks!
Comment 4 John Hein 2023-04-09 23:59:14 UTC
(In reply to Po-Chuan Hsieh from comment #3)
Thanks.

I just noticed the extra period in the IGNORE

===>  yarn-1.22.18_1 USES=nodejs has invalid arguments env..

Next time someone commits to this file, the trailing period should be removed.