On 2023/02/09 23:09, Matthew Weier O'Phinney <mweierophin...@gmail.com> wrote: > I'm not directly involved in maintenance, but my take on the scenario was > that these were rejected and reverted because they caused breakage
Your take is not quite correct. No PR was rejected due to breakage. There was exactly one (internal) breakage that occurred after merging, the DTrace build failure; it was a rather stupid mistake, but immediately after I learned about it, I submitted a trivial fix for it. Dmitry's demand for revert due to DTrace build failure: https://github.com/php/php-src/pull/10220#issuecomment-1383658247 My fix PR: https://github.com/php/php-src/pull/10334 Dmitry claimed it doesn't fix the build: https://github.com/php/php-src/pull/10220#issuecomment-1383706602 I asked him to explain why he thinks it didn't fix the build: https://github.com/php/php-src/pull/10220#issuecomment-1383714708 Dmitry refused to explain: https://github.com/php/php-src/pull/10220#issuecomment-1383739816 Reporter confirms my PR does indeed fix the build (proving Dmitry wrong): https://github.com/php/php-src/pull/10220#issuecomment-1383802334 Instead of applying my fix, everything was reverted. Does that sound reasonable to anybody? Do you always revert stuff when a merge breaks something instead of fixing the actual bug? If yes, why was https://github.com/php/php-src/commit/a21195650e53e3426680 not reverted after it caused a build failure AND a runtime crash? > This breakage was unacceptable without an RFC. I saw chatter from a number > of folks after the changes were merged about builds no longer compiling; I never heard of that. Can you give me a link to that chatter, please? If there was a problem with my work, it's important for me to learn about it. > A supermajority is required on any change that would lead to backwards > incompatibility for either end-users or extension writers. Your proposal is > something that would do the latter. No, it doesn't lead to backwards incompatibility. Did you see this sentence in my RFC? "If staying compatible with defective extensions is deemed important, these includes may be re-added to a header such as “php_compat.h”, possibly with a way to opt-out. That way, broken extensions still build, but PHP itself still benefits from a smaller and more correct set of #includes." And check the latest PR linked in my RFC: https://github.com/php/php-src/pull/10410 "One new commit adds compatibility #includes to main/php.h, because people on php-internals have suggested that source compatibility with third-party extensions should be retained, even if those extensions are buggy (e.g. forgetting to include errno.h even though they use errno)." There will be no backwards incompatibility, not even with broken third-party extensions. And if, by mistake, a backwards incompatibility gets reported, I'll take care to fix it. Did you see that my proposal to include third-party extensions in the nightly CI build got merged? https://github.com/php/php-src/pull/10404 This PR aims at ensuring best compatibility with third-party extensions, to be able to notice unintentional breakages as early as possible. What I'm trying to do is make PHP more reliable/stable/compatible than before. > As I pointed out earlier, the changes previously merged led to breakages > when compiling the project. How is that not enough? And as I pointed out earlier, I'd like to learn about those breakages. I hadn't heard of any. Please give me links so I can adjust my draft PRs to fix all known problems. > And dumping a huge bunch of PRs with such changes without first > discussing it with maintainers means a lot of effort reviewing That's not what happened. I did discuss these changes (on GitHub, not on this mailing list) tbefore I submitted the first cleanup PRs, and got very positive feedback. And my PRs got merged quickly, because those maintainers saw the obvious value in my cleanups. > — why are your proposed changes more important than any of the other > work the various maintainers are doing? I didn't say anything about my proposed changes being more important than something else. Such a comparison seems meaningless to me. Why bring it up? > This is why they asked for an RFC; something of this magnitude needs > discussion, because it impacts everybody already touching the > project, the people most familiar with it. After I posted on this mailing list, there was a lot of negative feedback on secondary questions (include comments, forward declarations). But the feedback on primary question, i.e. whether to clean up includes, was nearly 100% positive. The only counter-argument (against the cleanup) was that it would make merging branches harder. > The other point that has been brought up multiple times is that it > introduces breaking changes for extension maintainers. It does not. > Should these extensions be relying on one or more "god" headers instead of > the specific headers for the symbols they use? Probably not. I ... don't understand. What headers do extension authors rely on currently? Is that documented somewhere? What will, in your understanding, change about that after my cleanup? > I'm unsure why that's unclear or not enough for you. I tried my best to explain what's unclear. Please clarify by answering my questions. Max -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
