modern Tcl and correct quoting

[Rainer Müller]

Hello,

On 2013-06-12 08:05, Poor Yorick wrote:
> macport.tcl appears to have originally been written prior to Tcl 8.5, which was
> released in 2007, and uses idioms that are now considered obsolete.  There are
> also instances where curly brackets are used in combination with variable
> interpolation to attempt to create a well-formed list value, but this
is not
> robust.

Older releases of Mac OS X ships with Tcl 8.4 and that is what we are
currently targeting. I think Mac OS X 10.6 Snow Leopard first introduced
Tcl 8.5. While we officially support the latest two releases of Mac OS
X, we avoid breaking MacPorts deliberately on older releases at the
moment due to using newer language constructs.

Once we decide to drop support for older releases of Mac OS X, we could
move on and use these newer constructs. There are also some custom
additions we are using which would become obsolete.

> The attached diff contains various changes, some of which, admittedly, are a
> question of style, but many others have a real impact on correctness and
> performance.  Changes which are not fluff include:

Good work! Nice to see you digged right into one of the core source file
there.

However, many different things are changed in this single patch. With
regard to my comment above, could you please split it into the parts
that can be applied without breaking compatibility with 8.4 and
introducing features of 8.5? This would help a lot to get the easier
fixes in.

> 	eliminate redundant calls to [expr] in [if] tests.

This sounds fine and could be applied without breaking 8.4 compatibility.

> 	use "eq" instead of "==" where appropriate

Fine as well.

> 	brace arguments to [expr]

Is there a specific reason for this or is it just for style?

> 	replace [eval] with {*} expansion where appropriate

As you wrote yourself, {*} is not available in Tcl 8.4.

> 	use [list] to correctly quote arguments to [eval]

Hm, what is the reason for this change? As far as I understand it, eval
takes additional arguments to pass to the script passed in the first
argument. So what is the difference here?

> 	use [string map] to correcty interpolate variable arguments into
> 	dynamically-generated scripts.

I don't understand this change. What is the difference?

> Changes which are somewhat fluff:
> 
> 	remove quotes around single variables

Okay.

> Changes which are fluff:
> 
> 	replace "" with {}

Somehow, for programmers of C-like languages using "" for string
quotation feels more natural :-)

> 	remove quotes around values when they are not needed

The style that evolved in the MacPorts community is to use ${foo} for
expansion instead of $foo both in Portfiles and in base. I don't think
there is a technical reason for this, it just happened to be
historically this way.

Most of us learned Tcl by writing code for Portfiles and base, so this
style just sticks.

> 	remove extra spaces between things like parenthesis and curly brackets and their contents

Okay.

> 	replace double quotes with curly brackets where appropriate

(You already said that above)

> So far, I haven't succesfully set up a macports development environment where I
> could test the changes in this diff, but I did use a static syntax analyzer to
> catch as many typos as possible.

We can help you to set up a development environment for this.
The best is to set up a separate installation and ports tree for
development to avoid working on your main tree as you probably still
want to use the software from MacPorts. Follow the guide for this here:

http://guide.macports.org/#installing.macports.source.multiple

Note that you probably need to remove /opt/local/bin and /opt/local/sbin
from your PATH for building the second MacPorts instance in order to
link against Mac OS X system libraries (and also does not compile
otherwise).

> I hope the macports team will see fit to modernize its Tcl code.  If these
> changes are accepted, I would be happy to make the same effort with other .tcl
> files in the project.

Great! Looking forward to more contributions already :-)

> A couple of other issues I'm interested in working on:
> 
> 	The implementation of [system] curently passes its commands through a
> 	shell, but this makes it necessary to add an extra layer of shell quoting
> 	to the arguments to [system].  Macports currently includes no [shquote]
> 	command, so extra layer of quoting has been manually hard-coded into the
> 	values that are passed, which is not correct and only works if one steers
> 	clear of the edge cases.  [system] could be implemented so that it bypasses
> 	the shell, which would be more simple and robust.

At some locations we might actually be doing stuff that requires a
shell. Most common would be things like "cd $dir && ./foo", but should
be replaced with system -W $dir to set the working directory.

> 	macports_util.tcl implements [try], which, as of Tcl 8.6, is included as a
> 	built-in command.  In my development environment, this leads to an error
> 	when [package require] is invoked.  I propose to rename the macports [try]
> 	so that it can coexist with the builtin.

Sounds reasonable. Our the custom try-catch that is based upon TIP #89
[1] instead of TIP #329 [2], which is what has been implemented in Tcl 8.6.

[1] http://www.tcl.tk/cgi-bin/tct/tip/89
[2] http://www.tcl.tk/cgi-bin/tct/tip/329