public inbox for ecos-maintainers@sourceware.org
 help / color / mirror / Atom feed
From: John Dallaway <john@dallaway.org.uk>
To: Bart Veer <bartv@ecoscentric.com>, Gary Thomas <gary@mlbassoc.com>
Cc: ecos-maintainers@ecos.sourceware.org
Subject: Re: #!/usr/bin/env tclsh
Date: Fri, 06 Feb 2009 13:49:00 -0000	[thread overview]
Message-ID: <498C3FEB.5020508@dallaway.org.uk> (raw)
In-Reply-To: <pn4oz7g3d6.fsf@delenn.bartv.net>

Hi Bart and Gary

Bart Veer wrote:

>>>>>> "John" == John Dallaway <john@dallaway.org.uk> writes:
> 
>     John> This patch simplifies the #! magic used to invoke Tcl
>     John> scripts by using "/usr/bin/env tclsh" to find the tclsh
>     John> executable. Very old Cygwin installations providing only
>     John> tclsh83.exe or cygtclsh80.exe are no-longer supported.
>     John> Checked-in.
> 
> Actually, this patch has broken things in various ways. Consider e.g.
> file2c.tcl in the romfs package. The CDL invokes this using e.g.:
> 
>   sh file2c.tcl testromfs_le.bin testromfs_le.h
> 
> With the old magic this still worked fine because sh would ignore the
> #! at the start completely and move on to the 'exec sh -c' on line 3.
> With the new '#!/usr/bin/env tclsh' the sh invocation ignores the
> #! comment on line 1 so ends up trying to run the whole Tcl script as
> a shell script. Needless to say this is not very successful.
> 
> io/framebuf is similarly affected. services/memalloc/common is not. I
> have not yet checked all the other packages that use Tcl scripts.

Good catch.

> Possible solutions are:
> 
> 1) revert the change
> 2) remove the 'sh' bits from the relevant CDL scripts, treating the
>    Tcl script as plain executables.
> 3) make the CDL invoke /usr/bin/env tclsh directly, treating the
>    Tcl scripts as Tcl scripts.
> 
> (1) would be a bad move. I think I would prefer (3) to (2).

Option 2 presents problems with CVS checkouts of the eCos repository
where execute permissions on the various Tcl scripts cannot be assumed.

I agree with Bart that invoking "/usr/bin/env tclsh" within the various
CDL custom rules doesn't gain us anything. It seems entirely reasonable
to assume that tclsh is on the PATH and users will get an intelligible
error message if it is not. In the case of heapgen.tcl, I simply
modified the custom make rule to invoke "tclsh" rather than "sh" and I
suggest we do the same with the other scripts. Are you OK with this
proposal?

If we can agree on this, I am happy to identify and implement these changes.

John Dallaway

  parent reply	other threads:[~2009-02-06 13:49 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <496635B7.8060808@dallaway.org.uk>
     [not found] ` <49663810.3010202@eCosCentric.com>
2009-01-08 17:51   ` John Dallaway
2009-01-08 18:21     ` Jonathan Larmour
2009-02-06 12:24 ` Bart Veer
2009-02-06 12:38   ` Gary Thomas
2009-02-06 12:53     ` Bart Veer
2009-02-06 13:49   ` John Dallaway [this message]
2009-02-06 14:03     ` Gary Thomas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=498C3FEB.5020508@dallaway.org.uk \
    --to=john@dallaway.org.uk \
    --cc=bartv@ecoscentric.com \
    --cc=ecos-maintainers@ecos.sourceware.org \
    --cc=gary@mlbassoc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).