Remove include "sanity check" to get better error #19650
Conversation
Instead of rejecting directories / non-regular files early with a generic error, we should just accept them and error later when a read is attempted. This is more general and will generate a better error message on Linux. On Windows, the error remains the same as before.
fix format for include path
| @@ -8,7 +12,7 @@ include (__DIR__); | |||
| echo "Done\n"; | |||
| ?> | |||
| --EXPECTF-- | |||
| Warning: include(%s): Failed to open stream: %s in %s on line %d | |||
| Notice: include(): Read of %s bytes failed with errno=21 Is a directory in %s on line %d | |||
There was a problem hiding this comment.
We just had an internal discussion about this error downgrade which is happening due to error triggered during read which is notice. The decision is to revert this for now. I'm looking to error changes so might be able to handle this better.
It's not probably huge issue but might be quite late for this and it's worth to delay it. I'm also not sure if the Win inconsistency is a good thing.
There was a problem hiding this comment.
Note that only the first of two warnings gets downgraded. The second one (Failed opening '%s' for inclusion) is always a warning and consistent across OS.
There was a problem hiding this comment.
Yeah I noticed that - that's why I don't think it's a huge issue. I was thinking that it would be better to keep warning but more I think about it, this might seem possibly better. But it's still not nice that we need different test for Windows. I'm starting to think that the stat might be there to make it consistent so maybe checking how to get more detailed error on Windows as well would be ideal.
It's well past the feature freeze so I still think it's better to delay it as it needs a bit more considering.
Instead of rejecting directories / non-regular files early with a generic error, we should just accept them and error later when a read is attempted. This is more general and will generate a better error message on Linux. On Windows, the error remains the same as before.