[RFC/PATCH] WIP: add deprecation & experimental process/interface

[RFC/PATCH] WIP: add deprecation & experimental process/interface

[Ævar Arnfjörð Bjarmason]

This is the WIP start of a deprecation & experimental interface to
git. The goal is to formalize the workflow around deprecating
features, or around introducing new experimental features.

This is much more idea than code at the moment, but included is an
example showing how :/ might be deprecated[1] (let's not discuss /if/
we should do that here, this is just an example).

The plan, subject to RFC feedback is to:

 * Add a new config variable `core.version`. E.g. `core.version =
   2.14.0` With this the user can specify that they'd like
   new/experimental features introduced in that version (and below),
   as well as immediately getting new deprecations added in that
   version as errors.

   This is similar to perl's "use v<VERSION>".

 * Add a deprecated() function to to mark deprecated features.

   This will emit an arbitrary warning about the use of a feature once
   per-process (via static variable). The call needs to declare in
   what version the deprecation was added, and in what version it
   should start warning/dying.

   This sets up a well-defined path to deprecation, and allows users &
   packagers to plan upgrades, and e.g. set `core.version` to N
   versions in the future to see what would start warning/dying in
   those releases.

 * TODO: Add an experimental() function to mark experimental features.

   Depending on parameters & config an experimental feature might be
   off by default unless `core.use_experimental_<NAME> = true`, or it
   might warn unless that config is set.

 * TODO: Add new documentation (gitdeprecated.txt /
   gitexperimental.txt) aiming to exhaustively list deprecated &
   experimental features, and when it's planned that each of those
   will be fully removed or start/stop warning when used.

 * TODO: Subject to RFC feedback add a gitpolicy.txt similar to
   "perlpolicy" (mainly
   http://perldoc.perl.org/perlpolicy.html#BACKWARD-COMPATIBILITY-AND-DEPRECATION)
   describing how this deprecation/experimental process works. See
   also [2].

1. https://public-inbox.org/git/CACBZZX6K7ppVB0qYah76_+pjTKjsco3rHT0xRyKtF2H1dS4k_w@mail.gmail.com/
2. https://public-inbox.org/git/CACBZZX5oVKGZLKgS4aF0=XXtHO67ynS+zxSopDN9ErJGzV9n-A@mail.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 GIT-VERSION-GEN |  1 +
 Makefile        |  5 +++++
 deprecate.c     | 34 ++++++++++++++++++++++++++++++++++
 deprecate.h     |  7 +++++++
 sha1_name.c     |  3 +++
 5 files changed, 50 insertions(+)
 create mode 100644 deprecate.c
 create mode 100644 deprecate.h

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 4f94fc7574..c76bbedf86 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -37,4 +37,5 @@ fi
 test "$VN" = "$VC" || {
 	echo >&2 "GIT_VERSION = $VN"
 	echo "GIT_VERSION = $VN" >$GVF
+	echo "GIT_VERSION_INT = $(echo $VN | sed -e 's/^\([0-9]*\)\.\([0-9]*\)\..*/\1\2/')" >>$GVF
 }
diff --git a/Makefile b/Makefile
index e35542e631..1614b2b067 100644
--- a/Makefile
+++ b/Makefile
@@ -739,6 +739,7 @@ LIB_OBJS += csum-file.o
 LIB_OBJS += ctype.o
 LIB_OBJS += date.o
 LIB_OBJS += decorate.o
+LIB_OBJS += deprecate.o
 LIB_OBJS += diffcore-break.o
 LIB_OBJS += diffcore-delta.o
 LIB_OBJS += diffcore-order.o
@@ -1793,6 +1794,10 @@ version.sp version.s version.o: EXTRA_CPPFLAGS = \
 	'-DGIT_VERSION="$(GIT_VERSION)"' \
 	'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'
 
+deprecate.sp deprecate.s deprecate.o: GIT-VERSION-FILE
+deprecate.sp deprecate.s deprecate.o: EXTRA_CPPFLAGS = \
+	'-DGIT_VERSION_INT="$(GIT_VERSION_INT)"'
+
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
 	ln $< $@ 2>/dev/null || \
diff --git a/deprecate.c b/deprecate.c
new file mode 100644
index 0000000000..035a1adea1
--- /dev/null
+++ b/deprecate.c
@@ -0,0 +1,34 @@
+#include "cache.h"
+#include "deprecate.h"
+
+void deprecate(int *state, const char *message,
+		int dep_at, int warn_at, int die_at, int remove_at)
+{
+	/*
+	 * If we're going to warn let's do it once per-process, not
+	 * spew lots of warnings in a loop.
+	 */
+	if (*state == 1)
+		return;
+	else
+		*state = 1;
+
+	if (remove_at >= GIT_VERSION_INT) {
+		die("BUG: The '%s' deprecation should be removed in this release!");
+	} else if (die_at >= GIT_VERSION_INT) {
+		die(_("Deprecation error: %s"), message);
+	} else if (warn_at >= GIT_VERSION_INT) {
+		warning(_("Deprecation warning: %s"), message);
+	} else if (1) {
+		/*
+		 * TODO: Instead of `if 1` we should check a
+		 * core.version variable here.
+		 *
+		 * I.e. if set to core.version=2.13 the user is opting
+		 * in to get deprecations set at dep_at right away,
+		 * and also perhaps experimental features from a
+		 * sister experimental() interface.
+		 */
+		die(_("Early bird deprecation error: %s"), message);
+	}
+}
diff --git a/deprecate.h b/deprecate.h
new file mode 100644
index 0000000000..7d565ef0ed
--- /dev/null
+++ b/deprecate.h
@@ -0,0 +1,7 @@
+#ifndef DEPRECATE_H
+#define DEPRECATE_H
+
+extern void deprecate(int *state, const char *message,
+		      int dep_at, int warn_at, int die_at, int remove_at);
+
+#endif
diff --git a/sha1_name.c b/sha1_name.c
index 35c1e2a9e3..b381c39bd4 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1505,6 +1505,7 @@ static int get_sha1_with_context_1(const char *name,
 	int namelen = strlen(name);
 	const char *cp;
 	int only_to_die = flags & GET_SHA1_ONLY_TO_DIE;
+	static int dep_state = 0;
 
 	if (only_to_die)
 		flags |= GET_SHA1_QUIETLY;
@@ -1527,6 +1528,8 @@ static int get_sha1_with_context_1(const char *name,
 		char *new_path = NULL;
 		int pos;
 		if (!only_to_die && namelen > 2 && name[1] == '/') {
+			deprecate(&dep_state, _(":/<text> is deprecated. Use ^{/<text>} instead!"),
+				  213, 214, 215, 216);
 			struct commit_list *list = NULL;
 
 			for_each_ref(handle_one_ref, &list);
-- 
2.13.0.303.g4ebf302169

Re: [RFC/PATCH] WIP: add deprecation & experimental process/interface

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> The plan, subject to RFC feedback is to:
>
>  * Add a new config variable `core.version`. E.g. `core.version =
>    2.14.0` With this the user can specify that they'd like
>    new/experimental features introduced in that version (and below),
>    as well as immediately getting new deprecations added in that
>    version as errors.
>
>    This is similar to perl's "use v<VERSION>".

As long as this does not require very new version like v3.0.0 of Git
to pretend as if the user is still running an ancient v2.14.0, it is
a sensible proposal, I would think.

>  * Add a deprecated() function to to mark deprecated features.
>  * TODO: Add an experimental() function to mark experimental features.

Having burned by bitter experience during v1.6.0 transition, I do
welcome an attempt to make it easier to communicate deprecation
plans to end-users.

Re: [RFC/PATCH] WIP: add deprecation & experimental process/interface

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
> index 4f94fc7574..c76bbedf86 100755
> --- a/GIT-VERSION-GEN
> +++ b/GIT-VERSION-GEN
> @@ -37,4 +37,5 @@ fi
>  test "$VN" = "$VC" || {
>  	echo >&2 "GIT_VERSION = $VN"
>  	echo "GIT_VERSION = $VN" >$GVF
> +	echo "GIT_VERSION_INT = $(echo $VN | sed -e 's/^\([0-9]*\)\.\([0-9]*\)\..*/\1\2/')" >>$GVF
>  }

Unlike Perl's v1.2.3.4 notation, this forces us worry when we go
from v2.99.0 to v2.100.0 and eventually to v3.0, no?

> +	} else if (1) {
> +		/*
> +		 * TODO: Instead of `if 1` we should check a
> +		 * core.version variable here.
> +		 *
> +		 * I.e. if set to core.version=2.13 the user is opting
> +		 * in to get deprecations set at dep_at right away,
> +		 * and also perhaps experimental features from a
> +		 * sister experimental() interface.
> +		 */

This essentially forces us to always read _some_ configuration.
Some commands are meant to work outside repositories, so those who
want to affect them needs to write core.version in their global
configuration.  Some low-level plumbing commands may want to do
absolute minimum without configurablity.

I am not saying that it is absolutely a bad design decision to force
us to read some configuration (yet); it's just that it is something
that we have to keep in mind and always think about the
ramifications of.

> +		die(_("Early bird deprecation error: %s"), message);
> +	}
> +}

Re: [RFC/PATCH] WIP: add deprecation & experimental process/interface

[Duy Nguyen]

On Sat, May 27, 2017 at 6:10 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> This is the WIP start of a deprecation & experimental interface to
> git. The goal is to formalize the workflow around deprecating
> features, or around introducing new experimental features.
>
> This is much more idea than code at the moment, but included is an
> example showing how :/ might be deprecated[1] (let's not discuss /if/
> we should do that here, this is just an example).
>
> The plan, subject to RFC feedback is to:
>
>  * Add a new config variable `core.version`. E.g. `core.version =
>    2.14.0` With this the user can specify that they'd like
>    new/experimental features introduced in that version (and below),
>    as well as immediately getting new deprecations added in that
>    version as errors.

We have extensions.* for this purpose (or close to this purpose). I
think it's more flexible to go with extensions.* instead of a single
"core.version". extensions.* are non-optional though (if a git binary
does not understand it, the repo can't be accessed). So it's more
about fundamental experiments (like sha256 transition). I'm guessing
we can have a "soft" extensions (warn if not understand, instead of
die), like what we have in $GIT_DIR/index.

Deprecation via extension.* though may be unintuitive. But I think
something along that line (e.g. deprecation.*) might work.
-- 
Duy

Re: [RFC/PATCH] WIP: add deprecation & experimental process/interface

[Ævar Arnfjörð Bjarmason]

On Mon, May 29, 2017 at 3:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
>> index 4f94fc7574..c76bbedf86 100755
>> --- a/GIT-VERSION-GEN
>> +++ b/GIT-VERSION-GEN
>> @@ -37,4 +37,5 @@ fi
>>  test "$VN" = "$VC" || {
>>       echo >&2 "GIT_VERSION = $VN"
>>       echo "GIT_VERSION = $VN" >$GVF
>> +     echo "GIT_VERSION_INT = $(echo $VN | sed -e 's/^\([0-9]*\)\.\([0-9]*\)\..*/\1\2/')" >>$GVF
>>  }
>
> Unlike Perl's v1.2.3.4 notation, this forces us worry when we go
> from v2.99.0 to v2.100.0 and eventually to v3.0, no?

Yeah it's just a dirty hack to get that WIP working, although at this
rate it'll take us ~20 years to reach 3.0 if we go up to 99, and this
would purely be internal to the codebase.

I think it make sense for core.version to be e.g. 2.13, and parsed
internally to 2013, then we have room to go to 2.999 or ~200 years at
the current dev pace.

>> +     } else if (1) {
>> +             /*
>> +              * TODO: Instead of `if 1` we should check a
>> +              * core.version variable here.
>> +              *
>> +              * I.e. if set to core.version=2.13 the user is opting
>> +              * in to get deprecations set at dep_at right away,
>> +              * and also perhaps experimental features from a
>> +              * sister experimental() interface.
>> +              */
>
> This essentially forces us to always read _some_ configuration.
> Some commands are meant to work outside repositories, so those who
> want to affect them needs to write core.version in their global
> configuration.  Some low-level plumbing commands may want to do
> absolute minimum without configurablity.

Doesn't making sure that those codepaths just don't call
experimental() or deprecate() solve that issue? Presumably if
something is such low-level plumbing that it can't call deprecate() or
experimental() we'd just create a new incompatible command under a
different name if we'd like to change it.

Or are there some edge cases I'm missing?

> I am not saying that it is absolutely a bad design decision to force
> us to read some configuration (yet); it's just that it is something
> that we have to keep in mind and always think about the
> ramifications of.

*Nod*. It's definitely a bit of a chicken & egg problem, especially if
we ever wanted to have experimental or deprecated config-parsing
directives, but for most parts of the codebase it should be fine.

>> +             die(_("Early bird deprecation error: %s"), message);
>> +     }
>> +}

Re: [RFC/PATCH] WIP: add deprecation & experimental process/interface

[Ævar Arnfjörð Bjarmason]

On Mon, May 29, 2017 at 12:23 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sat, May 27, 2017 at 6:10 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> This is the WIP start of a deprecation & experimental interface to
>> git. The goal is to formalize the workflow around deprecating
>> features, or around introducing new experimental features.
>>
>> This is much more idea than code at the moment, but included is an
>> example showing how :/ might be deprecated[1] (let's not discuss /if/
>> we should do that here, this is just an example).
>>
>> The plan, subject to RFC feedback is to:
>>
>>  * Add a new config variable `core.version`. E.g. `core.version =
>>    2.14.0` With this the user can specify that they'd like
>>    new/experimental features introduced in that version (and below),
>>    as well as immediately getting new deprecations added in that
>>    version as errors.
>
> We have extensions.* for this purpose (or close to this purpose). I

From reading repository-version.txt it seems unrelated to what I'd
like to do. I.e. there you'd like to introduce a hard breakage and
it's already documented that if you encounter some extensions.* keys
you don't understand you *must not* proceed.

Whereas for this you'd like to e.g. turn on some experimental feature
in 2.16, but if you're running a 2.14 git you'd like it to just ignore
that config key it doesn't know about instead of git breaking.

> think it's more flexible to go with extensions.* instead of a single
> "core.version". extensions.* are non-optional though (if a git binary

I'd like there to be both, and the experimental() function would
define this on the source level, i.e. the call would include a
corresponding version where it was introduced, and a config key to
toggle it individually.

The reason to have both is that you can just upgrade git and say "I'm
not concerned about backcompat here, please give me all the latest
features" without having to exhaustively hunt down the list of things
we're shipping with that version.

> does not understand it, the repo can't be accessed). So it's more
> about fundamental experiments (like sha256 transition). I'm guessing
> we can have a "soft" extensions (warn if not understand, instead of
> die), like what we have in $GIT_DIR/index.

> Deprecation via extension.* though may be unintuitive. But I think
> something along that line (e.g. deprecation.*) might work.

Re: [RFC/PATCH] WIP: add deprecation & experimental process/interface

[Jeff King]

On Mon, May 29, 2017 at 01:20:02PM +0200, Ævar Arnfjörð Bjarmason wrote:

> >>  * Add a new config variable `core.version`. E.g. `core.version =
> >>    2.14.0` With this the user can specify that they'd like
> >>    new/experimental features introduced in that version (and below),
> >>    as well as immediately getting new deprecations added in that
> >>    version as errors.
> >
> > We have extensions.* for this purpose (or close to this purpose). I
> 
> From reading repository-version.txt it seems unrelated to what I'd
> like to do. I.e. there you'd like to introduce a hard breakage and
> it's already documented that if you encounter some extensions.* keys
> you don't understand you *must not* proceed.
> 
> Whereas for this you'd like to e.g. turn on some experimental feature
> in 2.16, but if you're running a 2.14 git you'd like it to just ignore
> that config key it doesn't know about instead of git breaking.

Right. repostoryformatversion (and extensions) is about the on-disk
format of a repo. If I understand correctly, this is about the user
specifying their preferred behaviors. Which is totally orthogonal.

The former must be set in the repo-level .git/config, and would
generally be set by Git itself when it writes a repo using that feature
(e.g., upcoming ref backends).  But this new thing would likely be set
in the ~/.gitconfig explicitly by the user, when they want to change the
behavior profile.

For that reason, I'd suggest using a name that is a little different
from "core.version", since it's easily missed up with
"core.repositoryformatversion". But that's a minor detail.

-Peff

Re: [RFC/PATCH] WIP: add deprecation & experimental process/interface

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>>  * Add a new config variable `core.version`. E.g. `core.version =
>>    2.14.0` With this the user can specify that they'd like
>>    new/experimental features introduced in that version (and below),
>>    as well as immediately getting new deprecations added in that
>>    version as errors.
>
> We have extensions.* for this purpose (or close to this purpose). I
> think it's more flexible to go with extensions.* instead of a single
> "core.version". extensions.* are non-optional though (if a git binary
> does not understand it, the repo can't be accessed). So it's more
> about fundamental experiments (like sha256 transition). I'm guessing
> we can have a "soft" extensions (warn if not understand, instead of
> die), like what we have in $GIT_DIR/index.
>
> Deprecation via extension.* though may be unintuitive. But I think
> something along that line (e.g. deprecation.*) might work.

The difference/orthogonal-ness of what Ævar wants to do and what the
extension mechanism wants to do has been covered by others correctly
so I won't repeat.  But I do agree with you that "run at this
version level" is probably less useful than a la carte "I want to
enroll in this and that experiment but not the other one".

I also agree with Peff that we should name it as distinctively as
the names used by extensions mechaism---we must strongly discourage
the latter from being futzed by the end users, while this opt-in
thing is very much open to them.