Skip to content

stdlib: remove unnecessarily complex library detection #29062

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
Jan 8, 2020

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Jan 8, 2020

The python module will always be available when TensorFlow is being
built, remove the ad-hoc detection for it.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

@compnerd compnerd added the tensorflow This is for "tensorflow" branch PRs. label Jan 8, 2020
@compnerd
Copy link
Member Author

compnerd commented Jan 8, 2020

CC: @dan-zheng

@compnerd
Copy link
Member Author

compnerd commented Jan 8, 2020

@swift-ci please test tensorflow

@@ -63,7 +56,7 @@ add_swift_target_library(swiftTensorFlow ${SWIFT_STDLIB_LIBRARY_BUILD_TYPES} IS_
SWIFT_MODULE_DEPENDS_FREEBSD Glibc
SWIFT_MODULE_DEPENDS_CYGWIN Glibc
SWIFT_MODULE_DEPENDS_HAIKU Glibc
${TENSORFLOW_DEPENDS_PYTHON}
SWIFT_MODULE_DEPENDS Python
Copy link
Contributor

Choose a reason for hiding this comment

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

For now, making Python an explicit (rather than informal) dependency of TensorFlow is good.

However, only a small portion of TensorFlow depends on Python. Eventually, we may want to add a TensorFlow+Python module. Related discussion on "cross-import overlays".

Perhaps the Python dependency issue will go away or change in nature when TensorFlow is no longer built as part of the standard library: tensorflow/swift-apis#488.

Copy link
Member Author

Choose a reason for hiding this comment

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

It definitely would be nice to have the ability to not have Python used. However, I think that we currently do not have that ability so this is simply structuring the code to be simpler in light of that. I think that when the time comes, providing a user controllable knob to enable the dependency is a better approach. The user knows what they want, we do not need to infer it, and that still would avoid the ad-hoc state tracking.

The python module will always be available when TensorFlow is being
built, remove the ad-hoc detection for it.
@compnerd
Copy link
Member Author

compnerd commented Jan 8, 2020

@swift-ci please test tensorflow

@compnerd compnerd merged commit 0df0544 into swiftlang:tensorflow Jan 8, 2020
@compnerd compnerd deleted the not-keeping-track branch January 8, 2020 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tensorflow This is for "tensorflow" branch PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants