leonbreedt / FavIcon

Swift library to detect icons supported by a website.
Other
126 stars 37 forks source link

Support Swift 5 / Swift Package Manager #27

Closed leonbreedt closed 4 years ago

leonbreedt commented 5 years ago

Work underway in branch https://github.com/leonbreedt/FavIcon/tree/swift-5.

steveluoxin commented 5 years ago

Hi! Thanks for the good work. Is there any way to know the image type? For instance png or jpg

neodave commented 4 years ago

Thanks for keeping up with this project and getting a Swift Package going, very helpful.

But I tried to convert from using this through cocoa pods and after installing with SPM I get this error: No such module 'libxmlFavicon' on the HTML.swift file in your sources folder. Not sure how to work around so going back to pod for now.

leonbreedt commented 4 years ago

@neodave libxml is my biggest headache, it's the way I use simple lightweight XML support with XPath, but Apple unfortunately keeps moving where they put the system libxml headers and libraries with each Xcode/OS release.

The problem is likely that it cannot find your header files when coming in through the SPM path, and if I recall correctly it was not possible to set some sane defaults via Package.swift.

See https://github.com/leonbreedt/FavIcon/blob/41eb6817e7bd8122d9dba06c04d9be27d7723970/Makefile#L19.

Without the extra flags at the end, swift build will fail, either due to not finding the LLVM module definition (-ISources/Modules), or due to not finding the libxml2 headers (-I${SDK_PATH...), which is likely the root cause of your error.

I haven't looked for a while, but maybe SPM now supports some more flexibility in finding system headers/libraries or custom LLVM modules?

neodave commented 4 years ago

Thank you so much for the reply. I will investigate and let you know what I find out

neodave commented 4 years ago

Quick follow up. Currently your HTML.swift files has these includes

import Foundation
import libxmlFavicon

Shouldn't that be ?

import Foundation
import libxml
import Favicon

In your link above you seem to be saying that the libxml should be included, but your import makes it look like a custom library libxnlFavicon

I will see if fixing the link to libxml does the trick

podkovyrin commented 4 years ago

I also tried to make FavIcon works via SPM but had no luck and run into this thread: https://forums.swift.org/t/referring-to-libxml2-in-swift-package-description/28880 For now, it seems impossible.

leonbreedt commented 4 years ago

@podkovyrin Thanks for doing this research. I guess the only way would be to vendor libxml2, but that makes it even more of a pain for users of this library, now they would have to build a C library as well.

I guess I should update the README.

robertveringa89 commented 4 years ago

Soooo, will there be a new release?

podkovyrin commented 4 years ago

I can confirm that just referring libxml2 module does the job.

diff --git i/Sources/FavIcon/HTML.swift w/Sources/FavIcon/HTML.swift
index 4998dcc..0de9590 100644
--- i/Sources/FavIcon/HTML.swift
+++ w/Sources/FavIcon/HTML.swift
@@ -16,7 +16,7 @@
 //

 import Foundation
-import libxmlFavicon
+import libxml2

 final class HTMLDocument {
     fileprivate var _document: htmlDocPtr!
diff --git i/Sources/FavIcon/XML.swift w/Sources/FavIcon/XML.swift
index 9ba052d..f5b305f 100644
--- i/Sources/FavIcon/XML.swift
+++ w/Sources/FavIcon/XML.swift
@@ -18,7 +18,7 @@
 import Foundation

 import Foundation
-import libxmlFavicon
+import libxml2

 final class XMLDocument {
     fileprivate var _document: xmlDocPtr!

However, the test target within a package is not working since it refers to resources that are not supported by Swift PM now. I guess their content can be embedded inside the tests code but it doesn't seem elegant.

leonbreedt commented 4 years ago

Hello all, good to hear that Apple now ships libxml2 in 11.4+.

Unfortunately, I am without a MacBookPro for the foreseeable future (ETA for it shipping here is now July), so cannot make/test the change.

leonbreedt commented 4 years ago

@podkovyrin @robertveringa89 @neodave later better than never ;)

I'm happy to report all is now well, it now builds out of the box on latest Xcode on a blank new OS installation without requiring anything custom like before, just Xcode 11.4.

I've released 3.1.0.

leonbreedt commented 4 years ago

Fixed by 4f2110c159d59095570b00b2c4091aa4e886749a.

neodave commented 4 years ago

Thank you for updating the package and for letting me know, really appreciate it.