Backport fixes for conflicts between [workspace] and [sources] relative paths to release-1.12 (#4513)

Author: MasonProtter

.github/workflows/test.yml

@@ -37,7 +37,7 @@ jobs:
         pkg-server:
           - "pkg.julialang.org" # Default to this for all except specific cases
         julia-version:
-          - 'nightly'
+          - '1.12-nightly'
         exclude:
           - os: ubuntu-latest
             julia-arch: aarch64
@@ -50,11 +50,11 @@ jobs:
         include:
           - os: ubuntu-latest
             julia-arch: 'x64'
-            julia-version: 'nightly'
+            julia-version: '1.12-nightly'
             pkg-server: ""
           - os: ubuntu-latest
             julia-arch: 'x64'
-            julia-version: 'nightly'
+            julia-version: '1.12-nightly'
             pkg-server: "pkg.julialang.org"
     steps:
       - name: Set git to use LF and fix TEMP on windows
@@ -95,7 +95,7 @@ jobs:
       - uses: julia-actions/setup-julia@v2
         with:
           # version: '1.6'
-          version: 'nightly'
+          version: '1.12-nightly'
       - name: Generate docs
         run: |
           julia --project --color=yes -e 'using Pkg; Pkg.activate("docs"); Pkg.instantiate();'

src/API.jl

@@ -191,15 +191,42 @@ function update_source_if_set(env, pkg)
     project = env.project
     source = get(project.sources, pkg.name, nothing)
     if source !== nothing
-        # This should probably not modify the dicts directly...
-        if pkg.repo.source !== nothing
-            source["url"] = pkg.repo.source
+        if pkg.repo == GitRepo()
+            delete!(project.sources, pkg.name)
+        else
+            # This should probably not modify the dicts directly...
+            if pkg.repo.source !== nothing
+                source["url"] = pkg.repo.source
+                delete!(source, "path")
+            end
+            if pkg.repo.rev !== nothing
+                source["rev"] = pkg.repo.rev
+                delete!(source, "path")
+            end
+            if pkg.repo.subdir !== nothing
+                source["subdir"] = pkg.repo.subdir
+            end
+            if pkg.path !== nothing
+                source["path"] = pkg.path
+                delete!(source, "url")
+                delete!(source, "rev")
+            end
         end
-        if pkg.repo.rev !== nothing
-            source["rev"] = pkg.repo.rev
+        if pkg.subdir !== nothing
+            source["subdir"] = pkg.subdir
+        end
+        path, repo = get_path_repo(project, env.project_file, env.manifest_file, pkg.name)
+        if path !== nothing
+            pkg.path = path
+        end
+        if repo.source !== nothing
+            pkg.repo.source = repo.source
         end
-        if pkg.path !== nothing
-            source["path"] = pkg.path
+        if repo.rev !== nothing
+            pkg.repo.rev = repo.rev
+        end
+        if repo.subdir !== nothing
+            pkg.repo.subdir = repo.subdir
         end
     end
 
@@ -347,14 +374,14 @@ end
 function append_all_pkgs!(pkgs, ctx, mode)
     if mode == PKGMODE_PROJECT || mode == PKGMODE_COMBINED
         for (name::String, uuid::UUID) in ctx.env.project.deps
-            path, repo = get_path_repo(ctx.env.project, name)
-            push!(pkgs, PackageSpec(name=name, uuid=uuid, path=path, repo=repo))
+            path, repo = get_path_repo(ctx.env.project, ctx.env.project_file, ctx.env.manifest_file, name)
+            push!(pkgs, PackageSpec(name = name, uuid = uuid, path = path, repo = repo))
         end
     end
     if mode == PKGMODE_MANIFEST || mode == PKGMODE_COMBINED
         for (uuid, entry) in ctx.env.manifest
-            path, repo = get_path_repo(ctx.env.project, entry.name)
-            push!(pkgs, PackageSpec(name=entry.name, uuid=uuid, path=path, repo=repo))
+            path, repo = get_path_repo(ctx.env.project, ctx.env.project_file, ctx.env.manifest_file, entry.name)
+            push!(pkgs, PackageSpec(name = entry.name, uuid = uuid, path = path, repo = repo))
         end
     end
     return
@@ -425,6 +452,7 @@ function pin(ctx::Context, pkgs::Vector{PackageSpec}; all_pkgs::Bool=false, kwar
                 pkgerror("pinning a package requires a single version, not a versionrange")
             end
         end
+        update_source_if_set(ctx.env, pkg)
     end
 
     project_deps_resolve!(ctx.env, pkgs)

src/Operations.jl

@@ -114,7 +114,7 @@ function load_project_deps(project::Project, project_file::String, manifest::Man
 
     for (name::String, uuid::UUID) in project.deps
         findfirst(pkg -> pkg.uuid == uuid, pkgs) === nothing || continue # do not duplicate packages
-        path, repo = get_path_repo(project, name)
+        path, repo = get_path_repo(project, project_file, manifest_file, name)
         entry = manifest_info(manifest, uuid)
         push!(pkgs_direct, entry === nothing ?
               PackageSpec(;uuid, name, path, repo) :
@@ -155,7 +155,7 @@ function load_all_deps(env::EnvCache, pkgs::Vector{PackageSpec}=PackageSpec[];
     pkgs = load_manifest_deps(env.manifest, pkgs; preserve=preserve)
     # Sources takes presedence over the manifest...
     for pkg in pkgs
-        path, repo = get_path_repo(env.project, pkg.name)
+        path, repo = get_path_repo(env.project, env.project_file, env.manifest_file, pkg.name)
         if path !== nothing
             pkg.path = path
         end
@@ -319,7 +319,7 @@ function reset_all_compat!(proj::Project)
     return nothing
 end
 
-function collect_project(pkg::Union{PackageSpec, Nothing}, path::String, julia_version)
+function collect_project(pkg::Union{PackageSpec, Nothing}, path::String, manifest_file::String, julia_version)
     deps = PackageSpec[]
     weakdeps = Set{UUID}()
     project_file = projectfile_path(path; strict=true)
@@ -329,9 +329,9 @@ function collect_project(pkg::Union{PackageSpec, Nothing}, path::String, julia_v
         pkgerror("julia version requirement for package at `$path` not satisfied: compat entry \"julia = $(get_compat_str(project, "julia"))\" does not include Julia version $julia_version")
     end
     for (name, uuid) in project.deps
-        path, repo = get_path_repo(project, name)
+        dep_path, repo = get_path_repo(project, project_file, manifest_file, name)
         vspec = get_compat(project, name)
-        push!(deps, PackageSpec(name=name, uuid=uuid, version=vspec, path=path, repo=repo))
+        push!(deps, PackageSpec(name = name, uuid = uuid, version = vspec, path = dep_path, repo = repo))
     end
     for (name, uuid) in project.weakdeps
         vspec = get_compat(project, name)
@@ -390,15 +390,15 @@ function collect_fixed!(env::EnvCache, pkgs::Vector{PackageSpec}, names::Dict{UU
     weak_map = Dict{UUID,Set{UUID}}()
 
     uuid = Types.project_uuid(env)
-    deps, weakdeps = collect_project(env.pkg, dirname(env.project_file), julia_version)
+    deps, weakdeps = collect_project(env.pkg, dirname(env.project_file), env.manifest_file, julia_version)
     deps_map[uuid] = deps
     weak_map[uuid] = weakdeps
     names[uuid] = env.pkg === nothing ? "project" : env.pkg.name
 
     for (path, project) in env.workspace
         uuid = Types.project_uuid(project, path)
-        pkg = project.name === nothing ? nothing : PackageSpec(name=project.name, uuid=uuid)
-        deps, weakdeps = collect_project(pkg, path, julia_version)
+        pkg = project.name === nothing ? nothing : PackageSpec(name = project.name, uuid = uuid)
+        deps, weakdeps = collect_project(pkg, path, env.manifest_file, julia_version)
         deps_map[Types.project_uuid(env)] = deps
         weak_map[Types.project_uuid(env)] = weakdeps
         names[uuid] = project.name === nothing ? "project" : project.name
@@ -418,7 +418,7 @@ function collect_fixed!(env::EnvCache, pkgs::Vector{PackageSpec}, names::Dict{UU
         if !isdir(path)
             pkgerror("expected package $(err_rep(pkg)) to exist at path `$path`")
         end
-        deps, weakdeps = collect_project(pkg, path, julia_version)
+        deps, weakdeps = collect_project(pkg, path, env.manifest_file, julia_version)
         deps_map[pkg.uuid] = deps
         weak_map[pkg.uuid] = weakdeps
     end
@@ -1883,7 +1883,7 @@ function up(ctx::Context, pkgs::Vector{PackageSpec}, level::UpgradeLevel;
     # TODO check all pkg.version == VersionSpec()
     # set version constraints according to `level`
     for pkg in pkgs
-        source_path, source_repo = get_path_repo(ctx.env.project, pkg.name)
+        source_path, source_repo = get_path_repo(ctx.env.project, ctx.env.project_file, ctx.env.manifest_file, pkg.name)
         entry = manifest_info(ctx.env.manifest, pkg.uuid)
         new = up_load_versions!(ctx, pkg, entry, source_path, source_repo, level)
         new && push!(new_git, pkg.uuid) #TODO put download + push! in utility function
@@ -2100,7 +2100,8 @@ end
 function abspath!(env::EnvCache, project::Project)
     for (key, entry) in project.sources
         if haskey(entry, "path")
-            entry["path"] = project_rel_path(env, entry["path"])
+            # Paths in project sources are project-relative, so join with project_file dir, not manifest_file dir
+            entry["path"] = normpath(joinpath(dirname(env.project_file), entry["path"]))
         end
     end
     return project

src/Types.jl

@@ -434,6 +434,22 @@ function EnvCache(env::Union{Nothing,String}=nothing)
     return env′
 end
 
+# Convert a path from project-relative to manifest-relative
+# If path is absolute, returns it as-is
+function project_path_to_manifest_path(project_file::String, manifest_file::String, path::String)
+    isabspath(path) && return path
+    abs_path = Pkg.safe_realpath(joinpath(dirname(project_file), path))
+    return relpath(abs_path, Pkg.safe_realpath(dirname(manifest_file)))
+end
+
+# Convert a path from manifest-relative to project-relative
+# If path is absolute, returns it as-is
+function manifest_path_to_project_path(project_file::String, manifest_file::String, path::String)
+    isabspath(path) && return path
+    abs_path = Pkg.safe_realpath(joinpath(dirname(manifest_file), path))
+    return relpath(abs_path, Pkg.safe_realpath(dirname(project_file)))
+end
+
 include("project.jl")
 include("manifest.jl")
 
@@ -1249,7 +1265,7 @@ function write_env(env::EnvCache; update_undo=true,
                    skip_writing_project::Bool=false)
     # Verify that the generated manifest is consistent with `sources`
     for (pkg, uuid) in env.project.deps
-        path, repo = get_path_repo(env.project, pkg)
+        path, repo = get_path_repo(env.project, env.project_file, env.manifest_file, pkg)
         entry = manifest_info(env.manifest, uuid)
         if path !== nothing
             @assert normpath(entry.path) == normpath(path)
@@ -1262,6 +1278,19 @@ function write_env(env::EnvCache; update_undo=true,
                 @assert entry.repo.subdir == repo.subdir
             end
         end
+        if entry !== nothing
+            if entry.path !== nothing
+                # Convert path from manifest-relative to project-relative before writing
+                project_relative_path = manifest_path_to_project_path(env.project_file, env.manifest_file, entry.path)
+                env.project.sources[pkg] = Dict("path" => project_relative_path)
+            elseif entry.repo != GitRepo()
+                d = Dict{String, String}()
+                entry.repo.source !== nothing && (d["url"] = entry.repo.source)
+                entry.repo.rev !== nothing && (d["rev"] = entry.repo.rev)
+                entry.repo.subdir !== nothing && (d["subdir"] = entry.repo.subdir)
+                env.project.sources[pkg] = d
+            end
+        end
     end
     if (env.project != env.original_project) && (!skip_writing_project)
         write_project(env)

src/project.jl

@@ -4,7 +4,7 @@
 listed_deps(project::Project; include_weak::Bool) =
     vcat(collect(keys(project.deps)), collect(keys(project.extras)), include_weak ? collect(keys(project.weakdeps)) : String[])
 
-function get_path_repo(project::Project, name::String)
+function get_path_repo(project::Project, project_file::String, manifest_file::String, name::String)
     source = get(project.sources, name, nothing)
     if source === nothing
         return nothing, GitRepo()
@@ -17,6 +17,10 @@ function get_path_repo(project::Project, name::String)
         pkgerror("`path` and `url` are conflicting specifications")
     end
     repo = GitRepo(url, rev, subdir)
+    # Convert path from project-relative to manifest-relative
+    if path !== nothing
+        path = project_path_to_manifest_path(project_file, manifest_file, path)
+    end
     return path, repo
 end
 

test/sources.jl

@@ -21,7 +21,7 @@ temp_pkg_dir() do project_path
                     cp("Project.toml.bak", "Project.toml"; force=true)
                     cp("BadManifest.toml", "Manifest.toml"; force=true)
                     Pkg.resolve()
-                    @test Pkg.project().sources["Example"] == Dict("url" => "https://github.com/JuliaLang/Example.jl")
+                    @test Pkg.project().sources["Example"] == Dict("rev" => "master", "url" => "https://github.com/JuliaLang/Example.jl")
                     @test Pkg.project().sources["LocalPkg"] == Dict("path" => "LocalPkg")
                 end
             end

test/workspaces.jl

@@ -25,8 +25,7 @@ temp_pkg_dir() do project_path
             Pkg.develop(path="PrivatePackage")
             d = TOML.parsefile("Project.toml")
             d["workspace"] = Dict("projects" => ["test", "docs", "benchmarks", "PrivatePackage"])
-            abs_path = abspath("PrivatePackage") # TODO: Make relative after #3842 is fixed
-            d["sources"] = Dict("PrivatePackage" => Dict("path" => abs_path))
+            d["sources"] = Dict("PrivatePackage" => Dict("path" => "PrivatePackage"))
             Pkg.Types.write_project(d, "Project.toml")
             write("src/MonorepoSub.jl", """
                 module MonorepoSub
@@ -98,8 +97,7 @@ temp_pkg_dir() do project_path
             Pkg.generate("TestSpecificPackage")
             Pkg.develop(path="TestSpecificPackage")
             d = TOML.parsefile("test/Project.toml")
-            abs_pkg = abspath("TestSpecificPackage") # TODO: Make relative after #3842 is fixed
-            d["sources"] = Dict("TestSpecificPackage" => Dict("path" => abs_pkg))
+            d["sources"] = Dict("TestSpecificPackage" => Dict("path" => "../TestSpecificPackage"))
             Pkg.Types.write_project(d, "test/Project.toml")
 
             @test !isfile("test/Manifest.toml")