Manually cherry-pick fix for normalize_path()#2751
Conversation
normalize_path()
|
Hi @SomeTiramisu , Please, add the test to exclude the possible regression for that. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2751 +/- ##
==========================================
- Coverage 74.63% 74.63% -0.01%
==========================================
Files 70 70
Lines 39898 39902 +4
==========================================
+ Hits 29778 29779 +1
- Misses 10120 10123 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
I'm new with the rust toolchain, so far I haven't found what obscure test triggered the What I can do is testing the improved accuracy of Curiously, |
|
It's whether I'm on the right track or completely lost. I fear going too deep for what looks like an edge case optimisation (allow preprocessor cache when include path contain a No more warning is a thing, but does this really improve performance? I still have to benchmark this (I will use mpc like in #2510 (comment)) |
`normalize_path()` is a no-op when path start with `..`, canonicalize. This should help to resolve `../whatever.h` include to an absolute path. is this really what we want here?
21ea439 to
854292f
Compare
|
This now looks like a two in one PR. On one hand Benchmark on over optimistic scenario (full clean rebuild, same code) with sccache, on a meson project (GIMP in my case) shows improvement on build time (because preprocessor cache is now working) at the cost of a bit slower caching. |
As discussed in #2510 (comment)
normalize_path()was copied from Cargo before commitrust-lang/cargo@7a6eaf9
which had an issue with ..s in processor output path (at least). This prevent
meson, or any exotic build environment to cache preprocessed objects.
This PR manually cherry pick the fix, but lack any associated test (yet)