#11602: install_scripts should use "$@" instead of $*
----------------------+-----------------------------------------------------
Reporter: Stefan | Owner: jason
Type: defect | Status: needs_review
Priority: minor | Milestone: sage-4.7.2
Component: misc | Keywords: install_scripts, hg, command line
Work_issues: | Upstream: N/A
Reviewer: | Author:
Merged: | Dependencies:
----------------------+-----------------------------------------------------
Comment(by jhpalmieri):
Replying to [comment:16 leif]:
> Ok, `install_scripts()` now (IMHO unfortunately) requires more of Sage,
which prevents usage from a simple shell script, but the problem with an
already set `SAGE_ROOT` seems to be solved as far as I can see. (The
"useless" `os.path.exists()` is a bit odd, but I see `os.path.samefile()`
requires this.)
Yes, and on OS X, there is at least one nonexistent item in the PATH,
because of these lines in sage-env:
{{{
if [ `uname` = "Darwin" ]; then
# For a framework Python build on OS X, Python's bin directory is not
local/bin
PATH="$SAGE_LOCAL/Frameworks/Python.framework/Versions/2.5/bin:$PATH"
&& export PATH
}}}
Note the "2.5". (And of course, someone might have some non-existent
directories in their path to start with.)
> The `stderr=PIPE` and `stdout=PIPE` in `have_program()` are quite
useless; there apparently is no such thing like `DEVNULL`, but we can
redirect both to `/dev/null` in the string passed to the shell (instead of
opening `/dev/null` and passing its `fileno()` to `subprocess.call()`).
Redirecting `stdout` should suffice anyway.
I don't see any problem with using PIPE to suppress output. Having
redirections as part of the shell command seems somehow less elegant to
me.
> The description of `ignore_existing` appears clumsy (and potentially
misleading) to me; I'd just say ''"If True, install a shortcut to Sage's
version of a program even if a system-wide version exists [or: is in
the/your PATH]."''.
Okay.
> Maybe we should also mention that the destination directory has to be
added to `PATH` unless it is already contained in it. (We could even check
this in `install_scripts()`.)
I've added some messages about this, both in the docstring and the message
printed as the function finishes.
> There btw. is a typo in the docstring, `s/`verbosely tell`/`verbosely
tell'''s'''`/` (and `ignore_existsing`[sic] in a comment). For better
readability, I'd also substitute `c` by e.g. `cmd` in the code.
Okay.
> Also, the message ''"The command ... is not available ..."'' should read
''"... not available '''in Sage'''"''. The `ìf-elif-else` is IMHO a bit
inappropriate, as the otherwise very verbose script doesn't tell if a
system-wide version was found but we install a shortcut because
`ignore_existing==True`. It could be:
{{{
#!python
if not cmd_inside_sage:
# We *may* also report if a system-wide version is available.
...
continue
if cmd_outside_sage:
print "The command '%s' is installed outside of Sage." % cmd
if not ignore_existing: # ambiguous unless one reads the
docstring
print "Not installing a shortcut to Sage's version."
continue
# normal operation, simply install the shortcut unless a file with
# the same name already exists in the target directory
...
}}}
I changed it to be something like this.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11602#comment:17>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
