Skip to content

Create symlinks for swift-frontend only once #61768

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Create symlinks for swift-frontend only once #61768

wants to merge 1 commit into from

Conversation

edymtt
Copy link
Contributor

@edymtt edymtt commented Oct 27, 2022 •
edited
Loading

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

@edymtt
Create symlinks for swift-frontend only once
ed57159 
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 edymtt mentioned this pull request Oct 27, 2022
Closed
@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 •
edited
Loading

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

compnerd
compnerd reviewed Oct 30, 2022
View reviewed changes
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?

tools/driver/CMakeLists.txt
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.

@asl asl mentioned this pull request Nov 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd compnerd left review comments

@drexin drexin Awaiting requested review from drexin

@gottesmm gottesmm Awaiting requested review from gottesmm

@etcwilde etcwilde Awaiting requested review from etcwilde

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@edymtt @compnerd