Log globbing anomalies: max path, drive enumeration

dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.

https://docs.microsoft.com/visualstudio/msbuild/msbuild

MIT License

5.21k stars 1.35k forks source link

Log globbing anomalies: max path, drive enumeration #3642

Open cdmihai opened 6 years ago

cdmihai commented 6 years ago

Drive enumeration. The more general problem is when the glob pattern contains properties which evaluate to empty. ($(src)/**/*.cs evaluates to /**/*.cs).
Original wildcard pattern is retuned (as it sometimes happens). Maybe also include the reason.

rainersigwald commented 4 years ago

Let's spike on this in the next sprint: if it's easy to do (because we have a logging context where we'd like to collect the timing/bad patterns and log them) we'll do it, and if not then we'll describe what does exist here.

Forgind commented 4 years ago

I have two partial solutions (one per commit) here. Do you think it's better to try to sense a globbing anomaly while expanding a property or wait until we're globbing and assume no one wants to enumerate their whole drive? I could also try to pass information about empty properties from one to the other, but that would be complicated and a lot of extra information, so I don't think it's worth it.

cdmihai commented 4 years ago

Keep in mind these things (property evaluation, glob parsing/expanding) are on the hot path, so newly added logic (like additional regex matching) has a good chance of being picked up by RPS. For globbing anomalies I would just change the globbing code to record diagnostics and bubble them upwards to the evaluator / expander which can log appropriate message. For drive enumeration I would avoid duplicating the knowledge of glob syntax between the glob parsing code and the expander (glob parsing code should be the only one that knows the syntax). The glob parsing code should just record a "drive enumeration" diagnostic when it detects that the fixed directory part is a drive, which then is bubbled up to a layer appropriate for logging.

rokonec commented 3 years ago

@ladipro has any of your recent changes in globs addressed that?

ladipro commented 3 years ago

@rokonec no, the recent globbing changes did not improve logging. The unintended drive enumeration problem was somewhat alleviated in #5669 but I agree that this issue should be in backlog.

mruxmohan4 commented 2 years ago

@ladipro are there any updates on logging/surfacing an exception for max path during wildcard expansion?

ladipro commented 2 years ago

@mruxmohan-msft there have been some internal discussions but no conclusion yet. The issue still exists. I'll leave a comment in #7029.

rokonec commented 7 months ago

@JanKrivanek This might be good candidate for analyzers V1, especially max path, it is so serious that it maybe shall be warning but warnings could break some builds