You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2020/02/28 23:09:51 UTC
Re: svn commit: r1874634 [3/3] - in
/subversion/branches/decouple-shelving-cli/subversion:
include/private/svn_client_shelf2.h libsvn_client/shelf2.c svn/shelf2-cmd.c
svn/shelf2-cmd.h svn/svn.c tests/cmdline/shelf2_tests.pyj
julianfoad@apache.org wrote on Fri, 28 Feb 2020 22:15 -0000:
> Author: julianfoad
> Date: Fri Feb 28 22:15:15 2020
> New Revision: 1874634
>
> URL: http://svn.apache.org/viewvc?rev=1874634&view=rev
> Log:
> Add the shelving v2 implementation from Subversion 1.11, as an alternative
> to the shelving v3 implementation from Subversion 1.12.
>
> They have substantially different pros and cons, so it is beneficial for the
> user to be able to choose.
>
> Make the shelving CLI version selectable by an environment variable:
> env. var. not set => shelving v3 enabled
> SVN_EXPERIMENTAL_COMMANDS=shelf3 => shelving v3 enabled
> SVN_EXPERIMENTAL_COMMANDS=shelf2 => shelving v2 enabled
> SVN_EXPERIMENTAL_COMMANDS= => no shelving CLI
>
> * subversion/svn/svn.c
> Enable shelving v3 or v2 or neither, depending on the environment variable
> SVN_EXPERIMENTAL_COMMANDS.
julianfoad@apache.org wrote on Fri, 28 Feb 2020 22:15 -0000:
> +++ subversion/branches/decouple-shelving-cli/subversion/svn/svn.c Fri Feb 28 22:15:15 2020
> @@ -2066,8 +2068,16 @@ sub_main(int *exit_code, int argc, const
> /* Init the temporary buffer. */
> svn_membuf__create(&buf, 0, pool);
>
> - /* Add externally defined commands */
> - add_commands(svn_cl__cmd_table_shelf3, pool);
> + /* Add experimental commands, if requested */
> + exp_cmds = getenv("SVN_EXPERIMENTAL_COMMANDS");
> + if (!exp_cmds || strstr(exp_cmds, "shelf3"))
> + {
> + add_commands(svn_cl__cmd_table_shelf3, pool);
> + }
> + else if (exp_cmds && strstr(exp_cmds, "shelf2"))
> + {
> + add_commands(svn_cl__cmd_table_shelf2, pool);
> + }
This doesn't seem forward-compatible: if in 1.15 we add
SVN_EXPERIMENTAL_COMMANDS=foo, using that environment variable setting
on 1.14 (assuming the above branch is merged by 1.14) would disable
shelving. I'm aware that compatibility is a lesser concern for experimental
features, but still, this sort of compatibility seems achievable.
TIMTOWTDI; for example, «os.putenv('SVN_EXPERIMENTAL_COMMANDS', 'shelf=3 foo=1')»,
or check «strstr(exp_cmds, "shelf")» in addition to whether exp_cmds is not NULL.
(I'm not sure whether you intend the envvar-based API to be released or
just to be an interim development phase, but I'm assuming the former
since the log message didn't say otherwise.)
Cheers,
Daniel
Re: svn commit: r1874634 [3/3] - in /subversion/branches/decouple-shelving-cli/subversion: include/private/svn_client_shelf2.h libsvn_client/shelf2.c svn/shelf2-cmd.c svn/shelf2-cmd.h svn/svn.c tests/cmdline/shelf2_tests.pyj
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Sat, 29 Feb 2020 08:12 +00:00:
>
> Daniel Shahaf wrote:
>
>
> >
> > This doesn't seem forward-compatible: if in 1.15 we add
> > SVN_EXPERIMENTAL_COMMANDS=foo, using that environment variable setting
> > on 1.14 (assuming the above branch is merged by 1.14) would disable
> > shelving. I'm aware that compatibility is a lesser concern for experimental
> > features, but still, this sort of compatibility seems achievable.
> >
> >
>
>
> I didn't want this commit to change the default, but I think it would
> be reasonable now to change the default to a simple opt-in (each
> feature is enabled iff the var contains its feature-name) for 1.14.
>
> WDYT?
Off the top of my head: that'd be a sensible design, but it's not the
only conceivable design that'd be sensible. For example, designs that
enable some or all experimental subcommands by default would be sensible
too, since those subcommands' names already have "x-" prefixes to warn
of their experimental status.
Cheers,
Daniel
Re: svn commit: r1874634 [3/3] - in /subversion/branches/decouple-shelving-cli/subversion: include/private/svn_client_shelf2.h libsvn_client/shelf2.c svn/shelf2-cmd.c svn/shelf2-cmd.h svn/svn.c tests/cmdline/shelf2_tests.pyj
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Sat, 29 Feb 2020 08:12 +00:00:
>
> Daniel Shahaf wrote:
>
>
> >
> > This doesn't seem forward-compatible: if in 1.15 we add
> > SVN_EXPERIMENTAL_COMMANDS=foo, using that environment variable setting
> > on 1.14 (assuming the above branch is merged by 1.14) would disable
> > shelving. I'm aware that compatibility is a lesser concern for experimental
> > features, but still, this sort of compatibility seems achievable.
> >
> >
>
>
> I didn't want this commit to change the default, but I think it would
> be reasonable now to change the default to a simple opt-in (each
> feature is enabled iff the var contains its feature-name) for 1.14.
>
> WDYT?
Off the top of my head: that'd be a sensible design, but it's not the
only conceivable design that'd be sensible. For example, designs that
enable some or all experimental subcommands by default would be sensible
too, since those subcommands' names already have "x-" prefixes to warn
of their experimental status.
Cheers,
Daniel
Re: svn commit: r1874634 [3/3] - in
/subversion/branches/decouple-shelving-cli/subversion:
include/private/svn_client_shelf2.h libsvn_client/shelf2.c svn/shelf2-cmd.c
svn/shelf2-cmd.h svn/svn.c tests/cmdline/shelf2_tests.pyj
Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
>
> This doesn't seem forward-compatible: if in 1.15 we add
> SVN_EXPERIMENTAL_COMMANDS=foo, using that environment variable setting
> on 1.14 (assuming the above branch is merged by 1.14) would disable
> shelving. I'm aware that compatibility is a lesser concern for experimental
> features, but still, this sort of compatibility seems achievable.
>
>
I didn't want this commit to change the default, but I think it would be reasonable now to change the default to a simple opt-in (each feature is enabled iff the var contains its feature-name) for 1.14.
WDYT?
- Julian
Re: svn commit: r1874634 [3/3] - in
/subversion/branches/decouple-shelving-cli/subversion:
include/private/svn_client_shelf2.h libsvn_client/shelf2.c svn/shelf2-cmd.c
svn/shelf2-cmd.h svn/svn.c tests/cmdline/shelf2_tests.pyj
Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
>
> This doesn't seem forward-compatible: if in 1.15 we add
> SVN_EXPERIMENTAL_COMMANDS=foo, using that environment variable setting
> on 1.14 (assuming the above branch is merged by 1.14) would disable
> shelving. I'm aware that compatibility is a lesser concern for experimental
> features, but still, this sort of compatibility seems achievable.
>
>
I didn't want this commit to change the default, but I think it would be reasonable now to change the default to a simple opt-in (each feature is enabled iff the var contains its feature-name) for 1.14.
WDYT?
- Julian