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.

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

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

Open
3 tasks done
dreampiggy opened this issue Dec 12, 2019 · 12 comments
Open
3 tasks done
Labels

Comments

@dreampiggy
Copy link

dreampiggy commented Dec 12, 2019

New issue checklist

  • I have reviewed the README and documentation
  • I have searched existing issues and this is not a duplicate
  • I have attempted to reproduce the issue and include an example project.

General information

  • IGListKit version: 4.0.0
  • iOS version(s): Any
  • CocoaPods/Carthage version: None
  • Xcode version: Xcode 11.2.1
  • Devices/Simulators affected: Any
  • Reproducible in the demo project? (Yes/No): Yes
  • Related issues:

Debug information

# Please include debug logs using the following lldb command:
po [IGListDebugger dump]

Just two screenshots, enough to say the issues...

image

image

@iperry90 iperry90 added the bug label Dec 12, 2019
@danqing
Copy link

danqing commented Dec 14, 2019

SPM isn't supported (#1368).

Any idea when it can be added? @lorixx :)

@dreampiggy
Copy link
Author

dreampiggy commented Dec 14, 2019

@danqing For SwiftPM on Objective-C, it's a pain because SwiftPM design assume using the raw C style for include public headers, however:

  • Objective-C developer used to use Object.h/Object.m in the same directory, not a standalone include directory for public headers. Tha's a suck for treating Objective-C the same as C.

If you want to supports SwiftPM, you can checkout what SDWebImage do, which have also two Target (one SDWebImage and another SDWebImageMapKit). Which use the custom header search path to achieve this, and also have to hide the Private Headers (I previouslly think facebook guys already solve this problem, but actually not)

@Bruce-pac
Copy link

@dreampiggy I note that SDWebImage use #import ".h" to import its file. And I try that way on IGListKit and write the Package.swift, it works. So Is that what you mean above you said?

@dreampiggy
Copy link
Author

@Bruce-pac Use both

  • Objective-C
@import IGListKit;
  • Swift
import IGListKit

cause the compile error showed in the first screenshot. You can have a check again at 4.0.0 version via SwiftPM.

@Bruce-pac
Copy link

Bruce-pac commented Dec 29, 2019

@dreampiggy yeah, a big reason of that is that the Package.swift manifest file is not right. I'm trying rewrite Package.swift file.
I note that SDWebImage use #import ".h" to import its file in its other file. such as this:

#import "SDAnimatedImage.h"
#import "NSImage+Compatibility.h"
#import "SDImageCoder.h"
#import "SDImageCodersManager.h"
#import "SDImageFrame.h"
#import "UIImage+MemoryCacheCost.h"
#import "SDImageAssetManager.h"

However, IGListKit use #import <A/B.h> to import its file in its other file.
The modified file like:

// swift-tools-version:5.1
import PackageDescription

let package = Package(name: "IGListKit",
                      platforms: [
                          .macOS(.v10_11),
                          .iOS(.v9),
                          .tvOS(.v9)
                      ],
                     products: [
//                          .library(name: "IGListKit", targets: ["IGListKit"]),
                          .library(name: "IGListDiffKit", targets: ["IGListDiffKit"])
                      ],
                     dependencies: [],
                     targets: [.target(name: "IGListDiffKit",
                               dependencies: [],
                               path: nil,
                               sources: nil,
                               publicHeadersPath: "Source/IGListDiffKit/Public",
                               cSettings: [
                               .headerSearchPath("Public"),
                               .headerSearchPath("Internal"),
                             ],
                               linkerSettings: [
                             .linkedLibrary("libc++"),
                             ]
                     ),
//                          .target(name: "IGListKit",
//                                  dependencies: ["IGListDiffKit"],
//                                  path: ".",
//                                  sources: ["Source/IGListKit"],
//                                  publicHeadersPath: "include/IGListKit",
//                                  cSettings: [
//                                    .headerSearchPath("Source/IGListKit"),
//                                    .headerSearchPath("Source/IGListKit/Internal"),
//                                    .headerSearchPath(".")
//                                ],
//                                  linkerSettings: [
//                                  .linkedLibrary("libc++"),
//                                  ]
//                                )
    ],
                     cxxLanguageStandard: .cxx11)

If I modify the import style to #import ".h" , IGListDiffKit can compile success, but the original import style #import <A/B.h> cause the compile error showed in the first screenshot.

So I guess this issue maybe has two solutions, one is like SDWebImage, other is like CocoaLumberjack.

@dreampiggy
Copy link
Author

dreampiggy commented Dec 29, 2019

If you use #import <A/B.h>. You must have public header search path, pointer to A's parent directory.
SwiftPM for C/ObjC, only support one public header directory, and does not have module umbrella header like CocoaPods/Carthage (it use a Umbrella Directory, see: https://clang.llvm.org/docs/Modules.html#umbrella-directory-declaration)

For example, move the source code's header, as this strcture

- Source <---- Public Header Directory
-- IGListKit
--- IGListAdapter.h

So the SwiftPM can search this with <IGListKit/IGListAdapter.h>

@claudiogomezGL
Copy link

Do you have an ETA on when this is gonna be released? @Bruce-pac @dreampiggy

@jafara
Copy link

jafara commented Sep 3, 2020

spm sucks, i stick with cocoapods

@lorixx
Copy link
Contributor

lorixx commented Sep 22, 2020

OMG I had exact issue that @Bruce-pac experienced before: #1368 (comment) this is what I am trying, maybe we cannot go around the Apple's setting to require "XXX.h" format instead of <Library/XXX>h> format here.

@dreampiggy interesting, I actually tried it out but I am not able to successfully build the project still.

@dreampiggy
Copy link
Author

dreampiggy commented Sep 22, 2020

I'm not the maintainer of IGListKit. I'm the maintainer of SDWebImge.

You can try to see how we use symbol link the header to keep the same import syntax of #import <SDWebImage/SDWebImage.h> across all these Package Managers:

  • CocoaPods (including static library/framework, use modular headers, 3 types)
  • Carthage (prebuilt binary framework)
  • SwiftPM

SDWebImage/SDWebImage#2987

Though I think these massive crazy Package Managers (first Carthage) may disappear after some years of adopting SwiftPM.

@3a4oT
Copy link
Contributor

3a4oT commented Oct 9, 2020

Some work in progress #1465

@3a4oT 3a4oT mentioned this issue Dec 14, 2020
4 tasks
@BrentMifsud
Copy link

any updates on this?

@lorixx lorixx mentioned this issue Aug 14, 2021
4 tasks
lorixx pushed a commit to lorixx/IGListKit that referenced this issue Aug 14, 2021
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 pushed a commit to lorixx/IGListKit that referenced this issue Aug 19, 2021
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
facebook-github-bot pushed a commit that referenced this issue Sep 1, 2021
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants