Make addressing deprecations acceptable in a patch release

[greg0ire]

Prompted by doctrine/orm#10736

Addressing a deprecation is not a bugfix, it does not make the software more stable. Deprecations are annoying though, and unless we switch to using the ~ operator instead of the ^ operator for our dependencies, we might get new ones out of the blue.
I propose to explicitly allow PRs that address deprecations in patch releases, so that users do not have to wait until the next minor to have fewer deprecations.

[greg0ire]

@mpdude please review

[mpdude]

Hm, a tricky one.

Maybe the question we should ask yourselves first is: "Do we find it acceptable to raise our own minimum requirements in bugfix releases?"

Assume we depend on version ^1.0 of some package. Now this package releases 1.1.0 (or, beware! 1.0.1?), adding a deprecation and telling us to switch to a new API that was part of that release.

In that case, I think we should not raise minimum requirements in our own next bugfix release: It would risk to cut off our users from this and following bugfixes in case they cannot (for whatever reason) accept the new (stricter) requirements.

But, do we want to add – in a bugfix release – code to do feature/version detection for the depended-upon package, just to remedy the deprecation? IMHO the answer is "no" as well.

So, the only way I see to deal with such deprecations in a graceful way is to wait for our own next minor release, where we can raise minimum deps to a version where the new/alternative API is availabe, and then use that.

[greg0ire]

The consequences of this is that we should either release minors more often, potentially as often as one of our dependencies has a new minor version, and/or we should prevent the new minor version from being installed by using the ~ operator rather than the ^ operator. Not great.

I think the best course of action might be to tolerate the feature/version detection code in patch releases.

[mpdude]

Isn’t the point of having packages and releases also to de-couple regarding time, that is, everyone dealing with upgrades at a time they see fit?

What pressure would it put on us having to release a minor every time a depended-upon package adds a deprecation?

Also, as a library we should try (IMHO) to be as liberal as possible (and practical) regarding dependency constraints… so, not use ~ and not raise minimum requirements without or little reason (I know that’s opinionated 🔥).

Maybe @nicolas-grekas can tell us how the Symfony project addresses this concern?

[mpdude]

Maybe you can say that a deprecation is an announcement that something needs to be done in the future. We will address it, but at a time that fits our release cycle.

It’s not a bug that needs more immediate attention.

[greg0ire]

What pressure would it put on us having to release a minor every time a depended-upon package adds a deprecation?

Releasing a minor version of a Doctrine package is not such big deal now that we are using laminas/automatic-releases. Maybe this is the way. The last minor version of doctrine/orm was quite big so maybe it would be a good thing in that it would force us to release smaller minors more often.

There's a drawback to that though: if the minor contains new deprecations, then our users will be trading indirect deprecations for other deprecations (from doctrine/orm directly or through some the framework integration package).

So maybe the real question we should ask ourselves is: why do users want deprecations to be fixed? That's probably because it clutters the output of phpunit, or the Symfony inspector. Maybe indirect deprecations shouldn't be shown by default?

[nicolas-grekas]

That's the big Yes from me because it enables smooth transitions, by allowing decoupling of dependency updates.
It allows escaping from dependency hell.
👍

[greg0ire]

So should we allow addressing a deprecation in a patch release iff it can be done without bumping the constraints, or introducing feature detection / version detection (because that's too complex)? Is that what you both are saying? Or should it be more simple than that and should doctrine/orm#10737 also have been retargeted in your opinion?

[mpdude]

@nicolas-grekas thank you for your swift response!

That's the big Yes from me because it enables smooth transitions,

I'm afraid I don't exactly get what you're referring to... 🤔 Could you please try to re-phrase that?

Do you mean not tightening requirements in bugfix releases, not using the ~ operator and/or not trying to address deprecations too eagerly?

[nicolas-grekas]

As bug fix, so that PR did right being merged on 2.15.
This is how we can enable the community to adopt new versions as quickly as possible, by making it as easy as possible to run deprecation-free (thus accelerating the adoption of the next major)

[nicolas-grekas]

I mean that depreciation should be addressed as bug fixes if that wasn't explicitly enough :)

[mpdude]

And if that means having to bump requirements on bugfix releases, that’s ok for you?

[greg0ire]

I'm upvoting the clarification, but after the discussion with @mpdude, I'm no longer sure I agree with it. Addressing a deprecation is not a bugfix, there IMO no bug, just an inconvenience for developers. Maybe what should be challenged is the inconvenience? I mean what happens if addressing the deprecation causes an actual bug? @mpdude was that what you were worried about when saying "do we want to add – in a bugfix release – code to do feature/version detection for the depended-upon package"?

Anyway, we clearly need more Doctrine maintainers to review this, it seems to be quite controversial.

[mpdude]

was that what you were worried about when saying "do we want to add – in a bugfix release – code to do feature/version detection for the depended-upon package"?

(*) Yes, kind of. In bugfixes, we want as little risk as possible. So, we should not add lots of detection/case switching logic there just to be able to avoid deprecations while keeping minimum requirements unchanged.

(*) If we want to address deprecations without such logic, we can probably only do it by raising minimum requirements.

If we raise requirements in bugfixes, people may not be able to upgrade to the new bugfix version although they were able to use the preceding bugfix.

In my opinion, a minor release is a promise that users get a certain feature set and that we will provide bugfixes to those users for a certain time. Raising minimum requirements means potentially excluding/dropping some of these users along the way, during the bugfix phase of said release.

Disclaimer:

What a package depends upon is an implementation detail,
not part of its exposed API. So dependency/minimum requirement changes are irrelevant with regard to the version number that needs to be attached. Technically, raising requirements in a bugfix is not wrong.

It’s more of a practical question if we want to impose this additional constraint. “No” means we can address deprecations faster. “Yes” means we can guarantee more predictable upgrade paths/support periods.

Everything is a tradeoff.

Update/edit: Paragraphs marked with (*) have been challenged by Nicolas below.

[nicolas-grekas]

There's absolutely no needs to bump any requirements, what a strange idea. I've spent years on Symfony following this process and I can tell by experience.

Not requiring a dep bump is precisely the reason why this should be fixed in patch releases. The other direction, fixing depreciation as a feature, means correlating the ability to run deprecation-free to a version bump of another dep, typically PHP. And THAT creates dependency hell, which is defined as the extremely high cost to upgrade dependencies because of chains of dependency requirements. Like the one we're talking about.

[greg0ire]

Feels like we have many evils to choose from, and we should determine what's the lesser evil.

• potential unstability due to complex detection code
• cluttered deprecation reports for users + deprecation reported as Github issues for maintainers + trading deprecations fixes for new deprecations coming with a new minor
• tightened dependencies

I think I agree with Nicolas, and that we should be super careful when reviewing code that addresses deprecation, but let it land on a patch release. We should insist on having the most simple fix on a patch release, even if that means postponing the real fix to the next minor.

[mpdude]

Well if a depended-upon package says “the API you’re using has been deprecated during our recent 1.1.0 release, use the new API instead”… what can we do?

If we switch to that new API, we need to bump to ^1.1, don’t we?

Edit: I mean bumping our own Composer requirements, not our version number.

[nicolas-grekas]

My experience is that the risk we're talking about is minimal. It's usually just a class_exists() check or similar. No need to FUD ourselves :)

[mpdude]

Ah so you’re saying not bump our own composer requirements, but add logic to recognize the new API and avoid the deprecation, and do that in a bugfix release?

[nicolas-grekas]

@mpdude 💯, that's how we do it and that works really well!

[mpdude]

👌🏻. But that also means you avoid or forbid bumping requirements in bugfixes, do you?

[greg0ire]

I pushed a note that should clarify this 🙂

[mpdude]

I can endorse the current state ✌🏻

In addition, depending on whether and how Nicolas answers my last question, do we want to state either

• that no requirement bumps in bugfixes are allowed (not limit that to the context of deprecations?)
• minor releases may raise requirements if that in turn helps simplify our own codebase (ie get rid of class_exists checks, to stick with the example) and/or use new APIs, but we don’t raise them just for the sake of it?

[nicolas-grekas]

👌🏻. But that also means you avoid or forbid bumping requirements in bugfixes, do you?

correct

of course, exception may happen, but the rule of thumb is that bumping a dep in a bugfix should not force a dep bump of a transitive dep (php especially of course, but others could be checked)

[mpdude]

Just realized that since we have no fixed/guaranteed release cycle like Symfony, in fact no promise is given.

We can release a new minor at any time, and once we do, we stop bugfixing the previous minor, right?

So, for someone using for example 2.15.1 and being constrained by requirements, the journey may end at any time. It does not matter if we bump reqs and name the result 2.15.2 or 2.16.0, the net effect is the same.

[nicolas-grekas]

Another rule of thumb: don't bump to the next minor unless all deprecations of the current one are fixed (which is clearly not the case today)

[nicolas-grekas]

The corollary is: make the CI fail when deprecations are raised, and don't release when the CI is not green ;)

[SenseException]

We can release a new minor at any time, and once we do, we stop bugfixing the previous minor, right?

That's the case.

It seems like the ~ vs. ^ conversation moved into the background. While one topic was not bumping 1.0 to 1.1 in the dependencies under certain conditions, newer releases of the same dependency would introduce new deprecations that couldn't be addressed when e.g. ^1.1 is used and a new 1.3.0 of a dependency got released.

[nicolas-grekas]

Any kind of restriction contributes to dependency hell, so no, please don't put an upper limit to deps with ~. Deprecations are meant to flow to en users and using ~ would be a way to actively defeat the communication intentions of the author of deps.
It's fine if a non maintained release start triggering deprecation because trasitive deps moved forward. There are many tools to deal with deprecations any way (and upgrading is one of them)

[greg0ire]

@derrabus please review

[greg0ire]

And also @morozov please 🙂

[morozov]

I haven't read the thread but as a library user, why should I care about the deprecations triggered by the library? My only concern is that the library does its job, and deprecations are not errors. I do not think that deprecations should be addressed in patch releases.

[greg0ire]

I'd say because they clutter logs and other todo lists created by framework integration with stuff that is not actionable at the application level. They're like an itch that needs to be scratched.

[morozov]

I'd say because they clutter logs and other todo lists created by framework integration with stuff that is not actionable at the application level. They're like an itch that needs to be scratched.

IMO this is a wrong attitude. As I said, as a user of a library, I don't care about its deprecations. Why do I even log or see them?

[greg0ire]

Because with doctrine/deprecations the logging strategy is global rather than dependent on the calling piece of code. You either log all deprecations, or none of them. This is related to doctrine/deprecations#30

[morozov]

It looks like a problem that needs to be addressed in the deprecations library, not in the ones that use it.

[greg0ire]

I think that's a non-trivial problem, and until it is addressed, I propose we accept addressing deprecations in patch releases, as a workaround. Let we know what you think about https://doctrine.slack.com/archives/GERFT2W5T/p1687079049728499?thread_ts=1685615488.792749&cid=GERFT2W5T , and if it seems like a viable option to most Doctrine maintainers, I might try to implement it.

[morozov]

I think that's a non-trivial problem, and until it is addressed, I propose we accept addressing deprecations in patch releases, as a workaround.

I disagree with that. This isn't a problem that needs to be solved. Which again contributes to the point that in general, runtime deprecations is quite a problematic approach.

Let we know what you think about […]

Shouldn't the user receive just the deprecations that their own code triggers? Essentially, if a deprecated code flow is invoked directly from outside of the vendor/ directory (e.g., the src/), it should trigger a deprecation. Otherwise, it shouldn't. I don't think the user needs to provide any additional configuration for that (at least that might be a sane default).

[greg0ire]

Ok, then maybe instead of providing packages, we could add either a boolean argument enableWith* methods, that would govern whether there is a filtering happening. Or maybe there should be a separate method just for disabling the filter.

[morozov]

I would consider reporting deprecations from within dependencies a bug and just fix it without introducing any new API. As a library user, why would I care about the library using other libraries' deprecated APIs?

[greg0ire]

To quote @stof (from https://doctrine.slack.com/archives/GERFT2W5T/p1685951885663699?thread_ts=1685615488.792749&cid=GERFT2W5T)

a project that want to be ready to migrate to the next major version will also care about whether their dependencies are still using deprecated APIs

doctrine/deprecations#30 (comment) raises similar concerns. I do agree that doing the filtering should be a sane default though.

[morozov]

The only thing the maintainer of a package (A) that uses a Doctrine library (C) via a 3rd-party dependency (B) can do is make sure that:

1. A uses the latest supported version of B.
2. B supports the version of C that they want to migrate to.

The exact deprecation messages and other details are not a concern for the A's maintainers. There are already tools like semantic versioning and dependency constraints defined at the package level that they can use for a smooth migration.

[stof]

Saying that users of a library should not see indirect deprecations would imply that the CI of the library should be able to discover all deprecations ever encountered when using the library in a project (as you cannot rely on your users telling you that for their own use case they reach a path where your library triggered a deprecation). And this requires much more than 100% branch coverage (even 100% path coverage won't ensure it) as the deprecation might be about some kind of values becoming forbidden while others are still allowed, or some specific combination of settings becoming forbidden in the future.

And users might not care about being deprecation-free immediately. But the whole point of deprecations is being able to prepare for the next major version. In such case, a user will definitely care whether Doctrine is ready or no to support the next version of Symfony for instance, because their own upgrade will be blocked by the compatibility state of Doctrine.

Personally, I don't care much whether solving deprecations gets done in a patch or minor release, as long as the minor release comes soon enough after the merge so that users can benefit from it.
In the case of Symfony, the shipping delay for a minor release is 6 months due to our schedule so it makes sense for us to fix as much deprecations as possible in patch releases (which have a shipping delay of at most 1 month). Without a fixed release schedule, the rule could then be implemented by releasing minors more often instead of patching deprecations.
Side note: depending on the amount of changes needed to solve a deprecation coming from a userland package, it also happens that Symfony does that in a minor version as the change would be too risky. Our rule is not absolute there.