date:20180325

Eric Sunshine  writes:

> What's the plan for oddball cases such as 66ae9a57b8 (t3404: rebase
> -i: demonstrate short SHA-1 collision, 2013-08-23) which depend
> implicitly upon SHA-1 without actually hardcoding any hashes? The test
> added by 66ae9a57b8, for instance, won't start failing in the face of
> NewHash, but it also won't be testing anything meaningful.
>
> Should such tests be dropped altogether? Should they be marked with a
> 'SHA1' predicate or be annotated with a comment as being
> SHA-1-specific? Something else?

Ideally, the existing one be annotated with prereq SHA1, and also
duplicated with a tweak to cause the same kind of (half-)collision
under the NewHash and be annotated with prereq NewHash.

It's a different matter how feasible it is to attain such an ideal,
though.  t1512 was fun to write, but it was quite a lot of work to
come up with bunch of blobs, trees and commits whose object names
share the common prefix 0{10}.

core.fsmonitor always perform rescans

2018-03-25 Thread Tatsuyuki Ishi

Hello,

I'm facing issue with core.fsmonitor.

I'm currently using the provided watchman hook, but it doesn't seem to
record the fact that it has queried the fsmonitor backend, and as a
result the timestamp passed to the hook doesn't seem to change.

As it always pass a timestamp before watchman has crawled the
directories, watchman will always return all files inside the
directory. This happens everytime I run a git command, resulting in
slowness.

Is the timestamp not being updated an intended behavior, or is this a bug?

Tatsuyuki Ishi

Re: [PATCH v2 00/36] Combine t/helper binaries into a single one

Nguyễn Thái Ngọc Duy   writes:

> v2 fixes a couple of typos in commit messages and use the cmd__ prefix
> for test commands instead of test_, which avoids a naming conflict
> with the existing function test_lazy_init_name_hash
>
> [the previous v2 send out was aborted because I messed it up with some
> other patches]

Yeah, I was wondering if the 7 v4 patches for --keep-pack should be
picked up, or I should just wait for a proper resubmission under its
own cover letter.

Re: [PATCH 00/10] Hash-independent tests (part 1)

On Sun, Mar 25, 2018 at 3:20 PM, brian m. carlson
 wrote:
> This is a series to make our tests hash-independent.  Many tests have
> hard-coded SHA-1 values in them, and it would be valuable to express
> these items in a hash-independent way for our hash transitions.
>
> The approach in this series relies on only three components for hash
> independence: git rev-parse, git hash-object, and EMPTY_BLOB and
> EMPTY_TREE.  Because many of our shell scripts and test components
> already rely on the first two, this seems like a safe assumption.
>
> For the same reason, this series avoids modifying tests that test these
> components or their expected SHA-1 values.  I expect that when we add
> another hash function, we'll copy these tests to expose both SHA-1 and
> NewHash versions.

What's the plan for oddball cases such as 66ae9a57b8 (t3404: rebase
-i: demonstrate short SHA-1 collision, 2013-08-23) which depend
implicitly upon SHA-1 without actually hardcoding any hashes? The test
added by 66ae9a57b8, for instance, won't start failing in the face of
NewHash, but it also won't be testing anything meaningful.

Should such tests be dropped altogether? Should they be marked with a
'SHA1' predicate or be annotated with a comment as being
SHA-1-specific? Something else?

Re: [PATCH v4 3/7] gc: add --keep-largest-pack option

Duy Nguyen  writes:

>> This conflicts with master because of your own 7e1eeaa431 ("completion:
>> use __gitcomp_builtin in _git_gc", 2018-02-09). I pushed out a
>> avar-pclouds/gc-auto-keep-base-pack branch to github.com/avar/git which
>> resolves it as:
>>
>> @@ -365,6 +393,8 @@ int cmd_gc(int argc, const char **argv, const char 
>> *prefix)
>> OPT_BOOL_F(0, "force", ,
>>N_("force running gc even if there may be 
>> another gc running"),
>>PARSE_OPT_NOCOMPLETE),
>> +   OPT_BOOL(0, "keep-largest-pack", _base_pack,
>> +N_("repack all other packs except the largest 
>> pack")),
>> OPT_END()
>> };
>>
>> I assume that's the intention here.
>
> Yeah, I want  to keep the same base for easy interdiff. There are
> worse conflicts are with the other series I'm helping Stefan.

Right now there are a couple of topics that wants to touch options
array and also the builtins command table, which are both good
examples of "central registry" that everybody needs to touch and are
bound to become a source of conflict.  Until I scream (and I try
hard not to have to), contributors should not have to worry too much
about causing conflicts---being traffic cop and directing orderly
integration of these topics is what the maintainer(s) do.

Thanks.

Re: [PATCH 2/2] send-email: supply a --send-delay=1 by default

Eric Sunshine  writes:

> A minor point: Are you sure that it's git-format-patch that's being
> careful about arranging Date: to display in the desired order, and not
> git-send-email? Looking at old patches I still have hanging around
> which were created with git-format-patch, I see the Date: headers are
> wildly out of order, presumably because the date is taken from
> Author-Date: and the patches were heavily rebased.

send-email uses the current time as the timestamp it lets MTA to see
(and for a N-patch series, the first patch gets current time minus
N, and later patches get newer timestamps with 1 second increment).

The Date: field in the input file to the command has nothing to
participate in this process; sending a series that has patches that
have been shuffled with "rebase -i" would still give older timestamp
to the earlier message while sending the series out.

That is sufficient for any MUA that is capable of sorting the
messages in the sender's timestamp order; even though there is no
way to force the actual order in which an MTA on the receiving end
sees the messages, it is not necessary and it would not help X-<.

Re: [PATCH 2/2] send-email: supply a --send-delay=1 by default

"brian m. carlson"  writes:

> I'm not sure that this is going to have the effect you want it to have.
> Let me give an example to demonstrate why.
> ...
> In short, I don't think this is going to be especially helpful because
> it won't change the status quo for a lot of senders.  You'd have to
> insert some significant delay in order to get the effect you desire, and
> even then things could still be delivered out of order.

Thanks for explaining it clearly.

In the past on this list those who do not get the store-and-forward
nature of e-mail transport have brought this up a few times, but
this approach fundamentally do not work, at least for the purpose of
forcing ordering of messages at the receiving end.

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

Signed-off-by: Joel Teichroeb 
---
 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
index 050de415b4..640de6d0aa 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -11,7 +11,14 @@
 #include "dir.h"
 
 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 clear"),
+   NULL
+};
+
+static const char * const git_stash_helper_drop_usage[] = {
+   N_("git stash--helper drop [-q|--quiet] []"),
NULL
 };
 
@@ -20,6 +27,11 @@ static const char * const git_stash_helper_apply_usage[] = {
NULL
 };
 
+static const char * const git_stash_helper_clear_usage[] = {
+   N_("git stash--helper clear"),
+   NULL
+};
+
 static const char *ref_stash = "refs/stash";
 static int quiet;
 static char stash_index_path[PATH_MAX];
@@ -166,6 +178,29 @@ static int get_stash_info(struct stash_info *info, int 
argc, const char **argv)
return 0;
 }
 
+static int do_clear_stash(void)
+{
+   struct object_id obj;
+   if (get_oid(ref_stash, ))
+   return 0;
+
+   return delete_ref(NULL, ref_stash, , 0);
+}
+
+static int clear_stash(int argc, const char **argv, const char *prefix)
+{
+   struct option options[] = {
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options, 
git_stash_helper_clear_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+   if (argc != 0)
+   return error(_("git stash--helper clear with parameters is 
unimplemented"));
+
+   return do_clear_stash();
+}
+
 static int reset_tree(struct object_id *i_tree, int update, int reset)
 {
struct unpack_trees_options opts;
@@ -399,6 +434,68 @@ static int apply_stash(int argc, const char **argv, const 
char *prefix)
return ret;
 }
 
+static int do_drop_stash(const char *prefix, struct stash_info *info)
+{
+   struct argv_array args = ARGV_ARRAY_INIT;
+   int ret;
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   argv_array_pushl(, "reflog", "delete", "--updateref", "--rewrite", 
NULL);
+   argv_array_push(, info->revision.buf);
+   ret = cmd_reflog(args.argc, args.argv, prefix);
+   if (!ret) {
+   if (!quiet)
+   printf(_("Dropped %s (%s)\n"), info->revision.buf, 
oid_to_hex(>w_commit));
+   } else {
+   return error(_("%s: Could not drop stash entry"), 
info->revision.buf);
+   }
+
+   cp.git_cmd = 1;
+   /* Even though --quiet is specified, rev-parse still outputs the hash */
+   cp.no_stdout = 1;
+   argv_array_pushl(, "rev-parse", "--verify", "--quiet", NULL);
+   argv_array_pushf(, "%s@{0}", ref_stash);
+   ret = run_command();
+
+   if (ret)
+   do_clear_stash();
+
+   return 0;
+}
+
+static int assert_stash_ref(struct stash_info *info)
+{
+   if (!info->is_stash_ref)
+   return error(_("'%s' is not a stash reference"), 
info->revision.buf);
+
+   return 0;
+}
+
+static int drop_stash(int argc, const char **argv, const char *prefix)
+{
+   struct stash_info info;
+   int ret;
+   struct option options[] = {
+   OPT__QUIET(, N_("be quiet, only report errors")),
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+   git_stash_helper_drop_usage, 0);
+
+   if (get_stash_info(, argc, argv))
+   return -1;
+
+   if (assert_stash_ref()) {
+   destroy_stash_info();
+   return -1;
+   }
+
+   ret = do_drop_stash(prefix, );
+   destroy_stash_info();
+   return ret;
+}
+
 int cmd_stash__helper(int argc, const char **argv, const char *prefix)
 {
int result = 0;
@@ -421,6 +518,10 @@ int cmd_stash__helper(int argc, const char **argv, const 
char *prefix)
usage_with_options(git_stash_helper_usage, options);
else if (!strcmp(argv[0], "apply"))
result = apply_stash(argc, argv, prefix);
+   else if (!strcmp(argv[0], "clear"))
+   result = clear_stash(argc, argv, prefix);
+   else if (!strcmp(argv[0], "drop"))
+   result = drop_stash(argc, argv, prefix);
else {
error(_("unknown subcommand: %s"), argv[0]);
usage_with_options(git_stash_helper_usage, options);
diff --git a/git-stash.sh b/git-stash.sh
index 0b5d1f3743..0b8f07b38a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -652,7 +652,7 @@ apply)
;;
 clear)
shift
-   clear_stash "$@"
+   git stash--helper clear "$@"
;;
 create)
shift
@@ -664,7 +664,7 @@ store)
;;
 drop)
shift
-

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

Signed-off-by: Joel Teichroeb 
---
 builtin/stash--helper.c | 41 +
 git-stash.sh| 39 ---
 2 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 4226c13be3..36e7e660a0 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -12,6 +12,7 @@
 
 static const char * const git_stash_helper_usage[] = {
N_("git stash--helper drop [-q|--quiet] []"),
+   N_("git stash--helper pop [--index] [-q|--quiet] []"),
N_("git stash--helper apply [--index] [-q|--quiet] []"),
N_("git stash--helper branch  []"),
N_("git stash--helper clear"),
@@ -23,6 +24,11 @@ static const char * const git_stash_helper_drop_usage[] = {
NULL
 };
 
+static const char * const git_stash_helper_pop_usage[] = {
+   N_("git stash--helper pop [--index] [-q|--quiet] []"),
+   NULL
+};
+
 static const char * const git_stash_helper_apply_usage[] = {
N_("git stash--helper apply [--index] [-q|--quiet] []"),
NULL
@@ -502,6 +508,39 @@ static int drop_stash(int argc, const char **argv, const 
char *prefix)
return ret;
 }
 
+static int pop_stash(int argc, const char **argv, const char *prefix)
+{
+   int index = 0, ret;
+   struct stash_info info;
+   struct option options[] = {
+   OPT__QUIET(, N_("be quiet, only report errors")),
+   OPT_BOOL(0, "index", ,
+   N_("attempt to recreate the index")),
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+   git_stash_helper_pop_usage, 0);
+
+   if (get_stash_info(, argc, argv))
+   return -1;
+
+   if (assert_stash_ref()) {
+   destroy_stash_info();
+   return -1;
+   }
+
+   if (do_apply_stash(prefix, , index)) {
+   printf_ln(_("The stash entry is kept in case you need it 
again."));
+   destroy_stash_info();
+   return -1;
+   }
+
+   ret = do_drop_stash(prefix, );
+   destroy_stash_info();
+   return ret;
+}
+
 static int branch_stash(int argc, const char **argv, const char *prefix)
 {
const char *branch = NULL;
@@ -567,6 +606,8 @@ int cmd_stash__helper(int argc, const char **argv, const 
char *prefix)
result = clear_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "drop"))
result = drop_stash(argc, argv, prefix);
+   else if (!strcmp(argv[0], "pop"))
+   result = pop_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "branch"))
result = branch_stash(argc, argv, prefix);
else {
diff --git a/git-stash.sh b/git-stash.sh
index c5fd4c6c44..1b8c7c4f63 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -565,39 +565,6 @@ assert_stash_ref() {
}
 }
 
-apply_stash () {
-   cd "$START_DIR"
-   git stash--helper apply "$@"
-   res=$?
-   cd_to_toplevel
-   return $res
-}
-
-pop_stash() {
-   assert_stash_ref "$@"
-
-   if apply_stash "$@"
-   then
-   drop_stash "$@"
-   else
-   status=$?
-   say "$(gettext "The stash entry is kept in case you need it 
again.")"
-   exit $status
-   fi
-}
-
-drop_stash () {
-   assert_stash_ref "$@"
-
-   git reflog delete --updateref --rewrite "${REV}" &&
-   say "$(eval_gettext "Dropped \${REV} (\$s)")" ||
-   die "$(eval_gettext "\${REV}: Could not drop stash entry")"
-
-   # clear_stash if we just dropped the last stash entry
-   git rev-parse --verify --quiet "$ref_stash@{0}" >/dev/null ||
-   clear_stash
-}
-
 test "$1" = "-p" && set "push" "$@"
 
 PARSE_CACHE='--not-parsed'
@@ -634,7 +601,8 @@ push)
;;
 apply)
shift
-   apply_stash "$@"
+   cd "$START_DIR"
+   git stash--helper apply "$@"
;;
 clear)
shift
@@ -654,7 +622,8 @@ drop)
;;
 pop)
shift
-   pop_stash "$@"
+   cd "$START_DIR"
+   git stash--helper pop "$@"
;;
 branch)
shift
-- 
2.16.2

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

Signed-off-by: Joel Teichroeb 
---
 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 +45,12 @@ test_expect_success 'applying bogus stash does nothing' '
test_cmp expect file
 '
 
+test_expect_success 'applying with too many agruments does nothing' '
+   test_must_fail git stash apply stash@{0} bar &&
+   echo 1 >expect &&
+   test_cmp expect file
+'
+
 test_expect_success 'apply does not need clean working directory' '
echo 4 >other-file &&
git stash apply &&
@@ -97,6 +103,10 @@ test_expect_success 'stash drop complains of extra options' 
'
test_must_fail git stash drop --foo
 '
 
+test_expect_success 'stash drop complains with too many refs' '
+   test_must_fail git stash drop stash@{1} stash@{2}
+'
+
 test_expect_success 'drop top stash' '
git reset --hard &&
git stash list > stashlist1 &&
@@ -160,6 +170,10 @@ test_expect_success 'stash pop' '
test 0 = $(git stash list | wc -l)
 '
 
+test_expect_success 'stash pop complains with too many refs' '
+   test_must_fail git stash pop stash@{1} stash@{2}
+'
+
 cat > expect << EOF
 diff --git a/file2 b/file2
 new file mode 100644
@@ -479,6 +493,10 @@ test_expect_success 'stash branch - stashes on stack, 
stash-like argument' '
test $(git ls-files --modified | wc -l) -eq 1
 '
 
+test_expect_success 'stash branch complains with too many refs' '
+   test_must_fail git stash branch stash-branch stash@{1} stash@{2}
+'
+
 test_expect_success 'stash show format defaults to --stat' '
git stash clear &&
test_when_finished "git reset --hard HEAD" &&
@@ -567,6 +585,10 @@ test_expect_success 'stash show -p - no stashes on stack, 
stash-like argument' '
test_cmp expected actual
 '
 
+test_expect_success 'stash show complains with too many refs' '
+   test_must_fail git stash show stash@{1} stash@{2}
+'
+
 test_expect_success 'stash drop - fail early if specified stash is not a stash 
reference' '
git stash clear &&
test_when_finished "git reset --hard HEAD && git stash clear" &&
-- 
2.16.2

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

I've been working on converting all of git stash to be a
builtin, however it's hard to get it all working at once with
limited time, so I've moved around half of it to a new
stash--helper builtin and called these functions from the shell
script. Once this is stabalized, it should be easier 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.

Since there seems to be interest from 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.

Previous threads:
v1: https://public-inbox.org/git/20180325173916.GE10909@hank/T/

Changes from v1:
 - Fixed memory leaks
 - Fixed formatting
 - Fixed typos
 - 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 Teichroeb (6):
  stash: Add tests for passing in too many refs
  stash: Add test for branch with no arguments
  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   |   1 +
 builtin/stash--helper.c | 620 
 git-stash.sh| 125 +-
 git.c   |   1 +
 t/t3903-stash.sh|  26 ++
 7 files changed, 658 insertions(+), 117 deletions(-)
 create mode 100644 builtin/stash--helper.c

-- 
2.16.2

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

Signed-off-by: Joel Teichroeb 
---
 .gitignore  |   1 +
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/stash--helper.c | 431 
 git-stash.sh|  75 +
 git.c   |   1 +
 6 files changed, 440 insertions(+), 70 deletions(-)
 create mode 100644 builtin/stash--helper.c

diff --git a/.gitignore b/.gitignore
index 833ef3b0b7..296d5f376d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -152,6 +152,7 @@
 /git-show-ref
 /git-stage
 /git-stash
+/git-stash--helper
 /git-status
 /git-stripspace
 /git-submodule
diff --git a/Makefile b/Makefile
index 96f6138f63..6cfdbe9a32 100644
--- a/Makefile
+++ b/Makefile
@@ -1028,6 +1028,7 @@ BUILTIN_OBJS += builtin/send-pack.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.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 42378f3aa4..a14fd85b0e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -219,6 +219,7 @@ extern int cmd_shortlog(int argc, const char **argv, const 
char *prefix);
 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_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..050de415b4
--- /dev/null
+++ b/builtin/stash--helper.c
@@ -0,0 +1,431 @@
+#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"
+
+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 int quiet;
+static char stash_index_path[PATH_MAX];
+
+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 int get_symbolic_name(const char *symbolic, struct strbuf *out)
+{
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   cp.git_cmd = 1;
+   argv_array_pushl(, "rev-parse", "--symbolic-full-name", NULL);
+   argv_array_pushf(, "%s", symbolic);
+   return pipe_command(, NULL, 0, out, 0, NULL, 0);
+}
+
+static int have_stash(void)
+{
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   cp.git_cmd = 1;
+   cp.no_stdout = 1;
+   argv_array_pushl(, "rev-parse", "--verify", "--quiet", NULL);
+   argv_array_pushf(, "%s", ref_stash);
+   return pipe_command(, NULL, 0, NULL, 0, NULL, 0);
+}
+
+static void destroy_stash_info(struct stash_info *info)
+{
+   strbuf_release(>revision);
+}
+
+static int get_stash_info(struct stash_info *info, int argc, const char **argv)
+{
+   struct strbuf w_commit_rev = STRBUF_INIT;
+   struct strbuf b_commit_rev = STRBUF_INIT;
+   struct strbuf w_tree_rev = STRBUF_INIT;
+   struct strbuf b_tree_rev = STRBUF_INIT;
+   struct strbuf i_tree_rev = STRBUF_INIT;
+   struct strbuf u_tree_rev = STRBUF_INIT;
+   struct strbuf symbolic = STRBUF_INIT;
+   struct strbuf out = STRBUF_INIT;
+   int ret;
+   const char *revision;
+   const char *commit = NULL;
+   char *end_of_rev;
+   info->is_stash_ref = 0;
+
+   if (argc > 1) {
+   int i;
+   fprintf(stderr, _("Too many revisions specified:"));
+   for (i = 0; i < argc; ++i) {
+   fprintf(stderr, " '%s'", argv[i]);
+   }
+   fprintf(stderr, "\n");
+
+   return -1;
+   }
+
+   if (argc == 1)
+   commit = argv[0];
+
+   strbuf_init(>revision, 0);
+   if (commit == NULL) {
+   if (have_stash()) {
+   destroy_stash_info(info);
+   return error(_("No stash entries found."));
+   }
+
+

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

Signed-off-by: Joel Teichroeb 
---
 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--helper.c
index 640de6d0aa..4226c13be3 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -13,6 +13,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
 };
@@ -27,6 +28,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
@@ -496,6 +502,45 @@ 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)
+{
+   const char *branch = NULL;
+   int ret;
+   struct argv_array args = ARGV_ARRAY_INIT;
+   struct stash_info info;
+   struct option options[] = {
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+   git_stash_helper_branch_usage, 0);
+
+   if (argc == 0)
+   return error(_("No branch name specified"));
+
+   branch = argv[0];
+
+   if (get_stash_info(, argc - 1, argv + 1))
+   return -1;
+
+   argv_array_pushl(, "checkout", "-b", NULL);
+   argv_array_push(, branch);
+   argv_array_push(, oid_to_hex(_commit));
+   ret = cmd_checkout(args.argc, args.argv, prefix);
+   if (ret) {
+   destroy_stash_info();
+   return -1;
+   }
+
+   ret = do_apply_stash(prefix, , 1);
+   if (!ret && info.is_stash_ref)
+   ret = do_drop_stash(prefix, );
+
+   destroy_stash_info();
+
+   return ret;
+}
+
 int cmd_stash__helper(int argc, const char **argv, const char *prefix)
 {
int result = 0;
@@ -522,6 +567,8 @@ int cmd_stash__helper(int argc, const char **argv, const 
char *prefix)
result = clear_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "drop"))
result = drop_stash(argc, argv, prefix);
+   else if (!strcmp(argv[0], "branch"))
+   result = branch_stash(argc, argv, prefix);
else {
error(_("unknown subcommand: %s"), argv[0]);
usage_with_options(git_stash_helper_usage, options);
diff --git a/git-stash.sh b/git-stash.sh
index 0b8f07b38a..c5fd4c6c44 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'
@@ -672,7 +658,8 @@ pop)
;;
 branch)
shift
-   apply_to_branch "$@"
+   cd "$START_DIR"
+   git stash--helper branch "$@"
;;
 *)
case $# in
-- 
2.16.2

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

Signed-off-by: Joel Teichroeb 
---
 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 'stash branch complains with too many 
refs' '
test_must_fail git stash branch stash-branch stash@{1} stash@{2}
 '
 
+test_expect_success 'stash branch complains with no arguments' '
+   test_must_fail git stash branch
+'
+
 test_expect_success 'stash show format defaults to --stat' '
git stash clear &&
test_when_finished "git reset --hard HEAD" &&
-- 
2.16.2

Angebot Darlehen bei 1.3%

2018-03-25 Thread PSA Finance UK Limited



Dies ist ein Kreditangebot von PSA Finance UK Limited.

Wir bieten alle Arten von Darlehen für Einzelpersonen und Unternehmen für jeden 
Zweck zu 1.3% Zinsen pro Jahr.

Bewerben Sie sich jetzt und weitere Informationen über unser Kreditangebot, 
werden Ihnen zugesandt, sobald wir Ihr Kreditantragsformular erhalten haben.

Ausleihen mindestens von 7,000.00 Euro maximal 50,000,000.00 Euro

FÜLLEN SIE DIE NACHSTEHENDEN INFORMATIONEN

Vollständiger Name:
Land:
Kreditbetrag:
Dauer:
Tel:

Email: info.psaf...@gmail.com

Freundliche Grüße,
Kathleen Fiona

Re: [PATCH 2/2] send-email: supply a --send-delay=1 by default

On Sun, Mar 25, 2018 at 2:28 PM, Ævar Arnfjörð Bjarmason
 wrote:
> The earlier change to add this option described the problem this
> option is trying to solve.
>
> This turns it on by default with a value of 1 second, which'll
> hopefully solve it, and if not user reports as well as the
> X-Mailer-Send-Delay header should help debug it.
> [...]
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -3070,7 +3070,18 @@ sendemail.smtpReloginDelay::
>  sendemail.smtpSendDelay::
> Seconds wait in between message sending before sending another
> -   message. Set it to 0 to impose no extra delay, defaults to 0.
> +   message. Set it to 0 to impose no extra delay, defaults to 1
> +   to wait 1 second.
> ++
> +The reason for imposing a default delay is because certain popular
> +E-Mail clients such as Google's GMail completely ignore the "Date"
> +header, which format-patch is careful to set such that the patches
> +will be displayed in order, and instead sort by the time the E-mail
> +was received.

A minor point: Are you sure that it's git-format-patch that's being
careful about arranging Date: to display in the desired order, and not
git-send-email? Looking at old patches I still have hanging around
which were created with git-format-patch, I see the Date: headers are
wildly out of order, presumably because the date is taken from
Author-Date: and the patches were heavily rebased.

Re: Git Merge contributor summit notes

On Sat, Mar 10 2018, Alex Vandiver wrote:

> New hash (Stefan, etc)
> --
>  - discussed on the mailing list
>  - actual plan checked in to 
> Documentation/technical/hash-function-transition.txt
>  - lots of work renaming
>  - any actual work with the transition plan?
>  - local conversion first; fetch/push have translation table
>  - like git-svn
>  - also modified pack and index format to have lookup/translation efficiently
>  - brian's series to eliminate SHA1 strings from the codebase
>  - testsuite is not working well because hardcoded SHA1 values
>  - flip a bit in the sha1 computation and see what breaks in the testsuite
>  - will also need a way to do the conversion itself; traverse and write out 
> new version
>  - without that, can start new repos, but not work on old ones
>  - on-disk formats will need to change -- something to keep in mind with new 
> index work
>  - documentation describes packfile and index formats
>  - what time frame are we talking?
>  - public perception question
>  - signing commits doesn't help (just signs commit object) unless you 
> "recursive sign"
>  - switched to SHA1dc; we detect and reject known collision technique
>  - do it now because it takes too long if we start when the collision drops
>  - always call it "new hash" to reduce bikeshedding
>  - is translation table a backdoor? has it been reviewed by crypto folks?
>- no, but everything gets translated
>  - meant to avoid a flag day for entire repositories
>  - linus can decide to upgrade to newhash; if pushes to server that is not 
> newhash aware, that's fine
>  - will need a wire protocol change
>  - v2 might add a capability for newhash
>  - "now that you mention md5, it's a good idea"
>  - can use md5 to test the conversion
>  - is there a technical reason for why not /n/ hashes?
>  - the slow step goes away as people converge to the new hash
>  - beneficial to make up some fake hash function for testing
>  - is there a plan on how we decide which hash function?
>  - trust junio to merge commits when appropriate
>  - conservancy committee explicitly does not make code decisions
>  - waiting will just give better data
>  - some hash functions are in silicon (e.g. microsoft cares)
>  - any movement in libgit2 / jgit?
>- basic stuff for libgit2; same testsuite problems
>- no work in jgit
>  - most optimistic forecast?
>- could be done in 1-2y
>  - submodules with one hash function?
>- unable to convert project unless all submodules are converted
>- OO-ing is not a prereq

Late reply, but one thing I brought up at the time is that we'll want to
keep this code around even after the NewHash migration at least for
testing purposes, should we ever need to move to NewNewHash.

It occurred to me recently that once we have such a layer it could be
(ab)used with some relatively minor changes to do any arbitrary
local-to-remote object content translation, unless I've missed something
(but I just re-read hash-function-transition.txt now...).

E.g. having a SHA-1 (or NewHash) local repo, but interfacing with a
remote server so that you upload a GPG encrypted version of all your
blobs, and have your trees reference those blobs.

Because we'd be doing arbitrary translations for all of
commits/trees/blobs this could go further than other bolted-on
encryption solutions for Git. E.g. paths in trees could be encrypted
too, as well as all the content of the commit object that isn't parent
info & the like (but that would have different hashes).

Basically clean/smudge filters on steroids, but for every object in the
repo. Anyone who got a hold of it would still see the shape of the repo
& approximate content size, but other than that it wouldn't be more info
than they'd get via `fast-export --anonymize` now.

I mainly find it interesting because presents an intersection between a
feature we might want to offer anyway, and something that would stress
the hash transition codepath going forward, to make sure it hasn't all
bitrotted by the time we'll need NewHash->NewNewHash.

Git hosting providers would hate it, but they should probably be
charging users by how much Michael Haggerty's git-sizer tool hates their
repo anyway :)

Re: [PATCH 2/2] send-email: supply a --send-delay=1 by default


On Sun, Mar 25 2018, brian m. carlson wrote:

> On Sun, Mar 25, 2018 at 06:28:03PM +, Ævar Arnfjörð Bjarmason wrote:
>> The earlier change to add this option described the problem this
>> option is trying to solve.
>>
>> This turns it on by default with a value of 1 second, which'll
>> hopefully solve it, and if not user reports as well as the
>> X-Mailer-Send-Delay header should help debug it.
>>
>> I think the trade-off of slowing down E-Mail sending to turn this on
>> makes sense because:
>>
>>  * GMail is a really common client, git.git's own unique authors by
>>%aE are ~30% @gmail.com, ~20% for linux.git. That's just patch
>>submitters, my guess is this it's much more common among those who
>>mostly read the list, and those users who aren't using mu4e / mutt
>>etc. anyway.
>>
>>  * There's really no point in having this feature at all if it's not
>>made the default, since the entire point is to be able to read a
>>list like the git ML or the LKML and have patches from others show
>>up in order.
>>
>>  * I don't think anyone's really sensitive to the sending part of
>>send-email taking longer. You just choose "all" and then switch to
>>another terminal while it does its thing if you have a huge series,
>>and for 1-3 patches I doubt anyone would notice this anyway.
>
> I'm not sure that this is going to have the effect you want it to have.
> Let me give an example to demonstrate why.
>
> If I send a series to the list, in order for this to work, you need my
> SMTP server (Postfix) to essentially send mails slowly enough to
> vger.kernel.org (ZMailer) that it doesn't batch them when it sends them
> to GMail.  The problem is that with my mail server, due to filtering and
> such, already takes at least a second to accept, process, and relay
> submitted messages.  vger still batched them and delivered them back to
> me out of order.  This will be even worse with large series.
>
> You are also assuming that my mail server will not have batched them and
> delivered them out of order, which it might well do, since Postfix uses
> a connection cache to machines that don't do STARTTLS (which, much to my
> annoyance, vger doesn't offer).
>
> In short, I don't think this is going to be especially helpful because
> it won't change the status quo for a lot of senders.  You'd have to
> insert some significant delay in order to get the effect you desire, and
> even then things could still be delivered out of order.

Good point. I also see that (via git log --author=Ævar --grep='^\[PATCH
') that this series itself arrived out of order (0 -> 2 -> 1), but I
don't know to what extent public-inbox itself might be batching things.

It would be interesting to get reports from other GMail users as to what
order these mails were shown in, but I think as soon as they're replied
to that info's gone, at least for 2/2, which is the potentially out of
order one in this case.

In general I realize that this won't be a general solution that'll work
in all cases. E.g. I have a local SMTP on my laptop, if I'm on a plane
it wouldn't matter if the delay was 2 hours, it would be batched up
locally and sent all at once.

I was hoping we could find some sweet spot where the systems along the
way (common smtpd's, majordomo, public-inbox's git repo) would as a
result get this right most of the time for the purposes of appeasing
this really common mail client, but maybe that's not going to work out.

Re: [PATCH v2 4/8] completion: factor out _git_xxx calling code

2018-03-25 Thread Recep Aslan

S

iPhone’umdan gönderildi

Re: [PATCH v7 0/3] RUNTIME_PREFIX relocatable Git

On Sun, Mar 25 2018, Dan Jacques wrote:

> This patch set expands support for the RUNTIME_PREFIX configuration flag,
> currently only used on Windows builds, to include Linux, Darwin, and
> FreeBSD. When Git is built with RUNTIME_PREFIX enabled, it resolves its
> ancillary paths relative to the runtime location of its executable
> rather than hard-coding them at compile-time, allowing a Git
> installation to be deployed to a path other than the one in which it
> was built/installed.
>
> Note that RUNTIME_PREFIX is not currently used outside of Windows.
> This patch set should not have an impact on default Git builds.
>
> This is a minor update based on comments from the v6 series. If all's
> well, I'm hoping this set is good to go.

This looks good to me this time around, couple of small nits (maybe
Junio can amend while queuing):

 * You add a dependnecy typo in 2/3 but fix it again in 3/3. Should be
   squashed.

 * s/\Q${gitexecdir_relative}\E$// in 2/3 can be done less verbosely as
   s/\Q$gitexecdir_relative\E$//. Discussed before in

https://public-inbox.org/git/cad1ruu-3q_syvjoru+vey2-0cpmz1el-41z6el7sq4usij0...@mail.gmail.com/
   seems like something you just forgot about.

Re: [PATCH 2/2] send-email: supply a --send-delay=1 by default

On Sun, Mar 25, 2018 at 06:28:03PM +, Ævar Arnfjörð Bjarmason wrote:
> The earlier change to add this option described the problem this
> option is trying to solve.
> 
> This turns it on by default with a value of 1 second, which'll
> hopefully solve it, and if not user reports as well as the
> X-Mailer-Send-Delay header should help debug it.
> 
> I think the trade-off of slowing down E-Mail sending to turn this on
> makes sense because:
> 
>  * GMail is a really common client, git.git's own unique authors by
>%aE are ~30% @gmail.com, ~20% for linux.git. That's just patch
>submitters, my guess is this it's much more common among those who
>mostly read the list, and those users who aren't using mu4e / mutt
>etc. anyway.
> 
>  * There's really no point in having this feature at all if it's not
>made the default, since the entire point is to be able to read a
>list like the git ML or the LKML and have patches from others show
>up in order.
> 
>  * I don't think anyone's really sensitive to the sending part of
>send-email taking longer. You just choose "all" and then switch to
>another terminal while it does its thing if you have a huge series,
>and for 1-3 patches I doubt anyone would notice this anyway.

I'm not sure that this is going to have the effect you want it to have.
Let me give an example to demonstrate why.

If I send a series to the list, in order for this to work, you need my
SMTP server (Postfix) to essentially send mails slowly enough to
vger.kernel.org (ZMailer) that it doesn't batch them when it sends them
to GMail.  The problem is that with my mail server, due to filtering and
such, already takes at least a second to accept, process, and relay
submitted messages.  vger still batched them and delivered them back to
me out of order.  This will be even worse with large series.

You are also assuming that my mail server will not have batched them and
delivered them out of order, which it might well do, since Postfix uses
a connection cache to machines that don't do STARTTLS (which, much to my
annoyance, vger doesn't offer).

In short, I don't think this is going to be especially helpful because
it won't change the status quo for a lot of senders.  You'd have to
insert some significant delay in order to get the effect you desire, and
even then things could still be delivered out of order.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

signature.asc
Description: PGP signature

[PATCH] Remove contrib/examples/*


On Sun, Mar 25 2018, Ævar Arnfjörð Bjarmason wrote:

> There were some side discussions at Git Merge this year about how we
> should just update the README to tell users they can dig these up from
> the history if the need them, do that.
>
> Looking at the "git log" for this directory we get quite a bit more
> patch churn than we should here, mainly from things fixing various
> tree-wide issues.
>
> There's also confusion on the list occasionally about how these should
> be treated, "Re: [PATCH 1/4] stash: convert apply to
> builtin" 
> ()
> being the latest example of that.

The people on CC got this, but it seems the git ML rejected the message
as it's too big. The abbreviated patches is here quoted inline, and at:
https://github.com/avar/git/commit/cc578c81c2cb2999b1a0b73954610bd74951c37b

> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>
> On Sun, Mar 25, 2018 at 6:51 PM, Joel Teichroeb  wrote:
>> 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 remove?
>>
>> I wasn't really sure if I should remove code from the .sh file as it
>> seems in the past the old .sh files have been kept around as examples.
>> Has that been done for previous conversions?
>
> I was skimming this patch and it seemed to me like it would be more
> readable if the *.sh code was removed in the same change (if
> possible). It's easier to review like that.
>
> Also, we should just stop maintainign contrib/examples/*.
>
>  contrib/examples/README|  23 +-
>  contrib/examples/builtin-fetch--tool.c | 575 ---
>  contrib/examples/git-am.sh | 975 
>  contrib/examples/git-checkout.sh   | 302 
>  contrib/examples/git-clean.sh  | 118 ---
>  contrib/examples/git-clone.sh  | 525 -
>  contrib/examples/git-commit.sh | 639 
>  contrib/examples/git-difftool.perl | 481 
>  contrib/examples/git-fetch.sh  | 379 --
>  contrib/examples/git-gc.sh |  37 -
>  contrib/examples/git-log.sh|  15 -
>  contrib/examples/git-ls-remote.sh  | 142 
>  contrib/examples/git-merge-ours.sh |  14 -
>  contrib/examples/git-merge.sh  | 620 
>  contrib/examples/git-notes.sh  | 121 ---
>  contrib/examples/git-pull.sh   | 381 --
>  contrib/examples/git-remote.perl   | 474 
>  contrib/examples/git-repack.sh | 194 -
>  contrib/examples/git-rerere.perl   | 284 ---
>  contrib/examples/git-reset.sh  | 106 ---
>  contrib/examples/git-resolve.sh| 112 ---
>  contrib/examples/git-revert.sh | 207 --
>  contrib/examples/git-svnimport.perl| 976 -
>  contrib/examples/git-svnimport.txt | 179 -
>  contrib/examples/git-tag.sh| 205 --
>  contrib/examples/git-verify-tag.sh |  45 --
>  contrib/examples/git-whatchanged.sh|  28 -
>  27 files changed, 20 insertions(+), 8137 deletions(-)
>  delete mode 100644 contrib/examples/builtin-fetch--tool.c
>  delete mode 100755 contrib/examples/git-am.sh
>  delete mode 100755 contrib/examples/git-checkout.sh
>  delete mode 100755 contrib/examples/git-clean.sh
>  delete mode 100755 contrib/examples/git-clone.sh
>  delete mode 100755 contrib/examples/git-commit.sh
>  delete mode 100755 contrib/examples/git-difftool.perl
>  delete mode 100755 contrib/examples/git-fetch.sh
>  delete mode 100755 contrib/examples/git-gc.sh
>  delete mode 100755 contrib/examples/git-log.sh
>  delete mode 100755 contrib/examples/git-ls-remote.sh
>  delete mode 100755 contrib/examples/git-merge-ours.sh
>  delete mode 100755 contrib/examples/git-merge.sh
>  delete mode 100755 contrib/examples/git-notes.sh
>  delete mode 100755 contrib/examples/git-pull.sh
>  delete mode 100755 contrib/examples/git-remote.perl
>  delete mode 100755 contrib/examples/git-repack.sh
>  delete mode 100755 contrib/examples/git-rerere.perl
>  delete mode 100755 contrib/examples/git-reset.sh
>  delete mode 100755 contrib/examples/git-resolve.sh
>  delete mode 100755 contrib/examples/git-revert.sh
>  delete mode 100755 contrib/examples/git-svnimport.perl
>  delete mode 100644 contrib/examples/git-svnimport.txt
>  delete mode 100755 contrib/examples/git-tag.sh
>  delete mode 100755 contrib/examples/git-verify-tag.sh
>  delete mode 100755 contrib/examples/git-whatchanged.sh
>
> diff --git a/contrib/examples/README b/contrib/examples/README
> index 6946f3dd2a..18bc60b021 100644
> --- a/contrib/examples/README
> +++ b/contrib/examples/README
> @@ -1,3 +1,20 @@
> -These are original scripted

[PATCH v7 3/3] exec_cmd: RUNTIME_PREFIX on some POSIX systems

Enable Git to resolve its own binary location using a variety of
OS-specific and generic methods, including:

- procfs via "/proc/self/exe" (Linux)
- _NSGetExecutablePath (Darwin)
- KERN_PROC_PATHNAME sysctl on BSDs.
- argv0, if absolute (all, including Windows).

This is used to enable RUNTIME_PREFIX support for non-Windows systems,
notably Linux and Darwin. When configured with RUNTIME_PREFIX, Git will
do a best-effort resolution of its executable path and automatically use
this as its "exec_path" for relative helper and data lookups, unless
explicitly overridden.

Small incidental formatting cleanup of "exec_cmd.c".

Signed-off-by: Dan Jacques 
Thanks-to: Robbie Iannucci 
Thanks-to: Junio C Hamano 
---
 Makefile   |  30 ++-
 cache.h|   1 +
 common-main.c  |   4 +-
 config.mak.uname   |   7 ++
 exec_cmd.c | 236 +++--
 exec_cmd.h |   4 +-
 gettext.c  |   8 +-
 git.c  |   2 +-
 t/t0061-run-command.sh |   2 +-
 9 files changed, 254 insertions(+), 40 deletions(-)

diff --git a/Makefile b/Makefile
index 033a55505..f84e816cf 100644
--- a/Makefile
+++ b/Makefile
@@ -441,6 +441,18 @@ all::
 # can be moved to arbitrary filesystem locations. RUNTIME_PREFIX also causes
 # Perl scripts to use a modified entry point header allowing them to resolve
 # support files at runtime.
+#
+# When using RUNTIME_PREFIX, define HAVE_BSD_KERN_PROC_SYSCTL if your platform
+# supports the KERN_PROC BSD sysctl function.
+#
+# When using RUNTIME_PREFIX, define PROCFS_EXECUTABLE_PATH if your platform
+# mounts a "procfs" filesystem capable of resolving the path of the current
+# executable. If defined, this must be the canonical path for the "procfs"
+# current executable path.
+#
+# When using RUNTIME_PREFIX, define HAVE_NS_GET_EXECUTABLE_PATH if your 
platform
+# supports calling _NSGetExecutablePath to retrieve the path of the running
+# executable.
 
 GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1664,10 +1676,23 @@ ifdef HAVE_BSD_SYSCTL
BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
 
+ifdef HAVE_BSD_KERN_PROC_SYSCTL
+   BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL
+endif
+
 ifdef HAVE_GETDELIM
BASIC_CFLAGS += -DHAVE_GETDELIM
 endif
 
+ifneq ($(PROCFS_EXECUTABLE_PATH),)
+   procfs_executable_path_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH))
+   BASIC_CFLAGS += 
'-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"'
+endif
+
+ifdef HAVE_NS_GET_EXECUTABLE_PATH
+   BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
@@ -1772,7 +1797,7 @@ PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA))
 # RUNTIME_PREFIX's resolution logic requires resource paths to be expressed
 # relative to each other and share an installation path.
 #
-# This is a dependnecy in:
+# This is a dependency in:
 # - Git's binary RUNTIME_PREFIX logic in (see "exec_cmd.c").
 # - The runtime prefix Perl header (see
 #   "perl/header_templates/runtime_prefix.template.pl").
@@ -2216,6 +2241,7 @@ endif
 exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX
 exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
+   '-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \
'-DBINDIR="$(bindir_relative_SQ)"' \
'-DPREFIX="$(prefix_SQ)"'
 
@@ -2233,7 +2259,7 @@ attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \
 
 gettext.sp gettext.s gettext.o: GIT-PREFIX
 gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \
-   -DGIT_LOCALE_PATH='"$(localedir_SQ)"'
+   -DGIT_LOCALE_PATH='"$(localedir_relative_SQ)"'
 
 http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS 
+= \
-DCURL_DISABLE_TYPECHECK
diff --git a/cache.h b/cache.h
index a61b2d3f0..d8c55d72b 100644
--- a/cache.h
+++ b/cache.h
@@ -428,6 +428,7 @@ static inline enum object_type object_type(unsigned int 
mode)
 #define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS"
 #define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH"
 #define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
+#define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
 
 /*
  * Environment variable used in handshaking the wire protocol.
diff --git a/common-main.c b/common-main.c
index 6a689007e..6516a1f89 100644
--- a/common-main.c
+++ b/common-main.c
@@ -32,12 +32,12 @@ int main(int argc, const char **argv)
 */
sanitize_stdfds();
 
+   git_resolve_executable_dir(argv[0]);
+
git_setup_gettext();
 
attr_start();
 
-   git_extract_argv0_path(argv[0]);
-
restore_sigpipe_to_default();
 
return cmd_main(argc, argv);
diff --git a/config.mak.uname b/config.mak.uname
index 6a1d0de0c..e1cfe5e5e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -37,6 +37,7 @@ ifeq ($(uname_S),Linux)
HAVE_GETDELIM = YesPlease

[PATCH v7 1/3] Makefile: generate Perl header from template file

Currently, the generated Perl script headers are emitted by commands in
the Makefile. This mechanism restricts options to introduce alternative
header content, needed by Perl runtime prefix support, and obscures the
origin of the Perl script header.

Change the Makefile to generate a header by processing a template file and
move the header content into the "perl/" subdirectory. The generated
header content will now be stored in the "GIT-PERL-HEADER" file. This
allows the content of the Perl header to be controlled by changing the path
of the template in the Makefile.

Signed-off-by: Dan Jacques 
Thanks-to: Ævar Arnfjörð Bjarmason 
Thanks-to: Johannes Schindelin 
---
 .gitignore |  1 +
 Makefile   | 27 +++---
 perl/header_templates/fixed_prefix.template.pl |  1 +
 3 files changed, 18 insertions(+), 11 deletions(-)
 create mode 100644 perl/header_templates/fixed_prefix.template.pl

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..89bd7bd8a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,7 @@
 /GIT-LDFLAGS
 /GIT-PREFIX
 /GIT-PERL-DEFINES
+/GIT-PERL-HEADER
 /GIT-PYTHON-VARS
 /GIT-SCRIPT-DEFINES
 /GIT-USER-AGENT
diff --git a/Makefile b/Makefile
index a1d8775ad..e479822ce 100644
--- a/Makefile
+++ b/Makefile
@@ -1975,20 +1975,15 @@ git.res: git.rc GIT-VERSION-FILE
 $(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
 
 ifndef NO_PERL
-$(SCRIPT_PERL_GEN):
-
+PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
 PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)
-$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-VERSION-FILE
+
+$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER 
GIT-VERSION-FILE
$(QUIET_GEN)$(RM) $@ $@+ && \
-   INSTLIBDIR='$(perllibdir_SQ)' && \
-   INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
-   INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
sed -e '1{' \
-e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
-   -e 'h' \
-   -e 's=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || 
"'"$$INSTLIBDIR"'"));=' \
-   -e 'H' \
-   -e 'x' \
+   -e 'rGIT-PERL-HEADER' \
+   -e 'G' \
-e '}' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
$< >$@+ && \
@@ -2002,6 +1997,16 @@ GIT-PERL-DEFINES: FORCE
echo "$$FLAGS" >$@; \
fi
 
+GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile
+   $(QUIET_GEN)$(RM) $@ && \
+   INSTLIBDIR='$(perllibdir_SQ)' && \
+   INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
+   INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
+   sed -e 's=@@PATHSEP@@=$(pathsep)=g' \
+   -e 's=@@INSTLIBDIR@@='$$INSTLIBDIR'=g' \
+   -e 's=@@PERLLIBDIR@@='$(perllibdir_SQ)'=g' \
+   $< >$@+ && \
+   mv $@+ $@
 
 .PHONY: gitweb
 gitweb:
@@ -2770,7 +2775,7 @@ ifndef NO_TCLTK
 endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
$(RM) GIT-USER-AGENT GIT-PREFIX
-   $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS
+   $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER 
GIT-PYTHON-VARS
 
 .PHONY: all install profile-clean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
diff --git a/perl/header_templates/fixed_prefix.template.pl 
b/perl/header_templates/fixed_prefix.template.pl
new file mode 100644
index 0..857b4391a
--- /dev/null
+++ b/perl/header_templates/fixed_prefix.template.pl
@@ -0,0 +1 @@
+use lib (split(/@@PATHSEP@@/, $ENV{GITPERLLIB} || '@@INSTLIBDIR@@'));
-- 
2.15.0.chromium12

[PATCH v7 2/3] Makefile: add Perl runtime prefix support

Broaden the RUNTIME_PREFIX flag to configure Git's Perl scripts to
locate the Git installation's Perl support libraries by resolving
against the script's path, rather than hard-coding that path at
build-time. Hard-coding at build time worked on previous
RUNTIME_PREFIX configurations (i.e., Windows) because the Perl
scripts were run within a virtual filesystem whose paths were
consistent regardless of the location of the actual installation.
This will no longer be the case for non-Windows RUNTIME_PREFIX users.

When enabled, RUNTIME_PREFIX now requires Perl's system paths to be
expressed relative to a common installation directory in the Makefile,
and uses that relationship to locate support files based on the known
starting point of the script being executed, much like RUNTIME_PREFIX
does for the Git binary.

This change enables Git's Perl scripts to work when their Git installation
is relocated or moved to another system, even when they are not in a
virtual filesystem environment.

Signed-off-by: Dan Jacques 
Thanks-to: Ævar Arnfjörð Bjarmason 
Thanks-to: Johannes Schindelin 
---
 Makefile | 67 +++-
 perl/Git/I18N.pm |  2 +-
 perl/header_templates/runtime_prefix.template.pl | 42 +++
 3 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 perl/header_templates/runtime_prefix.template.pl

diff --git a/Makefile b/Makefile
index e479822ce..033a55505 100644
--- a/Makefile
+++ b/Makefile
@@ -434,6 +434,13 @@ all::
 #
 # When cross-compiling, define HOST_CPU as the canonical name of the CPU on
 # which the built Git will run (for instance "x86_64").
+#
+# Define RUNTIME_PREFIX to configure Git to resolve its ancillary tooling and
+# support files relative to the location of the runtime binary, rather than
+# hard-coding them into the binary. Git installations built with RUNTIME_PREFIX
+# can be moved to arbitrary filesystem locations. RUNTIME_PREFIX also causes
+# Perl scripts to use a modified entry point header allowing them to resolve
+# support files at runtime.
 
 GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -471,6 +478,8 @@ ARFLAGS = rcs
 #   mandir
 #   infodir
 #   htmldir
+#   localedir
+#   perllibdir
 # This can help installing the suite in a relocatable way.
 
 prefix = $(HOME)
@@ -494,7 +503,10 @@ pathsep = :
 
 mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
 infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
+gitexecdir_relative = $(patsubst $(prefix)/%,%,$(gitexecdir))
+localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))
 htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
+perllibdir_relative = $(patsubst $(prefix)/%,%,$(perllibdir))
 
 export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir
 
@@ -1740,10 +1752,13 @@ mandir_relative_SQ = $(subst ','\'',$(mandir_relative))
 infodir_relative_SQ = $(subst ','\'',$(infodir_relative))
 perllibdir_SQ = $(subst ','\'',$(perllibdir))
 localedir_SQ = $(subst ','\'',$(localedir))
+localedir_relative_SQ = $(subst ','\'',$(localedir_relative))
 gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
+gitexecdir_relative_SQ = $(subst ','\'',$(gitexecdir_relative))
 template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
 prefix_SQ = $(subst ','\'',$(prefix))
+perllibdir_relative_SQ = $(subst ','\'',$(perllibdir_relative))
 gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
@@ -1754,6 +1769,31 @@ TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
 PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA))
 
+# RUNTIME_PREFIX's resolution logic requires resource paths to be expressed
+# relative to each other and share an installation path.
+#
+# This is a dependnecy in:
+# - Git's binary RUNTIME_PREFIX logic in (see "exec_cmd.c").
+# - The runtime prefix Perl header (see
+#   "perl/header_templates/runtime_prefix.template.pl").
+ifdef RUNTIME_PREFIX
+
+ifneq ($(filter /%,$(firstword $(gitexecdir_relative))),)
+$(error RUNTIME_PREFIX requires a relative gitexecdir, not: $(gitexecdir))
+endif
+
+ifneq ($(filter /%,$(firstword $(localedir_relative))),)
+$(error RUNTIME_PREFIX requires a relative localedir, not: $(localedir))
+endif
+
+ifndef NO_PERL
+ifneq ($(filter /%,$(firstword $(perllibdir_relative))),)
+$(error RUNTIME_PREFIX requires a relative perllibdir, not: $(perllibdir))
+endif
+endif
+
+endif
+
 # We must filter out any object files from $(GITLIBS),
 # as it is typically used like:
 #
@@ -1974,10 +2014,31 @@ git.res: git.rc GIT-VERSION-FILE
 # This makes sure we depend on the NO_PERL setting itself.
 $(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
 
+# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
+# since the locale directory is injected.
+perl_localedir_SQ = $(localedir_SQ)
+
 ifndef

[PATCH v7 0/3] RUNTIME_PREFIX relocatable Git

This patch set expands support for the RUNTIME_PREFIX configuration flag,
currently only used on Windows builds, to include Linux, Darwin, and
FreeBSD. When Git is built with RUNTIME_PREFIX enabled, it resolves its
ancillary paths relative to the runtime location of its executable
rather than hard-coding them at compile-time, allowing a Git
installation to be deployed to a path other than the one in which it
was built/installed.

Note that RUNTIME_PREFIX is not currently used outside of Windows.
This patch set should not have an impact on default Git builds.

This is a minor update based on comments from the v6 series. If all's
well, I'm hoping this set is good to go.

Previous threads:
v1: https://public-inbox.org/git/20171116170523.28696-1-...@google.com/
v2: https://public-inbox.org/git/20171119173141.4896-1-...@google.com/
v3: https://public-inbox.org/git/20171127164055.93283-1-...@google.com/
v4: https://public-inbox.org/git/20171129223807.91343-1-...@google.com/
v5: https://public-inbox.org/git/20180108030239.92036-1-...@google.com/
v5: https://public-inbox.org/git/20180108030239.92036-1-...@google.com/
v6: https://public-inbox.org/git/20180319025046.58052-1-...@google.com/

Changes in v7 from v6:

- Change Perl header based on avarab@'s suggestion.
- Rearranged Makefile lines to align with avarab@'s patch in next.
- Fix typos in commit messages and comments.

=== Testing ===

The latest patch set is available for testing on my GitHub fork, including
"travis.ci" testing. The "runtime-prefix" branch includes a "config.mak"
commit that enables runtime prefix for the Travis build; the
"runtime-prefix-no-config" omits this file, testing this patch without
runtime prefix enabled:
- https://github.com/danjacques/git/tree/runtime-prefix
- https://github.com/danjacques/git/tree/runtime-prefix-no-config
- https://travis-ci.org/danjacques/git/branches

Built/tested locally using this "config.mak" w/ autoconf:

=== Example config.mak ===

## (BEGIN config.mak)

RUNTIME_PREFIX = YesPlease
RUNTIME_PREFIX_PERL = YesPlease
gitexecdir = libexec/git-core
template_dir = share/git-core/templates
sysconfdir = etc

## (END config.mak)

=== Revision History ===

Changes in v6 from v5:

- Rebased on top of "master".
- Updated commit messages.
- Updated runtime prefix Perl header comment and code to clarify when and
  why FindBin is used.
- With Johannes' blessing on Git-for-Windows, folded "RUNTIME_PREFIX_PERL"
  functionality into "RUNTIME_PREFIX".
- Updated "run-command" test to accommodate RUNTIME_PREFIX trace messages.

Changes in v5 from v4:

- Rebase on top of "next", notably incorporating the
  "ab/simplify-perl-makefile" branch.
- Cleaner Makefile relative path enforcement.
- Update Perl header template path now that the "perl/" directory has
  fewer build-related files in it.
- Update Perl runtime prefix header to use a general system path resolution
  function.
- Implemented the injection of the locale directory into Perl's
  "Git/I18N.pm" module from the runtime prefix Perl script header.
- Updated Perl's "Git/I18N.pm" module to accept injected locale directory.
- Added more content to some comments.


Changes in v4 from v3:

- Incorporated some quoting and Makefile dependency fixes, courtesy of
  .

Changes in v3 from v2:

- Broken into multiple patches now that Perl is isolated in its own
  RUNTIME_PREFIX_PERL flag.
- Working with avarab@, several changes to Perl script runtime prefix
  support:
  - Moved Perl header body content from Makefile into external template
file(s).
  - Added generic "perllibdir" variable to override Perl installation
path.
  - RUNTIME_PREFIX_PERL generated script header is more descriptive and
consistent with how the C version operates.
  - Fixed Generated Perl header Makefile dependency, should rebuild
when dependent files and flags change.
- Changed some of the new RUNTIME_PREFIX trace strings to use consistent
  formatting and terminology.

Changes in v2 from v1:

- Added comments and formatting to improve readability of
  platform-sepecific executable path resolution sleds in
  `git_get_exec_path`.
- Consolidated "cached_exec_path" and "argv_exec_path" globals
  into "exec_path_value".
- Use `strbuf_realpath` instead of `realpath` for procfs resolution.
- Removed new environment variable exports. Git with RUNTIME_PREFIX no
  longer exports or consumes any additional environment information.
- Updated Perl script resolution strategy: rather than having Git export
  the relative executable path to the Perl scripts, they now resolve
  it independently when RUNTIME_PREFIX_PERL is enabled.
- Updated resolution strategy for "gettext()": use system_path() instead
  of special environment variable.
- Added `sysctl` executable resolution support for BSDs that don't
  mount "procfs" by default (most of them).

Dan Jacques (3):
  Makefile: generate Perl header from template file
  Makefile: add Perl runtime prefix support
  exec_cmd:

Re: [PATCH v3] filter-branch: fix errors caused by refs that point at non-committish

Yuki Kokubun  writes:

> References: 
>   
>   <5ab46520.0352650a.cc02b.a...@mx.google.com>
>   <20180323050913.5188-1-orga.chem@gmail.com> 
> Content-Type: text/plain
>
>> Grammo (third-person singular 'prints' misspelt without 's'; the
>> "when" clause has a complex subject but no verb).
>> 
>> Perhaps this will salvage what you meant:
>> 
>>  "git filter-branch -- --all" prints error messages when
>>  processing refs that point at objects that are not
>>  committish.
>
> Thanks. I'm gonna fix it.
>
>> Please sign-off your patch (cf. Documentation/SubmittingPatches).
>
> OK I'm gonna add it.
>
> I greatly appreciate for your kind review.
> I couldn't write this patch without your help.
> Thanks.

Heh, it's a team effort, and it is not like I am helping your effort
to build "open source contribution credit" for your coursework ;-)

Thank *you* for contributing to make Git a better system.

Re: Should I try to fix rebase interactive preserve-merges bug

2018-03-25 Thread Wink Saville

> AFAIK this breakage of preserve-merges is a design flaw which isn't
> really fixable, which is why --recreate-merges is being added.
>
> I believe the plan is to deprecate preserve-merges once
> recreate-merges has landed.
>
> Thanks,
> Jake


Txs for the info, hopefully others will respond.

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

On Sun, Mar 25, 2018 at 6:51 PM, Joel Teichroeb  wrote:
> 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 remove?
>
> I wasn't really sure if I should remove code from the .sh file as it
> seems in the past the old .sh files have been kept around as examples.

Yeah, some original shell scripts that have been converted are kept in
contrib/examples/, but the shell code has still been removed from the
.sh files when they were being converted.

> Has that been done for previous conversions?

I don't think there were some cases when the shell code was not
removed. I haven't looked at all the conversions in details though.

[PATCH 04/10] t1405: sort reflog entries in a hash-independent way

The test enumerates reflog entries in an arbitrary order and then sorts
them.  For SHA-1, this produces results that happen to sort in
alphabetical order, but for other hash algorithms they sort differently.
Ensure we sort the reflog entries in a hash-independent way by sorting
on the ref name instead of the object ID.  Remove an assumption about
the length of a hash by using cut with the delimiter and field options
instead of the character range option.

Signed-off-by: brian m. carlson 
---
 t/t1405-main-ref-store.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index e8115df5ba..a1e243a05c 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -45,7 +45,7 @@ test_expect_success 'rename_refs(master, new-master)' '
 '
 
 test_expect_success 'for_each_ref(refs/heads/)' '
-   $RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+   $RUN for-each-ref refs/heads/ | cut -d" " -f 2- >actual &&
cat >expected <<-\EOF &&
master 0x0
new-master 0x0
@@ -71,7 +71,7 @@ test_expect_success 'verify_ref(new-master)' '
 '
 
 test_expect_success 'for_each_reflog()' '
-   $RUN for-each-reflog | sort | cut -c 42- >actual &&
+   $RUN for-each-reflog | sort -k2 | cut -c 42- >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
refs/heads/master 0x0

[PATCH 01/10] t1011: abstract away SHA-1-specific constants

Adjust the test so that it computes the expected hash value dynamically
instead of relying on a hard-coded hash.  Hoist some code earlier in the
test to make this possible.

Signed-off-by: brian m. carlson 
---
 t/t1011-read-tree-sparse-checkout.sh | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/t1011-read-tree-sparse-checkout.sh 
b/t/t1011-read-tree-sparse-checkout.sh
index c167f606ca..0c6f48f302 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -15,8 +15,11 @@ test_description='sparse checkout tests
 . "$TEST_DIRECTORY"/lib-read-tree.sh
 
 test_expect_success 'setup' '
+   test_commit init &&
+   echo modified >>init.t &&
+
cat >expected <<-EOF &&
-   100644 77f0ba1734ed79d12881f81b36ee134de6a3327b 0   init.t
+   100644 $(git hash-object init.t) 0  init.t
100644 $EMPTY_BLOB 0sub/added
100644 $EMPTY_BLOB 0sub/addedtoo
100644 $EMPTY_BLOB 0subsub/added
@@ -28,8 +31,6 @@ test_expect_success 'setup' '
H subsub/added
EOF
 
-   test_commit init &&
-   echo modified >>init.t &&
mkdir sub subsub &&
touch sub/added sub/addedtoo subsub/added &&
git add init.t sub/added sub/addedtoo subsub/added &&

[PATCH 03/10] t1300: abstract away SHA-1-specific constants

Adjust the test so that it uses the computed blob value instead of
hard-coding a hash.

Signed-off-by: brian m. carlson 
---
 t/t1300-repo-config.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 4f8e6f5fde..dc7e6c2e77 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1587,10 +1587,10 @@ test_expect_success '--show-origin stdin with file 
include' '
 '
 
 test_expect_success !MINGW '--show-origin blob' '
-   cat >expect <<-\EOF &&
-   blob:a9d9f9e555b5c6f07cbe09d3f06fe3df11e09c08   user.custom=true
-   EOF
blob=$(git hash-object -w "$CUSTOM_CONFIG_FILE") &&
+   cat >expect <<-EOF &&
+   blob:$blob  user.custom=true
+   EOF
git config --blob=$blob --show-origin --list >output &&
test_cmp expect output
 '

[PATCH 10/10] t2107: abstract away SHA-1-specific constants

Use the $EMPTY_BLOB variable instead of hard-coding a hash.

Signed-off-by: brian m. carlson 
---
 t/t2107-update-index-basic.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 32ac6e09bd..1db7e6a1ab 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -85,9 +85,9 @@ test_expect_success '--chmod=+x and chmod=-x in the same 
argument list' '
>B &&
git add A B &&
git update-index --chmod=+x A --chmod=-x B &&
-   cat >expect <<-\EOF &&
-   100755 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0   A
-   100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0   B
+   cat >expect <<-EOF &&
+   100755 $EMPTY_BLOB 0A
+   100644 $EMPTY_BLOB 0B
EOF
git ls-files --stage A B >actual &&
test_cmp expect actual

[PATCH 05/10] t1411: abstract away SHA-1-specific constants

Adjust the test so that it uses a variable consisting of the current
HEAD instead of a hard-coded hash.

Signed-off-by: brian m. carlson 
---
 t/t1411-reflog-show.sh | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 6ac7734d79..596907758d 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -10,6 +10,7 @@ test_expect_success 'setup' '
git commit -m one
 '
 
+commit=$(git rev-parse --short HEAD)
 cat >expect <<'EOF'
 Reflog: HEAD@{0} (C O Mitter )
 Reflog message: commit (initial): one
@@ -20,8 +21,8 @@ test_expect_success 'log -g shows reflog headers' '
test_cmp expect actual
 '
 
-cat >expect <<'EOF'
-e46513e HEAD@{0}: commit (initial): one
+cat >expect expect <<'EOF'
-commit e46513e
+cat >expect <)
 Reflog message: commit (initial): one
 Author: A U Thor 
@@ -56,8 +57,8 @@ test_expect_success 'using @{now} syntax shows reflog date 
(multiline)' '
test_cmp expect actual
 '
 
-cat >expect <<'EOF'
-e46513e HEAD@{Thu Apr 7 15:13:13 2005 -0700}: commit (initial): one
+cat >expect expect <<'EOF'
-e46513e HEAD@{Thu Apr 7 15:13:13 2005 -0700}: commit (initial): one
+cat >expect expect <<'EOF'
-e46513e HEAD@{0}: commit (initial): one
+cat >expect <

[PATCH 09/10] t2101: abstract away SHA-1-specific constants

Adjust the test so that it uses variables and command substitution for
blobs instead of hard-coded hashes.

Signed-off-by: brian m. carlson 
---
 t/t2101-update-index-reupdate.sh | 41 
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/t/t2101-update-index-reupdate.sh b/t/t2101-update-index-reupdate.sh
index 168733a3c7..685ec45639 100755
--- a/t/t2101-update-index-reupdate.sh
+++ b/t/t2101-update-index-reupdate.sh
@@ -8,15 +8,15 @@ test_description='git update-index --again test.
 
 . ./test-lib.sh
 
-cat > expected <<\EOF
-100644 3b18e512dba79e4c8300dd08aeb37f8e728b8dad 0  file1
-100644 9db8893856a8a02eaa73470054b7c1c5a7c82e47 0  file2
-EOF
 test_expect_success 'update-index --add' '
echo hello world >file1 &&
echo goodbye people >file2 &&
git update-index --add file1 file2 &&
git ls-files -s >current &&
+   cat >expected <<-EOF &&
+   100644 $(git hash-object file1) 0   file1
+   100644 $(git hash-object file2) 0   file2
+   EOF
cmp current expected
 '
 
@@ -34,21 +34,17 @@ test_expect_success 'update-index --again' '
cmp current expected
 '
 
-cat > expected <<\EOF
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0  file2
-EOF
 test_expect_success 'update-index --remove --again' '
git update-index --remove --again &&
git ls-files -s >current &&
+   cat >expected <<-EOF &&
+   100644 $(git hash-object file2) 0   file2
+   EOF
cmp current expected
 '
 
 test_expect_success 'first commit' 'git commit -m initial'
 
-cat > expected <<\EOF
-100644 53ab446c3f4e42ce9bb728a0ccb283a101be4979 0  dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0  file2
-EOF
 test_expect_success 'update-index again' '
mkdir -p dir1 &&
echo hello world >dir1/file3 &&
@@ -58,13 +54,14 @@ test_expect_success 'update-index again' '
echo happy >dir1/file3 &&
git update-index --again &&
git ls-files -s >current &&
+   cat >expected <<-EOF &&
+   100644 $(git hash-object dir1/file3) 0  dir1/file3
+   100644 $(git hash-object file2) 0   file2
+   EOF
cmp current expected
 '
 
-cat > expected <<\EOF
-100644 d7fb3f695f06c759dbf3ab00046e7cc2da22d10f 0  dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0  file2
-EOF
+file2=$(git hash-object file2)
 test_expect_success 'update-index --update from subdir' '
echo not so happy >file2 &&
(cd dir1 &&
@@ -72,18 +69,22 @@ test_expect_success 'update-index --update from subdir' '
git update-index --again
) &&
git ls-files -s >current &&
-   cmp current expected
+   cat >expected <<-EOF &&
+   100644 $(git hash-object dir1/file3) 0  dir1/file3
+   100644 $file2 0 file2
+   EOF
+   test_cmp current expected
 '
 
-cat > expected <<\EOF
-100644 594fb5bb1759d90998e2bf2a38261ae8e243c760 0  dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0  file2
-EOF
 test_expect_success 'update-index --update with pathspec' '
echo very happy >file2 &&
cat file2 >dir1/file3 &&
git update-index --again dir1/ &&
git ls-files -s >current &&
+   cat >expected <<-EOF &&
+   100644 $(git hash-object dir1/file3) 0  dir1/file3
+   100644 $file2 0 file2
+   EOF
cmp current expected
 '

[PATCH 08/10] t2101: modernize test style

Most of our tests start with the opening quote of the test body on the
same line as the test_expect_success call.  Additionally, our tests are
usually indented with a single tab.  Update this test to be the same as
most others, which will make it easier to use inline heredocs in the
future.

Signed-off-by: brian m. carlson 
---
 t/t2101-update-index-reupdate.sh | 52 ++--
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/t/t2101-update-index-reupdate.sh b/t/t2101-update-index-reupdate.sh
index c8bce8c2e4..168733a3c7 100755
--- a/t/t2101-update-index-reupdate.sh
+++ b/t/t2101-update-index-reupdate.sh
@@ -12,15 +12,16 @@ cat > expected <<\EOF
 100644 3b18e512dba79e4c8300dd08aeb37f8e728b8dad 0  file1
 100644 9db8893856a8a02eaa73470054b7c1c5a7c82e47 0  file2
 EOF
-test_expect_success 'update-index --add' \
-   'echo hello world >file1 &&
-echo goodbye people >file2 &&
-git update-index --add file1 file2 &&
-git ls-files -s >current &&
-cmp current expected'
+test_expect_success 'update-index --add' '
+   echo hello world >file1 &&
+   echo goodbye people >file2 &&
+   git update-index --add file1 file2 &&
+   git ls-files -s >current &&
+   cmp current expected
+'
 
-test_expect_success 'update-index --again' \
-   'rm -f file1 &&
+test_expect_success 'update-index --again' '
+   rm -f file1 &&
echo hello everybody >file2 &&
if git update-index --again
then
@@ -29,16 +30,18 @@ test_expect_success 'update-index --again' \
else
echo happy - failed as expected
fi &&
-git ls-files -s >current &&
-cmp current expected'
+   git ls-files -s >current &&
+   cmp current expected
+'
 
 cat > expected <<\EOF
 100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0  file2
 EOF
-test_expect_success 'update-index --remove --again' \
-   'git update-index --remove --again &&
-git ls-files -s >current &&
-cmp current expected'
+test_expect_success 'update-index --remove --again' '
+   git update-index --remove --again &&
+   git ls-files -s >current &&
+   cmp current expected
+'
 
 test_expect_success 'first commit' 'git commit -m initial'
 
@@ -46,8 +49,8 @@ cat > expected <<\EOF
 100644 53ab446c3f4e42ce9bb728a0ccb283a101be4979 0  dir1/file3
 100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0  file2
 EOF
-test_expect_success 'update-index again' \
-   'mkdir -p dir1 &&
+test_expect_success 'update-index again' '
+   mkdir -p dir1 &&
echo hello world >dir1/file3 &&
echo goodbye people >file2 &&
git update-index --add file2 dir1/file3 &&
@@ -55,30 +58,33 @@ test_expect_success 'update-index again' \
echo happy >dir1/file3 &&
git update-index --again &&
git ls-files -s >current &&
-   cmp current expected'
+   cmp current expected
+'
 
 cat > expected <<\EOF
 100644 d7fb3f695f06c759dbf3ab00046e7cc2da22d10f 0  dir1/file3
 100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0  file2
 EOF
-test_expect_success 'update-index --update from subdir' \
-   'echo not so happy >file2 &&
+test_expect_success 'update-index --update from subdir' '
+   echo not so happy >file2 &&
(cd dir1 &&
cat ../file2 >file3 &&
git update-index --again
) &&
git ls-files -s >current &&
-   cmp current expected'
+   cmp current expected
+'
 
 cat > expected <<\EOF
 100644 594fb5bb1759d90998e2bf2a38261ae8e243c760 0  dir1/file3
 100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0  file2
 EOF
-test_expect_success 'update-index --update with pathspec' \
-   'echo very happy >file2 &&
+test_expect_success 'update-index --update with pathspec' '
+   echo very happy >file2 &&
cat file2 >dir1/file3 &&
git update-index --again dir1/ &&
git ls-files -s >current &&
-   cmp current expected'
+   cmp current expected
+'
 
 test_done

[PATCH 06/10] t1507: abstract away SHA-1-specific constants

Adjust the test so that it uses a variable consisting of the current
HEAD instead of a hard-coded hash.

Signed-off-by: brian m. carlson 
---
 t/t1507-rev-parse-upstream.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 2ce68cc277..93c77eac45 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -209,8 +209,9 @@ test_expect_success '@{u} works when tracking a local 
branch' '
test refs/heads/master = "$(full_name @{u})"
 '
 
+commit=$(git rev-parse HEAD)
 cat >expect <)
 Reflog message: branch: Created from HEAD
 Author: A U Thor 
@@ -224,7 +225,7 @@ test_expect_success 'log -g other@{u}' '
 '
 
 cat >expect <)
 Reflog message: branch: Created from HEAD
 Author: A U Thor

[PATCH 07/10] t2020: abstract away SHA-1 specific constants

Adjust the test so that it uses variables for the revisions we're
checking out instead of hard-coded hashes.

Signed-off-by: brian m. carlson 
---
 t/t2020-checkout-detach.sh | 40 +++---
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index bb4f2e0c63..1fa670625c 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -189,8 +189,12 @@ test_expect_success 'no advice given for explicit detached 
head state' '
 # Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (new format)
 test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not 
asked to' "
 
+   commit=$(git rev-parse --short=12 master^) &&
+   commit2=$(git rev-parse --short=12 master~2) &&
+   commit3=$(git rev-parse --short=12 master~3) &&
+
# The first detach operation is more chatty than the following ones.
-   cat >1st_detach <<-'EOF' &&
+   cat >1st_detach <<-EOF &&
Note: checking out 'HEAD^'.
 
You are in 'detached HEAD' state. You can look around, make experimental
@@ -202,18 +206,18 @@ test_expect_success 'describe_detached_head prints no 
SHA-1 ellipsis when not as
 
  git checkout -b 
 
-   HEAD is now at 7c7cd714e262 three
+   HEAD is now at \$commit three
EOF
 
# The remaining ones just show info about previous and current HEADs.
-   cat >2nd_detach <<-'EOF' &&
-   Previous HEAD position was 7c7cd714e262 three
-   HEAD is now at 139b20d8e6c5 two
+   cat >2nd_detach <<-EOF &&
+   Previous HEAD position was \$commit three
+   HEAD is now at \$commit2 two
EOF
 
-   cat >3rd_detach <<-'EOF' &&
-   Previous HEAD position was 139b20d8e6c5 two
-   HEAD is now at d79ce1670bdc one
+   cat >3rd_detach <<-EOF &&
+   Previous HEAD position was \$commit2 two
+   HEAD is now at \$commit3 one
EOF
 
reset &&
@@ -261,8 +265,12 @@ test_expect_success 'describe_detached_head prints no 
SHA-1 ellipsis when not as
 # Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (old format)
 test_expect_success 'describe_detached_head does print SHA-1 ellipsis when 
asked to' "
 
+   commit=$(git rev-parse --short=12 master^) &&
+   commit2=$(git rev-parse --short=12 master~2) &&
+   commit3=$(git rev-parse --short=12 master~3) &&
+
# The first detach operation is more chatty than the following ones.
-   cat >1st_detach <<-'EOF' &&
+   cat >1st_detach <<-EOF &&
Note: checking out 'HEAD^'.
 
You are in 'detached HEAD' state. You can look around, make experimental
@@ -274,18 +282,18 @@ test_expect_success 'describe_detached_head does print 
SHA-1 ellipsis when asked
 
  git checkout -b 
 
-   HEAD is now at 7c7cd714e262... three
+   HEAD is now at \$commit... three
EOF
 
# The remaining ones just show info about previous and current HEADs.
-   cat >2nd_detach <<-'EOF' &&
-   Previous HEAD position was 7c7cd714e262... three
-   HEAD is now at 139b20d8e6c5... two
+   cat >2nd_detach <<-EOF &&
+   Previous HEAD position was \$commit... three
+   HEAD is now at \$commit2... two
EOF
 
-   cat >3rd_detach <<-'EOF' &&
-   Previous HEAD position was 139b20d8e6c5... two
-   HEAD is now at d79ce1670bdc... one
+   cat >3rd_detach <<-EOF &&
+   Previous HEAD position was \$commit2... two
+   HEAD is now at \$commit3... one
EOF
 
reset &&

[PATCH 02/10] t1304: abstract away SHA-1-specific constants

Adjust the test so that it uses the $EMPTY_BLOB value instead of
hard-coding the hash.

Signed-off-by: brian m. carlson 
---
 t/t1304-default-acl.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index f5422f1d33..335d3f3211 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -54,7 +54,7 @@ test_expect_success SETFACL 'Setup test repo' '
 
 test_expect_success SETFACL 'Objects creation does not break ACLs with 
restrictive umask' '
# SHA1 for empty blob
-   check_perms_and_acl 
.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
+   check_perms_and_acl .git/objects/$(echo $EMPTY_BLOB | sed -e 
"s,^\(..\),\1/,")
 '
 
 test_expect_success SETFACL 'git gc does not break ACLs with restrictive 
umask' '

[PATCH 00/10] Hash-independent tests (part 1)

This is a series to make our tests hash-independent.  Many tests have
hard-coded SHA-1 values in them, and it would be valuable to express
these items in a hash-independent way for our hash transitions.

The approach in this series relies on only three components for hash
independence: git rev-parse, git hash-object, and EMPTY_BLOB and
EMPTY_TREE.  Because many of our shell scripts and test components
already rely on the first two, this seems like a safe assumption.

For the same reason, this series avoids modifying tests that test these
components or their expected SHA-1 values.  I expect that when we add
another hash function, we'll copy these tests to expose both SHA-1 and
NewHash versions.

Many of our tests use heredocs for defining expected values.  My
approach has been to interpolate values into the heredocs, as that
produces the best readability in my view.

These tests have been tested using my "short BLAKE2b" series (branch
blake2b-test-hash) and have also been tested based off master.

Comments on any aspect of this series are welcome, but opinions on the
approach or style are especially so.

brian m. carlson (10):
  t1011: abstract away SHA-1-specific constants
  t1304: abstract away SHA-1-specific constants
  t1300: abstract away SHA-1-specific constants
  t1405: sort reflog entries in a hash-independent way
  t1411: abstract away SHA-1-specific constants
  t1507: abstract away SHA-1-specific constants
  t2020: abstract away SHA-1 specific constants
  t2101: modernize test style
  t2101: abstract away SHA-1-specific constants
  t2107: abstract away SHA-1-specific constants

 t/t1011-read-tree-sparse-checkout.sh |  7 ++-
 t/t1300-repo-config.sh   |  6 +-
 t/t1304-default-acl.sh   |  2 +-
 t/t1405-main-ref-store.sh|  4 +-
 t/t1411-reflog-show.sh   | 21 ---
 t/t1507-rev-parse-upstream.sh|  5 +-
 t/t2020-checkout-detach.sh   | 40 +++-
 t/t2101-update-index-reupdate.sh | 91 +++-
 t/t2107-update-index-basic.sh|  6 +-
 9 files changed, 100 insertions(+), 82 deletions(-)

Re: [ANNOUNCE] Git v2.17.0-rc1

On 3/25/2018 2:42 PM, Ævar Arnfjörð Bjarmason wrote:

On Sun, Mar 25 2018, Derrick Stolee wrote:

On 3/23/2018 1:59 PM, Ævar Arnfjörð Bjarmason wrote:

On Wed, Mar 21 2018, Junio C. Hamano wrote:

A release candidate Git v2.17.0-rc1 is now available for testing
at the usual places. It is comprised of 493 non-merge commits
since v2.16.0, contributed by 62 people, 19 of which are new faces.

I have this deployed on some tens of K machines who all use git in one
way or another (from automated pulls, to users interactively), and rc0
before that, with a few patches on top from me + Takato + Duy + Derrick
since rc0 was released (and since today based on top of rc1). No issues
so far.

The specific in-house version I have is at:
https://github.com/git/git/compare/v2.17.0-rc1...bookingcom:booking-git-v2018-03-23-1

Thanks for testing the commit-graph feature, Ævar! I'm guessing you
have some mechanisms to ensure the 'git commit-graph write' command is
run on these machines and 'core.commitGraph' is set to true in the
config? I would love to hear how this benefits your org.

I haven't deployed any actual use of it at a wider scale, but I've done
some ad-hoc benchmarking with our internal version which has your
patches, and the results are very promising so far on the isolated test
cases where it helps (that you know about, e.g. rev-list --all).

So sorry, I don't have any meaningful testing of this, I just wanted an
easy way to ad-hoc test it & make sure it doesn't break other stuff for
now.

I also threw out most of the manual git maintenance stuff we had and
just rely on gc --auto now, so as soon as you have something to
integrate with that, along with those perf changes Peff suggested I'm
much more likely to play with it in some real way.

Thanks. Integration with 'gc --auto' is a high priority for me after the
patch lands.

The version on GitHub [1] is slightly ahead of v6 as I wait to reroll on
v2.17.0. It includes Peff's improvements to inspecting pack-indexes [2].

[1] https://github.com/derrickstolee/git/pull/2
[2]
https://github.com/derrickstolee/git/pull/2/commits/cb86817ee5c5127b32c93a22ef130f0db6207970

Re: [ANNOUNCE] Git v2.17.0-rc1

On Sun, Mar 25 2018, Derrick Stolee wrote:

> On 3/23/2018 1:59 PM, Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Mar 21 2018, Junio C. Hamano wrote:
>>
>>> A release candidate Git v2.17.0-rc1 is now available for testing
>>> at the usual places.  It is comprised of 493 non-merge commits
>>> since v2.16.0, contributed by 62 people, 19 of which are new faces.
>> I have this deployed on some tens of K machines who all use git in one
>> way or another (from automated pulls, to users interactively), and rc0
>> before that, with a few patches on top from me + Takato + Duy + Derrick
>> since rc0 was released (and since today based on top of rc1). No issues
>> so far.
>>
>> The specific in-house version I have is at:
>> https://github.com/git/git/compare/v2.17.0-rc1...bookingcom:booking-git-v2018-03-23-1
>
> Thanks for testing the commit-graph feature, Ævar! I'm guessing you
> have some mechanisms to ensure the 'git commit-graph write' command is
> run on these machines and 'core.commitGraph' is set to true in the
> config? I would love to hear how this benefits your org.

I haven't deployed any actual use of it at a wider scale, but I've done
some ad-hoc benchmarking with our internal version which has your
patches, and the results are very promising so far on the isolated test
cases where it helps (that you know about, e.g. rev-list --all).

So sorry, I don't have any meaningful testing of this, I just wanted an
easy way to ad-hoc test it & make sure it doesn't break other stuff for
now.

I also threw out most of the manual git maintenance stuff we had and
just rely on gc --auto now, so as soon as you have something to
integrate with that, along with those perf changes Peff suggested I'm
much more likely to play with it in some real way.

Re: [RFC][PATCH] git-stash: convert git stash list to C builtin

On 03/24, Paul-Sebastian Ungureanu wrote:
> Currently, because git stash is not fully converted to C, I
> introduced a new helper that will hold the converted commands.

Missing sign-off?  I think it's a good idea to sign off your work even
for RFC patches that you don't expect to be applied.  If for nothing
else, it helps people not wonder whether you just forgot the sign-off
or if you intentionally didn't add it.

> ---
>  Makefile|  1 +
>  builtin.h   |  1 +
>  builtin/stash--helper.c | 52 +
>  git-stash.sh|  7 +-
>  git.c   |  1 +
>  5 files changed, 56 insertions(+), 6 deletions(-)
>  create mode 100644 builtin/stash--helper.c
> 
> diff --git a/Makefile b/Makefile
> index a1d8775ad..8ca361c57 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1020,6 +1020,7 @@ BUILTIN_OBJS += builtin/send-pack.o
>  BUILTIN_OBJS += builtin/shortlog.o
>  BUILTIN_OBJS += builtin/show-branch.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 42378f3aa..2ddb4bd5c 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -220,6 +220,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_status(int argc, const char **argv, const char *prefix);
>  extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
> +extern int cmd_stash__helper(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);
>  extern int cmd_tag(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 0..61fd5390d
> --- /dev/null
> +++ b/builtin/stash--helper.c
> @@ -0,0 +1,52 @@
> +#include "builtin.h"
> +#include "cache.h"
> +#include "parse-options.h"
> +#include "argv-array.h"
> +
> +enum {
> + LIST_STASH = 1
> +};
> +
> +static const char * ref_stash = "refs/stash";
> +
> +static const char * const git_stash__helper_usage[] = {
> + N_("git stash--helper --list []"),
> + NULL
> +};
> +
> +static int list_stash(int argc, const char **argv, const char *prefix)
> +{
> + struct object_id obj;
> + struct argv_array args = ARGV_ARRAY_INIT;
> +
> + if (get_oid(ref_stash, ))
> + return 0;
> +
> + argv_array_pushl(, "log", "--format=%gd: %gs", "-g", 
> "--first-parent", "-m", NULL);
> + argv_array_pushv(, argv);
> + argv_array_push(, ref_stash);

This is missing the final '--' argument to 'git log'.  It's needed to
disambiguate the ref from paths.  If we don't have that, this git log
call will fail when a "refs/stash" directory exists.

> + return !!cmd_log(args.argc, args.argv, prefix);
> +}
> +
> +int cmd_stash__helper(int argc, const char **argv, const char *prefix)
> +{
> + int cmdmode = 0;
> +
> + struct option options[] = {
> + OPT_CMDMODE(0, "list", ,
> +  N_("list stash entries"), LIST_STASH),
> + OPT_END()
> + };
> +
> + argc = parse_options(argc, argv, prefix, options,
> +  git_stash__helper_usage, PARSE_OPT_KEEP_UNKNOWN);
> +
> + if (!cmdmode)
> + usage_with_options(git_stash__helper_usage, options);
> +
> + switch (cmdmode) {
> + case LIST_STASH:
> + return list_stash(argc, argv, prefix);
> + }
> + return 0;
> +}
> diff --git a/git-stash.sh b/git-stash.sh
> index fc8f8ae64..a5b9f5fb6 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -380,11 +380,6 @@ have_stash () {
>   git rev-parse --verify --quiet $ref_stash >/dev/null
>  }
>  
> -list_stash () {
> - have_stash || return 0
> - git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash --
> -}
> -
>  show_stash () {
>   ALLOW_UNKNOWN_FLAGS=t
>   assert_stash_like "$@"
> @@ -695,7 +690,7 @@ test -n "$seen_non_option" || set "push" "$@"
>  case "$1" in
>  list)
>   shift
> - list_stash "$@"
> + git stash--helper --list "$@"
>   ;;
>  show)
>   shift
> diff --git a/git.c b/git.c
> index 96cd734f1..6fd2ccd9a 100644
> --- a/git.c
> +++ b/git.c
> @@ -466,6 +466,7 @@ static struct cmd_struct commands[] = {
>   { "show-branch", cmd_show_branch, RUN_SETUP },
>   { "show-ref", cmd_show_ref, RUN_SETUP },
>   { "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
> + { "stash--helper", cmd_stash__helper, RUN_SETUP },
>   { "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
>   { "stripspace", cmd_stripspace },
>   { "submodule--helper", cmd_submodule__helper,

Re: [PATCH] unpack-trees: release oid_array after use in check_updates()


On 3/25/2018 12:31 PM, René Scharfe wrote:

Signed-off-by: Rene Scharfe 
---
That leak was introduced by c0c578b33c (unpack-trees: batch fetching of
missing blobs).

  unpack-trees.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/unpack-trees.c b/unpack-trees.c
index d5685891a5..e73745051e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -379,30 +379,31 @@ static int check_updates(struct unpack_trees_options *o)
struct oid_array to_fetch = OID_ARRAY_INIT;
int fetch_if_missing_store = fetch_if_missing;
fetch_if_missing = 0;
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
if ((ce->ce_flags & CE_UPDATE) &&
!S_ISGITLINK(ce->ce_mode)) {
if (!has_object_file(>oid))
oid_array_append(_fetch, >oid);
}
}
if (to_fetch.nr)
fetch_objects(repository_format_partial_clone,
  _fetch);
fetch_if_missing = fetch_if_missing_store;
+   oid_array_clear(_fetch);
}
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
  
  		if (ce->ce_flags & CE_UPDATE) {

if (ce->ce_flags & CE_WT_REMOVE)
die("BUG: both update and delete flags are set on 
%s",
ce->name);
display_progress(progress, ++cnt);
ce->ce_flags &= ~CE_UPDATE;
if (o->update && !o->dry_run) {
errs |= checkout_entry(ce, , NULL);
}
}
}


Ack. Looks correct.

-Stolee

[PATCH 1/2] send-email: add an option to impose delay sent E-Mails

Add a --send-delay option with a corresponding sendemail.smtpSendDelay
configuration variable. When set to e.g. 2, this causes send-email to
sleep 2 seconds before sending the next E-Mail. We'll only sleep
between sends, not before the first send, or after the last.

This option is useful because certain popular E-Mail clients
completely ignore the "Date" header, which format-patch is careful to
set such that the patches will be displayed in order, and instead sort
by the time the E-mail was received.

Google's GMail is a good example of such a client. It ostensibly sorts
by some approximation of received time (although not by any "Received"
header). It's more usual than not to see patches showing out of order
in GMail. To take a few examples of orders seen on recent patches on
the Git mailing list:

1 -> 3 -> 4 -> 2 -> 8 -> 7 (completion by Nguyễn Thái Ngọc Duy)
2 -> 0 -> 1 -> 3 (pack search by Derrick Stolee)
3 -> 2 -> 1 (fast-import by Jameson Miller)
2 -> 3 -> 1 -> 5 -> 4 -> 6 (diff-highlight by Jeff King)

My own patches to Git are always in order because this bothers me
enough that I never use the "all" option to send-email, instead I wait
a second and manually type "y" for each one, so I have years of
anecdotal data showing that this works with GMail. This new option
will allow me to set a config to do what I've been doing manually.

I've also noticed that E-Mail from some other contributors tends to be
in order, e.g. Michael Haggerty's usually are. When I asked him about
he just uses the "all" option, but digging further revealed that the
details of his mail routing were imposing a similar delay, so his
mails similarly arrived at Google with some delay.

The reason to add the new "X-Mailer-Send-Delay" header is to make it
easy to tell what the imposed delay was, if any. A subsequent change
will seek to make this option the default, being able to see from the
headers how long the sleep was.

The reason for why the getopt format is "send-delay=s" instead of
"send-delay=d" is because we're doing manual validation of the value
we get passed, which getopt would corrupt in cases of e.g. float
values before we could show a sensible error message.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt |  6 
 Documentation/git-send-email.txt |  4 +++
 git-send-email.perl  | 14 ++--
 t/t9001-send-email.sh| 55 
 4 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ce9102cea8..f155d349c0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3068,6 +3068,12 @@ sendemail.smtpReloginDelay::
Seconds wait before reconnecting to smtp server.
See also the `--relogin-delay` option of linkgit:git-send-email[1].
 
+sendemail.smtpSendDelay::
+   Seconds wait in between message sending before sending another
+   message. Set it to 0 to impose no extra delay, defaults to 0.
++
+See also the `--send-delay` option of linkgit:git-send-email[1].
+
 showbranch.default::
The default set of branches for linkgit:git-show-branch[1].
See linkgit:git-show-branch[1].
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 71ef97ba9b..169ad78a2b 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -268,6 +268,10 @@ must be used for each option.
with --batch-size option.  Defaults to the `sendemail.smtpReloginDelay`
configuration variable.
 
+--send-delay=::
+   Wait $ between sending emails. Defaults to the
+   `sendemail.smtpSendDelay` configuration variable.
+
 Automating
 ~~
 
diff --git a/git-send-email.perl b/git-send-email.perl
index 2fa7818ca9..013277ede2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -89,6 +89,7 @@ git send-email --dump-aliases
 --batch-size  * send max  message per connection.
 --relogin-delay   * delay  seconds between two 
successive login.
  This option can only be used with 
--batch-size
+--send-delay  * ensure that  seconds pass between 
two successive sends.
 
   Automating:
 --identity* Use the sendemail. options.
@@ -225,7 +226,7 @@ my ($cover_cc, $cover_to);
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
-my ($batch_size, $relogin_delay);
+my ($batch_size, $relogin_delay, $send_delay);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -259,6 +260,7 @@ my %config_settings = (
 "smtpauth" => \$smtp_auth,
 "smtpbatchsize" => \$batch_size,
 "smtprelogindelay" => \$relogin_delay,
+"smtpsenddelay" => \$send_delay,
 "to" => \@initial_to,
 "tocmd" => \$to_cmd,

[PATCH 2/2] send-email: supply a --send-delay=1 by default

The earlier change to add this option described the problem this
option is trying to solve.

This turns it on by default with a value of 1 second, which'll
hopefully solve it, and if not user reports as well as the
X-Mailer-Send-Delay header should help debug it.

I think the trade-off of slowing down E-Mail sending to turn this on
makes sense because:

 * GMail is a really common client, git.git's own unique authors by
   %aE are ~30% @gmail.com, ~20% for linux.git. That's just patch
   submitters, my guess is this it's much more common among those who
   mostly read the list, and those users who aren't using mu4e / mutt
   etc. anyway.

 * There's really no point in having this feature at all if it's not
   made the default, since the entire point is to be able to read a
   list like the git ML or the LKML and have patches from others show
   up in order.

 * I don't think anyone's really sensitive to the sending part of
   send-email taking longer. You just choose "all" and then switch to
   another terminal while it does its thing if you have a huge series,
   and for 1-3 patches I doubt anyone would notice this anyway.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt | 13 -
 git-send-email.perl  |  1 +
 t/t9001-send-email.sh|  4 ++--
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f155d349c0..bd578642c1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3070,7 +3070,18 @@ sendemail.smtpReloginDelay::
 
 sendemail.smtpSendDelay::
Seconds wait in between message sending before sending another
-   message. Set it to 0 to impose no extra delay, defaults to 0.
+   message. Set it to 0 to impose no extra delay, defaults to 1
+   to wait 1 second.
++
+The reason for imposing a default delay is because certain popular
+E-Mail clients such as Google's GMail completely ignore the "Date"
+header, which format-patch is careful to set such that the patches
+will be displayed in order, and instead sort by the time the E-mail
+was received.
++
+This causes sent E-Mail to be shown completely out of order in such
+clients, imposing the delay is a workaround that should usually work
+(although it's by no means guaranteed).
 +
 See also the `--send-delay` option of linkgit:git-send-email[1].
 
diff --git a/git-send-email.perl b/git-send-email.perl
index 013277ede2..ddbc44f1c9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -489,6 +489,7 @@ die sprintf(__("Unknown --confirm setting: '%s'\n"), 
$confirm)
unless $confirm =~ /^(?:auto|cc|compose|always|never)/;
 die sprintf(__("Invalid --send-delay setting: '%s'\n"), $send_delay)
if defined $send_delay and $send_delay !~ /^[0-9]+$/s;
+$send_delay = 1 unless defined $send_delay;
 
 # Debugging, print out the suppressions.
 if (0) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index fafa61c5d6..1580e00fce 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1713,7 +1713,7 @@ test_expect_success '--send-delay expects whole 
non-negative seconds' '
test_i18ngrep "Invalid --send-delay setting" errors
 '
 
-test_expect_success $PREREQ "there is no default --send-delay" '
+test_expect_success $PREREQ "there is a default --send-delay" '
clean_fake_sendmail &&
rm -fr outdir &&
git format-patch -3 -o outdir &&
@@ -1724,7 +1724,7 @@ test_expect_success $PREREQ "there is no default 
--send-delay" '
outdir/*.patch \
2>stderr >stdout &&
test $(grep -c "X-Mailer:" stdout) = 3 &&
-   test $(grep -c "X-Mailer-Send-Delay:" stdout) = 0
+   test $(grep -c "X-Mailer-Send-Delay:" stdout) = 2
 '
 
 test_expect_success $PREREQ '--send-delay adds a X-Mailer-Send-Delay header to 
affected E-Mails' '
-- 
2.16.2.804.g6dcf76e118

[PATCH 0/2] send-email: impose a delay while sending to appease GMail

GMail doesn't sort E-Mail by the "Date" header, but by when the E-Mail
was received. As a result patches sent to the git ML and LKML (and
friends) show up out of order in GMail.

This series works around that issue by sleeping for 1 second between
sending E-Mails.

If you're on the LKML and wondering why you got this, I figured
feedback from the other big user (that I know of) of send-email would
be helpful.

Ævar Arnfjörð Bjarmason (2):
  send-email: add an option to impose delay sent E-Mails
  send-email: supply a --send-delay=1 by default

 Documentation/config.txt | 17 ++
 Documentation/git-send-email.txt |  4 +++
 git-send-email.perl  | 15 +++--
 t/t9001-send-email.sh| 55 
 4 files changed, 89 insertions(+), 2 deletions(-)

-- 
2.16.2.804.g6dcf76e118

Re: [PATCH 4/3] sha1_name: use bsearch_pack() in unique_in_pack()


On 3/24/2018 12:41 PM, René Scharfe wrote:

Replace the custom binary search in unique_in_pack() with a call to
bsearch_pack().  This reduces code duplication and makes use of the
fan-out table of packs.

Signed-off-by: Rene Scharfe 
---
This is basically the same replacement as done by patch 3.  Speed is
less of a concern here -- at least I don't know a commonly used
command that needs to resolve lots of short hashes.


Thanks, René! Good teamwork on this patch series.

Reviewed-by: Derrick Stolee

Re: [ANNOUNCE] Git v2.17.0-rc1


On 3/23/2018 1:59 PM, Ævar Arnfjörð Bjarmason wrote:

On Wed, Mar 21 2018, Junio C. Hamano wrote:


A release candidate Git v2.17.0-rc1 is now available for testing
at the usual places.  It is comprised of 493 non-merge commits
since v2.16.0, contributed by 62 people, 19 of which are new faces.

I have this deployed on some tens of K machines who all use git in one
way or another (from automated pulls, to users interactively), and rc0
before that, with a few patches on top from me + Takato + Duy + Derrick
since rc0 was released (and since today based on top of rc1). No issues
so far.

The specific in-house version I have is at:
https://github.com/git/git/compare/v2.17.0-rc1...bookingcom:booking-git-v2018-03-23-1


Thanks for testing the commit-graph feature, Ævar! I'm guessing you have 
some mechanisms to ensure the 'git commit-graph write' command is run on 
these machines and 'core.commitGraph' is set to true in the config? I 
would love to hear how this benefits your org.


Thanks,
-Stolee

Re: Should I try to fix rebase interactive preserve-merges bug

2018-03-25 Thread Jacob Keller

On Sun, Mar 25, 2018 at 10:32 AM, Wink Saville  wrote:
> There is a "TODO known breakage" in t3404-rebase-interactve.sh:
>
>not ok 24 - exchange two commits with -p # TODO known breakage
>
> I'm contemplating trying to fix it. But with --recreate-merges coming
> maybe it's not worth the effort. Should I proceed with attempting a
> fix or is --preserve-merges going to be a synonym for
> --recreate-merges?
>
> -- Wink

AFAIK this breakage of preserve-merges is a design flaw which isn't
really fixable, which is why --recreate-merges is being added.

I believe the plan is to deprecate preserve-merges once
recreate-merges has landed.

Thanks,
Jake

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

On 03/24, Joel Teichroeb wrote:
> I've been working on converting all of git stash to be a
> builtin, however it's hard to get it all working at once with
> limited time, so I've moved around half of it to a new
> stash--helper builtin and called these functions from the shell
> script. Once this is stabalized, it should be easier 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.

Thanks for splitting this up into multiple patches, I found that much
more pleasant to review, and thanks for your continued work on this :)

> Since there seems to be interest from 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 to builtin
> 
>  .gitignore  |   1 +
>  Makefile|   1 +
>  builtin.h   |   1 +
>  builtin/stash--helper.c | 514 
> 
>  git-stash.sh|  13 +-
>  git.c   |   1 +
>  6 files changed, 526 insertions(+), 5 deletions(-)
>  create mode 100644 builtin/stash--helper.c
> 
> -- 
> 2.16.2
>

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

On 03/24, Joel Teichroeb wrote:
> ---
>  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
> +++ b/builtin/stash--helper.c
> @@ -12,6 +12,7 @@
>  
>  static const char * const git_stash_helper_usage[] = {
>   N_("git stash--helper drop [-q|--quiet] []"),
> + N_("git stash--helper pop [--index] [-q|--quiet] []"),
>   N_("git stash--helper apply [--index] [-q|--quiet] []"),
>   N_("git stash--helper branch  []"),
>   N_("git stash--helper clear"),
> @@ -23,6 +24,11 @@ static const char * const git_stash_helper_drop_usage[] = {
>   NULL
>  };
>  
> +static const char * const git_stash_helper_pop_usage[] = {
> + N_("git stash--helper pop [--index] [-q|--quiet] []"),
> + NULL
> +};
> +
>  static const char * const git_stash_helper_apply_usage[] = {
>   N_("git stash--helper apply [--index] [-q|--quiet] []"),
>   NULL
> @@ -402,6 +408,36 @@ static int drop_stash(int argc, const char **argv, const 
> char *prefix)
>   return do_drop_stash(prefix, );
>  }
>  
> +static int pop_stash(int argc, const char **argv, const char *prefix)
> +{
> + int index = 0;
> + const char *commit = NULL;
> + struct stash_info info;
> + struct option options[] = {
> + OPT__QUIET(, N_("be quiet, only report errors")),
> + OPT_BOOL(0, "index", ,
> + N_("attempt to ininstate the index")),
> + OPT_END()
> + };
> +
> + argc = parse_options(argc, argv, prefix, options,
> + git_stash_helper_pop_usage, 0);
> +
> + if (argc == 1)
> + commit = argv[0];
> +
> + if (get_stash_info(, commit))
> + return -1;
> +
> + if (!info.is_stash_ref)
> + return error(_("'%s' is not a stash reference"), commit);

The pattern above appears twice now, is it worth factoring it out into
a separate function, similar to 'assert_stash_ref'?

> +
> + if (do_apply_stash(prefix, , index))
> + return -1;

If we fail, currently we print "The stash entry is kept in case you
need it again", which we are loosing here.  I think that's useful
output in case the 'apply' command fails, especially in the case of a
merge conflict, where I think the 'apply' will fail as well, and the
user may be confused whether/why the stash is not dropped.

> +
> + return do_drop_stash(prefix, );
> +}
> +
>  static int branch_stash(int argc, const char **argv, const char *prefix)
>  {
>   const char *commit = NULL, *branch = NULL;
> @@ -464,6 +500,8 @@ int cmd_stash__helper(int argc, const char **argv, const 
> char *prefix)
>   result = clear_stash(argc, argv, prefix);
>   else if (!strcmp(argv[0], "drop"))
>   result = drop_stash(argc, argv, prefix);
> + else if (!strcmp(argv[0], "pop"))
> + result = pop_stash(argc, argv, prefix);
>   else if (!strcmp(argv[0], "branch"))
>   result = branch_stash(argc, argv, prefix);
>   else {
> diff --git a/git-stash.sh b/git-stash.sh
> index 54d0a6c21f..d595bbaf64 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -732,7 +732,8 @@ drop)
>   ;;
>  pop)
>   shift
> - pop_stash "$@"
> + cd "$START_DIR"
> + git stash--helper pop "$@"
>   ;;
>  branch)
>   shift
> -- 
> 2.16.2
>

Should I try to fix rebase interactive preserve-merges bug

2018-03-25 Thread Wink Saville

There is a "TODO known breakage" in t3404-rebase-interactve.sh:

   not ok 24 - exchange two commits with -p # TODO known breakage

I'm contemplating trying to fix it. But with --recreate-merges coming
maybe it's not worth the effort. Should I proceed with attempting a
fix or is --preserve-merges going to be a synonym for
--recreate-merges?

-- Wink

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

On 03/24, Joel Teichroeb wrote:
> ---
> [...]
> +
> +static const char *ref_stash = "refs/stash";
> +static int quiet;
> +static char stash_index_path[PATH_MAX];
> +
> +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;
> + const char *message;
> + const char *revision;
> + int is_stash_ref;
> + int has_u;
> + const char *patch;
> +};
> +
> +static int get_stash_info(struct stash_info *info, const char *commit)
> +{
> + struct strbuf w_commit_rev = STRBUF_INIT;
> + struct strbuf b_commit_rev = STRBUF_INIT;
> + struct strbuf w_tree_rev = STRBUF_INIT;
> + struct strbuf b_tree_rev = STRBUF_INIT;
> + struct strbuf i_tree_rev = STRBUF_INIT;
> + struct strbuf u_tree_rev = STRBUF_INIT;
> + struct strbuf commit_buf = STRBUF_INIT;
> + struct strbuf symbolic = STRBUF_INIT;
> + struct strbuf out = STRBUF_INIT;
> + int ret;
> + const char *revision = commit;
> + char *end_of_rev;
> + struct child_process cp = CHILD_PROCESS_INIT;
> + info->is_stash_ref = 0;
> +
> + if (commit == NULL) {
> + strbuf_addf(_buf, "%s@{0}", ref_stash);
> + revision = commit_buf.buf;

Before setting up the revisions here, as is done below, we used to
check if a stash even exists, if no commit was given.  So in a
repository with no stashes we would die with "No stash entries found",
while now we die with "error: refs/stash@{0} is not a valid
reference".  I think the error message we had previously was slightly
nicer, and we should try to keep it.

> + } else if (strspn(commit, "0123456789") == strlen(commit)) {
> + strbuf_addf(_buf, "%s@{%s}", ref_stash, commit);
> + revision = commit_buf.buf;
> + }
> + info->revision = revision;
> +
> + strbuf_addf(_commit_rev, "%s", revision);
> + strbuf_addf(_commit_rev, "%s^1", revision);
> + strbuf_addf(_tree_rev, "%s:", revision);
> + strbuf_addf(_tree_rev, "%s^1:", revision);
> + strbuf_addf(_tree_rev, "%s^2:", revision);
> +
> + ret = !get_oid(w_commit_rev.buf, >w_commit) &&
> + !get_oid(b_commit_rev.buf, >b_commit) &&
> + !get_oid(w_tree_rev.buf, >w_tree) &&
> + !get_oid(b_tree_rev.buf, >b_tree) &&
> + !get_oid(i_tree_rev.buf, >i_tree);
> +
> + strbuf_release(_commit_rev);
> + strbuf_release(_commit_rev);
> + strbuf_release(_tree_rev);
> + strbuf_release(_tree_rev);
> + strbuf_release(_tree_rev);
> +
> + if (!ret)
> + return error(_("%s is not a valid reference"), revision);

We used to distinguish between "not a valid reference" and "not a
stash-like commit" here.  I think just doing the first 'get_oid'
before the others, and returning the error if that fails, and then
doing the rest and returning the "not a stash-like commit" if one of
the other 'get_oid' calls fails would work, although I did not test it.

> +
> + strbuf_addf(_tree_rev, "%s^3:", revision);
> +
> + info->has_u = !get_oid(u_tree_rev.buf, >u_tree);
> +
> + strbuf_release(_tree_rev);
> +
> + end_of_rev = strchrnul(revision, '@');
> + strbuf_add(, revision, end_of_rev - revision);
> + cp.git_cmd = 1;
> + argv_array_pushl(, "rev-parse", "--symbolic-full-name", NULL);
> + argv_array_pushf(, "%s", symbolic.buf);
> + strbuf_release();
> + pipe_command(, NULL, 0, , 0, NULL, 0);
> +
> + if (out.len - 1 == strlen(ref_stash))
> + info->is_stash_ref = !strncmp(out.buf, ref_stash, out.len - 1);
> + strbuf_release();
> +
> + return 0;
> +}
> +
> [...]

Re: [PATCH v4] filter-branch: fix errors caused by refs that point at non-committish

Yuki Kokubun  writes:

> "git filter-branch -- --all" prints error messages when processing refs that
> point at objects that are not committish. Such refs can be created by
> "git replace" with trees or blobs. And also "git tag" with trees or blobs can
> create such refs.
>
> Filter these problematic refs out early, before they are seen by the logic to
> see which refs have been modified and which have been left intact (which is
> where the unwanted error messages come from), and warn that these refs are 
> left
> unwritten while doing so.
>
> Signed-off-by: Yuki Kokubun 
> ---
>  git-filter-branch.sh | 14 --
>  t/t7003-filter-branch.sh | 14 ++
>  2 files changed, 26 insertions(+), 2 deletions(-)

Good.  Will queue.  Thanks.

>
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 1b7e4b2cd..41efecb28 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -251,8 +251,18 @@ done < "$tempdir"/backup-refs
>  
>  # The refs should be updated if their heads were rewritten
>  git rev-parse --no-flags --revs-only --symbolic-full-name \
> - --default HEAD "$@" > "$tempdir"/raw-heads || exit
> -sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
> + --default HEAD "$@" > "$tempdir"/raw-refs || exit
> +while read ref
> +do
> + case "$ref" in ^?*) continue ;; esac
> +
> + if git rev-parse --verify "$ref"^0 >/dev/null 2>&1
> + then
> + echo "$ref"
> + else
> + warn "WARNING: not rewriting '$ref' (not a committish)"
> + fi
> +done >"$tempdir"/heads <"$tempdir"/raw-refs
>  
>  test -s "$tempdir"/heads ||
>   die "You must specify a ref to rewrite."
> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index 7cb60799b..04f79f32b 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name 
> vs pathname ambiguity' '
>   git show HEAD:$ambiguous
>  '
>  
> +test_expect_success 'rewrite repository including refs that point at 
> non-commit object' '
> + test_when_finished "git reset --hard original" &&
> + tree=$(git rev-parse HEAD^{tree}) &&
> + test_when_finished "git replace -d $tree" &&
> + echo A >new &&
> + git add new &&
> + new_tree=$(git write-tree) &&
> + git replace $tree $new_tree &&
> + git tag -a -m "tag to a tree" treetag $new_tree &&
> + git reset --hard HEAD &&
> + git filter-branch -f -- --all >filter-output 2>&1 &&
> + ! fgrep fatal filter-output
> +'
> +
>  test_done

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

Jeff King  writes:

> IMHO we should do one of:
>
>   1. Nothing. ;)
>
>   2. Complain about "-l" in list mode to help educate users about the
>  current craziness.

Nah.  We've seen this, perhaps not often but enough times over long
period of time.  The above two would not fly as a longer term
solution.

>
>   3. Drop "-l" (probably with a deprecation period); it seems unlikely
>  to me that anybody uses it for branch creation, and this would at
>  least reduce the confusion (then it would just be "so why don't we
>  have -l" instead of "why is -l not what I expect").
>
>   4. Repurpose "-l" as a shortcut for --list (also after a deprecation
>  period). This is slightly more dangerous in that it may confuse
>  people using multiple versions of Git that cross the deprecation
>  line. But that's kind of what the deprecation period is for...

3. is prerequisite for 4.  If we haven't gone through both in 5
years we should be ashamed of ourselves ;-)  But at least we should
start 3. and aim to finish 3. in 2 years if not sooner.

[PATCH v5] filter-branch: fix errors caused by refs that point at non-committish

"git filter-branch -- --all" prints error messages when processing refs that
point at objects that are not committish. Such refs can be created by
"git replace" with trees or blobs. And also "git tag" with trees or blobs can
create such refs.

Filter these problematic refs out early, before they are seen by the logic to
see which refs have been modified and which have been left intact (which is
where the unwanted error messages come from), and warn that these refs are left
unwritten while doing so.

Signed-off-by: Yuki Kokubun 
Reviewed-by: Junio C Hamano 
---
 git-filter-branch.sh | 14 --
 t/t7003-filter-branch.sh | 14 ++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 1b7e4b2cd..41efecb28 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -251,8 +251,18 @@ done < "$tempdir"/backup-refs
 
 # The refs should be updated if their heads were rewritten
 git rev-parse --no-flags --revs-only --symbolic-full-name \
-   --default HEAD "$@" > "$tempdir"/raw-heads || exit
-sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
+   --default HEAD "$@" > "$tempdir"/raw-refs || exit
+while read ref
+do
+   case "$ref" in ^?*) continue ;; esac
+
+   if git rev-parse --verify "$ref"^0 >/dev/null 2>&1
+   then
+   echo "$ref"
+   else
+   warn "WARNING: not rewriting '$ref' (not a committish)"
+   fi
+done >"$tempdir"/heads <"$tempdir"/raw-refs
 
 test -s "$tempdir"/heads ||
die "You must specify a ref to rewrite."
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 7cb60799b..04f79f32b 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs 
pathname ambiguity' '
git show HEAD:$ambiguous
 '
 
+test_expect_success 'rewrite repository including refs that point at 
non-commit object' '
+   test_when_finished "git reset --hard original" &&
+   tree=$(git rev-parse HEAD^{tree}) &&
+   test_when_finished "git replace -d $tree" &&
+   echo A >new &&
+   git add new &&
+   new_tree=$(git write-tree) &&
+   git replace $tree $new_tree &&
+   git tag -a -m "tag to a tree" treetag $new_tree &&
+   git reset --hard HEAD &&
+   git filter-branch -f -- --all >filter-output 2>&1 &&
+   ! fgrep fatal filter-output
+'
+
 test_done
-- 
2.16.2.18.g09cb46d

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

On 03/24, Joel Teichroeb wrote:
> ---
>  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
> +++ b/builtin/stash--helper.c
> @@ -12,6 +12,7 @@
>  
>  static const char * const git_stash_helper_usage[] = {
>   N_("git stash--helper apply [--index] [-q|--quiet] []"),
> + N_("git stash--helper branch  []"),
>   NULL
>  };
>  
> @@ -20,6 +21,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 *ref_stash = "refs/stash";
>  static int quiet;
>  static char stash_index_path[PATH_MAX];
> @@ -307,6 +313,42 @@ static int apply_stash(int argc, const char **argv, 
> const char *prefix)
>   return do_apply_stash(prefix, , index);
>  }
>  
> +static int branch_stash(int argc, const char **argv, const char *prefix)
> +{
> + const char *commit = NULL, *branch = NULL;
> + int ret;
> + struct argv_array args = ARGV_ARRAY_INIT;
> + struct stash_info info;
> + struct option options[] = {
> + OPT_END()
> + };
> +
> + argc = parse_options(argc, argv, prefix, options,
> + git_stash_helper_branch_usage, 0);
> +
> + if (argc != 0) {
> + branch = argv[0];
> + if (argc == 2)
> + commit = argv[1];
> + }
> +
> + if (get_stash_info(, commit))
> + return -1;

I see this is supposed to do something similar to what
'assert_stash_like' was doing.  However we never end up die'ing with
"... is not a a stash-like commit" here from what I can see.  I think
I can see where this is coming from, and I missed it when reading over
1/4 here.  I'll go back and comment there, where I think we're going
slightly wrong.

Either way while I tripped over the 'get_stash_info' call here, I
think it's the right thing to do.

> + argv_array_pushl(, "checkout", "-b", NULL);
> + argv_array_push(, branch);
> + argv_array_push(, sha1_to_hex(info.b_commit.hash));
> + ret = cmd_checkout(args.argc, args.argv, prefix);
> + if (ret)
> + return -1;
> +
> + ret = do_apply_stash(prefix, , 1);
> + if (!ret && info.is_stash_ref)
> + ret = do_drop_stash(prefix, );

'do_drop_stash' is only defined in the next patch.  Maybe maybe 2/4
and 3/4 need to swap places?

All patches should compile individually, and all tests should pass for
each patch, so we maintain bisectability of the codebase.

> +
> + return ret;
> +}
> +
>  int cmd_stash__helper(int argc, const char **argv, const char *prefix)
>  {
>   int result = 0;
> @@ -329,6 +371,8 @@ int cmd_stash__helper(int argc, const char **argv, const 
> char *prefix)
>   usage_with_options(git_stash_helper_usage, options);
>   else if (!strcmp(argv[0], "apply"))
>   result = apply_stash(argc, argv, prefix);
> + else if (!strcmp(argv[0], "branch"))
> + result = branch_stash(argc, argv, prefix);
>   else {
>   error(_("unknown subcommand: %s"), argv[0]);
>   usage_with_options(git_stash_helper_usage, options);
> diff --git a/git-stash.sh b/git-stash.sh
> index 92c084eb17..360643ad4e 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -736,7 +736,8 @@ pop)
>   ;;
>  branch)
>   shift
> - apply_to_branch "$@"
> + cd "$START_DIR"
> + git stash--helper branch "$@"
>   ;;
>  *)
>   case $# in
> -- 
> 2.16.2
>

Re: [PATCH v4] filter-branch: fix errors caused by refs that point at non-committish

References: <1521996898-7052-1-git-send-email-orga.chem@gmail.com>
Content-Type: text/plain

Sorry, I forgot add a line of "Reviewed-by".
I'm gonna send the fixed patch again.

[PATCH v4] filter-branch: fix errors caused by refs that point at non-committish

"git filter-branch -- --all" prints error messages when processing refs that
point at objects that are not committish. Such refs can be created by
"git replace" with trees or blobs. And also "git tag" with trees or blobs can
create such refs.

Filter these problematic refs out early, before they are seen by the logic to
see which refs have been modified and which have been left intact (which is
where the unwanted error messages come from), and warn that these refs are left
unwritten while doing so.

Signed-off-by: Yuki Kokubun 
---
 git-filter-branch.sh | 14 --
 t/t7003-filter-branch.sh | 14 ++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 1b7e4b2cd..41efecb28 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -251,8 +251,18 @@ done < "$tempdir"/backup-refs
 
 # The refs should be updated if their heads were rewritten
 git rev-parse --no-flags --revs-only --symbolic-full-name \
-   --default HEAD "$@" > "$tempdir"/raw-heads || exit
-sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
+   --default HEAD "$@" > "$tempdir"/raw-refs || exit
+while read ref
+do
+   case "$ref" in ^?*) continue ;; esac
+
+   if git rev-parse --verify "$ref"^0 >/dev/null 2>&1
+   then
+   echo "$ref"
+   else
+   warn "WARNING: not rewriting '$ref' (not a committish)"
+   fi
+done >"$tempdir"/heads <"$tempdir"/raw-refs
 
 test -s "$tempdir"/heads ||
die "You must specify a ref to rewrite."
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 7cb60799b..04f79f32b 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs 
pathname ambiguity' '
git show HEAD:$ambiguous
 '
 
+test_expect_success 'rewrite repository including refs that point at 
non-commit object' '
+   test_when_finished "git reset --hard original" &&
+   tree=$(git rev-parse HEAD^{tree}) &&
+   test_when_finished "git replace -d $tree" &&
+   echo A >new &&
+   git add new &&
+   new_tree=$(git write-tree) &&
+   git replace $tree $new_tree &&
+   git tag -a -m "tag to a tree" treetag $new_tree &&
+   git reset --hard HEAD &&
+   git filter-branch -f -- --all >filter-output 2>&1 &&
+   ! fgrep fatal filter-output
+'
+
 test_done
-- 
2.16.2.18.g09cb46d

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 remove?

I wasn't really sure if I should remove code from the .sh file as it
seems in the past the old .sh files have been kept around as examples.
Has that been done for previous conversions?

Re: [PATCH v3] filter-branch: fix errors caused by refs that point at non-committish

References: 

<5ab46520.0352650a.cc02b.a...@mx.google.com>
<20180323050913.5188-1-orga.chem@gmail.com> 
Content-Type: text/plain

> Grammo (third-person singular 'prints' misspelt without 's'; the
> "when" clause has a complex subject but no verb).
> 
> Perhaps this will salvage what you meant:
> 
>   "git filter-branch -- --all" prints error messages when
>   processing refs that point at objects that are not
>   committish.

Thanks. I'm gonna fix it.

> Please sign-off your patch (cf. Documentation/SubmittingPatches).

OK I'm gonna add it.

I greatly appreciate for your kind review.
I couldn't write this patch without your help.
Thanks.

Re: [PATCH] run-command: use strbuf_addstr() for adding a string to a strbuf

René Scharfe  writes:

> Patch generated with Coccinelle and contrib/coccinelle/strbuf.cocci.
>
> Signed-off-by: Rene Scharfe 
> ---
> That line was added by e73dd78699 (run-command.c: introduce
> trace_run_command()).

Thanks.

>
>  run-command.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index a483d5904a..84899e423f 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -621,7 +621,7 @@ static void trace_run_command(const struct child_process 
> *cp)
>   if (!trace_want(_default_key))
>   return;
>  
> - strbuf_addf(, "trace: run_command:");
> + strbuf_addstr(, "trace: run_command:");
>   if (cp->dir) {
>   strbuf_addstr(, " cd ");
>   sq_quote_buf_pretty(, cp->dir);

Re: [PATCH] run-command: use strbuf_addstr() for adding a string to a strbuf

2018-03-25 Thread Duy Nguyen

On Sun, Mar 25, 2018 at 12:57 PM, René Scharfe  wrote:
> Patch generated with Coccinelle and contrib/coccinelle/strbuf.cocci.
>
> Signed-off-by: Rene Scharfe 
> ---
> That line was added by e73dd78699 (run-command.c: introduce
> trace_run_command()).
>
>  run-command.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index a483d5904a..84899e423f 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -621,7 +621,7 @@ static void trace_run_command(const struct child_process 
> *cp)
> if (!trace_want(_default_key))
> return;
>
> -   strbuf_addf(, "trace: run_command:");
> +   strbuf_addstr(, "trace: run_command:");

Obviously correct. Ack.

> if (cp->dir) {
> strbuf_addstr(, " cd ");
> sq_quote_buf_pretty(, cp->dir);
> --
> 2.16.3



-- 
Duy

Re: [PATCH] bisect: use oid_to_hex() for converting object_id hashes to hex strings

"brian m. carlson"  writes:

> On Sun, Mar 25, 2018 at 12:57:36PM +0200, René Scharfe wrote:
>> Patch generated with Coccinelle and contrib/coccinelle/object_id.cocci.
>> 
>> Signed-off-by: Rene Scharfe 
>> ---
>> This is a belated follow-up to f0a6068a9f (bisect: debug: convert struct
>> object to object_id).
>
> This looks good to me.

Thanks.

Re: query on git submodule (ignore)

prashant Nidgunde  writes:

[cc: stefan, for his interest in improving 'git submodules']

> Hello,
>
> I am new to this community ,so please ignore if I am asking anything silly.
>
> Case :
> Today when I built my submodule , and did a git status , it shows as modified.
>
> After reading certain suggestions on web i found out that i can ignore
> that adding a line in .gitmodules
>
> But, I had to add that line manually ( which could be errorprone
> because of typos )
>
>
> Question:
> 1. Is it feasible to build a feature like :
>git submodule "zlib" ignore dirty ( which will
> ignore submodule zlib when its built and dirty  as it has new files in
> its directory)

How does it prevent you from saying 

git submodule "glib" ignore dirty

when you really meant "zlib"?  How is the command supposed to know
that you did *not* mean "glib", which may currently not exist in the
index nor in the working tree yet but you are about to create, and
doing the "ignore dirty" configuration as an earlier step of
multiple steps to add a submodule?

I personally doubt that the main issue you should be concerned about
is feasibility.  A larger issue is how it is supposed to help, iow,
is such a "feature" useful in the first place?

Whenever you hear yourself say "I have to do X manually, and I can
make mistakes. Can a command do it instead?", you have to ask
yourself: what pieces of information do you give to that command,
and how do you ensure you do not make typos on that command line?

Besides, the above syntax would not work.  What would a user do when
a submodule called "add" exists, for example?

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

On 03/24, Joel Teichroeb wrote:
> ---

Missing sign-off?  I saw it's missing in the other patches as well. 

> [...]
> +static int do_apply_stash(const char *prefix, struct stash_info *info, int 
> index)
> +{
> + struct merge_options o;
> + struct object_id c_tree;
> + struct object_id index_tree;
> + const struct object_id *bases[1];
> + int bases_count = 1;
> + struct commit *result;
> + int ret;
> + int has_index = index;
> +
> + read_cache_preload(NULL);
> + if (refresh_cache(REFRESH_QUIET))
> + return -1;
> +
> + if (write_cache_as_tree(c_tree.hash, 0, NULL) || reset_tree(c_tree, 0, 
> 0))
> + return error(_("Cannot apply a stash in the middle of a 
> merge"));
> +
> + if (index) {
> + if (!oidcmp(>b_tree, >i_tree) || !oidcmp(_tree, 
> >i_tree)) {
> + has_index = 0;
> + } else {
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf out = STRBUF_INIT;
> + struct argv_array args = ARGV_ARRAY_INIT;
> + cp.git_cmd = 1;
> + argv_array_pushl(, "diff-tree", "--binary", 
> NULL);
> + argv_array_pushf(, "%s^2^..%s^2", 
> sha1_to_hex(info->w_commit.hash), sha1_to_hex(info->w_commit.hash));
> + if (pipe_command(, NULL, 0, , 0, NULL, 0))
> + return -1;
> +
> + child_process_init();
> + cp.git_cmd = 1;
> + argv_array_pushl(, "apply", "--cached", NULL);
> + if (pipe_command(, out.buf, out.len, NULL, 0, NULL, 
> 0))
> + return -1;
> +
> + strbuf_release();
> + discard_cache();
> + read_cache();
> + if (write_cache_as_tree(index_tree.hash, 0, NULL))
> + return -1;
> +
> + argv_array_push(, "reset");
> + cmd_reset(args.argc, args.argv, prefix);
> + }
> + }
> +
> + if (info->has_u) {
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct child_process cp2 = CHILD_PROCESS_INIT;
> + int res;
> +
> + cp.git_cmd = 1;
> + argv_array_push(, "read-tree");
> + argv_array_push(, sha1_to_hex(info->u_tree.hash));
> + argv_array_pushf(_array, "GIT_INDEX_FILE=%s", 
> stash_index_path);
> +
> + cp2.git_cmd = 1;
> + argv_array_pushl(, "checkout-index", "--all", NULL);
> + argv_array_pushf(_array, "GIT_INDEX_FILE=%s", 
> stash_index_path);
> +
> + res = run_command() || run_command();
> + remove_path(stash_index_path);
> + if (res)
> + return error(_("Could not restore untracked files from 
> stash"));

A minor change in behaviour here is that we are removing the temporary
index file unconditionally here, while we would previously only remove
it if both 'read-tree' and 'checkout-index' would succeed.

I don't think that's a bad thing, we probably don't want users to try
and use that index file in any way, and I doubt that's part of anyones
workflow, so I think cleaning it up makes sense.

> + }
> +
> + init_merge_options();
> +
> + o.branch1 = "Updated upstream";
> + o.branch2 = "Stashed changes";
> +
> + if (!hashcmp(info->b_tree.hash, c_tree.hash))
> + o.branch1 = "Version stash was based on";
> +
> + if (quiet)
> + o.verbosity = 0;
> +
> + if (o.verbosity >= 3)
> + printf_ln(_("Merging %s with %s"), o.branch1, o.branch2);
> +
> + bases[0] = >b_tree;
> +
> + ret = merge_recursive_generic(, _tree, >w_tree, bases_count, 
> bases, );
> + if (ret != 0) {
> + struct argv_array args = ARGV_ARRAY_INIT;
> + argv_array_push(, "rerere");
> + cmd_rerere(args.argc, args.argv, prefix);
> + if (index)
> + printf_ln(_("Index was not unstashed."));

Minor nit:  I think the above should be 'fprintf_ln(stderr, ...)' to
match what we currently have.

> +
> + return ret;
> + }
> +
> [...]

Re: [PATCH 4/3] sha1_name: use bsearch_pack() in unique_in_pack()

Am 25.03.2018 um 18:19 schrieb Junio C Hamano:
> René Scharfe  writes:
> 
>> Replace the custom binary search in unique_in_pack() with a call to
>> bsearch_pack().  This reduces code duplication and makes use of the
>> fan-out table of packs.
>>
>> Signed-off-by: Rene Scharfe 
>> ---
>> This is basically the same replacement as done by patch 3.  Speed is
>> less of a concern here -- at least I don't know a commonly used
>> command that needs to resolve lots of short hashes.
> 
> Looks correct.  Did you find this by eyeballing, or do you have some
> interesting tool you use?

I was looking for SHA1 binary searches using something like this:

git grep -e '/ 2' -e hashcmp -W --all-match

René

[PATCH] unpack-trees: release oid_array after use in check_updates()

Signed-off-by: Rene Scharfe 
---
That leak was introduced by c0c578b33c (unpack-trees: batch fetching of
missing blobs).

 unpack-trees.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/unpack-trees.c b/unpack-trees.c
index d5685891a5..e73745051e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -379,30 +379,31 @@ static int check_updates(struct unpack_trees_options *o)
struct oid_array to_fetch = OID_ARRAY_INIT;
int fetch_if_missing_store = fetch_if_missing;
fetch_if_missing = 0;
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
if ((ce->ce_flags & CE_UPDATE) &&
!S_ISGITLINK(ce->ce_mode)) {
if (!has_object_file(>oid))
oid_array_append(_fetch, >oid);
}
}
if (to_fetch.nr)
fetch_objects(repository_format_partial_clone,
  _fetch);
fetch_if_missing = fetch_if_missing_store;
+   oid_array_clear(_fetch);
}
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
 
if (ce->ce_flags & CE_UPDATE) {
if (ce->ce_flags & CE_WT_REMOVE)
die("BUG: both update and delete flags are set 
on %s",
ce->name);
display_progress(progress, ++cnt);
ce->ce_flags &= ~CE_UPDATE;
if (o->update && !o->dry_run) {
errs |= checkout_entry(ce, , NULL);
}
}
}
-- 
2.16.3

Re: [PATCH v3] filter-branch: fix errors caused by refs that point at non-committish

Yuki Kokubun  writes:

> "git filter-branch -- --all" print error messages when refs that point at
> objects that are not committish.

Grammo (third-person singular 'prints' misspelt without 's'; the
"when" clause has a complex subject but no verb).

Perhaps this will salvage what you meant:

"git filter-branch -- --all" prints error messages when
processing refs that point at objects that are not
committish.

> Such refs can be created by "git replace" with
> trees or blobs. And also "git tag" with trees or blobs can create such refs.
>
> Filter these problematic refs out early, before they are seen by the logic to
> see which refs have been modified and which have been left intact (which is
> where the unwanted error messages come from), and warn that these refs are 
> left
> unwritten while doing so.
> ---

Please sign-off your patch (cf. Documentation/SubmittingPatches).

Otherwise this round looks good.

Thanks.

>  git-filter-branch.sh | 14 --
>  t/t7003-filter-branch.sh | 14 ++
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 1b7e4b2..41efecb 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -251,8 +251,18 @@ done < "$tempdir"/backup-refs
>  
>  # The refs should be updated if their heads were rewritten
>  git rev-parse --no-flags --revs-only --symbolic-full-name \
> - --default HEAD "$@" > "$tempdir"/raw-heads || exit
> -sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
> + --default HEAD "$@" > "$tempdir"/raw-refs || exit
> +while read ref
> +do
> + case "$ref" in ^?*) continue ;; esac
> +
> + if git rev-parse --verify "$ref"^0 >/dev/null 2>&1
> + then
> + echo "$ref"
> + else
> + warn "WARNING: not rewriting '$ref' (not a committish)"
> + fi
> +done >"$tempdir"/heads <"$tempdir"/raw-refs
>  
>  test -s "$tempdir"/heads ||
>   die "You must specify a ref to rewrite."
> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index 7cb6079..04f79f3 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name 
> vs pathname ambiguity' '
>   git show HEAD:$ambiguous
>  '
>  
> +test_expect_success 'rewrite repository including refs that point at 
> non-commit object' '
> + test_when_finished "git reset --hard original" &&
> + tree=$(git rev-parse HEAD^{tree}) &&
> + test_when_finished "git replace -d $tree" &&
> + echo A >new &&
> + git add new &&
> + new_tree=$(git write-tree) &&
> + git replace $tree $new_tree &&
> + git tag -a -m "tag to a tree" treetag $new_tree &&
> + git reset --hard HEAD &&
> + git filter-branch -f -- --all >filter-output 2>&1 &&
> + ! fgrep fatal filter-output
> +'
> +
>  test_done

Re: [PATCH 4/3] sha1_name: use bsearch_pack() in unique_in_pack()

René Scharfe  writes:

> Replace the custom binary search in unique_in_pack() with a call to
> bsearch_pack().  This reduces code duplication and makes use of the
> fan-out table of packs.
>
> Signed-off-by: Rene Scharfe 
> ---
> This is basically the same replacement as done by patch 3.  Speed is
> less of a concern here -- at least I don't know a commonly used
> command that needs to resolve lots of short hashes.

Looks correct.  Did you find this by eyeballing, or do you have some
interesting tool you use?

>
>  sha1_name.c | 21 ++---
>  1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 24894b3dbe..0185c6081a 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -150,31 +150,14 @@ static int match_sha(unsigned len, const unsigned char 
> *a, const unsigned char *
>  static void unique_in_pack(struct packed_git *p,
>  struct disambiguate_state *ds)
>  {
> - uint32_t num, last, i, first = 0;
> + uint32_t num, i, first = 0;
>   const struct object_id *current = NULL;
>  
>   if (open_pack_index(p) || !p->num_objects)
>   return;
>  
>   num = p->num_objects;
> - last = num;
> - while (first < last) {
> - uint32_t mid = first + (last - first) / 2;
> - const unsigned char *current;
> - int cmp;
> -
> - current = nth_packed_object_sha1(p, mid);
> - cmp = hashcmp(ds->bin_pfx.hash, current);
> - if (!cmp) {
> - first = mid;
> - break;
> - }
> - if (cmp > 0) {
> - first = mid+1;
> - continue;
> - }
> - last = mid;
> - }
> + bsearch_pack(>bin_pfx, p, );
>  
>   /*
>* At this point, "first" is the location of the lowest object

Hello Beautiful

2018-03-25 Thread Jack

Hi Dear, my name is Jack and i am seeking for a relationship in which i will 
feel loved after a series of failed relationships. 

I am hoping that you would be interested and we could possibly get to know each 
other more if you do not mind. I am open to answering questions from you as i 
think my approach is a little inappropriate. Hope to hear back from you.

Jack.

Re: [PATCH] bisect: use oid_to_hex() for converting object_id hashes to hex strings

On Sun, Mar 25, 2018 at 12:57:36PM +0200, René Scharfe wrote:
> Patch generated with Coccinelle and contrib/coccinelle/object_id.cocci.
> 
> Signed-off-by: Rene Scharfe 
> ---
> This is a belated follow-up to f0a6068a9f (bisect: debug: convert struct
> object to object_id).

This looks good to me.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [GSoC] Convert “git stash” to builtin proposal

2018-03-25 Thread Paul-Sebastian Ungureanu


On 25.03.2018 12:46, Christian Couder wrote:

On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu
 wrote:

On 23.03.2018 19:11, Christian Couder wrote:


* Ensure that no regression occurred: considering that there are plenty
of tests and that I have a good understanding of the function, this
should be a trivial task.


There are a lot of things that the test suite doesn't test.


Hopefully, by first adding new tests, any eventual bug will be spotted.


I was thinking about things like memory leaks that tests cannot easily spot.> 


I will do my best and follow best practices in order to avoid any memory 
leaks. However, to make sure that my code is really leak free, I will 
use Valgrind, which is already integrated with the testing framework.



Ok. Feel free to resend another version of your proposal.
Sorry for not sending the whole proposal again. I decided to send only 
the changed parts because the proposal is quite big and I wanted to 
avoid sending the same thing over and over again.


One thing I did not mention in the previous reply was that I also added 
a new paragraph to "Benefits to community" about 'git stash' being slow 
on Windows for a lot of users. I consider this alone to be a very good 
justification for this project and doing this project will be very 
beneficial for the Windows users.


Best regards,
Paul Ungureanu

Re: [GSoC] Convert “git stash” to builtin proposal

2018-03-25 Thread Paul-Sebastian Ungureanu


On 23.03.2018 19:06, Johannes Schindelin wrote:

[... proposal ...]


This is a pretty good proposal. The initial draft at converting `stash
list` is a good start (it will need to be converted to avoid spawning an
extra process, but that is something we can do incrementally, together).


Thank you for your kind words. It feels good to see other people 
appreciate my work. It is a strong incentive to keep going on. I made a 
few adjustments and I hope that the final version will be better.


Best regards,
Paul Ungureanu

[PATCH v5 1/6] worktree: improve message when creating a new worktree

Currently 'git worktree add' produces output like the following, when
'--no-checkout' is not given:

Preparing foo (identifier foo)
HEAD is now at 26da330922 

where the first line is written to stderr, and the second line coming
from 'git reset --hard' is written to stdout, even though both lines are
supposed to tell the user what has happened.  In addition to someone not
familiar with 'git worktree', this might seem as if the current HEAD was
modified, not the HEAD in the new working tree.

If the '--no-checkout' flag is given, the output of 'git worktree add'
is just:

Preparing foo (identifier foo)

even though the HEAD is set to a commit, which is just not checked out.

The identifier is also not particularly relevant for the user at the
moment, as it's only used internally to distinguish between different
worktrees that have the same $(basename ).

Fix these inconsistencies, and no longer show the identifier by making
the 'git reset --hard' call quiet, and printing the message directly
from the builtin command instead.

Signed-off-by: Thomas Gummerer 
---
 builtin/worktree.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7cef5b120b..535734cc7f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -303,8 +303,6 @@ static int add_worktree(const char *path, const char 
*refname,
strbuf_addf(, "%s/commondir", sb_repo.buf);
write_file(sb.buf, "../..");
 
-   fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
-
argv_array_pushf(_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
argv_array_pushf(_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
cp.git_cmd = 1;
@@ -320,10 +318,19 @@ static int add_worktree(const char *path, const char 
*refname,
if (ret)
goto done;
 
+   fprintf(stderr, _("new worktree HEAD is now at %s"),
+   find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
+
+   strbuf_reset();
+   pp_commit_easy(CMIT_FMT_ONELINE, commit, );
+   if (sb.len > 0)
+   fprintf(stderr, " %s", sb.buf);
+   fputc('\n', stderr);
+
if (opts->checkout) {
cp.argv = NULL;
argv_array_clear();
-   argv_array_pushl(, "reset", "--hard", NULL);
+   argv_array_pushl(, "reset", "--hard", "--quiet", NULL);
cp.env = child_env.argv;
ret = run_command();
if (ret)
-- 
2.16.1.77.g8685934aa2

[PATCH v5 5/6] worktree: teach "add" to check out existing branches

Currently 'git worktree add ' creates a new branch named after the
basename of the path by default.  If a branch with that name already
exists, the command refuses to do anything, unless the '--force' option
is given.

However we can do a little better than that, and check the branch out if
it is not checked out anywhere else.  This will help users who just want
to check an existing branch out into a new worktree, and save a few
keystrokes.

As the current behaviour is to simply 'die()' when a branch with the name
of the basename of the path already exists, there are no backwards
compatibility worries here.

We will still 'die()' if the branch is checked out in another worktree,
unless the --force flag is passed.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-worktree.txt |  9 +++--
 builtin/worktree.c | 19 +--
 t/t2025-worktree-add.sh| 19 ---
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 41585f535d..eaa6bf713f 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -61,8 +61,13 @@ $ git worktree add --track -b   
/
 
 +
 If `` is omitted and neither `-b` nor `-B` nor `--detach` used,
-then, as a convenience, a new branch based at HEAD is created automatically,
-as if `-b $(basename )` was specified.
+then, as a convenience, a worktree with a branch named after
+`$(basename )` (call it ``) is created.  If ``
+doesn't exist, a new branch based on HEAD is automatically created as
+if `-b ` was given.  If `` exists in the repository,
+it will be checked out in the new worktree, if it's not checked out
+anywhere else, otherwise the command will refuse to create the
+worktree (unless `--force` is used).
 
 list::
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index c296c3eacb..895838b943 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -28,6 +28,7 @@ struct add_opts {
int checkout;
int keep_locked;
const char *new_branch;
+   int checkout_existing_branch;
 };
 
 static int show_only;
@@ -317,7 +318,10 @@ static int add_worktree(const char *path, const char 
*refname,
if (ret)
goto done;
 
-   if (opts->new_branch)
+   if (opts->checkout_existing_branch)
+   fprintf_ln(stderr, _("checking out branch '%s'"),
+  refname);
+   else if (opts->new_branch)
fprintf_ln(stderr, _("creating branch '%s'"), opts->new_branch);
 
fprintf(stderr, _("new worktree HEAD is now at %s"),
@@ -370,7 +374,18 @@ static const char *dwim_branch(const char *path, struct 
add_opts *opts)
 {
int n;
const char *s = worktree_basename(path, );
-   opts->new_branch = xstrndup(s, n);
+   const char *branchname = xstrndup(s, n);
+   struct strbuf ref = STRBUF_INIT;
+
+   if (!strbuf_check_branch_ref(, branchname) &&
+   ref_exists(ref.buf)) {
+   opts->checkout_existing_branch = 1;
+   strbuf_release();
+   UNLEAK(branchname);
+   return branchname;
+   }
+
+   opts->new_branch = branchname;
if (guess_remote) {
struct object_id oid;
const char *remote =
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 2b95944973..ae602cf20e 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -198,13 +198,26 @@ test_expect_success '"add" with  omitted' '
test_cmp_rev HEAD bat
 '
 
-test_expect_success '"add" auto-vivify does not clobber existing branch' '
+test_expect_success '"add" checks out existing branch of dwimd name' '
test_commit c1 &&
test_commit c2 &&
git branch precious HEAD~1 &&
-   test_must_fail git worktree add precious &&
+   git worktree add precious &&
test_cmp_rev HEAD~1 precious &&
-   test_path_is_missing precious
+   (
+   cd precious &&
+   test_cmp_rev precious HEAD
+   )
+'
+
+test_expect_success '"add" auto-vivify fails with checked out branch' '
+   git checkout -b test-branch &&
+   test_must_fail git worktree add test-branch &&
+   test_path_is_missing test-branch
+'
+
+test_expect_success '"add --force" with existing dwimd name doesnt die' '
+   git worktree add --force test-branch
 '
 
 test_expect_success '"add" no auto-vivify with --detach and  omitted' '
-- 
2.16.1.77.g8685934aa2

[PATCH v5 6/6] t2025: rename now outdated branch name

Before the previous commit, the branch named precious was used to check
that 'git worktree' wouldn't clobber the branch.  While 'git worktree'
still shouldn't (and doesn't) modify the branch, that's no longer the
main thing the test is checking.

Rename the branch to avoid making future readers wonder why this
particular branch is more "precious" than others.

Signed-off-by: Thomas Gummerer 
---
 t/t2025-worktree-add.sh | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index ae602cf20e..fb99f4c46f 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -201,12 +201,12 @@ test_expect_success '"add" with  omitted' '
 test_expect_success '"add" checks out existing branch of dwimd name' '
test_commit c1 &&
test_commit c2 &&
-   git branch precious HEAD~1 &&
-   git worktree add precious &&
-   test_cmp_rev HEAD~1 precious &&
+   git branch dwim HEAD~1 &&
+   git worktree add dwim &&
+   test_cmp_rev HEAD~1 dwim &&
(
-   cd precious &&
-   test_cmp_rev precious HEAD
+   cd dwim &&
+   test_cmp_rev dwim HEAD
)
 '
 
-- 
2.16.1.77.g8685934aa2

[PATCH v5 4/6] worktree: factor out dwim_branch function

Factor out a dwim_branch function, which takes care of the dwim'ery in
'git worktree add '.  It's not too much code currently, but we're
adding a new kind of dwim in a subsequent patch, at which point it makes
more sense to have it as a separate function.

Factor it out now to reduce the patch noise in the next patch.

Signed-off-by: Thomas Gummerer 
---
 builtin/worktree.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 1e4a919a00..c296c3eacb 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -366,6 +366,20 @@ static int add_worktree(const char *path, const char 
*refname,
return ret;
 }
 
+static const char *dwim_branch(const char *path, struct add_opts *opts)
+{
+   int n;
+   const char *s = worktree_basename(path, );
+   opts->new_branch = xstrndup(s, n);
+   if (guess_remote) {
+   struct object_id oid;
+   const char *remote =
+   unique_tracking_name(opts->new_branch, );
+   return remote;
+   }
+   return NULL;
+}
+
 static int add(int ac, const char **av, const char *prefix)
 {
struct add_opts opts;
@@ -417,16 +431,9 @@ static int add(int ac, const char **av, const char *prefix)
}
 
if (ac < 2 && !opts.new_branch && !opts.detach) {
-   int n;
-   const char *s = worktree_basename(path, );
-   opts.new_branch = xstrndup(s, n);
-   if (guess_remote) {
-   struct object_id oid;
-   const char *remote =
-   unique_tracking_name(opts.new_branch, );
-   if (remote)
-   branch = remote;
-   }
+   const char *dwim_branchname = dwim_branch(path, );
+   if (dwim_branchname)
+   branch = dwim_branchname;
}
 
if (ac == 2 && !opts.new_branch && !opts.detach) {
-- 
2.16.1.77.g8685934aa2

[PATCH v5 0/6] worktree: teach "add" to check out existing branches

Thanks Eric for the review of the previous round and Duy and Junio for
additional comments.

Previous rounds are at <20180121120208.12760-1-t.gumme...@gmail.com>,
<20180204221305.28300-1-t.gumme...@gmail.com>,
<20180317220830.30963-1-t.gumme...@gmail.com> and
<2018031719.4940-1-t.gumme...@gmail.com>.

This round should address all of Eric's comments from the previous round.

As explained in more detail in a reply to the review comment directly,
I did not add an enum to 'struct add_opts', for 'force_new_branch' and
'checkout_existing_branch', but instead removed 'force_new_branch'
from the struct as it's not required.

The rest of the updates are mainly in the user facing messages,
documentation and one added test.

Interdiff below:

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 98731b71a7..eaa6bf713f 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -67,7 +67,7 @@ doesn't exist, a new branch based on HEAD is automatically 
created as
 if `-b ` was given.  If `` exists in the repository,
 it will be checked out in the new worktree, if it's not checked out
 anywhere else, otherwise the command will refuse to create the
-worktree.
+worktree (unless `--force` is used).
 
 list::
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index df5c0427ba..895838b943 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -28,7 +28,6 @@ struct add_opts {
int checkout;
int keep_locked;
const char *new_branch;
-   int force_new_branch;
int checkout_existing_branch;
 };
 
@@ -320,12 +319,12 @@ static int add_worktree(const char *path, const char 
*refname,
goto done;
 
if (opts->checkout_existing_branch)
-   fprintf(stderr, _("checking out branch '%s'"),
-   refname);
+   fprintf_ln(stderr, _("checking out branch '%s'"),
+  refname);
else if (opts->new_branch)
-   fprintf(stderr, _("creating branch '%s'"), opts->new_branch);
+   fprintf_ln(stderr, _("creating branch '%s'"), opts->new_branch);
 
-   fprintf(stderr, _("worktree HEAD is now at %s"),
+   fprintf(stderr, _("new worktree HEAD is now at %s"),
find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
 
strbuf_reset();
@@ -434,8 +433,7 @@ static int add(int ac, const char **av, const char *prefix)
if (!strcmp(branch, "-"))
branch = "@{-1}";
 
-   opts.force_new_branch = !!new_branch_force;
-   if (opts.force_new_branch) {
+   if (new_branch_force) {
struct strbuf symref = STRBUF_INIT;
 
opts.new_branch = new_branch_force;
@@ -472,7 +470,7 @@ static int add(int ac, const char **av, const char *prefix)
struct child_process cp = CHILD_PROCESS_INIT;
cp.git_cmd = 1;
argv_array_push(, "branch");
-   if (opts.force_new_branch)
+   if (new_branch_force)
argv_array_push(, "--force");
argv_array_push(, opts.new_branch);
argv_array_push(, branch);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 721b0e4c26..fb99f4c46f 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -198,15 +198,15 @@ test_expect_success '"add" with  omitted' '
test_cmp_rev HEAD bat
 '
 
-test_expect_success '"add" auto-vivify checks out existing branch' '
+test_expect_success '"add" checks out existing branch of dwimd name' '
test_commit c1 &&
test_commit c2 &&
-   git branch precious HEAD~1 &&
-   git worktree add precious &&
-   test_cmp_rev HEAD~1 precious &&
+   git branch dwim HEAD~1 &&
+   git worktree add dwim &&
+   test_cmp_rev HEAD~1 dwim &&
(
-   cd precious &&
-   test_cmp_rev precious HEAD
+   cd dwim &&
+   test_cmp_rev dwim HEAD
)
 '
 
@@ -216,6 +216,10 @@ test_expect_success '"add" auto-vivify fails with checked 
out branch' '
test_path_is_missing test-branch
 '
 
+test_expect_success '"add --force" with existing dwimd name doesnt die' '
+   git worktree add --force test-branch
+'
+
 test_expect_success '"add" no auto-vivify with --detach and  omitted' '
git worktree add --detach mish/mash &&
test_must_fail git rev-parse mash -- &&

Thomas Gummerer (6):
  worktree: improve message when creating a new worktree
  worktree: be clearer when "add" dwim-ery kicks in
  worktree: remove force_new_branch from struct add_opts
  worktree: factor out dwim_branch function
  worktree: teach "add" to check out existing branches
  t2025: rename now outdated branch name

 Documentation/git-worktree.txt |  9 --
 builtin/worktree.c | 64 +++---
 t/t2025-worktree-add.sh| 23 +++
 3 files changed, 72

[PATCH v5 2/6] worktree: be clearer when "add" dwim-ery kicks in

Currently there is no indication in the "git worktree add" output that
a new branch was created.  This would be especially useful information
in the case where the dwim of "git worktree add " kicks in, as the
user didn't explicitly ask for a new branch, but we create one from
them.

Print some additional output showing that a branch was created and the
branch name to help the user.

This will also be useful in the next commit, which introduces a new kind
of dwim-ery of checking out the branch if it exists instead of refusing
to create a new worktree in that case, and where it's nice to tell the
user which kind of dwim-ery kicked in.

Signed-off-by: Thomas Gummerer 
---
 builtin/worktree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 535734cc7f..a082230b6c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -318,6 +318,9 @@ static int add_worktree(const char *path, const char 
*refname,
if (ret)
goto done;
 
+   if (opts->new_branch)
+   fprintf_ln(stderr, _("creating branch '%s'"), opts->new_branch);
+
fprintf(stderr, _("new worktree HEAD is now at %s"),
find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
 
-- 
2.16.1.77.g8685934aa2

[PATCH v5 3/6] worktree: remove force_new_branch from struct add_opts

The 'force_new_branch' flag in 'struct add_opts' is only used inside the
add function, where we already have the same information stored in the
'new_branch_force' variable.  Avoid that unnecessary duplication.

Signed-off-by: Thomas Gummerer 
---
 builtin/worktree.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index a082230b6c..1e4a919a00 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -28,7 +28,6 @@ struct add_opts {
int checkout;
int keep_locked;
const char *new_branch;
-   int force_new_branch;
 };
 
 static int show_only;
@@ -405,8 +404,7 @@ static int add(int ac, const char **av, const char *prefix)
if (!strcmp(branch, "-"))
branch = "@{-1}";
 
-   opts.force_new_branch = !!new_branch_force;
-   if (opts.force_new_branch) {
+   if (new_branch_force) {
struct strbuf symref = STRBUF_INIT;
 
opts.new_branch = new_branch_force;
@@ -450,7 +448,7 @@ static int add(int ac, const char **av, const char *prefix)
struct child_process cp = CHILD_PROCESS_INIT;
cp.git_cmd = 1;
argv_array_push(, "branch");
-   if (opts.force_new_branch)
+   if (new_branch_force)
argv_array_push(, "--force");
argv_array_push(, opts.new_branch);
argv_array_push(, branch);
-- 
2.16.1.77.g8685934aa2

RE

2018-03-25 Thread Ms. Ella Golan

I am Ms.Ella Golan, I am the Executive Vice President Banking Division with 
FIRST INTERNATIONAL BANK OF ISRAEL LTD (FIBI). I am getting in touch with you 
regarding an extremely important and urgent matter. If you would oblige me the 
opportunity, I shall provide you with details upon your response.

Faithfully,
Ms.Ella Golan

Proposal

2018-03-25 Thread @Sheng Li Hung

I have a very profitable business for you

[PATCH] bisect: use oid_to_hex() for converting object_id hashes to hex strings

Patch generated with Coccinelle and contrib/coccinelle/object_id.cocci.

Signed-off-by: Rene Scharfe 
---
This is a belated follow-up to f0a6068a9f (bisect: debug: convert struct
object to object_id).

 bisect.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bisect.c b/bisect.c
index f6d05bd66f..319d60edad 100644
--- a/bisect.c
+++ b/bisect.c
@@ -144,10 +144,10 @@ static void show_list(const char *debug, int counted, int 
nr,
fprintf(stderr, "%3d", weight(p));
else
fprintf(stderr, "---");
-   fprintf(stderr, " %.*s", 8, 
sha1_to_hex(commit->object.oid.hash));
+   fprintf(stderr, " %.*s", 8, oid_to_hex(>object.oid));
for (pp = commit->parents; pp; pp = pp->next)
fprintf(stderr, " %.*s", 8,
-   sha1_to_hex(pp->item->object.oid.hash));
+   oid_to_hex(>item->object.oid));
 
subject_len = find_commit_subject(buf, _start);
if (subject_len)
-- 
2.16.3

[PATCH] run-command: use strbuf_addstr() for adding a string to a strbuf

Patch generated with Coccinelle and contrib/coccinelle/strbuf.cocci.

Signed-off-by: Rene Scharfe 
---
That line was added by e73dd78699 (run-command.c: introduce
trace_run_command()).

 run-command.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index a483d5904a..84899e423f 100644
--- a/run-command.c
+++ b/run-command.c
@@ -621,7 +621,7 @@ static void trace_run_command(const struct child_process 
*cp)
if (!trace_want(_default_key))
return;
 
-   strbuf_addf(, "trace: run_command:");
+   strbuf_addstr(, "trace: run_command:");
if (cp->dir) {
strbuf_addstr(, " cd ");
sq_quote_buf_pretty(, cp->dir);
-- 
2.16.3

Re: Null pointer dereference in git-submodule

Am 25.03.2018 um 11:50 schrieb Jeremy Feusi:
> 
> Hmm... That's weird. I can reproduce it on 3 independant systems with
> versions 2.16.2 up, although it does not work with version 2.11.0.
> Anyway, I figured out how to reproduce this bug. It is caused when a
> submodule is added and then the directory it resides in is moved or
> deleted without commiting. For example:
> 
> git init
> git submodule add https://github.com/git/git git
> mv git git.BAK
> git submodule status #this command segfaults

With the patch I sent in my first reply the last command reports:

fatal: no ref store in submodule 'git'

That may not be the most helpful message -- not just the ref store is
missing, the whole submodule is gone!

Come to think about it, this removal may be intended.  How about
showing the submodule as not being initialized at that point?

-- >8 --
Subject: [PATCH v2] submodule: check for NULL return of 
get_submodule_ref_store()

If we can't find a ref store for a submodule then assume it the latter
is not initialized (or was removed).  Print a status line accordingly
instead of causing a segmentation fault by passing NULL as the first
parameter of refs_head_ref().

Reported-by: Jeremy Feusi 
Signed-off-by: Rene Scharfe 
---
Test missing..

 builtin/submodule--helper.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index ee020d4749..ae3014ac5a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -654,9 +654,13 @@ static void status_submodule(const char *path, const 
struct object_id *ce_oid,
 displaypath);
} else if (!(flags & OPT_CACHED)) {
struct object_id oid;
+   struct ref_store *refs = get_submodule_ref_store(path);
 
-   if (refs_head_ref(get_submodule_ref_store(path),
- handle_submodule_head_ref, ))
+   if (!refs) {
+   print_status(flags, '-', path, ce_oid, displaypath);
+   goto cleanup;
+   }
+   if (refs_head_ref(refs, handle_submodule_head_ref, ))
die(_("could not resolve HEAD ref inside the "
  "submodule '%s'"), path);
 
-- 
2.17.0.rc1.38.g7c51fd80b8

Hello Beautiful

2018-03-25 Thread Jack

Hi Dear, my name is Jack and i am seeking for a relationship in which i will 
feel loved after a series of failed relationships. 

I am hoping that you would be interested and we could possibly get to know each 
other more if you do not mind. I am open to answering questions from you as i 
think my approach is a little inappropriate. Hope to hear back from you.

Jack.

RE

2018-03-25 Thread Ms. Ella Golan

I am Ms.Ella Golan, I am the Executive Vice President Banking Division with 
FIRST INTERNATIONAL BANK OF ISRAEL LTD (FIBI). I am getting in touch with you 
regarding an extremely important and urgent matter. If you would oblige me the 
opportunity, I shall provide you with details upon your response.

Faithfully,
Ms.Ella Golan

Re: Null pointer dereference in git-submodule

2018-03-25 Thread Jeremy Feusi


Reply-To: 

Hmm... That's weird. I can reproduce it on 3 independant systems with
versions 2.16.2 up, although it does not work with version 2.11.0.
Anyway, I figured out how to reproduce this bug. It is caused when a
submodule is added and then the directory it resides in is moved or
deleted without commiting. For example:

git init
git submodule add https://github.com/git/git git
mv git git.BAK
git submodule status #this command segfaults

cheers
Jeremy

In-Reply-To: 

On Sat, Mar 24, 2018 at 09:42:57PM +0100, René Scharfe wrote:
> Am 24.03.2018 um 18:42 schrieb Jeremy Feusi:
> > Hi,
> > While bootstrapping a gnu repository I noticed that git segfaulted when
> > called as "git submodule status". After compiling git with address
> > sanitizer and minimizing the directory I finally narrowed it down to the
> > files which I have attached as a tar archive. Here is a detailed backtrace:
> > 
> > AddressSanitizer:DEADLYSIGNAL
> > =
> > ==63400==ERROR: AddressSanitizer: SEGV on unknown address 0x 
> > (pc 0x00c27a93 bp 0x7ffdcb4eec10 sp 0x7ffdcb4eeb80 T0)
> > ==63400==The signal is caused by a READ memory access.
> > ==63400==Hint: address points to the zero page.
> >  #0 0xc27a92 in refs_read_raw_ref /home/jfe/git/refs.c:1451:20
> >  #1 0xc174a6 in refs_resolve_ref_unsafe /home/jfe/git/refs.c:1493:7
> >  #2 0xc1826a in refs_read_ref_full /home/jfe/git/refs.c:224:6
> >  #3 0xc26d53 in refs_head_ref /home/jfe/git/refs.c:1314:7
> >  #4 0x8071e6 in status_submodule 
> > /home/jfe/git/builtin/submodule--helper.c:658:7
> >  #5 0x806a89 in status_submodule_cb 
> > /home/jfe/git/builtin/submodule--helper.c:699:2
> >  #6 0x80523e in for_each_listed_submodule 
> > /home/jfe/git/builtin/submodule--helper.c:438:3
> >  #7 0x7f7e9a in module_status 
> > /home/jfe/git/builtin/submodule--helper.c:732:2
> >  #8 0x7efd69 in cmd_submodule__helper 
> > /home/jfe/git/builtin/submodule--helper.c:1859:11
> >  #9 0x51e024 in run_builtin /home/jfe/git/git.c:346:11
> >  #10 0x5192c2 in handle_builtin /home/jfe/git/git.c:554:8
> >  #11 0x51d0f0 in run_argv /home/jfe/git/git.c:606:4
> >  #12 0x518600 in cmd_main /home/jfe/git/git.c:683:19
> >  #13 0x8501d6 in main /home/jfe/git/common-main.c:43:9
> >  #14 0x7f49fdaf2f29 in __libc_start_main 
> > (/lib/x86_64-linux-gnu/libc.so.6+0x20f29)
> >  #15 0x41f4b9 in _start 
> > (/home/jfe/git/inst/libexec/git-core/git+0x41f4b9)
> > 
> > AddressSanitizer can not provide additional info.
> > SUMMARY: AddressSanitizer: SEGV /home/jfe/git/refs.c:1451:20 in 
> > refs_read_raw_ref
> > ==63400==ABORTING
> > 
> > As mentioned above, this bug is triggered by issuing the command
> > "git submodule status" while in the attached directory.
> > 
> > This bug was confirmed on Debian with version 2.16.1 and
> > 2.17.0.rc1.35.g90bbd502d as well as on Arch Linux with version 2.16.2
> > where further output is given by git:
> > 
> > /usr/lib/git-core/git-submodule: line 979:  8119 Segmentation fault  
> > (core dumped) git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix 
> > "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} 
> > ${cached:+--cached} ${recursive:+--recursive} "$@"
> > 
> 
> You may have minimized too much.  With the patch below I get:
> 
>   fatal: no ref store in submodule 'gnulib'
> 
> I guess you'll get a different one in your original repo.
> 
> The patch seems like a good idea in any case, though.
> 
> -- >8 --
> Subject: [PATCH] submodule: check for NULL return of get_submodule_ref_store()
> 
> refs_head_ref() requires a valid ref_store pointer to be given as its
> first argument.  get_submodule_ref_store() can return NULL.  Exit and
> report the failure to find a ref store in that case instead of
> segfaulting.
> 
> Reported-by: Jeremy Feusi 
> Signed-off-by: Rene Scharfe 
> ---
>  builtin/submodule--helper.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index ee020d4749..0f74e81005 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -654,9 +654,11 @@ static void status_submodule(const char *path, const 
> struct object_id *ce_oid,
>displaypath);
>   } else if (!(flags & OPT_CACHED)) {
>   struct object_id oid;
> + struct ref_store *refs = get_submodule_ref_store(path);
>  
> - if (refs_head_ref(get_submodule_ref_store(path),
> -   handle_submodule_head_ref, ))
> + if (!refs)
> + die(_("no ref store in submodule '%s'"), path);
> + if (refs_head_ref(refs, handle_submodule_head_ref, ))
>   die(_("could not resolve HEAD ref inside the "
>

Re: [GSoC] Convert “git stash” to builtin proposal

On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu
 wrote:
> On 23.03.2018 19:11, Christian Couder wrote:
>
>>> * Ensure that no regression occurred: considering that there are plenty
>>> of tests and that I have a good understanding of the function, this
>>> should be a trivial task.
>>
>> There are a lot of things that the test suite doesn't test.
>
> Hopefully, by first adding new tests, any eventual bug will be spotted.

I was thinking about things like memory leaks that tests cannot easily spot.

>>> * In the end, an analysis based on performance can be made.
>>> Benchmarking the script will be done by recording the running time
>>> given a large set of diversified tests that cover all the
>>> functionalities, before and after conversion. The new commands should
>>> run faster just because they were written in C, but I expect to see
>>> even more improvements.
>>
>> If you want to do benchmarks, you might want to add performance tests
>> into t/perf.
>
> That is great. I will surely make use of the existent testing framework to
> do benchmarks.

Good.

>>> ## Example of conversion (for “git stash list”)
>>> --
>>> To test my capabilities and to be sure that I am able to work on a
>>> project of this kind, I tried to convert “git stash list” into a built-
>>> in. The result can be found below in text-only version or at the Github
>>> link.
>>
>> I think it would be better if it was sent as a patch (maybe an RFC
>> patch) to the mailing list and add the link to the patch email in the
>> maling list archive to your proposal.
>
> I sent it as a [RFC] patch to the mailing list and included it in the
> proposal.
>
> 

Nice.

>> It could be useful if you described a bit more how you could (re)use
>> the work that has already been made.
>>
>> In the rest of your proposal it looks like you are starting from
>> scratch, which is a bit strange if a lot of progress has already been
>> made.
>
> The patches about converting ‘git stash’ are a great starting point and I
> will definitely use them. Each time I start converting a new command I will
> first take a look at what other patches there are, evaluate them and if I
> consider the patch to be good enough I will continue my work on top of that
> patch. Otherwise, I will start from scratch and that patch will only serve
> for comparison.
>
> One other resource that is of great importance is git itself. I can learn
> how a builtin is structured and how it is built by considering examples the
> other Git commands. Having a similar coding standard used, the codebase will
> be homogeneous and hopefully easier to maintain.
>
> Another important resource are commands that are Google Summer of Code
> projects from previous years (2016 and 2017) which had as purpose to convert
> “git bisect” and “git submodule”. I can always take a look at the patches
> they submitted and read their code reviews to avoid making same mistakes
> they did.

Nice.

>> It is probably better especially for reviewers and more common to work
>> in small batches, for example to send a patch series converting a few
>> related commands, then to start working on the next small batch of
>> commands in another patch series while improving the first patch
>> series according to reviews.
>>
>> Also we ask GSoC students to communicate publicly every week about
>> their project for example by writing a blg post or sending a report to
>> the mailing list.
>
> Noted.
>
>>> ## Git contributions
>>> --
>>> My first contribution to Git was to help making “git tag --contains
>>> ” les chatty if  is invalid. Looking over the list of available
>>> microprojects, there were a couple of microprojects which got my
>>> attention, but I picked this up because it seemed to be a long-standing
>>> bug (I noticed it was approached by students in 2016, 2017 and now in
>>> 2018). Thanks to the code reviews from the mailing list, after a few
>>> iterations I succeeded in coming up with a patch that not only fixed
>>> the mentioned problem, but also a few others that were having the same
>>> cause.
>>>
>>> It got merged in the proposed updates branch.
>>
>> What is its status in Junio's "What's cooking in Git" emails?
>
> It is now in the ‘next’ branch of the Git repository.
>
> I updated the proposal, in which I included these ideas and some additional
> examples. Thank you a lot!

Ok. Feel free to resend another version of your proposal.

Thanks.

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

On Sun, Mar 25, 2018 at 8:40 AM, Eric Sunshine  wrote:
> On Sat, Mar 24, 2018 at 1:37 PM, Joel Teichroeb  wrote:

>> +static int do_apply_stash(const char *prefix, struct stash_info *info, int 
>> index)
>> +{
>> +   if (index) {
>> +   if (!oidcmp(>b_tree, >i_tree) || 
>> !oidcmp(_tree, >i_tree)) {
>> +   has_index = 0;
>> +   } else {
>> +   struct child_process cp = CHILD_PROCESS_INIT;
>> +   struct strbuf out = STRBUF_INIT;
>> +   struct argv_array args = ARGV_ARRAY_INIT;
>> +   cp.git_cmd = 1;
>> +   argv_array_pushl(, "diff-tree", "--binary", 
>> NULL);
>> +   argv_array_pushf(, "%s^2^..%s^2", 
>> sha1_to_hex(info->w_commit.hash), sha1_to_hex(info->w_commit.hash));
>> +   if (pipe_command(, NULL, 0, , 0, NULL, 0))
>> +   return -1;
>
> Leaking 'out'?
>
>> +
>> +   child_process_init();
>> +   cp.git_cmd = 1;
>> +   argv_array_pushl(, "apply", "--cached", 
>> NULL);
>> +   if (pipe_command(, out.buf, out.len, NULL, 0, 
>> NULL, 0))
>> +   return -1;
>
> Leaking 'out'.

It might be a good idea to have small functions encapsulating the
forks of git commands. For example:

static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)
{
struct child_process cp = CHILD_PROCESS_INIT;
const char *w_commit_hex = oid_to_hex(w_commit);

cp.git_cmd = 1;
argv_array_pushl(, "diff-tree", "--binary", NULL);
argv_array_pushf(, "%s^2^..%s^2", w_commit_hex, w_commit_hex);

return pipe_command(, NULL, 0, out, 0, NULL, 0);
}

static int apply_cached(struct strbuf *out)
{
struct child_process cp = CHILD_PROCESS_INIT;

cp.git_cmd = 1;
argv_array_pushl(, "apply", "--cached", NULL);
return pipe_command(, out->buf, out->len, NULL, 0, NULL, 0);
}

This could help find leaks and maybe later make it easier to call
libified code instead of forking a git command.

Hello Beautiful

2018-03-25 Thread Jack

Cześć Drogi, nazywam się Jack i szukam związku, w którym będę czuć się kochany 
po serii nieudanych związków.

Mam nadzieję, że byłbyś zainteresowany i moglibyśmy się lepiej poznać, jeśli 
nie masz nic przeciwko. Jestem otwarty na udzielanie odpowiedzi na pytania od 
ciebie, ponieważ uważam, że moje podejście jest trochę niewłaściwe. Mam 
nadzieję, że odezwę się od ciebie.

Jacek.

Re: [GSoC][PATCH v4] test: avoid pipes in git related commands for test

On Fri, Mar 23, 2018 at 11:01 AM, Pratik Karki  wrote:
> I hope this follow-on patch[1] is ready for merge.

This iteration appears to address review comments from the last few
rounds, however, see below for a few new ones...

> Avoid using pipes downstream of Git commands since the exit codes
> of commands upstream of pipes get swallowed, thus potentially
> hiding failure of those commands. Instead, capture Git command
> output to a file and apply the downstream command(s) to that file.
>
> Signed-off-by: Pratik Karki 
> ---
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> @@ -840,8 +840,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch aligned 
> output' '
> test_commit long-tag &&
> (
> cd full-output &&
> -   git -c fetch.output=full fetch origin 2>&1 | \
> -   grep -e "->" | cut -c 22- >../actual
> +   git -c fetch.output=full fetch origin >actual2 2>&1 &&
> +   grep -e "->" actual2 | cut -c 22- >../actual

The file "actual2" is clearly distinct from the file "../actual", so
inventing a name ("actual2") isn't particularly helping; you could
just as easily also name it "actual" without hurting comprehension.
(Not necessarily worth a re-roll.)

> ) &&
> @@ -855,8 +855,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch compact 
> output' '
> test_commit extraaa &&
> (
> cd compact &&
> -   git -c fetch.output=compact fetch origin 2>&1 | \
> -   grep -e "->" | cut -c 22- >../actual
> +   git -c fetch.output=compact fetch origin >actual2 2>&1 &&
> +   grep -e "->" actual2 | cut -c 22- >../actual

Same comment.

> ) &&
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> @@ -74,11 +74,12 @@ test_expect_success 'iso-8859-1' '
> git commit -s -m den file &&
> -   git fast-export wer^..wer |
> -   sed "s/wer/i18n/" |
> +   git fast-export wer^..wer >actual &&
> +   sed "s/wer/i18n/" actual |
> (cd new &&
>  git fast-import &&
> -git cat-file commit i18n | grep "Áéí óú")
> +git cat-file commit i18n >actual &&
> +grep "Áéí óú" actual)

It was a bit surprising to see a new "actual" file created inside the
subshell even as 'sed' is processing a file named "actual" outside the
subshell, and, as a reader, I was concerned about bad interaction
between the operations. However, the file in the subshell is really
"new/actual", thus is distinct from the other "actual", so it's okay.

This is one of those cases, however, in which it might make sense to
give the files different names to make the code easier to grok, so
future readers don't stumble over this as well. For instance, the
outer file could be named "iso8859-1.fi" (or something), and the file
in the subshell can remain "actual". Not itself worth a re-roll, but
probably a good idea.

(This differs in couple ways from my comment above about t5510 tests
naming files "actual2" and "../actual". In that case, it was quite
clear that, within the cd'd subshell, file "../actual" was distinct
from the file created within the cd'd directory, so no confusion.
Moreover, those files were not being accessed at the same time,
whereas in this t9350 test, the 'sed' is reading from the a file at
the same time as 'git cat-file' is outputting to a similarly named
file, which is potentially confusing and requires extra brain cycles
to sort out.)

>  '
> @@ -87,18 +88,16 @@ test_expect_success 'import/export-marks' '
> test_line_count = 3 tmp-marks &&
> -   test $(
> -   git fast-export --import-marks=tmp-marks\
> -   --export-marks=tmp-marks HEAD |
> -   grep ^commit |
> +   git fast-export --import-marks=tmp-marks \
> +   --export-marks=tmp-marks HEAD >actual &&
> +   test $(grep ^commit actual |
> wc -l) \
> -eq 0 &&

Since the git-fast-export invocation has been pulled out of the
$(...), the entire 'test' expression is now short enough to fit easily
on one line. Making such a change would improve readability
considering how hard it is to read split over three lines like that
(with inconsistent indentation, moreover):

test $(grep ^commit actual | wc -l) -eq 0 &&

> echo change > file &&
> git commit -m "last commit" file &&
> -   test $(
> -   git fast-export --import-marks=tmp-marks \
> -   --export-marks=tmp-marks HEAD |
> -   grep ^commit\  |
> +   git fast-export --import-marks=tmp-marks \
> +   --export-marks=tmp-marks HEAD >actual &&
> +   test $(grep ^commit\  actual |
> wc -l) \
> -eq 1 &&

Same comment.

> test_line_count = 4 tmp-marks
> @@ -500,13 +501,13 @@ test_expect_success 'refs are updated even

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

On Sat, Mar 24, 2018 at 6:37 PM, Joel Teichroeb  wrote:
> diff --git a/git-stash.sh b/git-stash.sh
> index 92c084eb17..360643ad4e 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -736,7 +736,8 @@ pop)
> ;;
>  branch)
> shift
> -   apply_to_branch "$@"
> +   cd "$START_DIR"
> +   git stash--helper branch "$@"
> ;;
>  *)
> case $# in

Can the apply_to_branch() shell function be removed from git-stash.sh?

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

On Sat, Mar 24, 2018 at 6:37 PM, Joel Teichroeb  wrote:
> diff --git a/git-stash.sh b/git-stash.sh
> index fc8f8ae640..92c084eb17 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -711,7 +711,8 @@ push)
> ;;
>  apply)
> shift
> -   apply_stash "$@"
> +   cd "$START_DIR"
> +   git stash--helper apply "$@"
> ;;
>  clear)
> shift

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 remove?

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

2018-03-25 Thread Kaartic Sivaraam

On Sunday 25 March 2018 11:18 AM, Eric Sunshine wrote:
> On Sun, Mar 25, 2018 at 09:11:34AM +0530, Kaartic Sivaraam wrote:
>> On Sunday 25 March 2018 07:04 AM, Eric Sunshine wrote:
>>> Can we have a couple new tests: one checking "git branch --list" for
>>> the typical case (when rebasing off a named branch) and one checking
>>> when rebasing from a detached HEAD?
>>
>> Sure, but I guess it would take some time for me to add the tests. I'll
>> send a v2 with the suggested changes.
> 
> A couple more comments:
> 
> * Please run the commit message through a spell checker; it contains
>   several typographical errors.
> 

Thanks for motivating me to search for a spell checker. I have now
discovered the spell check feature (:set spell) in Vim!


> * I wonder if it makes sense to give slightly different output in the
>   detached HEAD case. Normal output is:
> 
>   (no branch, rebasing )
> 
>   and, with your change, detached HEAD output is:
> 
>   (no branch, rebasing d3adb33f)
> 
>   which is okay, but perhaps it could be better; for instance:
> 
>   (no branch, rebasing detached HEAD d3adb33f)
> 

I just recently discovered that the variable used to print information
related to detached HEAD (state.detached_from) might also contain remote
branch names (origin/master, etc.) other than commit hashes. So, it
might make sense to distinguish detached HEAD.


> Anyhow, I wrote the tests for you. When you re-roll, you can make the
> following patch 2/2 and your fix 1/2.
Thanks a lot!


> (If you go with the above idea
> of using a slightly different wording for the detached HEAD case, then
> you'll need to adjust the 'grep' slightly in the second test.)
> 
> --- >8 ---
> From: Eric Sunshine 
> Date: Sun, 25 Mar 2018 01:29:58 -0400
> Subject: [PATCH] t3200: verify "branch --list" sanity when rebasing from
>  detached HEAD
> 
> "git branch --list" shows an in-progress rebase as:
> 
>   * (no branch, rebasing )
> master
> ...
> 
> However, if the rebase is started from a detached HEAD, then there is no
> , and it would attempt to print a NULL pointer. The previous
> commit fixed this problem, so add a test to verify that the output is
> sane in this situation.
> 
> Signed-off-by: Eric Sunshine 
> ---
>  t/t3200-branch.sh | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 6c0b7ea4ad..d1f80c80ab 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -6,6 +6,7 @@
>  test_description='git branch assorted tests'
>  
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh
>  
>  test_expect_success 'prepare a trivial repository' '
>   echo Hello >A &&
> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with 
> --no-merged' '
>   test_must_fail git branch --merged HEAD --no-merged HEAD
>  '
>  
> +test_expect_success '--list during rebase' '
> + test_when_finished "reset_rebase" &&
> + git checkout master &&
> + FAKE_LINES="1 edit 2" &&
> + export FAKE_LINES &&
> + set_fake_editor &&
> + git rebase -i HEAD~2 &&
> + git branch --list >actual &&
> + grep "rebasing master" actual
> +'
> +
> +test_expect_success '--list during rebase from detached HEAD' '
> + test_when_finished "reset_rebase && git checkout master" &&
> + git checkout HEAD^0 &&
> + oid=$(git rev-parse --short HEAD) &&
> + FAKE_LINES="1 edit 2" &&
> + export FAKE_LINES &&
> + set_fake_editor &&
> + git rebase -i HEAD~2 &&
> + git branch --list >actual &&
> + grep "rebasing $oid" actual
> +'
> +
>  test_expect_success 'tracking with unexpected .fetch refspec' '
>   rm -rf a b c d &&
>   git init a &&
> 


-- 
Kaartic



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

2018-03-25 Thread Jacob Keller

On Sat, Mar 24, 2018 at 9:33 PM, Jeff King  wrote:
> IMHO we should do one of:
>
>   1. Nothing. ;)
>
>   2. Complain about "-l" in list mode to help educate users about the
>  current craziness.
>

I think we should do this at a minimum. It's easy, and it doesn't
break any scripts who are doing something sane.

>   3. Drop "-l" (probably with a deprecation period); it seems unlikely
>  to me that anybody uses it for branch creation, and this would at
>  least reduce the confusion (then it would just be "so why don't we
>  have -l" instead of "why is -l not what I expect").

Personally, I'd prefer this, because it's minimal effort on scripts
part to fix themselves to use the long option name for reflog, and
doesn't cause that much heart burn.

>
>   4. Repurpose "-l" as a shortcut for --list (also after a deprecation
>  period). This is slightly more dangerous in that it may confuse
>  people using multiple versions of Git that cross the deprecation
>  line. But that's kind of what the deprecation period is for...
>
> -Peff

I don't think this is particularly all that valuable, since we default
to list mode so it only helps if you want to pass an argument to the
list mode (since otherwise we'd create a branch). Maybe it could be
useful, but if we did it, I'd do it as a sort of double deprecation
period where we use one period to remove the -l functionality
entirely, before adding anything back. I think the *gain* of having -l
is not really worth it though.

Regards,
Jake

Re: query on git submodule (ignore)

2018-03-25 Thread Jacob Keller

On Sat, Mar 24, 2018 at 7:17 PM, prashant Nidgunde
 wrote:
> Hello,
>
> I am new to this community ,so please ignore if I am asking anything silly.
>
> Case :
> Today when I built my submodule , and did a git status , it shows as modified.
>
> After reading certain suggestions on web i found out that i can ignore
> that adding a line in .gitmodules
>
> But, I had to add that line manually ( which could be errorprone
> because of typos )
>
>
> Question:
> 1. Is it feasible to build a feature like :
>git submodule "zlib" ignore dirty ( which will
> ignore submodule zlib when its built and dirty  as it has new files in
> its directory)
>
> If this feature is feasible , how do i know if its developed  (
> awaiting merge ) or can I build the patch ?
>

I don't recall such a feature, but I'm sure patches to implement
something would be welcome to be reviewed! (For more information about
submitting patches you could read
https://github.com/git/git/blob/master/Documentation/SubmittingPatches)

I think having an option to automatically write this would be useful.
It may already be possible to do something similar via the git config
command with the -f file argument to edit the .gitmodules file (as it
uses the gitconfig format for its contents). However, this is
definitely not intuitive.

You can read the documentation for the commands using "git help
submodule" and "git help config". Patches are also definitely welcome
for updates to the documentation if it's not clear.

I know also that having a simpler interface to set submodules up so
that they are treated as unchanged would be useful as I have projects
at $dayjob which use submodules, and this is often a complaint against
their use by my co-workers (who sometimes then accidentally commit
re-wind updates to the submodules due to inattentiveness with use of
git add . or git commit -a).

Thanks,
Jake

Re: [RFC][PATCH] git-stash: convert git stash list to C builtin