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

Can the ccache build instructions be simplified? #4215

Closed
johnf opened this issue Sep 11, 2024 · 5 comments
Closed

Can the ccache build instructions be simplified? #4215

johnf opened this issue Sep 11, 2024 · 5 comments

Comments

@johnf
Copy link
Contributor

johnf commented Sep 11, 2024

Description

https://reactnative.dev/docs/build-speed has detailed instructions for enabling ccace for xcode, I think this can be simplified

What is the problem?

The current instructions indicate the following code should be added to the Podfile

installer.pods_project.targets.each do |target|
      target.build_configurations.each do |config|
        # Using the un-qualified names means you can swap in different implementations, for example ccache
        config.build_settings["CC"] = "clang"
        config.build_settings["LD"] = "clang"
        config.build_settings["CXX"] = "clang++"
        config.build_settings["LDPLUSPLUS"] = "clang++"
      end
    end

How can we address it?

I think this can be replace by uncommenting the :ccache_enabled argument to react_native_post_install

e.g.

    react_native_post_install(
      installer,
      config[:reactNativePath],
      :mac_catalyst_enabled => false,
      :ccache_enabled => true # this is normally commented out
    )

Why is it important?

RN now supports caching and from a quick look at the code does a better job at it. A pod update even recommends enabling ccache_enabled. So the docs aren't in line with the tooling.

Who needs this?

ios developers that want build peed improvements

When should this happen (use version numbers if needed)?

I would suggest updating the docs for all versions of RN that support the ccache_enabled flag

Other

I'd be happy to create a PR for the changes if the above is correct.

@cortinico
Copy link
Contributor

I'll defer to @cipolleschi here

@cipolleschi
Copy link
Contributor

Hi @johnf, could you open a PR with the change?

@johnf
Copy link
Contributor Author

johnf commented Sep 12, 2024

@cipolleschi Happy to!
Quick question about versioning. This change was implemented in 0.74. Does that mean I need to create versions in website/version_docs using the existing /docs for 0.72 and 0.73 and then update the various config files to support more versions?

@cipolleschi
Copy link
Contributor

@johnf If the change has been introduced in 0.74, I think it's fine to just update website/version_docs for 0.74, 0.75 and the docs ones. We can leave the older versions as they are.

@johnf
Copy link
Contributor Author

johnf commented Sep 16, 2024

@cipolleschi Thanks. I got confused. I was looking at an old branch I had, and the 0.75 and -0.74 directories didn't exist.

PR ready at #4218

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

No branches or pull requests

3 participants