Bug 127077 - Echo loses quotes
Summary: Echo loses quotes
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-build (show other bugs)
Version: 3.0.1   Edit
Hardware: PC Windows XP
: P3 minor (vote)
Target Milestone: 3.1   Edit
Assignee: Leo Treggiari CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2006-02-09 10:33 EST by Neil Rickards CLA
Modified: 2008-06-19 13:37 EDT (History)
0 users

See Also:


Attachments
Patch against CDT 3.0.2 RC1 (8.91 KB, patch)
2006-02-10 10:27 EST, Neil Rickards CLA
bjorn.freeman-benson: iplog+
Details | Diff
Patch against CDT 3.0.2 RC1 removing duplicate code (16.54 KB, patch)
2006-02-10 10:28 EST, Neil Rickards CLA
no flags Details | Diff
Patch against head (17.19 KB, patch)
2006-02-10 13:15 EST, Neil Rickards CLA
bjorn.freeman-benson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Neil Rickards CLA 2006-02-09 10:33:14 EST
Before the build command is inserted into the makefile a command of the form
    @echo myTool --option="C:\my files\temp.foo"
is inserted.  When this is printed the quotes are lost which is confusing.

One option is to put single quotes around the buildCmd:
    @echo 'myTool --option="C:\my files\temp.foo"'
however this would fail if buildCmd included a single quote

A better solution would be to escape the string being passed to echo:
    @echo myTool --option=\"C:\\my files\\temp.foo\"
Comment 1 Neil Rickards CLA 2006-02-10 10:27:00 EST
Created attachment 34495 [details]
Patch against CDT 3.0.2 RC1

Simple patch - approx. 23 pertinent lines
Comment 2 Neil Rickards CLA 2006-02-10 10:28:48 EST
Created attachment 34497 [details]
Patch against CDT 3.0.2 RC1 removing duplicate code

Patch that includes removal of 40 lines of duplicate code.  Should be functionally equivalent to attachment 34495 [details].  Approx 80 pertinent lines
Comment 3 Doug Schaefer CLA 2006-02-10 10:39:08 EST
RC2? There was no 3.0.2 RC2.
Comment 4 Leo Treggiari CLA 2006-02-10 11:07:34 EST
Hi Neil,

Doug can correct me if I'm wrong, but I believe that 3.0.2 is done, and we have no plans currently for another release on the 3.0 branch.  If you could apply your change to Head and generate the patch, that would make my life easier...

Thanks,
Leo
Comment 5 Neil Rickards CLA 2006-02-10 11:08:08 EST
Sorry - my mistake.  Tag was CDT_3_0_2_RC1
Comment 6 Doug Schaefer CLA 2006-02-10 11:45:02 EST
That is the plan of record. ISVs are free to take this patch and apply it locally. And if something serious comes up we may still do a 3.0.3.

We do need a patch for HEAD to get this into 3.1. Thanks Neil!
Comment 7 Neil Rickards CLA 2006-02-10 11:53:17 EST
No problem - I'm working on it now.  HEAD appears to require Eclipse > 3.1.1 - I'll try and get it finished today (UK time)
Comment 8 Leo Treggiari CLA 2006-02-10 11:58:21 EST
Hi Neil,

Yes, Head requires Eclipse 3.2 M4.  And, there is no rush.

Thanks again,
Leo
Comment 9 Neil Rickards CLA 2006-02-10 13:15:48 EST
Created attachment 34506 [details]
Patch against head
Comment 10 Leo Treggiari CLA 2006-02-13 13:18:03 EST
I've applied the changes in my workspace where I am making changes for the new dependency calculator interface.  I'll check them in at the same time - hopefully this week.
Comment 11 Leo Treggiari CLA 2006-02-23 13:05:59 EST
I have committed the changes to Head.
Comment 12 Leo Treggiari CLA 2006-03-30 13:11:03 EST
This change has caused a problem - see #133981.  The problem occurs with an "announcement" line, and not the echoing of a command line.  I haven't thought about this in detail yet.  Anyone have a suggestion?

Comment 13 Leo Treggiari CLA 2006-03-30 14:50:38 EST
I'm currently thinking that we should return the single quotes to the various "announcement" echos, but keep the new mechanism for command echos.  A better solution might be to make the behavior "shell" specific, but we don't currently have any knowledge of the shell the user is using.

Please speak up if you object.
Comment 14 Neil Rickards CLA 2006-03-31 05:30:16 EST
Given both the announcement and command lines are being run through the same echo shell command, I don't see the advantage to treating them separately.  I also know of tools that use parentheses on their command line.

The best we can do (other than knowing which shell we're using or moving away from makefiles) is to produce a command that works with as many shells as possible.  The best I can come up with is:
  - single quote entire string
  - no need to escape " \ / ( ) characters
  - replace ' with '"'"'

This works correctly on redhat and cygwin.  echo on DOS actually prints what you write, but I don't think we ever call that

Incidentally - why do we echo the command, then issue it with an @ to suppress the echo?  Couldn't we just issue the command without the @ and let it echo naturally?
Comment 15 Leo Treggiari CLA 2006-03-31 09:14:52 EST
Hi Neil,

(In reply to comment #14)
> This works correctly on redhat and cygwin.  echo on DOS actually prints what
> you write, but I don't think we ever call that

We have to worry about Windows for people who want to intagrate a Windows tool-chain (e.g., Visual C++).

> Incidentally - why do we echo the command, then issue it with an @ to suppress
> the echo?  Couldn't we just issue the command without the @ and let it echo
> naturally?

I don't know the answer to that.  MBS has done that since before I got involved.  Does anyone else know?
Comment 16 Chris Recoskie CLA 2006-03-31 09:58:53 EST
I don't know why either I'm afraid :-(
Comment 17 Neil Rickards CLA 2006-03-31 10:41:44 EST
Hi Leo,

With the above escaping scheme, Cygwin users would see the same as on RedHat.  For example:
  Starting build using Microsoft's Compiler(R)

MinGW users could well be using the Windows echo in which case they would see:
  'Starting build using Microsoft'"'"'s Compiler(R)'

This would be a problem with the echo only - the tool would behave correctly.  The alternative is to have the build actually break on the other platforms
Comment 18 Leo Treggiari CLA 2006-04-06 18:32:04 EDT
I asked Sean Evoy (the original MBS developer) about echoing the commands and then invoking them preceded by a '@'.  His answer was:

"Purely aesthetic on my part: I liked to see the commands as they were being executed. If it causes you problems, by all means get rid of it :-)".

So, I'd be willing to try just executing build commands without an 'echo' and the '@'.  Then we could go back to enclosing "build progress" echos in single quotes.  What do others think?
Comment 19 Mikhail Sennikovsky CLA 2006-04-07 10:51:45 EDT
(In reply to comment #18)
Hi Leo,

> So, I'd be willing to try just executing build commands without an 'echo' and
> the '@'.  Then we could go back to enclosing "build progress" echos in single
> quotes.  What do others think?
I agree with you.

Mikhail
Comment 20 Neil Rickards CLA 2006-04-07 10:56:08 EDT
I agree that seeing the commands being exectued is invaluable for debugging but these should be echoed automatically if we don't prepend the '@'

Making this change and going back to single quotes both sound like good idea.  Might still be worth replacing ' with '"'"' just in-case
Comment 21 Chris Recoskie CLA 2006-04-07 18:05:02 EDT
I agree with Leo and Neil.  I think the apostrophe issue is so uncommon that it will essentially never come up, but with Neil's fix at least the build will function if it does.
Comment 22 Leo Treggiari CLA 2006-04-28 09:41:11 EDT
I have committed the change that we agreed to, including "escaping" the single quotes in an echoed string.