Skip to content

Commit

Permalink
A better mechanism for coordinating internal breaking changes. (#53849)
Browse files Browse the repository at this point in the history
This was origiginally supposed to be an issue, but I just started
writing out the whole code in the issue text to explain what I want all
the behavior to be, so instead, here's the actual implementation of it,
with the motativation in the commit message, and the details of the
actual behavior in the code change ;)

Sometimes packages rely on Julia internals. This is in general
discouraged, but of course for some packages, there isn't really any
other option. If your packages needs to hook the julia internals in a
deep way or is specifically about introspecting the way that julia
itself works, then some amount of reliance on internals is inevitable.
In general, we're happy to let people touch the internals, as long as
they (and their users) are aware that things will break and it's on them
to fix things.

That said, I think we've been a little bit too *caveat emptor* on this
entire business. There's a number of really key packages that rely on
internals (I'm thinking in particular of Revise, Cthulhu and its
dependency stacks) that if they're broken, it's really hard to even
develop julia itself. In particular, these packages have been broken on
Julia master for a more than a week now (following #52415) and there has
been much frustration.

I think one of the biggest issues is that we're generally relying on
`VERSION` checks for these kinds of things. This isn't really a problem
when updating a package between released major versions, but for closely
coupled packages like the above you run into two problems:

1. Since the VERSION number of a package is not known ahead of time,
some breaking changes cannot be made atomically, i.e. we need to merge
the base PR (which bumps people's nightly) in order to get the version
number, which we then need to plug into the various PRs in all the
various packages. If something goes wrong in this process (as it did
this time), there can be extended breakage.

2. The VERSION based comparison can be wrong if you're on an older PR
(that's a head of the base branch, but with different commits). As a
result, when working on base PRs, you not infrequently run into a
situation, where you get a VERSION false-positive from updating a
package, introducing errors you didn't see before. This one isn't
usually that terrible, because a rebase will fix it (and you'll probably
need to rebase anyway), but it can be very confusing to get new and
unexpected errors from random packages.

I would propose that going forward, we strongly discourage closely
coupled packages from using `VERSION` comparisons and intead:

1. Where possible, probe for the feature or method signature that
they're actually looking for, if it's something small (e.g. a rename of
base type).
2. That we as julia base establish a mechanism for probing whether your
current pre-release julia has a certain change. My sketch proposal is in
this PR.
  • Loading branch information
Keno authored May 5, 2024
1 parent da6892f commit 2cf469d
Showing 1 changed file with 106 additions and 0 deletions.
106 changes: 106 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,111 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

# Internal changes mechanism.
# Instructions for Julia Core Developers:
# 1. When making a breaking change that is known to be depnedet upon by an
# important and closely coupled package, decide on a unique `change_name`
# for your PR and add it to the list below. In general, is is better to
# err on the side of caution and assign a `change_name` even if it is not
# clear that it is required. `change_name`s may also be assigned after the
# fact in a separate PR. (Note that this may cause packages to misbehave
# on versions in between the change and the assignment of the `change_name`,
# but this is often still better than the alternative of misbehaving on unknown
# versions).

# Instructions for Release Managers:
# 1. Upon tagging any release, clear the list of internal changes.
# 2. Upon tagging an -alpha version
# a. On master, set __next_removal_version to v"1.(x+1)-alpha"
# b. On the release branch, set __next_removal_version to v"1.x" (no -alpha)
# 3. Upong tagging a release candidate, clear the list of internal changes and
# set __next_removal_version to `nothing`.
const __next_removal_version = v"1.12-alpha"
const __internal_changes_list = (
:invertedlinetables,
:codeinforefactor,
# Add new change names above this line
)

if !isempty(__internal_changes_list)
if VERSION == __next_removal_version
error("You have tagged a new release without clearing the internal changes list.")
end
elseif __next_removal_version === nothing
error("You have tagged a new release candidate without clearing the internal changes list.")
end

"""
__has_internal_change(version_or::VersionNumber, change_name::Symbol)
Some Julia packages have known dependencies on Julia internals (e.g. for introspection of
internal julia datastructures). To ease the co-development of such packages with julia,
a `change_name` is assigned on a best-effort basis or when explicitly requested.
This `change_name` can be used to probe whether or not the particular pre-release build of julia has
a particular change. In particular this function tests change scheduled for `version_or`
is present in our current julia build, either because our current version
is greater than `version_or` or because we're running a pre-release build that
includes the change.
Using this mechanism is a superior alternative to commit-number based `VERSION`
comparisons, which can be brittle during pre-release stages when there are multiple
actively developed branches.
The list of changes is cleared twice during the release process:
1. With the release of the first alpha
2. For the first release candidate
No new `change_name`s will be added during release candidates or bugfix releases
(so in particular on any released version, the list of changes will be empty and
`__has_internal_change` will always be equivalent to a version comparison.
# Example
Julia version `v"1.12.0-DEV.173"` changed the internal representation of line number debug info.
Several debugging packages have custom code to display this information and need to be changed
accordingly. In previous practice, this would often be accomplished with something like the following
```
@static if VERSION > v"1.12.0-DEV.173"
# Code to handle new format
else
# Code to handle old format
end
```
However, because such checks cannot be introduced until a VERSION number is assigned
(which also automatically pushes out the change to all nightly users), there was a builtin period
of breakage. With `__has_internal_change`, this can instead be written as:
```
@static if __has_internal_change(v"1.12-alpha", :invertedlinenames)
# Code to handle new format
else
# Code to handle old format
end
```
To find out the correct verrsion to use as the first argument, you may use
`Base.__next_removal_version`, which is set to the next version number in which
the list of changes will be cleared.
The primary advantage of this approach is that it allows a new version of the
package to be tagged and released *in advance* of the break on the nightly
build, thus ensuring continuity of package operation for nightly users.
!!! warning
This functionality is intended to help package developers which make use of
internal julia functionality. Doing so is explicitly discouraged unless absolutely
required and comes with the explicit understanding that the package will break.
In particular, this is not a generic feature-testing mechanism, but only a
simple, courtesy coordination mechanism for changes that are known (or found) to
be breaking a package depending on julia internals.
"""
function __has_internal_change(version_or::VersionNumber, change_name::Symbol)
VERSION > version_or && return true
change_name in __internal_changes_list
end
export __has_internal_change

# Deprecated functions and objects
#
# Please add new deprecations at the bottom of the file.
Expand Down

2 comments on commit 2cf469d

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Please sign in to comment.