Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Effective deprecation mecanism #10393

Closed
dbuenzli opened this issue May 4, 2021 · 23 comments
Closed

Effective deprecation mecanism #10393

dbuenzli opened this issue May 4, 2021 · 23 comments

Comments

@dbuenzli
Copy link
Contributor

dbuenzli commented May 4, 2021

In PR #10392 @alainfrisch notes:

As per the discussion on that issue, Stdlib.{min,max} are not marked as deprecated. I think it will make it harder to eliminate them, and switching to specialized functions is easy enough (and otherwise, turning turning off the deprecated warning, or redefining local versions of min/max if used on complex types), so my preference would be to mark them as deprecated right now. But oh, well.

I don't think turning off warnings explicitly is a good strategy, it defeats the purpose of having a deprecation procedure. But then we also don't want programmers have to live with warnings that they can't act on other than by silencing them.

@xavierleroy once asked me a write up for a deprecation procedure. I never heard anything back. But the idea there was to wait for the deprecation workaround to hit debian stable before starting the effective deprecation so that people mostly don't have to live with the warning.

However that introduces quite a bit of maintenance bureaucracy (delay between introduction of the workaround and the formal deprecation) and it may feel weird to align the project on whatever debian decides for its maintenance cycles. But that assumed the current state of affairs.

This could be simplified by if we had:

  1. A version parameter on @deprecated deprecated-version, that indicates the version deprecated-version in which the feature is deprecated and the workaround introduced.
  2. A cli flag -min min-version that indicates the minimal OCaml version min-version with which the source we are compiling is supposed to compile (defaults to the compiler version itself).

That way you only start warning when min-version >= deprecated-version

@alainfrisch
Copy link
Contributor

Should we use the same "min-version" to warn when the user tries to use function marked with a @since annotation greater to our current target version? One could even use that version to disable (or trigger warning) on some new language features.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 4, 2021

Should we use the same "min-version" to warn when the user tries to use function marked with a @since annotation greater to our current target version?

That would be great !

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 4, 2021

But with @since there's a catch: libraries use that aswell with their own version scheme.

We need to be able to distinguish the stdlib and then we get into how we could generalize this for use by libraries and then we get into ah but the compiler still doesn't know what a library is.

Let's start simple !

@alainfrisch
Copy link
Contributor

  • The current deprecation is implemented as just another alert; it already has a string payload (the error message to show to the user, which usually would indicate how to replace the use of the deprecated component). Do you have in mind adding a second (optional) payload? Can we still use the generic alert mechanism, or do we need to specialize again for the deprecation stuff?

  • The deprecation alert can also be used by other libraries (without the version number). At least we should design the evolution of deprecation system so that it can later scale to supporting other libraries.

@gasche
Copy link
Member

gasche commented May 4, 2021

For the record, I like your write up on deprecation policy, that mostly matches what I had in mind as well (we discussed deprecation policies in the past but never reached a consensus). I would be in favor of starting doing this right now -- we can indeed discussion schemes to improve this.

(the -min-version <version> idea has been suggested in the past but then people find it too tempting to ask for warnings as well for language features that are not supported, etc., and that sets up a large bag of expectations that we cannot realistically deliver on -- adding version-checking code in the codebase hurts maintainability, and some parts of the compiler (the type-system in particular) have non-local changes of behavior, such that it would be very hard to detect cases that would be incompatible with older versions.)

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 4, 2021

@alainfrisch I have no idea how all this is implemented and how alerts effectively work. But yes I agree we should have in mind how it could be scaled to libraries.

It seems to me that the only point to address is the cli flag. That is, it should be repeatable and the syntax should be able to specify a name, ignored for now, e.g. -min [$NAME:]$VERSION (a bit of time should be spent on making that syntax clearly unambiguous for what $NAME and $VERSION could be in the future).

@gasche I think the problem with my write up is that by not warning as soon as the workaround is introduced it penalises later people who have the opportunity to act now. So it's not super user friendly (and as mentioned more bureaucratic for the dev team).

Also I can perfectly see -min as being a can of worms for language features but I don't think it should be a reason to reject it for libraries and the stdlib. The dev team should just make it clear that it's not willing to go down that route. Maybe we could reflect it in the cli name e.g. -lib-min.

(I do however think that using -min version with an ocaml version < version should issue a warning to tell the user that all bets are off or, alternatively, error)

@alainfrisch
Copy link
Contributor

A general design could be to add some predicate to alert annotations (in addition to the existing textual payload):

val foo: int -> int
 [@@alert my_alert "Blablabla" (ocaml_version >= "4.08")]

When referencing such a component, the alert is triggered only if the predicate holds; the predicate can refer to some variables (that can be passed on the CLI, or specified locally through attributes, in the same way we control which warnings/alerts are enabled). ocaml_version would have a default value. (We need to use a comparison which understands the syntax of versions.)

The deprecation attribute [@@deprecated "..."] is currently turned in [@@alert deprecated "..."]. We would then support a variant [@@deprecated "..." "4.08"], mapped into [@@alert deprecated "..." (ocaml_version >= "4.08")] (or using another variable than ocaml_version, to be specified on the CLI).

Similarly, we could introduce a [@@since "4.08"], which would be turned into [@@alert since "..." (ocaml_version < "4.08")] (it will trigger if we target an old version).

If a function is discovered to be buggy in a range of (old) versions, one could do something like: [@@alert buggy "This was buggy!" (ocaml_version >= "4.04" && ocaml_version <= "4.08")]; or, in an external library, some function could be trigger alerts based on a combination of the ocaml_version and the library's own version (e.g. it is discovered that some functions in old versions of the library are buggy when used with some versions of OCaml). Or, we could mark that some functions in Unix as available on Windows, but only for recent versions of it (so a predicate referring to e.g. windows_version). Perhaps a bit far-fetched...

Anyway, my point is that if we want to keep the internal machinery of generic alerts to support version comparison for both the existing "deprecated" and a new "since" alert (which compares in the other direction), and to do that for both OCaml version but also external libraries, we should perhaps directly extend the notion of alerts with a more general predicate mechanism, even if in practice we will only use simple predicates.

@alainfrisch
Copy link
Contributor

I don't think turning off warnings explicitly is a good strategy

Coming back to this statement, which problem do you see with:

  • The authors of a library X which is supposed to work with OCaml version N develop and use CI-testing with OCaml version N (with the deprecation alert enabled).
  • But, when the library is compiled by other people (through OPAM or whatever), the deprecation alert is disabled (as all warnings should be for released code).

Having finer-grained descriptions of versions in which a component has been deprecated (resp. introduced) helps casual contributors to X quickly detect uses of components that shouldn't be used for the expected target version for the library (OCaml N), without actually switching to that version of OCaml. It's useful, but it's just some helper (the real criterion is really whether the code will still compile with OCaml version N).

(That's why I don't really understand the problem of adding deprecation warning quickly enough, even without this fine-grained versioning system; but I might miss some perspective...)

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 4, 2021

I'm not getting your last message.

  • But, when the library is compiled by other people (through OPAM or whatever), the deprecation alert is disabled (as all warnings should be for released code).

Warnings as errors should be disabled for released code. Not warnings per se.

(That's why I don't really understand the problem of adding deprecation warning quickly enough, even without this fine-grained versioning system; but I might miss some perspective...)

Assume a workaround is introduced but the warning is not introduced at the same time.

Now suppose you only target versions which have the workaround. You may still end up using the deprecated stuff without being warned. This will entail maintenance work later when the warning is introduced. But had you been properly warned in the first place this maintenance work could have been avoided.

@alainfrisch
Copy link
Contributor

Warnings as errors should be disabled for released code. Not warnings per se.

Ah. It's true that I'm working under that assumption that the current state of the tooling makes non-fatal warning rather useless (they will just be lost in the log of the compilation, with no way to see them again without forcing a recompilation of everything; I think the build system should consider warnings as outputs of the compiler, properly keep track of them, and show them again on demand, for the whole code base).

But anyway, and a consumer of an external library, I'm not very interested to see (non-fatal) warnings in that foreign code (when I install it through OPAM, or when building my mono-repo where I've imported that code). Warnings are for developers, and as a consumer, I really don't care about them; if I'm ready to act on the warnings, it means I put myself in the position of a contributor for the external library.

In which case are you interested to see (non-fatal) warnings when compiling "foreign code" (through OPAM, or in a mono-repo)?

@alainfrisch
Copy link
Contributor

Assume a workaround is introduced but the warning is not introduced at the same time.

No need to convince me on that part :-) I'm clearly pushing for introducing the warning as soon as possible.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 6, 2021

I think the build system should consider warnings as outputs of the compiler, properly keep track of them, and show them again on demand, for the whole code base

A more solid foundation would be for the compiler to consider warnings as part of its written artefacts. That idea is floating in odoc, see ocaml/odoc#557

In which case are you interested to see (non-fatal) warnings when compiling "foreign code" (through OPAM, or in a mono-repo)?

I'm interested in knowing about the health of my dependencies and if they may start breaking in the future. So even as a user I'm interested in seeing them.

No need to convince me on that part :-) I'm clearly pushing for introducing the warning as soon as possible.

Then I really didn't understand what you were asking for :-)

@gasche
Copy link
Member

gasche commented May 6, 2021

(Optionally) emitting foo.warnings when compiling foo.ml makes a lot of sense to me. Would you want to create an issue to track this idea?

@alainfrisch
Copy link
Contributor

Then I really didn't understand what you were asking for :-)

I wanted to understand why it bothers people to add deprecation warnings as soon as possible. I can see how keeping more versioning information can bring additional convenience, but I've still a hard-time understanding what problems it would create in practice to add deprecation warnings e.g. for #10392.

I'm interested in knowing about the health of my dependencies and if they may start breaking in the future. So even as a user I'm interested in seeing them.

If you want to know if your dependencies use functions that are/will be deprecated, enable the warnings for them (as fatal or not, depending on your preference); and then I guess you want to have the information as early as possible. If you are not interested, turn the warnings off. Knowing at which version the function have been marked as deprecated is useful, I guess, if you intend to patch the library to avoid them, but don't want to force a dependency for that library on a recent version of OCaml; but then, as any contributor to that library, you should make sure to compile with the oldest version of OCaml they want to support (and otherwise their CI will complain when you submit your patch).

The extra convenience brought by keeping the version information in the deprecation attribute would allow you to avoid dealing now with the function if they only been deprecated too recently for the external library (meaning you cannot use a replacement function without forcing the library to depend on a more recent version of OCaml). This is useful, but then, for a very stable library, their author will have less incentive to force a dependency on a more version of OCaml, and thus less chance to avoid the deprecation warning in the future. At some point, either they will need to bump their minimal OCaml version requirement; or you will have to accept that their library will be broken when the functions really go away. Isn't it just pushing the problem for the future?

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 6, 2021

I wanted to understand why it bothers people to add deprecation warnings as soon as possible.

Because at the moment if you do so, then for the person who needs to support earlier OCaml versions you can either:

  1. Suffer through the warning each time you build until you only support versions that have the workaround (I did with String.lowercase et al, it was an annoying experience).
  2. Locally turn off the warning in your code. But that means the problem no longer exist. You never, ever, see the warning again (even if you bumped your OCaml version up, unless you are very careful about these things) and the day the deprecated functionality is removed the code will break which is equivalent to not having a deprecation procedure.

@alainfrisch
Copy link
Contributor

Ok, thanks, I think I start to get the picture. It's really an helper to library authors (who wants the support old versions of OCaml even with the latest version of their libraries), to decide when to rewrite their code base and bump their minimum OCaml version requirement. The thing is that currently, we never really remove deprecated functions, so there is an incentive for library author to keep supporting old versions and just ignore warnings (to increase the audience). And in turn, this reduces the incentive to users to upgrade to a newer version of OCaml. At the end, I've a slight feeling that a lot of energy is put into making sure the community can avoid adopting new versions of OCaml, and some of that energy might be better spent making sure adopting new versions works fine. But I'm not a very active contributor to the ecosystem, so I'll defer to your judgment.

@alainfrisch
Copy link
Contributor

(Optionally) emitting foo.warnings when compiling foo.ml makes a lot of sense to me. Would you want to create an issue to track this idea?

I've heard rumors that the dune developers intend to improve support for warnings, so perhaps they have better ideas (or just this one exactly).

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 6, 2021

Ok, thanks, I think I start to get the picture. It's really an helper to library authors.

Well not only library authors, authors in general.

At the end, I've a slight feeling that a lot of energy is put into making sure the community can avoid adopting new versions of OCaml, and some of that energy might be better spent making sure adopting new versions works fine.

The problem is that this may well be out of your control. I don't think the problem of sticking to older version is the audience, it's rather the OCaml version supported by OS package managers. If you want to create applications written in OCaml that can easily be packaged by them, then you are bound by their constraints on OCaml. See comment 2 in my deprecation write up.

(There are certainly other factors like the maintenance work, risk and testing needed for upgrading the compiler used by deployed application, not everyone may afford or have the resources to cope with the new release pace of OCaml)

I think the scheme proposed in this issue is the one which strikes the best balance for the user experience of deprecation managers, people who want to benefit from it asap and those who can't for good reasons.

@alainfrisch
Copy link
Contributor

If you want to create applications written in OCaml that can easily be packaged by them, then you are bound by their constraints on OCaml.

Is that really how it works (honest question)? I would have expected that application that turns out to be implemented in OCaml would have packaging instructions that starts by installing OCaml+OPAM and the libraries (or vendor everything, including OCaml). Is it really the case that packages depend on the OCaml compiler + OCaml libraries installed by the OS? Or do they rely on the OCaml + OPAM shipped by the OS, but external OCaml libraries are installed by OPAM? (I assume that OCaml libraries packages for various OS is a small subset of the OPAM universe, but perhaps I'm wrong.) The compatibility matrix between OCaml and ecosystem libraries + the impossibility to have multiple "global" versions of OCaml in the same OS package (or that would force to have version of all OCaml libraries packaged for each version, which would be a nightmare to maintain) would seriously restrict which version of OCaml and libraries can be used by such OCaml applications to be packaged.

@avsm
Copy link
Member

avsm commented May 6, 2021

The answer is "all of the above", @alainfrisch :-)

  • Developers (hacking on OCaml libraries): can use opam+source+latest OCaml with multiple switches
  • Users (just installing a binary): can use opam lockfiles / esy locks
  • Projects: can curate their library versions with lockfiles
  • OS package managers: these solve across multiple projects to find the common subset of library versions that work with the subset of OCaml, and ship those. See e.g. Debian or Homebrew or OpenBSD. They do not ship OCaml libraries typically but application binaries.

I talked about some of the challenges here for opam in an OCaml Workshop talk a few years ago. The tl;dr is that the opam-repository has solvers that can work in all of these modes now, but that all of the modes are in fact used in the wild.

@dbuenzli's deprecation proposal looks very workable to me. If you need a library for OCaml version parsing in the compiler, https://github.com/ocurrent/ocaml-version/blob/master/ocaml_version.ml#L17 can be relicensed for contribution to compiler-libs.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented May 6, 2021

Is that really how it works (honest question)? I would have expected that application that turns out to be implemented in OCaml would have packaging instructions that starts by installing OCaml+OPAM and the libraries (or vendor everything, including OCaml).

Suppose you implement say Firefox in OCaml (or more realistically a nice cli tool). What you describe here is the best way of never having any user :-) The key point of the comment I linked to is: "install applications when you don't care about the underlying technology".

@alainfrisch
Copy link
Contributor

I was under the impression that most Python and Javascript projects were relying on pip / npm to locally install their dependencies (instead of relying on the OS), with pip / npm themselves perhaps (or not) installed by the OS. In our case, that could be OPAM (perhaps) coming with the OS, and being responsible for installing OCaml libraries, but also the OCaml compiler itself. Anyway, I was thinking about packaging applications implemented in OCaml, not compiling such applications by end-users.

Thanks again @dbuenzli and @avsm for your insights on those issues.

@github-actions
Copy link

github-actions bot commented May 9, 2022

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 9, 2022
@github-actions github-actions bot closed this as completed Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants