Re: Git Evolve
> If we lose the Change-Id: footer without adding any new cruft in the > commit object header, that would be a great success. It would > be a failure if we end up touching the object header. Yes, I was thinking along the same lines. If obsolescence graph is stored as part of the commit header, it would get unconditionally shared whenever that commit is pushed, and I would consider it a desirable property if users could choose whether to push just a commit or a the obsolescence graph that contains the commit. Sometimes the reason you're amending a commit may be because the original contained content that shouldn't be pushed. Putting it in the header would also either cause it to get retained forever by git gc or create a situation where we permit dangling references - and neither seem desirable to me. It would be nice if users could retain the obsolescence graph for stuff a user is actively working on, but we could discard it (with 0 cost) for commits they're done with or were never interested in editing. On Sat, Sep 29, 2018 at 5:55 PM, Junio C Hamano wrote: > Stefan Xenos writes: > >> What is the evolve command? >> ... >> - Systems like gerrit would no longer need to rely on "change-id" tags >> in commit comments to associate commits with the change that they >> edit, since git itself would have that information. >> ... >> Is anyone else interested in this? Please email me directly or on this >> list. Let's chat: I want to make sure that whatever we come up with is >> at least as good as any similar technology that has come before. > > As you listed in the related technologies section, I think the > underlying machinery that supports "rebase -i", especially with the > recent addition of redoing the existing merges (i.e. "rebase -i > -r"), may be enough to rewrite the histories that were built on top > of a commit that has been obsoleted by amending. > > I would imagine that the main design effort you would need to make > is to figure out a good way to > > (1) keep track of which commits are obsoleted by which other ones > [*1*], and > > (2) to figure out what histories are still to be rebuilt in what > order on top of what commit efficiently. > > Once these are done, you should be able to write out the sequence of > instructions to feed the same sequencer machinery used by the > "rebase -i" command. > > [Side note] > > *1* It is very desirable to keep track of the evolution of a change > without polluting the commit object with things like Change-Id: > and other cruft, either in the body or in the header. If we > lose the Change-Id: footer without adding any new cruft in the > commit object header, that would be a great success. It would > be a failure if we end up touching the object header. > > > >
Re: [PATCH v9 09/21] stash: convert branch to builtin
On 09/26, Paul-Sebastian Ungureanu wrote:
> From: Joel Teichroeb
>
> Add stash branch to the helper and delete the apply_to_branch
> function from the shell script.
>
> Checkout does not currently provide a function for checking out
> a branch as cmd_checkout does a large amount of sanity checks
> first that we require here.
>
> Signed-off-by: Joel Teichroeb
> Signed-off-by: Paul-Sebastian Ungureanu
> ---
> builtin/stash--helper.c | 46 +
> git-stash.sh| 17 ++-
> 2 files changed, 48 insertions(+), 15 deletions(-)
>
> diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
> index 72472eaeb7..5841bd0e98 100644
> --- a/builtin/stash--helper.c
> +++ b/builtin/stash--helper.c
> @@ -14,6 +14,7 @@
> static const char * const git_stash_helper_usage[] = {
> N_("git stash--helper drop [-q|--quiet] []"),
> N_("git stash--helper apply [--index] [-q|--quiet] []"),
> + N_("git stash--helper branch []"),
> N_("git stash--helper clear"),
> NULL
> };
> @@ -28,6 +29,11 @@ static const char * const git_stash_helper_apply_usage[] =
> {
> NULL
> };
>
> +static const char * const git_stash_helper_branch_usage[] = {
> + N_("git stash--helper branch []"),
> + NULL
> +};
> +
> static const char * const git_stash_helper_clear_usage[] = {
> N_("git stash--helper clear"),
> NULL
> @@ -536,6 +542,44 @@ static int drop_stash(int argc, const char **argv, const
> char *prefix)
> return ret;
> }
>
> +static int branch_stash(int argc, const char **argv, const char *prefix)
> +{
> + int ret;
> + const char *branch = NULL;
> + struct stash_info info;
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct option options[] = {
> + OPT_END()
> + };
> +
> + argc = parse_options(argc, argv, prefix, options,
> + git_stash_helper_branch_usage, 0);
> +
> + if (!argc) {
> + fprintf_ln(stderr, "No branch name specified");
This should be marked for translation.
> + return -1;
> + }
> +
> + branch = argv[0];
> +
> + if (get_stash_info(, argc - 1, argv + 1))
> + return -1;
> +
> + cp.git_cmd = 1;
> + argv_array_pushl(, "checkout", "-b", NULL);
> + argv_array_push(, branch);
> + argv_array_push(, oid_to_hex(_commit));
> + ret = run_command();
> + if (!ret)
> + ret = do_apply_stash(prefix, , 1, 0);
> + if (!ret && info.is_stash_ref)
> + ret = do_drop_stash(prefix, , 0);
> +
> + free_stash_info();
> +
> + return ret;
> +}
> +
> int cmd_stash__helper(int argc, const char **argv, const char *prefix)
> {
> pid_t pid = getpid();
> @@ -562,6 +606,8 @@ int cmd_stash__helper(int argc, const char **argv, const
> char *prefix)
> return !!clear_stash(argc, argv, prefix);
> else if (!strcmp(argv[0], "drop"))
> return !!drop_stash(argc, argv, prefix);
> + else if (!strcmp(argv[0], "branch"))
> + return !!branch_stash(argc, argv, prefix);
>
> usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
> git_stash_helper_usage, options);
> diff --git a/git-stash.sh b/git-stash.sh
> index a99d5dc9e5..29d9f44255 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -598,20 +598,6 @@ drop_stash () {
> clear_stash
> }
>
> -apply_to_branch () {
> - test -n "$1" || die "$(gettext "No branch name specified")"
> - branch=$1
> - shift 1
> -
> - set -- --index "$@"
> - assert_stash_like "$@"
> -
> - git checkout -b $branch $REV^ &&
> - apply_stash "$@" && {
> - test -z "$IS_STASH_REF" || drop_stash "$@"
> - }
> -}
> -
> test "$1" = "-p" && set "push" "$@"
>
> PARSE_CACHE='--not-parsed'
> @@ -673,7 +659,8 @@ pop)
> ;;
> branch)
> shift
> - apply_to_branch "$@"
> + cd "$START_DIR"
> + git stash--helper branch "$@"
> ;;
> *)
> case $# in
> --
> 2.19.0.rc0.23.g1fb9f40d88
>
Re: [PATCH v9 07/21] stash: convert apply to builtin
On 09/26, Paul-Sebastian Ungureanu wrote:
> From: Joel Teichroeb
>
> Add a builtin helper for performing stash commands. Converting
> all at once proved hard to review, so starting with just apply
> lets conversion get started without the other commands being
> finished.
>
> The helper is being implemented as a drop in replacement for
> stash so that when it is complete it can 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
> Signed-off-by: Paul-Sebastian Ungureanu
> ---
> .gitignore | 1 +
> Makefile| 1 +
> builtin.h | 1 +
> builtin/stash--helper.c | 452
> git-stash.sh| 78 +--
> git.c | 1 +
> 6 files changed, 463 insertions(+), 71 deletions(-)
> create mode 100644 builtin/stash--helper.c
>
> diff --git a/.gitignore b/.gitignore
> index ffceea7d59..b59661cb88 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -157,6 +157,7 @@
> /git-show-ref
> /git-stage
> /git-stash
> +/git-stash--helper
> /git-status
> /git-stripspace
> /git-submodule
> diff --git a/Makefile b/Makefile
> index d03df31c2a..f900c68e69 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1093,6 +1093,7 @@ BUILTIN_OBJS += builtin/shortlog.o
> BUILTIN_OBJS += builtin/show-branch.o
> BUILTIN_OBJS += builtin/show-index.o
> BUILTIN_OBJS += builtin/show-ref.o
> +BUILTIN_OBJS += builtin/stash--helper.o
> BUILTIN_OBJS += builtin/stripspace.o
> BUILTIN_OBJS += builtin/submodule--helper.o
> BUILTIN_OBJS += builtin/symbolic-ref.o
> diff --git a/builtin.h b/builtin.h
> index 99206df4bd..317bc338f7 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -223,6 +223,7 @@ extern int cmd_show(int argc, const char **argv, const
> char *prefix);
> extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
> extern int cmd_show_index(int argc, const char **argv, const char *prefix);
> extern int cmd_status(int argc, const char **argv, const char *prefix);
> +extern int cmd_stash__helper(int argc, const char **argv, const char
> *prefix);
> extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
> extern int cmd_submodule__helper(int argc, const char **argv, const char
> *prefix);
> extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
> diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
> new file mode 100644
> index 00..7819dae332
> --- /dev/null
> +++ b/builtin/stash--helper.c
> @@ -0,0 +1,452 @@
> +#include "builtin.h"
> +#include "config.h"
> +#include "parse-options.h"
> +#include "refs.h"
> +#include "lockfile.h"
> +#include "cache-tree.h"
> +#include "unpack-trees.h"
> +#include "merge-recursive.h"
> +#include "argv-array.h"
> +#include "run-command.h"
> +#include "dir.h"
> +#include "rerere.h"
> +
> +static const char * const git_stash_helper_usage[] = {
> + N_("git stash--helper apply [--index] [-q|--quiet] []"),
> + NULL
> +};
> +
> +static const char * const git_stash_helper_apply_usage[] = {
> + N_("git stash--helper apply [--index] [-q|--quiet] []"),
> + NULL
> +};
> +
> +static const char *ref_stash = "refs/stash";
> +static struct strbuf stash_index_path = STRBUF_INIT;
> +
> +/*
> + * w_commit is set to the commit containing the working tree
> + * b_commit is set to the base commit
> + * i_commit is set to the commit containing the index tree
> + * u_commit is set to the commit containing the untracked files tree
> + * w_tree is set to the working tree
> + * b_tree is set to the base tree
> + * i_tree is set to the index tree
> + * u_tree is set to the untracked files tree
> + */
> +
> +struct stash_info {
> + struct object_id w_commit;
> + struct object_id b_commit;
> + struct object_id i_commit;
> + struct object_id u_commit;
> + struct object_id w_tree;
> + struct object_id b_tree;
> + struct object_id i_tree;
> + struct object_id u_tree;
> + struct strbuf revision;
> + int is_stash_ref;
> + int has_u;
> +};
> +
> +static void free_stash_info(struct stash_info *info)
> +{
> + strbuf_release(>revision);
> +}
> +
> +static void assert_stash_like(struct stash_info *info, const char *revision)
> +{
> + if (get_oidf(>b_commit, "%s^1", revision) ||
> + get_oidf(>w_tree, "%s:", revision) ||
> + get_oidf(>b_tree, "%s^1:", revision) ||
> + get_oidf(>i_tree, "%s^2:", revision)) {
> + free_stash_info(info);
> + error(_("'%s' is not a stash-like commit"), revision);
> + exit(128);
This seems to just emulate 'die()'. Can we just use that directly?
The only reason I could imagine for not doing that would be to keep
the same exit code, or the exact same message we had in the shell
script. But we're doing neither here. The
Re: [PATCH v9 04/21] stash: update test cases conform to coding guidelines
> Subject: stash: update test cases conform to coding guidelines
s/stash/t3903/
s/conform/to &/
Alternatively the subject could also be "t3903: modernize style",
which would be a bit shorter, and still convey the same information to
a reader of 'git log --oneline'.
On 09/26, Paul-Sebastian Ungureanu wrote:
> Removed whitespaces after redirection operators.
s/Removed/Remove/. Commit messages should always use the imperative
mood, as described in Documentation/SubmittingPatches.
>
> Signed-off-by: Paul-Sebastian Ungureanu
> ---
> t/t3903-stash.sh | 120 ---
> 1 file changed, 61 insertions(+), 59 deletions(-)
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index af7586d43d..de6cab1fe7 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -8,22 +8,22 @@ test_description='Test git stash'
> . ./test-lib.sh
>
> test_expect_success 'stash some dirty working directory' '
> - echo 1 > file &&
> + echo 1 >file &&
> git add file &&
> echo unrelated >other-file &&
> git add other-file &&
> test_tick &&
> git commit -m initial &&
> - echo 2 > file &&
> + echo 2 >file &&
> git add file &&
> - echo 3 > file &&
> + echo 3 >file &&
> test_tick &&
> git stash &&
> git diff-files --quiet &&
> git diff-index --cached --quiet HEAD
> '
>
> -cat > expect << EOF
> +cat >expect < diff --git a/file b/file
> index 0cfbf08..00750ed 100644
> --- a/file
> @@ -35,7 +35,7 @@ EOF
>
> test_expect_success 'parents of stash' '
> test $(git rev-parse stash^) = $(git rev-parse HEAD) &&
> - git diff stash^2..stash > output &&
> + git diff stash^2..stash >output &&
> test_cmp output expect
> '
>
> @@ -74,7 +74,7 @@ test_expect_success 'apply stashed changes' '
>
> test_expect_success 'apply stashed changes (including index)' '
> git reset --hard HEAD^ &&
> - echo 6 > other-file &&
> + echo 6 >other-file &&
> git add other-file &&
> test_tick &&
> git commit -m other-file &&
> @@ -99,12 +99,12 @@ test_expect_success 'stash drop complains of extra
> options' '
>
> test_expect_success 'drop top stash' '
> git reset --hard &&
> - git stash list > stashlist1 &&
> - echo 7 > file &&
> + git stash list >expected &&
> + echo 7 >file &&
> git stash &&
> git stash drop &&
> - git stash list > stashlist2 &&
> - test_cmp stashlist1 stashlist2 &&
> + git stash list >actual &&
> + test_cmp expected actual &&
> git stash apply &&
> test 3 = $(cat file) &&
> test 1 = $(git show :file) &&
> @@ -113,9 +113,9 @@ test_expect_success 'drop top stash' '
>
> test_expect_success 'drop middle stash' '
> git reset --hard &&
> - echo 8 > file &&
> + echo 8 >file &&
> git stash &&
> - echo 9 > file &&
> + echo 9 >file &&
> git stash &&
> git stash drop stash@{1} &&
> test 2 = $(git stash list | wc -l) &&
> @@ -160,7 +160,7 @@ test_expect_success 'stash pop' '
> test 0 = $(git stash list | wc -l)
> '
>
> -cat > expect << EOF
> +cat >expect < diff --git a/file2 b/file2
> new file mode 100644
> index 000..1fe912c
> @@ -170,7 +170,7 @@ index 000..1fe912c
> +bar2
> EOF
>
> -cat > expect1 << EOF
> +cat >expect1 < diff --git a/file b/file
> index 257cc56..5716ca5 100644
> --- a/file
> @@ -180,7 +180,7 @@ index 257cc56..5716ca5 100644
> +bar
> EOF
>
> -cat > expect2 << EOF
> +cat >expect2 < diff --git a/file b/file
> index 7601807..5716ca5 100644
> --- a/file
> @@ -198,79 +198,79 @@ index 000..1fe912c
> EOF
>
> test_expect_success 'stash branch' '
> - echo foo > file &&
> + echo foo >file &&
> git commit file -m first &&
> - echo bar > file &&
> - echo bar2 > file2 &&
> + echo bar >file &&
> + echo bar2 >file2 &&
> git add file2 &&
> git stash &&
> - echo baz > file &&
> + echo baz >file &&
> git commit file -m second &&
> git stash branch stashbranch &&
> test refs/heads/stashbranch = $(git symbolic-ref HEAD) &&
> test $(git rev-parse HEAD) = $(git rev-parse master^) &&
> - git diff --cached > output &&
> + git diff --cached >output &&
> test_cmp output expect &&
> - git diff > output &&
> + git diff >output &&
> test_cmp output expect1 &&
> git add file &&
> git commit -m alternate\ second &&
> - git diff master..stashbranch > output &&
> + git diff master..stashbranch >output &&
> test_cmp output expect2 &&
> test 0 = $(git stash list | wc -l)
> '
>
> test_expect_success 'apply -q is quiet' '
> - echo foo > file &&
> + echo foo >file &&
> git stash &&
> - git stash apply -q > output.out 2>&1 &&
> + git stash apply -q >output.out 2>&1 &&
> test_must_be_empty output.out
> '
>
> test_expect_success 'save -q is quiet' '
> - git
Re: [Outreachy] Introduce myself
Hi Ananya, On Sun, Sep 30, 2018 at 5:53 PM Ananya Krishna Maram wrote: > > Hi Git Community, Christian and Johannes, > > My initial Outreachy got accepted. Great! Welcome to the Git community! [...] > Having done a lot of assignment in C and > bash scripting and keen interest to learn about working of git > internals, I choose to contribute to this project. So I started > observing the patches sent to git mailing list. About possible projects I updated https://git.github.io/Outreachy-17/ but only the `git bisect` has been officially proposed as an Outreachy project. I hope Dscho (Johannes) will be ok to submit one of the 2 others soon and to register himself as a mentor or co-mentor on some of the projects. > I am currently looking for first patch opportunities to git. I came > across[1] and I will try to put maximum effort towards my goal and if > I need some clarification of the problem statement I guess you guys or > Outreachy mentors will be here to help me. The micro-project page you found is not up-to-date, so some micro-projects we propose might have already been taken by GSoC students last winter/spring. Sorry we didn't update the page or create another one. Anyway there are some micro-projects there like "Add more builtin patterns for userdiff" that are still valid and still good small tasks to get started working on Git. And there are explanations about how you can search for micro-projects (especially how to search for #leftoverbits on the mailing list archive). Thank you for your interest in contributing to Git, Christian.
Re: [PATCH v9 02/21] strbuf.c: add `strbuf_join_argv()`
On 09/26, Paul-Sebastian Ungureanu wrote:
> Implement `strbuf_join_argv()` to join arguments
> into a strbuf.
>
> Signed-off-by: Paul-Sebastian Ungureanu
> ---
> strbuf.c | 15 +++
> strbuf.h | 7 +++
> 2 files changed, 22 insertions(+)
>
> diff --git a/strbuf.c b/strbuf.c
> index 64041c3c24..3eb431b2b0 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -259,6 +259,21 @@ void strbuf_addbuf(struct strbuf *sb, const struct
> strbuf *sb2)
> strbuf_setlen(sb, sb->len + sb2->len);
> }
>
> +const char *strbuf_join_argv(struct strbuf *buf,
> + int argc, const char **argv, char delim)
> +{
> + if (!argc)
> + return buf->buf;
> +
> + strbuf_addstr(buf, *argv);
> + while (--argc) {
> + strbuf_addch(buf, delim);
> + strbuf_addstr(buf, *(++argv));
> + }
> +
> + return buf->buf;
Why are we returning buf-buf here? The strbuf is modified by the
function, so the caller can just use buf->buf directly if they want
to. Is there something I'm missing?
> +}
> +
> void strbuf_addchars(struct strbuf *sb, int c, size_t n)
> {
> strbuf_grow(sb, n);
> diff --git a/strbuf.h b/strbuf.h
> index 60a35aef16..7ed859bb8a 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -284,6 +284,13 @@ static inline void strbuf_addstr(struct strbuf *sb,
> const char *s)
> */
> extern void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2);
>
> +
stray newline? We usually only have one blank line between functions.
> +/**
> + *
> + */
Forgot to write some documentation here? :)
> +extern const char *strbuf_join_argv(struct strbuf *buf, int argc,
> + const char **argv, char delim);
> +
> /**
> * This function can be used to expand a format string containing
> * placeholders. To that end, it parses the string and calls the specified
> --
> 2.19.0.rc0.23.g1fb9f40d88
>
[Outreachy] Introduce myself
Hi Git Community, Christian and Johannes, My initial Outreachy got accepted. My name is Ananya Krishna(her), I am from south India. I completed my under-graduation. Due to some problems, I decided to stay back at home. And currently I would like to dedicate myself to learn from open communities. During the school, I showed keen interest in operating systems and implemented most of the scheduling, synchronization and some sample examples in memory management. I also did a lot of assignments in bash scripting. In my 3rd year of Btech, I was introduced to this magical tool `git` and got flattened by how it made me to keep track of versions of my assignments. Having done a lot of assignment in C and bash scripting and keen interest to learn about working of git internals, I choose to contribute to this project. So I started observing the patches sent to git mailing list. I am currently looking for first patch opportunities to git. I came across[1] and I will try to put maximum effort towards my goal and if I need some clarification of the problem statement I guess you guys or Outreachy mentors will be here to help me. Thanks, Ananya. [1](https://git.github.io/SoC-2018-Microprojects/)
Re: [PATCH v3 0/6] Fix the racy split index problem
Junio, On Fri, Sep 28, 2018 at 06:24:53PM +0200, SZEDER Gábor wrote: > Interdiff: > diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh > index c8acbc50d0..f053bf83eb 100755 > --- a/t/t1700-split-index.sh > +++ b/t/t1700-split-index.sh > @@ -6,6 +6,9 @@ test_description='split index mode tests' > > # We need total control of index splitting here > sane_unset GIT_TEST_SPLIT_INDEX The conflict resolution around here in 3f725b07d3 (Merge branch 'sg/split-index-racefix' into jch, 2018-09-29) accidentally removed the above line, which makes the test fail when run with 'GIT_TEST_SPLIT_INDEX=yes', e.g.: https://travis-ci.org/git/git/jobs/435077629#L3389 > +# A couple of tests expect the index to have a specific checksum, > +# but the presence of the optional FSMN extension would interfere > +# with those checks, so disable it in this test script. > sane_unset GIT_FSMONITOR_TEST > > # Create a file named as $1 with content read from stdin.
[PATCH v3 1/1] roll wt_status_state into wt_status and populate in the collect phase
Status variables were initialized in the collect phase and some
variables were later freed in the print functions.
A "struct wt_status" used to be sufficient for the output phase to
work. It was designed to be filled in the collect phase and consumed
in the output phase, but over time some fields were added and output
phase started filling the fields.
A "struct wt_status_state" that was used in other codepaths turned out
to be useful in the "git status" output. This is not tied to "struct
wt_status", so filling in the collect phase was not consistently
followed.
Move the status state structure variables into the status state
structure and populate them in the collect functions.
Create a new function to free the buffers that were being freed in the
print function. Call this new function in commit.c where both the
collect and print functions were being called.
Based on a patch suggestion by Junio C Hamano. [1]
[1] https://public-inbox.org/git/xmqqr2i5ueg4@gitster-ct.c.googlers.com/
Signed-off-by: Stephen P. Smith
---
builtin/commit.c | 3 ++
wt-status.c | 134 +--
wt-status.h | 38 +++---
3 files changed, 82 insertions(+), 93 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d6efd996f8..c91af2fe38 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -508,6 +508,7 @@ static int run_status(FILE *fp, const char *index_file,
const char *prefix, int
wt_status_collect(s);
wt_status_print(s);
+ wt_status_collect_free_buffers(s);
return s->committable;
}
@@ -1390,6 +1391,8 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
s.prefix = prefix;
wt_status_print();
+ wt_status_collect_free_buffers();
+
return 0;
}
diff --git a/wt-status.c b/wt-status.c
index 1822e95f65..cc3e84b997 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -744,21 +744,25 @@ static int has_unmerged(struct wt_status *s)
void wt_status_collect(struct wt_status *s)
{
- struct wt_status_state state;
wt_status_collect_changes_worktree(s);
-
if (s->is_initial)
wt_status_collect_changes_initial(s);
else
wt_status_collect_changes_index(s);
wt_status_collect_untracked(s);
- memset(, 0, sizeof(state));
- wt_status_get_state(, s->branch && !strcmp(s->branch, "HEAD"));
- if (state.merge_in_progress && !has_unmerged(s))
+ wt_status_get_state(>state, s->branch && !strcmp(s->branch, "HEAD"));
+ if (s->state.merge_in_progress && !has_unmerged(s))
s->committable = 1;
}
+void wt_status_collect_free_buffers(struct wt_status *s)
+{
+ free(s->state.branch);
+ free(s->state.onto);
+ free(s->state.detached_from);
+}
+
static void wt_longstatus_print_unmerged(struct wt_status *s)
{
int shown_header = 0;
@@ -1087,8 +1091,7 @@ static void wt_longstatus_print_tracking(struct wt_status
*s)
}
static void show_merge_in_progress(struct wt_status *s,
- struct wt_status_state *state,
- const char *color)
+ const char *color)
{
if (has_unmerged(s)) {
status_printf_ln(s, color, _("You have unmerged paths."));
@@ -1109,16 +1112,15 @@ static void show_merge_in_progress(struct wt_status *s,
}
static void show_am_in_progress(struct wt_status *s,
- struct wt_status_state *state,
const char *color)
{
status_printf_ln(s, color,
_("You are in the middle of an am session."));
- if (state->am_empty_patch)
+ if (s->state.am_empty_patch)
status_printf_ln(s, color,
_("The current patch is empty."));
if (s->hints) {
- if (!state->am_empty_patch)
+ if (!s->state.am_empty_patch)
status_printf_ln(s, color,
_(" (fix conflicts and then run \"git am
--continue\")"));
status_printf_ln(s, color,
@@ -1242,10 +1244,9 @@ static int read_rebase_todolist(const char *fname,
struct string_list *lines)
}
static void show_rebase_information(struct wt_status *s,
- struct wt_status_state *state,
- const char *color)
+ const char *color)
{
- if (state->rebase_interactive_in_progress) {
+ if (s->state.rebase_interactive_in_progress) {
int i;
int nr_lines_to_show = 2;
@@ -1296,28 +1297,26 @@ static void show_rebase_information(struct wt_status *s,
}
static void print_rebase_state(struct wt_status *s,
- struct wt_status_state *state,
- const char *color)
+
[PATCH v3 0/1] wt-status-state-cleanup
Junio suggested a cleanup patch, jc/wt-status-state-cleanup, which is the basis for this patch. This patch uses ss/wt-status-committable. Update to fix a spelling error in the commet message Stephen P. Smith (1): roll wt_status_state into wt_status and populate in the collect phase builtin/commit.c | 3 ++ wt-status.c | 134 +-- wt-status.h | 38 +++--- 3 files changed, 82 insertions(+), 93 deletions(-) -- 2.19.0
Re: [PATCH v2 1/2] t1300: extract and use test_cmp_config()
On Sat, Sep 29, 2018 at 05:30:04PM +0200, Nguyễn Thái Ngọc Duy wrote:
> In many config-related tests it's common to check if a config variable
> has expected value and we want to print the differences when the test
> fails. Doing it the normal way is three lines of shell code. Let's add
> a function do to all this (and a little more).
>
> This function has uses outside t1300 as well but I'm not going to
> convert them all. And it will be used in the next commit where
> per-worktree config feature is introduced.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> t/t1300-config.sh | 79 ++---
> t/test-lib-functions.sh | 24 +
> 2 files changed, 43 insertions(+), 60 deletions(-)
>
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index cdf1fed5d1..00c2b0f0eb 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -76,15 +76,11 @@ EOF
> test_expect_success 'unset type specifiers may be reset to conflicting ones'
> '
> - echo 1048576 >expect &&
> - git config --type=bool --no-type --type=int core.big >actual &&
> - test_cmp expect actual
> + test_cmp_config 1048576 --type=bool --no-type --type=int core.big
> '
>
> test_expect_success '--type rejects unknown specifiers' '
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index d82fac9d79..4cd7fb8fdf 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -747,6 +747,30 @@ test_cmp() {
> $GIT_TEST_CMP "$@"
> }
>
> +# similar to test_cmp but $2 is a config key instead of actual value
This doesn't describe very well the function's parameters: $1
specifies the expected value (as opposed to the file containing the
expected value), and the rest can be all kinds of 'git config'
options, not just a second argument with the config key; e.g. see call
in the above hunk.
Perheps only a brief description and a usage string like this would
sufficiently cover everything?
# Check that the given config key has the expected value.
#
# test_cmp_config [-C ]
# [...]
> +# it can also accept -C to read from a different repo, e.g.
> +#
> +# test_cmp_config -C xyz foo core.bar
> +#
> +# is sort of equivalent of
I think this comment should outright say that "is a better alternative
to", because it provides useful output on failure, and because it
doesn't hide 'git config's exit code.
Note that this second point does make a bit of a difference:
test_cmp_config "" nonexisting.variable
would fail, because
$ git config nonexisting.variable ; echo $?
1
and the &&-chain. However,
test "" = "$(git config nonexisting.variable)"
would still succeed, because the non-zero exit code is ignored.
I consider this a benefit, as it will protect us from a typo in the
name of a set but empty variable, even though we won't get any error
message.
> +#
> +# test "foo" = "$(git -C xyz core.bar)"
> +
Nit: unnecessary empty line.
> +test_cmp_config() {
> + if [ "$1" = "-C" ]
> + then
> + shift &&
> + GD="-C $1" &&
> + shift
> + else
> + GD=
> + fi &&
I think you could now safely declare GD as local. The test balloon
01d3a526ad (t: check whether the shell supports the "local"
keyword, 2017-10-26) has been out there for 4 releases / almost a
year, and we haven't heard about any issues, and the upcoming hash
translation test helpers use 'local' as well.
> + echo "$1" >expected &&
> + shift &&
> + git $GD config "$@" >actual &&
> + test_cmp expected actual
> +}
> +
> # test_cmp_bin - helper to compare binary files
>
> test_cmp_bin() {
> --
> 2.19.0.341.g3acb95d729
>
Re: [PATCH v2 2/2] worktree: add per-worktree config files
On Sun, Sep 30, 2018 at 9:24 AM Eric Sunshine wrote:
>
> On Sun, Sep 30, 2018 at 3:16 AM Duy Nguyen wrote:
> > On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine
> > wrote:
> > > > diff --git a/builtin/config.c b/builtin/config.c
> > > > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const
> > > > char *prefix)
> > > > + else if (use_worktree_config) {
> > > > + struct worktree **worktrees = get_worktrees(0);
> > > > + if (repository_format_worktree_config)
> > > > + given_config_source.file =
> > > > git_pathdup("config.worktree");
> > > > + else if (worktrees[0] && worktrees[1])
> > > > + die(_("--worktree cannot be used with multiple "
> > > > + "working trees unless the config\n"
> > > > + "extension worktreeConfig is enabled. "
> > > > + "Please read \"CONFIGURATION FILE\"\n"
> > > > + "section in \"git help worktree\" for
> > > > details"));
> > > > + else
> > > > + given_config_source.file =
> > > > git_pathdup("config");
> > >
> > > I'm not sure I understand the purpose of allowing --worktree when only
> > > a single worktree is present and extensions.worktreeConfig is not set.
> > > Can you talk about it a bit more?
> >
> > Unified API. If I write a script to do stuff and want it to work in
> > multiple worktrees as well, I should not need to do "if single
> > worktree, use "git config --local", if multiple use "git config
> > --worktree"". By using "git config --worktree" I tell git "this config
> > is per-worktree" and git should do the right thing, single worktree or
> > not.
>
> That's what I thought, but I still don't understand how that unified
> API is going to help if the script writer happens to have multiple
> worktrees but doesn't have extensions.worktreeConfig set, in which
> case the command will error out. I don't see how that simplifies
> things, but perhaps I'm missing something obvious.
No it does not simplify that step. The user has to enable it manually
(at least for now). v1 tried to enable it automatically, but I think
it's safer to go one step at a time.
--
Duy
Re: [PATCH v2 2/2] worktree: add per-worktree config files
On Sun, Sep 30, 2018 at 3:16 AM Duy Nguyen wrote:
> On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine wrote:
> > > diff --git a/builtin/config.c b/builtin/config.c
> > > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const
> > > char *prefix)
> > > + else if (use_worktree_config) {
> > > + struct worktree **worktrees = get_worktrees(0);
> > > + if (repository_format_worktree_config)
> > > + given_config_source.file =
> > > git_pathdup("config.worktree");
> > > + else if (worktrees[0] && worktrees[1])
> > > + die(_("--worktree cannot be used with multiple "
> > > + "working trees unless the config\n"
> > > + "extension worktreeConfig is enabled. "
> > > + "Please read \"CONFIGURATION FILE\"\n"
> > > + "section in \"git help worktree\" for
> > > details"));
> > > + else
> > > + given_config_source.file = git_pathdup("config");
> >
> > I'm not sure I understand the purpose of allowing --worktree when only
> > a single worktree is present and extensions.worktreeConfig is not set.
> > Can you talk about it a bit more?
>
> Unified API. If I write a script to do stuff and want it to work in
> multiple worktrees as well, I should not need to do "if single
> worktree, use "git config --local", if multiple use "git config
> --worktree"". By using "git config --worktree" I tell git "this config
> is per-worktree" and git should do the right thing, single worktree or
> not.
That's what I thought, but I still don't understand how that unified
API is going to help if the script writer happens to have multiple
worktrees but doesn't have extensions.worktreeConfig set, in which
case the command will error out. I don't see how that simplifies
things, but perhaps I'm missing something obvious.
Re: [PATCH v2 2/2] worktree: add per-worktree config files
On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine wrote:
> > diff --git a/builtin/config.c b/builtin/config.c
> > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char
> > *prefix)
> > + else if (use_worktree_config) {
> > + struct worktree **worktrees = get_worktrees(0);
> > + if (repository_format_worktree_config)
> > + given_config_source.file =
> > git_pathdup("config.worktree");
> > + else if (worktrees[0] && worktrees[1])
> > + die(_("--worktree cannot be used with multiple "
> > + "working trees unless the config\n"
> > + "extension worktreeConfig is enabled. "
> > + "Please read \"CONFIGURATION FILE\"\n"
> > + "section in \"git help worktree\" for
> > details"));
> > + else
> > + given_config_source.file = git_pathdup("config");
>
> I'm not sure I understand the purpose of allowing --worktree when only
> a single worktree is present and extensions.worktreeConfig is not set.
> Can you talk about it a bit more?
Unified API. If I write a script to do stuff and want it to work in
multiple worktrees as well, I should not need to do "if single
worktree, use "git config --local", if multiple use "git config
--worktree"". By using "git config --worktree" I tell git "this config
is per-worktree" and git should do the right thing, single worktree or
not.
--
Duy
