diff --git a/CHANGELOG.md b/CHANGELOG.md index 518521bde..dd756184a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), * The `JuliaHub.submit_job` function now allows submitting jobs that expose ports (via the `expose` argument). Related to that, the new `JuliaHub.request` function offers a simple interface for constructing authenticated HTTP.jl requests against the job, and the domain name of the job can be accessed via the new `.hostname` property of the `Job` object. (#14, #52) +## Version v0.1.10 - 2024-05-31 + +### Changed + +* When submitting an appbundle with the two-argument `JuliaHub.appbundle(bundle_directory, codefile)` method, JuliaHub.jl now ensures that `@__DIR__` `@__FILE`, and `include()` in the user code now work correctly. There is a subtle behavior change due to this, where now the user script _must_ be present within the uploaded appbundle tarball (previously it was possible to use a file that would get filtered out by `.juliabundleignore`). (#37, #53) + ## Version v0.1.9 - 2024-03-13 ### Fixed diff --git a/Project.toml b/Project.toml index 5156262ed..66fdcc694 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "JuliaHub" uuid = "bc7fa6ce-b75e-4d60-89ad-56c957190b6e" authors = ["JuliaHub Inc."] -version = "0.1.9" +version = "0.1.10" [deps] Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f" diff --git a/docs/src/guides/jobs.md b/docs/src/guides/jobs.md index b003e7bc0..60646c33c 100644 --- a/docs/src/guides/jobs.md +++ b/docs/src/guides/jobs.md @@ -79,7 +79,8 @@ The Julia environment in the directory is also immediately added into the bundle An appbundle can be constructed with the [`appbundle`](@ref) function, which takes as arguments the path to the directory to be bundled up, and a script _within that directory_. This is meant to be used for project directories where you have your Julia environment in the top level of the directory or repository. -For example, you can submit an bundle from a submit script on the top level of your project directory as follows: + +For example, suppose you have a script at the top level of your project directory, then you can submit a bundle as follows: ```@example import JuliaHub # hide @@ -98,8 +99,9 @@ When the job starts on JuliaHub, this environment is instantiated. A key feature of the appbundle is that development dependencies of the environment (i.e. packages added with `pkg> develop` or `Pkg.develop()`) are also bundled up into the archive that gets submitted to JuliaHub (including any current, uncommitted changes). Registered packages are installed via the package manager via the standard environment instantiation, and their source code is not included in the bundle directly. -When the JuliaHub job starts, the bundle is unpacked into the `appbundle/` directory (relative to the starting working directory). -E.g. if you have a `mydata.dat` file in the bundled directory, you can access it in the script at `joinpath("appbundle", "mydata.dat")`. +When the JuliaHub job starts, the working directory is set to the root of the unpacked appbundle directory. +This should be kept in mind especially when launching a script that is not at the root itself, and trying to open other files from the appbundle in that script (e.g. with `open`). +You can still use `@__DIR__` to load files relative to the script, and `include`s also work as expected (i.e. relative to the script file). Finally, a `.juliabundleignore` file can be used to exclude certain directories, by adding the relevant [globs](https://en.wikipedia.org/wiki/Glob_(programming)), similar to how `.gitignore` files work. In addition, `.git` directories are also automatically excluded from the bundle. diff --git a/src/appbundle-driver.jl b/src/appbundle-driver.jl new file mode 100644 index 000000000..11966574f --- /dev/null +++ b/src/appbundle-driver.jl @@ -0,0 +1,13 @@ +# This in an automatically generated driver script generated by JuliaHub.jl +# when submitting an appbundle. +let + path_components = [{PATH_COMPONENTS}] + path = abspath(pwd(), path_components...) + if !isfile(path) + path_relative = joinpath(path_components...) + error(""" + Unable to load requested script: $(path_relative) + at $(path)""") + end + path +end |> include diff --git a/src/jobsubmission.jl b/src/jobsubmission.jl index 862e30d76..8c2b20fd5 100644 --- a/src/jobsubmission.jl +++ b/src/jobsubmission.jl @@ -698,16 +698,18 @@ end Construct an appbundle-type JuliaHub batch job configuration. An appbundle is a directory containing a Julia environment that is bundled up, uploaded to JuliaHub, and then unpacked and instantiated as the job starts. +The primary, two-argument method will submit a job that runs a file from within the appbundle (specified by `codefile`, +which must be a path relative to the root of the appbundle). The code that gets executed is read from `codefile`, which should be a path to Julia source file relative to `directory`. ```julia JuliaHub.appbundle(@__DIR__, "my-script.jl") ``` -Alternatively, if `codefile` is omitted, the code can be provided as a string via the `code` keyword argument. +Alternatively, if `codefile` is omitted, the code must be provided as a string via the `code` keyword argument. ```julia -JuliaHub.AppBundle( +JuliaHub.appbundle( @__DIR__, code = \""" @show ENV @@ -734,17 +736,31 @@ The following should be kept in mind about how appbundles are handled: instantiation, and their source code is not included in the bundle directly. * When the JuliaHub job starts, the bundle is unpacked and the job's starting working directory - is set to the `appbundle/` directory, and you can e.g. load the data from those files with - just `read("my-data.txt", String)`. - - Note that `@__DIR__` points elsewhere and, relatedly, `include` in the main script should be - used with an absolute path (e.g. `include(joinpath(pwd(), "my-julia-file.jl"))`). + is set to the root of the unpacked appbundle directory, and you can e.g. load the data from those + files with just `read("my-data.txt", String)`. !!! compat "JuliaHub 6.2 and older" - On some older JuliaHub versions (6.2 and older), the working directory was set to the parent - directory of `appbundle/`, and so it was necessary to do `joinpath("appbundle", "mydata.dat")` - to load the code. + On some JuliaHub versions (6.2 and older), the working directory was set to the parent directory + of unpacked appbundle (with the appbundle directory called `appbundle`), and so it was necessary + to do `joinpath("appbundle", "mydata.dat")` to load files. + +* When submitting appbundles with the two-argument `codefile` method, you can expect `@__DIR__` and + `include` to work as expected. + + However, when submitting the Julia code as a string (via the `code` keyword argument), the behavior of + `@__DIR__` and `include` should be considered undefined and subject to change in the future. + +* The one-argument + `code` keyword argument method is a lower-level method, that more closely mirrors + the underlying platform API. The custom code that is passed via `code` is sometimes referred to as the + "driver script", and the two-argument method is implemented by submitting an automatically + constructed driver script that actually loads the specified file. + +!!! compat "Deprecation: v0.1.10" + + As of JuliaHub.jl v0.1.10, the ability to launch appbundles using the two-argument method where + the `codefile` parameter point to a file outside of the appbundle itself, is deprecated. You can still + submit the contents of the script as the driver script via the `code` keyword argument. """ function appbundle end @@ -769,14 +785,56 @@ function appbundle( end function appbundle(bundle_directory::AbstractString, codefile::AbstractString; kwargs...) - codefile = abspath(joinpath(bundle_directory, codefile)) haskey(kwargs, :code) && throw(ArgumentError("'code' keyword not supported if 'codefile' passed")) - isfile(codefile) || - throw(ArgumentError("'codefile' does not point to an existing file: $codefile")) - appbundle(bundle_directory; kwargs..., code=read(codefile, String)) + codefile_fullpath = abspath(bundle_directory, codefile) + isfile(codefile_fullpath) || + throw(ArgumentError("'codefile' does not point to an existing file: $codefile_fullpath")) + codefile_relpath = relpath(codefile_fullpath, bundle_directory) + # It is possible that the user passes a `codefile` path that is outside of the appbundle + # directory. This used to work back when `codefile` was just read() and submitted as the + # code argument. So we still support this, but print a loud deprecation warning. + if startswith(codefile_relpath, "..") + @warn """ + Deprecated: codefile outside of the appbundle $(codefile_relpath) + The support for codefiles outside of the appbundle will be removed in a future version. + Also note that in this mode, the behaviour of @__DIR__, @__FILE__, and include() with + a relative path are undefined. + + To avoid the warning, but retain the old behavior, you can explicitly pass the code + keyword argument instead of `codefile`: + + JuliaHub.appbundle( + bundle_directory; + code = read(joinpath(bundle_directory, codefile), String), + kwargs... + ) + """ + appbundle(bundle_directory; kwargs..., code=read(codefile_fullpath, String)) + else + # TODO: we could check that codefile actually exists within the appbundle tarball + # (e.g. to also catch if it is accidentally .juliabundleignored). This would require + # Tar.list-ing the bundled tarball, and checking that the file is in there. + driver_script = replace( + _APPBUNDLE_DRIVER_TEMPLATE, + "{PATH_COMPONENTS}" => _tuple_encode_path_components(codefile_relpath), + ) + appbundle(bundle_directory; kwargs..., code=driver_script) + end end +# We'll hard-code the file path directly into the driver script as string literals. +# We trust here that repr() will take care of any necessary escaping of the path +# components. In the end, we'll write the path "x/y/z" into the file as +# +# "x", "y", "z" +# +# Note: splitting up the path into components also helps avoid any cross-platform +# path separator issues. +_tuple_encode_path_components(path) = join(repr.(splitpath(path)), ",") + +const _APPBUNDLE_DRIVER_TEMPLATE = read(abspath(@__DIR__, "appbundle-driver.jl"), String) + function _upload_appbundle(appbundle_tar_path::AbstractString; auth::Authentication) isfile(appbundle_tar_path) || throw(ArgumentError("Appbundle file missing: $(appbundle_tar_path)")) diff --git a/test/jobenvs/job1/my-dependent-script.jl b/test/jobenvs/job1/my-dependent-script.jl new file mode 100644 index 000000000..bfebd5fc8 --- /dev/null +++ b/test/jobenvs/job1/my-dependent-script.jl @@ -0,0 +1 @@ +const MY_DEPENDENT_SCRIPT_1 = true diff --git a/test/jobenvs/job1/script.jl b/test/jobenvs/job1/script.jl index 759e6829d..7a4b32d3e 100644 --- a/test/jobenvs/job1/script.jl +++ b/test/jobenvs/job1/script.jl @@ -5,15 +5,31 @@ toml = TOML.parsefile(projecttoml) datastructures_version = toml["version"] # Check for the appbundle file -datafile = joinpath(@__DIR__, "appbundle", "datafile.txt") +datafile = joinpath(@__DIR__, "datafile.txt") datafile_hash = if isfile(datafile) bytes2hex(open(SHA.sha1, datafile)) end +# Try to load dependencies with relative paths: +script_include_success = try + include("my-dependent-script.jl") + include("subdir/my-dependent-script-2.jl") + true +catch e + e isa SystemError || rethrow() + @error "Unable to load" + false +end + results = Dict( "datastructures_version" => datastructures_version, "datafile_hash" => datafile_hash, "iswindows" => Sys.iswindows(), + "scripts" => Dict( + "include_success" => script_include_success, + "script_1" => isdefined(Main, :MY_DEPENDENT_SCRIPT_1), + "script_2" => isdefined(Main, :MY_DEPENDENT_SCRIPT_2), + ), ) @info "Storing RESULTS:\n$(results)" diff --git a/test/jobenvs/job1/subdir/my-dependent-script-2.jl b/test/jobenvs/job1/subdir/my-dependent-script-2.jl new file mode 100644 index 000000000..4278f9ba0 --- /dev/null +++ b/test/jobenvs/job1/subdir/my-dependent-script-2.jl @@ -0,0 +1 @@ +const MY_DEPENDENT_SCRIPT_2 = true diff --git a/test/jobs-live.jl b/test/jobs-live.jl index b32c3256a..23ab5bfb3 100644 --- a/test/jobs-live.jl +++ b/test/jobs-live.jl @@ -305,6 +305,13 @@ end @test VersionNumber(results["datastructures_version"]) == v"0.17.0" @test haskey(results, "datafile_hash") @test results["datafile_hash"] == datafile_hash + @test haskey(results, "scripts") + let s = results["scripts"] + @test s isa AbstractDict + @test get(s, "include_success", nothing) === true + @test get(s, "script_1", nothing) === true + @test get(s, "script_2", nothing) === true + end end end diff --git a/test/jobs.jl b/test/jobs.jl index 32103e1a6..94cc60083 100644 --- a/test/jobs.jl +++ b/test/jobs.jl @@ -81,12 +81,38 @@ end end end +function is_valid_julia_code(code::AbstractString) + try + ex = Meta.parse(code) + if ex.head === :incomplete + @error "Incomplete Julia expression in Julia code\n$(code)" ex + return false + end + catch exception + if isa(exception, Meta.ParseError) + @error "Invalid Julia code\n$(code)" exception + return false + end + end + return true +end + @testset "JuliaHub.appbundle" begin + driver_file_first_line = first(eachline(IOBuffer(JuliaHub._APPBUNDLE_DRIVER_TEMPLATE))) jobfile(path...) = joinpath(JOBENVS, "job1", path...) bundle = JuliaHub.appbundle(jobfile(), "script.jl") @test isfile(bundle.environment.tarball_path) - @test bundle.code == read(jobfile("script.jl"), String) + @test startswith(bundle.code, driver_file_first_line) + @test contains(bundle.code, "\"script.jl\"") + @test is_valid_julia_code(bundle.code) + + bundle = JuliaHub.appbundle(jobfile(), "subdir/my-dependent-script-2.jl") + @test isfile(bundle.environment.tarball_path) + @test startswith(bundle.code, driver_file_first_line) + @test contains(bundle.code, "\"subdir\"") + @test contains(bundle.code, "\"my-dependent-script-2.jl\"") + @test is_valid_julia_code(bundle.code) bundle = JuliaHub.appbundle(jobfile(); code="test()") @test isfile(bundle.environment.tarball_path) @@ -116,7 +142,17 @@ end cd(jobfile()) do bundle = JuliaHub.appbundle(".", "script.jl") @test isfile(bundle.environment.tarball_path) - @test bundle.code == read(jobfile("script.jl"), String) + @test startswith(bundle.code, driver_file_first_line) + @test contains(bundle.code, "\"script.jl\"") + @test is_valid_julia_code(bundle.code) + end + + # Deprecated case, where `codefile` comes from outside of the appbundle + # directory. In that case, `codefile` gets attached directly as the driver + # script. + let bundle = @test_logs (:warn,) JuliaHub.appbundle(jobfile(), "../job-dist/script.jl") + @test isfile(bundle.environment.tarball_path) + @test bundle.code == read(jobfile("../job-dist/script.jl"), String) end end