Bug 35304

Description Chapman Flack 2003-03-19 12:54:33 EST

You guessed right about my OS and platform (Linux/IA32), but the issue is
cross-platform.

The Ext Connection Method preference UI has a single line for the parameters to
the chosen command.It does not define what syntax the user needs to use to
specify where the line should break into separate elements of the cmdarray
argument to java.lang.Runtime.exec. What's worse, what the getExtCommand method
actually DOES is not any of the arguably correct choices. It uses a default
StringTokenizer and passes every token as a separate argument, without honoring
ANY convention for quoting arguments or escaping delimiters.

The :ext: connection type should allow a user to build a CVS connection out of
any suitable command or script. It's essential to pass the right parameters to
it unmangled.  The current implementation is a near-blocker to any user with an
:ext: command that takes any kind of parameters beyond single words with no
spaces. I say "near-blocker" only because the user can probably write a custom
script that embeds or unmangles the parameters, and invoke the script as a
wrapper for the intended command.  But because of bug 35142, even that approach
is somewhat hellish to test and get working.

This is the same issue the Ant developers ran into, and that led them to add
nested <arg> elements to ant's <exec> task so the developer has the needed
control.There are two arguably sensible approaches:

1. Retain the single parameter line in the UI, but make no attempt to parse it
into separate parameters. Instead, pass it intact and unbroken to the underlying
platform's command interpreter (e.g. with exec( { "sh", "-c", cmd+" "+params })
on a posixlike platform). That way you get the correct quoting and escaping
conventions for the platform (which are too subtle to ever try to replicate
within Eclipse; would be almost sure to get them wrong), and the user would
enter the parameter line in the familiar syntax and be confident of the right
semantics. Drawbacks: (minor) makes the syntax platform-specific;
(major) makes the implementation platform-specific (Eclipse has to know whether
to invoke sh or CMD or whatever).

2. Follow the ant approach and change the UI to have a separate text field for
each parameter (say, with a "next parameter" button to add another text field),
or some kind of explicit graphical doodad (out of band, not based on text
syntax) for delimiting parameters.  Map the parameters one-to-one and absolutely
untouched (except ${user} ${password} ${host} ${port} substitution, and even
those should probably be doodads) to the elements of cmdarray.

I'd favor approach 2.

On a related note, another evil thing getExtCommand does is force the parameter
list to be "${host} -l ${user}" if it is empty.  It should be enough that the UI
comes up with that as a default initial value. The current implementation blocks
a user setting up any command or script that doesn't take any extra parameters.
As in the first problem, a clever user can pass a dummy parameter and write a
wrapper script that discards the first parameter, and invoke that in place of
the real command; and as in the first problem, because of bug 35142, the user
has to figure out what's going wrong and test the workaround, blind.