[PATCH] for-each-repo: do nothing on empty config

From: Derrick Stolee <redacted>

'git for-each-repo --config=X' should return success without calling any
subcommands when the config key 'X' has no value. The current
implementation instead segfaults.

A user could run into this issue if they used 'git maintenance start' to
initialize their cron schedule using 'git for-each-repo
--config=maintenance.repo ...' but then using 'git maintenance
unregister' to remove the config option. (Note: 'git maintenance stop'
would remove the config _and_ remove the cron schedule.)

Add a simple test to ensure this works.

Reported-by: Andreas Bühmann <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
    for-each-repo: do nothing on empty config
    
    Thanks, Andreas, for drawing my attention to this bug.
    
    [1] https://github.com/gitgitgadget/git/issues/833

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-834%2Fderrickstolee%2Ffor-each-repo-empty-config-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-834/derrickstolee/for-each-repo-empty-config-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/834

 builtin/for-each-repo.c  | 3 +++
 t/t0068-for-each-repo.sh | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 5bba623ff12..9b9d2364f1a 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c

@@ -51,6 +51,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	values = repo_config_get_value_multi(the_repository,
 					     config_key);
 
+	if (!values)
+		return result;
+
 	for (i = 0; !result && i < values->nr; i++)
 		result = run_command_on_repo(values->items[i].string, &args);
 

diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index 136b4ec8392..adc1b81826a 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh

@@ -27,4 +27,8 @@ test_expect_success 'run based on configured value' '
 	grep again message
 '
 
+test_expect_success 'do nothing on empty config' '
+	git for-each-repo --config=bogus.config -- these args would fail
+'
+
 test_done

base-commit: 4950b2a2b5c4731e4c9d5b803739a6979b23fed6
-- 
gitgitgadget

On Tue, Jan 5, 2021 at 9:44 AM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

Probably not worth a re-roll, but the above has higher cognitive load than:

    if (!value)
        return 0;

which indicates clearly that the command succeeded, whereas `return
result` makes the reader scan all the code above the conditional to
figure out what values `result` could possibly hold.

The `these args would fail` arguments add mystery to the test,
prompting the reader to wonder what exactly is going on: "Fail how?",
"Is it supposed to fail?". It might be less confusing to use something
more direct like `nonexistent` or `does not exist`.

On 1/5/2021 12:55 PM, Eric Sunshine wrote:

On Tue, Jan 5, 2021 at 9:44 AM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

quoted

'git for-each-repo --config=X' should return success without calling any
subcommands when the config key 'X' has no value. The current
implementation instead segfaults.
[...]
Signed-off-by: Derrick Stolee <redacted>
---

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c

@@ -51,6 +51,9 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
        values = repo_config_get_value_multi(the_repository,
                                             config_key);
+       if (!values)
+               return result;

Probably not worth a re-roll, but the above has higher cognitive load than:

    if (!value)
        return 0;

which indicates clearly that the command succeeded, whereas `return
result` makes the reader scan all the code above the conditional to
figure out what values `result` could possibly hold.

True. Looking at this again, it might be better to just update the
loop to be

	for (i = 0; values && i < values->nr; i++) {

which would run the loop zero times when there are zero results.
 

quoted

diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh

@@ -27,4 +27,8 @@ test_expect_success 'run based on configured value' '
+test_expect_success 'do nothing on empty config' '
+       git for-each-repo --config=bogus.config -- these args would fail
+'

The `these args would fail` arguments add mystery to the test,
prompting the reader to wonder what exactly is going on: "Fail how?",
"Is it supposed to fail?". It might be less confusing to use something
more direct like `nonexistent` or `does not exist`.

I guess the point is that if we actually did try running a subcommand
on a repo (for instance, accidentally running it on the current repo)
then the command would fail when running "git these args would fail".

To me, "nonexistent" or "does not exist" doesn't adequately describe
this (hypothetical) situation.

Perhaps "fail-subcommand" might be better?

-Stolee

On Tue, Jan 5, 2021 at 9:20 PM Derrick Stolee [off-list ref] wrote:

On 1/5/2021 12:55 PM, Eric Sunshine wrote:

quoted

On Tue, Jan 5, 2021 at 9:44 AM Derrick Stolee via GitGitGadget
[off-list ref] wrote:
Probably not worth a re-roll, but the above has higher cognitive load than:

    if (!value)
        return 0;

which indicates clearly that the command succeeded, whereas `return
result` makes the reader scan all the code above the conditional to
figure out what values `result` could possibly hold.

True. Looking at this again, it might be better to just update the
loop to be

        for (i = 0; values && i < values->nr; i++) {

which would run the loop zero times when there are zero results.

I find the explicit `return 0` easier to reason about since I can see
immediately that it handles a particular boundary condition, after
which I no longer have to think about that condition as I continue
reading the code.

That said, I don't think it matters greatly one way or the other
whether you use `return result`, `return 0`, or adjust the loop
condition. It might matter if more functionality is added later, but
we don't have to worry about it now, and it's not worth re-rolling
just for this, or spending a lot of time discussing it.

quoted

quoted

+       git for-each-repo --config=bogus.config -- these args would fail

The `these args would fail` arguments add mystery to the test,
prompting the reader to wonder what exactly is going on: "Fail how?",
"Is it supposed to fail?". It might be less confusing to use something
more direct like `nonexistent` or `does not exist`.

I guess the point is that if we actually did try running a subcommand
on a repo (for instance, accidentally running it on the current repo)
then the command would fail when running "git these args would fail".

To me, "nonexistent" or "does not exist" doesn't adequately describe
this (hypothetical) situation.

Perhaps "fail-subcommand" might be better?

I had never even looked at git-for-each-repo before, so I took the
time to read the documentation and the source code before writing this
reply. Now that I understand what is supposed to happen, I might be
tempted to suggest `this-command-wont-be-run` as an argument, but
that's a mouthful. If you want to be really explicit that it should
fail if a bug gets re-introduced which causes the argument to be run
even though the config is empty, then perhaps use `false`:

    git for-each-repo --config=bogus.config -- false

By the way, when reading the documentation, some questions came to mind.

Is the behavior implemented by this patch desirable? That is, if the
user mistypes the name of the configuration variable, would it be
better to let the user know about the problem by returning an error
code rather than succeeding silently? Or do you foresee people
intentionally running the command against an empty config variable?
That might be legitimate in automation situations. If legitimate to
have an empty config, then would it be helpful to somehow distinguish
between an empty config variable and a non-existent one (assuming we
can even do that)?

Is the API of this command ideal? It feels odd to force the user to
specify required input via a command-line option rather than just as a
positional argument. In other words, since the config variable name is
mandatory, an alternative invocation format would be:

    git for-each-repo <config-var> <cmd>

Or do you foresee future enhancements expanding the ways in which the
repo list can be specified (such as from a file or stdin or directly
via the command-line), in which case the `--config` option makes
sense.

Derrick Stolee [off-list ref] writes:

quoted

Probably not worth a re-roll, but the above has higher cognitive load than:

    if (!value)
        return 0;

which indicates clearly that the command succeeded, whereas `return
result` makes the reader scan all the code above the conditional to
figure out what values `result` could possibly hold.

True. Looking at this again, it might be better to just update the
loop to be

	for (i = 0; values && i < values->nr; i++) {

which would run the loop zero times when there are zero results.

It is misleading, though.  It forces readers to first assume that
the loop body may update "values" from non-NULL to NULL in some
condition and hunt for such a code in vain.

If some clean-up starts to become needed after the loop, the "if the
value array is empty, immediately return 0" may have to be rewritten
to "if empty, jump to the clean-up part after the loop", but until
then, what Eric gave us would be the easiest to read, I would think.

To me, "nonexistent" or "does not exist" doesn't adequately describe
this (hypothetical) situation.

Perhaps "fail-subcommand" might be better?

I wonder if "false" or "exit 1" would fit the bill.  In any case, a
comment may help, perhaps?

	test_expect_success 'do nothing and succeed on empty/missing config' '
		# if this runs even once, "false" ensures a failure
		git for-each-repo --config=bogus.config -- false
	'

On 1/6/2021 3:05 AM, Junio C Hamano wrote:

Derrick Stolee [off-list ref] writes:

quoted

quoted

Probably not worth a re-roll, but the above has higher cognitive load than:

    if (!value)
        return 0;

which indicates clearly that the command succeeded, whereas `return
result` makes the reader scan all the code above the conditional to
figure out what values `result` could possibly hold.

True. Looking at this again, it might be better to just update the
loop to be

	for (i = 0; values && i < values->nr; i++) {

which would run the loop zero times when there are zero results.

It is misleading, though.  It forces readers to first assume that
the loop body may update "values" from non-NULL to NULL in some
condition and hunt for such a code in vain.

If some clean-up starts to become needed after the loop, the "if the
value array is empty, immediately return 0" may have to be rewritten
to "if empty, jump to the clean-up part after the loop", but until
then, what Eric gave us would be the easiest to read, I would think.

Ok. That works for me.

quoted

To me, "nonexistent" or "does not exist" doesn't adequately describe
this (hypothetical) situation.

Perhaps "fail-subcommand" might be better?

I wonder if "false" or "exit 1" would fit the bill.  In any case, a
comment may help, perhaps?

	test_expect_success 'do nothing and succeed on empty/missing config' '
		# if this runs even once, "false" ensures a failure
		git for-each-repo --config=bogus.config -- false
	'

I can add a comment, but keep in mind that this example would run the
subcommand as "git false". This isn't intended as an arbitrary script
runner, but a "please run the same Git command on a list of repos".

Thanks,
-Stolee

On 1/5/2021 11:20 PM, Eric Sunshine wrote:

On Tue, Jan 5, 2021 at 9:20 PM Derrick Stolee [off-list ref] wrote:

quoted

On 1/5/2021 12:55 PM, Eric Sunshine wrote:

quoted

On Tue, Jan 5, 2021 at 9:44 AM Derrick Stolee via GitGitGadget
[off-list ref] wrote:
Probably not worth a re-roll, but the above has higher cognitive load than:

    if (!value)
        return 0;

which indicates clearly that the command succeeded, whereas `return
result` makes the reader scan all the code above the conditional to
figure out what values `result` could possibly hold.

True. Looking at this again, it might be better to just update the
loop to be

        for (i = 0; values && i < values->nr; i++) {

which would run the loop zero times when there are zero results.

I find the explicit `return 0` easier to reason about since I can see
immediately that it handles a particular boundary condition, after
which I no longer have to think about that condition as I continue
reading the code.

That said, I don't think it matters greatly one way or the other
whether you use `return result`, `return 0`, or adjust the loop
condition. It might matter if more functionality is added later, but
we don't have to worry about it now, and it's not worth re-rolling
just for this, or spending a lot of time discussing it.

Junio made a good point that 'values' doesn't change during the loop,
so I shouldn't use it in the sentinel. I'll use your "return 0;"

quoted

quoted

quoted

+       git for-each-repo --config=bogus.config -- these args would fail

The `these args would fail` arguments add mystery to the test,
prompting the reader to wonder what exactly is going on: "Fail how?",
"Is it supposed to fail?". It might be less confusing to use something
more direct like `nonexistent` or `does not exist`.

I guess the point is that if we actually did try running a subcommand
on a repo (for instance, accidentally running it on the current repo)
then the command would fail when running "git these args would fail".

To me, "nonexistent" or "does not exist" doesn't adequately describe
this (hypothetical) situation.

Perhaps "fail-subcommand" might be better?

I had never even looked at git-for-each-repo before, so I took the
time to read the documentation and the source code before writing this
reply. Now that I understand what is supposed to happen, I might be
tempted to suggest `this-command-wont-be-run` as an argument, but
that's a mouthful. If you want to be really explicit that it should
fail if a bug gets re-introduced which causes the argument to be run
even though the config is empty, then perhaps use `false`:

    git for-each-repo --config=bogus.config -- false

I'm just going to repeat that this would run "git false". It achieves
the same goal where we interpret this as a failure if the subcommand
is run.

By the way, when reading the documentation, some questions came to mind.

Is the behavior implemented by this patch desirable? That is, if the
user mistypes the name of the configuration variable, would it be
better to let the user know about the problem by returning an error
code rather than succeeding silently? Or do you foresee people
intentionally running the command against an empty config variable?
That might be legitimate in automation situations. If legitimate to
have an empty config, then would it be helpful to somehow distinguish
between an empty config variable and a non-existent one (assuming we
can even do that)?

As mentioned in the message, this is the situation the background
maintenance would see if a user used 'git maintenance unregister' on
all of their repos or removed the 'maintenance.repo' config values
from their global config. I think it would be better to not start
failing the background commands.

Even outside of that context, we have no way to specify an "existing
but empty" multi-valued config value over a non-existing config
value. I'd rather prefer the interpretation "I succeeded in doing
nothing" instead of "I think you made a mistake, because there's
nothing to do."

Could we meet in the middle and print a warning to stderr?

	warning(_("supplied config key '%s' has no values"));

That would present a useful message to users running this command
manually (or constructing automation) without breaking scripts
that parse the output.

Is the API of this command ideal? It feels odd to force the user to
specify required input via a command-line option rather than just as a
positional argument. In other words, since the config variable name is
mandatory, an alternative invocation format would be:

    git for-each-repo <config-var> <cmd>

Or do you foresee future enhancements expanding the ways in which the
repo list can be specified (such as from a file or stdin or directly
via the command-line), in which case the `--config` option makes
sense.

I believe some voice interest in specifying a list of repositories
by providing a directory instead of a config value. That would
require scanning the immediate subdirectories to see if they are Git
repos.

A --stdin option would also be a good addition, if desired.

Since this command was intended for automation, not end-users, it
seemed that future-proofing the arguments in this way would be a good
idea. We want to remain flexible for future additions without
breaking compatibility.

Thanks,
-Stolee

On Wed, Jan 6, 2021 at 6:54 AM Derrick Stolee [off-list ref] wrote:

On 1/5/2021 11:20 PM, Eric Sunshine wrote:

quoted

Is the behavior implemented by this patch desirable? That is, if the
user mistypes the name of the configuration variable, would it be
better to let the user know about the problem by returning an error
code rather than succeeding silently? Or do you foresee people
intentionally running the command against an empty config variable?

As mentioned in the message, this is the situation the background
maintenance would see if a user used 'git maintenance unregister' on
all of their repos or removed the 'maintenance.repo' config values
from their global config. I think it would be better to not start
failing the background commands.

Even outside of that context, we have no way to specify an "existing
but empty" multi-valued config value over a non-existing config
value. I'd rather prefer the interpretation "I succeeded in doing
nothing" instead of "I think you made a mistake, because there's
nothing to do."

Could we meet in the middle and print a warning to stderr?

        warning(_("supplied config key '%s' has no values"));

That would present a useful message to users running this command
manually (or constructing automation) without breaking scripts
that parse the output.

I don't know. My knee-jerk response is that such a warning could get
annoying quickly if this is used mostly in an automated environment,
and an empty config value is likely to come up in practice. In that
case, I'm somewhat negative on adding the warning. (I could perhaps
see a --dry-run or --verbose mode issuing the above warning, but
that's outside the scope of this series.)

quoted

Is the API of this command ideal? It feels odd to force the user to
specify required input via a command-line option rather than just as a
positional argument. In other words, since the config variable name is
mandatory, an alternative invocation format would be:

    git for-each-repo <config-var> <cmd>

Or do you foresee future enhancements expanding the ways in which the
repo list can be specified (such as from a file or stdin or directly
via the command-line), in which case the `--config` option makes
sense.

I believe some voice interest in specifying a list of repositories
by providing a directory instead of a config value. That would
require scanning the immediate subdirectories to see if they are Git
repos.

A --stdin option would also be a good addition, if desired.

Since this command was intended for automation, not end-users, it
seemed that future-proofing the arguments in this way would be a good
idea. We want to remain flexible for future additions without
breaking compatibility.

Fair enough.

From: Derrick Stolee <redacted>

'git for-each-repo --config=X' should return success without calling any
subcommands when the config key 'X' has no value. The current
implementation instead segfaults.

A user could run into this issue if they used 'git maintenance start' to
initialize their cron schedule using 'git for-each-repo
--config=maintenance.repo ...' but then using 'git maintenance
unregister' to remove the config option. (Note: 'git maintenance stop'
would remove the config _and_ remove the cron schedule.)

Add a simple test to ensure this works.

Reported-by: Andreas Bühmann <redacted>
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Derrick Stolee <redacted>
---
    for-each-repo: do nothing on empty config
    
    Thanks, Andreas, for drawing my attention to this bug.
    
    [1] https://github.com/gitgitgadget/git/issues/833

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-834%2Fderrickstolee%2Ffor-each-repo-empty-config-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-834/derrickstolee/for-each-repo-empty-config-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/834

Range-diff vs v1:

 1:  36dccbb8c20 ! 1:  a1f1300bacb for-each-repo: do nothing on empty config
     @@ Commit message
          Add a simple test to ensure this works.
      
          Reported-by: Andreas Bühmann [off-list ref]
     +    Helped-by: Eric Sunshine [off-list ref]
          Signed-off-by: Derrick Stolee [off-list ref]
      
       ## builtin/for-each-repo.c ##
     @@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc, const char **argv, cons
       	values = repo_config_get_value_multi(the_repository,
       					     config_key);
       
     ++	/*
     ++	 * Do nothing on an empty list, which is equivalent to the case
     ++	 * where the config variable does not exist at all.
     ++	 */
      +	if (!values)
     -+		return result;
     ++		return 0;
      +
       	for (i = 0; !result && i < values->nr; i++)
       		result = run_command_on_repo(values->items[i].string, &args);
     @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
       '
       
      +test_expect_success 'do nothing on empty config' '
     -+	git for-each-repo --config=bogus.config -- these args would fail
     ++	# will fail if any subcommand is run
     ++	git for-each-repo --config=bogus.config -- false
      +'
      +
       test_done


 builtin/for-each-repo.c  | 7 +++++++
 t/t0068-for-each-repo.sh | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 5bba623ff12..52be64a4373 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c

@@ -51,6 +51,13 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	values = repo_config_get_value_multi(the_repository,
 					     config_key);
 
+	/*
+	 * Do nothing on an empty list, which is equivalent to the case
+	 * where the config variable does not exist at all.
+	 */
+	if (!values)
+		return 0;
+
 	for (i = 0; !result && i < values->nr; i++)
 		result = run_command_on_repo(values->items[i].string, &args);
 

diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index 136b4ec8392..c3efba4da0b 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh

@@ -27,4 +27,9 @@ test_expect_success 'run based on configured value' '
 	grep again message
 '
 
+test_expect_success 'do nothing on empty config' '
+	# will fail if any subcommand is run
+	git for-each-repo --config=bogus.config -- false
+'
+
 test_done

base-commit: 4950b2a2b5c4731e4c9d5b803739a6979b23fed6
-- 
gitgitgadget

From: Derrick Stolee <redacted>

'git for-each-repo --config=X' should return success without calling any
subcommands when the config key 'X' has no value. The current
implementation instead segfaults.

A user could run into this issue if they used 'git maintenance start' to
initialize their cron schedule using 'git for-each-repo
--config=maintenance.repo ...' but then using 'git maintenance
unregister' to remove the config option. (Note: 'git maintenance stop'
would remove the config _and_ remove the cron schedule.)

Add a simple test to ensure this works. Use 'git help --no-such-option'
as the potential subcommand to ensure that we will hit a failure if the
subcommand is ever run.

Reported-by: Andreas Bühmann <redacted>
Helped-by: Eric Sunshine [off-list ref]
Helped-by: Junio C Hamano [off-list ref]
Signed-off-by: Derrick Stolee <redacted>
---
    for-each-repo: do nothing on empty config
    
    Thanks, Andreas, for drawing my attention to this bug.
    
    V3: update comment in test and use git help --no-such-option as the
    subcommand to fail.
    
    [1] https://github.com/gitgitgadget/git/issues/833

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-834%2Fderrickstolee%2Ffor-each-repo-empty-config-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-834/derrickstolee/for-each-repo-empty-config-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/834

Range-diff vs v2:

 1:  a1f1300bacb ! 1:  31350c7845c for-each-repo: do nothing on empty config
     @@ Commit message
          unregister' to remove the config option. (Note: 'git maintenance stop'
          would remove the config _and_ remove the cron schedule.)
      
     -    Add a simple test to ensure this works.
     +    Add a simple test to ensure this works. Use 'git help --no-such-option'
     +    as the potential subcommand to ensure that we will hit a failure if the
     +    subcommand is ever run.
      
          Reported-by: Andreas Bühmann [off-list ref]
          Helped-by: Eric Sunshine [off-list ref]
     +    Helped-by: Junio C Hamano [off-list ref]
          Signed-off-by: Derrick Stolee [off-list ref]
      
       ## builtin/for-each-repo.c ##
     @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
       '
       
      +test_expect_success 'do nothing on empty config' '
     -+	# will fail if any subcommand is run
     -+	git for-each-repo --config=bogus.config -- false
     ++	# the whole thing would fail if for-each-ref iterated even
     ++	# once, because "git help --no-such-option" would fail
     ++	git for-each-repo --config=bogus.config -- help --no-such-option
      +'
      +
       test_done


 builtin/for-each-repo.c  | 7 +++++++
 t/t0068-for-each-repo.sh | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 5bba623ff12..52be64a4373 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c

@@ -51,6 +51,13 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	values = repo_config_get_value_multi(the_repository,
 					     config_key);
 
+	/*
+	 * Do nothing on an empty list, which is equivalent to the case
+	 * where the config variable does not exist at all.
+	 */
+	if (!values)
+		return 0;
+
 	for (i = 0; !result && i < values->nr; i++)
 		result = run_command_on_repo(values->items[i].string, &args);
 

diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index 136b4ec8392..4675e852517 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh

@@ -27,4 +27,10 @@ test_expect_success 'run based on configured value' '
 	grep again message
 '
 
+test_expect_success 'do nothing on empty config' '
+	# the whole thing would fail if for-each-ref iterated even
+	# once, because "git help --no-such-option" would fail
+	git for-each-repo --config=bogus.config -- help --no-such-option
+'
+
 test_done

base-commit: 4950b2a2b5c4731e4c9d5b803739a6979b23fed6
-- 
gitgitgadget