Added SPM support #1928

3a4oT · 2020-10-06T23:27:10Z

implemented SPM layout generator swift scripts/generate_spm_sources_layout.swift which use symlinks technics.
introduced the AsyncDisplayKitIGListKit library which is an umbrella for IG+Texture. To deal with SPM restrictions I did some kind of hack:)) I've symlinked the spm/Sources/AsyncDisplayKit (generated by a script) and created the AsyncDisplayKitIGListKit folder. It should be available ONLY for Package.swift. (TODO: upstream changes to Instagram)
refactored #include by dropping AsyncDisplayKit/
added a small tweak to be able to assemble for macCatalyst via SPM
Xcode 12 required (SPM only)
added sh build.sh build_listkit_xcode_spm_integration to the build pipeline which can verify Xcode's SPM integration.
added CI steps to check SPM based BUILDS for iOS, tvOS, macCatalyst.

experiment with IGListKit support and try to upstream changes to SPM number10 Instagram/IGListKit#1487
Update PINRemoteImage dependency and point it to official repository instead of fork
Figure out how to add the possibility for testing this integration.

CLAassistant · 2020-10-06T23:27:16Z

All committers have signed the CLA.

3a4oT · 2020-10-06T23:34:03Z

Source/ASDisplayNode.mm

    // CAEAGLLayer
    if([[view.layer class] isSubclassOfClass:[CAEAGLLayer class]]){
      _flags.canClearContentsOfLayer = NO;
    }
+#endif
  }


enable macCatalyst? :)

3a4oT · 2020-10-07T14:51:33Z

Still need to figure out how to test it

xezero · 2020-10-13T00:28:56Z

Source/Base/ASAvailability.h

+        #define AS_PIN_REMOTE_IMAGE 1
+    #endif
+
+    #ifndef AS_IG_LIST_KIT


Was this intended? My project doesn't use IGListKit but does use ASPINRemoteImageDownloader which requires AS_PIN_REMOTE_IMAGE to be set.

Also, I can't seem to be able to reference ASPINRemoteImageDownloader for some reason. I had to comment out the #if AS_PIN_REMOTE_IMAGE conditionals entirely in order for my project to even see the class. Not sure if it's an SPM package configuration issue or something else.

freemansion · 2020-11-16T13:40:14Z

any updates on this PR?

3a4oT · 2020-11-17T19:36:06Z

last commits should repair Xcode's SPM integration!

freemansion · 2020-11-18T06:32:17Z

Confirming, it compiles and works fine in my project. Let's get merge it.

NoliNik · 2020-11-18T08:31:52Z

Great job guys! Working for me!

xezero · 2020-11-21T01:37:20Z

For some reason I still can't access ASPINRemoteImageDownloader despite AS_PIN_REMOTE_IMAGE being defined in Package.swift. Is there something I'm missing?

MussaCharles · 2020-11-23T04:22:50Z

Hi, @3a4oT I tried using AsyncDisplayKitIGListKit from your fork on Xcode 12.2.
Unfortunately I am getting the following errors

on IGListKitUpdatingDelegate.h file: -

IGListUpdatingDelegate' has different definitions in different modules; first difference is definition in module 'IGListKit' found method 'performUpdateWithCollectionViewBlock:fromObjects:toObjectsBlock:animated:objectTransitionBlock:completion:' with 1st parameter of type 'IGListCollectionViewBlock _Nonnull __strong' (aka 'UICollectionView * _Nullable (^__strong)(void)')

Also similar error on IGListAdapter.h file

based on the error messages it seems like there is a namespace conflicts (I searched related issues, where the work around is renaming one of the module classname/function etc.. and this issue happens in Xcode 12, even though things worked well on Xcode 11.
Are you also using Xcode 12.2 on your current working version?

3a4oT · 2020-12-02T00:38:17Z

@MussaCharles thanks for the detailed feedback. Should be OK now. I've added the sample project (examples/ASIGListKitSPM/) which should verify that it won't break again. Would you mind giving it another shot?

@xezero PINRemoteImage should be available now :) so you can access ASPINRemoteImageDownloader.

xezero · 2020-12-02T02:57:02Z

@3a4oT Wow, great work on figuring those headers out! Looks intense 😅

MussaCharles · 2020-12-02T03:13:57Z

@MussaCharles thanks for the detailed feedback. Should be OK now. I've added the sample project (examples/ASIGListKitSPM/) which should verify that it won't break again. Would you mind giving it another shot?

@xezero PINRemoteImage should be available now :) so you can access ASPINRemoteImageDownloader.

@3a4oT Thanks for the updates, I will give it another shot later today or tomorrow and get back to you.

iosg1sw · 2020-12-11T15:21:44Z

Hi everyone! Is there any updates? I noticed that only Carthage build is not successful. This may be caused by that issue. Please check it out!

3a4oT · 2020-12-13T20:57:22Z

@MussaCharles Today I've integrated into another codebase which was using Carthage. I faced the same errors as you reported, to resolve issues make sure you delete any cached pods or Carthage artifacts (.framework) from your project. settings.

MussaCharles · 2020-12-14T02:29:55Z

@MussaCharles Today I've integrated into another codebase which was using Carthage. I faced the same errors as you reported, to resolve issues make sure you delete any cached pods or Carthage artifacts (.framework) from your project. settings.

Hi, @3a4oT Okay, I will double check to see if it was caused by the same files.
Also Sorry for the late. I was a bit occupied with work so I didn't get some time to test your last changes. I will have a look again soon and get back to you.
Happy Coding!!

that-scalcucci-guy · 2021-02-17T17:12:21Z

Just wondering what the status is on this currently. I've had to do quite a few of these integrations, would be more than happy to jump in.

garrettmoon · 2021-03-11T18:46:24Z

Thank you for putting in all this work!

I would like to avoid using a fork if possible on IGListKit… And since SPM currently doesn’t support running scripts there’s probably no way to add the spm directory to .gitignore is there? I wonder if we could use https://github.com/apple/swift-evolution/blob/main/proposals/0303-swiftpm-extensible-build-tools.md ?

stareque-atlassian · 2021-03-16T10:18:26Z

Cartfile

@@ -1,2 +1,2 @@
-github "pinterest/PINRemoteImage" "3.0.0-beta.14"
-github "pinterest/PINCache" "3.0.1-beta.7"
+github "pinterest/PINRemoteImage" "3.0.1"


PINRemoteImage needs to be updated to point to master (or atleast wait for next release), to include PINRemoteImage bugfixes related to SPM. And this should also transitively updated PINCache and PINOperation

rogerluan · 2021-06-10T22:45:29Z

Any update to this? 🙏

grangej · 2021-06-16T14:54:37Z

Updates please?!

benniebotha · 2021-08-04T00:12:56Z

Package.swift

+
+let sharedDefines: [CSetting] = [
+                                // Disable "old" textnode by default for SPM
+                                .define("AS_ENABLE_TEXTNODE", to: "0"),


Why is this disabled?
It seems to be enabled in ASAvailability.h does this not cause a mismatch between the Project version and the SPM version?

Thanks for mention that, I guess the main reason was - I need that to be a default since ASTextNode2 is a future I just enabled it by default :)

Suggested change

.define("AS_ENABLE_TEXTNODE", to: "0"),

.define("AS_ENABLE_TEXTNODE", to: "1"),

This option leads undefined symbol: _OBJC_CLASS_$_ ASTEXTNODE2- error when using ASTextNode2. If you can check, or if someone else meets this error, try changing it from 0 to 1.

This option makes usage of ASTextNode2 a default. So in your code, you should reference it just like ASTextNode

3a4oT · 2021-09-09T20:54:20Z

Just an small update:

So I was manage to upstream my SPM changes to Instagram. (SPM number10 Instagram/IGListKit#1487). This mean that technically we can drop my fork from the dependencies. However, this may be tricky since IGListKit on current master doesn't work well with Texture. More investigation required.
I'm waiting until SE-0303 will be available (probably Swift 5.6) to continue my work and implement code generation as a plugin.

annomusa · 2022-01-31T05:36:07Z

I'm waiting until SE-0303 will be available (probably Swift 5.6) to continue my work and implement code generation as a plugin.

Hai @3a4oT it's available now in Xcode 13.3 beta, perhaps you can continue this MR if you're available, thanks in advance 🙏

dehrom · 2022-03-11T10:21:55Z

Any updates please? 😥

3a4oT · 2022-03-11T10:26:44Z

As you may know, my country (Ukraine) was attacked by russian invaders, I'm on military service now and don't have time at the moment to support this fork. Invaders Must Die!

nickaroot · 2022-03-11T10:35:09Z

Please, don't mess up P. and russians...

So, on my fork SPM is working, if you need that

Package.swift example:
https://github.com/nickaroot/TextureUI/blob/master/Package.swift

bejeri4 · 2022-03-16T15:57:16Z

Слава Украине! Героям слава! 🇺🇦

vadimvitvickiy · 2022-04-06T09:55:40Z

Source/Private/ASIGListAdapterBasedDataSource.mm

-    [(IGListAdapterUpdater *)updater setAllowsBackgroundReloading:NO];
+   // [(IGListAdapterUpdater *)updater setAllowsBackgroundReloading:NO];


Shouldn't be commented, bug is still relevant, dataSource won't update properly using Texture+IGListKit when self.setAllowsBackgroundReloading == true, collectionView.window == nil

seems like no longer valid on the latest IGListKit

muukii · 2022-06-30T17:37:10Z

What other tasks do we have to accomplish this PR? I might help

wweevv-johndpope · 2022-08-29T05:22:01Z

please merge - need SPM integration urgently.

muukii · 2022-08-31T13:03:09Z

and I found a situation that break-points are not working when installed as swift-package.
I filed it on FB11418698

3a4oT · 2023-08-05T15:12:10Z

Hey all. No commitments, but I did some work this morning to synchronize the latest IGListKit and Texture. For now, it will live under my fork branch - 3a4oT#1. In a time of my absence, Xcode 14.3 arrives, which add one more challenge to make it work. We need to land new versions of PINOperation, PINRemoteImage, PINCache to be compatible with Xcode 14.3. Also, we need to make Instagram finally tag a new release on GitHub. For PIN-related repositories I saw that community started to submit PRs, but it seems like there are no active maintainers. For Instagram related task, I started a conversation . If you want to contribute and don't know how, please leave a comment/issue on those repositories so you may be heard.

rcancro · 2023-09-26T17:09:49Z

Apologies that this has sat for so long. I have a couple questions about this PR.

Why did you change all of the imports? Is this required for Package.swift to work? If it isn't required, splitting up these diffs would make it much easier for us to move forward with landing.

Is the "hack" for AsyncDisplayKitIGListKit still required? I see that IGListKit has a Package.swift file. Is it possible to use this instead?

brentleyjones · 2024-05-31T16:46:51Z

The imports change is required for SPM to work, yes.

takasurazeem · 2024-09-30T04:21:56Z

It this cannot be merged, can we please close this PR?

3a4oT mentioned this pull request Oct 6, 2020

SPM define pinterest/PINRemoteImage#574

Merged

3a4oT commented Oct 6, 2020

View reviewed changes

3a4oT marked this pull request as draft October 7, 2020 14:50

xezero reviewed Oct 13, 2020

View reviewed changes

3a4oT force-pushed the spm branch from af2fe8c to 41c03dd Compare December 2, 2020 15:54

3a4oT added 10 commits March 9, 2021 14:12

updated Cartfile and .gitignore

69424b7

initial work related to SPM. Created symbolic links for public headers.

e6bec65

IGListKit imports

4cf93ea

added AsyncDisplayKitIGListKit target.

749f39a

more experiments.

7432529

fixed missed dependencies.

c31845d

fixed spm Availability.

62d2496

excluded spm's "include" folder for podspec.

3cf5efe

fixed import

1bd56fb

added comment

f7c13f7

3a4oT added 2 commits March 10, 2021 18:30

implemented SPM layout generator

6fd4065

set Xcode 12.4

094d983

3a4oT force-pushed the spm branch from 41c03dd to 094d983 Compare March 10, 2021 16:36

3a4oT added 5 commits March 10, 2021 19:56

fixed generator

f5f430c

finished generator

8bacfd3

added to CI

8ddb565

build AsyncDisplayKitIGListKit.

1c4074f

try to generate layout

c480761

3a4oT marked this pull request as ready for review March 11, 2021 12:09

stareque-atlassian reviewed Mar 16, 2021

View reviewed changes

fixed broken smbolic links.

f06cc18

benniebotha reviewed Aug 4, 2021

View reviewed changes

vadimvitvickiy reviewed Apr 6, 2022

View reviewed changes

muukii mentioned this pull request Jan 5, 2023

Create Package.swift #2070

Closed

	.define("AS_ENABLE_TEXTNODE", to: "0"),
	.define("AS_ENABLE_TEXTNODE", to: "1"),

		[(IGListAdapterUpdater *)updater setAllowsBackgroundReloading:NO];
		// [(IGListAdapterUpdater *)updater setAllowsBackgroundReloading:NO];

Added SPM support #1928

Are you sure you want to change the base?

Added SPM support #1928

Conversation

3a4oT commented Oct 6, 2020 • edited Loading

CLAassistant commented Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

3a4oT commented Oct 7, 2020

Choose a reason for hiding this comment

xezero Oct 13, 2020 • edited Loading

Choose a reason for hiding this comment

freemansion commented Nov 16, 2020

3a4oT commented Nov 17, 2020

freemansion commented Nov 18, 2020

NoliNik commented Nov 18, 2020

xezero commented Nov 21, 2020

MussaCharles commented Nov 23, 2020 • edited Loading

3a4oT commented Dec 2, 2020

xezero commented Dec 2, 2020

MussaCharles commented Dec 2, 2020

iosg1sw commented Dec 11, 2020

3a4oT commented Dec 13, 2020

MussaCharles commented Dec 14, 2020

that-scalcucci-guy commented Feb 17, 2021

garrettmoon commented Mar 11, 2021

stareque-atlassian Mar 16, 2021 • edited Loading

Choose a reason for hiding this comment

rogerluan commented Jun 10, 2021 • edited Loading

grangej commented Jun 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

3a4oT commented Sep 9, 2021

annomusa commented Jan 31, 2022

dehrom commented Mar 11, 2022

3a4oT commented Mar 11, 2022

nickaroot commented Mar 11, 2022 • edited Loading

bejeri4 commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muukii commented Jun 30, 2022

wweevv-johndpope commented Aug 29, 2022

muukii commented Aug 31, 2022

3a4oT commented Aug 5, 2023 • edited Loading

rcancro commented Sep 26, 2023

brentleyjones commented May 31, 2024

takasurazeem commented Sep 30, 2024

3a4oT commented Oct 6, 2020 •

edited

Loading

CLAassistant commented Oct 6, 2020 •

edited

Loading

xezero Oct 13, 2020 •

edited

Loading

MussaCharles commented Nov 23, 2020 •

edited

Loading

stareque-atlassian Mar 16, 2021 •

edited

Loading

rogerluan commented Jun 10, 2021 •

edited

Loading

nickaroot commented Mar 11, 2022 •

edited

Loading

3a4oT commented Aug 5, 2023 •

edited

Loading