| Message ID | cover.1542925164.git.ungureanupaulsebastian@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Convert "git stash" to C builtin | expand |
On 11/23, Paul-Sebastian Ungureanu wrote: > Hello, > > This is the 11th iteration of C git stash. Here are some of the changes, > based on Thomas's and dscho's suggestions (from mailing list / pull request > #495): Thanks for your work on this! I have read through the range-diff and the new patch of this last round, and this addresses all the comments I had on v10 (and some more :)). I consider it Reviewed-by: Thomas Gummerer <t.gummerer@gmail.com> > - improved memory management. Now, the callers of `do_create_stash()` > are responsible of freeing the parameter they pass in. Moreover, the > stash message is now a pointer to a buffer (in the previous iteration > it was a pointer to a string). This should make it more clear who is > responsible of freeing the memory. > > - added `strbuf_insertf()` which inserts a format string at a given > position in the buffer. > > - some minor changes (changed "!oidcmp" to "oideq") > > - fixed merge conflicts > > Best regards, > Paul > > 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 > > Paul-Sebastian Ungureanu (17): > sha1-name.c: add `get_oidf()` which acts like `get_oid()` > strbuf.c: add `strbuf_join_argv()` > strbuf.c: add `strbuf_insertf()` and `strbuf_vinsertf()` > t3903: modernize style > stash: rename test cases to be more descriptive > stash: add tests for `git stash show` config > stash: mention options in `show` synopsis > stash: convert list to builtin > stash: convert show to builtin > stash: convert store to builtin > stash: convert create to builtin > stash: convert push to builtin > stash: make push -q quiet > stash: convert save to builtin > stash: convert `stash--helper.c` into `stash.c` > stash: optimize `get_untracked_files()` and `check_changes()` > stash: replace all `write-tree` child processes with API calls > > Documentation/git-stash.txt | 4 +- > Makefile | 2 +- > builtin.h | 1 + > builtin/stash.c | 1596 ++++++++++++++++++++++++++++++++++ > cache.h | 1 + > git-stash.sh | 752 ---------------- > git.c | 1 + > sha1-name.c | 19 + > strbuf.c | 51 ++ > strbuf.h | 16 + > t/t3903-stash.sh | 192 ++-- > t/t3907-stash-show-config.sh | 83 ++ > 12 files changed, 1897 insertions(+), 821 deletions(-) > create mode 100644 builtin/stash.c > delete mode 100755 git-stash.sh > create mode 100755 t/t3907-stash-show-config.sh > > -- > 2.19.1.878.g0482332a22 >
Thomas Gummerer <t.gummerer@gmail.com> writes: > Thanks for your work on this! I have read through the range-diff and > the new patch of this last round, and this addresses all the comments > I had on v10 (and some more :)). I consider it > Reviewed-by: Thomas Gummerer <t.gummerer@gmail.com> Thanks. One thing that bothers me is that this seems to have been rebased on 'master', but as long as we are rebasing, the updated series must also take into account of the sd/stash-wo-user-name topic, i.e. if we are rebasing it, it should be rebased on top of the result of git checkout -B ps/rebase-in-c master git merge --no-ff sd/stash-wo-user-name I think.
Junio C Hamano <gitster@pobox.com> writes: > Thomas Gummerer <t.gummerer@gmail.com> writes: > >> Thanks for your work on this! I have read through the range-diff and >> the new patch of this last round, and this addresses all the comments >> I had on v10 (and some more :)). I consider it >> Reviewed-by: Thomas Gummerer <t.gummerer@gmail.com> > > Thanks. > > One thing that bothers me is that this seems to have been rebased on > 'master', but as long as we are rebasing, the updated series must > also take into account of the sd/stash-wo-user-name topic, i.e. if > we are rebasing it, it should be rebased on top of the result of > > git checkout -B ps/rebase-in-c master > git merge --no-ff sd/stash-wo-user-name > > I think. https://travis-ci.org/git/git/builds/459619672 would show that this C reimplementation now regresses from the scripted version due to lack of such rebasing (i.e. porting a correction from scripted one).
Hi Junio, On Mon, 26 Nov 2018, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > Thomas Gummerer <t.gummerer@gmail.com> writes: > > > >> Thanks for your work on this! I have read through the range-diff and > >> the new patch of this last round, and this addresses all the comments > >> I had on v10 (and some more :)). I consider it > >> Reviewed-by: Thomas Gummerer <t.gummerer@gmail.com> > > > > Thanks. > > > > One thing that bothers me is that this seems to have been rebased on > > 'master', but as long as we are rebasing, the updated series must > > also take into account of the sd/stash-wo-user-name topic, i.e. if > > we are rebasing it, it should be rebased on top of the result of > > > > git checkout -B ps/rebase-in-c master > > git merge --no-ff sd/stash-wo-user-name > > > > I think. > > https://travis-ci.org/git/git/builds/459619672 would show that this > C reimplementation now regresses from the scripted version due to > lack of such rebasing (i.e. porting a correction from scripted one). Oh, you know, at first I *mis-read* your mail to mean "don't you rebase all the time!", but in this case (in contrast to earlier statements about rebasing between iterations of patch series), you *do* want Paul to rebase. Let me see what I can come up with in my `git-stash` branch on https://github.com/dscho/git Ciao, Dscho
Hi Junio, On Thu, 29 Nov 2018, Johannes Schindelin wrote: > On Mon, 26 Nov 2018, Junio C Hamano wrote: > > > Junio C Hamano <gitster@pobox.com> writes: > > > > > Thomas Gummerer <t.gummerer@gmail.com> writes: > > > > > >> Thanks for your work on this! I have read through the range-diff and > > >> the new patch of this last round, and this addresses all the comments > > >> I had on v10 (and some more :)). I consider it > > >> Reviewed-by: Thomas Gummerer <t.gummerer@gmail.com> > > > > > > Thanks. > > > > > > One thing that bothers me is that this seems to have been rebased on > > > 'master', but as long as we are rebasing, the updated series must > > > also take into account of the sd/stash-wo-user-name topic, i.e. if > > > we are rebasing it, it should be rebased on top of the result of > > > > > > git checkout -B ps/rebase-in-c master > > > git merge --no-ff sd/stash-wo-user-name > > > > > > I think. > > > > https://travis-ci.org/git/git/builds/459619672 would show that this > > C reimplementation now regresses from the scripted version due to > > lack of such rebasing (i.e. porting a correction from scripted one). > > Oh, you know, at first I *mis-read* your mail to mean "don't you rebase > all the time!", but in this case (in contrast to earlier statements about > rebasing between iterations of patch series), you *do* want Paul to > rebase. > > Let me see what I can come up with in my `git-stash` branch on > https://github.com/dscho/git There. I force-pushed an update that is based on sd/stash-wo-user-name and adds a `prepare_fallback_ident(name, email)` to `ident.c` for use in the built-in stash: https://github.com/dscho/git/commit/d37ce623fbd32e4345c701dea822e56de1a5417f It passes t3903 in a little over a minute with GIT_TEST_STASH_USE_BUILTIN=true and in a little less than seven minutes with GIT_TEST_STASH_USE_BUILTIN=false. Ciao, Dscho