Skip to content

Conversation

@Steelskin
Copy link
Contributor

Previously, libs were installed under lib/swift/${os}. They should be installed in the default library directory for the relevant target system.
In addition, swiftmodules were installed in the older layout format. This changes to use the standard modern layout format for swiftmodules.

Checklist

  • [*] I've read the Contribution Guidelines
  • [*] My contributions are licensed under the Swift license.
  • [*] I've followed the coding style of the rest of the project.
  • [*] I've added tests covering all new code paths my change adds to the project (if appropriate).
  • [*] I've added benchmarks covering new functionality (if appropriate).
  • [*] I've verified that my change does not break any existing tests or introduce unexplained benchmark regressions.
  • [*] I've updated the documentation if necessary.

Copy link
Member

@lorentey lorentey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me! Will this change affect builds of swift-collections that are part of Swift toolchains?

Oh, could you please target this change on the release/1.2 branch rather main? I expect we'll want to quickly ship this.

@Steelskin
Copy link
Contributor Author

The changes look good to me! Will this change affect builds of swift-collections that are part of Swift toolchains?

Oh, could you please target this change on the release/1.2 branch rather main? I expect we'll want to quickly ship this.

Ah sorry, I opened it as a draft PR because I am waiting for a review on apple/swift-argument-parser#685, which does the same thing but in swift-argument-parser. I intend to update the rest of the libraries used by the toolchain in the same manner and I would like to stay consistent.

Previously, libs were installed under `lib/swift/${os}`. They should be
installed in the default library directory for the relevant target
system.
In addition, swiftmodules were installed in the older layout format.
This changes to use the standard modern layout format for swiftmodules.
@Steelskin Steelskin force-pushed the fabrice/cmake-install-paths branch from 7c716d0 to e0021fb Compare February 3, 2025 21:16
@Steelskin Steelskin changed the base branch from main to release/1.2 February 3, 2025 21:17
@Steelskin Steelskin marked this pull request as ready for review February 3, 2025 21:17
@Steelskin Steelskin requested a review from lorentey February 3, 2025 21:17
@Steelskin
Copy link
Contributor Author

apple/swift-argument-parser#685 has landed. This is bringing the same changes to this repository. I changed the base branch to release/1.2 as requested. Will this make it back to main automatically or do I need to do something else?

This also reverts 40adeb7, which seems to have been done in error.
@Steelskin
Copy link
Contributor Author

I pushed an update that reverts commit 40adeb7, it seems to have been done in error. Other Swift libraries put the .swiftdoc in the .swiftmodule folder.

Steelskin added a commit to Steelskin/swift-installer-scripts that referenced this pull request Feb 3, 2025
Steelskin added a commit to Steelskin/swift-installer-scripts that referenced this pull request Feb 4, 2025
@lorentey
Copy link
Member

lorentey commented Feb 7, 2025

Will this make it back to main automatically or do I need to do something else?

You don't need to do anything else -- the changes will get picked up the next time we merge things forward. (This is a manual process, so there can be a delay. But this repo's main should not be used in production.)

@lorentey lorentey merged commit b5e880d into apple:release/1.2 Feb 7, 2025
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.

2 participants