Skip to content

Conversation

@edymtt
Copy link
Contributor

@edymtt edymtt commented Oct 27, 2022

To achieve to, rely solely on add_swift_tool_symlink and
remove usages of swift_install_in_component.

This follows the intent of #6053.

Addresses rdar://101755409, supports rdar://101396797

To achieve to, rely solely on `add_swift_tool_symlink` and remove usages of `swift_install_in_component`. This follows the intent of swiftlang#6053. Supports rdar://101396797
@edymtt
Copy link
Contributor Author

edymtt commented Oct 27, 2022

@swift-ci please smoke test

@edymtt
Copy link
Contributor Author

edymtt commented Oct 27, 2022

@swift-ci please build toolchain

@edymtt
Copy link
Contributor Author

edymtt commented Oct 27, 2022

I verified at desk that the generated build.ninja is the same, and that tools/driver/cmake_install.cmake drops the instructions related to swift_install_in_component

@edymtt
Copy link
Contributor Author

edymtt commented Oct 27, 2022

@swift-ci please build toolchain Windows

@edymtt
Copy link
Contributor Author

edymtt commented Oct 28, 2022

Hitting the following error in the testing of the Windows toolchain -- this seems unrelated to these changes, since it happens with other PRs (e.g. #37710)

Archiving artifacts ‘build/artifacts/*’ doesn’t match anything, but ‘*’ does. Perhaps that’s what you mean? ERROR: Step ‘Archive the artifacts’ failed: No artifacts found that match the file pattern "build/artifacts/*". Configuration error? 
@edymtt
Copy link
Contributor Author

edymtt commented Oct 28, 2022

Checked that the generated toolchains still have the symlinks

# macOS % ls Library61768/Developer/Toolchains/swift-PR-61768-370.xctoolchain/usr/bin | grep swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 16:05 swift -> swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 16:05 swift-api-digester -> swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 16:05 swift-api-extract -> swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 16:05 swift-autolink-extract -> swift-frontend -rwxr-xr-x 1 emiotto admin 493436544 Oct 27 16:05 swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 16:05 swift-symbolgraph-extract -> swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 16:05 swiftc -> swift-frontend # Linux % ls usr61768/bin/ | grep swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 13:43 swift -> swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 13:43 swift-api-digester -> swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 13:43 swift-api-extract -> swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 13:43 swift-autolink-extract -> swift-frontend -rwxr-xr-x 1 emiotto admin 196556720 Oct 27 13:36 swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 13:43 swift-symbolgraph-extract -> swift-frontend lrwxr-xr-x 1 emiotto admin 14 Oct 27 13:43 swiftc -> swift-frontend 
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Do we still install the symbolic links when we now do a ninja install-distribution?

COMPONENT compiler)
swift_install_in_component(FILES "${SWIFT_RUNTIME_OUTPUT_INTDIR}/swift-api-digester${CMAKE_EXECUTABLE_SUFFIX}"
DESTINATION "bin"
COMPONENT compiler)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, why were these duplicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not entirely sure why -- my best guess from looking at #10337 is that the original PR had some merge conflicts at one point, and leaving those instructions in there was one way to avoid that.

cc @CodaFi in case he remembers the context behind that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants