Skip to content

Conversation

@Titozzz
Copy link
Contributor

@Titozzz Titozzz commented Sep 7, 2019

Summary:

Let's detect if the user forgot to run pod install after installing a native library 😄

Test Plan:

yarn add some-library-with-ios-native-code

react-native run-ios will print an error message until you run pod install warning you that some-library-with-ios-native-code has not been found in the lockfile so it's probably not properly linked

@Titozzz
Copy link
Contributor Author

Titozzz commented Sep 7, 2019

FYI I directly wrote typescript even if the files around were js. Hope that's not an issue. I tried keeping the surface of the PR as thin as possible to make for easier code conflicts

@Titozzz
Copy link
Contributor Author

Titozzz commented Sep 7, 2019

maybe @orta you want to make sure I parse the podfile.lock correctly?

@Titozzz Titozzz changed the title Warn user if pod install was not run feat(ios): warn user if pod install was not run Sep 7, 2019
@orta
Copy link
Member

orta commented Sep 8, 2019

It's little bit janky - is there possibly already a proper yaml parser in the dependency tree? Sometimes the ruby yaml writer can be either key: "string" or key: string and you may end up with pod names which you think are "\"Pod+Name\"" and not "Pod+Name"


export default function readPodfileLock(podfileLockPath: string) {
logger.debug(`Reading ${podfileLockPath}`);
const podLockContent = fs.readFileSync(podfileLockPath, 'utf8');
Copy link
Member

Choose a reason for hiding this comment

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

this needs a try/catch with a nice error message on what happened

@Titozzz
Copy link
Contributor Author

Titozzz commented Sep 8, 2019

Thanks will do @orta

@Titozzz
Copy link
Contributor Author

Titozzz commented Sep 8, 2019

@orta I depend on "SPEC CHECKSUM" as the keys are easy to get from here, is that OK?

@thymikee
Copy link
Member

thymikee commented Sep 9, 2019

@Titozzz we've just merged the PR migrating platforms-ios to TS. Would you mind rebasing? :)

@orta
Copy link
Member

orta commented Sep 9, 2019

@Titozzz that should be fine, as you're mainly just looking for the existence 👍

@Titozzz
Copy link
Contributor Author

Titozzz commented Sep 9, 2019

Will do tomorrow

@thib92
Copy link
Contributor

thib92 commented Sep 12, 2019

Isn’t there a pod command or utility that can just make sure the pod lock is up to date with the Podfile?

@thymikee
Copy link
Member

thymikee commented Sep 12, 2019

Pushed some small adjustments. If we get more confidence in this feature, I think we should fail the build once we detect inconsistencies and make sure we're in a CocoaPods project (i.e. Podfile is present)

Screenshot 2019-09-13 at 00 45 32

Screenshot 2019-09-13 at 00 45 54

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Lovely!

@thymikee thymikee merged commit 926dcb7 into master Sep 12, 2019
@thymikee thymikee deleted the detect-pod-install branch September 12, 2019 22:45
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.

5 participants