SPM number10 #1487

3a4oT · 2020-12-14T22:19:38Z

Changes in this pull request

A better version of #1465 =)

SPM support with script-based generations.
added macOS Catalyst support

Generate SPM layout

From project's root run:

bash scripts/generate_spm_sources_layout.sh
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

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

cc @lorixx

facebook-github-bot

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

3a4oT · 2020-12-22T13:54:59Z

I see that job Facebook Internal - Builds & Tests failed but it's not very informative. Would be nice to get some feedback :)

lorixx · 2021-01-11T19:15:31Z

I checked and we have this failure:

Summary: 
In file included from fbobjc/VendorLib/IGListKit/src/Source/IGListKit/IGListAdapter.h:18:
fbobjc/VendorLib/IGListKit/src/Source/IGListKit/IGListAdapterDataSource.h:10:9: fatal error: 'IGListDiffable.h' file not found
#import "IGListDiffable.h"
        ^~~~~~~~~~~~~~~~~~
In file included from fbobjc/VendorLib/IGListKit/src/Source/IGListKit/IGListExperimentalAdapterUpdater.m:17:
fbobjc/VendorLib/IGListKit/src/Source/IGListDiffKit/Internal/IGListIndexSetResultInternal.h:10:9: fatal error: 'IGListIndexSetResult.h' file not found
#import "IGListIndexSetResult.h"
        ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from fbobjc/VendorLib/IGListKit/src/Source/IGListKit/Internal/IGListBindingSectionController+DebugDescription.h:8:
fbobjc/VendorLib/IGListKit/src/Source/IGListKit/IGListBindingSectionController.h:10:9: fatal error: 'IGListMacros.h' file not found
#import "IGListMacros.h"
        ^~~~ ...(14450 bytes)... 0:03:09,511][vcsutils.mercurial][DEBUG] run hg: hg id -i
=================================

RamblinWreck77 · 2021-01-21T14:29:33Z

Hey guys! Any updates? I'd love to get this out on the main repo as soon as possible. For reference, this branch has been working great for me in the latest xcode:

https://github.com/3a4oT/IGListKit/tree/spmBrain

We even took a snapshot and pushed it to prod with zero issues/crashes/reported problems.

3a4oT · 2021-02-09T11:38:35Z

Hey! this is on my to-do list (upcoming, maybe next weekend :)), but I'm not sure how to deal with internal tests that are failing for Instagram.

facebook-github-bot · 2021-03-05T11:39:42Z

@3a4oT has updated the pull request. You must reimport the pull request before landing.

3a4oT · 2021-03-05T13:20:36Z

OK, so I rebased with the current master! Seems like at least locally I still can build it as a swift package which is good. ~3 months later I see that the current master:

has broken Cocoapods and Carthage support
IGListDiffKit is no longer a macOS framework since it has "UIKIt" import
Some unit tests are failing locally
(I can say it because I have separate PR for GitHub-action based CI which was all green while then)
Current status of this PR:

From the error description fbobjc/VendorLib/IGListKit/src/Source/ I can assume that Facebook is using some internal build tool and if that's correct it makes things a little harder to debug. I guess errors like fbobjc/VendorLib/IGListKit/src/Source/IGListKit/IGListAdapterDataSource.h:10:9: fatal error: 'IGListDiffable.h' file not found #import "IGListDiffable.h" can be fixed by conditional imports but I need a way to trigger CI. Some side question, does the Instagram team interest to accept the third-party changes and work with contributors? I ask because :

you do not have a clear release schedule (at least for me)
you do not have a community available CI
this GitHub repository seems to be more like a mirror?

Any answer is good for me, I just need to know whether I should continue to invest time and try to make upstream or just create a hard fork for my own needs?=)

Thanks for the awesome framework :)

3a4oT · 2021-03-05T15:50:29Z

#1478 introduce CI jobs, would be great to land it first. @lorixx

BrentMifsud · 2021-08-10T20:17:01Z

any updates on SPM support? Its still broken for me.

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

lorixx · 2021-08-16T07:08:29Z

Thanks for raising the issue @3a4oT:

We def dont want to break the CI, however the Travis CI was deprecated mid last year and we hadn't prioritized fixing it. But I think we should use the Github Action! Thanks for your initiative to spin up the effort in another PR!
We would love to resolve these two: Add Github Action CI and support SPM as the community keeps growing and the SPM becomes mainstream.
I dont think the Github repo is a minor here, would def love to see this getting better and better!

Please keep the feedback and suggestions coming!

lorixx · 2021-08-18T21:14:07Z

Awesome @3a4oT ! Looks like all the Unit Tests + Build are done successfully! Do you mind rebase withe the master branch instead? It already includes some of the changes: e.g. Github Action.

Can you create a separate PR for fixing the CI? Thanks a lot!

lorixx · 2021-08-18T21:15:00Z

This is the most heroic PR the community got recently! Thanks Petro! @3a4oT

added Carthage job for Xcframework and legacy Lipo

3a4oT · 2021-08-19T08:16:00Z

@lorixx I rebased on master(please double-check since there was a mess with commits and I completely forgot what I did half a year ago :)).

moved most of the CI jobs to the latest macOS and Xcode 12.5.1
UITests failing on CI under macOS11, so I left it under macOS 10.15 and the latest available Xcode - 12.4
Added jobs that verify SPM targets can be built by an Xcode. RE. your "test plan". Run "swift build" directly is wrong for this package since SPM builds for the host environment and can't target anything other than macOS. The proper way to do so is to remove the project's *.xcproject/workspace files. And invoke xcodebuild tool which under the hood will use Package.swift for a build.
However, the above method does not guarantee that third-party apps can consume IGListKit without issues, we may need to add an Example project which integrates with the local version of IGListKit and build it as a CI job.

Does it all make sense for you?

lorixx · 2021-08-19T16:40:37Z

@lorixx I rebased on master(please double-check since there was a mess with commits and I completely forgot what I did half a year ago :)).

moved most of the CI jobs to the latest macOS and Xcode 12.5.1

UITests failing on CI under macOS11, so I left it under macOS 10.15 and the latest available Xcode - 12.4

Added jobs that verify SPM targets can be built by an Xcode. RE. your "test plan". Run "swift build" directly is wrong for this package since SPM builds for the host environment and can't target anything other than macOS. The proper way to do so is to remove the project's *.xcproject/workspace files. And invoke xcodebuild tool which under the hood will use Package.swift for a build.

However, the above method does not guarantee that third-party apps can consume IGListKit without issues, we may need to add an Example project which integrates with the local version of IGListKit and build it as a CI job.

Does it all make sense for you?

Thanks @3a4oT ! It all makes sense to me, I will trigger the internal build and ensure the latest didn't break internally as well then we can land the change. Finger cross! The only thing I would ask your help in the gigantic PR change, is to add comment in the change that you believe worth mentioning: e.g. Pod file change, special handling by changing the bitcode enabled or any workaround that applies to the swift package manager layout.

I might ask question inline the PR too.

Again, thank you so much for your hard work here!

facebook-github-bot · 2021-08-19T16:45:14Z

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

lorixx

Overall it looks great! The next step is to address my inline comment and document anything tricky workaround you did in the development. At the same time, I will work on the internal build to ensure the build works. Thanks Petro!

Source/IGListKit/IGListAdapterUpdaterCompatible.h

lorixx · 2021-08-19T19:02:47Z

scripts/generate_spm_sources_layout.sh

+
+#  2. Commit Changes
+
+#  Repeate those steps each time you delete/add project's files. **Make sure** to have this CI step which will check that `generate_spm_sources_layout.sh` is not broken.


Looks like we dont have this step in CI yet, maybe add it to the Github action file or the ./setup.sh too? What do you think?

Source/IGListDiffKit/IGListBatchUpdateData.mm

Source/IGListDiffKit/IGListDiff.mm

Podfile

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: Follow up with the PR: Instagram#1487 and to resolve some of the internal build failure. = Facebook = This is to follow up with the PR diff: D30428297 to fix the internal BUCK build error. Differential Revision: D30437725 fbshipit-source-id: 8eb6a48a614a857806ee0ab3152707e9b4fd147a

facebook-github-bot · 2021-08-23T11:10:37Z

@3a4oT has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-08-23T11:24:27Z

@3a4oT has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-08-23T11:28:33Z

@3a4oT has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-08-23T11:43:45Z

@3a4oT has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2021-08-23T12:14:18Z

@3a4oT has updated the pull request. You must reimport the pull request before landing.

3a4oT · 2021-08-23T12:29:52Z

@lorixx I addressed your feedback, please let me know whether I need to do something else

facebook-github-bot · 2021-08-23T16:06:28Z

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

lorixx · 2021-08-25T23:16:14Z

@3a4oT Just a quick update, we are still doing the internal review, I made some changes and the internal builds are passing fine. Will keep you posted, thanks!

3a4oT · 2021-08-26T08:10:42Z

Thanks for the update

facebook-github-bot · 2021-09-01T02:28:48Z

@lorixx merged this pull request in a1036e0.

lorixx · 2021-09-01T04:08:35Z

Congrats @3a4oT 🎉🎉🎉! Finally merged the change, SPM support should be now available! Btw, do you have idea why https://github.com/Instagram/IGListKit/runs/3479832142?check_suite_focus=true still has unit tests build issue? Thanks!

Summary: There were a lot of placeholder references to the PRs of committed features in the CHANGELOG. I went along each one and tagged each PR/commit that added the feature. Differential Revision: D45477107 fbshipit-source-id: 4d64366bbafc07de278aaacac7e8558b6483cae4

facebook-github-bot added the CLA Signed label Dec 14, 2020

3a4oT mentioned this pull request Dec 14, 2020

Swift Package Manager support #1465

Closed

5 tasks

facebook-github-bot reviewed Dec 15, 2020

View reviewed changes

3a4oT force-pushed the spmNumber10 branch from 1f53de8 to 20fb85f Compare March 5, 2021 11:39

3a4oT requested a review from facebook-github-bot March 5, 2021 15:48

lorixx mentioned this pull request Aug 14, 2021

SPM number10 (#1487) #1521

Closed

4 tasks

implemented spm layout generator script

afdc032

3a4oT force-pushed the spmNumber10 branch from 523d5d0 to afdc032 Compare August 19, 2021 05:49

3a4oT added 2 commits August 19, 2021 08:53

Xcode 12.5

6ebdee2

macOS 11 and Xcode 12.5.1 on CI

5be7110

added Carthage job for Xcframework and legacy Lipo

3a4oT force-pushed the spmNumber10 branch from eee5cd5 to 5be7110 Compare August 19, 2021 06:22

rollback to macOS 10.15 for UITests

b3e7040

3a4oT force-pushed the spmNumber10 branch from b5553fb to b3e7040 Compare August 19, 2021 06:44

Added CI job for raw "swift build"

f8f03e7

3a4oT force-pushed the spmNumber10 branch from 673535a to f8f03e7 Compare August 19, 2021 08:12

lorixx requested changes Aug 19, 2021

View reviewed changes

lorixx mentioned this pull request Aug 19, 2021

SPM number10 - fix the internal build #1529

Closed

removed IGListAdapterUpdaterCompatible.h, regenerated SPM layout

e1b223b

removed UIKit import from IGListDiffKit

c9c6482

cleanup Podfile

4f20762

added SPM-layout generator job to CI.

979eca9

facebook-github-bot closed this in a1036e0 Sep 1, 2021

facebook-github-bot added the Merged label Sep 1, 2021

3a4oT mentioned this pull request Sep 9, 2021

Added SPM support TextureGroup/Texture#1928

Open

3 tasks

lorixx mentioned this pull request Jan 14, 2022

Improve SwiftPM support #1546

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SPM number10 #1487

SPM number10 #1487

3a4oT commented Dec 14, 2020

3a4oT commented Dec 14, 2020

facebook-github-bot left a comment

3a4oT commented Dec 22, 2020

lorixx commented Jan 11, 2021

RamblinWreck77 commented Jan 21, 2021

3a4oT commented Feb 9, 2021

facebook-github-bot commented Mar 5, 2021

3a4oT commented Mar 5, 2021

3a4oT commented Mar 5, 2021

BrentMifsud commented Aug 10, 2021

lorixx commented Aug 16, 2021

lorixx commented Aug 18, 2021

lorixx commented Aug 18, 2021

3a4oT commented Aug 19, 2021 •

edited

Loading

lorixx commented Aug 19, 2021

facebook-github-bot commented Aug 19, 2021

lorixx left a comment

lorixx Aug 19, 2021

facebook-github-bot commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

3a4oT commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

lorixx commented Aug 25, 2021

3a4oT commented Aug 26, 2021

facebook-github-bot commented Sep 1, 2021

lorixx commented Sep 1, 2021 •

edited

Loading


		# 2. Commit Changes

		# Repeate those steps each time you delete/add project's files. Make sure to have this CI step which will check that `generate_spm_sources_layout.sh` is not broken.

SPM number10 #1487

SPM number10 #1487

Conversation

3a4oT commented Dec 14, 2020

Changes in this pull request

Generate SPM layout

Checklist

3a4oT commented Dec 14, 2020

facebook-github-bot left a comment

Choose a reason for hiding this comment

3a4oT commented Dec 22, 2020

lorixx commented Jan 11, 2021

RamblinWreck77 commented Jan 21, 2021

3a4oT commented Feb 9, 2021

facebook-github-bot commented Mar 5, 2021

3a4oT commented Mar 5, 2021

3a4oT commented Mar 5, 2021

BrentMifsud commented Aug 10, 2021

lorixx commented Aug 16, 2021

lorixx commented Aug 18, 2021

lorixx commented Aug 18, 2021

3a4oT commented Aug 19, 2021 • edited Loading

lorixx commented Aug 19, 2021

facebook-github-bot commented Aug 19, 2021

lorixx left a comment

Choose a reason for hiding this comment

lorixx Aug 19, 2021

Choose a reason for hiding this comment

facebook-github-bot commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

3a4oT commented Aug 23, 2021

facebook-github-bot commented Aug 23, 2021

lorixx commented Aug 25, 2021

3a4oT commented Aug 26, 2021

facebook-github-bot commented Sep 1, 2021

lorixx commented Sep 1, 2021 • edited Loading

3a4oT commented Aug 19, 2021 •

edited

Loading

lorixx commented Sep 1, 2021 •

edited

Loading