Commit 74fb1842 authored by Pierre-Yves David's avatar Pierre-Yves David 🐙
Browse files

phase: rename the requirement for internal-phase (BC)

The previous requirements covers both `internal` and `archived` phase.  However,
the `archived` phase is not ready for usage (while the internal one is mostly
ready for years). So we split the archived on in a dedicated requirements (see
previous changeset for details) and rename the one for internal-phase. This will
avoid older client trying to use the archived phase on `internal` only
repositories.

Since the requirements stayed experimental since its introduction. It seems
fine to drop the previous version.
parent 0c70d888a484
Pipeline #56370 passed with stage
in 44 minutes and 13 seconds
......@@ -1425,5 +1425,8 @@
default=False,
experimental=True,
)
# Moving this on by default means we are confident about the scaling of phases.
# This is not garanteed to be the case at the time this message is written.
coreconfigitem(
b'format',
......@@ -1428,6 +1431,6 @@
coreconfigitem(
b'format',
b'internal-phase',
b'use-internal-phase',
default=False,
experimental=True,
)
......
......@@ -3662,7 +3662,7 @@
requirements.discard(requirementsmod.REVLOGV1_REQUIREMENT)
requirements.add(requirementsmod.REVLOGV2_REQUIREMENT)
# experimental config: format.internal-phase
if ui.configbool(b'format', b'internal-phase'):
if ui.configbool(b'format', b'use-internal-phase'):
requirements.add(requirementsmod.INTERNAL_PHASE_REQUIREMENT)
# experimental config: format.exp-archived-phase
......
......@@ -29,7 +29,7 @@
# Enables the internal phase which is used to hide changesets instead
# of stripping them
INTERNAL_PHASE_REQUIREMENT = b'internal-phase'
INTERNAL_PHASE_REQUIREMENT = b'internal-phase-2'
# Enables the internal phase which is used to hide changesets instead
# of stripping them
......
......@@ -879,7 +879,7 @@
Check we deny its usage on older repository
$ hg init no-internal-phase --config format.internal-phase=no
$ hg init no-internal-phase --config format.use-internal-phase=no
$ cd no-internal-phase
$ hg debugrequires | grep internal-phase
[1]
......@@ -900,6 +900,6 @@
Check it works fine with repository that supports it.
$ hg init internal-phase --config format.internal-phase=yes
$ hg init internal-phase --config format.use-internal-phase=yes
$ cd internal-phase
$ hg debugrequires | grep internal-phase
......@@ -904,6 +904,6 @@
$ cd internal-phase
$ hg debugrequires | grep internal-phase
internal-phase
internal-phase-2
$ mkcommit A
test-debug-phase: new rev 0: x -> 1
test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256: -> draft
......
......@@ -14,7 +14,7 @@
$ cat <<EOF >> $HGRCPATH
> [format]
> internal-phase = yes
> use-internal-phase = yes
> EOF
#endif
......
......@@ -16,7 +16,7 @@
$ cat <<EOF >> $HGRCPATH
> [format]
> internal-phase = yes
> use-internal-phase = yes
> EOF
#endif
......
  • This change is causing us a great deal of consternation. As we've been migrating to internal-phase for the past ~2 mo, we're now reliant on format.internal-phase and also internal-phase as the name of the requirement. Moreover, these new names, particularly internal-phase-2, is debt-ridden and ugly. Can we consider simply reverting this change?

  • mentioned in commit d9f88448f8c8

    Toggle commit list
  • mentioned in merge request !230 (closed)

    Toggle commit list
  • These changes are important to be able to get internal phase out of experimental in the short term. I'm afraid that some of the fundamentals can't really go away. Mostly, if the requirements name wouldn't change, newer clients could create (on disk) repositories on which older clients could end up using the archived phase "by mistake" by accessing the very same files. The name change prevents that by gating to new clients that will understand the requirement as "use the internal phase only".

    The new name can be different if people have strong feelings about it, maybe internal-phase-only? phase-internal?).

    That being said, the goal is obviously not to cause you trouble and we should see what can be done here. For example, some config aliasing and some extra patches to automatically migrate the old requirement to the new one might be enough to ease the transition on your side. What do you think?

  • Mostly, if the requirements name wouldn't change, newer clients could create (on disk) repositories on which older clients could end up using the archived phase "by mistake" by accessing the very same files.

    I'd gotten the impression that the re-use of internal-phase for archive was an oversight and sparingly used and additionally protected by the cleanup-as-archived config. It seemed unlikely to me that someone would have cleanup-as-archive enabled, but internal-phase not (the one condition where older clients would end up using the archived phase).

    On the other hand, as you observed, the internal phase feature for shelve was fairly established. It was functional with just a few ux bugs (list, patch). In my opinion, it's far better potentially to break the archived feature than the more stable internal-phase. I realize now I maybe should have started with proposing to make internal-phase non-experimental before investing in it.

    Perhaps we could take this approach:

    1. Roll back internal-phase-2.
    2. While internal-phase is still experimental, if cleanup-as-archived is enabled but internal-phase is not and exp-archived-phase is not, warn that the user should disable cleanup-as-archived or enable exp-archived-phase.
    3. Make internal-phase non-experimental.
    4. After the warning above has had sufficient time to disseminate (1 yr?), enable internal-phase by default.

    This approach would cause a lot less disruption for repos that have already adopted the internal-phase for shelves, including designs and rollouts around this feature.

    I acknowledge that my proposal isn't completely safe. Someone with an old repo in the right configuration could miss the warning and end up with archived activated unintentionally.

    What about if instead of (3) and (4), jump straight to enabling internal-phase shelves by default? That is, start honoring internal-phase shelves without the requirement. Would that be possible?

    The new name can be different if people have strong feelings about it, maybe internal-phase-only? phase-internal?).

    Or maybe internal-phase-shelves or internal-phase-proper.

    For example, some config aliasing and some extra patches to automatically migrate the old requirement to the new one might be enough to ease the transition on your side. What do you think?

    We can probably manage that. The biggest uncertainty is around repos that already inherited the internal-phase requirement. Since this requirement no longer exists, I suspect those repos will become invalid, so it'll be important to remove internal-phase and replace it with internal-phase-{next}.

Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment