Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assign PROGRAM_FILE earlier #22092

Merged
merged 7 commits into from
Jun 13, 2017
+69 −42
Merged

Assign PROGRAM_FILE earlier #22092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9e0be07
Assign PROGRAM_FILE earlier
omus May 26, 2017
3b44286
Add to NEWS
omus May 29, 2017
ebf6b03
Test startup-file and PROGRAM_FILE
omus Jun 5, 2017
32c7e79
Modify user home properly on Windows
omus Jun 6, 2017
c5a1506
Style correction and remove escape function
omus Jun 7, 2017
8d4d7ac
Expand arguments tests to include the startup-file
omus Jun 12, 2017
7db08b5
Refactor PROGRAM_FILE tests
omus Jun 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ This section lists changes that do not have deprecation warnings.
of the socket. Previously the address of the remote endpoint was being
returned ([#21825]).

* Using `ARGS` within the ~/.juliarc.jl or within a .jl file loaded with `--load` will no
longer contain the script name as the first argument. Instead the script name will be
assigned to `PROGRAM_FILE`. ([#22092])

Library improvements
--------------------

Expand Down
9 changes: 6 additions & 3 deletions base/client.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ function process_options(opts::JLOptions)
color_set = (opts.color != 0)
global have_color = (opts.color == 1)
global is_interactive = (opts.isinteractive != 0)

# remove filename from ARGS
arg_is_program = opts.eval == C_NULL && opts.print == C_NULL && !isempty(ARGS)
global const PROGRAM_FILE = arg_is_program ? shift!(ARGS) : ""

while true
# startup worker.
# opts.startupfile, opts.load, etc should should not be processed for workers.
Expand Down Expand Up @@ -296,11 +301,9 @@ function process_options(opts::JLOptions)
break
end
# load file
if !isempty(ARGS) && !isempty(ARGS[1])
if !isempty(PROGRAM_FILE)
# program
repl = false
# remove filename from ARGS
global PROGRAM_FILE = shift!(ARGS)
if !is_interactive
ccall(:jl_exit_on_sigint, Void, (Cint,), 1)
end
Expand Down
2 changes: 1 addition & 1 deletion base/initdefs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ A string containing the script name passed to Julia from the command line. Note
script name remains unchanged from within included files. Alternatively see
[`@__FILE__`](@ref).
"""
PROGRAM_FILE = ""
:PROGRAM_FILE

"""
ARGS
Expand Down
96 changes: 58 additions & 38 deletions test/cmdlineargs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,53 +248,73 @@ let exename = `$(Base.julia_cmd()) --precompiled=yes --startup-file=no`
# tested in test/parallel.jl, test/examples.jl)
@test !success(`$exename --worker=true`)

escape(str) = replace(str, "\\", "\\\\")

# test passing arguments
let testfile = tempname()
try
# write a julia source file that just prints ARGS to STDOUT
write(testfile, """
println(ARGS)
mktempdir() do dir
testfile = joinpath(dir, tempname())
# write a julia source file that just prints ARGS to STDOUT
write(testfile, """
println(ARGS)
""")
@test readchomp(`$exename $testfile foo -bar --baz`) ==
"String[\"foo\", \"-bar\", \"--baz\"]"
@test readchomp(`$exename $testfile -- foo -bar --baz`) ==
"String[\"foo\", \"-bar\", \"--baz\"]"
cp(testfile, joinpath(dir, ".juliarc.jl"))

withenv((is_windows() ? "USERPROFILE" : "HOME") => dir) do
output = "String[\"foo\", \"-bar\", \"--baz\"]"
@test readchomp(`$exename $testfile foo -bar --baz`) == output
@test readchomp(`$exename $testfile -- foo -bar --baz`) == output
@test readchomp(`$exename -L $testfile -e 'exit(0)' -- foo -bar --baz`) ==
"String[\"foo\", \"-bar\", \"--baz\"]"
@test split(readchomp(`$exename -L $testfile $testfile`), '\n') ==
["String[\"$(escape(testfile))\"]", "String[]"]
output
@test readchomp(`$exename --startup-file=yes -e 'exit(0)' -- foo -bar --baz`) ==
output

output = "String[]\nString[]"
@test readchomp(`$exename -L $testfile $testfile`) == output
@test readchomp(`$exename --startup-file=yes $testfile`) == output

@test !success(`$exename --foo $testfile`)
@test readchomp(`$exename -L $testfile -e 'exit(0)' -- foo -bar -- baz`) == "String[\"foo\", \"-bar\", \"--\", \"baz\"]"
finally
rm(testfile)
@test readchomp(`$exename -L $testfile -e 'exit(0)' -- foo -bar -- baz`) ==
"String[\"foo\", \"-bar\", \"--\", \"baz\"]"
end
end

# test the script name
let a = tempname(), b = tempname()
try
write(a, """
println(@__FILE__)
println(PROGRAM_FILE)
println(length(ARGS))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't care about testing ARGS length any more?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Originally I added this since the number of arguments could vary depending on when we would run the check. I could still include this check but it would only be for verifying that the ARGS were no longer includes the PROGRAM_FILE as ARGS[1]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not keep checking the new expected value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests prior to this ("test passing arguments") deals with testing the number of ARGS already. Having them in this section is redundant and just adds noise as the length(ARGS) == 0. I'll make sure that the previous section covers what these tests previously did.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added new tests to address this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Tony

include(\"$(escape(b))\")
# test the program name remains constant
mktempdir() do dir
a = joinpath(dir, "a.jl")
b = joinpath(dir, "b.jl")
c = joinpath(dir, ".juliarc.jl")

write(a, """
println(@__FILE__)
println(PROGRAM_FILE)
include(\"$(escape_string(b))\")
""")
write(b, """
println(@__FILE__)
println(PROGRAM_FILE)
println(length(ARGS))
write(b, """
println(@__FILE__)
println(PROGRAM_FILE)
""")
@test split(readchomp(`$exename $a`), '\n') ==
["$a", "$a", "0", "$b", "$a", "0"]
@test split(readchomp(`$exename -L $b -e 'exit(0)'`), '\n') ==
["$(realpath(b))", "", "0"]
@test split(readchomp(`$exename -L $b $a`), '\n') ==
["$(realpath(b))", "", "1", "$a", "$a", "0", "$b", "$a", "0"]
finally
rm(a)
rm(b)
cp(b, c)

readsplit(cmd) = split(readchomp(cmd), '\n')

withenv((is_windows() ? "USERPROFILE" : "HOME") => dir) do
@test readsplit(`$exename $a`) ==
[a, a,
b, a]
@test readsplit(`$exename -L $b -e 'exit(0)'`) ==
[realpath(b), ""]
@test readsplit(`$exename -L $b $a`) ==
[realpath(b), a,
a, a,
b, a]
@test readsplit(`$exename --startup-file=yes -e 'exit(0)'`) ==
[c, ""]
@test readsplit(`$exename --startup-file=yes -L $b -e 'exit(0)'`) ==
[c, "",
realpath(b), ""]
@test readsplit(`$exename --startup-file=yes -L $b $a`) ==
[c, a,
realpath(b), a,
a, a,
b, a]
end
end

Expand Down