diff --git a/src/API.jl b/src/API.jl index 03eadf982e..9acdcf79e3 100644 --- a/src/API.jl +++ b/src/API.jl @@ -12,9 +12,9 @@ import FileWatching import Base: StaleCacheKey -import ..depots, ..depots1, ..logdir, ..devdir, ..printpkgstyle, .._autoprecompilation_enabled_scoped +import ..depots, ..depots1, ..logdir, ..devdir, ..printpkgstyle, .._autoprecompilation_enabled_scoped, ..manifest_rel_path import ..Operations, ..GitTools, ..Pkg, ..Registry -import ..can_fancyprint, ..pathrepr, ..isurl, ..PREV_ENV_PATH, ..atomic_toml_write +import ..can_fancyprint, ..pathrepr, ..isurl, ..PREV_ENV_PATH, ..atomic_toml_write, ..safe_realpath using ..Types, ..TOML using ..Types: VersionTypes using Base.BinaryPlatforms @@ -64,7 +64,7 @@ end function package_info(env::EnvCache, pkg::PackageSpec, entry::PackageEntry)::PackageInfo git_source = pkg.repo.source === nothing ? nothing : isurl(pkg.repo.source::String) ? pkg.repo.source::String : - Operations.project_rel_path(env, pkg.repo.source::String) + safe_realpath(manifest_rel_path(env, pkg.repo.source::String)) _source_path = Operations.source_path(env.manifest_file, pkg) if _source_path === nothing @debug "Manifest file $(env.manifest_file) contents:\n$(read(env.manifest_file, String))" @@ -81,7 +81,7 @@ function package_info(env::EnvCache, pkg::PackageSpec, entry::PackageEntry)::Pac is_tracking_registry = Operations.is_tracking_registry(pkg), git_revision = pkg.repo.rev, git_source = git_source, - source = Operations.project_rel_path(env, _source_path), + source = _source_path, dependencies = copy(entry.deps), #TODO is copy needed? ) return info @@ -250,6 +250,19 @@ function update_source_if_set(env, pkg) return end +# Normalize relative paths from user input (pwd-relative) to internal representation (manifest-relative) +# This ensures all relative paths in Pkg are consistently relative to the manifest file +function normalize_package_paths!(ctx::Context, pkgs::Vector{PackageSpec}) + for pkg in pkgs + if pkg.repo.source !== nothing && !isurl(pkg.repo.source) && !isabspath(pkg.repo.source) + # User provided a relative path (relative to pwd), convert to manifest-relative + absolute_path = abspath(pkg.repo.source) + pkg.repo.source = Types.relative_project_path(ctx.env.manifest_file, absolute_path) + end + end + return +end + function develop( ctx::Context, pkgs::Vector{PackageSpec}; shared::Bool = true, preserve::PreserveLevel = Operations.default_preserve(), platform::AbstractPlatform = HostPlatform(), kwargs... @@ -285,6 +298,8 @@ function develop( end end + normalize_package_paths!(ctx, pkgs) + new_git = handle_repos_develop!(ctx, pkgs, shared) Operations.update_registries(ctx; force = false, update_cooldown = Day(1)) @@ -337,6 +352,8 @@ function add( end end + normalize_package_paths!(ctx, pkgs) + repo_pkgs = PackageSpec[pkg for pkg in pkgs if (pkg.repo.source !== nothing || pkg.repo.rev !== nothing)] new_git = handle_repos_add!(ctx, repo_pkgs) # repo + unpinned -> name, uuid, repo.rev, repo.source, tree_hash @@ -782,7 +799,15 @@ function gc(ctx::Context = Context(); collect_delay::Union{Period, Nothing} = no end # Collect the locations of every repo referred to in this manifest - return [Types.add_repo_cache_path(e.repo.source) for (u, e) in manifest if e.repo.source !== nothing] + return [ + Types.add_repo_cache_path( + isurl(e.repo.source) ? e.repo.source : + safe_realpath( + isabspath(e.repo.source) ? e.repo.source : + normpath(joinpath(dirname(path), e.repo.source)) + ) + ) for (u, e) in manifest if e.repo.source !== nothing + ] end function process_artifacts_toml(path, pkgs_to_delete) @@ -1314,7 +1339,7 @@ function instantiate( ## Download repo at tree hash # determine canonical form of repo source if !isurl(repo_source) - repo_source = normpath(joinpath(dirname(ctx.env.project_file), repo_source)) + repo_source = manifest_rel_path(ctx.env, repo_source) end if !isurl(repo_source) && !isdir(repo_source) pkgerror("Did not find path `$(repo_source)` for $(err_rep(pkg))") diff --git a/src/Operations.jl b/src/Operations.jl index 7fc4770234..40fdf04af8 100644 --- a/src/Operations.jl +++ b/src/Operations.jl @@ -16,7 +16,7 @@ using Base.BinaryPlatforms import ...Pkg import ...Pkg: pkg_server, Registry, pathrepr, can_fancyprint, printpkgstyle, stderr_f, OFFLINE_MODE import ...Pkg: UPDATED_REGISTRY_THIS_SESSION, RESPECT_SYSIMAGE_VERSIONS, should_autoprecompile -import ...Pkg: usable_io, discover_repo, create_cachedir_tag +import ...Pkg: usable_io, discover_repo, create_cachedir_tag, manifest_rel_path ######### # Utils # @@ -130,7 +130,7 @@ end function source_path(manifest_file::String, pkg::Union{PackageSpec, PackageEntry}, julia_version = VERSION) return pkg.tree_hash !== nothing ? find_installed(pkg.name, pkg.uuid, pkg.tree_hash) : - pkg.path !== nothing ? joinpath(dirname(manifest_file), pkg.path) : + pkg.path !== nothing ? normpath(joinpath(dirname(manifest_file), pkg.path)) : is_or_was_stdlib(pkg.uuid, julia_version) ? Types.stdlib_path(pkg.name) : nothing end @@ -535,7 +535,7 @@ is_tracking_registry(pkg) = !is_tracking_path(pkg) && !is_tracking_repo(pkg) isfixed(pkg) = !is_tracking_registry(pkg) || pkg.pinned function collect_developed!(env::EnvCache, pkg::PackageSpec, developed::Vector{PackageSpec}) - source = project_rel_path(env, source_path(env.manifest_file, pkg)) + source = source_path(env.manifest_file, pkg) source_env = EnvCache(projectfile_path(source)) pkgs = load_project_deps(source_env.project, source_env.project_file, source_env.manifest, source_env.manifest_file) for pkg in pkgs @@ -548,10 +548,7 @@ function collect_developed!(env::EnvCache, pkg::PackageSpec, developed::Vector{P # otherwise relative to manifest file.... pkg.path = Types.relative_project_path( env.manifest_file, - project_rel_path( - source_env, - source_path(source_env.manifest_file, pkg) - ) + source_path(source_env.manifest_file, pkg) ) push!(developed, pkg) collect_developed!(env, pkg, developed) @@ -602,13 +599,13 @@ function collect_fixed!(env::EnvCache, pkgs::Vector{PackageSpec}, names::Dict{UU pkg.uuid === nothing && continue # add repo package if necessary source = source_path(env.manifest_file, pkg) - path = source === nothing ? nothing : project_rel_path(env, source) + path = source if (path === nothing || !isdir(path)) && (pkg.repo.rev !== nothing || pkg.repo.source !== nothing) # ensure revved package is installed # pkg.tree_hash is set in here Types.handle_repo_add!(Types.Context(env = env), pkg) # Recompute path - path = project_rel_path(env, source_path(env.manifest_file, pkg)) + path = source_path(env.manifest_file, pkg) end if !isdir(path) # Find which packages depend on this missing package for better error reporting @@ -1533,7 +1530,6 @@ end ################################ # Manifest update and pruning # ################################ -project_rel_path(env::EnvCache, path::String) = normpath(joinpath(dirname(env.manifest_file), path)) function prune_manifest(env::EnvCache) # if project uses another manifest, only prune project entry in manifest @@ -2582,7 +2578,7 @@ end function abspath!(env::EnvCache, manifest::Manifest) for (uuid, entry) in manifest if entry.path !== nothing - entry.path = project_rel_path(env, entry.path) + entry.path = manifest_rel_path(env, entry.path) end end return manifest @@ -2809,7 +2805,7 @@ function test( missing_runtests = String[] source_paths = String[] # source_path is the package root (not /src) for pkg in pkgs - sourcepath = project_rel_path(ctx.env, source_path(ctx.env.manifest_file, pkg, ctx.julia_version)) # TODO + sourcepath = source_path(ctx.env.manifest_file, pkg, ctx.julia_version) !isfile(testfile(sourcepath)) && push!(missing_runtests, pkg.name) push!(source_paths, sourcepath) end diff --git a/src/Types.jl b/src/Types.jl index 03c9ecbd00..5bbca47c09 100644 --- a/src/Types.jl +++ b/src/Types.jl @@ -801,8 +801,17 @@ end function handle_repo_develop!(ctx::Context, pkg::PackageSpec, shared::Bool) # First, check if we can compute the path easily (which requires a given local path or name) is_local_path = pkg.repo.source !== nothing && !isurl(pkg.repo.source) + # Preserve whether the original source was an absolute path - needed later to decide how to store the path + original_source_was_absolute = is_local_path && isabspath(pkg.repo.source) + if is_local_path || pkg.name !== nothing - dev_path = is_local_path ? pkg.repo.source : devpath(ctx.env, pkg.name, shared) + # Resolve manifest-relative paths to absolute paths for file system operations + dev_path = if is_local_path + isabspath(pkg.repo.source) ? pkg.repo.source : + Pkg.manifest_rel_path(ctx.env, pkg.repo.source) + else + devpath(ctx.env, pkg.name, shared) + end if pkg.repo.subdir !== nothing dev_path = joinpath(dev_path, pkg.repo.subdir) end @@ -818,7 +827,7 @@ function handle_repo_develop!(ctx::Context, pkg::PackageSpec, shared::Bool) resolve_projectfile!(pkg, dev_path) error_if_in_sysimage(pkg) if is_local_path - pkg.path = isabspath(dev_path) ? dev_path : relative_project_path(ctx.env.manifest_file, dev_path) + pkg.path = original_source_was_absolute ? dev_path : relative_project_path(ctx.env.manifest_file, dev_path) else pkg.path = shared ? dev_path : relative_project_path(ctx.env.manifest_file, dev_path) end @@ -847,7 +856,11 @@ function handle_repo_develop!(ctx::Context, pkg::PackageSpec, shared::Bool) cloned = false package_path = pkg.repo.subdir === nothing ? repo_path : joinpath(repo_path, pkg.repo.subdir) if !has_name(pkg) - LibGit2.close(GitTools.ensure_clone(ctx.io, repo_path, pkg.repo.source)) + # Resolve manifest-relative path to absolute before passing to git + repo_source_resolved = !isurl(pkg.repo.source) && !isabspath(pkg.repo.source) ? + Pkg.manifest_rel_path(ctx.env, pkg.repo.source) : + pkg.repo.source + LibGit2.close(GitTools.ensure_clone(ctx.io, repo_path, repo_source_resolved)) cloned = true resolve_projectfile!(pkg, package_path) end @@ -870,7 +883,11 @@ function handle_repo_develop!(ctx::Context, pkg::PackageSpec, shared::Bool) else mkpath(dirname(dev_path)) if !cloned - LibGit2.close(GitTools.ensure_clone(ctx.io, dev_path, pkg.repo.source)) + # Resolve manifest-relative path to absolute before passing to git + repo_source_resolved = !isurl(pkg.repo.source) && !isabspath(pkg.repo.source) ? + Pkg.manifest_rel_path(ctx.env, pkg.repo.source) : + pkg.repo.source + LibGit2.close(GitTools.ensure_clone(ctx.io, dev_path, repo_source_resolved)) else mv(repo_path, dev_path) end @@ -880,7 +897,13 @@ function handle_repo_develop!(ctx::Context, pkg::PackageSpec, shared::Bool) resolve_projectfile!(pkg, joinpath(dev_path, pkg.repo.subdir === nothing ? "" : pkg.repo.subdir)) end error_if_in_sysimage(pkg) - pkg.path = shared ? dev_path : relative_project_path(ctx.env.manifest_file, dev_path) + # When an explicit local path was given, preserve whether it was absolute or relative + # Otherwise, use shared flag to determine if path should be absolute (shared) or relative (local) + if is_local_path + pkg.path = original_source_was_absolute ? dev_path : relative_project_path(ctx.env.manifest_file, dev_path) + else + pkg.path = shared ? dev_path : relative_project_path(ctx.env.manifest_file, dev_path) + end if pkg.repo.subdir !== nothing pkg.path = joinpath(pkg.path, pkg.repo.subdir) end @@ -948,10 +971,12 @@ function handle_repo_add!(ctx::Context, pkg::PackageSpec) @assert pkg.repo.source !== nothing # We now have the source of the package repo, check if it is a local path and if that exists - repo_source = pkg.repo.source + repo_source = !isurl(pkg.repo.source) && !isabspath(pkg.repo.source) ? + normpath(joinpath(dirname(ctx.env.manifest_file), pkg.repo.source)) : + pkg.repo.source if !isurl(pkg.repo.source) - if isdir(pkg.repo.source) - git_path = joinpath(pkg.repo.source, ".git") + if isdir(repo_source) + git_path = joinpath(repo_source, ".git") if isfile(git_path) # Git submodule: .git is a file containing path to actual git directory git_ref_content = readline(git_path) @@ -961,22 +986,25 @@ function handle_repo_add!(ctx::Context, pkg::PackageSpec) git_info_path = git_path end if !isdir(git_info_path) - msg = "Did not find a git repository at `$(pkg.repo.source)`" - if isfile(joinpath(pkg.repo.source, "Project.toml")) || isfile(joinpath(pkg.repo.source, "JuliaProject.toml")) + msg = "Did not find a git repository at `$(repo_source)`" + if isfile(joinpath(repo_source, "Project.toml")) || isfile(joinpath(repo_source, "JuliaProject.toml")) msg *= ", perhaps you meant `Pkg.develop`?" end pkgerror(msg) end - LibGit2.with(GitTools.check_valid_HEAD, LibGit2.GitRepo(pkg.repo.source)) # check for valid git HEAD - LibGit2.with(LibGit2.GitRepo(pkg.repo.source)) do repo + LibGit2.with(GitTools.check_valid_HEAD, LibGit2.GitRepo(repo_source)) # check for valid git HEAD + LibGit2.with(LibGit2.GitRepo(repo_source)) do repo if LibGit2.isdirty(repo) - @warn "The repository at `$(pkg.repo.source)` has uncommitted changes. Consider using `Pkg.develop` instead of `Pkg.add` if you want to work with the current state of the repository." + @warn "The repository at `$(repo_source)` has uncommitted changes. Consider using `Pkg.develop` instead of `Pkg.add` if you want to work with the current state of the repository." end end - pkg.repo.source = isabspath(pkg.repo.source) ? safe_realpath(pkg.repo.source) : relative_project_path(ctx.env.manifest_file, pkg.repo.source) - repo_source = normpath(joinpath(dirname(ctx.env.manifest_file), pkg.repo.source)) + # Store the path: use the original path format (absolute vs relative) as the user provided + # Canonicalize repo_source for consistent hashing in cache paths + repo_source = safe_realpath(repo_source) + pkg.repo.source = isabspath(pkg.repo.source) ? repo_source : relative_project_path(ctx.env.manifest_file, repo_source) else - pkgerror("Path `$(pkg.repo.source)` does not exist.") + # For error messages, show the absolute path which is more informative than manifest-relative + pkgerror("Path `$(repo_source)` does not exist.") end end diff --git a/src/utils.jl b/src/utils.jl index 2319759380..383eade64a 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -212,3 +212,7 @@ function discover_repo(path::AbstractString) end return end + +# Resolve a manifest-relative path to an absolute path +# Note: Despite the name "manifest_rel_path", this resolves relative to the manifest file +manifest_rel_path(env, path::String) = normpath(joinpath(dirname(env.manifest_file), path)) diff --git a/test/new.jl b/test/new.jl index 74150c5fe3..c2496fae17 100644 --- a/test/new.jl +++ b/test/new.jl @@ -1496,7 +1496,7 @@ end cd_tempdir() do dir # adding a nonexistent directory @test_throws PkgError( - "Path `$(normpath("some/really/random/Dir"))` does not exist." + "Path `$(abspath("some/really/random/Dir"))` does not exist." ) Pkg.pkg"add some/really/random/Dir" # warn if not explicit about adding directory mkdir("Example") diff --git a/test/pkg.jl b/test/pkg.jl index 2953d19f00..5e9954b432 100644 --- a/test/pkg.jl +++ b/test/pkg.jl @@ -1182,4 +1182,56 @@ end end end +# issue #2291: relative paths in manifests should be resolved relative to manifest location +@testset "relative path resolution from different directories (issue #2291)" begin + mktempdir() do dir + # Create a local package with a git repo + pkg_path = joinpath(dir, "LocalPackage") + mkpath(joinpath(pkg_path, "src")) + write( + joinpath(pkg_path, "Project.toml"), """ + name = "LocalPackage" + uuid = "00000000-0000-0000-0000-000000000001" + version = "0.1.0" + """ + ) + write( + joinpath(pkg_path, "src", "LocalPackage.jl"), """ + module LocalPackage + greet() = "Hello from LocalPackage!" + end + """ + ) + + # Initialize git repo + LibGit2.with(LibGit2.init(pkg_path)) do repo + LibGit2.add!(repo, "*") + LibGit2.commit(repo, "Initial commit"; author = TEST_SIG, committer = TEST_SIG) + end + + # Create a project in a subdirectory and add the package with relative path + project_path = joinpath(dir, "project") + mkpath(project_path) + cd(project_path) do + Pkg.activate(".") + Pkg.add(Pkg.PackageSpec(path = "../LocalPackage")) + + # Verify the package was added with relative path + manifest = read_manifest(joinpath(project_path, "Manifest.toml")) + pkg_entry = manifest[UUID("00000000-0000-0000-0000-000000000001")] + @test pkg_entry.repo.source == "../LocalPackage" + end + + # Now change to parent directory and try to update - this should work + cd(dir) do + Pkg.activate("project") + Pkg.update() # This should not fail + # Check the package is installed by looking it up in dependencies + pkg_info = Pkg.dependencies()[UUID("00000000-0000-0000-0000-000000000001")] + @test pkg_info.name == "LocalPackage" + @test isinstalled(PackageSpec(uuid = UUID("00000000-0000-0000-0000-000000000001"), name = "LocalPackage")) + end + end +end + end # module