Skip to content
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

Add support for Musl (#429) #430

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

Add support for Musl (#429) #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

##### Enhancements

* None.
* Add support for Musl(Static Linux SDK).
[arasan01](http://github.com/arasan01)
[#429](https://github.com/jpsim/Yams/issues/429)

##### Bug Fixes

Expand Down
6 changes: 5 additions & 1 deletion Sources/Yams/Representer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import Foundation

#if os(iOS) || os(macOS) || os(watchOS) || os(tvOS) || os(visionOS)

Check warning on line 11 in Sources/Yams/Representer.swift

View workflow job for this annotation

GitHub Actions / macOS 13 with Xcode 14.3 

unknown operating system for build configuration 'os'

Check warning on line 11 in Sources/Yams/Representer.swift

View workflow job for this annotation

GitHub Actions / macOS 13 with Xcode 14.3 

unknown operating system for build configuration 'os'

Check warning on line 11 in Sources/Yams/Representer.swift

View workflow job for this annotation

GitHub Actions / macOS 13 with Xcode 14.3 

unknown operating system for build configuration 'os'

Check warning on line 11 in Sources/Yams/Representer.swift

View workflow job for this annotation

GitHub Actions / macOS 13 with Xcode 14.3 

unknown operating system for build configuration 'os'

Check warning on line 11 in Sources/Yams/Representer.swift

View workflow job for this annotation

GitHub Actions / macOS 13 with Xcode 14.3 

unknown operating system for build configuration 'os'

Check warning on line 11 in Sources/Yams/Representer.swift

View workflow job for this annotation

GitHub Actions / macOS 13 with Xcode 14.3 

unknown operating system for build configuration 'os'

Check warning on line 11 in Sources/Yams/Representer.swift

View workflow job for this annotation

GitHub Actions / macOS 13 with Xcode 14.3 

unknown operating system for build configuration 'os'

Check warning on line 11 in Sources/Yams/Representer.swift

View workflow job for this annotation

GitHub Actions / macOS 13 with Xcode 14.3 

unknown operating system for build configuration 'os'
import Darwin
private let cpow: (_: Double, _: Double) -> Double = Darwin.pow
#elseif os(Windows)
Expand All @@ -18,10 +18,14 @@
import CoreFoundation
import Bionic
private let cpow: (_: Double, _: Double) -> Double = Bionic.pow
#else
#elseif canImport(Glibc)
import CoreFoundation
import Glibc
private let cpow: (_: Double, _: Double) -> Double = Glibc.pow
#elseif canImport(Musl)
import CoreFoundation
import Musl
private let cpow: (_: Double, _: Double) -> Double = Musl.pow
Copy link

Choose a reason for hiding this comment

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

Weren't native Swift math functions added to the STL in 2019?
https://github.com/swiftlang/swift/pull/23140/files

Do we still need to import any C libraries here?

Copy link
Author

Choose a reason for hiding this comment

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

Looking at the background to the introduction of this, it now seems like it would be okay to use STL math functions. In that case, we would need to check that there are no problems with all toolchains. If we are only going to support Musl, I think it would have less of an impact if you just explicitly loaded cpow.

introduce commit: 682d498

commit message:

We must disambiguate `Double.pow(_: Double, _: Double) -> Double` and
`__C.pow(_: Double, _: Double) -> Double` as the Swift for TensorFlow
branch adds the former into the standard library.  The explicit module
dispatch ensures that we make the overload resolution unambiguous
allowing building Yams.

Copy link

@bradleymackey bradleymackey Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I can tell, this workaround was only to support some of the low level/standard library changes as part of Swift for Tensorflow. As that project is now archived (and has been for several years at this point), I think reverting these Tensorflow-specific workarounds is fine and probably desirable so we don't run into these issues with other platforms/libc. Implementation in #436.

#endif

public extension Node {
Expand Down
Loading