Bug 165623 - Mk/bsd.comands.mk et al -- conflicting uses of ${FILE}
Summary: Mk/bsd.comands.mk et al -- conflicting uses of ${FILE}
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Port Management Team
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-02 14:00 UTC by Matthew Seaman
Modified: 2017-06-26 00:30 UTC (History)
0 users

See Also:


Attachments
FILE_CMD.diff (7.26 KB, patch)
2012-03-02 14:00 UTC, Matthew Seaman
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Seaman freebsd_committer 2012-03-02 14:00:21 UTC
bsd.commands.mk has:

FILE?=		/usr/bin/file

but by far the most common use of ${FILE} in the ports is as a loop
control variable for iterating through a list of files.  This seems
contrary and undesirable.

Therefore change the variable to ${FILE_CMD} when the meaning is to
run the file(1) application.  There are only 10 instances that I can
find by exhaustive search of the ports.

Tinderbox results: https://redports.org/buildarchive/20120301135606-15513/
Comment 1 Michael Scheidell freebsd_committer 2012-03-02 21:12:53 UTC
does portlint need to be upgraded to tell you to use ${FILE_CMD} instead 
of ${FILE} ?

-- 
Michael Scheidell, CTO
o: 561-999-5000
d: 561-948-2259
 >*| *SECNAP Network Security Corporation

    * Best Mobile Solutions Product of 2011
    * Best Intrusion Prevention Product
    * Hot Company Finalist 2011
    * Best Email Security Product
    * Certified SNORT Integrator
Comment 2 Chris Rees 2012-03-03 12:56:27 UTC
On 2 Mar 2012 23:37, "Matthew Seaman" <m.seaman@infracaninophile.co.uk>
wrote:
>
> On 02/03/2012 21:20, Michael Scheidell wrote:
> >  does portlint need to be upgraded to tell you to use ${FILE_CMD}
instead
> >  of ${FILE} ?
>
> Perhaps, but that's going to annoy the vast majority of people that use
> ${FILE} to mean something other than the file(1) application.  I tend
> towards the view that there's so much prior art, and that saying:
>
> .for FILE in ${LIST_OF_FILES}
> ...
> .endfor
>
> is so natural a construct that trying to make people do it differently
> would be wildly unpopular.
>

Or we could put the portlint check in, and people who don't want to see it
complain could use lowers for loop vars.

Sorry to go on about this, but it's very important to differentiate,
considering that their behaviour is very different from regular variables.

Note that I withdraw my objection to FILE_CMD as per pointers from others :)

Chris
Comment 3 Matthew Seaman 2012-03-03 13:29:41 UTC
On 03/03/2012 12:56, Chris Rees wrote:

> Or we could put the portlint check in, and people who don't want to see it
> complain could use lowers for loop vars.
> 
> Sorry to go on about this, but it's very important to differentiate,
> considering that their behaviour is very different from regular variables.
> 
> Note that I withdraw my objection to FILE_CMD as per pointers from others :)


So make it a matter of style that loop iterators should be lower case?
That is an idea I can agree with.  Not being too prescriptive about it,
and allowing people to make that modification organically -- when they
have to update a port for another reason, etc. is also something I can
agree with.

Although it turns out there's fewer instances of using uppercase
iterators than I at first thought:

lucid-nonsense:/usr/ports:% grep -rlE '^\. *for +[A-Z]+' * | wc -l
     341

	Cheers,

	Matthew

-- 
Dr Matthew J Seaman MA, D.Phil.                   7 Priory Courtyard
                                                  Flat 3
PGP: http://www.infracaninophile.co.uk/pgpkey     Ramsgate
JID: matthew@infracaninophile.co.uk               Kent, CT11 9PW
Comment 4 Chris Rees 2012-03-03 13:34:58 UTC
On 3 March 2012 13:29, Matthew Seaman <m.seaman@infracaninophile.co.uk> wro=
te:
> On 03/03/2012 12:56, Chris Rees wrote:
>
>> Or we could put the portlint check in, and people who don't want to see =
it
>> complain could use lowers for loop vars.
>>
>> Sorry to go on about this, but it's very important to differentiate,
>> considering that their behaviour is very different from regular variable=
s.
>>
>> Note that I withdraw my objection to FILE_CMD as per pointers from other=
s :)
>
> So make it a matter of style that loop iterators should be lower case?
> That is an idea I can agree with. =A0Not being too prescriptive about it,
> and allowing people to make that modification organically -- when they
> have to update a port for another reason, etc. is also something I can
> agree with.
>
> Although it turns out there's fewer instances of using uppercase
> iterators than I at first thought:
>
> lucid-nonsense:/usr/ports:% grep -rlE '^\. *for +[A-Z]+' * | wc -l
> =A0 =A0 341

I think it's a matter of style, but also makes people think harder
when they make boo-boos like:

OTHER=3D TWO

.for iterator in ONE TWO THREE
.  if ${iterator} =3D=3D ${OTHER}
FOO=3D  bar
.  endif
.endfor

which ends up as a malformed conditional.  Alarm bells ring more
quickly if one sees the lowercase used on the left like this. (should
be ${OTHER} =3D=3D ${iterator})

Have I confused anyone yet?

Chris
Comment 5 Martin Wilke freebsd_committer 2012-03-03 15:35:13 UTC
Responsible Changed
From-To: freebsd-ports-bugs->freebsd-portmgr

over to portmgr
Comment 6 Mark Linimon freebsd_committer freebsd_triage 2012-03-05 16:16:09 UTC
Responsible Changed
From-To: freebsd-portmgr->portmgr

Canonicalize assignment.
Comment 7 Mark Linimon freebsd_committer freebsd_triage 2014-06-02 01:17:03 UTC
Infrastructure PR.
Comment 8 Mathieu Arnold freebsd_committer 2015-06-13 01:49:39 UTC
I think a better thing would be to just make it a policy to have .for variables that are always lowercase, and batch fix the ports tree to use lowercase variables.