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.

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

Yet more inference/invalidation fixes #1869

Merged
merged 8 commits into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ language: julia

julia:
# - 1.0
- 1.4
# - 1.4
- nightly

os:
- linux
- osx

env:
- JULIA_PKG_SERVER="https://pkg.julialang.org"
- JULIA_PKG_SERVER=""

notifications:
email: false

Expand All @@ -33,7 +37,7 @@ script:
jobs:
include:
- stage: docs
julia: 1.4
julia: nightly
os: linux
script:
- julia -e 'using UUIDs; write("Project.toml", replace(read("Project.toml", String), r"uuid = .*?\n" =>"uuid = \"$(uuid4())\"\n"))'
Expand Down
4 changes: 3 additions & 1 deletion src/API.jl
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,9 @@ function gc(ctx::Context=Context(); collect_delay::Period=Day(7), kwargs...)
function write_condensed_usage(usage_by_depot, fname)
for (depot, usage) in usage_by_depot
# Keep only the keys of the files that are still extant
usage = filter(p -> p[1] in all_index_files, usage)
usage = let oldusage = usage
filter(p -> p[1] in all_index_files, oldusage)
end

# Expand it back into a dict of arrays-of-dicts
usage = Dict(k => [Dict("time" => v)] for (k, v) in usage)
Expand Down
6 changes: 3 additions & 3 deletions src/Artifacts.jl
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ end
Given an `entry` for the artifact named `name`, located within the file `artifacts_toml`,
returns the `Platform` object that this entry specifies. Returns `nothing` on error.
"""
function unpack_platform(entry::Dict, name::String, artifacts_toml::String)
function unpack_platform(entry::Dict, name::String, artifacts_toml::String)::Union{Nothing,Platform}
if !haskey(entry, "os")
@error("Invalid artifacts file at '$(artifacts_toml)': platform-specific artifact entry '$name' missing 'os' key")
return nothing
Expand All @@ -403,9 +403,9 @@ function unpack_platform(entry::Dict, name::String, artifacts_toml::String)

# Helpers to pull out `Symbol`s and `VersionNumber`s while preserving `nothing`.
nosym(x::Nothing) = nothing
nosym(x) = Symbol(lowercase(x))
nosym(x) = Symbol(lowercase(x))::Symbol
nover(x::Nothing) = nothing
nover(x) = VersionNumber(x)
nover(x) = VersionNumber(x)::VersionNumber

# Extract architecture, libc, libgfortran version and cxxabi (if given)
arch = nosym(get(entry, "arch", nothing))
Expand Down
8 changes: 4 additions & 4 deletions src/BinaryPlatforms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ julia> arch(MacOS())
:x86_64
```
"""
arch(p::Platform) = p.arch
arch(p::Platform) = p.arch::Symbol
arch(u::UnknownPlatform) = nothing

"""
Expand All @@ -271,7 +271,7 @@ julia> libc(Linux(:aarch64))
julia> libc(FreeBSD(:x86_64))
```
"""
libc(p::Platform) = p.libc
libc(p::Platform) = p.libc::Union{Nothing,Symbol}
libc(u::UnknownPlatform) = nothing

"""
Expand All @@ -288,7 +288,7 @@ julia> call_abi(FreeBSD(:armv7l))
:eabihf
```
"""
call_abi(p::Platform) = p.call_abi
call_abi(p::Platform) = p.call_abi::Union{Nothing,Symbol}
call_abi(u::UnknownPlatform) = nothing

"""
Expand All @@ -301,7 +301,7 @@ julia> compiler_abi(Linux(:x86_64))
CompilerABI()
```
"""
compiler_abi(p::Platform) = p.compiler_abi
compiler_abi(p::Platform) = p.compiler_abi::CompilerABI
compiler_abi(p::UnknownPlatform) = CompilerABI()

# Also break out CompilerABI getters for our platforms
Expand Down
32 changes: 17 additions & 15 deletions src/Operations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,8 @@ function set_maximum_version_registry!(ctx::Context, pkg::PackageSpec)
pathvers = keys(load_versions(ctx, path; include_yanked=false))
union!(pkgversions, pathvers)
end
if length(pkgversions) == 0
pkg.version = VersionNumber(0)
else
max_version = maximum(pkgversions)
pkg.version = VersionNumber(max_version.major, max_version.minor, max_version.patch, max_version.prerelease, ("",))
end
max_version = maximum(pkgversions; init=VersionNumber(0))
pkg.version = VersionNumber(max_version.major, max_version.minor, max_version.patch, max_version.prerelease, ("",))
end

function collect_project!(ctx::Context, pkg::PackageSpec, path::String,
Expand Down Expand Up @@ -631,7 +627,12 @@ end
function download_artifacts(ctx::Context, pkgs::Vector{PackageSpec}; platform::Platform=platform_key_abi(),
verbose::Bool=false)
# Filter out packages that have no source_path()
pkg_roots = String[p for p in source_path.((ctx,), pkgs) if p !== nothing]
# pkg_roots = String[p for p in source_path.((ctx,), pkgs) if p !== nothing] # this runs up against inference limits?
pkg_roots = String[]
for pkg in pkgs
p = source_path(ctx, pkg)
p !== nothing && push!(pkg_roots, p)
end
return download_artifacts(ctx, pkg_roots; platform=platform, verbose=verbose)
end

Expand Down Expand Up @@ -694,12 +695,12 @@ function download_source(ctx::Context, pkgs::Vector{PackageSpec},
end

widths = [textwidth(pkg.name) for (pkg, _) in pkgs_to_install]
max_name = length(widths) == 0 ? 0 : maximum(widths)
max_name = maximum(widths; init=0)

########################################
# Install from archives asynchronously #
########################################
jobs = Channel(ctx.num_concurrent_downloads);
jobs = Channel{eltype(pkgs_to_install)}(ctx.num_concurrent_downloads);
results = Channel(ctx.num_concurrent_downloads);
@async begin
for pkg in pkgs_to_install
Expand Down Expand Up @@ -741,7 +742,7 @@ function download_source(ctx::Context, pkgs::Vector{PackageSpec},

missed_packages = Tuple{PackageSpec, String}[]
for i in 1:length(pkgs_to_install)
pkg, exc_or_success, bt_or_path = take!(results)
pkg::PackageSpec, exc_or_success, bt_or_path = take!(results)
exc_or_success isa Exception && pkgerror("Error when installing package $(pkg.name):\n",
sprint(Base.showerror, exc_or_success, bt_or_path))
success, path = exc_or_success, bt_or_path
Expand Down Expand Up @@ -906,7 +907,7 @@ function build_versions(ctx::Context, uuids::Vector{UUID}; might_need_to_resolve
# toposort builds by dependencies
order = dependency_order_uuids(ctx, map(first, builds))
sort!(builds, by = build -> order[first(build)])
max_name = isempty(builds) ? 0 : maximum(textwidth.([build[2] for build in builds]))
max_name = maximum(build->textwidth(build[2]), builds; init=0)
# build each package versions in a child process
for (uuid, name, source_path, version) in builds
pkg = PackageSpec(;uuid=uuid, name=name, version=version)
Expand Down Expand Up @@ -1649,8 +1650,9 @@ function diff_array(old_ctx::Union{Context,Nothing}, new_ctx::Context; manifest=
end
old = manifest ? load_manifest_deps(old_ctx) : load_direct_deps(old_ctx)
# merge old and new into single array
all_uuids = union([pkg.uuid for pkg in old], [pkg.uuid for pkg in new])
return [(uuid, index_pkgs(old, uuid), index_pkgs(new, uuid)) for uuid in all_uuids]
T, S = Union{UUID,Nothing}, Union{PackageSpec,Nothing}
all_uuids = union(T[pkg.uuid for pkg in old], T[pkg.uuid for pkg in new])
return Tuple{T,S,S}[(uuid, index_pkgs(old, uuid), index_pkgs(new, uuid)) for uuid in all_uuids]
end

function is_package_downloaded(ctx, pkg::PackageSpec)
Expand Down Expand Up @@ -1752,8 +1754,8 @@ function status(ctx::Context, pkgs::Vector{PackageSpec}=PackageSpec[];
old_ctx = Context(;env=env_diff)
end
# display
filter_uuids = [pkg.uuid for pkg in pkgs if pkg.uuid !== nothing]
filter_names = [pkg.name for pkg in pkgs if pkg.name !== nothing]
filter_uuids = [pkg.uuid::UUID for pkg in pkgs if pkg.uuid !== nothing]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
filter_uuids = [pkg.uuid::UUID for pkg in pkgs if pkg.uuid !== nothing]
filter_uuids = UUID[pkg.uuid for pkg in pkgs if pkg.uuid !== nothing]

maybe? Same below.

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 version here avoids one runtime dispatch and one invalidation-worthy call to convert (from inside the setindex! call):

julia> f(x) = Int[x]
f (generic function with 1 method)

julia> g(x) = [x::Int]
g (generic function with 1 method)

julia> code_typed(f, (Integer,))
1-element Array{Any,1}:
 CodeInfo(
1%1 = $(Expr(:foreigncall, :(:jl_alloc_array_1d), Array{Int64,1}, svec(Any, Int64), 0, :(:ccall), Array{Int64,1}, 1, 1))::Array{Int64,1}
│        Base.setindex!(%1, x, 1)::Any
└──      return %1
) => Array{Int64,1}

julia> code_typed(g, (Integer,))
1-element Array{Any,1}:
 CodeInfo(
1 ─      Core.typeassert(x, Main.Int)::Int64%2 = π (x, Int64)
│   %3 = $(Expr(:foreigncall, :(:jl_alloc_array_1d), Array{Int64,1}, svec(Any, Int64), 0, :(:ccall), Array{Int64,1}, 1, 1))::Array{Int64,1}
│        Base.arraysize(%3, 1)::Int64
└──      goto #3 if not true
2 ─      Base.arrayset(false, %3, %2, 1)::Array{Int64,1}
3 ┄      goto #4
4return %3
) => Array{Int64,1}

Copy link
Member

Choose a reason for hiding this comment

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

Alright, thanks for exaplining.

filter_names = [pkg.name::String for pkg in pkgs if pkg.name !== nothing]
diff = old_ctx !== nothing
header = something(header, diff ? :Diff : :Status)
if mode == PKGMODE_PROJECT || mode == PKGMODE_COMBINED
Expand Down
38 changes: 22 additions & 16 deletions src/PlatformEngines.jl
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,9 @@ function probe_platform_engines!(;verbose::Bool = false)
if haskey(ENV, "BINARYPROVIDER_DOWNLOAD_ENGINE")
engine = ENV["BINARYPROVIDER_DOWNLOAD_ENGINE"]
es = split(engine)
dl_ngs = filter(e -> e[1].exec[1:length(es)] == es, download_engines)
dl_ngs = let es=es
filter(e -> e[1].exec[1:length(es)] == es, download_engines)
end
if isempty(dl_ngs)
all_ngs = join([d[1].exec[1] for d in download_engines], ", ")
warn_msg = "Ignoring BINARYPROVIDER_DOWNLOAD_ENGINE as its value "
Expand All @@ -437,7 +439,9 @@ function probe_platform_engines!(;verbose::Bool = false)
if haskey(ENV, "BINARYPROVIDER_COMPRESSION_ENGINE")
engine = ENV["BINARYPROVIDER_COMPRESSION_ENGINE"]
es = split(engine)
comp_ngs = filter(e -> e[1].exec[1:length(es)] == es, compression_engines)
comp_ngs = let es=es
filter(e -> e[1].exec[1:length(es)] == es, compression_engines)
end
if isempty(comp_ngs)
all_ngs = join([c[1].exec[1] for c in compression_engines], ", ")
warn_msg = "Ignoring BINARYPROVIDER_COMPRESSION_ENGINE as its "
Expand Down Expand Up @@ -480,7 +484,7 @@ function probe_platform_engines!(;verbose::Bool = false)
end

# Search for a compression engine
for (test, unpack, package, list, parse, symlink) in compression_engines
for (test::Cmd, unpack, package, list, parse, symlink) in compression_engines
if probe_cmd(`$test`; verbose=verbose)
# Set our compression command generators
gen_unpack_cmd = unpack
Expand Down Expand Up @@ -608,7 +612,7 @@ function get_server_dir(url::AbstractString, server=pkg_server())
joinpath(depots1(), "servers", m.captures[1])
end

const AUTH_ERROR_HANDLERS = []
const AUTH_ERROR_HANDLERS = Pair{Union{String, Regex},Any}[]

function handle_auth_error(url, err; verbose::Bool = false)
handled, should_retry = false, false
Expand Down Expand Up @@ -640,7 +644,7 @@ is `true`.

`register_auth_error_handler` returns a zero-arg function that can be called to deregister the handler.
"""
function register_auth_error_handler(urlscheme::Union{AbstractString, Regex}, f)
function register_auth_error_handler(urlscheme::Union{AbstractString, Regex}, @nospecialize(f))
unique!(pushfirst!(AUTH_ERROR_HANDLERS, urlscheme => f))
return () -> deregister_auth_error_handler(urlscheme, f)
end
Expand All @@ -650,8 +654,8 @@ end

Removes `f` from the stack of authentication error handlers.
"""
function deregister_auth_error_handler(urlscheme::Union{AbstractString, Regex}, f)
filter!(handler -> handler !== (urlscheme => f), AUTH_ERROR_HANDLERS)
function deregister_auth_error_handler(urlscheme::Union{String, Regex}, @nospecialize(f))
filter!(handler -> !(handler.first == urlscheme && handler.second === f), AUTH_ERROR_HANDLERS)
return nothing
end

Expand All @@ -677,7 +681,7 @@ function get_auth_header(url::AbstractString; verbose::Bool = false)
@warn "auth file without access_token field" file=auth_file
return handle_auth_error(url, "no-access-token"; verbose=verbose)
end
auth_header = "Authorization: Bearer $(auth_info["access_token"])"
auth_header = "Authorization: Bearer $(auth_info["access_token"]::String)"
# handle token expiration and refresh
expires_at = Inf
if haskey(auth_info, "expires_at")
Expand Down Expand Up @@ -705,7 +709,7 @@ function get_auth_header(url::AbstractString; verbose::Bool = false)
end
verbose && @info "Refreshing expired auth token..." file=auth_file
tmp = tempname()
refresh_auth = "Authorization: Bearer $(auth_info["refresh_token"])"
refresh_auth = "Authorization: Bearer $(auth_info["refresh_token"]::String)"
try download(refresh_url, tmp, auth_header=refresh_auth, verbose=verbose)
catch err
@warn "token refresh failure" file=auth_file url=refresh_url err=err
Expand Down Expand Up @@ -740,7 +744,7 @@ function get_auth_header(url::AbstractString; verbose::Bool = false)
end
end
mv(tmp, auth_file, force=true)
return "Authorization: Bearer $(auth_info["access_token"])"
return "Authorization: Bearer $(auth_info["access_token"]::String)"
end

function hash_data(strs::AbstractString...)
Expand Down Expand Up @@ -780,7 +784,7 @@ function load_telemetry_file(file::AbstractString)
end
end
# bail early if fully opted out
get(info, "telemetry", true) == false && return info
get(info, "telemetry", true) === false && return info
# some validity checking helpers
is_valid_uuid(x) = false
is_valid_salt(x) = false
Expand Down Expand Up @@ -873,10 +877,12 @@ function get_telemetry_headers(url::AbstractString, notify::Bool=true)
system = Pkg.BinaryPlatforms.triplet(Pkg.BinaryPlatforms.platform_key_abi())
push!(headers, "Julia-System: $system")
# install-specific information
if info["client_uuid"] != false
push!(headers, "Julia-Client-UUID: $(info["client_uuid"])")
if info["secret_salt"] != false
project = Base.active_project()
if info["client_uuid"] !== false
client_uuid = info["client_uuid"]::String
push!(headers, "Julia-Client-UUID: $client_uuid")
if info["secret_salt"] !== false
secret_salt = info["secret_salt"]::String
salt_hash = hash_data("salt", client_uuid, secret_salt)
if project !== nothing
project_hash = hash_data("project", project, info["secret_salt"])
push!(headers, "Julia-Project-Hash: $project_hash")
Expand All @@ -885,7 +891,7 @@ function get_telemetry_headers(url::AbstractString, notify::Bool=true)
end
# CI indicator variables
ci_variables = get(info, "ci_variables", CI_VARIABLES)
ci_variables == true && (ci_variables = CI_VARIABLES)
ci_variables === true && (ci_variables = CI_VARIABLES)
if ci_variables != false
ci_info = String[]
for var in CI_VARIABLES ∩ map(uppercase, ci_variables)
Expand Down
10 changes: 5 additions & 5 deletions src/REPLMode/REPLMode.jl
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ function OptionSpecs(decs::Vector{OptionDeclaration})::Dict{String, OptionSpec}
specs = Dict()
for x in decs
opt_spec = OptionSpec(;x...)
@assert get(specs, opt_spec.name, nothing) === nothing # don't overwrite
@assert !haskey(specs, opt_spec.name) # don't overwrite
specs[opt_spec.name] = opt_spec
if opt_spec.short_name !== nothing
@assert get(specs, opt_spec.short_name, nothing) === nothing # don't overwrite
specs[opt_spec.short_name] = opt_spec
@assert !haskey(specs, opt_spec.short_name::String) # don't overwrite
specs[opt_spec.short_name::String] = opt_spec
end
end
return specs
Expand Down Expand Up @@ -104,8 +104,8 @@ function CommandSpecs(declarations::Vector{CommandDeclaration})::Dict{String,Com
@assert !haskey(specs, spec.canonical_name) "duplicate spec entry"
specs[spec.canonical_name] = spec
if spec.short_name !== nothing
@assert !haskey(specs, spec.short_name) "duplicate spec entry"
specs[spec.short_name] = spec
@assert !haskey(specs, spec.short_name::String) "duplicate spec entry"
specs[spec.short_name::String] = spec
end
end
return specs
Expand Down
4 changes: 2 additions & 2 deletions src/Registry.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module Registry
import ..Pkg, ..Types, ..API
using ..Pkg: depots1
using ..Types: RegistrySpec, Context, Context!

using UUIDs

"""
Pkg.Registry.add(url::String)
Expand Down Expand Up @@ -103,7 +103,7 @@ status(; kwargs...) = status(Context(); kwargs...)
function status(ctx::Context; io::IO=stdout, as_api=false, kwargs...) # TODO split as_api into own function
Context!(ctx; io=io, kwargs...)
regs = Types.collect_registries()
regs = unique(r -> r.uuid, regs) # Maybe not?
regs = unique(r -> r.uuid, regs; seen=Set{Union{UUID,Nothing}}()) # Maybe not?
as_api && return regs
Types.printpkgstyle(ctx, Symbol("Registry Status"), "")
if isempty(regs)
Expand Down
2 changes: 1 addition & 1 deletion src/Resolve/graphtype.jl
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ end
function showlogjournal(io::IO, rlog::ResolveLog)
journal = rlog.journal
id(p) = p == UUID0 ? "[global event]" : pkgID(p, rlog)
padding = maximum(length(id(p)) for (p,_) in journal)
padding = maximum(length(id(p)) for (p,_) in journal; init=0)
for (p,msg) in journal
println(io, ' ', rpad(id(p), padding), ": ", msg)
end
Expand Down
Loading