Getting notAFile error when trying to add PBXFileReference to a dynamic framework

[fuzza]

Context 🕵️‍♀️

I'm writing a script that integrates Carthage dynamic frameworks to XCode project using xcproj

What 🌱

In order to create PBXFileReference to dynamic framework file, I'm trying to use the following function from PBXProjObjects+Helpers.swift:

public func addFile(
        at filePath: Path,
        toGroup: PBXGroup,
        sourceTree: PBXSourceTree = .group,
        sourceRoot: Path) throws -> ObjectReference<PBXFileReference>

In case of dynamic framework, it throws an XCodeProjEditingError.notAFile(path: filePath) exception regardless if file exsits or not.

After some investigation, I found that filePath parameter is validated using PathKit's isFile var which returns false if the path points to a directory.

Since all dynamic frameworks are directories from file system PoV, it produces the issue.
I assume that adding files of some other types, such as xcassets, pbxproj etc. could be affected as well.

Proposal 🎉

The simplest fix for the issue is to change isFile check to exists.

More complex one is to explicitly mark PBXFileReference.fileTypeHash with file or directory attribute (e.g using enum) and choose appropriate validation method.

I would appreciate any feedback on the issue and let me know if you want me to tackle this issue.

[welcome]

Thanks for opening your first issue here! Be sure to follow the issue template!

[pepicrft]

Hey @fuzza! Very good catch. I'd say just check if the file exists before adding it. I think it should be enough but you can ask @ilyapuchka to get more context on why he used isFile. Feel free to tackle the issue an open a PR that we can merge.

[ilyapuchka]

Yes, I guess to do that isFile is too strict, I didn't think about frameworks when implementing those helper methods as they are for adding source files. So it should be changed to just existence check t work with bundles like frameworks or even maybe image sets.

[jcampbell05]

:) In Xcake we had to handle special handling for bundles i.e framework, settings, xcassets etc Basically we just had a list of extensions we know to be bundles and treat them as if they are files.

[yonaskolb]

@jcampbell05 that layer is handled by XcodeGen, which does something similar.
In this case .exists will be enough as users are explicitly passing the path they want to add 👍

[pepicrft]

Agree with @yonaskolb. From xcproj we expect the consumer of the library, like XcodeGen, to handle the file types accordingly.

[fuzza]

@pepibumur, @yonaskolb, @ilyapuchka
Thanks for a quick feedback guys, I'll submit a PR with a fix soon.