Git development
 help / color / mirror / Atom feed
* [PATCH 2/3] object-store: factor out odb_clear_loose_cache()
From: René Scharfe @ 2019-01-06 16:45 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason
In-Reply-To: <3512c798-aa42-6fba-ee82-d33a8985be91@web.de>

Add and use a function for emptying the loose object cache, so callers
don't have to know any of its implementation details.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 object-store.h | 3 +++
 object.c       | 2 +-
 packfile.c     | 7 ++-----
 sha1-file.c    | 7 +++++++
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/object-store.h b/object-store.h
index 7236c571c0..709bf856b6 100644
--- a/object-store.h
+++ b/object-store.h
@@ -61,6 +61,9 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
 struct oid_array *odb_loose_cache(struct object_directory *odb,
 				  const struct object_id *oid);
 
+/* Empty the loose object cache for the specified object directory. */
+void odb_clear_loose_cache(struct object_directory *odb);
+
 struct packed_git {
 	struct packed_git *next;
 	struct list_head mru;
diff --git a/object.c b/object.c
index 79d636091c..a5c5cf830f 100644
--- a/object.c
+++ b/object.c
@@ -485,7 +485,7 @@ struct raw_object_store *raw_object_store_new(void)
 static void free_object_directory(struct object_directory *odb)
 {
 	free(odb->path);
-	oid_array_clear(&odb->loose_objects_cache);
+	odb_clear_loose_cache(odb);
 	free(odb);
 }
 
diff --git a/packfile.c b/packfile.c
index 8c6b47cc77..0fe9c21bf1 100644
--- a/packfile.c
+++ b/packfile.c
@@ -994,11 +994,8 @@ void reprepare_packed_git(struct repository *r)
 {
 	struct object_directory *odb;
 
-	for (odb = r->objects->odb; odb; odb = odb->next) {
-		oid_array_clear(&odb->loose_objects_cache);
-		memset(&odb->loose_objects_subdir_seen, 0,
-		       sizeof(odb->loose_objects_subdir_seen));
-	}
+	for (odb = r->objects->odb; odb; odb = odb->next)
+		odb_clear_loose_cache(odb);
 
 	r->objects->approximate_object_count_valid = 0;
 	r->objects->packed_git_initialized = 0;
diff --git a/sha1-file.c b/sha1-file.c
index cb8583b634..2f965b2688 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2178,6 +2178,13 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 	strbuf_release(&buf);
 }
 
+void odb_clear_loose_cache(struct object_directory *odb)
+{
+	oid_array_clear(&odb->loose_objects_cache);
+	memset(&odb->loose_objects_subdir_seen, 0,
+	       sizeof(odb->loose_objects_subdir_seen));
+}
+
 static int check_stream_sha1(git_zstream *stream,
 			     const char *hdr,
 			     unsigned long size,
-- 
2.20.1

^ permalink raw reply related

* [PATCH 1/3] object-store: factor out odb_loose_cache()
From: René Scharfe @ 2019-01-06 16:45 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason
In-Reply-To: <3512c798-aa42-6fba-ee82-d33a8985be91@web.de>

Add and use a function for loading the entries if a loose object
subdirectory for a given object ID.  It frees callers from deriving the
fanout key; they can use the returned oid_array reference for lookups or
forward range scans.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 object-store.h |  7 +++++++
 sha1-file.c    | 12 +++++++++---
 sha1-name.c    | 10 +++++-----
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/object-store.h b/object-store.h
index 60758efad8..7236c571c0 100644
--- a/object-store.h
+++ b/object-store.h
@@ -54,6 +54,13 @@ void add_to_alternates_memory(const char *dir);
  */
 void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
 
+/*
+ * Populate and return the loose object cache array corresponding to the
+ * given object ID.
+ */
+struct oid_array *odb_loose_cache(struct object_directory *odb,
+				  const struct object_id *oid);
+
 struct packed_git {
 	struct packed_git *next;
 	struct list_head mru;
diff --git a/sha1-file.c b/sha1-file.c
index 5a272f70de..cb8583b634 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -924,7 +924,6 @@ static int open_sha1_file(struct repository *r,
 static int quick_has_loose(struct repository *r,
 			   const unsigned char *sha1)
 {
-	int subdir_nr = sha1[0];
 	struct object_id oid;
 	struct object_directory *odb;
 
@@ -932,8 +931,7 @@ static int quick_has_loose(struct repository *r,
 
 	prepare_alt_odb(r);
 	for (odb = r->objects->odb; odb; odb = odb->next) {
-		odb_load_loose_cache(odb, subdir_nr);
-		if (oid_array_lookup(&odb->loose_objects_cache, &oid) >= 0)
+		if (oid_array_lookup(odb_loose_cache(odb, &oid), &oid) >= 0)
 			return 1;
 	}
 	return 0;
@@ -2152,6 +2150,14 @@ static int append_loose_object(const struct object_id *oid, const char *path,
 	return 0;
 }
 
+struct oid_array *odb_loose_cache(struct object_directory *odb,
+				  const struct object_id *oid)
+{
+	int subdir_nr = oid->hash[0];
+	odb_load_loose_cache(odb, subdir_nr);
+	return &odb->loose_objects_cache;
+}
+
 void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 {
 	struct strbuf buf = STRBUF_INIT;
diff --git a/sha1-name.c b/sha1-name.c
index b24502811b..a656481c6a 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -87,21 +87,21 @@ static int match_sha(unsigned, const unsigned char *, const unsigned char *);
 
 static void find_short_object_filename(struct disambiguate_state *ds)
 {
-	int subdir_nr = ds->bin_pfx.hash[0];
 	struct object_directory *odb;
 
 	for (odb = the_repository->objects->odb;
 	     odb && !ds->ambiguous;
 	     odb = odb->next) {
 		int pos;
+		struct oid_array *loose_objects;
 
-		odb_load_loose_cache(odb, subdir_nr);
-		pos = oid_array_lookup(&odb->loose_objects_cache, &ds->bin_pfx);
+		loose_objects = odb_loose_cache(odb, &ds->bin_pfx);
+		pos = oid_array_lookup(loose_objects, &ds->bin_pfx);
 		if (pos < 0)
 			pos = -1 - pos;
-		while (!ds->ambiguous && pos < odb->loose_objects_cache.nr) {
+		while (!ds->ambiguous && pos < loose_objects->nr) {
 			const struct object_id *oid;
-			oid = odb->loose_objects_cache.oid + pos;
+			oid = loose_objects->oid + pos;
 			if (!match_sha(ds->len, ds->bin_pfx.hash, oid->hash))
 				break;
 			update_candidates(ds, oid);
-- 
2.20.1

^ permalink raw reply related

* jk/loose-object-cache
From: René Scharfe @ 2019-01-06 16:39 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqh8explya.fsf@gitster-ct.c.googlers.com>

Am 28.12.2018 um 19:04 schrieb Junio C Hamano:
> * jk/loose-object-cache (2018-11-24) 10 commits
>   (merged to 'next' on 2018-12-28 at 5a5faf384e)
>  + odb_load_loose_cache: fix strbuf leak
>  + fetch-pack: drop custom loose object cache
>  + sha1-file: use loose object cache for quick existence check
>  + object-store: provide helpers for loose_objects_cache
>  + sha1-file: use an object_directory for the main object dir
>  + handle alternates paths the same as the main object dir
>  + sha1_file_name(): overwrite buffer instead of appending
>  + rename "alternate_object_database" to "object_directory"
>  + submodule--helper: prefer strip_suffix() to ends_with()
>  + fsck: do not reuse child_process structs
> 
>  Originally merged to 'next' on 2018-11-24
> 
>  Code clean-up with optimization for the codepath that checks
>  (non-)existence of loose objects.
> 
>  Will merge to 'master'.

So this has hit master in the meantime.  We discussed a sort performance
fix in [1]; I'll reply with a short series containing a cleaned-up and
rebased version as a follow-up.

  object-store: factor out odb_loose_cache()
  object-store: factor out odb_clear_loose_cache()
  object-store: use one oid_array per subdirectory for loose cache

 object-store.h | 12 +++++++++++-
 object.c       |  2 +-
 packfile.c     |  7 ++-----
 sha1-file.c    | 24 ++++++++++++++++++++----
 sha1-name.c    | 10 +++++-----
 5 files changed, 39 insertions(+), 16 deletions(-)

[1] https://public-inbox.org/git/221cb2e4-a024-e301-2b3f-e37dcd93795e@web.de/

^ permalink raw reply

* [PATCH v3 4/4] t3506: validate '-m 1 -ff' is now accepted for non-merge commits
From: Sergey Organov @ 2018-12-14  4:53 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <cover.1546789223.git.sorganov@gmail.com>

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 t/t3506-cherry-pick-ff.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh
index fb889ac..127dd00 100755
--- a/t/t3506-cherry-pick-ff.sh
+++ b/t/t3506-cherry-pick-ff.sh
@@ -64,10 +64,10 @@ test_expect_success 'merge setup' '
 	git checkout -b new A
 '
 
-test_expect_success 'cherry-pick a non-merge with --ff and -m should fail' '
+test_expect_success 'cherry-pick explicit first parent of a non-merge with --ff' '
 	git reset --hard A -- &&
-	test_must_fail git cherry-pick --ff -m 1 B &&
-	git diff --exit-code A --
+	git cherry-pick --ff -m 1 B &&
+	git diff --exit-code C --
 '
 
 test_expect_success 'cherry pick a merge with --ff but without -m should fail' '
-- 
2.10.0.1.g57b01a3


^ permalink raw reply related

* [PATCH v3 3/4] t3502: validate '-m 1' argument is now accepted for non-merge commits
From: Sergey Organov @ 2018-12-14  4:53 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <cover.1546789223.git.sorganov@gmail.com>

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 t/t3502-cherry-pick-merge.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh
index b160271..8b635a1 100755
--- a/t/t3502-cherry-pick-merge.sh
+++ b/t/t3502-cherry-pick-merge.sh
@@ -40,12 +40,12 @@ test_expect_success 'cherry-pick -m complains of bogus numbers' '
 	test_expect_code 129 git cherry-pick -m 0 b
 '
 
-test_expect_success 'cherry-pick a non-merge with -m should fail' '
+test_expect_success 'cherry-pick explicit first parent of a non-merge' '
 
 	git reset --hard &&
 	git checkout a^0 &&
-	test_expect_code 128 git cherry-pick -m 1 b &&
-	git diff --exit-code a --
+	git cherry-pick -m 1 b &&
+	git diff --exit-code c --
 
 '
 
@@ -84,12 +84,12 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent should f
 
 '
 
-test_expect_success 'revert a non-merge with -m should fail' '
+test_expect_success 'revert explicit first parent of a non-merge' '
 
 	git reset --hard &&
 	git checkout c^0 &&
-	test_must_fail git revert -m 1 b &&
-	git diff --exit-code c
+	git revert -m 1 b &&
+	git diff --exit-code a --
 
 '
 
-- 
2.10.0.1.g57b01a3


^ permalink raw reply related

* [PATCH v3 2/4] cherry-pick: do not error on non-merge commits when '-m 1' is specified
From: Sergey Organov @ 2018-12-14  4:53 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <cover.1546789223.git.sorganov@gmail.com>

When cherry-picking multiple commits, it's impossible to have both
merge- and non-merge commits on the same command-line. Not specifying
'-m 1' results in cherry-pick refusing to handle merge commits, while
specifying '-m 1' fails on non-merge commits.

This patch allows '-m 1' for non-merge commits. As mainline is always
the only parent for a non-merge commit, it makes little sense to
disable it.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 sequencer.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index e1a4dd1..d0fd61b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1766,9 +1766,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 			return error(_("commit %s does not have parent %d"),
 				oid_to_hex(&commit->object.oid), opts->mainline);
 		parent = p->item;
-	} else if (0 < opts->mainline)
-		return error(_("mainline was specified but commit %s is not a merge."),
-			oid_to_hex(&commit->object.oid));
+	} else if (1 < opts->mainline)
+		/*
+		 *  Non-first parent explicitly specified as mainline for
+		 *  non-merge commit
+		 */
+		return error(_("commit %s does not have parent %d"),
+			     oid_to_hex(&commit->object.oid), opts->mainline);
 	else
 		parent = commit->parents->item;
 
-- 
2.10.0.1.g57b01a3


^ permalink raw reply related

* [PATCH v3 1/4] t3510: stop using '-m 1' to force failure mid-sequence of cherry-picks
From: Sergey Organov @ 2018-12-14  4:53 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <cover.1546789223.git.sorganov@gmail.com>

We are going to allow 'git cherry-pick -m 1' for non-merge commits, so
this method to force failure will stop to work.

Use '-m 4' instead as it's very unlikely we will ever have such an
octopus in this test setup.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 t/t3510-cherry-pick-sequence.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index c84eeef..941d502 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -61,7 +61,11 @@ test_expect_success 'cherry-pick mid-cherry-pick-sequence' '
 
 test_expect_success 'cherry-pick persists opts correctly' '
 	pristine_detach initial &&
-	test_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours initial..anotherpick &&
+	# to make sure that the session to cherry-pick a sequence
+	# gets interrupted, use a high-enough number that is larger
+	# than the number of parents of any commit we have created
+	mainline=4 &&
+	test_expect_code 128 git cherry-pick -s -m $mainline --strategy=recursive -X patience -X ours initial..anotherpick &&
 	test_path_is_dir .git/sequencer &&
 	test_path_is_file .git/sequencer/head &&
 	test_path_is_file .git/sequencer/todo &&
@@ -69,7 +73,7 @@ test_expect_success 'cherry-pick persists opts correctly' '
 	echo "true" >expect &&
 	git config --file=.git/sequencer/opts --get-all options.signoff >actual &&
 	test_cmp expect actual &&
-	echo "1" >expect &&
+	echo "$mainline" >expect &&
 	git config --file=.git/sequencer/opts --get-all options.mainline >actual &&
 	test_cmp expect actual &&
 	echo "recursive" >expect &&
-- 
2.10.0.1.g57b01a3


^ permalink raw reply related

* [PATCH v3 0/4] Allow 'cherry-pick -m 1' for non-merge commits
From: Sergey Organov @ 2018-12-14  4:39 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <xmqq5zvwesvz.fsf@gitster-ct.c.googlers.com>

Change from v2: t/t3502: disambiguation added to prevent failing on
case-insensitive file systems.

When cherry-picking multiple commits, it's impossible to have both
merge- and non-merge commits on the same command-line. Not specifying
'-m 1' results in cherry-pick refusing to handle merge commits, while
specifying '-m 1' fails on non-merge commits.

This patch series allow '-m 1' for non-merge commits as well as fixes
relevant tests in accordance.

It also opens the way to making '-m 1' the default, that would be
inline with the trends to assume first parent to be the mainline in
most workflows.

Sergey Organov (4):
  t3510: stop using '-m 1' to force failure mid-sequence of cherry-picks
  cherry-pick: do not error on non-merge commits when '-m 1' is
    specified
  t3502: validate '-m 1' argument is now accepted for non-merge commits
  t3506: validate '-m 1 -ff' is now accepted for non-merge commits

 sequencer.c                     | 10 +++++++---
 t/t3502-cherry-pick-merge.sh    | 12 ++++++------
 t/t3506-cherry-pick-ff.sh       |  6 +++---
 t/t3510-cherry-pick-sequence.sh |  8 ++++++--
 4 files changed, 22 insertions(+), 14 deletions(-)

-- 
2.10.0.1.g57b01a3


^ permalink raw reply

* [PATCH] helper/test-ref-store: fix "new-sha1" vs "old-sha1" typo
From: Christian Couder @ 2019-01-06 15:46 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Duy Nguyen, Christian Couder

It looks like it is a copy-paste error  made in 80f2a6097c
(t/helper: add test-ref-store to test ref-store functions,
2017-03-26) to pass "old-sha1" instead of "new-sha1" to
notnull() when we get the new sha1 argument from
const char **argv.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/helper/test-ref-store.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index e9e0541276..799fc00aa1 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -233,7 +233,7 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv)
 {
 	const char *msg = notnull(*argv++, "msg");
 	const char *refname = notnull(*argv++, "refname");
-	const char *new_sha1_buf = notnull(*argv++, "old-sha1");
+	const char *new_sha1_buf = notnull(*argv++, "new-sha1");
 	const char *old_sha1_buf = notnull(*argv++, "old-sha1");
 	unsigned int flags = arg_flags(*argv++, "flags");
 	struct object_id old_oid;
-- 
2.20.1.26.gc246996f60


^ permalink raw reply related

* Re: [PATCH v2 3/4] t3502: validate '-m 1' argument is now accepted for non-merge commits
From: Sergey Organov @ 2019-01-06 14:41 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: git, gitster
In-Reply-To: <20190103172221.GB4673@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Fri, Dec 14, 2018 at 07:53:51AM +0300, Sergey Organov wrote:
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>

[...]

>>  
>> @@ -84,12 +84,12 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent should f
>>  
>>  '
>>  
>> -test_expect_success 'revert a non-merge with -m should fail' '
>> +test_expect_success 'revert explicit first parent of a non-merge' '
>>  
>>  	git reset --hard &&
>>  	git checkout c^0 &&
>> -	test_must_fail git revert -m 1 b &&
>> -	git diff --exit-code c
>> +	git revert -m 1 b &&
>> +	git diff --exit-code a
>
> You need disambiguaion here, otherwise this test fails on
> case-insensitive file systems:
>
>   ++git diff --exit-code a
>   fatal: ambiguous argument 'a': both revision and filename
>   Use '--' to separate paths from revisions, like this:
>   'git <command> [<revision>...] -- [<file>...]'
>   error: last command exited with $?=128
>   not ok 8 - revert explicit first parent of a non-merge

Good catch, -- thanks a lot!

-- Sergey

^ permalink raw reply

* Re: [PATCH v2] worktree: allow to (re)move worktrees with uninitialized submodules
From: Eric Sunshine @ 2019-01-06  7:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, Git List
In-Reply-To: <xmqqftu8avfu.fsf@gitster-ct.c.googlers.com>

On Fri, Jan 4, 2019 at 5:51 PM Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> > Uninitialized submodules have nothing valueable for us to be worried
> > about. They are just SHA-1. Let "worktree remove" and "worktree move"
> > continue in this case so that people can still use multiple worktrees
> > on repos with optional submodules that are never populated, like
> > sha1collisiondetection in git.git when checked out by doc-diff script.
>
> Is this a fair description for this 1-patch topic?
>
>         "git worktree remove" and "git worktree move" failed to work
>         when there is an uninitialized submodule, which has been fixed.

Saying "failed to work" makes it sound as if those two subcommands are
broken or buggy, which is not the case. Instead, they are overly
cautious and refuse to allow the operation if _any_ submodule
(initialized or not) is detected. This patch just loosens that check
to allow the operations to succeed for uninitialized submodules, thus
avoiding this (existing) annoying behavior, using git.git as an
example:

    % cd git
    % git worktree add --detach ../foo
    % git worktree remove ../foo
    fatal: working trees containing submodules cannot be moved or removed

Perhaps the change could instead be summarized as:

    "git worktree remove" and "git worktree move" were overly
    cautious, refusing to operate if any submodule was detected, even
    an uninitialized one. This safeguard has been refined to take only
    initialized submodules into account since worktrees with
    uninitialized ones can be safely removed and moved.

^ permalink raw reply

* Re: A few questions regarding git annotated tags
From: Jeff King @ 2019-01-06  6:50 UTC (permalink / raw)
  To: Michal Novotny; +Cc: git
In-Reply-To: <CANT8FXRRTpAaW0JxYCt94f52eKAz1cBAGpPA84CUTMJUgQrkuw@mail.gmail.com>

On Sat, Jan 05, 2019 at 04:50:30PM +0100, Michal Novotny wrote:

> I could potentially make it so that I tag subtrees instead of commits
> and then derive the needed information from these subtree tags. This
> could be useful if I have multiple rpm packages in different subtrees
> of the same repo. I could then tag the subtree where the rpm package
> is placed.
> 
> This could bring some simplification into the code but as far as I
> know, you cannot easily checkout a tree tag, which is something a
> packager should be able to do easily: to checkout a state of repo when
> a certain subpackage was tagged. This is the first question. Can you
> e.g. do:
> 
> git tag somename HEAD:
> 
> and then do something similar to
> 
> git checkout somename
> 
> which would restore the repository or at least the respective subtree
> of it into the state when "somename" tag was created?

No, there's no easy way to check out a bare tree (and in fact, HEAD is
forbidden to point to a non-commit).

You could hack around that by making a new commit that wraps the tree,
like:

  commit=$(echo 'wrap foo package' | git commit-tree HEAD:foo)
  git tag foo-1.2.3 $commit

There are also useful things to do with the tag of the bare tree. E.g.,
export it via git-archive, diff it, "git checkout -p" changes from it,
etc. But actually creating a working tree state from it is awkward:

  # move to being on an "unborn" branch foo
  git checkout --orphan foo

  # load the desired tree state; "-u" will update the working tree
  # files
  git read-tree -m -u foo-1.2.3

  # if we were to commit now, it would become the root commit of the
  # "foo" branch, with no parents. That would make it pretty useful for
  # things like merging between tags.
  git commit -m 'kind of weird'

So it seems kind of awkward and useless.  I'm not sure I totally
understand your problem space, but if you can have actual commits with a
logical progression (i.e., where the parent links actually mean
something), I think Git's tools will be more useful.

> Right now, I am putting a package name directly into tag name so I
> know what tags belong to what package based on that. And I am using
> normal annotated tags. This works quite well, I would say, but at one
> point I need to use shared state to move the discovered package name
> from one part of the code to another so that the other part can work
> with the correct subset of the available annotated tags. I wouldn't
> need to do that if I could derive the correct tag subset based just on
> the path to the subtree where a package is placed.

I'm not sure I understand this bit. Even if you tag a subtree, like:

  git tag foo-1.2.3 HEAD:foo

then that tree doesn't "know" that it was originally at the path "foo".
You'd have to tag the root tree, and then know to look in the "foo"
subtree from there. At which point you might as well tag the commit that
contains that root tree. Whether it happens to touch the "foo" path or
not, it represents a particular state.

> Alternative approach to creating the tree tags would be to store the
> path information into annotated tag message, which I could do. But is
> there a relatively simple way to filter tags based on their message
> content? Can I put the information into some other part of tag than
> name or the message so that it can be easily filtered?

I don't think there's an easy way to show only tags matching a pattern.
You could do something like:

  git tag -m 'path: foo' foo

  git for-each-ref --format='%(refname:strip=2) %(subject)' refs/tags/ |
  grep 'path: foo' |
  awk '{print $1}'

to grep their subjects (or body, if you want to make the grep stage a
little more clever). Obviously that is not really a structured lookup,
but if you control the tag contents, it might be OK.

In commit messages there's a concept of machine-readable trailers, like
"Signed-off-by", etc, and even some tools for displaying those. But
there's not currently any support for parsing them out of tag objects.


I sort of answered your questions literally, but TBH I'm still not
entirely sure what you're trying to accomplish. So hopefully it was
useful, but feel free to follow up with more questions. ;)

-Peff

^ permalink raw reply

* Re: [PATCH v3] Simplify handling of setup_git_directory_gently() failure cases.
From: Jeff King @ 2019-01-06  6:22 UTC (permalink / raw)
  To: Erin Dahlgren; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <CAP_Smy0Phh83XW-m0_cGg6WuLpDySshP7Ys2OrD8no1hJzohpg@mail.gmail.com>

On Sat, Jan 05, 2019 at 08:57:32AM -0800, Erin Dahlgren wrote:

> > So what next? Erin, are you interested in using the details of this
> > conversation to take the cleanups a bit further?
> 
> Sure, no problem. If this is urgent, then I would probably be more
> inclined to keep this small and do more cleanup in followup patches.
> But if it's not urgent (that is my understanding), I'd be happy to
> take the cleanups further. I'm traveling today through next week but
> I'll try to post another version addressing the comments in the next
> couple of days.

Nope, I don't think it's urgent at all.  I just didn't want to dump work
on you if you were losing interest in the topic. :)

-Peff

^ permalink raw reply

* Re: [PATCH 1/3] Add 'human' date format
From: Jeff King @ 2019-01-06  6:19 UTC (permalink / raw)
  To: Stephen P Smith
  Cc: git, Linus Torvalds, Junio C Hamano,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <4462659.Bys67ThUBR@thunderbird>

On Fri, Jan 04, 2019 at 06:03:18AM -0700, Stephen P Smith wrote:

> On Friday, January 4, 2019 12:50:35 AM MST Jeff King wrote:
> > On Thu, Jan 03, 2019 at 06:19:56AM -0700, Stephen P. Smith wrote:
> > > 
> > > I didn't see anything in the code which would prohibit setting something
> > > like that.
> > 
> > Yeah, I don't think supporting that is too hard. I was thinking
> > something like this:
> 
> I take it that if I update Linus's patch, I still keep Junio's and Linus' 
> sign-off line for the purpose of the chain of custody?  Of should I use a 
> second patch?

I think the most interesting question is the actual authorship (i.e.,
the "From:" field).  I think people are generally OK with having their
patches polished a bit to fix obvious bugs or short-comings. But at some
point if you make too many changes they or may not want to have the
result attributed to them. ;)

For the particular change I suggested, it's borderline to me on whether
it hits that case, so I'd probably err on the side of caution. And I'd
either expect Linus to say "yeah, that sounds like a good direction", or
I'd do it as a separate patch. And if a separate patch, I'd probably
tease Linus's patch out into two separate ones: one to add "human", and
one to implement "auto". And then drop the "auto" one in favor of your
new patch (with you as the author).

And I think that makes the signoff questions go away for this instance
(keep the signoffs for Linus's, and just signoff the new patch
yourself). But here's some general pontificating in that direction:

    Normally you can just drop Junio's signoff. The chain of custody is
    usually "author, then maintainer" and he'll re-add his maintainer
    signoff when he picks up your patch. In this case of this patch it's
    "author, then polisher, then maintainer", but Junio is still at the
    end.

    Now one can argue that Junio picked up Linus's patch, which you then
    picked up from Junio's repository and fed back to Junio. But you
    could just as well have picked Linus's patch up from the mailing
    list and then polished it. So I don't know that having Junio twice
    in the chain is really that interesting.

    Generally, yes, I'd keep Linus's signoff in a situation like this.
    He is asserting that the original work done meets the DCO
    requirements. You polishing the patch does not change that (of
    course you could introduce a bunch of new code that doesn't meet the
    DCO and sign it off anyway, but that's why there's ordering in the
    chain of custody. Somebody investigating would probably walk
    backwards up the chain).

-Peff

^ permalink raw reply

* Re: Git extra hook, pre-upload
From: Jeff King @ 2019-01-06  6:07 UTC (permalink / raw)
  To: Xheroz 128; +Cc: git
In-Reply-To: <A842150D-2A45-4ABB-9B0F-DF516F7A2737@gmail.com>

[re-add git@vger cc]

On Sat, Jan 05, 2019 at 01:47:11PM +0900, Xheroz 128 wrote:

> Thank you for all your replies. I have read the thread from 2011 and
> understand the reason of the insecure part using pre and post upload
> hooks.
> But for my project I would still like to use pre-upload hook. But I do
> not know how to enable the ALLOW_INSECURE_HOOKS in my system. How to
> configure it?

We never actually merged that patch, so there is no such build-time knob
in Git. I.e., you can either use the uploadpack.packObjectsHook feature,
or you will have to make a custom patch yourself.

-Peff

^ permalink raw reply

* Re: How DELTA objects values work and are calculated
From: Duy Nguyen @ 2019-01-06  2:32 UTC (permalink / raw)
  To: Farhan Khan; +Cc: Git Mailing List
In-Reply-To: <581076b0-95c5-9af9-dec5-5a9bccfe2634@gmail.com>

On Sun, Jan 6, 2019 at 5:32 AM Farhan Khan <khanzf@gmail.com> wrote:
> Hi Duy,
>
> Thanks for explaining the Delta objects.
>
> What does a OBJ_REF_DELTA object itself consist of?

from pack-format.txt

     (deltified representation)
     n-byte type and length (3-bit type, (n-1)*7+4-bit length)
     20-byte base object name if OBJ_REF_DELTA or a negative relative
offset from the delta object's position in the pack if this
is an OBJ_OFS_DELTA object
     compressed delta data


> Do you have to uncompress it to parse its values?

The delta part is compressed, so yes. The "base object name" is not.

> How do you get its size?

Uncompress until the end the delta until the end. zlib stream has some
sort of "end-of-stream" marker so it knows when to stop.

> I read through resolve deltas which leads to threaded_second_pass, where
> you suggested to start, but I do not understand what is happening at a
> high level and get confused while reading the code.
>
>  From threaded_second_pass, execution goes into a for-loop that runs
> resolve_base(), which runs runs find_unresolved_deltas(). Is this
> finding the unresolved deltas of the current object (The current
> OBJ_REF_DELTA we are going through)? This then runs
> find_unresolved_deltas() and shortly afterwards
> find_unresolved_deltas_1(). It seems that find_unresolved_deltas_1() is
> applying deltas, but I am not certain.

Ah I forgot how "fun" these functions were :) The obvious way to
resolve an delta object is to resolve (recursively) its base object
first, then you apply delta on top and are done. However that implies
recursion, and also not really cache friendly. So what
find_unresolve_deltas_1() does is backward. It starts at a (already
resolved, e.g. non-delta) base object, then applies deltas for all
delta objects that immediately depend on it, then continue to resolve
delta objects depending on these children... The
find_*_delta_children() functions find these deltas, then
find_unresolve_deltas_1() will call resolve_delta() to do the real
work

- the delta type (OBJ_REF_.. or OBJ_OFS_...) is already known at this
point. I believe we know from the first pass
- the delta is uncompressed here, with get_data_from_pack()
- the base object is obtained via get_base_data(), which is recursive,
but since we go backwards from parent to child, base->data should be
already valid and get_base_data() becomes no-op

> I do not understand what is happening in any of these functions. There
> are some comments on builtin/index-pack.c:883-904
>
> Overall, I do not understand this entire process, what values to capture
> along the way, and how they are consumed. Please provide some guidance
> on how this process works.

An easier way to understand this is actually run it through a debugger
(in single thread mode). Create a small repo with a handful of deltas.
Use "git verify-pack -v" to see what object is delta and where... then
you have something to double check while you step through the code.

>
> Thank you!
> Farhan
-- 
Duy

^ permalink raw reply

* [PATCH v5 3/3] branch: Add an extra verbose output displaying worktree path for refs checked out in a linked worktree
From: nbelakovski @ 2019-01-06  0:26 UTC (permalink / raw)
  To: git; +Cc: peff, rafa.almas, gitster, avarab, Nickolai Belakovski
In-Reply-To: <CAC05386q2iGoiJ_fRgwoOTF23exEN2D1+oh4VjajEvYQ58O1TQ@mail.gmail.com>

From: Nickolai Belakovski <nbelakovski@gmail.com>

---
 Documentation/git-branch.txt | 5 ++++-
 builtin/branch.c             | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index b3eca6ffdc..6d1fc59e32 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -163,12 +163,15 @@ This option is only applicable in non-verbose mode.
 
 -v::
 -vv::
+-vvv::
 --verbose::
 	When in list mode,
 	show sha1 and commit subject line for each head, along with
 	relationship to upstream branch (if any). If given twice, print
 	the name of the upstream branch, as well (see also `git remote
-	show <remote>`).
+	show <remote>`). If given 3 times, print the path of the linked
+	worktree, if applicable (not applicable for main worktree since
+	its path will be a subset of $PWD)
 
 -q::
 --quiet::
diff --git a/builtin/branch.c b/builtin/branch.c
index 2a24153b78..56589a3684 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -366,6 +366,10 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
 		strbuf_addstr(&local, branch_get_color(BRANCH_COLOR_RESET));
 		strbuf_addf(&local, " %s ", obname.buf);
 
+		if (filter->verbose > 2)
+			strbuf_addf(&local, "%s%%(if:notequals=*)%%(HEAD)%%(then)%%(if)%%(worktreepath)%%(then)%%(worktreepath) %%(end)%%(end)%s",
+				    branch_get_color(BRANCH_COLOR_WORKTREE), branch_get_color(BRANCH_COLOR_RESET));
+
 		if (filter->verbose > 1)
 			strbuf_addf(&local, "%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)"
 				    "%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)",
-- 
2.14.2


^ permalink raw reply related

* [PATCH v5 2/3] branch: Mark and color a branch differently if it is checked out in a linked worktree
From: nbelakovski @ 2019-01-06  0:26 UTC (permalink / raw)
  To: git; +Cc: peff, rafa.almas, gitster, avarab, Nickolai Belakovski
In-Reply-To: <CAC05386q2iGoiJ_fRgwoOTF23exEN2D1+oh4VjajEvYQ58O1TQ@mail.gmail.com>

From: Nickolai Belakovski <nbelakovski@gmail.com>

In order to more clearly display which branches are active, the output
of git branch is modified to mark branches checkout out in a linked
worktree with a "+" and color them in cyan (in contrast to the current
branch, which will still be denoted with a "*" and colored in green)

This is meant to simplify workflows related to worktree, particularly
due to the limitations of not being able to check out the same branch in
two worktrees and the inability to delete a branch checked out in a
worktree. When performing branch operations like checkout and delete, it
would be useful to know more readily if the branches in which the user
is interested are already checked out in a worktree.

The git worktree list command contains the relevant information, however
this is a much less frquently used command than git branch.
---
 Documentation/git-branch.txt | 15 ++++++++-------
 builtin/branch.c             | 12 ++++++++----
 t/t3200-branch.sh            |  8 ++++----
 t/t3203-branch-output.sh     | 21 +++++++++++++++++++++
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index bf5316ffa9..b3eca6ffdc 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -26,13 +26,14 @@ DESCRIPTION
 -----------
 
 If `--list` is given, or if there are no non-option arguments, existing
-branches are listed; the current branch will be highlighted with an
-asterisk.  Option `-r` causes the remote-tracking branches to be listed,
-and option `-a` shows both local and remote branches. If a `<pattern>`
-is given, it is used as a shell wildcard to restrict the output to
-matching branches. If multiple patterns are given, a branch is shown if
-it matches any of the patterns.  Note that when providing a
-`<pattern>`, you must use `--list`; otherwise the command is interpreted
+branches are listed; the current branch will be highlighted in green and
+marked with an asterisk.  Any branches checked out in linked worktrees will
+be highlighted in cyan and marked with a plus sign. Option `-r` causes the
+remote-tracking branches to be listed, and option `-a` shows both local and
+remote branches. If a `<pattern>` is given, it is used as a shell wildcard to
+restrict the output to matching branches. If multiple patterns are given, a
+branch is shown if it matches any of the patterns.  Note that when providing
+a `<pattern>`, you must use `--list`; otherwise the command is interpreted
 as branch creation.
 
 With `--contains`, shows only the branches that contain the named commit
diff --git a/builtin/branch.c b/builtin/branch.c
index 0c55f7f065..2a24153b78 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -47,6 +47,7 @@ static char branch_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_NORMAL,       /* LOCAL */
 	GIT_COLOR_GREEN,        /* CURRENT */
 	GIT_COLOR_BLUE,         /* UPSTREAM */
+	GIT_COLOR_CYAN,         /* WORKTREE */
 };
 enum color_branch {
 	BRANCH_COLOR_RESET = 0,
@@ -54,7 +55,8 @@ enum color_branch {
 	BRANCH_COLOR_REMOTE = 2,
 	BRANCH_COLOR_LOCAL = 3,
 	BRANCH_COLOR_CURRENT = 4,
-	BRANCH_COLOR_UPSTREAM = 5
+	BRANCH_COLOR_UPSTREAM = 5,
+	BRANCH_COLOR_WORKTREE = 6
 };
 
 static const char *color_branch_slots[] = {
@@ -64,6 +66,7 @@ static const char *color_branch_slots[] = {
 	[BRANCH_COLOR_LOCAL]	= "local",
 	[BRANCH_COLOR_CURRENT]	= "current",
 	[BRANCH_COLOR_UPSTREAM] = "upstream",
+	[BRANCH_COLOR_WORKTREE] = "worktree",
 };
 
 static struct string_list output = STRING_LIST_INIT_DUP;
@@ -342,9 +345,10 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
 	struct strbuf local = STRBUF_INIT;
 	struct strbuf remote = STRBUF_INIT;
 
-	strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %s%%(end)",
-		    branch_get_color(BRANCH_COLOR_CURRENT),
-		    branch_get_color(BRANCH_COLOR_LOCAL));
+	strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else)%%(if)%%(worktreepath)%%(then)+ %s%%(else)  %s%%(end)%%(end)",
+			branch_get_color(BRANCH_COLOR_CURRENT),
+			branch_get_color(BRANCH_COLOR_WORKTREE),
+			branch_get_color(BRANCH_COLOR_LOCAL));
 	strbuf_addf(&remote, "  %s",
 		    branch_get_color(BRANCH_COLOR_REMOTE));
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 478b82cf9b..e404f6e23c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -292,7 +292,7 @@ test_expect_success 'git branch --list -v with --abbrev' '
 test_expect_success 'git branch --column' '
 	COLUMNS=81 git branch --column=column >actual &&
 	cat >expected <<\EOF &&
-  a/b/c     bam       foo       l       * master    n         o/p       r
+  a/b/c   + bam       foo       l       * master    n         o/p       r
   abc       bar       j/k       m/m       master2   o/o       q
 EOF
 	test_cmp expected actual
@@ -307,7 +307,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '
 	cat >expected <<EOF &&
   a/b/c
   abc
-  bam
++ bam
   bar
   foo
   j/k
@@ -332,7 +332,7 @@ test_expect_success 'git branch with column.*' '
 	git config --unset column.branch &&
 	git config --unset column.ui &&
 	cat >expected <<\EOF &&
-  a/b/c   bam   foo   l   * master    n     o/p   r
+  a/b/c + bam   foo   l   * master    n     o/p   r
   abc     bar   j/k   m/m   master2   o/o   q
 EOF
 	test_cmp expected actual
@@ -349,7 +349,7 @@ test_expect_success 'git branch -v with column.ui ignored' '
 	cat >expected <<\EOF &&
   a/b/c
   abc
-  bam
++ bam
   bar
   foo
   j/k
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index ee6787614c..94ab05ad59 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -240,6 +240,27 @@ test_expect_success 'git branch --format option' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success '"add" a worktree' '
+	mkdir worktree_dir &&
+	git worktree add -b master_worktree worktree_dir master
+'
+
+cat >expect <<'EOF'
+* <GREEN>(HEAD detached from fromtag)<RESET>
+  ambiguous<RESET>
+  branch-one<RESET>
+  branch-two<RESET>
+  master<RESET>
++ <CYAN>master_worktree<RESET>
+  ref-to-branch<RESET> -> branch-one
+  ref-to-remote<RESET> -> origin/branch-one
+EOF
+test_expect_success TTY 'worktree colors correct' '
+	test_terminal git branch >actual.raw &&
+	test_decode_color <actual.raw >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success "set up color tests" '
 	echo "<RED>master<RESET>" >expect.color &&
 	echo "master" >expect.bare &&
-- 
2.14.2


^ permalink raw reply related

* [PATCH v5 1/3] ref-filter: add worktreepath atom
From: nbelakovski @ 2019-01-06  0:26 UTC (permalink / raw)
  To: git; +Cc: peff, rafa.almas, gitster, avarab, Nickolai Belakovski
In-Reply-To: <CAC05386q2iGoiJ_fRgwoOTF23exEN2D1+oh4VjajEvYQ58O1TQ@mail.gmail.com>

From: Nickolai Belakovski <nbelakovski@gmail.com>

Add an atom providing the path of the linked worktree where this ref is
checked out, if it is checked out in any linked worktrees, and empty
string otherwise.
---
 Documentation/git-for-each-ref.txt |  5 +++
 ref-filter.c                       | 71 ++++++++++++++++++++++++++++++++++++++
 t/t6302-for-each-ref-filter.sh     | 15 ++++++++
 3 files changed, 91 insertions(+)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 901faef1bf..caba1c23b8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -209,6 +209,11 @@ symref::
 	`:lstrip` and `:rstrip` options in the same way as `refname`
 	above.
 
+worktreepath::
+	The absolute path to the worktree in which the ref is checked
+	out, if it is checked out in any linked worktree. Empty string
+	otherwise.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index 5de616befe..e7ca45f39b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -20,6 +20,8 @@
 #include "commit-slab.h"
 #include "commit-graph.h"
 #include "commit-reach.h"
+#include "worktree.h"
+#include "hashmap.h"
 
 static struct ref_msg {
 	const char *gone;
@@ -75,6 +77,22 @@ static struct expand_data {
 	struct object_info info;
 } oi, oi_deref;
 
+struct ref_to_worktree_entry {
+	struct hashmap_entry ent; /* must be the first member! */
+	struct worktree *wt; /* key is wt->head_ref */
+};
+
+static int ref_to_worktree_map_cmpfnc(const void *unused_lookupdata, const void *existing_hashmap_entry_to_test,
+				   const void *key, const void *keydata_aka_refname)
+{
+	const struct ref_to_worktree_entry *e = existing_hashmap_entry_to_test;
+	const struct ref_to_worktree_entry *k = key;
+	return strcmp(e->wt->head_ref, keydata_aka_refname ? keydata_aka_refname : k->wt->head_ref);
+}
+
+static struct hashmap ref_to_worktree_map;
+static struct worktree **worktrees = NULL;
+
 /*
  * An atom is a valid field atom listed below, possibly prefixed with
  * a "*" to denote deref_tag().
@@ -420,6 +438,34 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a
 	return 0;
 }
 
+static int worktree_atom_parser(const struct ref_format *format,
+				struct used_atom *atom,
+				const char *arg,
+				struct strbuf *unused_err)
+{
+	int i;
+
+	if (worktrees)
+		return 0;
+
+	worktrees = get_worktrees(0);
+
+	hashmap_init(&ref_to_worktree_map, ref_to_worktree_map_cmpfnc, NULL, 0);
+
+	for (i = 0; worktrees[i]; i++) {
+		if (worktrees[i]->head_ref) {
+			struct ref_to_worktree_entry *entry;
+			entry = xmalloc(sizeof(*entry));
+			entry->wt = worktrees[i];
+			hashmap_entry_init(entry, strhash(worktrees[i]->head_ref));
+
+			hashmap_add(&ref_to_worktree_map, entry);
+		}
+	}
+
+	return 0;
+}
+
 static struct {
 	const char *name;
 	info_source source;
@@ -461,6 +507,7 @@ static struct {
 	{ "flag", SOURCE_NONE },
 	{ "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser },
 	{ "color", SOURCE_NONE, FIELD_STR, color_atom_parser },
+	{ "worktreepath", SOURCE_NONE, FIELD_STR, worktree_atom_parser },
 	{ "align", SOURCE_NONE, FIELD_STR, align_atom_parser },
 	{ "end", SOURCE_NONE },
 	{ "if", SOURCE_NONE, FIELD_STR, if_atom_parser },
@@ -1500,6 +1547,21 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 	return 0;
 }
 
+static char *get_worktree_path(const struct used_atom *atom, const struct ref_array_item *ref)
+{
+	struct strbuf val = STRBUF_INIT;
+	struct hashmap_entry entry;
+	struct ref_to_worktree_entry *lookup_result;
+
+	hashmap_entry_init(&entry, strhash(ref->refname));
+	lookup_result = hashmap_get(&ref_to_worktree_map, &entry, ref->refname);
+
+	if (lookup_result)
+		strbuf_addstr(&val, lookup_result->wt->path);
+
+	return strbuf_detach(&val, NULL);
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  */
@@ -1537,6 +1599,10 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 
 		if (starts_with(name, "refname"))
 			refname = get_refname(atom, ref);
+		else if (starts_with(name, "worktreepath")) {
+			v->s = get_worktree_path(atom, ref);
+			continue;
+		}
 		else if (starts_with(name, "symref"))
 			refname = get_symref(atom, ref);
 		else if (starts_with(name, "upstream")) {
@@ -2020,6 +2086,11 @@ void ref_array_clear(struct ref_array *array)
 		free_array_item(array->items[i]);
 	FREE_AND_NULL(array->items);
 	array->nr = array->alloc = 0;
+	if (worktrees)
+	{
+		hashmap_free(&ref_to_worktree_map, 1);
+		free_worktrees(worktrees);
+	}
 }
 
 static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata)
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index fc067ed672..87e0222ea1 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -441,4 +441,19 @@ test_expect_success '--merged is incompatible with --no-merged' '
 	test_must_fail git for-each-ref --merged HEAD --no-merged HEAD
 '
 
+test_expect_success '"add" a worktree' '
+	mkdir worktree_dir &&
+	git worktree add -b master_worktree worktree_dir master
+'
+
+test_expect_success 'validate worktree atom' '
+	cat >expect <<-EOF &&
+	master: $(pwd)
+	master_worktree: $(pwd)/worktree_dir
+	side: not checked out
+	EOF
+	git for-each-ref --format="%(refname:short): %(if)%(worktreepath)%(then)%(worktreepath)%(else)not checked out%(end)" refs/heads/ >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.14.2


^ permalink raw reply related

* [PATCH v5 0/3]
From: nbelakovski @ 2019-01-06  0:26 UTC (permalink / raw)
  To: git; +Cc: peff, rafa.almas, gitster, avarab, Nickolai Belakovski
In-Reply-To: <CAC05386q2iGoiJ_fRgwoOTF23exEN2D1+oh4VjajEvYQ58O1TQ@mail.gmail.com>

From: Nickolai Belakovski <nbelakovski@gmail.com>

Replying to my original email to try to clean up the email chain

> Thanks for keeping with this.  I think we're getting quite close

Thanks to you as well for continuing to review the change set and provide feedback!
It does feel rather close, I'm getting exciting about following it through, even if
we just end up merging the worktreepath commit and not the ones to modify the branch
output, since I can always just make a local alias that uses the worktreepath atom.

The last set of changes all made sense, very non-controversial, so I've simply implemented
them. Beyond that, I moved where the structures for the ref<->worktree map are defined now
that they're no longer associated with used_atom. They still feel a little awkwardly
placed to me; I couldn't quite find a way I liked of arranging them together while also
sticking to the style in the rest of the code but I think it's a little better that
all of the relevant structs and the cmpfnc are all in the same place.

Travis-CI results: https://travis-ci.org/nbelakovski/git/builds/475825245

Nickolai Belakovski (3):
  ref-filter: add worktreepath atom
  branch: Mark and color a branch differently if it is checked out in a
    linked worktree
  branch: Add an extra verbose output displaying worktree path for refs
    checked out in a linked worktree

 Documentation/git-branch.txt       | 20 ++++++-----
 Documentation/git-for-each-ref.txt |  5 +++
 builtin/branch.c                   | 16 ++++++---
 ref-filter.c                       | 71 ++++++++++++++++++++++++++++++++++++++
 t/t3200-branch.sh                  |  8 ++---
 t/t3203-branch-output.sh           | 21 +++++++++++
 t/t6302-for-each-ref-filter.sh     | 15 ++++++++
 7 files changed, 140 insertions(+), 16 deletions(-)

-- 
2.14.2


^ permalink raw reply

* Make "git log --count" work like "git rev-list"
From: Linus Torvalds @ 2019-01-05 22:46 UTC (permalink / raw)
  To: Junio Hamano C; +Cc: Git List Mailing

[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]

This is a ridiculous patch. And I understand entirely if nobody else
cares, because I don't think anybody else has ever even noticed.

It turns out that I still use "git rev-list" outside of some hard-core
scripting for one silly reason: the linux-next statistics are all
about non-merge commits, and so to do a rough comparison with
linux-next, I do

        git rev-list --count --no-merges v4.20..

to get an approximate idea of how much I've merged compared to what is
in linux-next.

(See also

        http://neuling.org/linux-next-size.html

for the graphical view of it all, even though it looks a bit odd right
now because of how linux-next wasn't being updated over the holidats
and right at the 4.19 release).

Anyway, I've occasionally thought to myself that I should just fix
"git log" to learn that too, so that I wouldn't have to type out "git
rev-list". Because "git log" does actually take the "--count"
argument, it just doesn't honor it.

This is that patch.

                    Linus

[-- Attachment #2: 0001-git-log-honor-the-count-argument.patch --]
[-- Type: text/x-patch, Size: 2570 bytes --]

From e3bc5387404bcefbd86081fbc82a93fc5c9efa99 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sat, 5 Jan 2019 14:39:41 -0800
Subject: [PATCH] git log: honor the '--count' argument

"git log" is really the human-facing useful command that long long ago
used to scripted around "git rev-list".

In fact, if you want to use the old scripting code, you can still
approximate "git log" with something like

   git rev-list --pretty HEAD | less

but you'd be silly to do that, since "git log" is clearly a much nicer
interface and is what people should use.

Except I find myself still occasionally using "git rev-list" simply
because "git log" doesn't do one odd little quirk: commit counting.

So if you want to count the number of non-merge commits since the last
kernel version, you'd have to do something like

   git rev-list --count --no-merges v4.20..

because while "git log" actually silently _accepts_ the "--count"
argument, it doesn't do anything about it.

This little patch copies the rev-list code for counting to "git log".

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 builtin/log.c | 11 +++++++++++
 log-tree.c    | 10 ++++++++++
 2 files changed, 21 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index e8e51068bd..62bef62f8a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -411,6 +411,17 @@ static int cmd_log_walk(struct rev_info *rev)
 	if (close_file)
 		fclose(rev->diffopt.file);
 
+	if (rev->count) {
+		if (rev->left_right && rev->cherry_mark)
+			printf("%d\t%d\t%d\n", rev->count_left, rev->count_right, rev->count_same);
+		else if (rev->left_right)
+			printf("%d\t%d\n", rev->count_left, rev->count_right);
+		else if (rev->cherry_mark)
+			printf("%d\t%d\n", rev->count_left + rev->count_right, rev->count_same);
+		else
+			printf("%d\n", rev->count_left + rev->count_right);
+	}
+
 	if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
 	    rev->diffopt.flags.check_failed) {
 		return 02;
diff --git a/log-tree.c b/log-tree.c
index 10680c139e..49ff485320 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -913,6 +913,16 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	struct log_info log;
 	int shown, close_file = opt->diffopt.close_file;
 
+	if (opt->count) {
+		if (commit->object.flags & PATCHSAME)
+			opt->count_same++;
+		else if (commit->object.flags & SYMMETRIC_LEFT)
+			opt->count_left++;
+		else
+			opt->count_right++;
+		return 1;
+	}
+
 	log.commit = commit;
 	log.parent = NULL;
 	opt->loginfo = &log;
-- 
2.20.1.101.g60ba6df0c4


^ permalink raw reply related

* Re: How DELTA objects values work and are calculated
From: Farhan Khan @ 2019-01-05 22:32 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8Da7+sNfxvTRz1DRn27TjvBXNAipKB=eumA6q+sVsVjcA@mail.gmail.com>



On 1/4/19 11:46 PM, Duy Nguyen wrote:
> On Sat, Jan 5, 2019 at 9:49 AM Farhan Khan <khanzf@gmail.com> wrote:
>>
>> Hi all,
>>
>> I'm having trouble understanding how OBJ_REF_DELTA and OBJ_REF_DELTA
>> (deltas) work in git. Where does git calculate the sha1 hash values
>> when doing "git index-pack" in builtin/index-pack.c. I think my lack
>> of understanding of the code is compounded the fact that I do not
>> understand what the two object types are.
>>
>>  From tracing the code starting from index-pack, all non-delta object
>> type hashes are calculated in index-pack.c:1131 (parse_pack_objects).
>> However, when the function ends, the delta objects hash values are set
>> to all 0's.
> 
> Delta objects depend on other objects (and even delta ones). To
> calculate its sha1 values we may need to recursively calculate sha1
> values of its base objects. This is why we do it in a separate phase
> because the calculation is more complicated than non-delta objects.
> 
>> My questions are:
>> A) How do Delta objects work?
> 
> A delta object consists of a reference to the base object (either an
> sha1 value, or the offset to where the object is) and a "delta" to be
> applied on (it's basically a binary diff).
> 
>> B) Where and how are the sha1 values calculated?
> 
> Start at threaded_second_pass() in index-pack.c, we go through all
> delta objects here and try to calculate their sha1 values. Eventually
> you'll hit resolve_delta(), where the delta is actually applied to the
> base object in the patch_delta() call, and the sha1 value calculated
> in the following hash_object_file() call.
> 
>>
>> I have read Documentation/technical/pack-format.txt, but am still not clear.
>>
>> Thank you!
>> --
>> Farhan Khan
>> PGP Fingerprint: B28D 2726 E2BC A97E 3854 5ABE 9A9F 00BC D525 16EE
> 
> 
> 


Hi Duy,

Thanks for explaining the Delta objects.

What does a OBJ_REF_DELTA object itself consist of? Do you have to 
uncompress it to parse its values? How do you get its size?

I read through resolve deltas which leads to threaded_second_pass, where 
you suggested to start, but I do not understand what is happening at a 
high level and get confused while reading the code.

 From threaded_second_pass, execution goes into a for-loop that runs 
resolve_base(), which runs runs find_unresolved_deltas(). Is this 
finding the unresolved deltas of the current object (The current 
OBJ_REF_DELTA we are going through)? This then runs 
find_unresolved_deltas() and shortly afterwards 
find_unresolved_deltas_1(). It seems that find_unresolved_deltas_1() is 
applying deltas, but I am not certain.

I do not understand what is happening in any of these functions. There 
are some comments on builtin/index-pack.c:883-904

Overall, I do not understand this entire process, what values to capture 
along the way, and how they are consumed. Please provide some guidance 
on how this process works.

Thank you!
Farhan

^ permalink raw reply

* Re: [git-users] git checkout file with custom mtime
From: Daniel Fanjul @ 2019-01-05 21:44 UTC (permalink / raw)
  To: git-users; +Cc: Git List
In-Reply-To: <1413fbaa-8a83-0f43-afcb-5cb67683b505@iee.org>

I'm on Ubuntu. I do not use LFS. I track mods and saved games of
Skyrim with git, TESV.exe sorts the saved games only by their mtime. I
know it is not the most usual use case for git.

I agree with that viewpoint and I like the way git works right now, I
do not want to change that. Checking out the saved games and then
fixing the mtime works but forces a lot of unneeded I/O.

I forgot to mention that 'git update-index --assume-unchanged' does
not solve this well enough. Eventually 'git status' rereads the file
when that flag is removed. A better way for my use case would be being
able to set the proper mtime without forcing a rehash of the file that
yields the same object.

Thanks for your reply.

^ permalink raw reply

* Re: [git-users] git checkout file with custom mtime
From: Philip Oakley @ 2019-01-05 20:35 UTC (permalink / raw)
  To: git-users, Daniel Fanjul, Git List
In-Reply-To: <951cafaa-877b-4815-862f-5ffc43e6976a@googlegroups.com>

Daniel,


Do you use the Git LFS (Large File System) add-on?, are you on Windows 
or Linux?, and what tools need mtime (or is it something about the 
process of using the tool..)?

The Git viewpoint is that the mtime shouldn't be important for the 
version storage & control aspects, though it maybe for the external 
compiler tooling, so they do tend to try to keep the mtime/ctime consistent.

I'm not aware of specific capability to do what you ask, but it may be 
worth discussing this on the git mailing list "Git List 
<git@vger.kernel.org>" (which only accepts 100% plain text, no HTML, 
messages). The mailing list archive is at 
https://public-inbox.org/git/?q= where you can search for mtime/ctime 
discussions.

There will be a Git developer conference at the end of the month, so it 
is worth raining it soonish, even if it becomes an add on the fires via 
a post checkout hook that updates the mtimes from a stored file of 
'true' mtimes (plus updates the index's view of those mtimes.

Philip

On 05/01/2019 13:33, Daniel Fanjul wrote:
> Hi all,
>
> I have some large files tracked in git and I have to track their mtime 
> because of some legacy software. With another tool I save and restore 
> their mtime. When I restore their mtime git status rereads the files 
> to update the mtime in the index. I would like to improve that because 
> there are too many files, the whole I/O is too slow and the whole 
> process is triggered too often.
>
> I would like a way to tell git to checkout a file and set a given 
> mtime at the same time so the index is updated with the mtime but the 
> file is not rewritten because the working copy is clean. This would 
> solve my problem. Do you know a way to do this?
>
> Do you know any other way to handle this properly?
>
> Thanks in advance, and happy new year,
> Daniel.
> -- 
> You received this message because you are subscribed to the Google 
> Groups "Git for human beings" group.
> To unsubscribe from this group and stop receiving emails from it, send 
> an email to git-users+unsubscribe@googlegroups.com 
> <mailto:git-users+unsubscribe@googlegroups.com>.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH] diff: add support for reading files literally with --no-index
From: brian m. carlson @ 2019-01-05 17:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Nieder, git, Jeff King, Duy Nguyen
In-Reply-To: <xmqq36q8cjgf.fsf@gitster-ct.c.googlers.com>

[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]

On Fri, Jan 04, 2019 at 11:26:56AM -0800, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> >>  - --dereference to control whether to follow symlinks
> >
> > This is actually surprisingly difficult. The reason I implemented this
> > only for no-index mode is because there are actually several places we
> > can stat a file in the diff code, and implementing a --dereference
> > option that catches all of those cases and getting the option passed
> > down to them is non-trivial.
> 
> Another thing to worry about is symlinks that point outside the
> working tree.  When a tracked content "dir/link" is a symlink to
> "/etc/motd", it probably makes sense to open("/etc/motd") and read()
> it on the working tree side of the diff, and probably even on the
> index side of the diff, but what about obtaining contents for
> "dir/link" in a year-old commit under --deference mode?  I am not
> sure if it makes sense to read from the filesystem in such a case.
> 
> I personally am perfectly fine if this "do not compare readlink(2),
> but read contents literally" is limited to the --no-index mode.

That's a good point. I think I'll stick with the current design, then,
since that seems like the least surprising way forward. It also means
that we don't read outside of the working tree unless --no-index is in
use, which may be beneficial for security purposes.

Thanks for a helpful perspective.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox