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

Prevent Xcode warnings for missing files #282

Merged
merged 9 commits into from
Dec 3, 2017

Conversation

groue
Copy link
Owner

@groue groue commented Dec 1, 2017

Context

The default GRDB distribution generates Xcode warnings about missing files whenever one directly embeds GRDB.xcodeproj in another project. Those warnings have been notified in #275.

Those warnings are related to GRDBCustom, the flavor of GRDB that links to a custom SQLite build. The missing files allow the user to customize SQLite compile-time options, and are not present in the distribution. Building the GRDBCustom targets present in GRDB.xcodeproj generates those files if needed, but other GRDB targets do not: hence the warnings.

Solution

This PR aims at removing those warnings, as transparently as possible for the end-user:

GRDB targets that may be embedded in another project now come with a Run Script Phase that generates the missing files when the targets are built.

Unfortunately, it looks like Xcode does not notice that the files are generated, and the warnings remain. Only after the parent project has been closed, and opened again, can I see that warnings are gone. This is the best I could achieve so far.

Implementation details and caveats

The Run Script Phase invokes the preventXcodeWarningsForMissingFiles target in the Makefile. It generates the three missing files by copying an example file at their path if they are missing.

One of those files is special: SQLiteCustom/src/SQLiteLib-USER.xcconfig is located inside the swiftlyfalling/SQLiteLib submodule. This submodule supports custom SQLite builds. The warning is generated because the missing file is included in the SQLiteCustom/GRDB.xcconfig distribution file. Messing with git was not an option, so the script downloads the full submodule before generating the missing file.

A consequence is that building GRDB will always download the full SQLite sources that come with https://github.com/swiftlyfalling/SQLiteLib. That's an important caveat.

A limitation of the script is that it requires git, and at least one internet connection for the first download of the swiftlyfalling/SQLiteLib submodule. That's because I didn't write the script in a way that ignores any error (another commit in this PR could make it more forgiving).

@schveiguy, @james-rantmedia, @swiftlyfalling, I'm wondering what you all think about this. Don't hesitate updating your submodules in order to check the behavior of this PR on your own project.

As a reminder, there are other ways to get rid of those pesky warnings (here, and there).

@james-rant
Copy link

I think my only concern with this approach is the download of the sqlite sources. Do we know how big the download is? The reason I'm concerned is because we use the free tier of Bitrise for our CI, and are limited to 30 mins per build. If the download is relatively small, then it shouldn't cause any issue.

@groue
Copy link
Owner Author

groue commented Dec 1, 2017

@james-rantmedia: The SQLiteCustom/src submodule currently weights 72M. GRDB alone is 99M.

Your question makes me realize I may have badly expressed myself: the download of SQLiteCustom/src happens only once. Each GRDB build does check for the presence of SQLiteCustom/src, but only downloads it once. Network also is hit only once.

@schveiguy
Copy link
Contributor

Hm... I see why this is a difficult thing to do, since src is really supposed to contain the SQLite sources.

I was able to simply touch these files, and the regular build no longer has the warnings. But if you touched the file, and then wanted to use the Custom target, the git clone would have issues seeing the existing file already there.

Another possibility that I could think of:

  1. Change imports of that file to import another file, local to GRDB. This file will not be included in the git clone.
  2. If you build GRDB, have a script that simply touches the file if it doesn't exist (i.e. create a blank file).
  3. If you build the GRDBCustom target, it rewrites the file like this:
#include "src/SQLiteLib-USER.xcconfig"

What do you think? It's a bit odd that xcode doesn't get rid of the warnings when you satisfy them, but have to reopen the project.

@schveiguy
Copy link
Contributor

Each GRDB build does check for the presence of SQLiteCustom/src, but only downloads it once. Network also is hit only once.

I think this wouldn't matter for CI, since it would probably build from scratch each time (at least that's what I've seen)

@james-rant
Copy link

Yeah, that's right. Bitrise does a fresh clone each CI build, so it would download GRDB and dependent submodules each time.

@groue
Copy link
Owner Author

groue commented Dec 1, 2017

@schveiguy wrote:

I was able to simply touch these files, and the regular build no longer has the warnings. But if you touched the file, and then wanted to use the Custom target, the git clone would have issues seeing the existing file already there.

Yes.

Another possibility that I could think of:

  1. Change imports of that file to import another file, local to GRDB. This file will not be included in the git clone.
  2. If you build GRDB, have a script that simply touches the file if it doesn't exist (i.e. create a blank file).
  3. If you build the GRDBCustom target, it rewrites the file like this: #include "src/SQLiteLib-USER.xcconfig"

I guess this would work, but this would definitely not help maintenance.

It's a bit odd that xcode doesn't get rid of the warnings when you satisfy them, but have to reopen the project.

I have set the output files in the Run Script phases, hoping that this would trigger some inner Xcode guts, but it did not. ¯\_(ツ)_/¯ I don't know what else could be done.

About your CI concerns: we'll see how travis handles this PR. It should be OK, because the preventXcodeWarningsForMissingFiles Makefile target downloads the SQLite sources in the same way as the Makefile target that tests GRBDCustom.

@schveiguy
Copy link
Contributor

About your CI concerns

I think @james-rantmedia is concerned with his own CI testing, not GRDB's :) He probably does not build the Custom version, which means the entire downloading is a waste (doubly so, since xcode isn't even likely involved!)

A question on this: does the preventXcodeWarningsForMissingFiles target run when you build with make on a CI system? If not, then it shouldn't do the download, no?

@groue
Copy link
Owner Author

groue commented Dec 1, 2017

I think @james-rantmedia is concerned with his own CI testing, not GRDB's

Oops, you're of course right! @schveiguy, @james-rantmedia: you can look at the Makefile target in https://github.com/groue/GRDB.swift/pull/282/files.

I'm really sorry that this little topic turns out to be so involving, and so far-reaching. Neither I or @swiftlyfalling did realize that those warnings would eventually pop out with the release of Xcode 9, when GRDBCustom was introduced in #62.

Frankly, I'd rather split GRDB.xcodeproj into three distinct projects. This is because isolating the GRDBCustom targets in their own project would remove all warnings from the regular GRDB project, and fix this issue at the root, instead of working around it.

I'll of course deeply apologize to GRDBCustom users, and GRDBCipher users who don't use CocoaPods, because of the breaking change introduced by the renaming of the project. But this is a pill much more easier to swallow than the solutions we are exploring in this PR, which already looked overly complex to me, and now may even turn into a concern for users CI !? This is just begging for trouble.

GRDB is not Linux, I'm not Linus, and we can look at backward compatibility with a little grain of salt.

I'll wait a little bit for other good ideas from you, dear fellow developers, and then I'll close this PR and cut the Gordian Knot as explained above.

@james-rant
Copy link

It's a little selfish of me, but what you describe sounds great to me, as I have yet to fully start using GRDB -- it's at the very beginning of a new project for me, so I have no attachment to backwards compatibility.

But I must also say that I love the way you write! You have a great command of the English language :)

@groue
Copy link
Owner Author

groue commented Dec 1, 2017

It's a little selfish of me

That's OK. I'm not sure there are many many GRDB users who use GRDBCustom, or GRDBCipher without CocoaPods. We're talking about advanced usages and experienced users, here. A short and clear howto should make everybody happy.

But I must also say that I love the way you write! You have a great command of the English language :)

Thanks! I thought I was writing some kind of English-flavored French 😅

@schveiguy
Copy link
Contributor

@groue I have no further suggestions, and am fine with the gordian knot solution 😄

I'm not sure how it's structured, but as long as GRDBCustom and GRDBCipher still reuse all the code that is in GRDB, I would think it's a fine solution.

Sorry to start this headache for you, and thanks for the great support!

@groue groue merged commit 5e3c62c into master Dec 3, 2017
@groue groue deleted the preventXcodeWarningsForMissingFiles branch December 3, 2017 14:55
@groue
Copy link
Owner Author

groue commented Dec 3, 2017

All right, folks! GRDB v2.4.0 is out, with one project for each flavor, and no warnings. See the v2.4.0 release notes for more details.

Thanks @schveiguy for voicing out your concerns about those warnings. It has been a little headache, this is true, but we now have a saner setup 👍

@james-rant
Copy link

Thanks @groue! Just updated everything my end and it's all good. Warnings are definitely gone and I could undo the GRDBCustom stuff I did to suppress them initially.

Thanks again!

groue added a commit to sobri909/GRDB.swift that referenced this pull request Apr 20, 2018
... so that all GRDB flavors can use the new technique.
GRDB has three distinct projects since groue#282.
groue added a commit that referenced this pull request Jun 7, 2018
... so that all GRDB flavors can use the new technique.
GRDB has three distinct projects since #282.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants