From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
Goal
~~~~
This is a patch series about libifying `git apply` functionality, and
using this libified functionality in `git am`, so that no 'git apply'
process is spawn anymore. This makes `git am` significantly faster, so
`git rebase`, when it uses the am backend, is also significantly
faster.
Previous discussions and patches series
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This has initially been discussed in the following thread:
http://thread.gmane.org/gmane.comp.version-control.git/287236/
Then the following patch series were sent:
RFC: http://thread.gmane.org/gmane.comp.version-control.git/288489/
v1: http://thread.gmane.org/gmane.comp.version-control.git/292324/
v2: http://thread.gmane.org/gmane.comp.version-control.git/294248/
v3: http://thread.gmane.org/gmane.comp.version-control.git/295429/
v4: http://thread.gmane.org/gmane.comp.version-control.git/296350/
v5: http://thread.gmane.org/gmane.comp.version-control.git/296490/
Highlevel view of the patches in the series
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This new patch series is built on top of the above previous work.
More precisely, this is "part 2" of the full patch series which is
built on top of the "part 1" of the full patch series. And as the
"part 1" is now in "next", this "part 2" is built on top of "next".
- Patches 01/44 to 33/44 were in v1 and v2.
They finish libifying the apply functionality that was in
builtin/apply.c and move it into apply.{c,h}. And they use this
libified functionality in git am so that it doesn't launch git apply
processes any more.
Following great suggestions from Eric and Junio, there are some
changes in these patches to improve on v2:
- Many commit messages for patches that make a function return
-1 were simplified by removing "by using error()".
- 'struct lockfile' instance should be managed properly, as
rollback_lock_file() should be called in all error paths.
- The patch that added calls to rollback_lock_file() has been
squashed into the patch that make apply_all_patches() return
-1 on error. The resulting patch is 13/44.
- 'struct apply_state' is now moved to apply.h at the beginning
of this series.
- Some useless braces were removed and the commit message was
fixed in patch 05/44.
- error_errno() is now used instead of error() in patch 03/44.
- Patches 34/44 to 43/44 were new in v2.
They implement a way to make the libified apply silent. It is a new
feature in the libified apply functionality.
This could be in a separate series, but unfortunately using the
libified apply in "git am" unmasks the fact that "git am", since it
was a shell script, has been silencing the apply functionality by
redirecting file descriptors to /dev/null and it looks like this is
not acceptable in C.
I am not yet sure that "be_silent" is a good name for the new
variable added by these patches.
Path 43/44 that adds --silent to `git apply` should probably be
discarded. I plan to do it in the next version.
I had planned to perhaps add tests for this new feature, but if 43/44
is discarded it may not be needed anymore.
- Patch 44/44 is new.
It replaces some calls to error() with calls to error_errno().
General comments
~~~~~~~~~~~~~~~~
Sorry if this patch series is still long. I can split it into two or
more series if it is prefered.
I can also send diffs between this version and the previous one, but
for now I'd rather not send them in this email, as it would make it
very long.
The benefits are not just related to not creating new processes. When
`git am` launched a `git apply` process, this new process had to read
the index from disk. Then after the `git apply`process had terminated,
`git am` dropped its index and read the index from disk to get the
index that had been modified by the `git apply`process. This was
inefficient and also prevented the split-index mechanism to provide
many performance benefits.
Using this series as rebase material, Duy explains it like this:
> Without the series, the picture is not so surprising. We run git-apply
> 80+ times, each consists of this sequence
>
> read index
> write index (cache tree updates only)
> read index again
> optionally initialize name hash (when new entries are added, I guess)
> read packed-refs
> write index
>
> With this series, we run a single git-apply which does
>
> read index (and sharedindex too if in split-index mode)
> initialize name hash
> write index 80+ times
(See: http://thread.gmane.org/gmane.comp.version-control.git/292324/focus=292460)
Links
~~~~~
This patch series is available here:
https://github.com/chriscool/git/commits/libify-apply-use-in-am65
The previous versions are available there:
v1: https://github.com/chriscool/git/commits/libify-apply-use-in-am25
v2: https://github.com/chriscool/git/commits/libify-apply-use-in-am54
Performance numbers
~~~~~~~~~~~~~~~~~~~
Only tests on Linux have been performed using a very early version of
this series. It could be interesting to test on other platforms
especially Windows and perhaps OSX too.
- Around mid April Ævar did a huge many-hundred commit rebase on the
kernel with untracked cache.
command: git rebase --onto 1993b17 52bef0c 29dde7c
Vanilla "next" without split index: 1m54.953s
Vanilla "next" with split index: 1m22.476s
This series on top of "next" without split index: 1m12.034s
This series on top of "next" with split index: 0m15.678s
Ævar used his Debian laptop with SSD.
- Around mid April I tested rebasing 13 commits in Booking.com's
monorepo on a Red Hat 6.5 server with split-index and
GIT_TRACE_PERFORMANCE=1.
With Git v2.8.0, the rebase took 6.375888383 s, with the git am
command launched by the rebase command taking 3.705677431 s.
With this series on top of next, the rebase took 3.044529494 s, with
the git am command launched by the rebase command taking 0.583521168
s.
Christian Couder (44):
apply: move 'struct apply_state' to apply.h
builtin/apply: make apply_patch() return -1 instead of die()ing
builtin/apply: read_patch_file() return -1 instead of die()ing
builtin/apply: make find_header() return -1 instead of die()ing
builtin/apply: make parse_chunk() return a negative integer on error
builtin/apply: make parse_single_patch() return -1 on error
builtin/apply: make parse_whitespace_option() return -1 instead of
die()ing
builtin/apply: make parse_ignorewhitespace_option() return -1 instead
of die()ing
builtin/apply: move init_apply_state() to apply.c
apply: make init_apply_state() return -1 instead of exit()ing
builtin/apply: make check_apply_state() return -1 instead of die()ing
builtin/apply: move check_apply_state() to apply.c
builtin/apply: make apply_all_patches() return -1 on error
builtin/apply: make parse_traditional_patch() return -1 on error
builtin/apply: make gitdiff_*() return 1 at end of header
builtin/apply: make gitdiff_*() return -1 on error
builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
builtin/apply: make build_fake_ancestor() return -1 on error
builtin/apply: make remove_file() return -1 on error
builtin/apply: make add_conflicted_stages_file() return -1 on error
builtin/apply: make add_index_file() return -1 on error
builtin/apply: make create_file() return -1 on error
builtin/apply: make write_out_one_result() return -1 on error
builtin/apply: make write_out_results() return -1 on error
builtin/apply: make try_create_file() return -1 on error
builtin/apply: make create_one_file() return -1 on error
builtin/apply: rename option parsing functions
apply: rename and move opt constants to apply.h
Move libified code from builtin/apply.c to apply.{c,h}
apply: make some parsing functions static again
run-command: make dup_devnull() non static
environment: add set_index_file()
builtin/am: use apply api in run_apply()
write_or_die: use warning() instead of fprintf(stderr, ...)
apply: add 'be_silent' variable to 'struct apply_state'
apply: make 'be_silent' incompatible with 'apply_verbosely'
apply: don't print on stdout when be_silent is set
usage: add set_warn_routine()
usage: add get_error_routine() and get_warn_routine()
apply: change error_routine when be_silent is set
am: use be_silent in 'struct apply_state' to shut up applying patches
run-command: make dup_devnull() static again
builtin/apply: add a cli option for be_silent
apply: use error_errno() where possible
Makefile | 1 +
apply.c | 4868 ++++++++++++++++++++++++++++++++++++++++++++++++
apply.h | 133 ++
builtin/am.c | 91 +-
builtin/apply.c | 4815 +----------------------------------------------
cache.h | 1 +
environment.c | 10 +
git-compat-util.h | 3 +
run-command.c | 2 +-
t/t4012-diff-binary.sh | 4 +-
t/t4254-am-corrupt.sh | 2 +-
usage.c | 15 +
write_or_die.c | 6 +-
13 files changed, 5132 insertions(+), 4819 deletions(-)
create mode 100644 apply.c
create mode 100644 apply.h
--
2.9.0.rc2.362.g3cd93d0
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors
to the caller instead of die()ing.
As a first step in this direction, let's make apply_patch() return
-1 in case of errors instead of dying. For now its only caller
apply_all_patches() will exit(1) when apply_patch() return -1.
In a later patch, apply_all_patches() will return -1 too instead of
exiting.
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 54 +++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 15 deletions(-)
@@ -4413,6 +4421,7 @@ static int apply_patch(struct apply_state *state,structstrbufbuf=STRBUF_INIT;/* owns the patch text */structpatch*list=NULL,**listp=&list;intskipped_patch=0;+intres=0;state->patch_input_file=filename;read_patch_file(&buf,fd);
@@ -4455,21 +4466,22 @@ static int apply_patch(struct apply_state *state,if(state->update_index&&state->newfd<0)state->newfd=hold_locked_index(state->lock_file,1);-if(state->check_index){-if(read_cache()<0)-die(_("unable to read index file"));+if(state->check_index&&read_cache()<0){+res=error(_("unable to read index file"));+gotoend;}if((state->check||state->apply)&&check_patch_list(state,list)<0&&-!state->apply_with_reject)-exit(1);+!state->apply_with_reject){+res=-1;+gotoend;+}if(state->apply&&write_out_results(state,list)){-if(state->apply_with_reject)-exit(1);/* with --3way, we still need to write the index out */-return1;+res=state->apply_with_reject?-1:1;+gotoend;}if(state->fake_ancestor)
@@ -4484,10 +4496,11 @@ static int apply_patch(struct apply_state *state,if(state->summary)summary_patch_list(list);+end:free_patch_list(list);strbuf_release(&buf);string_list_clear(&state->fn_table,0);-return0;+returnres;}staticvoidgit_apply_config(void)
@@ -4625,6 +4638,7 @@ static int apply_all_patches(struct apply_state *state,intoptions){inti;+intres;interrs=0;intread_stdin=1;
@@ -4633,7 +4647,10 @@ static int apply_all_patches(struct apply_state *state,intfd;if(!strcmp(arg,"-")){-errs|=apply_patch(state,0,"<stdin>",options);+res=apply_patch(state,0,"<stdin>",options);+if(res<0)+exit(1);+errs|=res;read_stdin=0;continue;}elseif(0<state->prefix_length)
@@ -4646,12 +4663,19 @@ static int apply_all_patches(struct apply_state *state,die_errno(_("can't open patch '%s'"),arg);read_stdin=0;set_default_whitespace_mode(state);-errs|=apply_patch(state,fd,arg,options);+res=apply_patch(state,fd,arg,options);+if(res<0)+exit(1);+errs|=res;close(fd);}set_default_whitespace_mode(state);-if(read_stdin)-errs|=apply_patch(state,0,"<stdin>",options);+if(read_stdin){+res=apply_patch(state,0,"<stdin>",options);+if(res<0)+exit(1);+errs|=res;+}if(state->whitespace_error){if(state->squelch_whitespace_errors&&
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_whitespace_option() should return -1 instead
of calling die().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we must make 'struct apply_state'
usable outside "builtin/apply.c".
Let's do that by creating a new "apply.h" and moving
'struct apply_state' there.
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
apply.h | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
builtin/apply.c | 98 +-----------------------------------------------------
2 files changed, 101 insertions(+), 97 deletions(-)
create mode 100644 apply.h
@@ -0,0 +1,100 @@+#ifndef APPLY_H+#define APPLY_H++enumws_error_action{+nowarn_ws_error,+warn_on_ws_error,+die_on_ws_error,+correct_ws_error+};++enumws_ignore{+ignore_ws_none,+ignore_ws_change+};++/*+*Weneedtokeeptrackofhowsymlinksinthepreimageare+*manipulatedbythepatches.Apatchtoadda/b/cwherea/b+*isasymlinkshouldnotbeallowedtoaffectthedirectory+*thesymlinkpointsat,butifthesamepatchremovesa/b,+*itisperfectlyfine,asthepatchremovesa/btomakeroom+*tocreateadirectorya/bsothata/b/ccanbecreated.+*+*Seealso"struct string_list symlink_changes"in"struct+*apply_state".+*/+#define SYMLINK_GOES_AWAY 01+#define SYMLINK_IN_RESULT 02++structapply_state{+constchar*prefix;+intprefix_length;++/* These are lock_file related */+structlock_file*lock_file;+intnewfd;++/* These control what gets looked at and modified */+intapply;/* this is not a dry-run */+intcached;/* apply to the index only */+intcheck;/* preimage must match working tree, don't actually apply */+intcheck_index;/* preimage must match the indexed version */+intupdate_index;/* check_index && apply */++/* These control cosmetic aspect of the output */+intdiffstat;/* just show a diffstat, and don't actually apply */+intnumstat;/* just show a numeric diffstat, and don't actually apply */+intsummary;/* just report creation, deletion, etc, and don't actually apply */++/* These boolean parameters control how the apply is done */+intallow_overlap;+intapply_in_reverse;+intapply_with_reject;+intapply_verbosely;+intno_add;+intthreeway;+intunidiff_zero;+intunsafe_paths;++/* Other non boolean parameters */+constchar*fake_ancestor;+constchar*patch_input_file;+intline_termination;+structstrbufroot;+intp_value;+intp_value_known;+unsignedintp_context;++/* Exclude and include path parameters */+structstring_listlimit_by_name;+inthas_include;++/* Various "current state" */+intlinenr;/* current line number */+structstring_listsymlink_changes;/* we have to track symlinks */++/*+*For"diff-stat"likebehaviour,wekeeptrackofthebiggestchange+*we'veseen,andthelongestfilename.Thatallowsustodosimple+*scaling.+*/+intmax_change;+intmax_len;++/*+*Recordsfilenamesthathavebeentouched,inordertohandle+*thecasewheremorethanonepatchestouchthesamefile.+*/+structstring_listfn_table;++/* These control whitespace errors */+enumws_error_actionws_error_action;+enumws_ignorews_ignore_action;+constchar*whitespace_option;+intwhitespace_error;+intsquelch_whitespace_errors;+intapplied_after_fixing_ws;+};++#endif
@@ -20,103 +20,7 @@#include"xdiff-interface.h"#include"ll-merge.h"#include"rerere.h"--enumws_error_action{-nowarn_ws_error,-warn_on_ws_error,-die_on_ws_error,-correct_ws_error-};---enumws_ignore{-ignore_ws_none,-ignore_ws_change-};--/*-*Weneedtokeeptrackofhowsymlinksinthepreimageare-*manipulatedbythepatches.Apatchtoadda/b/cwherea/b-*isasymlinkshouldnotbeallowedtoaffectthedirectory-*thesymlinkpointsat,butifthesamepatchremovesa/b,-*itisperfectlyfine,asthepatchremovesa/btomakeroom-*tocreateadirectorya/bsothata/b/ccanbecreated.-*-*Seealso"struct string_list symlink_changes"in"struct-*apply_state".-*/-#define SYMLINK_GOES_AWAY 01-#define SYMLINK_IN_RESULT 02--structapply_state{-constchar*prefix;-intprefix_length;--/* These are lock_file related */-structlock_file*lock_file;-intnewfd;--/* These control what gets looked at and modified */-intapply;/* this is not a dry-run */-intcached;/* apply to the index only */-intcheck;/* preimage must match working tree, don't actually apply */-intcheck_index;/* preimage must match the indexed version */-intupdate_index;/* check_index && apply */--/* These control cosmetic aspect of the output */-intdiffstat;/* just show a diffstat, and don't actually apply */-intnumstat;/* just show a numeric diffstat, and don't actually apply */-intsummary;/* just report creation, deletion, etc, and don't actually apply */--/* These boolean parameters control how the apply is done */-intallow_overlap;-intapply_in_reverse;-intapply_with_reject;-intapply_verbosely;-intno_add;-intthreeway;-intunidiff_zero;-intunsafe_paths;--/* Other non boolean parameters */-constchar*fake_ancestor;-constchar*patch_input_file;-intline_termination;-structstrbufroot;-intp_value;-intp_value_known;-unsignedintp_context;--/* Exclude and include path parameters */-structstring_listlimit_by_name;-inthas_include;--/* Various "current state" */-intlinenr;/* current line number */-structstring_listsymlink_changes;/* we have to track symlinks */--/*-*For"diff-stat"likebehaviour,wekeeptrackofthebiggestchange-*we'veseen,andthelongestfilename.Thatallowsustodosimple-*scaling.-*/-intmax_change;-intmax_len;--/*-*Recordsfilenamesthathavebeentouched,inordertohandle-*thecasewheremorethanonepatchestouchthesamefile.-*/-structstring_listfn_table;--/* These control whitespace errors */-enumws_error_actionws_error_action;-enumws_ignorews_ignore_action;-constchar*whitespace_option;-intwhitespace_error;-intsquelch_whitespace_errors;-intapplied_after_fixing_ws;-};+#include"apply.h"staticconstchar*constapply_usage[]={N_("git apply [<options>] [<patch>...]"),
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_single_patch() should return -1 instead of
calling die().
Let's do that by using error() and let's adjust the related test
cases accordingly.
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 17 +++++++++++++----
t/t4012-diff-binary.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
@@ -1666,6 +1666,10 @@ static int parse_fragment(struct apply_state *state,**The(fragment->patch,fragment->size)pairpointsintothememorygiven*bythecaller,notacopy,whenwereturn.+*+*Returns:+*-1incaseoferror,+*thenumberofbytesinthepatchotherwise.*/staticintparse_single_patch(structapply_state*state,constchar*line,
@@ -1683,8 +1687,10 @@ static int parse_single_patch(struct apply_state *state,fragment=xcalloc(1,sizeof(*fragment));fragment->linenr=state->linenr;len=parse_fragment(state,line,size,patch,fragment);-if(len<=0)-die(_("corrupt patch at line %d"),state->linenr);+if(len<=0){+free(fragment);+returnerror(_("corrupt patch at line %d"),state->linenr);+}fragment->patch=line;fragment->size=len;oldlines+=fragment->oldlines;
@@ -1720,9 +1726,9 @@ static int parse_single_patch(struct apply_state *state,patch->is_delete=0;if(0<patch->is_new&&oldlines)-die(_("new file %s depends on old contents"),patch->new_name);+returnerror(_("new file %s depends on old contents"),patch->new_name);if(0<patch->is_delete&&newlines)-die(_("deleted file %s still has contents"),patch->old_name);+returnerror(_("deleted file %s still has contents"),patch->old_name);if(!patch->is_delete&&!newlines&&context)fprintf_ln(stderr,_("** warning: "
@@ -2024,6 +2030,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long sisize-offset-hdrsize,patch);+if(patchsize<0)+return-1;+if(!patchsize){staticconstchargit_binary[]="GIT binary patch\n";inthd=hdrsize+offset;
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", init_apply_state() should return -1 instead of
calling exit().
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
apply.c | 11 ++++++-----
apply.h | 6 +++---
builtin/apply.c | 3 ++-
3 files changed, 11 insertions(+), 9 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, find_header() should return -1 instead of calling
die().
Unfortunately find_header() already returns -1 when no header is found,
so let's make it return -2 instead in this case.
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 33 ++++++++++++++++++++++-----------
t/t4254-am-corrupt.sh | 2 +-
2 files changed, 23 insertions(+), 12 deletions(-)
@@ -1419,6 +1419,14 @@ static int parse_fragment_header(const char *line, int len, struct fragment *frareturnoffset;}+/*+*Findfilediffheader+*+*Returns:+*-1incaseoferror+*-2ifnoheaderwasfound+*thesizeoftheheaderinbytes(called"offset")otherwise+*/staticintfind_header(structapply_state*state,constchar*line,unsignedlongsize,
@@ -1452,8 +1460,8 @@ static int find_header(struct apply_state *state,structfragmentdummy;if(parse_fragment_header(line,len,&dummy)<0)continue;-die(_("patch fragment without header at line %d: %.*s"),-state->linenr,(int)len-1,line);+returnerror(_("patch fragment without header at line %d: %.*s"),+state->linenr,(int)len-1,line);}if(size<len+6)
@@ -1469,18 +1477,18 @@ static int find_header(struct apply_state *state,continue;if(!patch->old_name&&!patch->new_name){if(!patch->def_name)-die(Q_("git diff header lacks filename information when removing "-"%d leading pathname component (line %d)",-"git diff header lacks filename information when removing "-"%d leading pathname components (line %d)",-state->p_value),-state->p_value,state->linenr);+returnerror(Q_("git diff header lacks filename information when removing "+"%d leading pathname component (line %d)",+"git diff header lacks filename information when removing "+"%d leading pathname components (line %d)",+state->p_value),+state->p_value,state->linenr);patch->old_name=xstrdup(patch->def_name);patch->new_name=xstrdup(patch->def_name);}if(!patch->is_delete&&!patch->new_name)-die("git diff header lacks filename information "-"(line %d)",state->linenr);+returnerror("git diff header lacks filename information "+"(line %d)",state->linenr);patch->is_toplevel_relative=1;*hdrsize=git_hdr_len;returnoffset;
@@ -1505,7 +1513,7 @@ static int find_header(struct apply_state *state,state->linenr+=2;returnoffset;}-return-1;+return-2;}staticvoidrecord_ws_error(structapply_state*state,
@@ -1996,6 +2004,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long siinthdrsize,patchsize;intoffset=find_header(state,buffer,size,&hdrsize,patch);+if(offset==-1)+exit(1);+if(offset<0)returnoffset;
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", gitdiff_*() functions should return -1 instead
of calling die().
A previous patch made it possible for gitdiff_*() functions to
return -1 in case of error. Let's take advantage of that to
make gitdiff_verify_name() return -1 on error, and to have
gitdiff_oldname() and gitdiff_newname() directly return
what gitdiff_verify_name() returns.
Helped-by: Nguyễn Thái Ngọc Duy [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
@@ -827,54 +827,56 @@ static int gitdiff_hdrend(struct apply_state *state,#define DIFF_OLD_NAME 0#define DIFF_NEW_NAME 1-staticvoidgitdiff_verify_name(structapply_state*state,-constchar*line,-intisnull,-char**name,-intside)+staticintgitdiff_verify_name(structapply_state*state,+constchar*line,+intisnull,+char**name,+intside){if(!*name&&!isnull){*name=find_name(state,line,NULL,state->p_value,TERM_TAB);-return;+return0;}if(*name){intlen=strlen(*name);char*another;if(isnull)-die(_("git apply: bad git-diff - expected /dev/null, got %s on line %d"),-*name,state->linenr);+returnerror(_("git apply: bad git-diff - expected /dev/null, got %s on line %d"),+*name,state->linenr);another=find_name(state,line,NULL,state->p_value,TERM_TAB);-if(!another||memcmp(another,*name,len+1))-die((side==DIFF_NEW_NAME)?+if(!another||memcmp(another,*name,len+1)){+free(another);+returnerror((side==DIFF_NEW_NAME)?_("git apply: bad git-diff - inconsistent new filename on line %d"):_("git apply: bad git-diff - inconsistent old filename on line %d"),state->linenr);+}free(another);}else{/* expect "/dev/null" */if(memcmp("/dev/null",line,9)||line[9]!='\n')-die(_("git apply: bad git-diff - expected /dev/null on line %d"),state->linenr);+returnerror(_("git apply: bad git-diff - expected /dev/null on line %d"),state->linenr);}++return0;}staticintgitdiff_oldname(structapply_state*state,constchar*line,structpatch*patch){-gitdiff_verify_name(state,line,-patch->is_new,&patch->old_name,-DIFF_OLD_NAME);-return0;+returngitdiff_verify_name(state,line,+patch->is_new,&patch->old_name,+DIFF_OLD_NAME);}staticintgitdiff_newname(structapply_state*state,constchar*line,structpatch*patch){-gitdiff_verify_name(state,line,-patch->is_delete,&patch->new_name,-DIFF_NEW_NAME);-return0;+returngitdiff_verify_name(state,line,+patch->is_delete,&patch->new_name,+DIFF_NEW_NAME);}staticintgitdiff_oldmode(structapply_state*state,
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
The gitdiff_*() functions that are called as p->fn() in parse_git_header()
should return 1 instead of -1 in case of end of header or unrecognized
input, as these are not real errors. It just instructs the parser to break
out.
This makes it possible for gitdiff_*() functions to return -1 in case of a
real error. This will be done in a following patch.
Helped-by: Nguyễn Thái Ngọc Duy [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", write_out_one_result() should just return what
remove_file() and create_file() are returning instead of calling
exit().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 38 ++++++++++++++++----------------------
1 file changed, 16 insertions(+), 22 deletions(-)
@@ -4276,36 +4276,29 @@ static int create_file(struct apply_state *state, struct patch *patch)}/* phase zero is to remove, phase one is to create */-staticvoidwrite_out_one_result(structapply_state*state,-structpatch*patch,-intphase)+staticintwrite_out_one_result(structapply_state*state,+structpatch*patch,+intphase){if(patch->is_delete>0){-if(phase==0){-if(remove_file(state,patch,1))-exit(1);-}-return;+if(phase==0)+returnremove_file(state,patch,1);+return0;}if(patch->is_new>0||patch->is_copy){-if(phase==1){-if(create_file(state,patch))-exit(1);-}-return;+if(phase==1)+returncreate_file(state,patch);+return0;}/**Renameormodificationboilsdowntothesame*thing:removetheold,writethenew*/-if(phase==0){-if(remove_file(state,patch,patch->is_rename))-exit(1);-}-if(phase==1){-if(create_file(state,patch))-exit(1);-}+if(phase==0)+returnremove_file(state,patch,patch->is_rename);+if(phase==1)+returncreate_file(state,patch);+return0;}staticintwrite_out_one_reject(structapply_state*state,structpatch*patch)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", die_on_unsafe_path() should return -1 using
error() instead of calling die(), so while doing that let's change
its name to check_unsafe_path().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", try_create_file() should return -1 in case of
error.
Unfortunately try_create_file() currently returns -1 to signal a
recoverable error. To fix that, let's make it return 1 in case of
a recoverable error and -1 in case of an unrecoverable error.
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
@@ -4139,38 +4139,45 @@ static int add_index_file(struct apply_state *state,return0;}+/*+*Returns:+*-1ifanunrecoverableerrorhappened+*0ifeverythingwentwell+*1ifarecoverableerrorhappened+*/staticinttry_create_file(constchar*path,unsignedintmode,constchar*buf,unsignedlongsize){-intfd;+intfd,res;structstrbufnbuf=STRBUF_INIT;if(S_ISGITLINK(mode)){structstatst;if(!lstat(path,&st)&&S_ISDIR(st.st_mode))return0;-returnmkdir(path,0777);+return!!mkdir(path,0777);}if(has_symlinks&&S_ISLNK(mode))/* Although buf:size is counted string, it also is NUL*terminated.*/-returnsymlink(buf,path);+return!!symlink(buf,path);fd=open(path,O_CREAT|O_EXCL|O_WRONLY,(mode&0100)?0777:0666);if(fd<0)-return-1;+return1;if(convert_to_working_tree(path,buf,size,&nbuf)){size=nbuf.len;buf=nbuf.buf;}-write_or_die(fd,buf,size);+res=!write_or_whine_pipe(fd,buf,size,path);strbuf_release(&nbuf);-if(close(fd)<0)-die_errno(_("closing file '%s'"),path);-return0;+if(close(fd)<0&&!res)+returnerror(_("closing file '%s': %s"),path,strerror(errno));++returnres?-1:0;}/*
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", build_fake_ancestor() should return -1 instead
of calling die().
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 41 ++++++++++++++++++++++++++---------------
1 file changed, 26 insertions(+), 15 deletions(-)
@@ -3889,11 +3889,12 @@ static int preimage_sha1_in_gitlink_patch(struct patch *p, unsigned char sha1[20}/* Build an index that contains the just the files needed for a 3way merge */-staticvoidbuild_fake_ancestor(structpatch*list,constchar*filename)+staticintbuild_fake_ancestor(structpatch*list,constchar*filename){structpatch*patch;structindex_stateresult={NULL};staticstructlock_filelock;+intres;/* Once we start supporting the reverse patch, it may be*worthshowingthenewsha1prefix,butuntilthen...
@@ -3911,31 +3912,38 @@ static void build_fake_ancestor(struct patch *list, const char *filename)if(!preimage_sha1_in_gitlink_patch(patch,sha1));/* ok, the textual part looks sane */else-die("sha1 information is lacking or useless for submodule %s",-name);+returnerror("sha1 information is lacking or "+"useless for submodule %s",name);}elseif(!get_sha1_blob(patch->old_sha1_prefix,sha1)){;/* ok */}elseif(!patch->lines_added&&!patch->lines_deleted){/* mode-only change: update the current */if(get_current_sha1(patch->old_name,sha1))-die("mode change for %s, which is not "-"in current HEAD",name);+returnerror("mode change for %s, which is not "+"in current HEAD",name);}else-die("sha1 information is lacking or useless "-"(%s).",name);+returnerror("sha1 information is lacking or useless "+"(%s).",name);ce=make_cache_entry(patch->old_mode,sha1,name,0,0);if(!ce)-die(_("make_cache_entry failed for path '%s'"),name);-if(add_index_entry(&result,ce,ADD_CACHE_OK_TO_ADD))-die("Could not add %s to temporary index",name);+returnerror(_("make_cache_entry failed for path '%s'"),+name);+if(add_index_entry(&result,ce,ADD_CACHE_OK_TO_ADD)){+free(ce);+returnerror("Could not add %s to temporary index",+name);+}}hold_lock_file_for_update(&lock,filename,LOCK_DIE_ON_ERROR);-if(write_locked_index(&result,&lock,COMMIT_LOCK))-die("Could not write temporary index to %s",filename);-+res=write_locked_index(&result,&lock,COMMIT_LOCK);discard_index(&result);++if(res)+returnerror("Could not write temporary index to %s",filename);++return0;}staticvoidstat_patch_list(structapply_state*state,structpatch*patch)
@@ -4476,8 +4484,11 @@ static int apply_patch(struct apply_state *state,gotoend;}-if(state->fake_ancestor)-build_fake_ancestor(list,state->fake_ancestor);+if(state->fake_ancestor&&+build_fake_ancestor(list,state->fake_ancestor)){+res=-1;+gotoend;+}if(state->diffstat)stat_patch_list(state,list);
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
As these functions are going to be part of the libified
apply api, let's give them a name that is more specific
to the apply api.
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
@@ -4583,9 +4583,9 @@ static int option_parse_include(const struct option *opt,return0;}-staticintoption_parse_p(conststructoption*opt,-constchar*arg,-intunset)+staticintapply_option_parse_p(conststructoption*opt,+constchar*arg,+intunset){structapply_state*state=opt->value;state->p_value=atoi(arg);
@@ -4593,8 +4593,8 @@ static int option_parse_p(const struct option *opt,return0;}-staticintoption_parse_space_change(conststructoption*opt,-constchar*arg,intunset)+staticintapply_option_parse_space_change(conststructoption*opt,+constchar*arg,intunset){structapply_state*state=opt->value;if(unset)
@@ -4604,8 +4604,8 @@ static int option_parse_space_change(const struct option *opt,return0;}-staticintoption_parse_whitespace(conststructoption*opt,-constchar*arg,intunset)+staticintapply_option_parse_whitespace(conststructoption*opt,+constchar*arg,intunset){structapply_state*state=opt->value;state->whitespace_option=arg;
@@ -4614,8 +4614,8 @@ static int option_parse_whitespace(const struct option *opt,return0;}-staticintoption_parse_directory(conststructoption*opt,-constchar*arg,intunset)+staticintapply_option_parse_directory(conststructoption*opt,+constchar*arg,intunset){structapply_state*state=opt->value;strbuf_reset(&state->root);
@@ -4729,13 +4729,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix)structoptionbuiltin_apply_options[]={{OPTION_CALLBACK,0,"exclude",&state,N_("path"),N_("don't apply changes matching the given path"),-0,option_parse_exclude},+0,apply_option_parse_exclude},{OPTION_CALLBACK,0,"include",&state,N_("path"),N_("apply changes matching the given path"),-0,option_parse_include},+0,apply_option_parse_include},{OPTION_CALLBACK,'p',NULL,&state,N_("num"),N_("remove <num> leading slashes from traditional diff paths"),-0,option_parse_p},+0,apply_option_parse_p},OPT_BOOL(0,"no-add",&state.no_add,N_("ignore additions made by the patch")),OPT_BOOL(0,"stat",&state.diffstat,
@@ -4767,13 +4767,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix)N_("ensure at least <n> lines of context match")),{OPTION_CALLBACK,0,"whitespace",&state,N_("action"),N_("detect new or modified lines that have whitespace errors"),-0,option_parse_whitespace},+0,apply_option_parse_whitespace},{OPTION_CALLBACK,0,"ignore-space-change",&state,NULL,N_("ignore changes in whitespace when finding context"),-PARSE_OPT_NOARG,option_parse_space_change},+PARSE_OPT_NOARG,apply_option_parse_space_change},{OPTION_CALLBACK,0,"ignore-whitespace",&state,NULL,N_("ignore changes in whitespace when finding context"),-PARSE_OPT_NOARG,option_parse_space_change},+PARSE_OPT_NOARG,apply_option_parse_space_change},OPT_BOOL('R',"reverse",&state.apply_in_reverse,N_("apply the patch in reverse")),OPT_BOOL(0,"unidiff-zero",&state.unidiff_zero,
@@ -4791,7 +4791,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix)RECOUNT),{OPTION_CALLBACK,0,"directory",&state,N_("root"),N_("prepend <root> to all filenames"),-0,option_parse_directory},+0,apply_option_parse_directory},OPT_END()};
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
The constants for the "inaccurate-eof" and the "recount" options will
be used in both "apply.c" and "builtin/apply.c", so they need to go
into "apply.h", and therefore they need a name that is more specific
to the API they belong to.
Signed-off-by: Christian Couder <redacted>
---
apply.h | 3 +++
builtin/apply.c | 11 ++++-------
2 files changed, 7 insertions(+), 7 deletions(-)
@@ -4480,8 +4477,8 @@ static int apply_patch(struct apply_state *state,intnr;patch=xcalloc(1,sizeof(*patch));-patch->inaccurate_eof=!!(options&INACCURATE_EOF);-patch->recount=!!(options&RECOUNT);+patch->inaccurate_eof=!!(options&APPLY_OPT_INACCURATE_EOF);+patch->recount=!!(options&APPLY_OPT_RECOUNT);nr=parse_chunk(state,buf.buf+offset,buf.len-offset,patch);if(nr<0){free_patch(patch);
@@ -4785,10 +4782,10 @@ int cmd_apply(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&state.apply_verbosely,N_("be verbose")),OPT_BIT(0,"inaccurate-eof",&options,N_("tolerate incorrectly detected missing new-line at the end of file"),-INACCURATE_EOF),+APPLY_OPT_INACCURATE_EOF),OPT_BIT(0,"recount",&options,N_("do not trust the line counts in the hunk headers"),-RECOUNT),+APPLY_OPT_RECOUNT),{OPTION_CALLBACK,0,"directory",&state,N_("root"),N_("prepend <root> to all filenames"),0,apply_option_parse_directory},
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", create_file() should just return what
add_conflicted_stages_file() and add_index_file() are returning
instead of calling exit().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
@@ -4258,7 +4258,7 @@ static int add_conflicted_stages_file(struct apply_state *state,return0;}-staticvoidcreate_file(structapply_state*state,structpatch*patch)+staticintcreate_file(structapply_state*state,structpatch*patch){char*path=patch->new_name;unsignedmode=patch->new_mode;
@@ -4269,13 +4269,10 @@ static void create_file(struct apply_state *state, struct patch *patch)mode=S_IFREG|0644;create_one_file(state,path,mode,buf,size);-if(patch->conflicted_threeway){-if(add_conflicted_stages_file(state,patch))-exit(1);-}else{-if(add_index_file(state,path,mode,buf,size))-exit(1);-}+if(patch->conflicted_threeway)+returnadd_conflicted_stages_file(state,patch);+else+returnadd_index_file(state,path,mode,buf,size);}/* phase zero is to remove, phase one is to create */
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", add_index_file() should return -1 instead of
calling die().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 48 +++++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 17 deletions(-)
@@ -4111,20 +4111,32 @@ static void add_index_file(struct apply_state *state,constchar*s;if(!skip_prefix(buf,"Subproject commit ",&s)||-get_sha1_hex(s,ce->sha1))-die(_("corrupt patch for submodule %s"),path);+get_sha1_hex(s,ce->sha1)){+free(ce);+returnerror(_("corrupt patch for submodule %s"),path);+}}else{if(!state->cached){-if(lstat(path,&st)<0)-die_errno(_("unable to stat newly created file '%s'"),-path);+if(lstat(path,&st)<0){+free(ce);+returnerror(_("unable to stat newly "+"created file '%s': %s"),+path,strerror(errno));+}fill_stat_cache_info(ce,&st);}-if(write_sha1_file(buf,size,blob_type,ce->sha1)<0)-die(_("unable to create backing store for newly created file %s"),path);+if(write_sha1_file(buf,size,blob_type,ce->sha1)<0){+free(ce);+returnerror(_("unable to create backing store "+"for newly created file %s"),path);+}}-if(add_cache_entry(ce,ADD_CACHE_OK_TO_ADD)<0)-die(_("unable to add cache entry for %s"),path);+if(add_cache_entry(ce,ADD_CACHE_OK_TO_ADD)<0){+free(ce);+returnerror(_("unable to add cache entry for %s"),path);+}++return0;}staticinttry_create_file(constchar*path,unsignedintmode,constchar*buf,unsignedlongsize)
@@ -4260,8 +4272,10 @@ static void create_file(struct apply_state *state, struct patch *patch)if(patch->conflicted_threeway){if(add_conflicted_stages_file(state,patch))exit(1);-}else-add_index_file(state,path,mode,buf,size);+}else{+if(add_index_file(state,path,mode,buf,size))+exit(1);+}}/* phase zero is to remove, phase one is to create */
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", create_one_file() should return -1 instead of
calling exit().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", write_out_results() should return -1 instead of
calling exit().
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
@@ -4498,10 +4506,17 @@ static int apply_patch(struct apply_state *state,gotoend;}-if(state->apply&&write_out_results(state,list)){-/* with --3way, we still need to write the index out */-res=state->apply_with_reject?-1:1;-gotoend;+if(state->apply){+intwrite_res=write_out_results(state,list);+if(write_res<0){+res=-1;+gotoend;+}+if(write_res>0){+/* with --3way, we still need to write the index out */+res=state->apply_with_reject?-1:1;+gotoend;+}}if(state->fake_ancestor&&
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
We will need this function in a later commit to redirect stdout
and stderr to /dev/null.
Helped-by: Johannes Sixt [off-list ref]
Helped-by: Johannes Schindelin [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
run-command.c | 2 +-
run-command.h | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
This variable should prevent anything to be printed on both stderr
and stdout.
Let's not take care of stdout and apply_verbosely for now though,
as that will be taken care of in following patches.
Signed-off-by: Christian Couder <redacted>
---
apply.c | 43 +++++++++++++++++++++++++++++--------------
apply.h | 1 +
2 files changed, 30 insertions(+), 14 deletions(-)
@@ -1808,7 +1809,7 @@ static int parse_single_patch(struct apply_state *state,returnerror(_("new file %s depends on old contents"),patch->new_name);if(0<patch->is_delete&&newlines)returnerror(_("deleted file %s still has contents"),patch->old_name);-if(!patch->is_delete&&!newlines&&context)+if(!patch->is_delete&&!newlines&&context&&!state->be_silent)fprintf_ln(stderr,_("** warning: ""file %s becomes empty but is not deleted"),
@@ -3031,8 +3032,8 @@ static int apply_one_fragment(struct apply_state *state,*Warnifitwasnecessarytoreducethenumber*ofcontextlines.*/-if((leading!=frag->leading)||-(trailing!=frag->trailing))+if((leading!=frag->leading||+trailing!=frag->trailing)&&!state->be_silent)fprintf_ln(stderr,_("Context reduced to (%ld/%ld)"" to apply fragment at %d"),leading,trailing,applied_pos+1);
@@ -3529,7 +3530,8 @@ static int try_threeway(struct apply_state *state,read_blob_object(&buf,pre_sha1,patch->old_mode))returnerror("repository lacks the necessary blob to fall back on 3-way merge.");-fprintf(stderr,"Falling back to three-way merge...\n");+if(!state->be_silent)+fprintf(stderr,"Falling back to three-way merge...\n");img=strbuf_detach(&buf,&len);prepare_image(&tmp_image,img,len,1);
@@ -3559,7 +3561,9 @@ static int try_threeway(struct apply_state *state,status=three_way_merge(image,patch->new_name,pre_sha1,our_sha1,post_sha1);if(status<0){-fprintf(stderr,"Failed to fall back on three-way merge...\n");+if(!state->be_silent)+fprintf(stderr,+"Failed to fall back on three-way merge...\n");returnstatus;}
@@ -3571,9 +3575,15 @@ static int try_threeway(struct apply_state *state,hashcpy(patch->threeway_stage[0].hash,pre_sha1);hashcpy(patch->threeway_stage[1].hash,our_sha1);hashcpy(patch->threeway_stage[2].hash,post_sha1);-fprintf(stderr,"Applied patch to '%s' with conflicts.\n",patch->new_name);+if(!state->be_silent)+fprintf(stderr,+"Applied patch to '%s' with conflicts.\n",+patch->new_name);}else{-fprintf(stderr,"Applied patch to '%s' cleanly.\n",patch->new_name);+if(!state->be_silent)+fprintf(stderr,+"Applied patch to '%s' cleanly.\n",+patch->new_name);}return0;}
@@ -4472,7 +4482,8 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)"Applying patch %%s with %d rejects...",cnt),cnt);-say_patch_name(stderr,sb.buf,patch);+if(!state->be_silent)+say_patch_name(stderr,sb.buf,patch);strbuf_release(&sb);cnt=strlen(patch->new_name);
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
It should be an error to have both be_silent and apply_verbosely set,
so let's check that in check_apply_state().
And by the way let's not automatically set apply_verbosely when
be_silent is set.
Signed-off-by: Christian Couder <redacted>
---
apply.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
@@ -122,8 +122,11 @@ int check_apply_state(struct apply_state *state, int force_apply)returnerror(_("--3way outside a repository"));state->check_index=1;}-if(state->apply_with_reject)-state->apply=state->apply_verbosely=1;+if(state->apply_with_reject){+state->apply=1;+if(!state->be_silent)+state->apply_verbosely=1;+}if(!force_apply&&(state->diffstat||state->numstat||state->summary||state->check||state->fake_ancestor))state->apply=0;if(state->check_index&&is_not_gitdir)
@@ -135,6 +138,8 @@ int check_apply_state(struct apply_state *state, int force_apply)}if(state->check_index)state->unsafe_paths=0;+if(state->be_silent&&state->apply_verbosely)+returnerror(_("incompatible internal 'be_silent' and 'apply_verbosely' flags"));if(!state->lock_file)returnerror("BUG: state->lock_file should not be NULL");
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
Let's make it possible to request a silent operation on the
command line.
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 2 ++
1 file changed, 2 insertions(+)
@@ -74,6 +74,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix)OPT_BOOL(0,"allow-overlap",&state.allow_overlap,N_("allow overlapping hunks")),OPT__VERBOSE(&state.apply_verbosely,N_("be verbose")),+OPT_BOOL(0,"silent",&state.be_silent,+N_("do not print any output")),OPT_BIT(0,"inaccurate-eof",&options,N_("tolerate incorrectly detected missing new-line at the end of file"),APPLY_OPT_INACCURATE_EOF),
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
There are already set_die_routine() and set_error_routine(),
so let's add set_warn_routine() as this will be needed in a
following commit.
Signed-off-by: Christian Couder <redacted>
---
git-compat-util.h | 1 +
usage.c | 5 +++++
2 files changed, 6 insertions(+)
@@ -3505,7 +3505,7 @@ static int load_current(struct apply_state *state,ce=active_cache[pos];if(lstat(name,&st)){if(errno!=ENOENT)-returnerror(_("%s: %s"),name,strerror(errno));+returnerror_errno("%s",name);if(checkout_target(&the_index,ce,&st))return-1;}
@@ -3664,7 +3664,7 @@ static int check_preimage(struct apply_state *state,}elseif(!state->cached){stat_ret=lstat(old_name,st);if(stat_ret&&errno!=ENOENT)-returnerror(_("%s: %s"),old_name,strerror(errno));+returnerror_errno("%s",old_name);}if(state->check_index&&!previous){
@@ -3686,7 +3686,7 @@ static int check_preimage(struct apply_state *state,}elseif(stat_ret<0){if(patch->is_new<0)gotois_new;-returnerror(_("%s: %s"),old_name,strerror(errno));+returnerror_errno("%s",old_name);}if(!state->cached&&!previous)
@@ -3745,7 +3745,7 @@ static int check_to_create(struct apply_state *state,returnEXISTS_IN_WORKTREE;}elseif((errno!=ENOENT)&&(errno!=ENOTDIR)){-returnerror("%s: %s",new_name,strerror(errno));+returnerror_errno("%s",new_name);}return0;}
@@ -4260,9 +4260,9 @@ static int add_index_file(struct apply_state *state,if(!state->cached){if(lstat(path,&st)<0){free(ce);-returnerror(_("unable to stat newly "-"created file '%s': %s"),-path,strerror(errno));+returnerror_errno(_("unable to stat newly "+"created file '%s'"),+path);}fill_stat_cache_info(ce,&st);}
@@ -4316,7 +4316,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,strbuf_release(&nbuf);if(close(fd)<0&&!res)-returnerror(_("closing file '%s': %s"),path,strerror(errno));+returnerror_errno(_("closing file '%s'"),path);returnres?-1:0;}
@@ -4386,8 +4386,8 @@ static int create_one_file(struct apply_state *state,++nr;}}-returnerror(_("unable to write file '%s' mode %o: %s"),-path,mode,strerror(errno));+returnerror_errno(_("unable to write file '%s' mode %o: %s"),+path,mode);}staticintadd_conflicted_stages_file(structapply_state*state,
@@ -4514,7 +4514,7 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)rej=fopen(namebuf,"w");if(!rej)-returnerror(_("cannot open %s: %s"),namebuf,strerror(errno));+returnerror_errno(_("cannot open %s"),namebuf);/* Normal git tools never deal with .rej, so do not pretend*thisisagitpatchbysaying--gitorgivingextended
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
Let's make it possible to get the current error_routine and warn_routine,
so that we can store them before using set_error_routine() or
set_warn_routine() to use new ones.
This way we will be able put back the original routines, when we are done
with using new ones.
Signed-off-by: Christian Couder <redacted>
---
git-compat-util.h | 2 ++
usage.c | 10 ++++++++++
2 files changed, 12 insertions(+)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
As there is no caller of dup_devnull() outside run-command.c any more,
let's make dup_devnull() static again.
Signed-off-by: Christian Couder <redacted>
---
run-command.c | 2 +-
run-command.h | 6 ------
2 files changed, 1 insertion(+), 7 deletions(-)
@@ -109,6 +109,11 @@ void clear_apply_state(struct apply_state *state)/* &state->fn_table is cleared at the end of apply_patch() */}+staticvoidmute_routine(constchar*bla,va_listparams)+{+/* do nothing */+}+intcheck_apply_state(structapply_state*state,intforce_apply){intis_not_gitdir=!startup_info->have_repository;
@@ -143,6 +148,13 @@ int check_apply_state(struct apply_state *state, int force_apply)if(!state->lock_file)returnerror("BUG: state->lock_file should not be NULL");+if(state->be_silent){+state->saved_error_routine=get_error_routine();+state->saved_warn_routine=get_warn_routine();+set_error_routine(mute_routine);+set_warn_routine(mute_routine);+}+return0;}
@@ -4760,6 +4772,7 @@ int apply_all_patches(struct apply_state *state,{inti;intres;+intretval=-1;interrs=0;intread_stdin=1;
@@ -4838,12 +4851,18 @@ int apply_all_patches(struct apply_state *state,state->newfd=-1;}-return!!errs;+retval=!!errs;rollback_end:if(state->newfd>=0){rollback_lock_file(state->lock_file);state->newfd=-1;}-return-1;++if(state->be_silent){+set_error_routine(state->saved_error_routine);+set_warn_routine(state->saved_warn_routine);+}++returnretval;}
@@ -89,6 +89,10 @@ struct apply_state {*/structstring_listfn_table;+/* This is to save some reporting routines */+void(*saved_error_routine)(constchar*err,va_listparams);+void(*saved_warn_routine)(constchar*warn,va_listparams);+/* These control whitespace errors */enumws_error_actionws_error_action;enumws_ignorews_ignore_action;
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
Some parsing functions that were used in both "apply.c" and
"builtin/apply.c" are now only used in the former, so they
can be made static to "apply.c".
Signed-off-by: Christian Couder <redacted>
---
apply.c | 6 +++---
apply.h | 5 -----
2 files changed, 3 insertions(+), 8 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
This replaces run_apply() implementation with a new one that
uses the apply api that has been previously prepared in
apply.c and apply.h.
This shoud improve performance a lot in certain cases.
As the previous implementation was creating a new `git apply`
process to apply each patch, it could be slow on systems like
Windows where it is costly to create new processes.
Also the new `git apply` process had to read the index from
disk, and when the process was done the calling process
discarded its own index and read back from disk the new
index that had been created by the `git apply` process.
This could be very inefficient with big repositories that
have big index files, especially when the system decided
that it was a good idea to run the `git apply` processes on
a different processor core.
Also eliminating index reads enables further performance
improvements by using:
`git update-index --split-index`
For example here is a benchmark of a multi hundred commit
rebase on the Linux kernel on a Debian laptop with SSD:
command: git rebase --onto 1993b17 52bef0c 29dde7c
Vanilla "next" without split index: 1m54.953s
Vanilla "next" with split index: 1m22.476s
This series on top of "next" without split index: 1m12.034s
This series on top of "next" with split index: 0m15.678s
(using branch "next" from mid April 2016.)
Benchmarked-by: Ævar Arnfjörð Bjarmason [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/am.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 86 insertions(+), 18 deletions(-)
@@ -1521,39 +1522,106 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)*/staticintrun_apply(conststructam_state*state,constchar*index_file){-structchild_processcp=CHILD_PROCESS_INIT;--cp.git_cmd=1;--if(index_file)-argv_array_pushf(&cp.env_array,"GIT_INDEX_FILE=%s",index_file);+structargv_arrayapply_paths=ARGV_ARRAY_INIT;+structargv_arrayapply_opts=ARGV_ARRAY_INIT;+structapply_stateapply_state;+intsave_stdout_fd,save_stderr_fd;+intres,opts_left;+char*save_index_file;+staticstructlock_filelock_file;++structoptionam_apply_options[]={+{OPTION_CALLBACK,0,"whitespace",&apply_state,N_("action"),+N_("detect new or modified lines that have whitespace errors"),+0,apply_option_parse_whitespace},+{OPTION_CALLBACK,0,"ignore-space-change",&apply_state,NULL,+N_("ignore changes in whitespace when finding context"),+PARSE_OPT_NOARG,apply_option_parse_space_change},+{OPTION_CALLBACK,0,"ignore-whitespace",&apply_state,NULL,+N_("ignore changes in whitespace when finding context"),+PARSE_OPT_NOARG,apply_option_parse_space_change},+{OPTION_CALLBACK,0,"directory",&apply_state,N_("root"),+N_("prepend <root> to all filenames"),+0,apply_option_parse_directory},+{OPTION_CALLBACK,0,"exclude",&apply_state,N_("path"),+N_("don't apply changes matching the given path"),+0,apply_option_parse_exclude},+{OPTION_CALLBACK,0,"include",&apply_state,N_("path"),+N_("apply changes matching the given path"),+0,apply_option_parse_include},+OPT_INTEGER('C',NULL,&apply_state.p_context,+N_("ensure at least <n> lines of context match")),+{OPTION_CALLBACK,'p',NULL,&apply_state,N_("num"),+N_("remove <num> leading slashes from traditional diff paths"),+0,apply_option_parse_p},+OPT_BOOL(0,"reject",&apply_state.apply_with_reject,+N_("leave the rejected hunks in corresponding *.rej files")),+OPT_END()+};/**Ifweareallowedtofallbackon3-waymerge,don'tgivefalse*errorsduringtheinitialattempt.*/+if(state->threeway&&!index_file){-cp.no_stdout=1;-cp.no_stderr=1;+save_stdout_fd=dup(1);+dup_devnull(1);+save_stderr_fd=dup(2);+dup_devnull(2);}-argv_array_push(&cp.args,"apply");+if(index_file){+save_index_file=get_index_file();+set_index_file((char*)index_file);+}-argv_array_pushv(&cp.args,state->git_apply_opts.argv);+if(init_apply_state(&apply_state,NULL,&lock_file))+die("init_apply_state() failed");++argv_array_push(&apply_opts,"apply");+argv_array_pushv(&apply_opts,state->git_apply_opts.argv);++opts_left=parse_options(apply_opts.argc,apply_opts.argv,+NULL,am_apply_options,NULL,0);++if(opts_left!=0)+die("unknown option passed thru to git apply");if(index_file)-argv_array_push(&cp.args,"--cached");+apply_state.cached=1;else-argv_array_push(&cp.args,"--index");+apply_state.check_index=1;-argv_array_push(&cp.args,am_path(state,"patch"));+if(check_apply_state(&apply_state,0))+die("check_apply_state() failed");-if(run_command(&cp))-return-1;+argv_array_push(&apply_paths,am_path(state,"patch"));-/* Reload index as git-apply will have modified it. */-discard_cache();-read_cache_from(index_file?index_file:get_index_file());+res=apply_all_patches(&apply_state,apply_paths.argc,apply_paths.argv,0);++/* Restore stdout and stderr */+if(state->threeway&&!index_file){+dup2(save_stdout_fd,1);+close(save_stdout_fd);+dup2(save_stderr_fd,2);+close(save_stderr_fd);+}++if(index_file)+set_index_file(save_index_file);++argv_array_clear(&apply_paths);+argv_array_clear(&apply_opts);++if(res)+returnres;++if(index_file){+/* Reload index as apply_all_patches() will have modified it. */+discard_cache();+read_cache_from(index_file);+}return0;}
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", remove_file() should return -1 instead of
calling die().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:51
Introduce set_index_file() to be able to temporarily change the index file.
It should be used like this:
/* Save current index file */
old_index_file = get_index_file();
set_index_file((char *)tmp_index_file);
/* Do stuff that will use tmp_index_file as the index file */
...
/* When finished reset the index file */
set_index_file(old_index_file);
Signed-off-by: Christian Couder <redacted>
---
cache.h | 1 +
environment.c | 10 ++++++++++
2 files changed, 11 insertions(+)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", add_conflicted_stages_file() should return -1
instead of calling die().
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", parse_traditional_patch() should return -1
instead of calling die().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
@@ -755,10 +755,10 @@ static int has_epoch_timestamp(const char *nameline)*files,wecanhappilychecktheindexforamatch,butforcreatinga*newfileweshouldtrytomatchwhatever"patch"does.Ihavenoidea.*/-staticvoidparse_traditional_patch(structapply_state*state,-constchar*first,-constchar*second,-structpatch*patch)+staticintparse_traditional_patch(structapply_state*state,+constchar*first,+constchar*second,+structpatch*patch){char*name;
@@ -803,7 +803,9 @@ static void parse_traditional_patch(struct apply_state *state,}}if(!name)-die(_("unable to find filename in patch at line %d"),state->linenr);+returnerror(_("unable to find filename in patch at line %d"),state->linenr);++return0;}staticintgitdiff_hdrend(structapply_state*state,
@@ -1462,7 +1464,8 @@ static int find_header(struct apply_state *state,continue;/* Ok, we'll consider it a patch */-parse_traditional_patch(state,line,line+len,patch);+if(parse_traditional_patch(state,line,line+len,patch))+return-1;*hdrsize=len+nextlen;state->linenr+=2;returnoffset;
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To libify `git apply` functionality we must make check_apply_state()
usable outside "builtin/apply.c".
Let's do that by moving it into "apply.c".
Signed-off-by: Christian Couder <redacted>
---
apply.c | 32 ++++++++++++++++++++++++++++++++
apply.h | 1 +
builtin/apply.c | 32 --------------------------------
3 files changed, 33 insertions(+), 32 deletions(-)
@@ -90,3 +90,35 @@ void clear_apply_state(struct apply_state *state)/* &state->fn_table is cleared at the end of apply_patch() */}++intcheck_apply_state(structapply_state*state,intforce_apply)+{+intis_not_gitdir=!startup_info->have_repository;++if(state->apply_with_reject&&state->threeway)+returnerror("--reject and --3way cannot be used together.");+if(state->cached&&state->threeway)+returnerror("--cached and --3way cannot be used together.");+if(state->threeway){+if(is_not_gitdir)+returnerror(_("--3way outside a repository"));+state->check_index=1;+}+if(state->apply_with_reject)+state->apply=state->apply_verbosely=1;+if(!force_apply&&(state->diffstat||state->numstat||state->summary||state->check||state->fake_ancestor))+state->apply=0;+if(state->check_index&&is_not_gitdir)+returnerror(_("--index outside a repository"));+if(state->cached){+if(is_not_gitdir)+returnerror(_("--cached outside a repository"));+state->check_index=1;+}+if(state->check_index)+state->unsafe_paths=0;+if(!state->lock_file)+returnerror("BUG: state->lock_file should not be NULL");++return0;+}
@@ -4541,38 +4541,6 @@ static int option_parse_directory(const struct option *opt,return0;}-staticintcheck_apply_state(structapply_state*state,intforce_apply)-{-intis_not_gitdir=!startup_info->have_repository;--if(state->apply_with_reject&&state->threeway)-returnerror("--reject and --3way cannot be used together.");-if(state->cached&&state->threeway)-returnerror("--cached and --3way cannot be used together.");-if(state->threeway){-if(is_not_gitdir)-returnerror(_("--3way outside a repository"));-state->check_index=1;-}-if(state->apply_with_reject)-state->apply=state->apply_verbosely=1;-if(!force_apply&&(state->diffstat||state->numstat||state->summary||state->check||state->fake_ancestor))-state->apply=0;-if(state->check_index&&is_not_gitdir)-returnerror(_("--index outside a repository"));-if(state->cached){-if(is_not_gitdir)-returnerror(_("--cached outside a repository"));-state->check_index=1;-}-if(state->check_index)-state->unsafe_paths=0;-if(!state->lock_file)-returnerror("BUG: state->lock_file should not be NULL");--return0;-}-staticintapply_all_patches(structapply_state*state,intargc,constchar**argv,
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To libify `git apply` functionality we must make init_apply_state()
usable outside "builtin/apply.c".
Let's do that by moving it into a new "apply.c".
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
Makefile | 1 +
apply.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
apply.h | 10 +++++++
builtin/apply.c | 88 -------------------------------------------------------
4 files changed, 102 insertions(+), 88 deletions(-)
create mode 100644 apply.c
@@ -0,0 +1,91 @@+#include"cache.h"+#include"lockfile.h"+#include"apply.h"++staticvoidgit_apply_config(void)+{+git_config_get_string_const("apply.whitespace",&apply_default_whitespace);+git_config_get_string_const("apply.ignorewhitespace",&apply_default_ignorewhitespace);+git_config(git_default_config,NULL);+}++intparse_whitespace_option(structapply_state*state,constchar*option)+{+if(!option){+state->ws_error_action=warn_on_ws_error;+return0;+}+if(!strcmp(option,"warn")){+state->ws_error_action=warn_on_ws_error;+return0;+}+if(!strcmp(option,"nowarn")){+state->ws_error_action=nowarn_ws_error;+return0;+}+if(!strcmp(option,"error")){+state->ws_error_action=die_on_ws_error;+return0;+}+if(!strcmp(option,"error-all")){+state->ws_error_action=die_on_ws_error;+state->squelch_whitespace_errors=0;+return0;+}+if(!strcmp(option,"strip")||!strcmp(option,"fix")){+state->ws_error_action=correct_ws_error;+return0;+}+returnerror(_("unrecognized whitespace option '%s'"),option);+}++intparse_ignorewhitespace_option(structapply_state*state,+constchar*option)+{+if(!option||!strcmp(option,"no")||+!strcmp(option,"false")||!strcmp(option,"never")||+!strcmp(option,"none")){+state->ws_ignore_action=ignore_ws_none;+return0;+}+if(!strcmp(option,"change")){+state->ws_ignore_action=ignore_ws_change;+return0;+}+returnerror(_("unrecognized whitespace ignore option '%s'"),option);+}++voidinit_apply_state(structapply_state*state,+constchar*prefix,+structlock_file*lock_file)+{+memset(state,0,sizeof(*state));+state->prefix=prefix;+state->prefix_length=state->prefix?strlen(state->prefix):0;+state->lock_file=lock_file;+state->newfd=-1;+state->apply=1;+state->line_termination='\n';+state->p_value=1;+state->p_context=UINT_MAX;+state->squelch_whitespace_errors=5;+state->ws_error_action=warn_on_ws_error;+state->ws_ignore_action=ignore_ws_none;+state->linenr=1;+strbuf_init(&state->root,0);++git_apply_config();+if(apply_default_whitespace&&parse_whitespace_option(state,apply_default_whitespace))+exit(1);+if(apply_default_ignorewhitespace&&parse_ignorewhitespace_option(state,apply_default_ignorewhitespace))+exit(1);+}++voidclear_apply_state(structapply_state*state)+{+string_list_clear(&state->limit_by_name,0);+string_list_clear(&state->symlink_changes,0);+strbuf_release(&state->root);++/* &state->fn_table is cleared at the end of apply_patch() */+}
@@ -4594,41 +4541,6 @@ static int option_parse_directory(const struct option *opt,return0;}-staticvoidinit_apply_state(structapply_state*state,-constchar*prefix,-structlock_file*lock_file)-{-memset(state,0,sizeof(*state));-state->prefix=prefix;-state->prefix_length=state->prefix?strlen(state->prefix):0;-state->lock_file=lock_file;-state->newfd=-1;-state->apply=1;-state->line_termination='\n';-state->p_value=1;-state->p_context=UINT_MAX;-state->squelch_whitespace_errors=5;-state->ws_error_action=warn_on_ws_error;-state->ws_ignore_action=ignore_ws_none;-state->linenr=1;-strbuf_init(&state->root,0);--git_apply_config();-if(apply_default_whitespace&&parse_whitespace_option(state,apply_default_whitespace))-exit(1);-if(apply_default_ignorewhitespace&&parse_ignorewhitespace_option(state,apply_default_ignorewhitespace))-exit(1);-}--staticvoidclear_apply_state(structapply_state*state)-{-string_list_clear(&state->limit_by_name,0);-string_list_clear(&state->symlink_changes,0);-strbuf_release(&state->root);--/* &state->fn_table is cleared at the end of apply_patch() */-}-staticvoidcheck_apply_state(structapply_state*state,intforce_apply){intis_not_gitdir=!startup_info->have_repository;
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", check_apply_state() should return -1 instead of
calling die().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
@@ -4541,17 +4541,17 @@ static int option_parse_directory(const struct option *opt,return0;}-staticvoidcheck_apply_state(structapply_state*state,intforce_apply)+staticintcheck_apply_state(structapply_state*state,intforce_apply){intis_not_gitdir=!startup_info->have_repository;if(state->apply_with_reject&&state->threeway)-die("--reject and --3way cannot be used together.");+returnerror("--reject and --3way cannot be used together.");if(state->cached&&state->threeway)-die("--cached and --3way cannot be used together.");+returnerror("--cached and --3way cannot be used together.");if(state->threeway){if(is_not_gitdir)-die(_("--3way outside a repository"));+returnerror(_("--3way outside a repository"));state->check_index=1;}if(state->apply_with_reject)
@@ -4559,16 +4559,18 @@ static void check_apply_state(struct apply_state *state, int force_apply)if(!force_apply&&(state->diffstat||state->numstat||state->summary||state->check||state->fake_ancestor))state->apply=0;if(state->check_index&&is_not_gitdir)-die(_("--index outside a repository"));+returnerror(_("--index outside a repository"));if(state->cached){if(is_not_gitdir)-die(_("--cached outside a repository"));+returnerror(_("--cached outside a repository"));state->check_index=1;}if(state->check_index)state->unsafe_paths=0;if(!state->lock_file)-die("BUG: state->lock_file should not be NULL");+returnerror("BUG: state->lock_file should not be NULL");++return0;}staticintapply_all_patches(structapply_state*state,
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To finish libifying the apply functionality, apply_all_patches() should not
die() or exit() in case of error, but return -1.
While doing that we must take care that file descriptors are properly closed
and, if needed, reset a sensible value.
Also, according to the lockfile API, when finished with a lockfile, one
should either commit it or roll it back.
This is even more important now that the same lockfile can be passed
to init_apply_state() many times to be reused by series of calls to
the apply lib functions.
Helped-by: Eric Sunshine [off-list ref]
Helped-by: Nguyễn Thái Ngọc Duy [off-list ref]
Helped-by: Johannes Schindelin [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
@@ -4614,12 +4618,22 @@ static int apply_all_patches(struct apply_state *state,}if(state->update_index){-if(write_locked_index(&the_index,state->lock_file,COMMIT_LOCK))-die(_("Unable to write new index file"));+res=write_locked_index(&the_index,state->lock_file,COMMIT_LOCK);+if(res){+error(_("Unable to write new index file"));+gotorollback_end;+}state->newfd=-1;}return!!errs;++rollback_end:+if(state->newfd>=0){+rollback_lock_file(state->lock_file);+state->newfd=-1;+}+return-1;}intcmd_apply(intargc,constchar**argv,constchar*prefix)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing. Let's do that by returning -1 instead of
die()ing in read_patch_file().
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", parse_ignorewhitespace_option() should return
-1 instead of calling die().
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing or exit()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_chunk() should return -1 instead of calling
die() or exit().
As parse_chunk() is called only by apply_patch() which already
returns -1 when an error happened, let's make apply_patch() return -1
when parse_chunk() returns -1.
If find_header() returns -2 because no patch header has been found, it
is ok for parse_chunk() to also return -2. If find_header() returns -1
because an error happened, it is ok for parse_chunk() to do the same.
Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
@@ -2067,7 +2067,7 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si*/if((state->apply||state->check)&&(!patch->is_binary&&!metadata_changes(patch)))-die(_("patch with only garbage at line %d"),state->linenr);+returnerror(_("patch with only garbage at line %d"),state->linenr);}returnoffset+hdrsize+patchsize;
@@ -4449,6 +4449,10 @@ static int apply_patch(struct apply_state *state,nr=parse_chunk(state,buf.buf+offset,buf.len-offset,patch);if(nr<0){free_patch(patch);+if(nr==-1){+res=-1;+gotoend;+}break;}if(state->apply_in_reverse)
From: René Scharfe <hidden> Date: 2016-06-16 02:19:52
Am 10.06.2016 um 22:11 schrieb Christian Couder:
quoted hunk
Let's make it possible to request a silent operation on the
command line.
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 2 ++
1 file changed, 2 insertions(+)
@@ -74,6 +74,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix)OPT_BOOL(0,"allow-overlap",&state.allow_overlap,N_("allow overlapping hunks")),OPT__VERBOSE(&state.apply_verbosely,N_("be verbose")),+OPT_BOOL(0,"silent",&state.be_silent,+N_("do not print any output")),OPT_BIT(0,"inaccurate-eof",&options,N_("tolerate incorrectly detected missing new-line at the end of file"),APPLY_OPT_INACCURATE_EOF),
Why not -q/--quiet as for most other commands?
Furthermore, you could use OPT__VERBOSITY, which causes -v and -q to
update the same variable variable and thus lets parseopt handle their
interaction. Perhaps verbosity == 1 could mean verbose, 0 normal, -1 no
infos, -2 no warnings and -3 no errors?
And if you add the ability to silence the apply functions before using
them you don't have to export and unexport dup_devnull().
René
I'm not adding arguments to what we've heard on whether to use /dev/null
in this series or not. But if the outcome is to keep this patch, please
remove the #ifndef that we see in the context lines (and the matching
#endif), too. Otherwise, the build fails on Windows for each patch in
the series until this change is reverted in patch 42/44.
-- Hannes
From: Christian Couder <hidden> Date: 2016-06-16 02:19:52
On Fri, Jun 10, 2016 at 10:59 PM, René Scharfe [off-list ref] wrote:
Am 10.06.2016 um 22:11 schrieb Christian Couder:
quoted
Let's make it possible to request a silent operation on the
command line.
Signed-off-by: Christian Couder <redacted>
---
builtin/apply.c | 2 ++
1 file changed, 2 insertions(+)
*prefix)
OPT_BOOL(0, "allow-overlap", &state.allow_overlap,
N_("allow overlapping hunks")),
OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
+ OPT_BOOL(0, "silent", &state.be_silent,
+ N_("do not print any output")),
OPT_BIT(0, "inaccurate-eof", &options,
N_("tolerate incorrectly detected missing new-line
at the end of file"),
APPLY_OPT_INACCURATE_EOF),
Why not -q/--quiet as for most other commands?
First as I say in the cover letter, I am going to discard this patch.
That is because it is not necessary, and it appeared a bit
controversial whether I should use -q/--quiet or not in the v2
discussions.
Furthermore, you could use OPT__VERBOSITY, which causes -v and -q to update
the same variable variable and thus lets parseopt handle their interaction.
Perhaps verbosity == 1 could mean verbose, 0 normal, -1 no infos, -2 no
warnings and -3 no errors?
Yeah, that could be done.
And if you add the ability to silence the apply functions before using them
you don't have to export and unexport dup_devnull().
Yeah, I will do something like that in the next version.
Thanks,
Christian.
I'm not adding arguments to what we've heard on whether to use /dev/null in
this series or not. But if the outcome is to keep this patch, please remove
the #ifndef that we see in the context lines (and the matching #endif), too.
Otherwise, the build fails on Windows for each patch in the series until
this change is reverted in patch 42/44.
Ok, I will find a way to avoid that build failure, though I don't test
on Windows.
Thanks,
Christian.