[WIP] tidy: skip triagebot.toml path checks for local submodule directories that we can't read

[jieyouxu]

Fixes #139856 where tidy fails locally if some submodules are not checked out as part of triagebot.toml path existence checks (initially implemented in #137885 to help detect broken triagebot.toml paths).

I say "unchecked out", but really it's an extremely naive heuristic of "don't bother checking paths to or under a submodule if we can't even read the submodule dir".

I'm not happy about this implementation because it's quite convoluted.

Testing

You can test this locally by:

• First set build.submodules = true to be able to successfully run ./x test tidy.
• Set build.submodules = false.
• Explicitly deinit a submodule, e.g. src/tools/enzyme.
• Change or add a [mentions] path to e.g. src/tools/enzyme/xd, observe no tidy error is emitted.
• CHange or add a [mentions] path to non-submodule, e.g. xd, observe a tidy error is emitted.

Alternatives

Honestly, I'm also kinda tempted to just either:

1. Gate this check as CI-only, or even
2. Remove this check entirely.

But I suppose other options are:

• Only run this check if triagebot.toml is modified, which does not actually fix root cause of tidy triagebot.toml trigger_files fails on jj git clone unchecked out submodules #139856.

Review advice

• Can't assume git exists
• More than happy to hear better solutions

cc @Kobzol @onur-ozkan for opinions

r? bootstrap

[Kobzol]

The implementation looks fine to me. I have just one question.

What is "Vacuously"?

[jieyouxu]

The implementation looks fine to me. I have just one question.

What is "Vacuously"?

I guess I just wanted to say we don't know if it exists or not, we just don't actually check it and instead simply skip it, I can reword that a bit lol

[Kobzol]

I just found the word funny xD

I tried it locally, but it seems like tidy doesn't even get that far?

./x test tidy
Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.06s
submodule library/backtrace does not appear to be checked out, but it is required for this step
Consider setting `build.submodules = true` or manually initializing the submodules.

[jieyouxu]

Oh right, sorry, it's because I have like an partially initialized set of submodules... I.e. what I did was to first build.submodule = true to be able to run ./x test tidy, and then manually git submodule deinit src/tools/enzyme, then introduce a src/tools/enzyme/xd mentions path that's invalid. I updated the repro steps, hopefully you can also repro.

[Kobzol]

Right, I was just wondering how common is that? Do people run tidy without initialized submodules? It would mean that they would need to manually initialize certain modules otherwise required to run tidy - do you think that's common? Furthermore, it looks like tidy already breaks without some submodules, so this check only solves a part of the issue.

[jieyouxu]

Right, I was just wondering how common is that? Do people run tidy without initialized submodules? It would mean that they would need to manually initialize certain modules otherwise required to run tidy - do you think that's common? Furthermore, it looks like tidy already breaks without some submodules, so this check only solves a part of the issue.

Yeah that is a good point, I wouldn't expect this configuration to be super common. I.e. this PR only kinda mitigates the problem for triagebot checks, but tidy probably will still fail on other checks.

[Kobzol]

Based on that, I'm not completely sure if it's super worth it, but it's not a lot of code, so fine by me. You can r=me once CI is green if you want to land it, up to you.

[jieyouxu]

Tbh I don't want to land this as-is because I don't feel the complexity pulls its weight. Maybe just don't run this locally?

[jieyouxu]

Update: okay so I asked Waffle for extra clarifications (it's not submodules = false), apparently it's because jj git clone doesn't create empty subdirectories for git submodules like git does, because jj's submodule support is not yet completed

The tool is fairly feature-complete, but some important features like support for Git submodules are not yet completed.

In that sense, I think this is the wrong approach (and even if we wanted to mitigate this for jj purposes I'd at most only skip locally), so I'm going to close this.