Re: [PATCH v5 0/5] Convert some stash functionality to a builtin

Re-sending, but this time in plain text (why doesn't gmail for android support that...) On Mon, Apr 30, 2018 at 7:35 AM, Joel Teichroeb <j...@teichroeb.net> wrote: > Hi Paul, > > That sounds great to me! > > Thanks, > Joel > > > On Sat, Apr 28, 2018,

[PATCH v5 2/5] stash: convert apply to builtin

simply be renamed and the shell script deleted. Delete the contents of the apply_stash shell function and replace it with a call to stash--helper apply until pop is also converted. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- .gitignore | 1 + Ma

[PATCH v5 5/5] stash: convert pop to builtin

Add stash pop to the helper and delete the pop_stash, drop_stash, assert_stash_ref and pop_stash functions from the shell script now that they are no longer needed. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 41 +++

[PATCH v5 3/5] stash: convert drop and clear to builtin

the commands are called externally. Once pop is converted they can both be removed. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 107 git-stash.sh| 4 +- 2 files changed, 109 insertions(+), 2 del

[PATCH v5 0/5] Convert some stash functionality to a builtin

* is used Joel Teichroeb (5): stash: improve option parsing test coverage stash: convert apply to builtin stash: convert drop and clear to builtin stash: convert branch to builtin stash: convert pop to builtin .gitignore | 1 + Makefile| 1 + builtin.h

[PATCH v5 1/5] stash: improve option parsing test coverage

In preparation for converting the stash command incrementally to a builtin command, this patch improves test coverage of the option parsing. Both for having too many parameters, or too few. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.s

[PATCH v5 4/5] stash: convert branch to builtin

Add stash branch to the helper and delete the apply_to_branch function from the shell script. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 51 + git-stash.sh| 17 ++--- 2 files chang

Re: [PATCH v4 2/5] stash: convert apply to builtin

On Thu, Mar 29, 2018 at 1:07 PM, Junio C Hamano <gits...@pobox.com> wrote: > Joel Teichroeb <j...@teichroeb.net> writes: > >> +static int get_stash_info(struct stash_info *info, int argc, const char >> **argv) >> +{ > > So, this roughly corresponds

[PATCH v4 4/5] stash: convert branch to builtin

Add stash branch to the helper and delete the apply_to_branch function from the shell script. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 47 +++ git-stash.sh| 17 ++--- 2 files chang

[PATCH v4 5/5] stash: convert pop to builtin

Add stash pop to the helper and delete the pop_stash, drop_stash, assert_stash_ref and pop_stash functions from the shell script now that they are no longer needed. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 41 +++

[PATCH v4 2/5] stash: convert apply to builtin

simply be renamed and the shell script deleted. Delete the contents of the apply_stash shell function and replace it with a call to stash--helper apply until pop is also converted. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- .gitignore | 1 + Ma

[PATCH v4 0/5] Convert some stash functionality to a builtin

/git/20180327054432.26419-1-j...@teichroeb.net/ Changes from v3: - Fix the Windows build - Fix a use after free in the error handling Joel Teichroeb (5): stash: improve option parsing test coverage stash: convert apply to builtin stash: convert drop and clear to builtin stash: convert

[PATCH v4 1/5] stash: improve option parsing test coverage

In preparation for converting the stash command incrementally to a builtin command, this patch improves test coverage of the option parsing. Both for having too many paramerters, or too few. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 16

[PATCH v4 3/5] stash: convert drop and clear to builtin

the commands are called externally. Once pop is converted they can both be removed. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 101 git-stash.sh| 4 +- 2 files changed, 103 insertions(+), 2 del

Re: [PATCH v2 3/6] stash: convert apply to builtin

On Mon, Mar 26, 2018 at 12:05 AM, Christian Couder <christian.cou...@gmail.com> wrote: > On Mon, Mar 26, 2018 at 3:14 AM, Joel Teichroeb <j...@teichroeb.net> wrote: >> Signed-off-by: Joel Teichroeb <j...@teichroeb.net> > > The commit message in this patch and

Re: [PATCH 1/4] stash: convert apply to builtin

On Sun, Mar 25, 2018 at 9:43 AM, Thomas Gummerer <t.gumme...@gmail.com> wrote: > On 03/24, Joel Teichroeb wrote: >> --- > > Missing sign-off? I saw it's missing in the other patches as well. > Thanks! I always forget to add a sign-off. >> [...]

Re: [PATCH v3 2/5] stash: convert apply to builtin

On Tue, Mar 27, 2018 at 9:02 AM, Johannes Schindelin wrote: > Hi Joel, > > [...] >> + >> +static int do_apply_stash(const char *prefix, struct stash_info *info, int >> index) >> +{ >> + struct merge_options o; >> + struct object_id c_tree; >> + struct

[PATCH v3 5/5] stash: convert pop to builtin

Add stash pop to the helper and delete the pop_stash, drop_stash, assert_stash_ref and pop_stash functions from the shell script now that they are no longer needed. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 41 +++

[PATCH v3 4/5] stash: convert branch to builtin

Add stash branch to the helper and delete the apply_to_branch function from the shell script. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 47 +++ git-stash.sh| 17 ++--- 2 files chang

[PATCH v3 3/5] stash: convert drop and clear to builtin

the commands are called externally. Once pop is converted they can both be removed. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 101 git-stash.sh| 4 +- 2 files changed, 103 insertions(+), 2 del

[PATCH v3 2/5] stash: convert apply to builtin

simply be renamed and the shell script deleted. Delete the contents of the apply_stash shell function and replace it with a call to stash--helper apply until pop is also converted. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- .gitignore | 1 + Ma

[PATCH v3 0/5] Convert some stash functionality to a builtin

/ Changes from v2: - Fixed formatting (I keep forgetting to set vim to tabs) - Renamed destroy to free - Redid my tests to validate more (Thanks Johannes) - Deleted more shell code that isn't needed anymore Joel Teichroeb (5): stash: improve option parsing test coverage stash: convert

[PATCH v3 1/5] stash: improve option parsing test coverage

In preparation for converting the stash command incrementally to a builtin command, this patch improves test coverage of the option parsing. Both for having too many paramerters, or too few. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 16

[PATCH v2 4/6] stash: convert drop and clear to builtin

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 101 git-stash.sh| 4 +- 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c

[PATCH v2 6/6] stash: convert pop to builtin

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 41 + git-stash.sh| 39 --- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/builtin/stash--helper.c b/b

[PATCH v2 1/6] stash: Add tests for passing in too many refs

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 22 ++ 1 file changed, 22 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index aefde7b172..7146e27bb5 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -45,6

[PATCH v2 0/6] Convert some stash functionality to a builtin

- Removed old .sh code that has been replaced - Fix differences in error messages - Added tests to ensure argument handling work when specifiying multiple refs, or no arguments for branch - Refactored the code a bit - Reordered the patches so they compile and test successfully Joel

[PATCH v2 3/6] stash: convert apply to builtin

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- .gitignore | 1 + Makefile| 1 + builtin.h | 1 + builtin/stash--helper.c | 431 git-stash.sh| 75 +

[PATCH v2 5/6] stash: convert branch to builtin

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/stash--helper.c | 47 +++ git-stash.sh| 17 ++--- 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/builtin/stash--helper.c b/builtin/stash--he

[PATCH v2 2/6] stash: Add test for branch with no arguments

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 4 1 file changed, 4 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 7146e27bb5..261571d02a 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -497,6 +497,10 @@ test_expect_success

Re: [PATCH 1/4] stash: convert apply to builtin

On Sun, Mar 25, 2018 at 1:09 AM, Christian Couder wrote: > It seems to me that the apply_stash() shell function is also used in > pop_stash() and in apply_to_branch(). Can the new helper be used there > too instead of apply_stash()? And then could apply_stash() be

[PATCH 2/4] stash: convert branch to builtin

--- builtin/stash--helper.c | 44 git-stash.sh| 3 ++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c index e9a9574f40..18c4aba665 100644 --- a/builtin/stash--helper.c +++

[PATCH 0/4] Convert some stash functionality to a builtin

GSOC students who want to work on converting builtins, I figured I should finish what I have that works now so they could build on top of it. Joel Teichroeb (4): stash: convert apply to builtin stash: convert branch to builtin stash: convert drop and clear to builtin stash: convert pop

[PATCH 3/4] stash: convert drop and clear to builtin

--- builtin/stash--helper.c | 93 + git-stash.sh| 4 +-- 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c index 18c4aba665..1598b82ac2 100644 --- a/builtin/stash--helper.c

[PATCH 4/4] stash: convert pop to builtin

--- builtin/stash--helper.c | 38 ++ git-stash.sh| 3 ++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c index 1598b82ac2..b912f84c97 100644 --- a/builtin/stash--helper.c +++

[PATCH 1/4] stash: convert apply to builtin

--- .gitignore | 1 + Makefile| 1 + builtin.h | 1 + builtin/stash--helper.c | 339 git-stash.sh| 3 +- git.c | 1 + 6 files changed, 345 insertions(+), 1

Re: [PATCH 2/2] stash: implement builtin stash helper

Thanks for your comments! I've incorporated them all for the next patch set.

[PATCH 2/2] stash: implement builtin stash helper

Start moving stash functions over to builtin c code and call them in the shell script, instead of converting it all at once. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- Makefile| 1 + builtin.h | 1 + builtin/stash--helper.c

[PATCH 1/2] merge: close the index lock when not writing the new index

If the merge does not have anything to do, it does not unlock the index, causing any further index operations to fail. Thus, always unlock the index regardless of outcome. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- merge-recursive.c | 9 ++--- 1 file changed, 6 insertions

[PATCH 0/2] Convert some stash functionality to a builtin

to convert the rest of the commands one at a time without breaking anything. I've sent most of this code before, but that was targetting a full replacement of stash. The code is overall the same, but with some code review changes and updates for internal api changes. Joel Teichroeb (2): merge: close

Re: [PATCH v3 0/4] Implement git stash as a builtin command

, Johannes Schindelin <johannes.schinde...@gmx.de> wrote: > Hi Joel, > > On Sun, 28 May 2017, Joel Teichroeb wrote: > >> I've rewritten git stash as a builtin c command. All tests pass, >> and I've added two new tests. Test coverage is around 95% with the >> only

Re: [PATCH 1/3] stash: add test for stash create with no files

Hi Junio, I was just too lazy to write a cover letter, and thought these would make sense on their own. I'll make sure to include a cover letter next time. I just ripped them out of my patch series on implementing stash as a builtin[1]. Since I haven't had time, I figured I could at least get

[PATCH 3/3] stash: add test for stashing in a detached state

All that we are really testing here is that the message is correct when we are not on any branch. All other functionality is already tested elsewhere. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 12 1 file changed, 12 insertions(+) diff --git a/t

[PATCH 1/3] stash: add test for stash create with no files

Ensure the command suceeds and outputs nothing Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 4046817d70..f0708ced27 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-st

[PATCH 2/3] stash: Add a test for when apply fails during stash branch

If the return value of merge recursive is not checked, the stash could end up being dropped even though it was not applied properly Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 14 ++ 1 file changed, 14 insertions(+) diff --git a/t/t3903-stash.

Re: [PATCH v4 5/5] stash: implement builtin stash

On Sun, Jun 11, 2017 at 2:27 PM, Thomas Gummerer wrote: >> + >> +int cmd_stash(int argc, const char **argv, const char *prefix) >> +{ >> + int result = 0; >> + pid_t pid = getpid(); >> + >> + struct option options[] = { >> + OPT_END() >> + }; >> +

Re: [PATCH v4 5/5] stash: implement builtin stash

On Fri, Jun 16, 2017 at 3:47 PM, Junio C Hamano <gits...@pobox.com> wrote: > Joel Teichroeb <j...@teichroeb.net> writes: >> +/* >> + * Untracked files are stored by themselves in a parentless commit, for >> + * ease of unpacking later. >> + */ >> +stat

Re: [PATCH v4 2/5] stash: Add a test for when apply fails during stash branch

On Tue, Jun 13, 2017 at 12:40 PM, Junio C Hamano <gits...@pobox.com> wrote: > Joel Teichroeb <j...@teichroeb.net> writes: > >> If the return value of merge recurisve is not checked, the stash could end >> up being dropped even though it was not applied prop

Re: [PATCH v4 3/5] stash: add test for stashing in a detached state

On Tue, Jun 13, 2017 at 12:45 PM, Junio C Hamano <gits...@pobox.com> wrote: > Joel Teichroeb <j...@teichroeb.net> writes: > >> Signed-off-by: Joel Teichroeb <j...@teichroeb.net> >> --- >> t/t3903-stash.sh | 12 >> 1 file changed, 12 ins

Re: [PATCH v4 0/5] Implement git stash as a builtin command

I haven't seen any response. Would it be possible for anyone to review? Thanks, Joel On 6/7/2017 5:55 PM, Joel Teichroeb wrote: I've rewritten git stash as a builtin c command. All tests pass, and I've added two new tests. Test coverage is around 95% with the only things missing coverage being

Re: Re: git stash --include-untracked possible destructive behavior

Looks like that series was merged last week, fixing this issue. Marc, the fix will probably be in git 2.14. Joel

Re: git stash --include-untracked possible destructive behavior

It looks like it's an issue with git clean, as that's what stash uses internally. You can try your same test, but replace git stash with "git clean -df" and it will delete Data. Maybe clean should check if there are any ignored files in an untracked directory and only remove it if -x is

[PATCH v4 1/5] stash: add test for stash create with no files

Ensure the command gives the correct return code Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 3b4bed5c9a..cc923e6335 100755 --- a/t/t3903-stash.sh +++ b/t

[PATCH v4 4/5] merge: close the index lock when not writing the new index

If the merge does not have anything to do, it does not unlock the index, causing any further index operations to fail. Thus, always unlock the index regardless of outcome. Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- merge-recursive.c | 9 ++--- 1 file changed, 6 insertions

[PATCH v4 2/5] stash: Add a test for when apply fails during stash branch

If the return value of merge recurisve is not checked, the stash could end up being dropped even though it was not applied properly Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 14 ++ 1 file changed, 14 insertions(+) diff --git a/t/t3903-stash.

[PATCH v4 3/5] stash: add test for stashing in a detached state

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 5399fb05ca..ce4c8fe3d6 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -822,6 +822,18 @@ test_expect_s

[PATCH v4 0/5] Implement git stash as a builtin command

review comments Outstanding issue: * Not all argv array memory is cleaned up Joel Teichroeb (5): stash: add test for stash create with no files stash: Add a test for when apply fails during stash branch stash: add test for stashing in a detached state merge: close the index lock when

[PATCH v4 5/5] stash: implement builtin stash

Implement all git stash functionality as a builtin command Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- Makefile |2 +- builtin.h |1 + builtin/stash.c

Re: [PATCH v3 4/4] stash: implement builtin stash

I'm running into a lot of trouble using argv_array_clear. It seems that some of the builtin git cmd functions move the parameters around, and write new pointers to argv. There's three options I have now, and I'm not sure which is the best one. 1. Fix all the builtin cmd functions that I use to

Re: [PATCH v3 4/4] stash: implement builtin stash

Once I have all those leaks fixed, is there a way to make sure I'm not missing any? I tried using valgrind with leak-check enabled, but there are too many leaks from other git commands. Joel

Re: [PATCH v3 4/4] stash: implement builtin stash

On Sun, May 28, 2017 at 11:26 AM, Ævar Arnfjörð Bjarmason <ava...@gmail.com> wrote: > On Sun, May 28, 2017 at 6:56 PM, Joel Teichroeb <j...@teichroeb.net> wrote: >> Implement all git stash functionality as a builtin command > > First thanks for working on this, it'

[PATCH v3 3/4] close the index lock when not writing the new index

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- builtin/add.c | 3 ++- builtin/mv.c | 8 +--- builtin/rm.c | 3 ++- merge-recursive.c | 8 +--- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 9f53f020d0..6b04

[PATCH v3 2/4] stash: add test for stashing in a detached state

Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index aaae221304..b86851ef46 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -808,6 +808,17 @@ test_expect_s

[PATCH v3 4/4] stash: implement builtin stash

Implement all git stash functionality as a builtin command Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- Makefile |2 +- builtin.h |1 + builtin/stash.c

[PATCH v3 1/4] stash: add test for stash create with no files

Ensure the command gives the correct return code Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- t/t3903-stash.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 3b4bed5c9a..aaae221304 100755 --- a/t/t3903-stash.sh +++ b/t

[PATCH v3 0/4] Implement git stash as a builtin command

I've rewritten git stash as a builtin c command. All tests pass, and I've added two new tests. Test coverage is around 95% with the only things missing coverage being error handlers. Joel Teichroeb (4): stash: add test for stash create with no files stash: add test for stashing in a detached

[PATCH/RFC V2] stash: implement builtin stash

Implement all git stash functionality as a builtin command Signed-off-by: Joel Teichroeb <j...@teichroeb.net> --- I've been working on rewriting git stash as a c builtin and I have all but three tests passing. I'm having a bit of trouble fixing them, as well as a few other issues, so I'd

[PATCH] [RFC] Converting git stash to a builtin

I've been working on rewriting git stash as a c builtin and I have all but three tests passing. I'm having a bit of trouble fixing them, as well as a few other issues, so I'd really appreciate some help. Don't bother commenting on the small details yet as I still need to go though the code to make

[RFC] Converting git stash to a builtin

I've been working on rewriting git stash as a c builtin and I have all but three tests passing. I'm having a bit of trouble fixing them, as well as a few other issues, so I'd really appreciate some help. Don't bother commenting on the small details yet as I still need to go though the code to make