patchpush: allow atomic flag via configuration

>> Added a "push.atomic" option to git-config to allow site-specific
>> configuration of the atomic flag of git push
>
> I don't really use --atomic myself, but this seems like a reasonable
> thing to want, and the implementation looks cleanly done.
>
> My one question would be whether people would want this to actually be
> specific to a particular remote, and not just on for a given repository
> (your "site-specific" in the description made me think of that). In that
> case it would be better as part of the remote.* config.

> Jeff King <peff@peff.net> writes:
> 
> > My one question would be whether people would want this to actually be
> > specific to a particular remote, and not just on for a given repository
> > (your "site-specific" in the description made me think of that). In that
> > case it would be better as part of the remote.* config.
> 
> Yeah, I had the same reaction.  
> 
> Conceptually, this sits next to remote.*.push that defines which set
> of refs are sent by default, and remote.<name>.pushAtomic does make
> sense.  If (and only if) it turns out to be cumbersome for somebody
> to set the configuration for each and every remote, it is OK to also
> add push.atomic to serve as a fallback for remote.*.pushAtomic, I
> would think, but adding only push.atomic feels somewhat backwards.

>>> My one question would be whether people would want this to actually be
>>> specific to a particular remote, and not just on for a given repository
>>> (your "site-specific" in the description made me think of that). In that
>>> case it would be better as part of the remote.* config.
>>
>> Yeah, I had the same reaction.
>>
>> Conceptually, this sits next to remote.*.push that defines which set
>> of refs are sent by default, and remote.<name>.pushAtomic does make
>> sense.  If (and only if) it turns out to be cumbersome for somebody
>> to set the configuration for each and every remote, it is OK to also
>> add push.atomic to serve as a fallback for remote.*.pushAtomic, I
>> would think, but adding only push.atomic feels somewhat backwards.
>
> Thanks for your feedback
>
> I'm mostly using single remotes that's why I didn't even think of making
> it configurable per remote. But you're right that makes more sense.
>
> I'll try to make that modification to the patch.
>
> As for my use case: I'd like to use default atomic pushes when pushing a
> new tag among our stable branch, but inevitably forgetting to rebase
> beforehand. Therefore pushing a "dangling" commit/tag

> +push.atomic::
> +       If set to true enable `--atomic` option by default.  You
> +       may override this configuration at time of push by specifying
> +       `--no-atomic`.
> +

> +test_expect_success 'atomic option possible via git-config' '
> +       # prepare the repo
> +       mk_repo_pair &&
> +       (
> +               cd workbench &&
> +               test_commit one &&
> +               git checkout -b second master &&
> +               test_commit two &&
> +               git push --mirror up
> +       ) &&
> +       # a third party modifies the server side:
> +       (
> +               cd upstream &&
> +               git checkout second &&
> +               git tag test_tag second
> +       ) &&
> +       # see if we can now push both branches.
> +       (
> +               cd workbench &&
> +               git config push.atomic true &&
> +               git checkout master &&
> +               test_commit three &&
> +               git checkout second &&
> +               test_commit four &&
> +               git tag test_tag &&
> +               test_must_fail git push --tags up master second
> +       ) &&
> +       test_refs master HEAD@{3} &&
> +       test_refs second HEAD@{1}
> +'
> +

> On Fri, Mar 24, 2017 at 6:17 PM, Romuald Brunet <romuald@chivil.com> wrote:
>> +test_expect_success 'atomic option possible via git-config' '
>> +       # prepare the repo
>> +       mk_repo_pair &&
>> +       (
>> +               cd workbench &&
>> +               test_commit one &&
>> +               git checkout -b second master &&
>> +               test_commit two &&
>> +               git push --mirror up
>> +       ) &&
>> +       # a third party modifies the server side:
>> +       (
>> +               cd upstream &&
>> +               git checkout second &&
>> +               git tag test_tag second
>> +       ) &&
>> +       # see if we can now push both branches.
>> +       (
>> +               cd workbench &&
>> +               git config push.atomic true &&
>> +               git checkout master &&
>> +               test_commit three &&
>> +               git checkout second &&
>> +               test_commit four &&
>> +               git tag test_tag &&
>> +               test_must_fail git push --tags up master second
>> +       ) &&
>> +       test_refs master HEAD@{3} &&
>> +       test_refs second HEAD@{1}
>> +'
>> +
>
> Sent my earlier E-Mail too soon, so I missed this, there's no test
> here for what "git config push.atomic false" && "git push --atomic"
> does, is that atomic or not? I.e. what does "git -c push.atomic=false
> push --atomic" do? Does the CLI option override the config as it
> should?