Swift Package Manager support #1465

3a4oT · 2020-10-09T12:51:07Z

Changes in this pull request

Added proper support for Swift Package Manager (SPM).

"inverted" the current master's files structure by putting all public headers to the include folder and all internal/private moved to the package's root. Swift Package Manager documentation
created symbolical links for IGListKit with the relative path to IGListDiffKit's private headers to work around an issue for private headers search path. Seems like it works pretty well for SPM, cocoapods and original Xcode workspace.
added macOS Catalyst support

Supported targets:

- IGListSwiftKit

- IGListKit

- IGListDiffKit

TODO:

add a way to test SPM support.

Issue fixed: #1368 #1406

Checklist

All tests pass. Demo project builds and runs.
I added tests, an experiment, or detailed why my change isn't tested.
I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
I have reviewed the contributing guide

- updated .gitignore for SPM - updated Package.swift and set 5.3 as minimum required version. - tweaked IGListCompatibility header imports with TARGET_OS_MACCATALYST so we can build for mac Catalyst.

- support for macCatalyst - changed internal imports to use double quotes instead of angel brackets.

- tweaked imports to workaround swiftPM limitations. - IGListKit already module builds with IGListDiffKit sources and not use it as a separate module dependency. It allows us to avoid "#include<> " mess and keep project structure so cocoapods/carthage/xcode tests should just works. - added USE_SWIFT_PACKAGE_FROM_XCODE to proper support case where swift package was imported as the dependency to Xcode project (using build in support).

3a4oT · 2020-10-09T13:04:54Z

Not sure how to trigger CI?

3a4oT · 2020-10-10T11:20:16Z

So strange. If you checkout on commit c2caf741657552458560b25a4fe14655eb40a1ef IGListKit can be built as a Package from Xcode without errors, but if you try to integrate it as a dependency on a regular Xcode project as soon as you try import IGListKit you will see a compilation error. Maybe it's an Xcode bug since it's definitely related to the headers search path since imports in angle brackets (<IGListDiffkit/someheader.h> ) can't be resolved by the Xcode project which depends on IGListKit

I made some more experiments and moved all public headers to the include folder and changes all import to use local mode with double quotes "IGListDiffkit.h". This seems to work for IGListKit as a Swift package (from Xcode) and can be imported as a dependency for some regular Xcode project ( I have to define USE_PACKAGE_FROM_XCODE to properly manage imports). So the latest commit seems to work, now need to figure out if other things don't break cocoapods (lint passed locally, but I have doubts whether it works on real projects), unit test.

# Conflicts: # IGListKit.xcodeproj/project.pbxproj # Package.swift # Source/IGListKit/IGListKit.h

lorixx · 2020-10-13T17:55:17Z

Hey hey, thanks for working on this! I also tried out a bit but couldn't figure out how to go around the file structure thing.

Looks like your latest approach is to change the files structure to go around the search path thing from Xcode setting, do they work in your latest version? I think it would be a bummer to change all the <IGListKit/XXX.h> to be "XXX.h" format within the library here. But let's see how it goes.

3a4oT · 2020-10-13T20:43:46Z

Hey @lorixx! Yes, I've "inverted" the current master's files structure by putting all public headers to the include folder and all internal/private moved to the package's root. Swift Package Manager documentation

created symbolical links for IGListKit with the relative path to IGListDiffKit's private headers to work around an issue for private headers search path. Seems like it works pretty well for SPM, cocoapods and original Xcode workspace.

So far I was able to verify:

that the cocoapods support works as expected, and projects from the Example folder passed tests (iOS one).
Xcode 12 SPM integration work as expected.

I have some issues with running unit tests from Xcode locally, but it's the same issue in the current master (my env is Xcode 12.0.1, Xcode 11.7). I see that .travis mapped to Xcode 10.2 but maybe there is some other way to make it work on a newer environment? BTW. do you trigger CI manually?

lorixx · 2020-10-21T05:28:55Z

BTW. do you trigger CI manually?

@3a4oT I think one issue I saw might be related here is that the PR is in "Draft" mode, thus it doesn't trigger the CI I believe, for example #1430 this PR triggered the CI.

SebastianBoldt · 2020-11-03T18:08:58Z

We moved most of our dependencies to SPM because Cocoapods doesn't play very well with Xcode 12.
The only thing thats missing is IGListKit. Any Progress on this one?

3a4oT · 2020-11-03T19:40:33Z

I have some other work right now, but my plan is to alloc some time during the upcoming weekend. :)

3a4oT · 2020-11-17T21:57:23Z

Tests (github workflow) passed on my fork:
https://github.com/3a4oT/IGListKit/actions/runs/368991395

3a4oT · 2020-12-02T01:29:48Z

@lorixx friendly ping ...

lorixx · 2020-12-04T21:50:24Z

Thanks again for working on this! Does this work for the other integrations? Cocoapods and Carthage.
Also looks like this change added a new github CI support which is great, did you get the build/test result from the previous travis CI tool? cc @bdotdub @maxolls @Ziewvater We might need to discuss the overall code structure change here.

3a4oT · 2020-12-04T21:57:41Z

Hey @lorixx . Yeah, Carthage and cocoa pods passed. The latest test results can be found here https://github.com/3a4oT/IGListKit/actions/runs/372069244 . I have opened separate PR for CI support if you want to go it first for better visibility.

lorixx · 2020-12-04T21:59:18Z

Hey @lorixx . Yeah, Carthage and cocoa pods passed. The latest test results can be found here https://github.com/3a4oT/IGListKit/actions/runs/372069244 . I have opened separate PR for CI support if you want to go it first for better visibility.

yeah I think it's better to decouple the SPM support and the new Github CI support, since they are doing different things and related to each other.

3a4oT · 2020-12-04T21:59:31Z

#1478 CI

RamblinWreck77 · 2020-12-07T15:51:12Z

@3a4oT Just tested out your spmBrain branch in my project and everything is looking great! I'll have our QA poke around and see if they find any issues but so far so good. Nice work :)

RamblinWreck77 · 2020-12-09T16:07:51Z

Just an update, no issues found! Seems production ready to me!

RamblinWreck77 · 2020-12-10T18:56:41Z

@lorixx is there a timeline available for getting this merged? We've actually pushed this to our production environment without issue, glad to finally be free of cocoapods!

facebook-github-bot

@lorixx has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

lorixx

I just found out that all the files are deleted and re-added, this lost all the previous git history or blame, could you use the "git mv" or similar to mark the file as a move instead of "delete+add"? The goal is to keep the git history and blame for each file so that we know who was the original author. Thanks!

lorixx · 2020-12-11T21:15:21Z

.github/workflows/CI.yml

@@ -0,0 +1,224 @@
+name: "IGListKit CI"


Remove this since you created a separate PR here.

I just found out that all the files are deleted and re-added, this lost all the previous git history or blame, could you use the "git mv" or similar to mark the file as a move instead of "delete+add"? The goal is to keep the git history and blame for each file so that we know who was the original author. Thanks!

Sorry for the mess, it was a hard time put those together and I moved files around with different combinations. I think I found a better solution for it, please see #1487. I tried to keep changes as small as possible.

PS. About CI: while there is another open #1478 only for CI support, this PR was based on it since I need some way to verify my changes. If I remove support (in #1487 I didn't add it at all, for now) how we will see the progress/status? Or your plan is to land #1478 first?

3a4oT · 2020-12-14T22:35:00Z

@RamblinWreck77 thanks for the feedback!

RamblinWreck77 · 2021-01-04T17:10:59Z

Any progress on official support?

relsirc · 2021-02-16T09:45:01Z

any update on this?

acosmicflamingo · 2021-06-13T23:58:30Z

You know what I'm going to ask :)

Summary: ## Changes in this pull request A better version of Instagram#1465 =) - SPM support with script-based generations. - added macOS Catalyst support ### Generate SPM layout 1. From **project's root** run: `bash scripts/generate_spm_sources_layout.sh` 2. Commit Changes Repeat those steps each time you delete/add the project's files. **Make sure** to have this CI step which will check that `generate_spm_sources_layout.sh` is not broken. Issue fixed: Instagram#1368 Instagram#1406 ### Checklist - [ ] All tests pass. Demo project builds and runs. - [ ] I added tests, an experiment, or detailed why my change isn't tested. - [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [ ] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Pull Request resolved: Instagram#1487 Reviewed By: candance Differential Revision: D25562739 Pulled By: lorixx fbshipit-source-id: eb4f9e82e6b4842aae71585e0c1377c13cf21196

Summary: ## Changes in this pull request A better version of Instagram#1465 =) - SPM support with script-based generations. - added macOS Catalyst support ### Generate SPM layout 1. From **project's root** run: `bash scripts/generate_spm_sources_layout.sh` 2. Commit Changes Repeat those steps each time you delete/add the project's files. **Make sure** to have this CI step which will check that `generate_spm_sources_layout.sh` is not broken. Issue fixed: Instagram#1368 Instagram#1406 ### Checklist - [ ] All tests pass. Demo project builds and runs. - [ ] I added tests, an experiment, or detailed why my change isn't tested. - [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [ ] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Pull Request resolved: Instagram#1487 Differential Revision: D30428297 Pulled By: lorixx fbshipit-source-id: 7fe5e99f2c6faf695a74588743a17fcafd02de44

Summary: ## Changes in this pull request A better version of #1465 =) - SPM support with script-based generations. - added macOS Catalyst support ### Generate SPM layout 1. From **project's root** run: `bash scripts/generate_spm_sources_layout.sh` 2. Commit Changes Repeat those steps each time you delete/add the project's files. **Make sure** to have this CI step which will check that `generate_spm_sources_layout.sh` is not broken. Issue fixed: #1368 #1406 ### Checklist - [ ] All tests pass. Demo project builds and runs. - [ ] I added tests, an experiment, or detailed why my change isn't tested. - [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [ ] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Pull Request resolved: #1487 Reviewed By: DimaVartanian, candance Differential Revision: D30428297 Pulled By: lorixx fbshipit-source-id: 655291ff03445dec9b0b8cd97916f0c88207e9a7

TimOliver · 2023-04-26T02:41:58Z

Hey folks! Sorry to revive this old thread. I'm going through the list of PRs now and seeing which ones we can close out in preparation of a new version launch.

It seems to me that this PR was further refined down the line and this was used to create the SPM setup we have today. I can definitely confirm that SPM support works great now (We're using it for the sample projects now!) so I think we can close this PR out having done its job.

Thanks again for all your contributions!

3a4oT added 3 commits October 9, 2020 13:06

IGListDiffKit:

82378a5

- updated .gitignore for SPM - updated Package.swift and set 5.3 as minimum required version. - tweaked IGListCompatibility header imports with TARGET_OS_MACCATALYST so we can build for mac Catalyst.

IGListKit:

48ff62b

- support for macCatalyst - changed internal imports to use double quotes instead of angel brackets.

3a4oT marked this pull request as draft October 9, 2020 12:51

facebook-github-bot added the CLA Signed label Oct 9, 2020

This was referenced Oct 9, 2020

Xcode 11 SPM Support/Release #1368

Closed

Swift Package Manager integration already broken (4.0.0), though it's not a pain #1406

Open

Added SPM support TextureGroup/Texture#1928

Open

more SPM

7b111bc

3a4oT added 10 commits October 11, 2020 12:09

symlinks

b1845d2

Merge branch 'master' into spmBrain

f654020

added spm support for IGListDiffKit

40b82dd

added SPM support for IGListKit

d783d79

added spm support for IGListSwiftKit

26d6092

fixed pod integration

41833cc

cxx11

a746022

exp 1

2632f6b

podspec

f8faa5d

Merge branch 'moreSPM' into spmBrain

b48353c

# Conflicts: # IGListKit.xcodeproj/project.pbxproj # Package.swift # Source/IGListKit/IGListKit.h

3a4oT added 3 commits October 13, 2020 22:18

dropped USE_PACKAGE_FROM_XCODE

b5bf2af

work with podspec

abc97a0

cleanup Package.swift

961b4ca

Merge branch 'master' into spmBrain

8d6d2ed

3a4oT added 2 commits November 17, 2020 23:26

fixed macOS tests

e565ab3

remove redundent jobs.

f212406

3a4oT added 3 commits November 19, 2020 00:14

added test project to examples folder to track Xcode's spm integration

072e409

filled Changelog

d6367c5

try workaround CI issues for Carthage build

5072642

3a4oT force-pushed the spmBrain branch from 268b21d to 5072642 Compare November 19, 2020 10:28

aaronbrethorst mentioned this pull request Dec 10, 2020

Move IGListKit to SPM; remove Carthage OneBusAway/onebusaway-ios#331

Closed

2 tasks

facebook-github-bot reviewed Dec 11, 2020

View reviewed changes

lorixx requested changes Dec 11, 2020

View reviewed changes

lorixx reviewed Dec 11, 2020

View reviewed changes

3a4oT mentioned this pull request Dec 14, 2020

SPM number10 #1487

Closed

4 tasks

lorixx mentioned this pull request Aug 14, 2021

SPM number10 (#1487) #1521

Closed

4 tasks

TimOliver closed this Apr 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swift Package Manager support #1465

Swift Package Manager support #1465

3a4oT commented Oct 9, 2020 •

edited

Loading

3a4oT commented Oct 9, 2020

3a4oT commented Oct 10, 2020

lorixx commented Oct 13, 2020 •

edited

Loading

3a4oT commented Oct 13, 2020 •

edited

Loading

lorixx commented Oct 21, 2020

SebastianBoldt commented Nov 3, 2020

3a4oT commented Nov 3, 2020

3a4oT commented Nov 17, 2020

3a4oT commented Dec 2, 2020

lorixx commented Dec 4, 2020

3a4oT commented Dec 4, 2020

lorixx commented Dec 4, 2020

3a4oT commented Dec 4, 2020

RamblinWreck77 commented Dec 7, 2020

RamblinWreck77 commented Dec 9, 2020

RamblinWreck77 commented Dec 10, 2020

facebook-github-bot left a comment

lorixx left a comment

lorixx Dec 11, 2020

3a4oT Dec 14, 2020

3a4oT Dec 14, 2020

3a4oT commented Dec 14, 2020

RamblinWreck77 commented Jan 4, 2021

relsirc commented Feb 16, 2021

acosmicflamingo commented Jun 13, 2021

TimOliver commented Apr 26, 2023

Swift Package Manager support #1465

Swift Package Manager support #1465

Conversation

3a4oT commented Oct 9, 2020 • edited Loading

Changes in this pull request

Checklist

3a4oT commented Oct 9, 2020

3a4oT commented Oct 10, 2020

lorixx commented Oct 13, 2020 • edited Loading

3a4oT commented Oct 13, 2020 • edited Loading

lorixx commented Oct 21, 2020

SebastianBoldt commented Nov 3, 2020

3a4oT commented Nov 3, 2020

3a4oT commented Nov 17, 2020

3a4oT commented Dec 2, 2020

lorixx commented Dec 4, 2020

3a4oT commented Dec 4, 2020

lorixx commented Dec 4, 2020

3a4oT commented Dec 4, 2020

RamblinWreck77 commented Dec 7, 2020

RamblinWreck77 commented Dec 9, 2020

RamblinWreck77 commented Dec 10, 2020

facebook-github-bot left a comment

Choose a reason for hiding this comment

lorixx left a comment

Choose a reason for hiding this comment

lorixx Dec 11, 2020

Choose a reason for hiding this comment

3a4oT Dec 14, 2020

Choose a reason for hiding this comment

3a4oT Dec 14, 2020

Choose a reason for hiding this comment

3a4oT commented Dec 14, 2020

RamblinWreck77 commented Jan 4, 2021

relsirc commented Feb 16, 2021

acosmicflamingo commented Jun 13, 2021

TimOliver commented Apr 26, 2023

3a4oT commented Oct 9, 2020 •

edited

Loading

lorixx commented Oct 13, 2020 •

edited

Loading

3a4oT commented Oct 13, 2020 •

edited

Loading