Skip to content

fix: Search gitignored directories for config files - #12791

Closed
rmburg wants to merge 1 commit into
tauri-apps:devfrom
rmburg:fix-config-detection
Closed

fix: Search gitignored directories for config files#12791
rmburg wants to merge 1 commit into
tauri-apps:devfrom
rmburg:fix-config-detection

Conversation

@rmburg

@rmburg rmburg commented Feb 23, 2025

Copy link
Copy Markdown
Contributor

Fixes #3527

This PR changes the lookup function to search gitignored directories as well.
lookup is used to find the tauri config and package.json files.

#3527 was closed without being fixed: Tauri v2 currently can't find its config files if they are gitignored, as is often the case for people who track their dotfiles with a git repo in their home directory.

@FabianLars

Copy link
Copy Markdown
Member

hmm, this wasn't really a "bug". It was added intentionally because for many users the cli took ages to find the config. As a compromise we added a manual check for ./src-tauri/tauri.conf.json before the gitignore aware lookup but it seems like that doesn't work anymore?

Also, this change is a bit extra extreme as it even disables tauri's own gitignore file...

i don't mind somehow improving the lookup but i personally would not merge a PR that heavily degrades dx for most of our users for the 10 people that gitignored their whole system.

@rmburg

rmburg commented Feb 23, 2025

Copy link
Copy Markdown
Contributor Author

Fixing the manual check would work for me as well.

I'm left with two things I don't understand:

  • Why does tauri respect .gitignore files at all when searching for config files?
    This doesn't seem to me like something we actually want.
    If it is for performance reasons, e.g. to avoid searching in the target or node_modules directory,
    we could just exclude these manually.
  • How is the DX negatively affected by the changes in this PR?

@FabianLars

Copy link
Copy Markdown
Member

Why does tauri respect .gitignore files at all when searching for config files?
This doesn't seem to me like something we actually want.

performance reasons

If it is for performance reasons, e.g. to avoid searching in the target or node_modules directory,
we could just exclude these manually.

first of all, that's what i meant with tauri's own gitignore file, secondly it's a bit hard to predict what kind of folders have to be gitignored (with the tauri dir potentially being in a dir with an unfortunate name). i'm not totally against this approach though.
Maybe we could also simply use https://docs.rs/ignore/latest/ignore/struct.WalkBuilder.html#method.parents (we already use git_global to ignore global gitignore files) and see if that's good enough.

How is the DX negatively affected by the changes in this PR?

If tauri dev takes 10 seconds to start i consider this bad dx. again, the gitignore aware lookup was added intentionally to fix exactly this.

@rmburg

rmburg commented Feb 23, 2025

Copy link
Copy Markdown
Contributor Author

If tauri dev takes 10 seconds to start i consider this bad dx. again, the gitignore aware lookup was added intentionally to fix exactly this.

Of course. I just measured the runtime of the call to lookup in an invocation of cargo tauri dev (with my changes, in a medium-sized tauri repo), and it came out to just over 4ms. For something that usually runs once per dev session, I find this to be completely acceptable.

@rmburg

rmburg commented Feb 23, 2025

Copy link
Copy Markdown
Contributor Author

If you want to test this yourself, you can git apply this diff to use the setup I used to get to that number.

Diff, hidden for readability

diff --git a/crates/tauri-cli/src/helpers/app_paths.rs b/crates/tauri-cli/src/helpers/app_paths.rs
index b20b991ef..3d209f9dd 100644
--- a/crates/tauri-cli/src/helpers/app_paths.rs
+++ b/crates/tauri-cli/src/helpers/app_paths.rs
@@ -8,6 +8,7 @@ use std::{
   ffi::OsStr,
   path::{Path, PathBuf},
   sync::OnceLock,
+  time::Instant,
 };
 
 use ignore::WalkBuilder;
@@ -101,7 +102,8 @@ pub fn resolve_tauri_dir() -> Option<PathBuf> {
     return Some(src_dir);
   }
 
-  lookup(&src_dir, |path| {
+  let earlier = Instant::now();
+  let result = lookup(&src_dir, |path| {
     folder_has_configuration_file(Target::Linux, path) || is_configuration_file(Target::Linux, path)
   })
   .map(|p| {
@@ -110,7 +112,13 @@ pub fn resolve_tauri_dir() -> Option<PathBuf> {
     } else {
       p.parent().unwrap().to_path_buf()
     }
-  })
+  });
+
+  let duration = Instant::now().duration_since(earlier);
+
+  dbg!(duration);
+
+  result
 }
 
 pub fn resolve() {

@rmburg

rmburg commented Mar 1, 2025

Copy link
Copy Markdown
Contributor Author

the gitignore aware lookup was added intentionally to fix exactly this.

Was there really a concrete issue with performance before this was added?
As stated before, I've measured the performance impact and found it to be minimal.
However, with a sample size of one, I can't be sure this wasn't an issue on other setups.
To me this feels like a micro-optimization without a clear benefit, to the detriment of DX.

@FabianLars

Copy link
Copy Markdown
Member

yes, the 10 seconds i mentioned weren't an exaggeration, a few people actually faced delays that long (including me when i tested their projects).
Now that was years ago so who knows if that's still reproducible now, maybe there were other changes that prevents this issue nowadays.

@rmburg

rmburg commented Mar 1, 2025

Copy link
Copy Markdown
Contributor Author

I see, thanks for the insight. How do you suggest we move forward now?

Even assuming that searching for the config file in the wrong places is causing performance issues, using the .gitignore file for this feels like a band-aid fix. Maybe we can build something less brittle by manually ignoring the usual suspects (target and node_modules)?

@FabianLars

Copy link
Copy Markdown
Member

Maybe we can build something less brittle by manually ignoring the usual suspects (target and node_modules)?

That's what i meant with "tauri's own gitignore file" in my first message.

Judging by the reports we've got so far from those that had issues with the current lookup, simply setting parents(false) so that only the projects gitignore files are applied should have fixed it for all of them.

This would the route i'd prefer. First we disable the parent dir gitignore files and then wait a while to see if there's a need to go further. (again, nobody that complained about this had the config ignored in their project directly iirc)
This way we would also ignore build caches and whatever the non-npm projects do that's gitignored.

lucasfernog added a commit that referenced this pull request Mar 2, 2025
alternative approach to #12791 as suggested by @Fabian-Lars
@FabianLars

Copy link
Copy Markdown
Member

#12871

@FabianLars FabianLars closed this Mar 3, 2025
@rmburg

rmburg commented Mar 6, 2025

Copy link
Copy Markdown
Contributor Author

Thank you!

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.

[bug] Couldn't recognize the current folder as a Tauri project

2 participants