Skip to content

[Android] Add .cxx directories to app template .gitignore - #161069

Merged
auto-submit[bot] merged 5 commits into
flutter:masterfrom
gmackall:gitignore_cxx_dirs
Jan 7, 2025
Merged

auto-submit[bot] merged 5 commits into
flutter:masterfrom
gmackall:gitignore_cxx_dirs

Conversation

@gmackall

@gmackall gmackall commented Jan 2, 2025 •

Copy link
Copy Markdown
Member

Fixes (but I still need to write the draft of the blog section so don't autoclose github 🙏 ) #160372.

-Module templates already exclude the entire .android directory.
-Plugin templates already exclude .cxx.

Modified the existing gitignores with

nl=$'\n'
sed -i'' -e '/GeneratedPluginRegistrant.java/a\'$'\n''.cxx/'"\\${nl}"'' $(find . -path \*android/.gitignore)

(which was probably not the best way)

Fun fact: apparently the .gitignore files (including the ones we copy to the template apps) aren't indexed for search (the .gitignore.tmpl are, but not the ones that we copy exactly such as this one).

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions Bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jan 2, 2025
@gmackall
gmackall marked this pull request as ready for review January 2, 2025 22:15
@github-actions github-actions Bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. f: integration_test The flutter/packages/integration_test plugin labels Jan 2, 2025
@gmackall
gmackall requested review from a team and gnprice January 2, 2025 22:16
@gmackall
gmackall force-pushed the gitignore_cxx_dirs branch from 02301b0 to b51964f Compare January 2, 2025 22:29

@gnprice gnprice left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

One nit below. I also did a quick search for existing gitignore files this should perhaps be added to:
$ git ls-files | grep android/.gitignore
Other than the ones already in this PR, that finds:

engine/src/flutter/shell/platform/android/.gitignore
engine/src/flutter/testing/scenario_app/android/.gitignore
packages/integration_test/android/.gitignore

I think the second one is a true match that should get this change. The third I'm not sure about. The first is a false positive.

Oh and this search for relevant templates:

$ git ls-files packages/flutter_tools/ | grep android.*gitignore
packages/flutter_tools/templates/app/android.tmpl/.gitignore
packages/flutter_tools/templates/plugin/android.tmpl/.gitignore
packages/flutter_tools/templates/plugin_shared/android.tmpl/.gitignore

The app/ one is included here, and plugin_shared/ already has this line. I don't understand the difference between plugin/ and plugin_shared/ — should plugin/ get this too?

Comment thread packages/flutter_tools/templates/app/android.tmpl/.gitignore Outdated
@gmackall

gmackall commented Jan 6, 2025

Copy link
Copy Markdown
Member Author

Thanks!

One nit below. I also did a quick search for existing gitignore files this should perhaps be added to: $ git ls-files | grep android/.gitignore Other than the ones already in this PR, that finds:

engine/src/flutter/shell/platform/android/.gitignore
engine/src/flutter/testing/scenario_app/android/.gitignore
packages/integration_test/android/.gitignore

I think the second one is a true match that should get this change. The third I'm not sure about. The first is a false positive.

Oh and this search for relevant templates:

$ git ls-files packages/flutter_tools/ | grep android.*gitignore
packages/flutter_tools/templates/app/android.tmpl/.gitignore
packages/flutter_tools/templates/plugin/android.tmpl/.gitignore
packages/flutter_tools/templates/plugin_shared/android.tmpl/.gitignore

scenario_app actually doesn't use the flutter tool, so I don't think it should be impacted here by the change to force ndk download (it's days are numbered regardless - it was a way to test behavior in something like a full flutter app, and essentially replicated whatever framework/tool functionality was needed for the test).

packages/integration_test/android/.gitignore - I added it here as well. I don't know that it will matter (I haven't seen the cxx directories being created by my change for plugin directories), but our plugin templates include it in the gitignore so I think it is fine to add it anyways.

The app/ one is included here, and plugin_shared/ already has this line. I don't understand the difference between plugin/ and plugin_shared/ — should plugin/ get this too?

Also added to the plugin/ template. My understanding is that generally when both a foo and foo_shared directory exist, that the output of the template is a function of both of the directories. But I also know that the files that don't end in tmpl get copied as is, so I'm not sure how that is possible here (perhaps one of the .gitignore files just wins?) Either way, I've checked the output of the template manually and verified that the .gitignore contains the .cxx/ line we want (both before and after adding to the plugin/ file).

@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 6, 2025

@gnprice gnprice left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good. Thanks!

@gmackall gmackall added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Jan 7, 2025
@auto-submit
auto-submit Bot added this pull request to the merge queue Jan 7, 2025
Merged via the queue into flutter:master with commit f3d2204 Jan 7, 2025
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
githubmonkey added a commit to githubmonkey/flutter_shaders that referenced this pull request Mar 25, 2025
BriceCroix added a commit to MovnaTeam/movna that referenced this pull request May 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests f: integration_test The flutter/packages/integration_test plugin framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants