New Manifest.toml format: Pt 1. Teach Pkg new format, but default to …

…old format (#2561)

* implement manifest format with deps field

* temporarily run CI on base PR

* simpler convert_flat_format_manifest

* default to old manifest format

* remove warning about maintaining old manifest format

* add unknown manifest format warning

* fix

* only print the major and minor part of the `manifest_format` to file

* add tests for manifest.toml formats

* add test for unknown manifest format warning

* provide manifest file path in warning, when available

* Revert "temporarily run CI on base PR"

This reverts commit d4a1e14.

* add support for unknown extra data in Manifest

* fixes

* add missing write_manifest method

* fix with windows-safe regex

Co-authored-by: KristofferC <[email protected]>

src/Operations.jl

@@ -759,7 +759,7 @@ function prune_manifest(env::EnvCache)
     env.manifest = prune_manifest(env.manifest, keep)
 end
 
-function prune_manifest(manifest::Dict, keep::Vector{UUID})
+function prune_manifest(manifest::Manifest, keep::Vector{UUID})
     while !isempty(keep)
         clean = true
         for (uuid, entry) in manifest
@@ -772,7 +772,8 @@ function prune_manifest(manifest::Dict, keep::Vector{UUID})
         end
         clean && break
     end
-    return Dict(uuid => entry for (uuid, entry) in manifest if uuid in keep)
+    manifest.deps = Dict(uuid => entry for (uuid, entry) in manifest if uuid in keep)
+    return manifest
 end
 
 
@@ -1405,7 +1406,7 @@ function sandbox_preserve(env::EnvCache, target::PackageSpec, test_project::Stri
     return prune_manifest(env.manifest, keep)
 end
 
-function abspath!(env::EnvCache, manifest::Dict{UUID,PackageEntry})
+function abspath!(env::EnvCache, manifest::Manifest)
     for (uuid, entry) in manifest
         if entry.path !== nothing
             entry.path = project_rel_path(env, entry.path)
@@ -1477,7 +1478,7 @@ function sandbox(fn::Function, ctx::Context, target::PackageSpec, target_path::S
                 allow_reresolve || rethrow()
                 @debug err
                 @warn "Could not use exact versions of packages in manifest, re-resolving"
-                temp_ctx.env.manifest = Dict(uuid => entry for (uuid, entry) in temp_ctx.env.manifest if isfixed(entry))
+                temp_ctx.env.manifest.deps = Dict(uuid => entry for (uuid, entry) in temp_ctx.env.manifest.deps if isfixed(entry))
                 Pkg.resolve(temp_ctx; io=devnull, skip_writing_project=true)
                 @debug "Using _clean_ dep graph"
             end

src/Types.jl

@@ -259,7 +259,25 @@ Base.:(==)(t1::PackageEntry, t2::PackageEntry) = t1.name == t2.name &&
     t1.uuid == t2.uuid
     # omits `other`
 Base.hash(x::PackageEntry, h::UInt) = foldr(hash, [x.name, x.version, x.path, x.pinned, x.repo, x.tree_hash, x.deps, x.uuid], init=h)  # omits `other`
-const Manifest = Dict{UUID,PackageEntry}
+
+Base.@kwdef mutable struct Manifest
+    julia_version::Union{Nothing,VersionNumber} = Base.VERSION
+    manifest_format::VersionNumber = v"1.0.0" # default to older flat format
+    deps::Dict{UUID,PackageEntry} = Dict{UUID,PackageEntry}()
+    other::Dict{String,Any} = Dict{String,Any}()
+end
+Base.:(==)(t1::Manifest, t2::Manifest) = all(x -> (getfield(t1, x) == getfield(t2, x))::Bool, fieldnames(Manifest))
+Base.hash(m::Manifest, h::UInt) = foldr(hash, [getfield(m, x) for x in fieldnames(Manifest)], init=h)
+Base.getindex(m::Manifest, i_or_key) = getindex(m.deps, i_or_key)
+Base.get(m::Manifest, key, default) = get(m.deps, key, default)
+Base.setindex!(m::Manifest, i_or_key, value) = setindex!(m.deps, i_or_key, value)
+Base.iterate(m::Manifest) = iterate(m.deps)
+Base.iterate(m::Manifest, i::Int) = iterate(m.deps, i)
+Base.length(m::Manifest) = length(m.deps)
+Base.empty!(m::Manifest) = empty!(m.deps)
+Base.values(m::Manifest) = values(m.deps)
+Base.keys(m::Manifest) = keys(m.deps)
+Base.haskey(m::Manifest, key) = haskey(m.deps, key)
 
 function Base.show(io::IO, pkg::PackageEntry)
     f = []

src/manifest.jl

@@ -105,19 +105,19 @@ function normalize_deps(name, uuid, deps::Vector{String}, manifest::Dict{String,
     return final
 end
 
-function validate_manifest(stage1::Dict{String,Vector{Stage1}})
+function validate_manifest(julia_version::Union{Nothing,VersionNumber}, manifest_format::VersionNumber, stage1::Dict{String,Vector{Stage1}}, other::Dict{String, Any})
     # expand vector format deps
     for (name, infos) in stage1, info in infos
         info.entry.deps = normalize_deps(name, info.uuid, info.deps, stage1)
     end
     # invariant: all dependencies are now normalized to Dict{String,UUID}
-    manifest = Dict{UUID, PackageEntry}()
+    deps = Dict{UUID, PackageEntry}()
     for (name, infos) in stage1, info in infos
-        manifest[info.uuid] = info.entry
+        deps[info.uuid] = info.entry
     end
     # now just verify the graph structure
-    for (entry_uuid, entry) in manifest, (name, uuid) in entry.deps
-        dep_entry = get(manifest, uuid, nothing)
+    for (entry_uuid, entry) in deps, (name, uuid) in entry.deps
+        dep_entry = get(deps, uuid, nothing)
         if dep_entry === nothing
             pkgerror("`$(entry.name)=$(entry_uuid)` depends on `$name=$uuid`, ",
                      "but no such entry exists in the manifest.")
@@ -127,38 +127,56 @@ function validate_manifest(stage1::Dict{String,Vector{Stage1}})
                      "but entry with UUID `$uuid` has name `$(dep_entry.name)`.")
         end
     end
-    return manifest
+    return Manifest(; julia_version, manifest_format, deps, other)
 end
 
-function Manifest(raw::Dict)::Manifest
+function Manifest(raw::Dict, f_or_io::Union{String, IO})::Manifest
+    julia_version = isnothing(raw["julia_version"]) ? nothing : VersionNumber(raw["julia_version"])
+    manifest_format = VersionNumber(raw["manifest_format"])
+    if !in(manifest_format.major, 1:2)
+        if f_or_io isa IO
+            @warn "Unknown Manifest.toml format version detected in streamed manifest. Unexpected behavior may occur" manifest_format maxlog = 1
+        else
+            @warn "Unknown Manifest.toml format version detected in file `$(f_or_io)`. Unexpected behavior may occur" manifest_format maxlog = 1
+        end
+    end
     stage1 = Dict{String,Vector{Stage1}}()
-    for (name, infos) in raw, info in infos
-        entry = PackageEntry()
-        entry.name = name
-        uuid = nothing
-        deps = nothing
-        try
-            entry.pinned      = read_pinned(get(info, "pinned", nothing))
-            uuid              = read_field("uuid",          nothing, info, safe_uuid)::UUID
-            entry.version     = read_field("version",       nothing, info, safe_version)
-            entry.path        = read_field("path",          nothing, info, safe_path)
-            entry.repo.source = read_field("repo-url",      nothing, info, identity)
-            entry.repo.rev    = read_field("repo-rev",      nothing, info, identity)
-            entry.repo.subdir = read_field("repo-subdir",   nothing, info, identity)
-            entry.tree_hash   = read_field("git-tree-sha1", nothing, info, safe_SHA1)
-            entry.uuid        = uuid
-            deps = read_deps(get(info::Dict, "deps", nothing))
-        catch
-            # TODO: Should probably not unconditionally log something
-            @error "Could not parse entry for `$name`"
-            rethrow()
+    if haskey(raw, "deps") # deps field doesn't exist if there are no deps
+        for (name, infos) in raw["deps"], info in infos
+            entry = PackageEntry()
+            entry.name = name
+            uuid = nothing
+            deps = nothing
+            try
+                entry.pinned      = read_pinned(get(info, "pinned", nothing))
+                uuid              = read_field("uuid",          nothing, info, safe_uuid)::UUID
+                entry.version     = read_field("version",       nothing, info, safe_version)
+                entry.path        = read_field("path",          nothing, info, safe_path)
+                entry.repo.source = read_field("repo-url",      nothing, info, identity)
+                entry.repo.rev    = read_field("repo-rev",      nothing, info, identity)
+                entry.repo.subdir = read_field("repo-subdir",   nothing, info, identity)
+                entry.tree_hash   = read_field("git-tree-sha1", nothing, info, safe_SHA1)
+                entry.uuid        = uuid
+                deps = read_deps(get(info::Dict, "deps", nothing))
+            catch
+                # TODO: Should probably not unconditionally log something
+                @error "Could not parse entry for `$name`"
+                rethrow()
+            end
+            entry.other = info::Union{Dict,Nothing}
+            stage1[name] = push!(get(stage1, name, Stage1[]), Stage1(uuid, entry, deps))
+        end
+        # by this point, all the fields of the `PackageEntry`s have been type casted
+        # but we have *not* verified the _graph_ structure of the manifest
+    end
+    other = Dict{String, Any}()
+    for (k, v) in raw
+        if k in ("julia_version", "deps", "manifest_format")
+            continue
         end
-        entry.other = info::Union{Dict,Nothing}
-        stage1[name] = push!(get(stage1, name, Stage1[]), Stage1(uuid, entry, deps))
+        other[k] = v
     end
-    # by this point, all the fields of the `PackageEntry`s have been type casted
-    # but we have *not* verified the _graph_ structure of the manifest
-    return validate_manifest(stage1)
+    return validate_manifest(julia_version, manifest_format, stage1, other)
 end
 
 function read_manifest(f_or_io::Union{String, IO})
@@ -174,7 +192,19 @@ function read_manifest(f_or_io::Union{String, IO})
         end
         rethrow()
     end
-    return Manifest(raw)
+    if Base.is_v1_format_manifest(raw)
+        raw = convert_flat_format_manifest(raw)
+    end
+    return Manifest(raw, f_or_io)
+end
+
+function convert_flat_format_manifest(old_raw_manifest::Dict)
+    new_raw_manifest = Dict{String, Any}(
+            "deps" => old_raw_manifest,
+            "julia_version" => nothing,
+            "manifest_format" => "1.0.0" # must be a string here to match raw dict
+        )
+    return new_raw_manifest
 end
 
 ###########
@@ -194,7 +224,19 @@ function destructure(manifest::Manifest)::Dict
         unique_name[entry.name] = !haskey(unique_name, entry.name)
     end
 
-    raw = Dict{String,Any}()
+    # maintain the format of the manifest when writing
+    if manifest.manifest_format.major == 1
+        raw = Dict{String,Vector{Dict{String,Any}}}()
+    elseif manifest.manifest_format.major == 2
+        raw = Dict{String,Any}()
+        raw["julia_version"] = manifest.julia_version
+        raw["manifest_format"] = string(manifest.manifest_format.major, ".", manifest.manifest_format.minor)
+        raw["deps"] = Dict{String,Vector{Dict{String,Any}}}()
+        for (k, v) in manifest.other
+            raw[k] = v
+        end
+    end
+
     for (uuid, entry) in manifest
         new_entry = something(entry.other, Dict{String,Any}())
         new_entry["uuid"] = string(uuid)
@@ -225,7 +267,11 @@ function destructure(manifest::Manifest)::Dict
                 end
             end
         end
-        push!(get!(raw, entry.name, Dict{String,Any}[]), new_entry)
+        if manifest.manifest_format.major == 1
+            push!(get!(raw, entry.name, Dict{String,Any}[]), new_entry)
+        elseif manifest.manifest_format.major == 2
+            push!(get!(raw["deps"], entry.name, Dict{String,Any}[]), new_entry)
+        end
     end
     return raw
 end
@@ -234,17 +280,21 @@ function write_manifest(env::EnvCache)
     mkpath(dirname(env.manifest_file))
     write_manifest(env.manifest, env.manifest_file)
 end
-write_manifest(manifest::Manifest, manifest_file::AbstractString) =
-    write_manifest(destructure(manifest), manifest_file)
-function write_manifest(io::IO, manifest::Dict)
+function write_manifest(manifest::Manifest, manifest_file::AbstractString)
+    return write_manifest(destructure(manifest), manifest_file)
+end
+function write_manifest(io::IO, manifest::Manifest)
+    return write_manifest(io, destructure(manifest))
+end
+function write_manifest(io::IO, raw_manifest::Dict)
     print(io, "# This file is machine-generated - editing it directly is not advised\n\n")
-    TOML.print(io, manifest, sorted=true) do x
-        x isa UUID || x isa SHA1 || x isa VersionNumber || pkgerror("unhandled type `$(typeof(x))`")
-        return string(x)
+    TOML.print(io, raw_manifest, sorted=true) do x
+        (typeof(x) in [String, Nothing, UUID, SHA1, VersionNumber]) && return string(x)
+        error("unhandled type `$(typeof(x))`")
     end
     return nothing
 end
-function write_manifest(manifest::Dict, manifest_file::AbstractString)
-    str = sprint(write_manifest, manifest)
+function write_manifest(raw_manifest::Dict, manifest_file::AbstractString)
+    str = sprint(write_manifest, raw_manifest)
     write(manifest_file, str)
 end

test/manifest/formats/v1.0/Manifest.toml

@@ -0,0 +1,11 @@
+# This file is machine-generated - editing it directly is not advised
+
+[[Logging]]
+uuid = "56ddb016-857b-54e1-b83d-db4d58db5568"
+
+[[Random]]
+deps = ["Serialization"]
+uuid = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
+
+[[Serialization]]
+uuid = "9e88b42a-f829-5b0c-bbe9-9e923198166b"

test/manifest/formats/v1.0/Project.toml

@@ -0,0 +1,3 @@
+[deps]
+Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
+Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"

test/manifest/formats/v2.0/Manifest.toml

@@ -0,0 +1,17 @@
+# This file is machine-generated - editing it directly is not advised
+
+julia_version = "1.7.0-DEV.1199"
+manifest_format = "2.0"
+some_other_field = "other"
+some_other_data = [1,2,3,4]
+
+[[deps.Logging]]
+uuid = "56ddb016-857b-54e1-b83d-db4d58db5568"
+
+[[deps.Random]]
+deps = ["Serialization"]
+uuid = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
+
+[[deps.Serialization]]
+uuid = "9e88b42a-f829-5b0c-bbe9-9e923198166b"
+

test/manifest/formats/v2.0/Project.toml

@@ -0,0 +1,3 @@
+[deps]
+Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
+Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"

test/manifest/formats/v3.0_unknown/Manifest.toml

@@ -0,0 +1,14 @@
+# This file is machine-generated - editing it directly is not advised
+
+julia_version = "1.7.0-DEV.1199"
+manifest_format = "3.0" # NOT ACTUALLY v3.0 format. Just here to test a warning!
+
+[[deps.Logging]]
+uuid = "56ddb016-857b-54e1-b83d-db4d58db5568"
+
+[[deps.Random]]
+deps = ["Serialization"]
+uuid = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
+
+[[deps.Serialization]]
+uuid = "9e88b42a-f829-5b0c-bbe9-9e923198166b"

test/manifest/formats/v3.0_unknown/Project.toml

@@ -0,0 +1,3 @@
+[deps]
+Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
+Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"