Allow relative path with `swift package add-dependency` command #7871

hi2gage · 2024-08-10T18:38:05Z

Enable passing relative path into swift package add-dependency

Motivation:

Currently only AbsolutePaths are supported with --type path flag

swift package add-dependency /packagePath --type path

There is a need to support RelativePath because it's not uncommon to have a package structure inside of an xcodeproj as shown below.

LocalPackages
├── ChildPackage
│   ├── .gitignore
│   ├── .swiftpm
│   ├── Package.swift
│   ├── Sources
│   └── Tests
└── ParentPackage
    ├── .build
    ├── .gitignore
    ├── .swiftpm
    ├── Package.swift
    ├── Sources
    └── Tests

If we want to open ParentPackage by itself, it will not be able to resolve that package.

This pr allows for the user to add a package with a RelativePath path via the swift package add-dependency --type path command.

Access level for

.package(name:url:requirement:traits:)
.package(name:url:requirement:)
.package(id:requirement:traits:)

were upgraded from private to package allowing access to these functions from the PackageCommands module.

Modifications:

Enable passing relative path into swift package add-dependency
Add unit test coverage

Result:

Both of the following commands are valid

swift package add-dependency ../relative --type path

swift package add-dependency /absolute --type path

hi2gage · 2024-08-27T23:20:16Z

@bnbarham Just wanted to bump this.

bnbarham

Thanks @hi2gage! Definitely valuable to support relative paths here.

Due to access level and not wanting to make certain enums public, I opted to use MappablePackageDependency instead of Package.Dependency to represent the values coming from the CLI command.

Which enums would those be? Ideally MappablePackageDependency would not be public, it really shouldn't be visible to commands.

hi2gage · 2024-09-04T06:00:24Z

Thanks @hi2gage! Definitely valuable to support relative paths here.

Due to access level and not wanting to make certain enums public, I opted to use MappablePackageDependency instead of Package.Dependency to represent the values coming from the CLI command.

Which enums would those be? Ideally MappablePackageDependency would not be public, it really shouldn't be visible to commands.

@bnbarham No problem!

I thought I would need to make Package.Dependency.SourceControlRequirement and Package.Dependency.RegistryRequirement public, but I avoided doing so because of these comments:

// intentionally private to hide enum detail
private static func package(
        name: String? = nil,
        url: String,
        requirement: Package.Dependency.SourceControlRequirement
    ) -> Package.Dependency {
        return .init(name: name, location: url, requirement: requirement, traits: nil)
    }

but looking back over this again I realized this was written prior to the package access level being introduced. Any reason to not move these 3 functions from private -> package?

It would really simplify this PR to not use MappablePackageDependency as you mentioned.

bnbarham · 2024-09-12T00:47:31Z

I thought I would need to make Package.Dependency.SourceControlRequirement and Package.Dependency.RegistryRequirement public, but I avoided doing so because of these comments:

Aren't they already 🤔? I assume the issue was that there's no way to create a Dependency with them today, but couldn't you call the existing initializers (which end up mapping to these)?

Any reason to not move these 3 functions from private -> package?

If they need to be (depending on the above) - they just can't be public as this is the package manifest API.

hi2gage · 2024-09-12T07:26:49Z

Hey @bnbarham I just pushed up the changes to use Package.Dependency so we can talk specifics.

I thought I would need to make Package.Dependency.SourceControlRequirement and Package.Dependency.RegistryRequirement public, but I avoided doing so because of these comments:

Aren't they already 🤔?

Yes they are public 🤦I meant to say:
I thought I would need to make the Dependency initializers that contain those enums public. Not the enums themselves public.

I assume the issue was that there's no way to create a Dependency with them today, but couldn't you call the existing initializers (which end up mapping to these)?

Technically I could call the existing initializers, but it requires some funky switching. The changes I just pushed uses the existing initializers. So let me know what you think about that.

This being said if I made those initializers package access level I could take this code:

let packageDependency: PackageDescription.Package.Dependency
switch (firstRequirement, to) {
case (let .exact(version), nil):
    packageDependency = .package(url: self.dependency, exact: version)
case (let .range(range), let to?):
    packageDependency = .package(url: self.dependency, (range.lowerBound ..< to))
case (let .range(range), nil):
    packageDependency = .package(url: self.dependency, range)
case (let .revision(revision), nil):
    packageDependency = .package(url: self.dependency, revision: revision)
case (let .branch(branch), nil):
    packageDependency = .package(url: self.dependency, branch: branch)
case (.branch, _?), (.revision, _?), (.exact, _?):
    throw StringError("--to can only be specified with --from or --up-to-next-minor-from")
case (_, _):
    throw StringError("unknown requirement")
}

->

let packageDependency2: PackageDescription.Package.Dependency = .package(
    url: self.dependency,
    requirement: requirement,
    traits: []
)

Any reason to not move these 3 functions from private -> package?

If they need to be (depending on the above) - they just can't be public as this is the package manifest API.

Agreed, they can't be public, making them package would be helpful but we can get around it if you want to keep those initializers locked down.

bnbarham · 2024-09-19T02:42:20Z

I'd be fine making them package, especially to avoid that switch 😅 . @dschaefer2 / @plemarquand any opinions?

hi2gage · 2024-09-30T23:25:10Z

I'd be fine making them package, especially to avoid that switch 😅 . @dschaefer2 / @plemarquand any opinions?

Great! I just pushed up changes making those package.

hi2gage · 2024-10-15T23:23:54Z

@bnbarham Just wanted to bump this, let me know if there is anything I can to help move this PR along.

dschaefer2 · 2024-10-16T01:08:13Z

@swift-ci please test

dschaefer2 · 2024-10-16T01:08:38Z

@swift-ci please test windows

dschaefer2 · 2024-10-16T01:30:29Z

@hi2gage that did the trick. Running the tests now. I'm a little worried about adding the PackageDescription dependency since it was really only intended for the package manifest processing, but it actually makes sense how you're using it.

This is actually pretty timely. We've been working at improving support for monorepos where relative packages are common. This will help setting them up.

dschaefer2 · 2024-11-07T17:14:00Z

@swift-ci please test

dschaefer2 · 2024-11-07T17:14:17Z

@swift-ci please test windows

hi2gage · 2024-11-13T05:22:49Z

@dschaefer2 Just pushed up what I believe is the fix. PackageDescription was not getting added to SwiftPM_EXPORTS. I was able to bootstrap SPM locally so I think this should work on windows as well.

hi2gage · 2024-11-13T05:22:54Z

@swift-ci please test windows

dschaefer2 · 2024-11-14T05:59:40Z

Don't worry about the branching. We'll work to get this into 6.1. Our CI has been under the weather the last few days.

dschaefer2 · 2024-11-14T15:23:52Z

Actually, they're going to set up an auto merge of main to releases/6.1 so it'll just go there automatically. I think they just wanted to branch the compiler now, which makes sense. It needs time to settle down.

bnbarham · 2024-11-14T19:50:11Z

@swift-ci please test

hi2gage · 2024-11-15T18:00:38Z

Sounds good! Any further action needed from me on this PR to get those windows tests passing?

dschaefer2 · 2024-11-15T18:39:20Z

@swift-ci please test windows

dschaefer2 · 2024-11-15T19:58:20Z

I am a little worried about adding the dependency from SwiftPM to PackageDescription. It seems a lot of effort was expended in the past to avoid that, including duplicating most of the types, which I agree is pretty annoying. I assume that was to keep it only for the package manifest library.

@bnbarham any thoughts there? Any toolchain build issue we may run into?

bnbarham · 2024-11-15T20:02:18Z

@bnbarham any thoughts there? Any toolchain build issue we may run into?

@neonichu probably has the best background there

dschaefer2 · 2024-11-20T05:22:52Z

Hmm, the Windows build is failing with the missing dependency. It builds with CMake as well.

Sorry for the delayed responses. Too many things on the go at once :(.

hi2gage · 2024-12-06T00:17:56Z

No worries at all. Holidays are a busy time for me. The more I think about this the more I think that making this as DRY as possible might more pain than it's worth.

Looking at PackageDescription+Syntax.swift really all we need is a model that resents a dependency that is added.

Now it would have been convenient to use PackageDescription.Package.Dependency but maybe we just create a new model that will represent a dependency internally?

I propose we add a lightweight struct inside PackageModelSyntax that presents a dependence. Since PackageModelSyntax is a dependency of Commands then we can create this new model directly inside of SwiftPackageCommand.AddDependency

Thoughts on making this a little less DRY?

Example (Just copied the main properties that are required for this use case):

public struct Dependency {
    /// The type of dependency.
    @available(_PackageDescription, introduced: 5.6)
    public enum Kind {
        /// A dependency located at the given path.
        /// - Parameters:
        ///    - name: The name of the dependency.
        ///    - path: The path to the dependency.
        case fileSystem(name: String?, path: String)
        /// A dependency based on a source control requirement.
        ///  - Parameters:
        ///    - name: The name of the dependency.
        ///    - location: The Git URL of the dependency.
        ///    - requirement: The version-based requirement for a package.
        case sourceControl(name: String?, location: String, requirement: SourceControlRequirement)
        /// A dependency based on a registry requirement.
        /// - Parameters:
        ///   - id: The package identifier of the dependency.
        ///   - requirement: The version based requirement for a package.
        case registry(id: String, requirement: RegistryRequirement)
    }

    /// A description of the package dependency.
    @available(_PackageDescription, introduced: 5.6)
    public let kind: Kind
}

// MARK: - SourceControlRequirement

extension Dependency {
    @available(_PackageDescription, introduced: 5.6)
    public enum SourceControlRequirement {
        /// An exact version based requirement.
        case exact(Version)
        /// A requirement based on a range of versions.
        case range(Range<Version>)
        /// A commit based requirement.
        case revision(String)
        /// A branch based requirement.
        case branch(String)
    }
}

// MARK: - RegistryRequirement

extension Dependency {
    @available(_PackageDescription, introduced: 999)
    public enum RegistryRequirement {
        /// A requirement based on an exact version.
        case exact(Version)
        /// A requirement based on a range of versions.
        case range(Range<Version>)
    }
}

// MARK: - Version

public struct Version: Sendable {

    /// The major version according to the semantic versioning standard.
    public let major: Int

    /// The minor version according to the semantic versioning standard.
    public let minor: Int

    /// The patch version according to the semantic versioning standard.
    public let patch: Int

    /// The pre-release identifier according to the semantic versioning standard, such as `-beta.1`.
    public let prereleaseIdentifiers: [String]

    /// The build metadata of this version according to the semantic versioning standard, such as a commit hash.
    public let buildMetadataIdentifiers: [String]

the Command

dschaefer2 · 2024-12-06T21:27:26Z

Agreed. Does the PackageModel module contain types you could use?

hi2gage · 2024-12-07T05:26:11Z

Agreed. Does the PackageModel module contain types you could use?

Actually yes looking through the PackageModel module again MappablePackageDependency.Kind works perfectly.

Pushed up a new commit with that change. Tests are passing locally and no dependencies were added. So hopefully that passes the smell test.

hi2gage · 2024-12-07T05:26:50Z

Sources/PackageModelSyntax/AddPackageDependency.swift

@@ -18,7 +18,7 @@ import SwiftSyntax
 import SwiftSyntaxBuilder

 /// Add a package dependency to a manifest's source code.
-public struct AddPackageDependency {


swiftformat made this change

hi2gage · 2024-12-07T05:26:55Z

Sources/PackageModelSyntax/AddTarget.swift

@@ -18,7 +18,7 @@ import SwiftSyntaxBuilder
 import struct TSCUtility.Version

 /// Add a target to a manifest's source code.
-public struct AddTarget {


swiftformat made this change

dschaefer2 · 2024-12-09T16:34:18Z

So much cleaner. Thank you!

dschaefer2 · 2024-12-09T16:38:24Z

@swift-ci please test windows

hi2gage · 2025-02-10T06:41:08Z

Hey @dschaefer2 just wanted to bump this. Anything else needed from me?

dschaefer2 · 2025-02-10T15:34:57Z

Sorry about that. Things got pretty busy. Let me kick off the tests again.

dschaefer2 · 2025-02-10T15:35:08Z

@swift-ci please test

dschaefer2 · 2025-02-14T16:17:10Z

Finally back to this. Looks good. Thank you!

…tlang#7871) Enable passing relative path into `swift package add-dependency` ### Motivation: Currently only `AbsolutePath`s are supported with `--type path` flag ```bash swift package add-dependency /packagePath --type path ``` There is a need to support `RelativePath` because it's not uncommon to have a package structure inside of an xcodeproj as shown below. ```bash LocalPackages ├── ChildPackage │ ├── .gitignore │ ├── .swiftpm │ ├── Package.swift │ ├── Sources │ └── Tests └── ParentPackage ├── .build ├── .gitignore ├── .swiftpm ├── Package.swift ├── Sources └── Tests ``` If we want to open `ParentPackage` by itself, it will not be able to resolve that package. This pr allows for the user to add a package with a `RelativePath` path via the `swift package add-dependency --type path` command. Access level for - `.package(name:url:requirement:traits:)` - `.package(name:url:requirement:)` - `.package(id:requirement:traits:)` were upgraded from `private` to `package` allowing access to these functions from the `PackageCommands` module. ### Modifications: - Enable passing relative path into `swift package add-dependency` - Add unit test coverage ### Result: Both of the following commands are valid ```bash swift package add-dependency ../relative --type path ``` ```bash swift package add-dependency /absolute --type path ```

hi2gage marked this pull request as ready for review August 10, 2024 22:09

hi2gage requested review from bnbarham, MaxDesiatov, jakepetroules, francescomikulis and xedin as code owners August 10, 2024 22:09

hi2gage changed the title ~~Enable passing relative path into swift package add-dependency~~ Allow relative path with swift package add-dependency command Aug 10, 2024

hi2gage force-pushed the add_relative_paths branch from c9073bd to 3925579 Compare August 27, 2024 23:14

hi2gage requested review from dschaefer2, shawnhyam, bripeticca, plemarquand and owenv as code owners August 27, 2024 23:14

bnbarham reviewed Aug 29, 2024

View reviewed changes

hi2gage closed this Sep 12, 2024

hi2gage force-pushed the add_relative_paths branch from 3925579 to fbca3e6 Compare September 12, 2024 05:38

hi2gage reopened this Sep 12, 2024

hi2gage force-pushed the add_relative_paths branch from fa8e2a3 to 153aa99 Compare September 12, 2024 07:05

hi2gage force-pushed the add_relative_paths branch from 153aa99 to 6aae1fd Compare September 30, 2024 23:19

hi2gage requested a review from bnbarham September 30, 2024 23:24

hi2gage force-pushed the add_relative_paths branch from 094ffe0 to 9615510 Compare November 6, 2024 05:51

dschaefer2 approved these changes Nov 15, 2024

View reviewed changes

Add RelativePath

63210b8

hi2gage force-pushed the add_relative_paths branch from 77615b9 to 63210b8 Compare December 7, 2024 05:26

hi2gage commented Dec 7, 2024

View reviewed changes

dschaefer2 merged commit b84362b into swiftlang:main Feb 14, 2025
6 checks passed

plemarquand mentioned this pull request Jun 17, 2025

swift package add-dependency --type path does not work with relative paths #8832

Closed

1 task

Allow relative path with swift package add-dependency command #7871

Allow relative path with swift package add-dependency command #7871

Uh oh!

Conversation

hi2gage commented Aug 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation:

Modifications:

Result:

Uh oh!

hi2gage commented Aug 27, 2024

Uh oh!

bnbarham left a comment

Choose a reason for hiding this comment

Uh oh!

hi2gage commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnbarham commented Sep 12, 2024

Uh oh!

hi2gage commented Sep 12, 2024

Uh oh!

bnbarham commented Sep 19, 2024

Uh oh!

hi2gage commented Sep 30, 2024

Uh oh!

hi2gage commented Oct 15, 2024

Uh oh!

dschaefer2 commented Oct 16, 2024

Uh oh!

dschaefer2 commented Oct 16, 2024

Uh oh!

dschaefer2 commented Oct 16, 2024

Uh oh!

dschaefer2 commented Nov 7, 2024

Uh oh!

dschaefer2 commented Nov 7, 2024

Uh oh!

hi2gage commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hi2gage commented Nov 13, 2024

Uh oh!

dschaefer2 commented Nov 14, 2024

Uh oh!

dschaefer2 commented Nov 14, 2024

Uh oh!

bnbarham commented Nov 14, 2024

Uh oh!

hi2gage commented Nov 15, 2024

Uh oh!

dschaefer2 commented Nov 15, 2024

Uh oh!

dschaefer2 commented Nov 15, 2024

Uh oh!

bnbarham commented Nov 15, 2024

Uh oh!

dschaefer2 commented Nov 20, 2024

Uh oh!

hi2gage commented Dec 6, 2024

Example (Just copied the main properties that are required for this use case):

Uh oh!

dschaefer2 commented Dec 6, 2024

Uh oh!

hi2gage commented Dec 7, 2024

Uh oh!

hi2gage Dec 7, 2024

Choose a reason for hiding this comment

Uh oh!

hi2gage Dec 7, 2024

Choose a reason for hiding this comment

Uh oh!

dschaefer2 commented Dec 9, 2024

Uh oh!

dschaefer2 commented Dec 9, 2024

Uh oh!

hi2gage commented Feb 10, 2025

Uh oh!

dschaefer2 commented Feb 10, 2025

Uh oh!

Allow relative path with `swift package add-dependency` command #7871

Allow relative path with `swift package add-dependency` command #7871

hi2gage commented Aug 10, 2024 •

edited

Loading

hi2gage commented Sep 4, 2024 •

edited

Loading

hi2gage commented Nov 13, 2024 •

edited

Loading