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

Jump to bottom

Add support for Swift Package Manager v. 5.3 #333

Closed
wants to merge 12 commits into from
Closed

Add support for Swift Package Manager v. 5.3 #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fde0a80
Add SwiftPM package manifest.
mattpolzin Nov 19, 2020
01ec5b3
try public headers path
mattpolzin Nov 19, 2020
dbb1550
also try explicitly looking for C headers in correct directory.
mattpolzin Nov 19, 2020
bf13be5
switch from 1 public header path to instead symbolic linking all head…
mattpolzin Nov 19, 2020
283f030
delete all headers from include symlinks except for the umbrella header.
mattpolzin Nov 19, 2020
c41f884
and back again to include dir but without umbrella header in it
mattpolzin Nov 19, 2020
549442f
ditch objc umbrella header and instead put all public headers into in…
mattpolzin Nov 19, 2020
6a5a56d
symlink resources, copy to package bundle, support package bundle fro…
mattpolzin Nov 19, 2020
7261f2a
Add excludes to remediate warnings in package manifest
mattpolzin Nov 19, 2020
feef6b1
Add note to changelog.
mattpolzin Nov 19, 2020
8c679a3
merge w/ upstream
mattpolzin Dec 21, 2020
6c1726b
Remove test target from Package.swift because it was not set up prope…
mattpolzin Dec 21, 2020
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
## `develop`

Changes for users of the library currently on `develop`:
- fixed a NaN when setting `zoomScale` in `NYTScalingImageView` when `ANIMATED_GIF_SUPPORT=1`
- fixed a NaN when setting `zoomScale` in `NYTScalingImageView` when `ANIMATED_GIF_SUPPORT=1`
- Add a SwiftPM package manifest.

## [5.0.5](https://github.com/nytimes/NYTPhotoViewer/releases/tag/5.0.5)

Expand Down
3 changes: 3 additions & 0 deletions NYTPhotoViewer/Resource Loading/NSBundle+NYTPhotoViewer.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
@implementation NSBundle (NYTPhotoViewer)

+ (instancetype)nyt_photoViewerResourceBundle {
#if SWIFT_PACKAGE
return SWIFTPM_MODULE_BUNDLE;
#endif
static NSBundle *resourceBundle = nil;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
Expand Down
1 change: 1 addition & 0 deletions NYTPhotoViewer/Resources/NYTPhotoViewerCloseButtonX.png
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewer.bundle/NYTPhotoViewerCloseButtonX.png
Copy link
Contributor

@mr-fixit mr-fixit Dec 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I see these PNGs are duplicated, in the new .bundle and in the new location (the Resources folder). is that necessary?

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately, yes. One would think the resources could just be copied from the .bundle (since it is just a folder) but without the Resources folder SwiftPM will not bother to create a bundle (or at least it skips creating the SWIFTPM_MODULE_BUNDLE macro that is used here to access the SwiftPM bundle).

I did give this a shot because it made plenty of sense to me: I removed the Resources folder and its symlinked files and changed the respective copy lines of the package manifest but then I got a build error in Xcode that SWIFTPM_MODULE_BUNDLE was not defined. Reverting my change allowed me to build successfully again.

There are a few places where the symlinks in this PR could be removed up by making changes to the project structure as a whole to align more closely with what SwiftPM expects out-of-box (the Resources folder being one of those) but my approach here was to leave as much alone as possible; adding symlinks meant I did not need to change the project at large.

Copy link
Author

Choose a reason for hiding this comment

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

Actually, I want to call out explicitly something I've said but not made really front and center so far; although GitHub makes it look like I copied a bunch of files, they are all actually symlinks. I'm still not over-the-moon about them, but at least this has not created a situation where these files need to be modified/deleted/etc. in multiple places.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've been looking into this, hoping to find a way to avoid adding the complication of maintaining symlinks to the resources in the .bundle. Your commit history tells me you traveled a similar road vis-a-vis the headers. I was hoping something like this would work:

            resources: [
                .copy("NYTPhotoViewer.bundle/NYTPhotoViewerCloseButtonX.png")

but found this bit of code causes NYTPhotoViewer.bundle to be added to the sources, not the png.

I'm still wrapping my head around the internals of SPM, but I'm thinking the final answer will be to reorganize the resources and includes of NYTPhotoViewer so that SPM can be supported without symlinks.

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense. If it were mine to maintain, I would prefer the project reorganization over the symlinks as well.

Let me know if there’s anything else I can do to help out. I’m no expert, but I do have a bit of history with SPM at this point.

Copy link
Contributor

Choose a reason for hiding this comment

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

I asked about this on the Swift forums, see https://forums.swift.org/t/package-resources-from-a-directory-named-with-an-extension/43423

1 change: 1 addition & 0 deletions NYTPhotoViewer/Resources/[email protected]
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewer.bundle/[email protected]
1 change: 1 addition & 0 deletions NYTPhotoViewer/Resources/[email protected]
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewer.bundle/[email protected]
1 change: 1 addition & 0 deletions NYTPhotoViewer/Resources/NYTPhotoViewerCloseButtonXLandscape.png
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewer.bundle/NYTPhotoViewerCloseButtonXLandscape.png
1 change: 1 addition & 0 deletions NYTPhotoViewer/Resources/[email protected]
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewer.bundle/[email protected]
1 change: 1 addition & 0 deletions NYTPhotoViewer/Resources/[email protected]
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewer.bundle/[email protected]
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NSBundle+NYTPhotoViewer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../Resource Loading/NSBundle+NYTPhotoViewer.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhoto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../Protocols/NYTPhoto.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoCaptionView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoCaptionView.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoCaptionViewLayoutWidthHinting.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../Protocols/NYTPhotoCaptionViewLayoutWidthHinting.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoDismissalInteractionController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoDismissalInteractionController.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoTransitionAnimator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoTransitionAnimator.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoTransitionController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoTransitionController.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoViewController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewController.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoViewerArrayDataSource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewerArrayDataSource.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoViewerContainer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../Protocols/NYTPhotoViewerContainer.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoViewerDataSource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../Protocols/NYTPhotoViewerDataSource.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotoViewerSinglePhotoDataSource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotoViewerSinglePhotoDataSource.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotosOverlayView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotosOverlayView.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTPhotosViewController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTPhotosViewController.h
1 change: 1 addition & 0 deletions NYTPhotoViewer/include/NYTScalingImageView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../NYTScalingImageView.h
40 changes: 40 additions & 0 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// swift-tools-version:5.3
import PackageDescription

let package = Package(
name: "NYTPhotoViewer",
platforms: [
.iOS(.v9)
],
products: [
.library(
name: "NYTPhotoViewer",
targets: ["NYTPhotoViewer"]),
],
dependencies: [
.package(url: "https://github.com/pinterest/PINRemoteImage.git", from: "3.0.1")
],
targets: [
.target(
name: "NYTPhotoViewer",
dependencies: ["PINRemoteImage"],
path: "NYTPhotoViewer",
exclude: [
"NYTPhotoViewer.bundle",
"Info.plist"
],
resources: [
.copy("Resources/NYTPhotoViewerCloseButtonX.png"),
.copy("Resources/[email protected]"),
.copy("Resources/[email protected]"),
.copy("Resources/NYTPhotoViewerCloseButtonXLandscape.png"),
.copy("Resources/[email protected]"),
.copy("Resources/[email protected]"),
],
cSettings: [
.headerSearchPath("./"),
.headerSearchPath("./Protocols"),
.headerSearchPath("./Resource Loading")
])
]
)