Re: [PATCH V2] git-apply: add --allow-empty flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 27, 2021 at 10:08 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Jerry Zhang <jerry@xxxxxxxxxx> writes:
>
> > Some users or scripts will pipe "git diff"
> > output to "git apply" when replaying diffs
> > or commits. In these cases, they will rely
> > on the return value of "git apply" to know
> > whether the diff was applied successfully.
> >
> > However, for empty commits, "git apply" will
> > fail. This complicates scripts since they
> > have to either buffer the diff and check
> > its length, or run diff again with "exit-code",
> > essentially doing the diff twice.
> >
> > Add the "--allow-empty" flag to "git apply"
> > which allows it to handle both empty diffs
> > and empty commits created by "git format-patch
> > --always" by doing nothing and returning 0.
> >
> > Add tests for both with and without --allow-empty.
> >
> > Signed-off-by: Jerry Zhang <jerry@xxxxxxxxxx>
> > ---
> > This patch applies on top of "git-apply: add --quiet flag".
> > The conflict is in Documentation -> Synopsis and is
> > trivial to solve.
>
> > V1 -> V2:
> > - Moved behavior under a flag
> > - Added tests for both cases
>
> When people add a flag (a boolean option), it becomes tempting to
> add a corresponding configuration variable.
>
> I wonder it this step should start calling
>
>         git apply --no-allow-empty
>
> when "git am" drives it, so that a futre end-user configuration
> variable would not break it?
>
That's currently not necessary, since "git am" can catch empty patches
before it ever calls into apply.c:

     if (is_empty_or_missing_file(am_path(state, "patch"))) {
         printf_ln(_("Patch is empty."));
         die_user_resolve(state);
     }

If someone does make a config variable I think it might be convenient to
teach "git am" to use "allow-empty" as well, since applying empty commits
can be useful for documenting the history. The config variable could then
control the behavior of both "am" and "apply" since they are used for very
similar things. Of course that's up to the decision of that hypothetical person
since I don't currently have a need to change "am".



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux