[PATCH v5 3/4] format-patch: introduce --base=auto option

[Xiaolong Ye]

Introduce --base=auto to record the base commit info automatically, the
base_commit will be the merge base of tip commit of the upstream branch
and revision-range specified in cmdline.

Helped-by: Junio C Hamano 
Helped-by: Wu Fengguang 
Signed-off-by: Xiaolong Ye 
---
 Documentation/git-format-patch.txt |  6 ++
 builtin/log.c  | 27 ---
 t/t4014-format-patch.sh| 15 +++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-format-patch.txt 
b/Documentation/git-format-patch.txt
index 1d790f1..bdeecd5 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -575,6 +575,12 @@ You can also use `git format-patch --base=P -3 C` to 
generate patches
 for A, B and C, and the identifiers for P, X, Y, Z are appended at the
 end of the first message.
 
+If set `--base=auto` in cmdline, it will track base commit automatically,
+the base commit will be the merge base of tip commit of the remote-tracking
+branch and revision-range specified in cmdline.
+For a local branch, you need to track a remote branch by `git branch
+--set-upstream-to` before using this option.
+
 EXAMPLES
 
 
diff --git a/builtin/log.c b/builtin/log.c
index ee332ab..7851d20 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1205,9 +1205,30 @@ static struct commit *get_base_commit(const char 
*base_commit,
struct commit **rev;
int i = 0, rev_nr = 0;
 
-   base = lookup_commit_reference_by_name(base_commit);
-   if (!base)
-   die(_("Unknown commit %s"), base_commit);
+   if (!strcmp(base_commit, "auto")) {
+   struct branch *curr_branch = branch_get(NULL);
+   const char *upstream = branch_get_upstream(curr_branch, NULL);
+   if (upstream) {
+   unsigned char sha1[20];
+   if (get_sha1(upstream, sha1))
+   die(_("Failed to resolve '%s' as a valid 
ref."), upstream);
+   struct commit *commit = lookup_commit_or_die(sha1, 
"upstream base");
+   struct commit_list *base_list = 
get_merge_bases_many(commit, total, list);
+   /* There should be one and only one merge base. */
+   if (!base_list || base_list->next)
+   die(_("Could not find exact merge base."));
+   base = base_list->item;
+   free_commit_list(base_list);
+   } else {
+   die(_("Failed to get upstream, if you want to record 
base commit automatically,\n"
+ "please use git branch --set-upstream-to to track 
a remote branch.\n"
+ "Or you could specify base commit by 
--base= manually."));
+   }
+   } else {
+   base = lookup_commit_reference_by_name(base_commit);
+   if (!base)
+   die(_("Unknown commit %s"), base_commit);
+   }
 
ALLOC_ARRAY(rev, total);
for (i = 0; i < total; i++)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index a6ce727..afcf8b8 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1475,4 +1475,19 @@ test_expect_success 'format-patch --base error handling' 
'
! git format-patch --base=HEAD~ -3
 '
 
+test_expect_success 'format-patch --base=auto' '
+   git checkout -b new master &&
+   git branch --set-upstream-to=master &&
+   echo "A" >>file &&
+   git add file &&
+   git commit -m "New change #A" &&
+   echo "B" >>file &&
+   git add file &&
+   git commit -m "New change #B" &&
+   git format-patch --stdout --base=auto -2 >patch &&
+   grep -e "^base-commit:" patch >actual &&
+   echo "base-commit: $(git rev-parse master)" >expected &&
+   test_cmp expected actual
+'
+
 test_done
-- 
2.8.1.221.ga4c6ba7

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/4] format-patch: add '--base' option to record base tree info

[Xiaolong Ye]

Maintainers or third party testers may want to know the exact base tree
the patch series applies to. Teach git format-patch a '--base' option
to record the base tree info and append it at the end of the first
message (either the cover letter or the first patch in the series).

The base tree info consists of the "base commit", which is a well-known
commit that is part of the stable part of the project history everybody
else works off of, and zero or more "prerequisite patches", which are
well-known patches in flight that is not yet part of the "base commit"
that need to be applied on top of "base commit" in topological order
before the patches can be applied.

The "base commit" is shown as "base-commit: " followed by the 40-hex of
the commit object name.  A "prerequisite patch" is shown as
"prerequisite-patch-id: " followed by the 40-hex "patch id", which can
be obtained by passing the patch through the "git patch-id --stable"
command.

Imagine that on top of the public commit P, you applied well-known
patches X, Y and Z from somebody else, and then built your three-patch
series A, B, C, the history would be like:

---P---X---Y---Z---A---B---C

With "git format-patch --base=P -3 C" (or variants thereof, e.g. with
"--cover-letter" of using "Z..C" instead of "-3 C" to specify the
range), the base tree information block is shown at the end of the
first message the command outputs (either the first patch, or the
cover letter), like this:

base-commit: P
prerequisite-patch-id: X
prerequisite-patch-id: Y
prerequisite-patch-id: Z

Helped-by: Junio C Hamano 
Helped-by: Wu Fengguang 
Signed-off-by: Xiaolong Ye 
---
 Documentation/git-format-patch.txt |  54 ++
 builtin/log.c  | 139 +
 t/t4014-format-patch.sh|  15 
 3 files changed, 208 insertions(+)

diff --git a/Documentation/git-format-patch.txt 
b/Documentation/git-format-patch.txt
index 6821441..1d790f1 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -265,6 +265,11 @@ you can use `--suffix=-patch` to get 
`0001-description-of-my-change-patch`.
   Output an all-zero hash in each patch's From header instead
   of the hash of the commit.
 
+--base=::
+   Record the base tree information to identify the state the
+   patch series applies to.  See the BASE TREE INFORMATION section
+   below for details.
+
 --root::
Treat the revision argument as a , even if it
is just a single commit (that would normally be treated as a
@@ -520,6 +525,55 @@ This should help you to submit patches inline using KMail.
 5. Back in the compose window: add whatever other text you wish to the
message, complete the addressing and subject fields, and press send.
 
+BASE TREE INFORMATION
+-
+
+The base tree information block is used for maintainers or third party
+testers to know the exact state the patch series applies to. It consists
+of the 'base commit', which is a well-known commit that is part of the
+stable part of the project history everybody else works off of, and zero
+or more 'prerequisite patches', which are well-known patches in flight
+that is not yet part of the 'base commit' that need to be applied on top
+of 'base commit' in topological order before the patches can be applied.
+
+The 'base commit' is shown as "base-commit: " followed by the 40-hex of
+the commit object name.  A 'prerequisite patch' is shown as
+"prerequisite-patch-id: " followed by the 40-hex 'patch id', which can
+be obtained by passing the patch through the `git patch-id --stable`
+command.
+
+Imagine that on top of the public commit P, you applied well-known
+patches X, Y and Z from somebody else, and then built your three-patch
+series A, B, C, the history would be like:
+
+
+---P---X---Y---Z---A---B---C
+
+
+With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
+`--cover-letter` of using `Z..C` instead of `-3 C` to specify the
+range), the base tree information block is shown at the end of the
+first message the command outputs (either the first patch, or the
+cover letter), like this:
+
+
+base-commit: P
+prerequisite-patch-id: X
+prerequisite-patch-id: Y
+prerequisite-patch-id: Z
+
+
+For non-linear topology, such as
+
+
+---P---X---A---M---C
+\ /
+ Y---Z---B
+
+
+You can also use `git format-patch --base=P -3 C` to generate patches
+for A, B and C, and the identifiers for P, X, Y, Z are appended at the
+end of the first message.
 
 EXAMPLES
 
diff --git a/builtin/log.c b/builtin/log.c
index dff3fbb..ee332ab 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1191,6 +1191,131 @@ static int from_callback(const 

[PATCH v5 4/4] format-patch: introduce format.useAutoBase configuration

[Xiaolong Ye]

This allows to record the base commit automatically, it is equivalent
to set --base=auto in cmdline.

The format.useAutoBase has lower priority than command line option,
so if user set format.useAutoBase and pass the command line option in
the meantime, base_commit will be the one passed to command line
option.

Signed-off-by: Xiaolong Ye 
---
 Documentation/config.txt |  5 +
 builtin/log.c| 17 +++--
 t/t4014-format-patch.sh  | 18 ++
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..1fe2a85 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1259,6 +1259,11 @@ format.outputDirectory::
Set a custom directory to store the resulting files instead of the
current working directory.
 
+format.useAutoBase::
+   A boolean value which lets you enable the `--base=auto` option of
+   format-patch by default.
+
+
 filter..clean::
The command which is used to convert the content of a worktree
file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/builtin/log.c b/builtin/log.c
index 7851d20..c3aeef8 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -702,6 +702,7 @@ static void add_header(const char *value)
 #define THREAD_DEEP 2
 static int thread;
 static int do_signoff;
+static int base_auto;
 static const char *signature = git_version_string;
 static const char *signature_file;
 static int config_cover_letter;
@@ -786,6 +787,10 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
}
if (!strcmp(var, "format.outputdirectory"))
return git_config_string(_output_directory, var, value);
+   if (!strcmp(var, "format.useautobase")) {
+   base_auto = git_config_bool(var, value);
+   return 0;
+   }
 
return git_log_config(var, value, cb);
 }
@@ -1205,7 +1210,11 @@ static struct commit *get_base_commit(const char 
*base_commit,
struct commit **rev;
int i = 0, rev_nr = 0;
 
-   if (!strcmp(base_commit, "auto")) {
+   if (base_commit && strcmp(base_commit, "auto")) {
+   base = lookup_commit_reference_by_name(base_commit);
+   if (!base)
+   die(_("Unknown commit %s"), base_commit);
+   } else if ((base_commit && !strcmp(base_commit, "auto")) || base_auto) {
struct branch *curr_branch = branch_get(NULL);
const char *upstream = branch_get_upstream(curr_branch, NULL);
if (upstream) {
@@ -1224,10 +1233,6 @@ static struct commit *get_base_commit(const char 
*base_commit,
  "please use git branch --set-upstream-to to track 
a remote branch.\n"
  "Or you could specify base commit by 
--base= manually."));
}
-   } else {
-   base = lookup_commit_reference_by_name(base_commit);
-   if (!base)
-   die(_("Unknown commit %s"), base_commit);
}
 
ALLOC_ARRAY(rev, total);
@@ -1666,7 +1671,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
}
 
memset(, 0, sizeof(bases));
-   if (base_commit) {
+   if (base_commit || base_auto) {
struct commit *base = get_base_commit(base_commit, list, nr);
reset_revision_walk();
prepare_bases(, base, list, nr);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index afcf8b8..dfee0b6 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1490,4 +1490,22 @@ test_expect_success 'format-patch --base=auto' '
test_cmp expected actual
 '
 
+test_expect_success 'format-patch format.base option' '
+   test_when_finished "git config --unset format.useAutoBase" &&
+   git config format.useAutoBase true &&
+   git format-patch --stdout -1 >patch &&
+   grep -e "^base-commit:" patch >actual &&
+   echo "base-commit: $(git rev-parse master)" >expected &&
+   test_cmp expected actual
+'
+
+test_expect_success 'format-patch --base overrides format.base' '
+   test_when_finished "git config --unset format.useAutoBase" &&
+   git config format.useAutoBase true &&
+   git format-patch --stdout --base=HEAD~ -1 >patch &&
+   grep -e "^base-commit:" patch >actual &&
+   echo "base-commit: $(git rev-parse HEAD~)" >expected &&
+   test_cmp expected actual
+'
+
 test_done
-- 
2.8.1.221.ga4c6ba7

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/4] patch-ids: make commit_patch_id() a public helper function

[Xiaolong Ye]

Make commit_patch_id() available to other builtins.

Helped-by: Junio C Hamano 
Signed-off-by: Xiaolong Ye 
---
 patch-ids.c | 2 +-
 patch-ids.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/patch-ids.c b/patch-ids.c
index b7b3e5a..a4d0016 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -4,7 +4,7 @@
 #include "sha1-lookup.h"
 #include "patch-ids.h"
 
-static int commit_patch_id(struct commit *commit, struct diff_options *options,
+int commit_patch_id(struct commit *commit, struct diff_options *options,
unsigned char *sha1)
 {
if (commit->parents)
diff --git a/patch-ids.h b/patch-ids.h
index c8c7ca1..eeb56b3 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -13,6 +13,8 @@ struct patch_ids {
struct patch_id_bucket *patches;
 };
 
+int commit_patch_id(struct commit *commit, struct diff_options *options,
+   unsigned char *sha1);
 int init_patch_ids(struct patch_ids *);
 int free_patch_ids(struct patch_ids *);
 struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
-- 
2.8.1.221.ga4c6ba7

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 0/4] Add --base option to git-format-patch to record base tree info

[Xiaolong Ye]

Thanks for Junio's reviews and suggestions.

This version contains the following changes since v4:

 - Refine the commit log as well as the documentation according to
   Junio's comments.

 - Separate out get_base_commit function from prepare_bases to obtain
   the base commit.

 - Use repeated pair-wise computation to get the merge base for the
   validation of base commit.

 - Extract "auto handling thing" from prepare_bases and put it into
   get_base_commit.

 - Use format.useAutoBase boolean variable for the auto configuration
   in format section.


Thanks,
Xiaolong.

Xiaolong Ye (4):
  patch-ids: make commit_patch_id() a public helper function
  format-patch: add '--base' option to record base tree info
  format-patch: introduce --base=auto option
  format-patch: introduce format.useAutoBase configuration

 Documentation/config.txt   |   5 ++
 Documentation/git-format-patch.txt |  60 ++
 builtin/log.c  | 165 +
 patch-ids.c|   2 +-
 patch-ids.h|   2 +
 t/t4014-format-patch.sh|  48 +++
 6 files changed, 281 insertions(+), 1 deletion(-)

-- 
2.8.1.221.ga4c6ba7

base-commit: e6ac6e1f7d54584c2b03f073b5f329a37f4a9561
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git status core dump with bad sector!

[Jeff King]

On Thu, Apr 14, 2016 at 10:59:57AM -0400, Eric Chamberland wrote:

> just cloned a repo and it checked-out wihtout any error (with git 2.2.0) but
> got come corrupted files (because I got some sdd failures).
> 
> Then, I get a git core dump when trying to "git status" into the repo which
> have a "bad sector" on sdd drive (crypted partition).
> 
> I tried with git 2.2.0 AND git version 2.8.1.185.gdc0db2c.dirty (just
> modified the Makefile to remove STRIP part)
> 
> In both cases, I have a  Bus error (core dumped)

Interesting. There was a known issue with reading corrupted pack .idx
files, but it was fixed in v2.8.0. So this could be a new thing.

SIGBUS is somewhat rare, though (usually just accessing unmapped memory
should get us a SIGSEGV). What platform are you on? I seem to recall
that hardware like ARM that cares about memory alignment is more likely
to get a SIGBUS.

> Program received signal SIGBUS, Bus error.
> 0x77866d58 in ?? () from /lib64/libcrypto.so.1.0.0
> (gdb) bt
> #0  0x77866d58 in ?? () from /lib64/libcrypto.so.1.0.0
> #1  0x3334d90d8c20f3f0 in ?? ()
> #2  0xe59b5a6cd844a601 in ?? ()
> #3  0xc587a53f67985ae7 in ?? ()
> #4  0x3ce81893e5541777 in ?? ()
> #5  0xdeb18349a4b042ea in ?? ()
> #6  0x8254de489067ec4b in ?? ()
> #7  0x6fbef2439704c81b in ?? ()
> #8  0xe0eee2bb385a96da in ?? ()
> #9  0x76e19ab3 in ?? ()
> #10 0x7fffc4d0 in ?? ()
> #11 0x001d in ?? ()
> #12 0x77863f80 in SHA1_Update () from /lib64/libcrypto.so.1.0.0
> #13 0x005102c0 in write_sha1_file_prepare
> (buf=buf@entry=0x76c81000, len=1673936, type=,
> sha1=sha1@entry=0x7fffc750 "\340_~", hdr=hdr@entry=0x7fffc570 "blob
> 1673936",

So I'd assume here that the problem is in accessing the memory in "buf".
to actually compute the sha1. That is mmap'd data, but the process is
fairly bland (mmap however many bytes stat() tells us the file has, and
then compute the sha1). You mentioned a bad sector; could it be that the
filesystem is corrupted, and the OS is giving us SIGBUS when trying to
read unavailable bytes from an mmap'd file?

That would explain the SIGBUS versus SIGSEGV.

What happens if you "cat" the file in question:

> #15 0x005159f8 in index_mem (sha1=sha1@entry=0x7fffc750
> "\340_~", buf=buf@entry=0x76c81000, size=1673936,
> type=type@entry=OBJ_BLOB,
> path=path@entry=0x80a818 
> "Ressources/dev/Test.ExportationVTK/Ressources.Avion/Avion.Quadratique.cont.vtu.etalon",
> flags=flags@entry=0) at sha1_file.c:3305

Can it show all of the bytes? I guess from the "size" field it's too big
to manually verify, but "cat >/dev/null" should be enough to see if we
can read the whole thing.

> Ii would have expected git to first gave me an error when checking out the
> files!!! Here is the log:
> 
> Checking out files:  99% (28645/28934)
> Checking out files: 100% (28934/28934)
> Checking out files: 100% (28934/28934), done.
> Already on 'master'
> Your branch is up-to-date with 'origin/master'.
> On valide le dépôt TestValidation avec la référence:
> 9b4a485202b2b52922377842c15bfd605d240667
> HEAD is now at 9b4a485 BUG: On spécifie bash comme shell...
> 
> But at least 1 file is corrupted!
> 
> I keep preciously this faulty repo to further investigation with someone who
> can help dig into the coredump and correct it...

So _if_ my guess is right that you have filesystem corruption, git may
not even know about it. It wrote the file, and the OS said "OK,
success", not knowing it had been partially corrupted.

And if that guess is right, it also means there's no git bug to fix.
SIGBUS is the natural way for the OS to tell the process that mmap'd
data isn't available.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

[Torsten Bögershausen]

* tb/convert-eol-autocrlf (2016-04-19) 4 commits
 - convert.c: ident + core.autocrlf didn't work
 - t0027: test cases for combined attributes
 - convert: allow core.autocrlf=input and core.eol=crlf
 - t0027: avoid false "unchanged" due to lstat() matching after a change

 Setting core.autocrlf to 'input' and core.eol to 'crlf' used to be
 rejected, but because the code gives precedence to core.autcrlf,
 there is no need to, hence we no longer reject the combination.

 Will merge to 'next'.
I know that I asked for an early merge of 4/4, but there is a new version
with a fix for the leaking filter coming out this evening, european time.
Please hold it.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

[Jeff King]

On Thu, Apr 21, 2016 at 03:20:39PM -0700, Junio C Hamano wrote:

> * jk/push-client-deadlock-fix (2016-04-20) 5 commits
>  - t5504: drop sigpipe=ok from push tests
>  - fetch-pack: isolate sigpipe in demuxer thread
>  - send-pack: isolate sigpipe in demuxer thread
>  - run-command: teach async threads to ignore SIGPIPE
>  - send-pack: close demux pipe before finishing async process
> 
>  "git push" from a corrupt repository that attempts to push a large
>  number of refs deadlocked waiting for a rejection from the
>  receiving end that will never come.
> 
>  Will merge to 'next'.

Minor nit, but the deadlock is the other way around: the rejection
showed up and our demuxer is blocked writing to a reader who does not
care about it.

Might be worth fixing since this text goes into the topic merge commit
(though I really hope nobody ever has to debug it enough ever again for
that distinction to matter :) ).

> * da/user-useconfigonly (2016-04-01) 2 commits
>  - ident: give "please tell me" message upon useConfigOnly error
>  - ident: check for useConfigOnly before auto-detection of name/email
> 
>  The "user.useConfigOnly" configuration variable makes it an error
>  if users do not explicitly set user.name and user.email.  However,
>  its check was not done early enough and allowed another error to
>  trigger, reporting that the default value we guessed from the
>  system setting was unusable.  This was a suboptimal end-user
>  experience as we want the users to set user.name/user.email without
>  relying on the auto-detection at all.
> 
>  Waiting for Acks.
>  ($gmane/290340)

I think you are waiting for the Ack from the original author on your
tweaks. But FWIW, what you have queued looks good to me.

> * dk/gc-more-wo-pack (2016-01-13) 4 commits
>  - gc: clean garbage .bitmap files from pack dir
>  - t5304: ensure non-garbage files are not deleted
>  - t5304: test .bitmap garbage files
>  - prepare_packed_git(): find more garbage
> 
>  Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
>  .bitmap and .keep files.
> 
>  Waiting for a reroll.
>  ($gmane/284368).

This one's getting pretty stale, but as I recall was pretty close to
done.  I'll try to give it a look in the next couple of days.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

[Junio C Hamano]

Santiago Torres  writes:

> On Thu, Apr 21, 2016 at 03:20:39PM -0700, Junio C Hamano wrote:
>> * st/verify-tag (2016-04-19) 6 commits
>>  - tag -v: verfy directly rather than exec-ing verify-tag
>>  - verify-tag: move tag verification code to tag.c
>>  - verify-tag: prepare verify_tag for libification
>>  - verify-tag: update variable name and type
>>  - t7030: test verifying multiple tags
>>  - builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
>> 
>>  Unify internal logic between "git tag -v" and "git verify-tag"
>>  commands by making one directly call into the other.
>> 
>>  Will merge to 'next'.
>
> Hi Junio, 
>
> Ramsay Jones[1] Suggested we dropped the extern qualifier on the
> declaration of verify-tag() in tag.c as it is causing a warning with
> sparse.
>
> Should I re-roll this before it's merged into next?

Yes please.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 'next'ed --allow-unrelated-histories could cause lots of grief

[Junio C Hamano]

Joey Hess  writes:

> Junio C Hamano wrote:
>>   merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment
>
> I hope this patch lands, it will save me a lot of bother.

Yup, it is somewhat ugly but the least bad option among command line
option (which would break with older versions of Git), configuration
variables (which would encourage users to opt out of safety
unconditionallly) and environment (which allows scripts to opt-in),
I would say.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 'next'ed --allow-unrelated-histories could cause lots of grief

[Joey Hess]

Yaroslav Halchenko wrote:
> - for git-annex (Joey was CCed from the beginning, not sure if annex
>   would be affected though), it would be merging of git-annex
>   branches while joining multiple annexes for the sync (e.g. by git
>   annex assistant). 

Not entirely accurate (git-annex merges its git-annex branches using a
custom merge method and not involving git-merge). The actual use case is
two users (or one user with two devices) each with a git-annex
repository who decide to share their files by combining the two
repositories. This is pretty far from the kernel world, so it's not like
bisection is something they care about.

However, I also see --allow-unrelated-histories as very useful to
prevent many foot-shooting maneuvers. Especially when a repository has
special-purpose branches, like git-annex's git-annex branch, or other
branches that are never intended to be merged into master. It's a not
entirely uncommon mistake for users to merge in such a branch, and the
users who make such a mistake often don't know enough git to easily
recover from it.

Junio C Hamano wrote:
>   merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment

I hope this patch lands, it will save me a lot of bother.

-- 
see shy jo


signature.asc
Description: PGP signature

Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

[Santiago Torres]

On Thu, Apr 21, 2016 at 03:20:39PM -0700, Junio C Hamano wrote:
> * st/verify-tag (2016-04-19) 6 commits
>  - tag -v: verfy directly rather than exec-ing verify-tag
>  - verify-tag: move tag verification code to tag.c
>  - verify-tag: prepare verify_tag for libification
>  - verify-tag: update variable name and type
>  - t7030: test verifying multiple tags
>  - builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
> 
>  Unify internal logic between "git tag -v" and "git verify-tag"
>  commands by making one directly call into the other.
> 
>  Will merge to 'next'.

Hi Junio, 

Ramsay Jones[1] Suggested we dropped the extern qualifier on the
declaration of verify-tag() in tag.c as it is causing a warning with
sparse.

Should I re-roll this before it's merged into next?

Thanks!
-Santiago.

[1] http://thread.gmane.org/gmane.comp.version-control.git/292029
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

[Pranit Bauva]

On Fri, Apr 22, 2016 at 3:50 AM, Junio C Hamano  wrote:

> [Cooking]
>
> * pb/commit-verbose-config (2016-04-19) 6 commits
>  - commit: add a commit.verbose config variable
>  - t7507-commit-verbose: improve test coverage by testing number of diffs
>  - parse-options.c: make OPTION_COUNTUP respect "unspecified" values
>  - t0040-parse-options: improve test coverage
>  - test-parse-options: print quiet as integer
>  - t0040-test-parse-options.sh: fix style issues
>
>  "git commit" learned to pay attention to "commit.verbose"
>  configuration variable and act as if "--verbose" option was
>  given from the command line.
>
>  Is this going to be rerolled?
>  ($gmane/291382)

The changes weren't that big enough and I had my end semester exams
coming so I decided not to re-roll it.
If you think contrary, I can do a re-roll with the changes suggested
by Eric Sunshine.

Regards,
Pranit Bauva
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: problems serving non-bare repos with submodules over http

[Jacob Keller]

On Thu, Apr 21, 2016 at 10:48 AM, Stefan Beller  wrote:
> On Thu, Apr 21, 2016 at 10:45 AM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
>>> In case of non bare:
>>>
>>> Get the repo and all its submodules from the specified remote.
>>> (As the submodule is right there, that's the best guess to get it from,
>>> no need to get it from somewhere else. The submodule at the remote
>>> is the closest match you can get for replicating the superproject with
>>> its submodules.)
>>>
>>> This way is heavy underutilized as it wasn't exercised as often I would
>>> guess,
>>
>> My guess is somewhat different. It is not used because the right
>> semantics for such a use case hasn't been defined yet (in other
>> words, what you suggested is _wrong_ as is).  You need to remember
>> that a particular clone may not be interested in all submodules, and
>> it is far from "the closest match".
>
> Yes, when that clone doesn't have some submodules, we can still fall back
> on the .gitmodules file.
>
> If you have a submodule chances are, you are interested in it and modified it.
> So the highest chance to get your changes is from your remote, no?
> --

I agree with Stefan. I think that if I clone from my local non-bare
repository that may have work done inside the submodule it would be
best if the clone could grab the submodules directly from here and get
this work which might not yet be in the "real" remote yet.

The case could be made that you don't want to do this, I suppose..
Generally I think since we're already connected to this remote we know
we can access it, and getting submodules from here means we know it
will work, and give us the actual sha1 that the clone is using.

If we use .gitmodules, we'll possibly get a module that doesn't have
the commit, and the current gitmodules url might not even work
anymore.

That is, I don't really understand any downside to Stefan's
proposal,and I see a bunch of upside.

Thanks,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

[Stefan Beller]

>
> * sb/clone-shallow-passthru (2016-04-13) 3 commits
>  - clone: add t5614 to test cloning submodules with shallowness involved
>  - clone: add `--shallow-submodules` flag
>  - submodule clone: pass along `local` option
>
>  "git clone" learned "--shallow-submodules" option.
>
>  Needs review.
>
>

Lars, as the original author of [1]:

[PATCH v3 0/3] add test to demonstrate that shallow recursive clones fail

do you have an opinion about this series one way or another?

Thanks,
Stefan

[1] http://thread.gmane.org/gmane.comp.version-control.git/282779
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Loosening "two project merge" safety

[Junio C Hamano]

Junio C Hamano  writes:

> Yaroslav Halchenko gave a vague "forcing 'git merge' users to always
> give --allow-unrelated-histories option when they create crap/insane
> merges are not nice", which I couldn't guess the validity due to
> lack of concrete use case.  Just in case it is substantiated, here
> is a series to selectively and safely loosen the safety for specific
> use cases and users.
>
> Junio C Hamano (4):
>   t3033: avoid 'ambiguous refs' warning
>   pull: pass --allow-unrelated-histories to "git merge"
>   merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment
>   merge: introduce merge.allowUnrelatedhistories configuration option

I've queued the first two on 'pu'.

I do not think the Kernel folks do not mind the latter two too much,
but I am holding onto them for now.  Unless the original "Gaah" did
not come from Linus, I might even have said that this additional
safety should be opt-in for people who know what they are doing
(i.e. those who want the safety would set the new configuration),
but I am undecided right now.

>
>  Documentation/git-merge.txt | 14 +-
>  Documentation/git.txt   |  7 +++
>  Documentation/merge-config.txt  |  7 +++
>  Documentation/merge-options.txt |  8 
>  builtin/merge.c |  6 ++
>  builtin/pull.c  | 11 +++
>  t/t3033-merge-toplevel.sh   | 31 ++-
>  t/t5521-pull-options.sh | 28 
>  8 files changed, 98 insertions(+), 14 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

What's cooking in git.git (Apr 2016, #06; Thu, 21)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

The 'master' branch now has the fifth batch of topics of this cycle.

There are a handful of topics that are stuck; they are marked as
"Needs review", "Needs an Ack", etc. in the following list.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* jd/p4-jobs-in-commit (2016-04-19) 2 commits
 - git-p4: add P4 jobs to git commit message
 - git-p4: clean-up code style in tests

 "git p4" learned to record P4 jobs in Git commit that imports from
 the history in Perforce.

 Will merge to 'next'.


* js/replace-edit-use-editor-configuration (2016-04-20) 1 commit
 - replace --edit: respect core.editor

 "git replace -e" did not honour "core.editor" configuration.

 Will merge to 'next'.


* ls/p4-lfs (2016-04-19) 2 commits
 - git-p4: fix Git LFS pointer parsing
 - travis-ci: update Git-LFS and P4 to the latest version

 Recent update to Git LFS broke "git p4" by changing the output from
 its "lfs pointer" subcommand.


* sb/mv-submodule-fix (2016-04-19) 1 commit
 - mv: allow moving nested submodules

 "git mv old new" did not adjust the path for a submodule that lives
 as a subdirectory inside old/ directory correctly.

 Will merge to 'next'.


* tb/convert-eol-autocrlf (2016-04-19) 4 commits
 - convert.c: ident + core.autocrlf didn't work
 - t0027: test cases for combined attributes
 - convert: allow core.autocrlf=input and core.eol=crlf
 - t0027: avoid false "unchanged" due to lstat() matching after a change

 Setting core.autocrlf to 'input' and core.eol to 'crlf' used to be
 rejected, but because the code gives precedence to core.autcrlf,
 there is no need to, hence we no longer reject the combination.

 Will merge to 'next'.


* bc/object-id (2016-04-19) 6 commits
 - match-trees: convert several leaf functions to use struct object_id
 - tree-walk: convert tree_entry_extract() to use struct object_id
 - struct name_entry: use struct object_id instead of unsigned char sha1[20]
 - match-trees: convert shift_tree() and shift_tree_by() to use object_id
 - test-match-trees: convert to use struct object_id
 - sha1-name: introduce a get_oid() function

 Will be rerolled.
 ($gmane/291950)


* ep/http-curl-trace (2016-04-20) 3 commits
 - git.txt: document the new GIT_TRACE_CURL environment variable
 - imap-send.c: introduce the GIT_TRACE_CURL enviroment variable
 - http.c: implement the GIT_TRACE_CURL environment variable

 HTTP transport gained an option to produce more detailed debugging
 trace.

 Still under discussion.
 ($gmane/292074)


* nd/worktree-various-heads (2016-04-20) 13 commits
 - branch: do not rename a branch under bisect or rebase
 - worktree.c: test if branch being bisected in another worktree
 - wt-status.c: split bisect detection out of wt_status_get_state()
 - worktree.c: test if branch being rebased in another worktree
 - worktree.c: avoid referencing to worktrees[i] multiple times
 - wt-status.c: make wt_status_check_rebase() work on any worktree
 - SQUASH???
 - wt-status.c: split rebase detection out of wt_status_get_state()
 - path.c: refactor and add worktree_git_path()
 - worktree.c: mark current worktree
 - worktree.c: make find_shared_symref() return struct worktree *
 - worktree.c: store "id" instead of "git_dir"
 - path.c: add git_common_path() and strbuf_git_common_path()

 The experimental "multiple worktree" feature gains more safety to
 forbid operations on a branch that is checked out or being actively
 worked on elsewhere, by noticing that e.g. it is being rebased.

 Being reviewed.
 ($gmane/292050)


* bw/rebase-merge-entire-branch (2016-04-20) 1 commit
 - git-rebase--merge: don't include absent parent as a base

 "git rebase -m" could be asked to rebase an entire branch starting
 from the root, but failed by assuming that there always is a parent
 commit to the first commit on the branch.

 Will merge to 'next'.


* jk/push-client-deadlock-fix (2016-04-20) 5 commits
 - t5504: drop sigpipe=ok from push tests
 - fetch-pack: isolate sigpipe in demuxer thread
 - send-pack: isolate sigpipe in demuxer thread
 - run-command: teach async threads to ignore SIGPIPE
 - send-pack: close demux pipe before finishing async process

 "git push" from a corrupt repository that attempts to push a large
 number of refs deadlocked waiting for a rejection from the
 receiving end that will never come.

 Will merge to 'next'.


* jc/merge-refuse-new-root (2016-04-21) 2 commits
 - pull: pass --allow-unrelated-histories to "git merge"
 - t3033: avoid 'ambiguous refs' warning

 "git pull" has been taught to pass --allow-unrelated-histories
 option to underlying "git 

Re: history damage in linux.git

[Junio C Hamano]

Stefan Beller  writes:

> Combining Junios and Linus idea:
>
> * We want to have the minimal history, i.e. that tag with the fewest
> cummulative parent commits. (i.e. v3.13-rc7 is better than v3.13
> because `git log --oneline v3.13-rc7 |wc -l` (414317) is smaller tha
> `git log --oneline v3.13 |wc -l` (414530).
> The difference is 213.
>
> tags/v3.13-rc7~9^2~14^2~42 has 9 + 14 + 42 additional steps (65)
>
> tags/v3.13~5^2~4^2~2^2~1^2~42 has 5 + 4 + 2 + 1 +42 steps (54)
>
> tags/v3.13~5^2~4^2~2^2~1^2~42 has 9 less steps, but its base tag
> has a higher weight by 213.
>
> v4.6-rc1 has even more weight (588477).
>
> So I guess what I propose is to take the weight of a tag into account
> via `git log --oneline  |wc -l` as that gives the tag which encloses
> least history?
>
> We also do not want to have "a lot of side traversals", so we could
> punish each additional addendum by a heuristic.

These are essentially shooting for what Linus meant "something
optimal", contrasting his "improved heuristics" version, and it is
good that you are thinking about these issues.

It may be a bit tricky to think the "optimum description" in terms
of "how many commits are behind these tags", though.  One thing that
commonly happens is:

 (1) A fix X is developed on an ancient codebase, e.g. on top of
 v1.0.0 release.

 (2) X is first merged to the current 'master' and becomes part of
 the next release v2.0.0.

 (3) X is later merged to the maintenance track and included in
 v1.0.1.

There are a few questions you would want to ask "describe --contains
X" and depending on the reason why you are asking the question, the
desired answer may be different:

 - In which release did the fix X first appear?  (answer: v2.0.0)
 - What is the oldest release with the fix X?(answer: v1.0.1)

I happen to be a maintainer who prefers to have a tidy containment
relationships among his releases, and after the above three steps,
there will be this:

 (4) Later v1.0.1 is merged back to 'master' branch and a tag v2.0.1
 on the maintenance track for v2.0.x series would contain it.

But not all projects are run that way.  Often older maintenance
tracks will never merge back to the current development track
(i.e. fixes are only ported-back).

If the v1.0.x codebase had an unrelated problem in the code that no
longer exists in v2.0.x codebase, the maintenance track may have
quite a many commits that exist only there and not in 'master', and
when (3) happens, the "weight", i.e. the commits behind the tag, of
v1.0.1 may be greater than the weight of v2.0.0 in such a project.

In other words, an algorithm that uses "how many commits are behind
the tag" as one of the deciding factor to name X would choose
between v1.0.1 and v2.0.0 depending on what other things that have
nothing to do with X happend on these parallel development tracks,
which may not be desirable.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[git-multimail] smtplib, check certificate

[Simon P]

Hi,

It seems that smtplib doesn't check if a certificate is valid (signed by
a trusted CA).

For my personal usage, I patched the starttls code in git-multimail:
only for starttls with smtplib.

This patch is inspired from

https://github.com/graingert/secure-smtplib/blob/master/src/secure_smtplib/__init__.py

It could be easy to add support cert check in for smtps (see
secure_smtplib).

This patch was tested only on git-multimail (v1.2)

It introduces two new options:
  - multimailhook.smtpcheckcert (default false)
  - multimailhook.smtpcacerts (default
   /etc/ssl/certs/ca-certificates.crt)

Best regards,
Simon P.
diff --git a/git-multimail/git_multimail.py b/git-multimail/git_multimail.py
index fae5c91..b49ed9d 100755
--- a/git-multimail/git_multimail.py
+++ b/git-multimail/git_multimail.py
@@ -57,6 +57,7 @@ import subprocess
 import shlex
 import optparse
 import smtplib
+import ssl
 import time
 import cgi
 
@@ -1945,6 +1946,7 @@ class SMTPMailer(Mailer):
  smtpservertimeout=10.0, smtpserverdebuglevel=0,
  smtpencryption='none',
  smtpuser='', smtppass='',
+ smtpcacerts='/etc/ssl/certs/ca-certificates.crt',smtpcheckcert=False
  ):
 if not envelopesender:
 sys.stderr.write(
@@ -1974,13 +1976,43 @@ class SMTPMailer(Mailer):
 if self.security == 'none':
 self.smtp = call(smtplib.SMTP, self.smtpserver, timeout=self.smtpservertimeout)
 elif self.security == 'ssl':
+if smtpcheckcert:
+msg = "Checking certificate is not supported for ssl, prefer starttls"
+raise smtplib.SMTPException(msg)
 self.smtp = call(smtplib.SMTP_SSL, self.smtpserver, timeout=self.smtpservertimeout)
 elif self.security == 'tls':
 if ':' not in self.smtpserver:
 self.smtpserver += ':587'  # default port for TLS
 self.smtp = call(smtplib.SMTP, self.smtpserver, timeout=self.smtpservertimeout)
-self.smtp.ehlo()
-self.smtp.starttls()
+if smtpcheckcert:
+# inspired form:
+#   https://github.com/graingert/secure-smtplib/blob/master/src/secure_smtplib/__init__.py
+# but add the path to trusted ca, and force ceritficate verification.
+self.smtp.ehlo_or_helo_if_needed()
+if not self.smtp.has_extn("starttls"):
+msg = "STARTTLS extension not supported by server"
+raise smtplib.SMTPException(msg)
+(resp, reply) = self.smtp.docmd("STARTTLS")
+if resp == 220:
+self.smtp.sock = ssl.wrap_socket(
+self.smtp.sock,
+ca_certs=smtpcacerts,
+cert_reqs=ssl.CERT_REQUIRED
+)
+if not hasattr(self.smtp.sock, "read"):
+# using httplib.FakeSocket with Python 2.5.x or earlier
+self.smtp.sock.read = self.smtp.sock.recv
+self.smtp.file = smtplib.SSLFakeFile(self.smtp.sock)
+self.smtp.helo_resp = None
+self.smtp.ehlo_resp = None
+self.smtp.esmtp_features = {}
+self.smtp.does_esmtp = 0
+else:
+msg = "Wrong answer to the STARTTLS command"
+raise smtplib.SMTPException(msg)
+else:
+self.smtp.ehlo()
+self.smtp.starttls()
 self.smtp.ehlo()
 else:
 sys.stdout.write('*** Error: Control reached an invalid option. ***')
@@ -3500,6 +3532,8 @@ def choose_mailer(config, environment):
 smtpencryption = config.get('smtpencryption', default='none')
 smtpuser = config.get('smtpuser', default='')
 smtppass = config.get('smtppass', default='')
+smtpcacerts = config.get('smtpcacerts', default='/etc/ssl/certs/ca-certificates.crt')
+smtpcheckcert = config.get_bool('smtpcheckcert', default='false')
 mailer = SMTPMailer(
 envelopesender=(environment.get_sender() or environment.get_fromaddr()),
 smtpserver=smtpserver, smtpservertimeout=smtpservertimeout,
@@ -3507,6 +3541,8 @@ def choose_mailer(config, environment):
 smtpencryption=smtpencryption,
 smtpuser=smtpuser,
 smtppass=smtppass,
+smtpcacerts=smtpcacerts,
+smtpcheckcert=smtpcheckcert
 )
 elif mailer == 'sendmail':
 command = config.get('sendmailcommand')

Re: [PATCH 2/4] pull: pass --allow-unrelated-histories to "git merge"

[Junio C Hamano]

Stefan Beller  writes:

> On Thu, Apr 21, 2016 at 12:24 PM, Junio C Hamano  wrote:
>> An earlier commit said:
>
> And by earlier you meant to say e379fdf34f (2016-03-18, merge: refuse
> to create too cool a merge by default)?

The text quoted does come from that commit, but because there is
nothing more that a reader would need to read from that commit to
follow and understand this change, including its log message with
the quoted parts, I did not give a reference to the commit, in order
to avoid wasting readers' time.  In other words, yes I meant that
commit, and no, I didn't mean to "say" e379fdf  I meant to say
"An earlier commit" and taht is what I did.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 'next'ed --allow-unrelated-histories could cause lots of grief

[Yaroslav Halchenko]

On Thu, 21 Apr 2016, Junio C Hamano wrote:
> >> It is not very productive to make such an emotional statement
> >> without substantiating _why_ a merge that adds a new root, which was
> >> declared in the thread above as "crap" (in the context of the kernel
> >> project),

> > Sorry if my statement sounded too emotional ;)  I will outline some of
> > the use-cases below.

> Thanks.  Emotional is fine, as long as you _also_ have useful
> information.

Gotcha:  I will follow "emotional + useful == fine" advice closer
next time ;)

Thank you a lot for the suggested patch with the env variable
workaround!
-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Linus Torvalds]

On Thu, Apr 21, 2016 at 12:27 PM, Junio C Hamano  wrote:
> Linus Torvalds  writes:
>
>> But this patch is small and simple, and has some excuses for its
>> behavior. What do people think?
>
> I like it that you call it "excuse" not "rationale", as I couldn't
> form a logical connection between your "4 (2) letters" and "1
> (100)" at all ;-)

Think of the distance number as a "order of magnitude in complexity",
and it actually makes a certain amount of sense.

It's not the same as the length of the string, but the "log()" of the
distance number really does give a kind of complexity value.

Think of it this way: if things are entirely linear (all just first
parenthood), there will be just a single simple number, and the
relationship between the simple distance number (that just increments
by one for each parent traversed) and the length of the string that
describes it will really be "log10(distance)". That's literally how
many characters you need to describe the linear distance number.

So a simple linear distance of 'n' commits will need on the order of
'log10(n)' digits to describe it (ie a number around a thousand will
need around three digits).

The "100" and "1" are just extending that notion of distance to
the more complex cases., and expresses their complexity in the same
logarithmic units. The same way you need four digits to express a
_linear_ distance of 1, you need four characters to express that
"~n^p" case of "merge parent p, n generations back".

And if you don't have the generation thing, you only need two
characters to express parent #'p': "^p".

So two characters really *are* equivalent to ~100 linear steps, and
four characters really *are* equivalent to ~1 linear steps.

So it's not _just_ an excuse. There's an actual rationale for picking
those numbers, and why they are equivalent in a complexity measure.

 Linus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 'next'ed --allow-unrelated-histories could cause lots of grief

[Junio C Hamano]

Yaroslav Halchenko  writes:

>> It is not very productive to make such an emotional statement
>> without substantiating _why_ a merge that adds a new root, which was
>> declared in the thread above as "crap" (in the context of the kernel
>> project),
>
> Sorry if my statement sounded too emotional ;)  I will outline some of
> the use-cases below.

Thanks.  Emotional is fine, as long as you _also_ have useful
information.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] pull: pass --allow-unrelated-histories to "git merge"

[Stefan Beller]

On Thu, Apr 21, 2016 at 12:24 PM, Junio C Hamano  wrote:
> An earlier commit said:

And by earlier you meant to say e379fdf34f (2016-03-18, merge: refuse
to create too cool a merge by default)?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment

[Junio C Hamano]

It is rumored that some scripts rely on being able to regularly
create two project merges.  Instead of forcing them to pass the
option --allow-unrelated-histories when they call "git merge", allow
them to set and export an environment at the beginning and forget
about it.

This will be less damaging than adding a configuration variable to
disable the safety, as contaminating the configuration file of users
of such a script will allow any invocation of "git merge", not
limited to such a script, to go without the safety.

Signed-off-by: Junio C Hamano 
---
 Documentation/git.txt | 3 +++
 builtin/merge.c   | 3 +++
 builtin/pull.c| 7 ++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 754dc80..5c9380d 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1138,6 +1138,9 @@ of clones and fetches.
  - any external helpers are named by their protocol (e.g., use
`hg` to allow the `git-remote-hg` helper)
 
+'GIT_MERGE_ALLOW_UNRELATED_HISTORIES'::
+   Allow "git merge" to merge unrelated histories by default.
+
 
 Discussion[[Discussion]]
 
diff --git a/builtin/merge.c b/builtin/merge.c
index e3db41b..4e8b1a1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1191,6 +1191,9 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
head_commit = lookup_commit_or_die(head_sha1, "HEAD");
 
git_config(git_merge_config, NULL);
+   if (getenv("GIT_MERGE_ALLOW_UNRELATED_HISTORIES"))
+   allow_unrelated_histories =
+   git_env_bool("GIT_MERGE_ALLOW_UNRELATED_HISTORIES", 0);
 
if (branch_mergeoptions)
parse_branch_merge_options(branch_mergeoptions);
diff --git a/builtin/pull.c b/builtin/pull.c
index 797932d..4e886a5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,7 +86,7 @@ static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
-static int opt_allow_unrelated_histories;
+static int opt_allow_unrelated_histories = -1; /* unspecified */
 
 /* Options passed to git-fetch */
 static char *opt_all;
@@ -159,6 +159,9 @@ static struct option pull_options[] = {
OPT_SET_INT(0, "allow-unrelated-histories",
_allow_unrelated_histories,
N_("allow merging unrelated histories"), 1),
+   OPT_SET_INT(0, "no-allow-unrelated-histories",
+   _allow_unrelated_histories,
+   N_("do not allow merging unrelated histories"), 0),
 
/* Options passed to git-fetch */
OPT_GROUP(N_("Options related to fetching")),
@@ -609,6 +612,8 @@ static int run_merge(void)
argv_array_push(, opt_gpg_sign);
if (opt_allow_unrelated_histories > 0)
argv_array_push(, "--allow-unrelated-histories");
+   else if (!opt_allow_unrelated_histories)
+   argv_array_push(, "--no-allow-unrelated-histories");
 
argv_array_push(, "FETCH_HEAD");
ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
-- 
2.8.1-422-g6d9b748

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] t3033: avoid 'ambiguous refs' warning

[Junio C Hamano]

Because "test_commit five" creates a commit and point it with a tag
'five', doing so on a branch whose name is 'five' will later result
in an 'ambiguous refs' warning.  Even though it is harmless because
all the later references are for the tag, there is no reason for the
branch to be called 'five'.  Give it a name that describes its
purpose more clearly, i.e. "newroot".

Signed-off-by: Junio C Hamano 
---
 t/t3033-merge-toplevel.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3033-merge-toplevel.sh b/t/t3033-merge-toplevel.sh
index c1379b0..d314599 100755
--- a/t/t3033-merge-toplevel.sh
+++ b/t/t3033-merge-toplevel.sh
@@ -19,7 +19,7 @@ test_expect_success setup '
test_commit three &&
git checkout right &&
test_commit four &&
-   git checkout --orphan five &&
+   git checkout --orphan newroot &&
test_commit five &&
git checkout master
 '
-- 
2.8.1-422-g6d9b748

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] merge: introduce merge.allowUnrelatedhistories configuration option

[Junio C Hamano]

It was rumored that in an unspecified workflow it is common to
create what Kernel folks call "crazy" and "insane" merges of two
unrelated histories, and having to give --allow-unrelated-histories
option every time is cumbersome.

Just in case the rumor will substanticated with a proper rationale
in the future, prepare a change to allow disabling the safety by
default.

Signed-off-by: Junio C Hamano 
---
 Documentation/git.txt  |  4 
 Documentation/merge-config.txt |  7 +++
 builtin/merge.c|  3 +++
 t/t3033-merge-toplevel.sh  | 29 +
 t/t5521-pull-options.sh|  9 -
 5 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 5c9380d..f2edac1 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1140,6 +1140,10 @@ of clones and fetches.
 
 'GIT_MERGE_ALLOW_UNRELATED_HISTORIES'::
Allow "git merge" to merge unrelated histories by default.
+   It is recommended that a script that regularly wants to
+   create such a merge to set and export this environment
+   variable upfront, instead of forcing its users to set
+   merge.allowunrelatedhistories configuration variable.
 
 
 Discussion[[Discussion]]
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index 002ca58..8b3d14b 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -47,6 +47,13 @@ merge.stat::
Whether to print the diffstat between ORIG_HEAD and the merge result
at the end of the merge.  True by default.
 
+merge.allowUnrelatedhistories::
+   Setting this option to true (false) makes `git merge` and `git
+   pull` to pretend as if the `--allow-unrelated-histories`
+   (`--no-allow-unrelated-histories`) option was given from the
+   command line.  The configuration is ignored when one of these
+   options is explicitly given from the command line.
+
 merge.tool::
Controls which merge tool is used by linkgit:git-mergetool[1].
The list below shows the valid built-in values.
diff --git a/builtin/merge.c b/builtin/merge.c
index 4e8b1a1..e979c68 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -583,6 +583,9 @@ static int git_merge_config(const char *k, const char *v, 
void *cb)
} else if (!strcmp(k, "commit.gpgsign")) {
sign_commit = git_config_bool(k, v) ? "" : NULL;
return 0;
+   } else if (!strcmp(k, "merge.allowunrelatedhistories")) {
+   allow_unrelated_histories = git_config_bool(k, v);
+   return 0;
}
 
status = fmt_merge_msg_config(k, v, cb);
diff --git a/t/t3033-merge-toplevel.sh b/t/t3033-merge-toplevel.sh
index d314599..583b837 100755
--- a/t/t3033-merge-toplevel.sh
+++ b/t/t3033-merge-toplevel.sh
@@ -149,4 +149,33 @@ test_expect_success 'two-project merge with 
--allow-unrelated-histories' '
git diff --exit-code five
 '
 
+test_expect_success 'two-project merge with merge.allowunrelatedhistories' '
+   t3033_reset &&
+
+   # make sure configuration parser works
+   git reset --hard four &&
+   test_config merge.allowunrelatedhistories notabool &&
+   test_must_fail git merge . HEAD &&
+
+   # disabled explicitly and redundantly by configuration
+   git reset --hard four &&
+   test_config merge.allowunrelatedhistories false &&
+   test_must_fail git merge five &&
+
+   # disabled explicitly by configuration, overridden by command line
+   git reset --hard four &&
+   test_config merge.allowunrelatedhistories false &&
+   git merge --allow-unrelated-histories five &&
+
+   # enabled by configuration but explicitly disabled
+   git reset --hard four &&
+   test_config merge.allowunrelatedhistories true &&
+   test_must_fail git merge --no-allow-unrelated-histories five &&
+
+   # enabled by configuration
+   git reset --hard four &&
+   test_config merge.allowunrelatedhistories true &&
+   git merge five
+'
+
 test_done
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index ded8f98..50f0887 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -161,7 +161,14 @@ test_expect_success 'git pull --allow-unrelated-histories' 
'
(
cd dst &&
test_must_fail git pull ../src side &&
-   git pull --allow-unrelated-histories ../src side
+   git pull --allow-unrelated-histories ../src side &&
+
+   git reset --hard one &&
+   git config merge.allowunrelatedhistories no &&
+   test_must_fail git pull ../src side &&
+   git config merge.allowunrelatedhistories yes &&
+   test_must_fail git pull --no-allow-unrelated-histories ../src 
side &&
+   git pull ../src side
)
 '
 
-- 
2.8.1-422-g6d9b748

--
To unsubscribe from this 

[PATCH 0/4] Loosening "two project merge" safety

[Junio C Hamano]

Yaroslav Halchenko gave a vague "forcing 'git merge' users to always
give --allow-unrelated-histories option when they create crap/insane
merges are not nice", which I couldn't guess the validity due to
lack of concrete use case.  Just in case it is substantiated, here
is a series to selectively and safely loosen the safety for specific
use cases and users.

Junio C Hamano (4):
  t3033: avoid 'ambiguous refs' warning
  pull: pass --allow-unrelated-histories to "git merge"
  merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment
  merge: introduce merge.allowUnrelatedhistories configuration option

 Documentation/git-merge.txt | 14 +-
 Documentation/git.txt   |  7 +++
 Documentation/merge-config.txt  |  7 +++
 Documentation/merge-options.txt |  8 
 builtin/merge.c |  6 ++
 builtin/pull.c  | 11 +++
 t/t3033-merge-toplevel.sh   | 31 ++-
 t/t5521-pull-options.sh | 28 
 8 files changed, 98 insertions(+), 14 deletions(-)

-- 
2.8.1-422-g6d9b748

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] pull: pass --allow-unrelated-histories to "git merge"

[Junio C Hamano]

An earlier commit said:

We could add the same option to "git pull" and have it passed
through to underlying "git merge".  I do not have a fundamental
opposition against such a feature, but this commit does not do
so and instead leaves it as low-hanging fruit for others,
because such a "two project merge" would be done after fetching
the other project into some location in the working tree of an
existing project and making sure how well they fit together, it
is sufficient to allow a local merge without such an option
pass-through from "git pull" to "git merge".

Prepare a patch to make it a reality, just in case it is needed.

Signed-off-by: Junio C Hamano 
---
 Documentation/git-merge.txt | 14 +-
 Documentation/merge-options.txt |  8 
 builtin/pull.c  |  6 ++
 t/t5521-pull-options.sh | 21 +
 4 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 689aa4c..b758d55 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
[-s ] [-X ] [-S[]]
+   [--[no-]allow-unrelated-histories]
[--[no-]rerere-autoupdate] [-m ] [...]
 'git merge'  HEAD ...
 'git merge' --abort
@@ -98,19 +99,6 @@ commit or stash your changes before running 'git merge'.
 'git merge --abort' is equivalent to 'git reset --merge' when
 `MERGE_HEAD` is present.
 
---allow-unrelated-histories::
-   By default, `git merge` command refuses to merge histories
-   that do not share a common ancestor.  This option can be
-   used to override this safety when merging histories of two
-   projects that started their lives independently.  As that is
-   a very rare occasion, no configuration variable to enable
-   this by default exists and will not be added, and the list
-   of options at the top of this documentation does not mention
-   this option.  Also `git pull` does not pass this option down
-   to `git merge` (instead, you `git fetch` first, examine what
-   you will be merging and then `git merge` locally with this
-   option).
-
 ...::
Commits, usually other branch heads, to merge into our branch.
Specifying more than one commit will create a merge with
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index f08e9b8..dfb43d0 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -114,3 +114,11 @@ ifndef::git-pull[]
reporting.
 
 endif::git-pull[]
+
+--allow-unrelated-histories::
+   By default, `git merge` command refuses to merge histories
+   that do not share a common ancestor.  This option can be
+   used to override this safety when merging histories of two
+   projects that started their lives independently. As that is
+   a very rare occasion, no configuration variable to enable
+   this by default exists and will not be added.
diff --git a/builtin/pull.c b/builtin/pull.c
index 5145fc6..797932d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
+static int opt_allow_unrelated_histories;
 
 /* Options passed to git-fetch */
 static char *opt_all;
@@ -155,6 +156,9 @@ static struct option pull_options[] = {
OPT_PASSTHRU('S', "gpg-sign", _gpg_sign, N_("key-id"),
N_("GPG sign commit"),
PARSE_OPT_OPTARG),
+   OPT_SET_INT(0, "allow-unrelated-histories",
+   _allow_unrelated_histories,
+   N_("allow merging unrelated histories"), 1),
 
/* Options passed to git-fetch */
OPT_GROUP(N_("Options related to fetching")),
@@ -603,6 +607,8 @@ static int run_merge(void)
argv_array_pushv(, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(, opt_gpg_sign);
+   if (opt_allow_unrelated_histories > 0)
+   argv_array_push(, "--allow-unrelated-histories");
 
argv_array_push(, "FETCH_HEAD");
ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 18372ca..ded8f98 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -144,4 +144,25 @@ test_expect_success 'git pull --all --dry-run' '
)
 '
 
+test_expect_success 'git pull --allow-unrelated-histories' '
+   test_when_finished "rm -fr src dst" &&
+   git init src &&
+   (
+   cd src &&
+   test_commit one &&
+   test_commit two
+   ) &&
+   git clone src dst &&
+   (
+   cd src &&
+   git checkout --orphan 

Re: history damage in linux.git

[Junio C Hamano]

Linus Torvalds  writes:

> But this patch is small and simple, and has some excuses for its
> behavior. What do people think?

I like it that you call it "excuse" not "rationale", as I couldn't
form a logical connection between your "4 (2) letters" and "1
(100)" at all ;-)

Modulo the usual style issues (e.g. we frown upon patches in
attachement that makes it harder to quote and comment), I think this
is a strict improvement and is a good measure until somebody does a
full "topologically closest" solution.

>  Linus
>
>  builtin/name-rev.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 092e03c3cc9b..0354c8d222e1 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -16,9 +16,6 @@ typedef struct rev_name {
>  
>  static long cutoff = LONG_MAX;
>  
> -/* How many generations are maximally preferred over _one_ merge traversal? 
> */
> -#define MERGE_TRAVERSAL_WEIGHT 65535
> -
>  static void name_rev(struct commit *commit,
>   const char *tip_name, int generation, int distance,
>   int deref)
> @@ -55,19 +52,26 @@ copy_data:
>   parents;
>   parents = parents->next, parent_number++) {
>   if (parent_number > 1) {
> + int weight;
>   size_t len;
>   char *new_name;
>  
>   strip_suffix(tip_name, "^0", );
> - if (generation > 0)
> +
> + // The extra merge traversal "weight" depends
> + // on how complex the resulting name is.
> + if (generation > 0) {
> + weight = 1;
>   new_name = xstrfmt("%.*s~%d^%d", (int)len, 
> tip_name,
>  generation, parent_number);
> - else
> + } else {
> + weight = 100;
>   new_name = xstrfmt("%.*s^%d", (int)len, 
> tip_name,
>  parent_number);
> + }
>  
>   name_rev(parents->item, new_name, 0,
> - distance + MERGE_TRAVERSAL_WEIGHT, 0);
> + distance + weight, 0);
>   } else {
>   name_rev(parents->item, tip_name, generation + 1,
>   distance + 1, 0);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 'next'ed --allow-unrelated-histories could cause lots of grief

[Yaroslav Halchenko]

On Thu, 21 Apr 2016, Junio C Hamano wrote:

> Yaroslav Halchenko  writes:

> > I have decided to try 2.8.1.369.geae769a available from debian
> > experimental and through our (datalad) tests failing I became
> > aware of the 

> > 
> > https://github.com/git/git/pull/158/commits/e379fdf34fee96cd205be83ff4e71699bdc32b18
> > merge: refuse to create too cool a merge by default

> > which is planned for the next release.

> See http://thread.gmane.org/gmane.linux.kernel.gpio/15365/focus=15401
> for the backstory.

THANKS for the link!

> As this is to allow maintainers at higher levels of hierarchy not to
> have to worry about stupid mistakes happen at maintainers at lower
> levels, I'm afraid that turning this into an opt-in safety would
> defeat the point of the change in a major way.

> > ... BUT not sure if it is so
> > important as to cause a change in behavior on which some projects using
> > git through the cmdline interface might have been relying upon for
> > years!

> It is not very productive to make such an emotional statement
> without substantiating _why_ a merge that adds a new root, which was
> declared in the thread above as "crap" (in the context of the kernel
> project),

Sorry if my statement sounded too emotional ;)  I will outline some of
the use-cases below.

Meanwhile, I could argue that 'crap' in the above thread refers to the
entirety of the branch and thus more to the originating detached
root commit.  Action of creating of such a branch was also described  as
someone 'has done something TOTALLY INSANE'. And as "maintainers at
higher levels" expressed: "You actually have to *work* at making shit
like this".

I do see now the reason for introduced change of behavior clearer BUT I
would still argue that it should better be supported somehow by at least
a configuration option to not make poor mortals like yours truly
to sweat to stay compatible with multiple versions of git.

> is necessary and is a good idea in "some projects".  Maybe
> there is a valid use case that those from the kernel land didn't
> think about.

FWIW

- for git-annex (Joey was CCed from the beginning, not sure if annex
  would be affected though), it would be merging of git-annex
  branches while joining multiple annexes for the sync (e.g. by git
  annex assistant). 

- In our (datalad) case, repository is initialized with stock 'master'
  branch which carries configuration, which then used by the 'crawler'
  to initiate 'incoming' branch to contain (only) incoming data, which
  is later branched into 'incoming-processed' and later merged into
  'master', which is where such problematic merge would happen.  With
  such a workflow we can easily maintain custom changes within master,
  while automate changes to the crawled and processed data within
  those other branches being merged into master for final consumption.

  As a somewhat ugly workaround, we could probably artificially
  create an empty initial commit (forgot even how to do this magic) and
  branch-off it I guess, but I see other unit-tests failing as well, so
  I guess the situation is more chronic.

- In Debian packaging world, people often use 'debian' overlay branch
  which has no common ancestor with 'upstream' sources, but which need
  to be merged for 'in-tree' work.  I don't remember though if any of
  the tools rely on this feature (gbp iirc overlays not through the
  merge), but I know that I have used it quite a few times
  (interactively though, so yes -- could add a flag).

> > Given that git is quite 'self-healing', i.e. if someone has managed to
> > make a merge he didn't intend to, there is always a way back (e.g., as
> > simple as git reset --hard HEAD^),

> That is only true if people notice.  A mere warning would not be an
> effective prevention measure for a user who has to perform dozens of
> merges a day.

Could be argued...  Generally git's warnings is not something to be
ignored IMHO.  Do you ignore a balk of them on a daily basis?  if so --
they might better be downgraded to some kind of 'info' level msg then.

> I am personally on the fence, but right now I am on the side of
> keeping the behaviour as implemented and documented, simply because
> I haven't heard anything concrete to convince me why some people
> need to regularly do a "crap" merge (in other words, in what context
> these are not "crap" merges but ability to create them is a valuable
> part of everyday workflow).

once again -- IMHO it wasn't a 'merge' which was a crap, but the state
of the branch to be merged, and getting to that stage was non-trivial
endeavor as well ;)

Since the referenced situation happened only in 2016, I think that such
merges leading to undesired outcomes were quite a rare incident.  On the
other hand I do not have any statistic on how many of similar
merges were intensional, but I guess there could easily be thousands of
such merges intended at least in the git-annex world alone.  The point
is that it would 

Re: 'next'ed --allow-unrelated-histories could cause lots of grief

[Joey Hess]

Yaroslav Halchenko wrote:
> which is planned for the next release.  I guess it is indeed a
> worthwhile accident-prevention measure BUT not sure if it is so
> important as to cause a change in behavior on which some projects using
> git through the cmdline interface might have been relying upon for
> years!

Not only through the command line interface. The git-annex webapp has
common use cases that will be broken by this change.

> Moreover, it was explicitly stated that "no configuration variable to
> enable this by default exists and will not be added", which would cause
> 3rd party scripts/code/projects relying on previous behavior  to provide
> version specific handling (either to add that
> --allow-unrelated-histories or not)... very cumbersome!

Agreed, a configuration setting that could be passed via -c would be
much less cumbersome than checking the version of git in order to only
pass the option to git versions that understand it. This would also
provide a way to get git pull to allow such merges.

Compare with, for example, the change to default to an interactive
merge, where GIT_MERGE_AUTOEDIT=no was provided to ease compatability.

-- 
see shy jo


signature.asc
Description: PGP signature

Re: history damage in linux.git

[Linus Torvalds]

On Thu, Apr 21, 2016 at 11:05 AM, Jeff King  wrote:
>
> I actually think the best name for aed06b9 is probably:
>
>   v3.13-rc1~65^2^2~42

Sounds likely. I don't know how to find that best match without a
complete rewrite, though.

My recent patch that got

  v3.13-rc2~32^2^2~47

comes close to that, and the complexity is similar but the numbers are
actually smaller, so I guess my heuristic did indeed find a "simpler"
name, but yes, the one based on 3.13-rc1 would definitely be the
better one.

> which I found by picking the oldest tag from "git tag --contains" and
> plugging it into "git describe --match".

Yeah, so you basically did the "let's figure out minimal inclusion" by hand.

> Sadly, neither git's internal
> version-sorting nor GNU's "sort -V" knows that "v1.0-rc1" comes before
> "v1.0", so I had to rely on "--sort=taggerdate".

I'm not seeing the "sadly".

I think "--sort=taggerdate" is pretty much the only sane sort there is
for tags, unless you do a true and full topological one (ie sort based
on by how many commits that tag encompasses, but also by how each tag
contains another tag).

  Linus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Jeff King]

On Thu, Apr 21, 2016 at 10:59:52AM -0700, Linus Torvalds wrote:

> That said, I do think that a much bigger conceptual change that
> actually does full traversal and be much more complicated might be the
> only "correct" solution.
> 
> So my patch is just a "improve heuristics" small fixlet rather than
> something optimal.

Yeah, I'd agree with both points. Unless somebody is planning to work on
the bigger change in the near future, your patch is a strict improvement
to the heuristic, and is worth applying.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Jeff King]

On Thu, Apr 21, 2016 at 10:23:10AM -0700, Linus Torvalds wrote:

> > which is technically true, but kind of painful to read. It may be that a
> > reasonable weight is somewhere between "1" and "65535", though.
> 
> Based on my tests, the "right" number is somewhere in the 500-1000
> range for this particular case. But it's still a completely made up
> number.

Yeah, exactly. I think if we're going to tweak the weight heuristic it
would be worth choosing a random sample of commits throughout history
and seeing how they look with various weights.

> > However, I think the more fundamental confusion with git-describe is
> > that people expect the shortest distance to be the "first" tag that
> > contained the commit, and that is clearly not true in a branchy history.
> 
> Yeah.
> 
> And I don't think people care *too* much, because I'm sure this has
> happened before, it's just that before when it happened it wasn't
> quite _so_ far off the expected path..

I think about once a year somebody complains to the list that
git-describe chose a bad name. I don't know how many confused users it
takes to muster one complain to the list, though. ;)

> > I actually think most people would be happy with an algorithm more like:
> >
> >   1. Find the "oldest" tag (either by timestamp, or by version-sorting
> >  the tags) that contains the commit in question.
> 
> Yes, we might want to base the "distance" at least partly on the age
> of the base commits.

I had actually meant my (1) and (2) to be part of the same algorithm.
That is, to literally* do a two-pass check over the history, where first
we find the "best" tag, and then compute the distance from that tag. The
first concern trumps the latter completely.

* Where "literally" only means that's the conceptual model. We probably
  could do it in one pass if we're clever, but it would behave as if
  we made the two passes.

Another way to find the "oldest" tag that I didn't mention is to find
all containing tags, and then eliminate any that contain another tag
(similar to the way we cull duplicate merge bases). That gives you an
answer based on the topology, which is more robust than timestamps or
tag names. But it doesn't necessarily provide a single answer, so you'd
still have to break ties with timestamps or name-sorting.

> >   2. Find the "simplest" path from that tag to the commit, where we
> >  are striving mostly for shortness of explanation, not of path (so
> >  "~500" is way better than "~20^2~30^2~14", even though the latter
> >  is technically a shorter path).
> 
> Well, so the three different paths I've seen are:
> 
>  - standard git (65536), and 1000:
>aed06b9 tags/v4.6-rc1~9^2~792
> 
>  - non-first-parent cost: 500:
>aed06b9 tags/v3.13-rc7~9^2~14^2~42
> 
>  - non-first parent cost: 1:
>aed06b9 tags/v3.13~5^2~4^2~2^2~1^2~42
> 
> so there clearly are multiple valid answers.
> 
> I would actually claim that the middle one is the best one - but I
> claim that based on your algorithm case #1. The last one may be the
> shortest actual path, but it's a shorter path to a newer tag that is a
> superset of the older tag, so the middle one is actually not just
> better based on age, but is a better choice based on "minimal actual
> history".

Yeah, I'd agree the middle one is the best one, because the other tags
contain -rc7. Finding the best tag is much more important than the path
distance, because that's the part that humans read and care about. The
rest is mostly machine-readable to find the exact commit, so we want any
path that's accurate, and not too cumbersome to look at or cut and
paste (and obviously shorter path is better, too).

I actually think the best name for aed06b9 is probably:

  v3.13-rc1~65^2^2~42

which I found by picking the oldest tag from "git tag --contains" and
plugging it into "git describe --match". Sadly, neither git's internal
version-sorting nor GNU's "sort -V" knows that "v1.0-rc1" comes before
"v1.0", so I had to rely on "--sort=taggerdate".

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Linus Torvalds]

On Thu, Apr 21, 2016 at 10:43 AM, Linus Torvalds
 wrote:
>
> In other words, I'm trying to convince people that my patch not only
> gives a good result, but that the "weight numbers" I use make some
> kind of conceptual sense from a complexity cost angle.

Basically, the patch approximates the numerical representation of the
distance measure with the complexity of the suffix.

It's not a *true* length of the suffix (which does heavily favor
first-parent use, kind of like the current code), but I think it's
better than the pretty random "+65535" that the current git code has.
That number is clearly just completely made up. The new numbers at
least have some kind of logic behind them.

And the current code obviously does give really bad results. Picking
the v4.6-rc1 tag as a base just because it happens to get a lot of
first-parent traversals (800!) from one second-parent commit that is
close to 4.6-rc1 is just obscene.

So the more I look at my patch, the more I go "it's a real improvement
on the current situation".

That said, I do think that a much bigger conceptual change that
actually does full traversal and be much more complicated might be the
only "correct" solution.

So my patch is just a "improve heuristics" small fixlet rather than
something optimal.

 Linus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: problems serving non-bare repos with submodules over http

[Stefan Beller]

On Thu, Apr 21, 2016 at 10:45 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> In case of non bare:
>>
>> Get the repo and all its submodules from the specified remote.
>> (As the submodule is right there, that's the best guess to get it from,
>> no need to get it from somewhere else. The submodule at the remote
>> is the closest match you can get for replicating the superproject with
>> its submodules.)
>>
>> This way is heavy underutilized as it wasn't exercised as often I would
>> guess,
>
> My guess is somewhat different. It is not used because the right
> semantics for such a use case hasn't been defined yet (in other
> words, what you suggested is _wrong_ as is).  You need to remember
> that a particular clone may not be interested in all submodules, and
> it is far from "the closest match".

Yes, when that clone doesn't have some submodules, we can still fall back
on the .gitmodules file.

If you have a submodule chances are, you are interested in it and modified it.
So the highest chance to get your changes is from your remote, no?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: problems serving non-bare repos with submodules over http

[Junio C Hamano]

Stefan Beller  writes:

> In case of non bare:
>
> Get the repo and all its submodules from the specified remote.
> (As the submodule is right there, that's the best guess to get it from,
> no need to get it from somewhere else. The submodule at the remote
> is the closest match you can get for replicating the superproject with
> its submodules.)
>
> This way is heavy underutilized as it wasn't exercised as often I would
> guess, 

My guess is somewhat different. It is not used because the right
semantics for such a use case hasn't been defined yet (in other
words, what you suggested is _wrong_ as is).  You need to remember
that a particular clone may not be interested in all submodules, and
it is far from "the closest match".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Stefan Beller]

On Thu, Apr 21, 2016 at 10:23 AM, Linus Torvalds
 wrote:
> On Thu, Apr 21, 2016 at 10:08 AM, Jeff King  wrote:
>>
>> Right, because it makes the names longer. We give the second-parent
>> traversal a heuristic cost. If we drop that cost to "1", like:
>
> So I dropped it to 500 (removed the two last digits), and it gave a
> reasonable answer. With 1000, it gave the same "based on 4.6" answer
> as the current 65536 value does.
>
>> which is technically true, but kind of painful to read. It may be that a
>> reasonable weight is somewhere between "1" and "65535", though.
>
> Based on my tests, the "right" number is somewhere in the 500-1000
> range for this particular case. But it's still a completely made up
> number.
>
>> However, I think the more fundamental confusion with git-describe is
>> that people expect the shortest distance to be the "first" tag that
>> contained the commit, and that is clearly not true in a branchy history.
>
> Yeah.
>
> And I don't think people care *too* much, because I'm sure this has
> happened before, it's just that before when it happened it wasn't
> quite _so_ far off the expected path..
>
>> I actually think most people would be happy with an algorithm more like:
>>
>>   1. Find the "oldest" tag (either by timestamp, or by version-sorting
>>  the tags) that contains the commit in question.
>
> Yes, we might want to base the "distance" at least partly on the age
> of the base commits.
>
>>   2. Find the "simplest" path from that tag to the commit, where we
>>  are striving mostly for shortness of explanation, not of path (so
>>  "~500" is way better than "~20^2~30^2~14", even though the latter
>>  is technically a shorter path).
>
> Well, so the three different paths I've seen are:
>
>  - standard git (65536), and 1000:
>aed06b9 tags/v4.6-rc1~9^2~792
>
>  - non-first-parent cost: 500:
>aed06b9 tags/v3.13-rc7~9^2~14^2~42
>
>  - non-first parent cost: 1:
>aed06b9 tags/v3.13~5^2~4^2~2^2~1^2~42
>
> so there clearly are multiple valid answers.
>
> I would actually claim that the middle one is the best one - but I
> claim that based on your algorithm case #1. The last one may be the
> shortest actual path, but it's a shorter path to a newer tag that is a
> superset of the older tag, so the middle one is actually not just
> better based on age, but is a better choice based on "minimal actual
> history".
>
>Linus

Combining Junios and Linus idea:

* We want to have the minimal history, i.e. that tag with the fewest
cummulative parent commits. (i.e. v3.13-rc7 is better than v3.13
because `git log --oneline v3.13-rc7 |wc -l` (414317) is smaller tha
`git log --oneline v3.13 |wc -l` (414530).
The difference is 213.

tags/v3.13-rc7~9^2~14^2~42 has 9 + 14 + 42 additional steps (65)

tags/v3.13~5^2~4^2~2^2~1^2~42 has 5 + 4 + 2 + 1 +42 steps (54)

tags/v3.13~5^2~4^2~2^2~1^2~42 has 9 less steps, but its base tag
has a higher weight by 213.

v4.6-rc1 has even more weight (588477).

So I guess what I propose is to take the weight of a tag into account
via `git log --oneline  |wc -l` as that gives the tag which encloses
least history?

We also do not want to have "a lot of side traversals", so we could
punish each additional addendum by a heuristic.


> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Linus Torvalds]

On Thu, Apr 21, 2016 at 10:23 AM, Junio C Hamano  wrote:
>
> I think avoiding side branches to describe with the weight is a
> right thing to do, i.e. if you have this history:
>
> X---o---o---o---o---v4.6
>  \ /
>   o---o
>
> you do not want to explain X as "v4.6~^2~2", and instead you want it
> as "v4.6~5", even though the former is 4 hops while the latter is 5
> hops (which is longer).

Yes. I have a new suggestion: make the "merge weight" depend on how
complex the name ends up being.

And that algorithm actually gives a completely new and improved path:

   aed06b9 tags/v3.13-rc2~32^2^2~47

which is still complex, but is actually the best one I've found so far.

To compare against the previous ones I looked at:

   v4.6-rc1~9^2~792<- current git code
   v3.13-rc2~32^2^2~47 <- new with attached patch
   v3.13-rc7~9^2~14^2~42 <- merge weight 500
   v3.13~5^2~4^2~2^2~1^2~42   <- merge weight 1

that new one is actually the second-most dense, and uses the oldest
tag. So it's a good combination of denseness, but still gets the best
actual base tag.

The logic of the patch is that there are different "complexities" in
the naming, and it's not just whether you are following a second
parent, it's also if you're doing generational hops.

So when you do a first-parent change, the name stays simple (the last
number changes), and that means that the "distance" weight is low (so
that's the normal "+1" we do for first-parent.

But if it's not a first parent, there are two different cases:

 - generation > 0: we add "~%d^%d", and we approximate that with "four
characters", and use a cost that is four orders of magnitude higher
(so 1).

 - generation == 0: we add just "^%d", so generally just two
characters. So use a cost that is just two orders of magnitude higher
(so 100).

In other words, I'm trying to convince people that my patch not only
gives a good result, but that the "weight numbers" I use make some
kind of conceptual sense from a complexity cost angle.

With that, the patch itself is attached.

I think it's better than what "git name-rev" does now. Is it optimal?
No, I think the *optimal* would use some kind of "does one tag contain
the other" logic, and discarding all base names that are just
supersets of another base that still reaches the target.

But this patch is small and simple, and has some excuses for its
behavior. What do people think?

 Linus
 builtin/name-rev.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 092e03c3cc9b..0354c8d222e1 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -16,9 +16,6 @@ typedef struct rev_name {
 
 static long cutoff = LONG_MAX;
 
-/* How many generations are maximally preferred over _one_ merge traversal? */
-#define MERGE_TRAVERSAL_WEIGHT 65535
-
 static void name_rev(struct commit *commit,
const char *tip_name, int generation, int distance,
int deref)
@@ -55,19 +52,26 @@ copy_data:
parents;
parents = parents->next, parent_number++) {
if (parent_number > 1) {
+   int weight;
size_t len;
char *new_name;
 
strip_suffix(tip_name, "^0", );
-   if (generation > 0)
+
+   // The extra merge traversal "weight" depends
+   // on how complex the resulting name is.
+   if (generation > 0) {
+   weight = 1;
new_name = xstrfmt("%.*s~%d^%d", (int)len, 
tip_name,
   generation, parent_number);
-   else
+   } else {
+   weight = 100;
new_name = xstrfmt("%.*s^%d", (int)len, 
tip_name,
   parent_number);
+   }
 
name_rev(parents->item, new_name, 0,
-   distance + MERGE_TRAVERSAL_WEIGHT, 0);
+   distance + weight, 0);
} else {
name_rev(parents->item, tip_name, generation + 1,
distance + 1, 0);

Re: [PATCH/RFC/GSoC 0/2] add a add.patch config variable

[Dominik Fischer]

Matthieu Moy  writes:
> any transition plan would be far more painful than the possible benefits
I agree, it cannot be expected that every external script sets 
GIT_RECURSION_DEPTH before issuing any command just because of this 
little option.


As an exercise for GSoC, I implemented it nonetheless to get more 
familiar with the code base and testing system and receive some 
feedback. Perhaps the counter can even be useful for some actually 
significant use case in the future.


Regards.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Junio C Hamano]

Linus Torvalds  writes:

> On Thu, Apr 21, 2016 at 9:36 AM, Linus Torvalds
>  wrote:
>>
>> This seems to be a git bug.
>>
>> That commit aed06b9 can also be described as
>>
>> v3.13-rc7~9^2~14^2~42
>>
>> so describing it as 'v4.6-rc1~9^2~792' is clearly not closer in any way.
>
> Hmm. I think I see what's up. The git distance function has a special
> hack for preferring first-parent traversal, introduced long long ago
> with commit ac076c29ae8d ("name-rev: Fix non-shortest description").
>
> Changing that
>
>   #define MERGE_TRAVERSAL_WEIGHT 65535
>
> to be a smaller value makes git find the shorter path.
>
> I do not know what the correct fix is, though.

I think avoiding side branches to describe with the weight is a
right thing to do, i.e. if you have this history:

X---o---o---o---o---v4.6
 \ /
  o---o

you do not want to explain X as "v4.6~^2~2", and instead you want it
as "v4.6~5", even though the former is 4 hops while the latter is 5
hops (which is longer).

But when comparing a name based on v4.6 (which I think the algorithm
with the weight heuristics would choose v4.6~5) and another name
based on v3.13, I suspect that we compare them with number of hops
with the weight heuristics, and that is what gives us a wrong
result, isn't it?

I think it should instead compare the number of true hops.

v3.13-rc7~9^2~14^2~42 = 9 + 1 + 14 + 1 + 42 = 67 hops from v3.13
v4.6-rc1~9^2~792 = 9 + 1 + 792 = 802 hops from v4.6-rc1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Linus Torvalds]

On Thu, Apr 21, 2016 at 10:08 AM, Jeff King  wrote:
>
> Right, because it makes the names longer. We give the second-parent
> traversal a heuristic cost. If we drop that cost to "1", like:

So I dropped it to 500 (removed the two last digits), and it gave a
reasonable answer. With 1000, it gave the same "based on 4.6" answer
as the current 65536 value does.

> which is technically true, but kind of painful to read. It may be that a
> reasonable weight is somewhere between "1" and "65535", though.

Based on my tests, the "right" number is somewhere in the 500-1000
range for this particular case. But it's still a completely made up
number.

> However, I think the more fundamental confusion with git-describe is
> that people expect the shortest distance to be the "first" tag that
> contained the commit, and that is clearly not true in a branchy history.

Yeah.

And I don't think people care *too* much, because I'm sure this has
happened before, it's just that before when it happened it wasn't
quite _so_ far off the expected path..

> I actually think most people would be happy with an algorithm more like:
>
>   1. Find the "oldest" tag (either by timestamp, or by version-sorting
>  the tags) that contains the commit in question.

Yes, we might want to base the "distance" at least partly on the age
of the base commits.

>   2. Find the "simplest" path from that tag to the commit, where we
>  are striving mostly for shortness of explanation, not of path (so
>  "~500" is way better than "~20^2~30^2~14", even though the latter
>  is technically a shorter path).

Well, so the three different paths I've seen are:

 - standard git (65536), and 1000:
   aed06b9 tags/v4.6-rc1~9^2~792

 - non-first-parent cost: 500:
   aed06b9 tags/v3.13-rc7~9^2~14^2~42

 - non-first parent cost: 1:
   aed06b9 tags/v3.13~5^2~4^2~2^2~1^2~42

so there clearly are multiple valid answers.

I would actually claim that the middle one is the best one - but I
claim that based on your algorithm case #1. The last one may be the
shortest actual path, but it's a shorter path to a newer tag that is a
superset of the older tag, so the middle one is actually not just
better based on age, but is a better choice based on "minimal actual
history".

   Linus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Jeff King]

On Thu, Apr 21, 2016 at 09:59:13AM -0700, Junio C Hamano wrote:

> Linus Torvalds  writes:
> 
> > That commit aed06b9 can also be described as
> >
> > v3.13-rc7~9^2~14^2~42
> >
> > so describing it as 'v4.6-rc1~9^2~792' is clearly not closer in any way.
> 
> I seem to recall that name-rev had an unexplained heuristics to
> strongly avoid following second parent changes (I see two ^2 in the
> path from 3.13-rc7 above).

Right, because it makes the names longer. We give the second-parent
traversal a heuristic cost. If we drop that cost to "1", like:

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 092e03c..03be8be 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -17,7 +17,7 @@ typedef struct rev_name {
 static long cutoff = LONG_MAX;
 
 /* How many generations are maximally preferred over _one_ merge traversal? */
-#define MERGE_TRAVERSAL_WEIGHT 65535
+#define MERGE_TRAVERSAL_WEIGHT 1
 
 static void name_rev(struct commit *commit,
const char *tip_name, int generation, int distance,


then this case gives:

  v3.13~5^2~4^2~2^2~1^2~42

which is technically true, but kind of painful to read. It may be that a
reasonable weight is somewhere between "1" and "65535", though.

However, I think the more fundamental confusion with git-describe is
that people expect the shortest distance to be the "first" tag that
contained the commit, and that is clearly not true in a branchy history.

I actually think most people would be happy with an algorithm more like:

  1. Find the "oldest" tag (either by timestamp, or by version-sorting
 the tags) that contains the commit in question.

  2. Find the "simplest" path from that tag to the commit, where we
 are striving mostly for shortness of explanation, not of path (so
 "~500" is way better than "~20^2~30^2~14", even though the latter
 is technically a shorter path).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: problems serving non-bare repos with submodules over http

[Stefan Beller]

On Wed, Apr 20, 2016 at 8:14 PM, Yaroslav Halchenko  wrote:
> NB Thank you for the lively discussion!
>
> On Wed, 20 Apr 2016, Stefan Beller wrote:
>
>> >> So currently the protocol doesn't allow to even specify the submodules
>> >> directories.
>
>> > Depends on what you exactly mean by "the protocol", but the
>> > networking protocol is about accessing a single repository.  It is
>> > up to you to decide where to go next after learning what you can
>> > learn from the result, typically by following what appears in
>> > the .gitmodules file.
>
>> Right. But the .gitmodules file is not sufficient.
>
> why?

What do you expect from cloning a repo with submodules?

In case of a bare repo:

Get the repo from the specified remote and get the submodules
from "somewhere" (and .gitmodules helps you guessing where
"somewhere" is).

This has been the traditional way, and the .gitmodules file
is just a helper for a best guess where to get a submodule sha1
from. (The repo pointed at from the .gitmodules file may not exist
any more; or it may have forgot the wanted commit)

In case of non bare:

Get the repo and all its submodules from the specified remote.
(As the submodule is right there, that's the best guess to get it from,
no need to get it from somewhere else. The submodule at the remote
is the closest match you can get for replicating the superproject with
its submodules.)

This way is heavy underutilized as it wasn't exercised as often I would
guess, so the "wrong" default (to obtain the submodule information from
.gitmodules instead of from the remote directly) was not pointed out before.

Now that the client wants to make a decision where to get the
submodules from, based on the bare-ness of the remote, it may
require changes in the wire protocol, such that the remote simply
advertises it is a (non-)bare repository when you clone the superproject
from it. Then the client can make a better decision where to get the
submodules from.




>
>> >...<
>
>> I think on a hosting site they could even coexist when having the
>> layout as above.
>
>>  top.git/
>>  top.git/refs/{heads,tags,...}/...
>>  top.git/objects/...
>>  sub.git/
>>  sub.git/refs/{heads,tags,...}/...
>>  sub.git/objects/...
>
>>  # the following only exist in non bare:
>>  top.git/modules/sub.git/
>>  top.git/modules/sub.git/refs/{heads,tags,...}/...
>>  top.git/modules/sub.git/objects/...
>
>> The later files would be more reflective of what you *really*
>> want if you clone from top.git.
>
> may be there is no need for assumptions and .gitmodules should be
> sufficient?
>
> - absolute url in .gitmodules provides absolute URL/path to the
>   submodule of interest, regardless either submodule is present in
>   originating repository as updated submodule.  Either cloning it
>   instead of original repository would be more efficient is already a
>   heuristic which might fail miserably (may be I have a faster
>   connection to the original repository pointed by the absolute
>   url than to this particular repository)
>
> - relative url in .gitmodules provides relative location to the location
>   of the "top" repository, and that is only when that submodule "absolute"
>   url should be resolved relative to the one of the "top" repository

I think the .gitmodules file is not sufficient for the following reason:

* As a "downstream" user you cannot change remote locations without
altering the history. Maybe you just want to have a mirror of some cool
open source project without the hassle to always merge and maintain changes
in your local submodules configuration. (c.f. git config url..insteadOf
for repos, just for submodule specific)
>
> NB I will consider it a separate issue either relative paths
> without '../' prefix are having any sense in bare repositories.

I guess it is a separate issue.

>
> or have I missed the point?
> --
> Yaroslav O. Halchenko
> Center for Open Neuroscience http://centerforopenneuroscience.org
> Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
> Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
> WWW:   http://www.linkedin.com/in/yarik
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Linus Torvalds]

On Thu, Apr 21, 2016 at 9:36 AM, Linus Torvalds
 wrote:
>
> This seems to be a git bug.
>
> That commit aed06b9 can also be described as
>
> v3.13-rc7~9^2~14^2~42
>
> so describing it as 'v4.6-rc1~9^2~792' is clearly not closer in any way.

Hmm. I think I see what's up. The git distance function has a special
hack for preferring first-parent traversal, introduced long long ago
with commit ac076c29ae8d ("name-rev: Fix non-shortest description").

Changing that

  #define MERGE_TRAVERSAL_WEIGHT 65535

to be a smaller value makes git find the shorter path.

I do not know what the correct fix is, though.

  Linus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Junio C Hamano]

Linus Torvalds  writes:

> That commit aed06b9 can also be described as
>
> v3.13-rc7~9^2~14^2~42
>
> so describing it as 'v4.6-rc1~9^2~792' is clearly not closer in any way.

I seem to recall that name-rev had an unexplained heuristics to
strongly avoid following second parent changes (I see two ^2 in the
path from 3.13-rc7 above).

> However did git decide to use that further-away name? That looks odd.
>
> I'm wondering if it's because of the new root commit we have, and that
> confuses the distance calculation somehow? That came in somewhat
> recently (mid March) with the GPIO pull.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC/GSoC 0/2] add a add.patch config variable

[Junio C Hamano]

Matthieu Moy  writes:

> A config variable to set an option by default is good when the user
> wants to set it and forget about it. In this case, you clearly can't
> "forget about it", as running "git add" and "git add -p" correspond to
> really different use-cases.

That is the most sensible explanation I've heard so far; the
configuration variable add.patch should not exist, period.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC/GSoC 0/2] add a add.patch config variable

[Matthieu Moy]

Dominik Fischer  writes:

> I strove to create an add.patch configuration option that did the same
> as always passing the parameter --patch to git-add. Junio C Hamano
> then made me aware that when set, this option would influence and
> possibly destroy other commands that internally use git-add. So I
> implemented the recursion counter, which is now the first of the two
> commits.

This does not solve the problem of scripting. People write scripts, and
write git commands in these scripts. "git add" is a very good candidate
to appear in scripts, indeed.

You can't break people's scripts without a very solid transition plan.
And most of the time, safe transition plans are also painful for
everyone (remember the push.default change? I'm happy it's done, but
that wasn't a lightweight change ...).

In this case, it's quite clear to me that any transition plan would be
far more painful than the possible benefits. Without breaking backward
compatibility, I can already set "alias.p = add --patch" and use "git p"
instead of "git add --patch". Even better: that's 2 less keystrokes.

A config variable to set an option by default is good when the user
wants to set it and forget about it. In this case, you clearly can't
"forget about it", as running "git add" and "git add -p" correspond to
really different use-cases.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 'next'ed --allow-unrelated-histories could cause lots of grief

[Junio C Hamano]

Yaroslav Halchenko  writes:

> I have decided to try 2.8.1.369.geae769a available from debian
> experimental and through our (datalad) tests failing I became
> aware of the 
>
> 
> https://github.com/git/git/pull/158/commits/e379fdf34fee96cd205be83ff4e71699bdc32b18
> merge: refuse to create too cool a merge by default
>
> which is planned for the next release.

See http://thread.gmane.org/gmane.linux.kernel.gpio/15365/focus=15401
for the backstory.

As this is to allow maintainers at higher levels of hierarchy not to
have to worry about stupid mistakes happen at maintainers at lower
levels, I'm afraid that turning this into an opt-in safety would
defeat the point of the change in a major way.

> ... BUT not sure if it is so
> important as to cause a change in behavior on which some projects using
> git through the cmdline interface might have been relying upon for
> years!

It is not very productive to make such an emotional statement
without substantiating _why_ a merge that adds a new root, which was
declared in the thread above as "crap" (in the context of the kernel
project), is necessary and is a good idea in "some projects".  Maybe
there is a valid use case that those from the kernel land didn't
think about.

> Given that git is quite 'self-healing', i.e. if someone has managed to
> make a merge he didn't intend to, there is always a way back (e.g., as
> simple as git reset --hard HEAD^),

That is only true if people notice.  A mere warning would not be an
effective prevention measure for a user who has to perform dozens of
merges a day.

I am personally on the fence, but right now I am on the side of
keeping the behaviour as implemented and documented, simply because
I haven't heard anything concrete to convince me why some people
need to regularly do a "crap" merge (in other words, in what context
these are not "crap" merges but ability to create them is a valuable
part of everyday workflow).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Linus Torvalds]

On Thu, Apr 21, 2016 at 6:24 AM, Andreas Schwab  wrote:
>
> The branches recently pulled by Linus contain commits with rather old
> author dates, eg 8cad489261c564d4ee1db4de4ac365af56807d8a or
> 281baf7a702693deaa45c98ef0c5161006b48257.  That will probably cause git
> describe --contains to take a different path through the history.

Nothing in git should look at author dates (except for the obvious
code that then shows the date).

The committer date is in fact used for the traversal heuristics for
some cases, but those are different and recent in the examples you
note.

This seems to be a git bug.

That commit aed06b9 can also be described as

v3.13-rc7~9^2~14^2~42

so describing it as 'v4.6-rc1~9^2~792' is clearly not closer in any way.

However did git decide to use that further-away name? That looks odd.

I'm wondering if it's because of the new root commit we have, and that
confuses the distance calculation somehow? That came in somewhat
recently (mid March) with the GPIO pull.

   Linus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Matthieu Moy]

Olaf Hering  writes:

> On Thu, Apr 21, John Keeping wrote:
>
>>  $ git tag --contains aed06b9cfcabf8644ac5f6f108c0b3d01522f88b
>
> Thanks for that, I did not know this variant.
>
> Unless git does not do it for me, I may hackup my script like that to
> find the earlierst tag:

"git tag" has a --sort key, so you can sort on dates, and "| head -n 1".
See also "git for-each-ref" which is essentially a superset of what "git
tag" does, and for-each-ref is plumbing, so safer to use in scripts (we
have a strong tradition of backward compatibility for plumbing).

This relies on the dates recorded in the commits, which may be wrong
(typically if someone commited on a machine with an incorrect clock).
But hopefully not.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC/GSoC 0/2] add a add.patch config variable

[Dominik Fischer]

Indeed this needs more explanations for everyone who did not read the 
posts before.


I strove to create an add.patch configuration option that did the same 
as always passing the parameter --patch to git-add. Junio C Hamano then 
made me aware that when set, this option would influence and possibly 
destroy other commands that internally use git-add. So I implemented the 
recursion counter, which is now the first of the two commits. With this, 
git-add is able to only consider the configuration option when run 
directly by the user, not affecting any commands building upon it.


I would be interested whether this is a suited method to restrict the 
effect of a configuration option to cases where a command is explicitly 
invoked by the user.


Regards.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

'next'ed --allow-unrelated-histories could cause lots of grief

[Yaroslav Halchenko]

Dear Git Gurus,

I guess whenever it rains it indeed pours, so it is me whining again.

I have decided to try 2.8.1.369.geae769a available from debian
experimental and through our (datalad) tests failing I became
aware of the 


https://github.com/git/git/pull/158/commits/e379fdf34fee96cd205be83ff4e71699bdc32b18
merge: refuse to create too cool a merge by default

which is planned for the next release.  I guess it is indeed a
worthwhile accident-prevention measure BUT not sure if it is so
important as to cause a change in behavior on which some projects using
git through the cmdline interface might have been relying upon for
years!

Given that git is quite 'self-healing', i.e. if someone has managed to
make a merge he didn't intend to, there is always a way back (e.g., as
simple as git reset --hard HEAD^), I am really not sure how valuable
such change of default behavior would be?  Could it may be made into a
warning instead? or reversed option "--dont-allow-unrelated-histories"?

Moreover, it was explicitly stated that "no configuration variable to
enable this by default exists and will not be added", which would cause
3rd party scripts/code/projects relying on previous behavior  to provide
version specific handling (either to add that
--allow-unrelated-histories or not)... very cumbersome!  If nothing else
remains, could there at least be a config option which we could
then use regardless of the version of git we are using for such merges?

P.S. Please maintain CC list

Thank you in advance for your consideration,
-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Olaf Hering]

On Thu, Apr 21, John Keeping wrote:

>   $ git tag --contains aed06b9cfcabf8644ac5f6f108c0b3d01522f88b

Thanks for that, I did not know this variant.

Unless git does not do it for me, I may hackup my script like that to
find the earlierst tag:

 for i in `git tag --contains aed06b9cfcabf8644ac5f6f108c0b3d01522f88b`
 do
  git log --max-count=1 --oneline --pretty=%ct:%h:$i $i | cat
 done | sort -n | sed 's@^.*:@@;q'

Olaf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/12] worktree.c: mark current worktree

[Junio C Hamano]

Jeff King  writes:

> To me, the benefit is that you don't have to care about ignore_case. You
> have asked to compare two paths, and any system-appropriate magic should
> be applied. That may be icase, or it may be weird unicode normalization.
>
> I think the key thing missing is that this is only about _filesystem_
> paths. You would not want to use it for tree-to-tree pathname
> comparisons. So maybe "fspath" or something would be more descriptive.

Yup, I would be very happy with "fs" in the name.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/12] worktree.c: mark current worktree

[Jeff King]

On Thu, Apr 21, 2016 at 08:37:32AM -0700, Junio C Hamano wrote:

> >> > While we're at it, how about renaming it to pathcmp (and its friend
> >> > strncmp_icase to pathncmp)?
> >> 
> >> Yes, that seems like a good idea. For anyone familiar with
> >> strcasecmp() or stricmp(), having "icase" in the name makes it seem as
> >> though it's unconditionally case-insensitive, so dropping it from the
> >> name would likely be beneficial.
> >
> > Seconded (thirded?). I have been caught by this confusion in the past,
> > too.
> 
> I agree that strcmp_icase() gives a false impression that it always
> ignores case differences, but a new name that does not at all hint
> that it may do icase comparison as necessary will catch me by an
> opposite confusion in the future.

To me, the benefit is that you don't have to care about ignore_case. You
have asked to compare two paths, and any system-appropriate magic should
be applied. That may be icase, or it may be weird unicode normalization.

I think the key thing missing is that this is only about _filesystem_
paths. You would not want to use it for tree-to-tree pathname
comparisons. So maybe "fspath" or something would be more descriptive.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC/GSoC 0/2] add a add.patch config variable

[Johannes Schindelin]

Hi Dominik,

On Thu, 21 Apr 2016, XZS wrote:

> The following patches try to provide an add.patch configuration variable
> while keeping out of the way of other commands. To do so, an environment
> variable counts how often git was recursively invoked. The variable is
> then only considered in the first recursion.

This dives right into the "how". Maybe a good idea would be to start with
a paragraph that explains the "what" and the "why" first, and only then go
into the details how you implemented it, what other options you considered
and why you preferred your solution?

Even after skimming the patch series, I am still a bit puzzled what issue
it solves...

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/12] worktree.c: mark current worktree

[Junio C Hamano]

Jeff King  writes:

> On Thu, Apr 21, 2016 at 10:23:09AM -0400, Eric Sunshine wrote:
>
>> > While we're at it, how about renaming it to pathcmp (and its friend
>> > strncmp_icase to pathncmp)?
>> 
>> Yes, that seems like a good idea. For anyone familiar with
>> strcasecmp() or stricmp(), having "icase" in the name makes it seem as
>> though it's unconditionally case-insensitive, so dropping it from the
>> name would likely be beneficial.
>
> Seconded (thirded?). I have been caught by this confusion in the past,
> too.

I agree that strcmp_icase() gives a false impression that it always
ignores case differences, but a new name that does not at all hint
that it may do icase comparison as necessary will catch me by an
opposite confusion in the future.

I have not yet formed a firm opinion if pathcmp() conveys enough
hint.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/12] worktree.c: mark current worktree

[Jeff King]

On Thu, Apr 21, 2016 at 10:23:09AM -0400, Eric Sunshine wrote:

> > While we're at it, how about renaming it to pathcmp (and its friend
> > strncmp_icase to pathncmp)?
> 
> Yes, that seems like a good idea. For anyone familiar with
> strcasecmp() or stricmp(), having "icase" in the name makes it seem as
> though it's unconditionally case-insensitive, so dropping it from the
> name would likely be beneficial.

Seconded (thirded?). I have been caught by this confusion in the past,
too.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/12] worktree.c: mark current worktree

[Eric Sunshine]

On Thu, Apr 21, 2016 at 5:33 AM, Duy Nguyen  wrote:
> On Thu, Apr 21, 2016 at 3:19 PM, Duy Nguyen  wrote:
>> On Thu, Apr 21, 2016 at 2:20 PM, Eric Sunshine  
>> wrote:
>>> On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy  
>>> wrote:
 Signed-off-by: Nguyễn Thái Ngọc Duy 
 +   strbuf_addstr(_dir, absolute_path(get_git_dir()));
 +   for (i = 0; i < counter; i++) {
 +   struct worktree *wt = list[i];
 +   strbuf_addstr(, 
 absolute_path(get_worktree_git_dir(wt)));
 +   wt->is_current = !strcmp_icase(git_dir.buf, path.buf);
>>>
>>> Can you talk a bit about why this uses 'icase'? Should it be
>>> respecting cache.h:ignore_case?
>>
>> It does.That function (in dir.c) is just one-liner
>>
>> return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
>>
>> I admit though, the naming does not make that clear.

Ugh, this is only the fourth patch, yet the second stupid review
mistake I've made thus far in this series. For some reason, I kept
reading this as a call to strcasecmp() or stricmp() rather than
strcmp_icase(). Worse, I had even consulted path.c:strcmp_icase() to
see how the issue was handled there.

> While we're at it, how about renaming it to pathcmp (and its friend
> strncmp_icase to pathncmp)?

Yes, that seems like a good idea. For anyone familiar with
strcasecmp() or stricmp(), having "icase" in the name makes it seem as
though it's unconditionally case-insensitive, so dropping it from the
name would likely be beneficial.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 19/33] refs: allow log-only updates

[Michael Haggerty]

On 03/01/2016 01:52 AM, David Turner wrote:
> The refs infrastructure learns about log-only ref updates, which only
> update the reflog.  Later, we will use this to separate symbolic
> reference resolution from ref updating.
> 
> Signed-off-by: David Turner 
> Signed-off-by: Junio C Hamano 
> ---
>  refs/files-backend.c | 15 ++-
>  refs/refs-internal.h |  7 +++
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 1f565cb..189b86e 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2702,7 +2702,7 @@ static int commit_ref_update(struct ref_lock *lock,
>   }
>   }
>   }
> - if (commit_ref(lock)) {
> + if (!(flags & REF_LOG_ONLY) && commit_ref(lock)) {
>   error("Couldn't set %s", lock->ref_name);
>   unlock_ref(lock);
>   return -1;
> @@ -3056,7 +3056,8 @@ static int files_transaction_commit(struct 
> ref_transaction *transaction,
>   goto cleanup;
>   }
>   if ((update->flags & REF_HAVE_NEW) &&
> - !(update->flags & REF_DELETING)) {
> + !(update->flags & REF_DELETING) &&
> + !(update->flags & REF_LOG_ONLY)) {
>   int overwriting_symref = ((update->type & REF_ISSYMREF) 
> &&
> (update->flags & 
> REF_NODEREF));
>  
> @@ -3086,7 +3087,9 @@ static int files_transaction_commit(struct 
> ref_transaction *transaction,
>   update->flags |= REF_NEEDS_COMMIT;
>   }
>   }
> - if (!(update->flags & REF_NEEDS_COMMIT)) {
> +
> + if (!(update->flags & REF_LOG_ONLY) &&
> + !(update->flags & REF_NEEDS_COMMIT)) {

I was just going over this series again, and I think this hunk is
incorrect. If REF_LOG_ONLY, we created and opened the lockfile. And we
didn't call write_ref_to_logfile(), so the lockfile is still open. That
means that we want to call close_ref() here to free up the file
descriptor. (Note that close_ref() closes the lockfile but doesn't
release the lock. That is done further down by unlock_ref().)

So I think this hunk should be omitted.

I realize that this patch series is obsolete, so there is no need to
re-submit. I just wanted to get a sanity check as I implement a new
version of this patch that I'm not misunderstanding something.

> [...]

Michael

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Andreas Schwab]

Olaf Hering  writes:

> How can I find out whats going on? Is my git(1) 2.8.1 broken, or did
> Linus just pull some junk tree (and does he continue to do so)?

The branches recently pulled by Linus contain commits with rather old
author dates, eg 8cad489261c564d4ee1db4de4ac365af56807d8a or
281baf7a702693deaa45c98ef0c5161006b48257.  That will probably cause git
describe --contains to take a different path through the history.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[John Keeping]

On Thu, Apr 21, 2016 at 01:30:04PM +0200, Olaf Hering wrote:
> To track the changes in hyperv related files I created some scripts
> years ago to automate the process of finding relevant commits in
> linux.git. Part of that process is to record the tag when a commit
> appeared in mainline. This worked fine, until very recently.
> 
> Suddenly years-old commits are declared as having-just-arrived in
> linux.git. Look at this example:
> 
>   $ git log --oneline -- drivers/input/serio/hyperv-keyboard.c
>   2048157 Drivers: hv: vmbus: fix the building warning with hyperv-keyboard
>   62238f3 Input: hyperv-keyboard - register as a wakeup source
>   c3c4d99 Input: hyperv-keyboard - pass through 0xE1 prefix
>   aed06b9 Input: add a driver to support Hyper-V synthetic keyboard
>   $ git describe --contains aed06b9
>   v4.6-rc1~9^2~792
>   $ git show aed06b9 | head
>   commit aed06b9cfcabf8644ac5f6f108c0b3d01522f88b
>   Author: K. Y. Srinivasan 
>   Date:   Wed Sep 18 12:50:42 2013 -0700
> 
> Obviously that and other commits are in the tree since a very long time.
> 
> How can I find out whats going on? Is my git(1) 2.8.1 broken, or did
> Linus just pull some junk tree (and does he continue to do so)?

I suspect it indicates that an old tree was pulled in such that the path
to v4.6-rc1 is shorter than to the older version.  The commit is clearly
in v3.13-rc1:

$ git tag --contains aed06b9cfcabf8644ac5f6f108c0b3d01522f88b
v3.13
v3.13-rc1
v3.13-rc2
[snip]

The behaviour of describe is a bit clearer if you limit it to v3.*:

$ git describe --match='v3.*' --contains 
aed06b9cfcabf8644ac5f6f108c0b3d01522f88b
v3.13-rc7~9^2~14^2~42

$ git describe --match='v3.13-rc1' --contains 
aed06b9cfcabf8644ac5f6f108c0b3d01522f88b
v3.13-rc1~65^2^2~42

It seems that the path to v4.6-rc1 is "more direct" than to either of
these commits: there is only one second-parent merge transition.

>From a quick look, I think the problem is in commit c155c7492c9a ("Merge
branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input")
which merges a branch that has repeatedly had master merged back into it
but does not build on any recent releases.  The most recent tag on the
first-parent history of that branch is v3.0-rc4.

I think it is as simple as git-describe (or git-name-rev which is used
in the --contains case) preferring a less branchy path, which has been
introduced in v4.6 with the merge commit above.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Matthieu Moy]

Olaf Hering  writes:

> On Thu, Apr 21, Matthieu Moy wrote:
>
>> My guess is that this commit has been sitting for a long time in a
>> repo outside Linus' linux.git, and got merged only recently.
>
> Thats what it looks like. And thats what I'm complaining about. But in
> fact that file is there since v3.13-rc7 (if the tag is really correct in
> my patch file), since at least end of 2014.

Ah, indeed. It was merged right before 3.13. See "git tag --contains
aed06b9cfcab".

It's indeed weird that "git describe --contains" gives a named based on
tag v4.6-rc1, but it is not really incorrect since tags/v4.6-rc1~9^2~792
is indeed a correct name for aed06b9cfcabf8644ac5f6f108c0b3d01522f88b,
and actually uses a tag that follows it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Olaf Hering]

On Thu, Apr 21, Matthieu Moy wrote:

> My guess is that this commit has been sitting for a long time in a
> repo outside Linus' linux.git, and got merged only recently.

Thats what it looks like. And thats what I'm complaining about. But in
fact that file is there since v3.13-rc7 (if the tag is really correct in
my patch file), since at least end of 2014.



And after some checking its in linux-3.13-rc1 from 2013-11-22, which
indicates that such damage has already happend earlier. At least I have
a record from Nov 2014 for the patch file. There is a slim chance that I
tweaked that particular tag menually in the patch file.

This is a list of changed tags in my patch collection:

 Date: Wed, 18 Sep 2013 12:50:42 -0700
-Patch-mainline: v3.13-rc7
+Patch-mainline: v4.6-rc1
 Subject: Input: add a driver to support Hyper-V synthetic keyboard
 Git-commit: aed06b9cfcabf8644ac5f6f108c0b3d01522f88b

 Date: Sun, 12 Jan 2014 11:09:14 -0800
-Patch-mainline: v3.14
+Patch-mainline: v4.6-rc1
 Subject: Input: hyperv-keyboard - pass through 0xE1 prefix
 Git-commit: c3c4d99485ea51cd354ed3cd955a8310703456b6

 Date: Wed, 6 Aug 2014 13:33:54 -0700
-Patch-mainline: v3.17-rc1
+Patch-mainline: v4.6-rc1
 Subject: Input: hyperv-keyboard - register as a wakeup source
 Git-commit: 62238f3aadc9bc56da70100e19ec61b9f8d72a5f

 Date: Mon, 30 Nov 2015 19:22:13 +0300
-Patch-mainline: v4.5-rc1
+Patch-mainline: v4.5-rc2
 Subject: drivers/hv: replace enum hv_message_type by u32
 Git-commit: 7797dcf63f11b6e1d34822daf2317223d0f4ad46

 Date: Mon, 30 Nov 2015 19:22:14 +0300
-Patch-mainline: v4.5-rc1
+Patch-mainline: v4.5-rc2
 Subject: drivers/hv: Move HV_SYNIC_STIMER_COUNT into Hyper-V UAPI x86 header
 Git-commit: 4f39bcfd1c132522380138a323f9af7902766301

 Date: Mon, 30 Nov 2015 19:22:15 +0300
-Patch-mainline: v4.5-rc1
+Patch-mainline: v4.5-rc2
 Subject: drivers/hv: Move struct hv_message into UAPI Hyper-V x86 header
 Git-commit: 5b423efe11e822e092e8c911a6bad17eadf718eb

 Date: Mon, 30 Nov 2015 19:22:16 +0300
-Patch-mainline: v4.5-rc1
+Patch-mainline: v4.5-rc2
 Subject: drivers/hv: Move struct hv_timer_message_payload into UAPI Hyper-V 
x86 header
 Git-commit: c71acc4c74dddeeede69fdd4f0b1a124f9df

 Date: Mon, 30 Nov 2015 19:22:21 +0300
-Patch-mainline: v4.5-rc1
+Patch-mainline: v4.5-rc2
 Subject: kvm/x86: Hyper-V SynIC timers
 Git-commit: 1f4b34f825e8cef6f493d06b46605384785b3d16

 Date: Tue, 2 Feb 2016 11:45:02 +0800
-Patch-mainline: v4.6-rc1
+Patch-mainline: v4.6-rc2
 Subject: x86/cpu: Convert printk(KERN_ ...) to pr_(...)
 Git-commit: 1b74dde7c47c19a73ea3e9fac95ac27b5d3d50c5

Olaf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: history damage in linux.git

[Matthieu Moy]

Olaf Hering  writes:

> To track the changes in hyperv related files I created some scripts
> years ago to automate the process of finding relevant commits in
> linux.git. Part of that process is to record the tag when a commit
> appeared in mainline. This worked fine, until very recently.
>
> Suddenly years-old commits are declared as having-just-arrived in
> linux.git. Look at this example:
>
>   $ git log --oneline -- drivers/input/serio/hyperv-keyboard.c
>   2048157 Drivers: hv: vmbus: fix the building warning with hyperv-keyboard
>   62238f3 Input: hyperv-keyboard - register as a wakeup source
>   c3c4d99 Input: hyperv-keyboard - pass through 0xE1 prefix
>   aed06b9 Input: add a driver to support Hyper-V synthetic keyboard
>   $ git describe --contains aed06b9
>   v4.6-rc1~9^2~792
>   $ git show aed06b9 | head
>   commit aed06b9cfcabf8644ac5f6f108c0b3d01522f88b
>   Author: K. Y. Srinivasan 
>   Date:   Wed Sep 18 12:50:42 2013 -0700
>
> Obviously that and other commits are in the tree since a very long time.

I cannot take that conclusion from the excerpts above. What I can
conclude is that the commit was made a long time ago (according to
https://github.com/torvalds/linux/commit/aed06b9cfcabf8644ac5f6f108c0b3d01522f88b
the commit was initially made on Sep 18 2013 by K. Y. Srinivasan, and
then applied by Dmitry Torokhov the day after to "some" tree.

My guess is that this commit has been sitting for a long time in a
repo outside Linus' linux.git, and got merged only recently.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

history damage in linux.git

[Olaf Hering]

To track the changes in hyperv related files I created some scripts
years ago to automate the process of finding relevant commits in
linux.git. Part of that process is to record the tag when a commit
appeared in mainline. This worked fine, until very recently.

Suddenly years-old commits are declared as having-just-arrived in
linux.git. Look at this example:

  $ git log --oneline -- drivers/input/serio/hyperv-keyboard.c
  2048157 Drivers: hv: vmbus: fix the building warning with hyperv-keyboard
  62238f3 Input: hyperv-keyboard - register as a wakeup source
  c3c4d99 Input: hyperv-keyboard - pass through 0xE1 prefix
  aed06b9 Input: add a driver to support Hyper-V synthetic keyboard
  $ git describe --contains aed06b9
  v4.6-rc1~9^2~792
  $ git show aed06b9 | head
  commit aed06b9cfcabf8644ac5f6f108c0b3d01522f88b
  Author: K. Y. Srinivasan 
  Date:   Wed Sep 18 12:50:42 2013 -0700

Obviously that and other commits are in the tree since a very long time.

How can I find out whats going on? Is my git(1) 2.8.1 broken, or did
Linus just pull some junk tree (and does he continue to do so)?

Olaf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/12] worktree.c: mark current worktree

[Duy Nguyen]

On Thu, Apr 21, 2016 at 3:19 PM, Duy Nguyen  wrote:
> On Thu, Apr 21, 2016 at 2:20 PM, Eric Sunshine  
> wrote:
>> On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy  
>> wrote:
>>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>>> ---
>>> diff --git a/worktree.c b/worktree.c
>>> @@ -178,6 +182,18 @@ struct worktree **get_worktrees(void)
>>> }
>>> ALLOC_GROW(list, counter + 1, alloc);
>>> list[counter] = NULL;
>>> +
>>> +   strbuf_addstr(_dir, absolute_path(get_git_dir()));
>>> +   for (i = 0; i < counter; i++) {
>>> +   struct worktree *wt = list[i];
>>> +   strbuf_addstr(, 
>>> absolute_path(get_worktree_git_dir(wt)));
>>> +   wt->is_current = !strcmp_icase(git_dir.buf, path.buf);
>>
>> Can you talk a bit about why this uses 'icase'? Should it be
>> respecting cache.h:ignore_case?
>
> It does.That function (in dir.c) is just one-liner
>
> return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
>
> I admit though, the naming does not make that clear.

While we're at it, how about renaming it to pathcmp (and its friend
strncmp_icase to pathncmp)?
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC/GSoC 1/2] count recursion depth

[XZS]

Some commands may want to act differently when called transitively by
other git commands in contrast to when called by the user directly. The
variable recursion_depth provides all built-ins with a way to tell these
cases apart.

Scripts can use the underlying environment variable GIT_RECURSION_DEPTH
to the same purpose.

Wrappers around git can intentionally set GIT_RECURSION_DEPTH to ensure
a command acts as if it was called internally.

Signed-off-by: XZS 
---
 cache.h|  1 +
 git.c  | 17 +
 t/t0001-init.sh|  1 +
 t/t0120-recursion-depth.sh | 17 +
 4 files changed, 36 insertions(+)
 create mode 100755 t/t0120-recursion-depth.sh

diff --git a/cache.h b/cache.h
index 9f09540..105607c 100644
--- a/cache.h
+++ b/cache.h
@@ -1777,6 +1777,7 @@ struct startup_info {
const char *prefix;
 };
 extern struct startup_info *startup_info;
+extern int recursion_depth;
 
 /* merge.c */
 struct commit_list;
diff --git a/git.c b/git.c
index 968a8a4..0bcc7b4 100644
--- a/git.c
+++ b/git.c
@@ -25,6 +25,7 @@ static const char *env_names[] = {
 };
 static char *orig_env[4];
 static int save_restore_env_balance;
+int recursion_depth;
 
 static void save_env_before_alias(void)
 {
@@ -630,12 +631,28 @@ static void restore_sigpipe_to_default(void)
signal(SIGPIPE, SIG_DFL);
 }
 
+static int get_recursion_depth(void)
+{
+   const char *envrec = getenv("GIT_RECURSION_DEPTH");
+   return envrec ? strtol(envrec, NULL, 10) : 0;
+}
+
+static int set_recursion_depth(int depth)
+{
+   char number[10]; // TODO compute length
+   snprintf(number, sizeof(number), "%i", depth);
+   return setenv("GIT_RECURSION_DEPTH", number, 1);
+}
+
 int main(int argc, char **av)
 {
const char **argv = (const char **) av;
const char *cmd;
int done_help = 0;
 
+   recursion_depth = get_recursion_depth();
+   set_recursion_depth(recursion_depth + 1);
+
cmd = git_extract_argv0_path(argv[0]);
if (!cmd)
cmd = "git-help";
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index a5b9e7a..69e7532 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -93,6 +93,7 @@ test_expect_success 'No extra GIT_* on alias scripts' '
sed -n \
-e "/^GIT_PREFIX=/d" \
-e "/^GIT_TEXTDOMAINDIR=/d" \
+   -e "/^GIT_RECURSION_DEPTH=/d" \
-e "/^GIT_/s/=.*//p" |
sort
EOF
diff --git a/t/t0120-recursion-depth.sh b/t/t0120-recursion-depth.sh
new file mode 100755
index 000..5aeb71a
--- /dev/null
+++ b/t/t0120-recursion-depth.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+test_description='recursion counter'
+
+. ./test-lib.sh
+
+test_expect_success 'recursion counter is 1 on direct run' '
+   git config alias.one "!echo \$GIT_RECURSION_DEPTH" &&
+   test "$(git one)" -eq 1
+'
+
+test_expect_success 'recursion counter is greater 1 on transitive run' '
+   git config alias.two "!git one" &&
+   test "$(git two)" -gt 1
+'
+
+test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC/GSoC 2/2] add a add.patch config variable

[XZS]

Users may like to review their changes when staging by default. It is
also a convenient safety feature for beginners nudging them to have a
second look at their changes when composing a commit.

To this end, the config variable allows to have git-add to always act
like -p was passed.

To not affect other commands that use the add built-in, the variable
looses its effect when invoked transitively.

Signed-off-by: XZS 
---

I corrected the errorneous use of -p in the test. Thanks to Christian
Couder for the notice.

 Documentation/config.txt   |  6 ++
 Documentation/git-add.txt  |  3 +++
 builtin/add.c  |  3 +++
 contrib/completion/git-completion.bash |  1 +
 git.c  |  3 ++-
 t/t3701-add-interactive.sh | 27 +++
 6 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index aea6bd1..73f7dfa 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -752,6 +752,12 @@ add.ignore-errors (deprecated)::
as it does not follow the usual naming convention for configuration
variables.
 
+add.patch::
+   Configures 'git add' to always interactively choose hunks, hinting the
+   user to review changes before staging. This is equivalent to adding the
+   '--patch' option to linkgit:git-add[1] and can be overwritten by
+   invoking git-add with --no-patch.
+
 alias.*::
Command aliases for the linkgit:git[1] command wrapper - e.g.
after defining "alias.last = cat-file commit HEAD", the invocation
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 6a96a66..cdb6663 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -92,6 +92,9 @@ OPTIONS
 This effectively runs `add --interactive`, but bypasses the
 initial command menu and directly jumps to the `patch` subcommand.
 See ``Interactive mode'' for details.
++
+The configuration variable `add.patch` can be set to true to make
+this the default behaviour.
 
 -e::
 --edit::
diff --git a/builtin/add.c b/builtin/add.c
index 145f06e..3249a55 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -272,6 +272,9 @@ static int add_config(const char *var, const char *value, 
void *cb)
!strcmp(var, "add.ignore-errors")) {
ignore_add_errors = git_config_bool(var, value);
return 0;
+   } else if (!strcmp(var, "add.patch") && recursion_depth <= 0) {
+   patch_interactive = git_config_bool(var, value);
+   return 0;
}
return git_default_config(var, value, cb);
 }
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e3918c8..597d20f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1969,6 +1969,7 @@ _git_config ()
esac
__gitcomp "
add.ignoreErrors
+   add.patch
advice.commitBeforeMerge
advice.detachedHead
advice.implicitIdentity
diff --git a/git.c b/git.c
index 0bcc7b4..df2fe58 100644
--- a/git.c
+++ b/git.c
@@ -2,6 +2,7 @@
 #include "exec_cmd.h"
 #include "help.h"
 #include "run-command.h"
+#include 
 
 const char git_usage_string[] =
"git [--version] [--help] [-C ] [-c name=value]\n"
@@ -639,7 +640,7 @@ static int get_recursion_depth(void)
 
 static int set_recursion_depth(int depth)
 {
-   char number[10]; // TODO compute length
+   char number[(int) ceil(log10(pow(2, sizeof(int];
snprintf(number, sizeof(number), "%i", depth);
return setenv("GIT_RECURSION_DEPTH", number, 1);
 }
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index deae948..7ba2817 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -380,4 +380,31 @@ test_expect_success 'patch mode ignores unmerged entries' '
test_cmp expected diff
 '
 
+test_expect_success 'patch mode can be activated per option' '
+   git config add.patch true &&
+   git reset --hard &&
+   echo change >test &&
+   yes | git add > output &&
+   cat output &&
+   grep "Stage this hunk \[y,n,q,a,d,/,e,?\]?" output
+'
+
+test_expect_success 'add.patch configuration does not affect transitive add 
invocations' '
+   git reset --hard &&
+   git checkout -b main >/dev/null 2>&1 &&
+   git branch branch &&
+   echo change >test &&
+   git add --no-patch test &&
+   git commit -m commit >/dev/null 2>&1 &&
+   git checkout branch >/dev/null 2>&1 &&
+   echo other >test &&
+   git add --no-patch test &&
+   git commit -m other >/dev/null 2>&1 &&
+   test_must_fail git merge main >/dev/null 2>&1 &&
+   git config merge.tool mybase &&
+   git config mergetool.mybase.cmd "cat \"\$BASE\" >\"\$MERGED\"" &&
+   git config 

[PATCH/RFC/GSoC 0/2] add a add.patch config variable

[XZS]

The following patches try to provide an add.patch configuration variable
while keeping out of the way of other commands. To do so, an environment
variable counts how often git was recursively invoked. The variable is
then only considered in the first recursion.

To ensure other commands work as expected also when add.patch is set, I
added a test exemplifying the case with mergetool. To confirm the same
for other commands, I also reran all tests with an activated add.patch,
all direct invocations of git-add in the tests augmented with
--no-patch. You can reproduce this by running the following commands
from the root of the git source code repository.

sed -i '/add\.patch/,/}/ {
  # pretend add.patch to always be active
  s/\(patch_interactive = \).*;/\11;/
}' builtin/add.c
find t -type f -exec sed -i ' # in all tests
  /(use\|forget\|hint/! { # exclude help texts
# replace git add invocations, also when options are passed to git,
# but not subcommands named add.
s/git\( -[^ ]\+ [^ ]\+\| --[^ ]\+=[^ ]\+\)* add/& --no-patch/g
  }
  /patch mode can be activated per option/ {
# find add.patched test now invalid and deactivate
s/\(test_expect_\)success/\1failure/
  }
' '{}' + 

I am unsure whether I placed all the code into the correct locations, so
comments are much appreciated, as well as opinions about the concept of
a recursion counter in general.

Regards,
XZS.

XZS (2):
  count recursion depth
  add a add.patch config variable

 Documentation/config.txt   |  6 ++
 Documentation/git-add.txt  |  3 +++
 builtin/add.c  |  3 +++
 cache.h|  1 +
 contrib/completion/git-completion.bash |  1 +
 git.c  | 18 ++
 t/t0001-init.sh|  1 +
 t/t0120-recursion-depth.sh | 17 +
 t/t3701-add-interactive.sh | 27 +++
 9 files changed, 77 insertions(+)
 create mode 100755 t/t0120-recursion-depth.sh

-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/12] worktree.c: mark current worktree

[Duy Nguyen]

On Thu, Apr 21, 2016 at 2:20 PM, Eric Sunshine  wrote:
> On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy  
> wrote:
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>> diff --git a/worktree.c b/worktree.c
>> @@ -178,6 +182,18 @@ struct worktree **get_worktrees(void)
>> }
>> ALLOC_GROW(list, counter + 1, alloc);
>> list[counter] = NULL;
>> +
>> +   strbuf_addstr(_dir, absolute_path(get_git_dir()));
>> +   for (i = 0; i < counter; i++) {
>> +   struct worktree *wt = list[i];
>> +   strbuf_addstr(, 
>> absolute_path(get_worktree_git_dir(wt)));
>> +   wt->is_current = !strcmp_icase(git_dir.buf, path.buf);
>
> Can you talk a bit about why this uses 'icase'? Should it be
> respecting cache.h:ignore_case?

It does.That function (in dir.c) is just one-liner

return ignore_case ? strcasecmp(a, b) : strcmp(a, b);

I admit though, the naming does not make that clear.

>> +   strbuf_reset();
>> +   if (wt->is_current)
>> +   break;
>> +   }
>> +   strbuf_release(_dir);
>> +   strbuf_release();
>
> Minor: Would it make sense to place this new code in its own function
> -- say, mark_current_worktree() -- to keep get_worktrees() from
> becoming overlong?

Good idea. Will do.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] git-p4: add P4 jobs to git commit message

[Luke Diamand]

On 20 April 2016 at 16:51, Junio C Hamano  wrote:
> Luke Diamand  writes:
>
>> One thing I wondered about is whether this should be enabled by
>> default or not. Long-time users of git-p4 might be a bit surprised to
>> find their git commits suddenly gaining an extra Job: field.
>
> Ahh, I didn't even wonder about but that is not because I didn't
> think it matters.
>
> Does this change affect reproducibility of importing the history
> from P4, doesn't it?  Would that be a problem?

It would change the history created, but I don't see why that would be
a problem.

>
> How common is it to have the "extra" Job: thing in the history on P4
> side?

Where I work currently we don't use jobs (at present). Where I worked
before, jobs were created automatically to track issues in JIRA, and
were (supposed to be) entered into commits. It's potentially quite
useful so I guess might be quite widespread.

> If the answer to this question is "on rare occasions and only
> when there is a very good reason to have 'jobs' associated with the
> changelist", then the 'might be a bit surprised' brought by this
> change can probably be explained away as "a fix to a (design) bug
> that used to discard crucial information" that (unfortunately) have
> to change the resulting Git object names.
>

Luke
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/12] worktree.c: mark current worktree

[Eric Sunshine]

On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy  wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/worktree.c b/worktree.c
> @@ -178,6 +182,18 @@ struct worktree **get_worktrees(void)
> }
> ALLOC_GROW(list, counter + 1, alloc);
> list[counter] = NULL;
> +
> +   strbuf_addstr(_dir, absolute_path(get_git_dir()));
> +   for (i = 0; i < counter; i++) {
> +   struct worktree *wt = list[i];
> +   strbuf_addstr(, absolute_path(get_worktree_git_dir(wt)));
> +   wt->is_current = !strcmp_icase(git_dir.buf, path.buf);

Can you talk a bit about why this uses 'icase'? Should it be
respecting cache.h:ignore_case?

> +   strbuf_reset();
> +   if (wt->is_current)
> +   break;
> +   }
> +   strbuf_release(_dir);
> +   strbuf_release();

Minor: Would it make sense to place this new code in its own function
-- say, mark_current_worktree() -- to keep get_worktrees() from
becoming overlong?

> return list;
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git rebase -i without altering the committer date

[Johannes Schindelin]

Hi,

On Thu, 21 Apr 2016, Johannes Sixt wrote:

> Am 20.04.2016 um 23:47 schrieb Andreas Schwab:
> > Shaun Jackman  writes:
> >
> > > I'd like to insert a commit between two commits without changing
> > > the committer date or author date of that commit or the subsequent
> > > commits.
> >
> > The easiest way to implement that is to add a graft to redirect the
> > parent of the second commit to the inserted commit, then use git
> > filter-branch to make the graft permanent.
> 
> This only inserts a new project state, but does not propagate the changes
> brought in by the new commit to the subsequent commits. This propagation of
> changes could also be done with filter-branch, but it may be difficult
> depending on circumstances.

I agree that rebase -i is the wrong wrench for this job. Either use
filter-branch or fast-export/edit/fast-import.

Or take a step back and ask yourself why you need to fool anybody about
the commit date... ;-D

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 03/12] worktree.c: make find_shared_symref() return struct worktree *

[Eric Sunshine]

On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy  wrote:
> This gives the caller more information and they can answer things like,
> "is it the main worktree" or "is it the current worktree". The latter
> question is needed for the "checkout a rebase branch" case later.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/worktree.h b/worktree.h
> @@ -36,9 +36,10 @@ extern void free_worktrees(struct worktree **);
>  /*
>   * Check if a per-worktree symref points to a ref in the main worktree
>   * or any linked worktree, and return the path to the exising worktree

Doesn't "return the path" become outdated with this patch?

Also (not a new problem): s/exising/existing/

> - * if it is.  Returns NULL if there is no existing ref.  The caller is
> - * responsible for freeing the returned path.
> + * if it is. Returns NULL if there is no existing ref. The result
> + * may be destroyed by the next call.
>   */
> -extern char *find_shared_symref(const char *symref, const char *target);
> +extern const struct worktree *find_shared_symref(const char *symref,
> +const char *target);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html