Skip to content

[Commands] AddSetting: Searching for a target in the manifest shouldn… #8699

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

Merged
merged 1 commit into from
May 22, 2025

Conversation

xedin
Copy link
Contributor

@xedin xedin commented May 19, 2025

…'t walk targets recursively

Motivation:

Otherwise in situations like:

let package = Package(
  name: "packages",
  targets: [
    .target(
      name: "MyTest",
      dependencies: [
        .byName(name: "Dependency")
      ]
    ),
    .target(
      name: "Dependency"
    )
  ]
)

When adding settings to Dependency we'd find .byName(name: "Dependency")
instead of .target(name: "Dependency", ...).

Modifications:

  • Replace use of findFirst in AddSwiftSetting.addToTarget with a custom walk over the targets array.

Result:

Find correct element in the manifest "targets" array.

@xedin
Copy link
Contributor Author

xedin commented May 19, 2025

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented May 21, 2025

@swift-ci please test self hosted Windows platform

@xedin
Copy link
Contributor Author

xedin commented May 21, 2025

@swift-ci please test self hosted Windows platform

@xedin xedin force-pushed the fix-findFirst-in-addSetting branch from dc92027 to de4a631 Compare May 21, 2025 21:40
@xedin
Copy link
Contributor Author

xedin commented May 21, 2025

@swift-ci please test

…'t walk targets recursively

Otherwise in situations like:

```swift
let package = Package(
  name: "packages",
  targets: [
    .target(
      name: "MyTest",
      dependencies: [
        .byName(name: "Dependency")
      ]
    ),
    .target(
      name: "Dependency"
    )
  ]
)
```

When adding settings to `Dependency` we'd find `.byName(name: "Dependency")`
instead of `.target(name: "Dependency", ...)`.
@xedin xedin force-pushed the fix-findFirst-in-addSetting branch from de4a631 to 612cc07 Compare May 21, 2025 21:57
@xedin
Copy link
Contributor Author

xedin commented May 21, 2025

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented May 22, 2025

@swift-ci please test Windows platform

dschaefer2 pushed a commit that referenced this pull request May 22, 2025
…d fixes (#8710)

Includes:
  
  - #8671
  - #8677
  - #8700
  - #8699

---------

Co-authored-by: Anthony Latsis <[email protected]>
@xedin xedin merged commit 6e8b312 into swiftlang:main May 22, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants