Skip to content

feat: run pod repo update when pod install failed #1061

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 19, 2020
Merged

feat: run pod repo update when pod install failed #1061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
61e9ae4
feat: update repo
grabbou Mar 18, 2020
b954c39
chore: update
grabbou Mar 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/cli/src/commands/init/init.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ async function installDependencies({
});

if (process.platform === 'darwin') {
await installPods({projectName, loader, shouldUpdatePods: false});
await installPods({projectName, loader});
}

loader.succeed();
Expand Down
13 changes: 3 additions & 10 deletions packages/cli/src/commands/upgrade/upgrade.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,7 @@ const installDeps = async (root: string, newVersion: string) => {
}
};

const installCocoaPodsDeps = async (
projectDir: string,
thirdPartyIOSDeps: Array<Config['dependencies'][string]>,
) => {
const installCocoaPodsDeps = async (projectDir: string) => {
if (process.platform === 'darwin') {
try {
logger.info(
Expand All @@ -207,7 +204,6 @@ const installCocoaPodsDeps = async (
);
await installPods({
projectName: projectDir.split('/').pop() || '',
shouldUpdatePods: thirdPartyIOSDeps.length > 0,
});
} catch (error) {
if (error.stderr) {
Expand Down Expand Up @@ -323,9 +319,6 @@ async function upgrade(argv: Array<string>, ctx: Config) {
projectDir,
'node_modules/react-native/package.json',
));
const thirdPartyIOSDeps = Object.values(ctx.dependencies).filter(
dependency => dependency.platforms.ios,
);

const newVersion = await getVersionToUpgradeTo(
argv,
Expand All @@ -346,7 +339,7 @@ async function upgrade(argv: Array<string>, ctx: Config) {
if (patch === '') {
logger.info('Diff has no changes to apply, proceeding further');
await installDeps(projectDir, newVersion);
await installCocoaPodsDeps(projectDir, thirdPartyIOSDeps);
await installCocoaPodsDeps(projectDir);

logger.success(
`Upgraded React Native to v${newVersion} 🎉. Now you can review and commit the changes`,
Expand Down Expand Up @@ -382,7 +375,7 @@ async function upgrade(argv: Array<string>, ctx: Config) {
}
} else {
await installDeps(projectDir, newVersion);
await installCocoaPodsDeps(projectDir, thirdPartyIOSDeps);
await installCocoaPodsDeps(projectDir);
logger.info('Running "git status" to check what changed...');
await execa('git', ['status'], {stdio: 'inherit'});
}
Expand Down
63 changes: 39 additions & 24 deletions packages/cli/src/tools/installPods.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,44 @@ type PromptCocoaPodsInstallation = {
promptQuestion: string;
};

async function updatePods(loader: ora.Ora) {
async function runPodInstall(
loader: ora.Ora,
projectName: string,
shouldHandleRepoUpdate: boolean = true,
) {
try {
loader.start(
`Installing CocoaPods dependencies ${chalk.dim(
'(this may take a few minutes)',
)}`,
);
await execa('pod', ['install']);
} catch (error) {
// "pod" command outputs errors to stdout (at least some of them)
const stderr = error.stderr || error.stdout;

/**
* If CocoaPods failed due to repo being out of date, it will
* include the update command in the error message.
*
* `shouldHandleRepoUpdate` will be set to `false` to
* prevent infinite loop (unlikely scenario)
*/
if (stderr.includes('pod repo update') && shouldHandleRepoUpdate) {
await runPodUpdate(loader);
await runPodInstall(loader, projectName, false);
} else {
loader.fail();
throw new Error(
Copy link
Member

@thymikee thymikee Mar 19, 2020

Choose a reason for hiding this comment

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

Let's use CLIError and pass the original error so that issues like this one are easier to deal with. Would be cool to do the same for runPodUpdate function (line 68)

Copy link
Member Author

@grabbou grabbou Mar 19, 2020

Choose a reason for hiding this comment

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

Was just copying and pasting the existing code and actually started thinking why we no longer use CLIError.

Copy link
Member Author

@grabbou grabbou Mar 19, 2020

Choose a reason for hiding this comment

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

Turns out this will take more than should be in scope of this PR and I guess we should track it separately.

CLIError will inline the entire message into a single line + we already logger.error here (in other CLI commands, we don't do that as we have global try-catch).

I would be a bit nervous about changing it here due to potential side effects. What you think of setting up a follow up issue?

Copy link
Member

@thymikee thymikee Mar 19, 2020

Choose a reason for hiding this comment

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

I'm good with a followup

`Failed to install CocoaPods dependencies for iOS project, which is required by this template.\nPlease try again manually: "cd ./${projectName}/ios && pod install".\nCocoaPods documentation: ${chalk.dim.underline(
'https://cocoapods.org/',
)}`,
);
}
}
}

async function runPodUpdate(loader: ora.Ora) {
try {
loader.start(
`Updating CocoaPods repositories ${chalk.dim(
Expand Down Expand Up @@ -127,11 +164,9 @@ async function installCocoaPods(loader: ora.Ora) {
async function installPods({
projectName,
loader,
shouldUpdatePods,
}: {
projectName: string;
loader?: ora.Ora;
shouldUpdatePods?: boolean;
}) {
loader = loader || new NoopLoader();
try {
Expand All @@ -157,27 +192,7 @@ async function installPods({
await installCocoaPods(loader);
}

if (shouldUpdatePods) {
await updatePods(loader);
}

try {
loader.start(
`Installing CocoaPods dependencies ${chalk.dim(
'(this may take a few minutes)',
)}`,
);
await execa('pod', ['install']);
} catch (error) {
// "pod" command outputs errors to stdout (at least some of them)
logger.log(error.stderr || error.stdout);

throw new Error(
`Failed to install CocoaPods dependencies for iOS project, which is required by this template.\nPlease try again manually: "cd ./${projectName}/ios && pod install".\nCocoaPods documentation: ${chalk.dim.underline(
'https://cocoapods.org/',
)}`,
);
}
await runPodInstall(loader, projectName);
} catch (error) {
throw error;
} finally {
Expand Down