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

Don't add unnecessary headers build phase #118

Merged
merged 1 commit into from
Oct 31, 2017

Conversation

ryohey
Copy link
Collaborator

@ryohey ryohey commented Oct 31, 2017

I found that header files are unwantedly copied into the application package.
I checked that only the dynamicLibrary and the framework contained headers from the newly created project from the template.

type needs headers
application NO
framework YES
dynamicLibrary YES
staticLibrary copies by Copy Files build phase
bundle NO
unitTestBundle NO
uiTestBundle NO
appExtension NO
commandLineTool NO
watchApp NO
watch2App NO
watchExtension NO
watch2Extension NO
tvExtension NO
messagesApplication NO
messagesExtension NO
stickerPack NO
xpcService NO

@yonaskolb
Copy link
Owner

yonaskolb commented Oct 31, 2017

Thanks @ryohey!
@keith @pepibumur @toshi0383, does this check out to you?

addObject(headersBuildPhase)
buildPhases.append(headersBuildPhase.reference)
if target.type == .framework || target.type == .dynamicLibrary {
let headersBuildPhase = PBXHeadersBuildPhase(reference: generateUUID(PBXHeadersBuildPhase.self, target.name), files: getBuildFilesForPhase(.headers))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also only create this if getBuildFilesForPhase here returns a non-empty array?

Copy link
Owner

@yonaskolb yonaskolb Oct 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this check could be added though the same would apply to the sources and resources build phase. I had it like this originally incase people wanted to easily add files to these build phases in Xcode (that's assuming they are only generating once and then checking in the project file)
In either case it's not something we have to cover in this PR

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah makes sense, I guess for some of these it's just preference, since in our all swift project we'll never need this build phase.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think the empty ones could be removed

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been done in #149

@keith
Copy link
Collaborator

keith commented Oct 31, 2017

One thought, but looks good to me!

@yonaskolb yonaskolb merged commit ec576e3 into yonaskolb:master Oct 31, 2017
@ryohey ryohey deleted the fix-copy-headers branch October 31, 2017 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants