* [PATCHv2 3/4] cache.h: document add_[file_]to_index
From: Stefan Beller @ 2017-01-18 23:21 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index 87eccdb211..03c46b9b99 100644
--- a/cache.h
+++ b/cache.h
@@ -609,13 +609,24 @@ extern int remove_index_entry_at(struct index_state *, int pos);
extern void remove_marked_cache_entries(struct index_state *istate);
extern int remove_file_from_index(struct index_state *, const char *path);
-#define ADD_CACHE_VERBOSE 1
-#define ADD_CACHE_PRETEND 2
-#define ADD_CACHE_IGNORE_ERRORS 4
-#define ADD_CACHE_IGNORE_REMOVAL 8
-#define ADD_CACHE_INTENT 16
+
+#define ADD_CACHE_VERBOSE 1 /* verbose */
+#define ADD_CACHE_PRETEND 2 /* dry run */
+#define ADD_CACHE_IGNORE_ERRORS 4 /* ignore errors */
+#define ADD_CACHE_IGNORE_REMOVAL 8 /* do not remove files from index */
+#define ADD_CACHE_INTENT 16 /* intend to add later; stage empty file */
+/*
+ * These two are used to add the contents of the file at path
+ * to the index, marking the working tree up-to-date by storing
+ * the cached stat info in the resulting cache entry. A caller
+ * that has already run lstat(2) on the path can call
+ * add_to_index(), and all others can call add_file_to_index();
+ * the latter will do necessary lstat(2) internally before
+ * calling the former.
+ */
extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
extern int add_file_to_index(struct index_state *, const char *path, int flags);
+
extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* [PATCHv2 1/4] cache.h: document index_name_pos
From: Stefan Beller @ 2017-01-18 23:21 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/cache.h b/cache.h
index 1b67f078dd..3dbba69aec 100644
--- a/cache.h
+++ b/cache.h
@@ -575,7 +575,26 @@ extern int verify_path(const char *path);
extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
extern void adjust_dirname_case(struct index_state *istate, char *name);
extern struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
+
+/*
+ * Searches for an entry defined by name and namelen in the given index.
+ * If the return value is positive (including 0) it is the position of an
+ * exact match. If the return value is negative, the negated value minus 1
+ * is the position where the entry would be inserted.
+ * Example: The current index consists of these files and its stages:
+ *
+ * b#0, d#0, f#1, f#3
+ *
+ * index_name_pos(&index, "a", 1) -> -1
+ * index_name_pos(&index, "b", 1) -> 0
+ * index_name_pos(&index, "c", 1) -> -2
+ * index_name_pos(&index, "d", 1) -> 1
+ * index_name_pos(&index, "e", 1) -> -3
+ * index_name_pos(&index, "f", 1) -> 2
+ * index_name_pos(&index, "g", 1) -> -5
+ */
extern int index_name_pos(const struct index_state *, const char *name, int namelen);
+
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
#define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */
#define ADD_CACHE_SKIP_DFCHECK 4 /* Ok to skip DF conflict checks */
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* [PATCHv2 4/4] documentation: retire unfinished documentation
From: Stefan Beller @ 2017-01-18 23:21 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-1-sbeller@google.com>
When looking for documentation for a specific function, you may be tempted
to run
git -C Documentation grep index_name_pos
only to find the file technical/api-in-core-index.txt, which doesn't
help for understanding the given function. It would be better to not find
these functions in the documentation, such that people directly dive into
the code instead.
In the previous patches we have documented
* index_name_pos()
* remove_index_entry_at()
* add_[file_]to_index()
in cache.h
We already have documentation for:
* add_index_entry()
* read_index()
Which leaves us with a TODO for:
* cache -> the_index macros
* refresh_index()
* discard_index()
* ie_match_stat() and ie_modified(); how they are different and when to
use which.
* write_index() that was renamed to write_locked_index
* cache_tree_invalidate_path()
* cache_tree_update()
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/technical/api-in-core-index.txt | 21 ---------------------
1 file changed, 21 deletions(-)
delete mode 100644 Documentation/technical/api-in-core-index.txt
diff --git a/Documentation/technical/api-in-core-index.txt b/Documentation/technical/api-in-core-index.txt
deleted file mode 100644
index adbdbf5d75..0000000000
--- a/Documentation/technical/api-in-core-index.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-in-core index API
-=================
-
-Talk about <read-cache.c> and <cache-tree.c>, things like:
-
-* cache -> the_index macros
-* read_index()
-* write_index()
-* ie_match_stat() and ie_modified(); how they are different and when to
- use which.
-* index_name_pos()
-* remove_index_entry_at()
-* remove_file_from_index()
-* add_file_to_index()
-* add_index_entry()
-* refresh_index()
-* discard_index()
-* cache_tree_invalidate_path()
-* cache_tree_update()
-
-(JC, Linus)
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* Re: "git diff --ignore-space-change --stat" lists files with only whitespace differences as "changed"
From: Junio C Hamano @ 2017-01-18 22:15 UTC (permalink / raw)
To: Jeff King; +Cc: Matt McCutchen, git
In-Reply-To: <20170118210821.xugr6jnvzgoxpynb@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 18, 2017 at 12:57:12PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > So I dunno. A sensible rule to me is "iff -p would show a diff header,
>> > then --stat should mention it".
>>
>> True but tricky (you need a better definition of "a diff header").
>>
>> In addition to a new and deleted file, does a file whose executable
>> bit was flipped need mention? If so, then "diff --git" is the diff
>> header in the above. Otherwise "@@ ... @@", iow, "iff -p would show
>> any hunk".
>>
>> I think the patch implements the latter, which I think is sensible.
>
> I would think the former is more sensible (and is what my patch is
> working towards).
Doh (yes, "diff --git" was what I meant). As a mode-flipping patch
does not have hunk but does show the header, it wants to be included
in "git diff --stat" output, I would think, independent of -b issue.
In fact
chmod +x README.md
git diff --stat
does show a 0-line diffstat.
> Doing:
>
> >empty
> git add empty
> git diff --cached
>
> shows a "diff --git" header, but no hunk. I think it should show a
> diffstat (and does with my patch).
>
> I was thinking the rule should be something like:
>
> if (p->status == DIFF_STATUS_MODIFIED &&
> !file->added && !file->deleted))
>
> and otherwise include the entry, since it would be an add, delete,
> rename, etc, which carries useful information.
>
> Though a pure rename would not hit this code path at all, I would think
> (it would not trigger "!same_contents"). And a rename where there was a
> whitespace only change probably _should_ be omitted from "-b".
>
> Ditto for a pure mode change, I think. We do not run the contents
> through diff at all, so it does not hit this code path.
>
> -Peff
^ permalink raw reply
* Re: [PATCHv2 3/4] cache.h: document add_[file_]to_index
From: Brandon Williams @ 2017-01-19 0:00 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git
In-Reply-To: <20170118232145.31606-4-sbeller@google.com>
On 01/18, Stefan Beller wrote:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> cache.h | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 87eccdb211..03c46b9b99 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -609,13 +609,24 @@ extern int remove_index_entry_at(struct index_state *, int pos);
>
> extern void remove_marked_cache_entries(struct index_state *istate);
> extern int remove_file_from_index(struct index_state *, const char *path);
> -#define ADD_CACHE_VERBOSE 1
> -#define ADD_CACHE_PRETEND 2
> -#define ADD_CACHE_IGNORE_ERRORS 4
> -#define ADD_CACHE_IGNORE_REMOVAL 8
> -#define ADD_CACHE_INTENT 16
> +
> +#define ADD_CACHE_VERBOSE 1 /* verbose */
> +#define ADD_CACHE_PRETEND 2 /* dry run */
> +#define ADD_CACHE_IGNORE_ERRORS 4 /* ignore errors */
> +#define ADD_CACHE_IGNORE_REMOVAL 8 /* do not remove files from index */
> +#define ADD_CACHE_INTENT 16 /* intend to add later; stage empty file */
I usually prefer having defines like these use shift operators to set
the desired bit '(1<<2)' instead of '4', etc. Is there a preference for
git as a whole? I know this is just a documentation change so maybe
this isn't even the place to discuss this.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCHv2 3/4] cache.h: document add_[file_]to_index
From: Stefan Beller @ 2017-01-19 0:08 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20170119000037.GF10641@google.com>
On Wed, Jan 18, 2017 at 4:00 PM, Brandon Williams <bmwill@google.com> wrote:
> On 01/18, Stefan Beller wrote:
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> cache.h | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/cache.h b/cache.h
>> index 87eccdb211..03c46b9b99 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -609,13 +609,24 @@ extern int remove_index_entry_at(struct index_state *, int pos);
>>
>> extern void remove_marked_cache_entries(struct index_state *istate);
>> extern int remove_file_from_index(struct index_state *, const char *path);
>> -#define ADD_CACHE_VERBOSE 1
>> -#define ADD_CACHE_PRETEND 2
>> -#define ADD_CACHE_IGNORE_ERRORS 4
>> -#define ADD_CACHE_IGNORE_REMOVAL 8
>> -#define ADD_CACHE_INTENT 16
>> +
>> +#define ADD_CACHE_VERBOSE 1 /* verbose */
>> +#define ADD_CACHE_PRETEND 2 /* dry run */
>> +#define ADD_CACHE_IGNORE_ERRORS 4 /* ignore errors */
>> +#define ADD_CACHE_IGNORE_REMOVAL 8 /* do not remove files from index */
>> +#define ADD_CACHE_INTENT 16 /* intend to add later; stage empty file */
>
> I usually prefer having defines like these use shift operators to set
> the desired bit '(1<<2)' instead of '4', etc. Is there a preference for
> git as a whole? I know this is just a documentation change so maybe
> this isn't even the place to discuss this.
eh, and I forgot to remove the comments that Junio thought of as redundant.
I agree that (1<<N)) is usually better than the actual number. But I think
we do not want to change that for the same reason as we don't want to add
these comments there: Digging into history just got more complicated here.
("Who introduced ADD_CACHE_INTENT and why?" you need to skip the
reformatting/adding document patch to actually find the answer.)
Thanks for spotting,
Stefan
>
> --
> Brandon Williams
^ permalink raw reply
* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Eric Wong @ 2017-01-19 0:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: Santiago Torres, meta, git, peff, sunshine, walters,
Lukas Puehringer
In-Reply-To: <20170118201644.GA13758@starla>
Eric Wong <e@80x24.org> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> >
> > http://public-inbox.org/git/20170118182831.pkhqu2np3bh2puei@LykOS.localdomain/
>
> Eeep! This looks like a regression I introduced when working
> around Richard Hansen's S/MIME mails the other week on git@vger:
>
> https://public-inbox.org/meta/20170110222235.GB27356@dcvr/T/#u
Yep, I copied SUPER and used it improperly in a subclass. Should be
fixed now, and reimported several messages from my Maildir. NNTP
readers will see new article numbers for reimported Message-IDs.
^ permalink raw reply
* Re: [PATCH] attr: mark a file-local symbol as static
From: Ramsay Jones @ 2017-01-19 1:22 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <20170118230541.GE10641@google.com>
On 18/01/17 23:05, Brandon Williams wrote:
> On 01/18, Ramsay Jones wrote:
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> ---
>>
>> Hi Brandon,
>>
>> If you need to re-roll your 'bw/attr' branch, could you please
>> squash this into the relevant patch (commit 8908457159,
>> "attr: use hashmap for attribute dictionary", 12-01-2017).
>>
>> Also, I note that, although they are declared as part of the
>> public attr api, attr_check_clear() and attr_check_reset() are
>> also not called outside of attr.c. Are these functions part of
>> the public api?
>>
>> Also, a minor point, but attr_check_reset() is called (line 1050)
>> before it's definition (line 1114). This is not a problem, given
>> the declaration in attr.h, but I prefer definitions to come before
>> use, where possible.
>>
>> Thanks!
>>
>> ATB,
>> Ramsay Jones
>
> Yes of course, I believe Stefan also pointed that out earlier today so I
> have it fixed locally.
Yep, I noticed Stefan's email just a few minutes after I hit
send! ;-)
> For attr_check_clear() and attr_check_reset() the intent is that they
> are the accepted way to either clear or reset the attr_check structure.
> Currently most users of the attribute system don't have a need to clear
> or reset the structure but there could be future callers who need that
> functionality. If you feel like they shouldn't be part of the api right
> now then I'm open to changing that for this series.
No, I just wanted to check that they were intended to be part of
the public api and that you anticipate additional callers in the
future.
[I would still prefer definitions before use, but many people would
not agree with me, so ...]
ATB,
Ramsay Jones
^ permalink raw reply
* [PATCHv3 0/4] start documenting core functions
From: Stefan Beller @ 2017-01-19 3:18 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-2-sbeller@google.com>
v3:
* dropped commets on constants as they were not helpful
* fixed one result in the example for index_name_pos(&index, "f", 1) (which
is -3 now)
v2:
included all suggestions from Junio
v1:
The two single patches[1] are turned into a series here.
[1] https://public-inbox.org/git/20170117200147.25425-1-sbeller@google.com/
Thanks,
Stefan
Stefan Beller (4):
cache.h: document index_name_pos
cache.h: document remove_index_entry_at
cache.h: document add_[file_]to_index
documentation: retire unfinished documentation
Documentation/technical/api-in-core-index.txt | 21 -------------
cache.h | 43 +++++++++++++++++++++++----
read-cache.c | 1 -
3 files changed, 38 insertions(+), 27 deletions(-)
delete mode 100644 Documentation/technical/api-in-core-index.txt
--
2.11.0.299.g762782ba8a
^ permalink raw reply
* [PATCHv3 1/4] cache.h: document index_name_pos
From: Stefan Beller @ 2017-01-19 3:18 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/cache.h b/cache.h
index 1b67f078dd..1469ddeafe 100644
--- a/cache.h
+++ b/cache.h
@@ -575,7 +575,26 @@ extern int verify_path(const char *path);
extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
extern void adjust_dirname_case(struct index_state *istate, char *name);
extern struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
+
+/*
+ * Searches for an entry defined by name and namelen in the given index.
+ * If the return value is positive (including 0) it is the position of an
+ * exact match. If the return value is negative, the negated value minus 1
+ * is the position where the entry would be inserted.
+ * Example: The current index consists of these files and its stages:
+ *
+ * b#0, d#0, f#1, f#3
+ *
+ * index_name_pos(&index, "a", 1) -> -1
+ * index_name_pos(&index, "b", 1) -> 0
+ * index_name_pos(&index, "c", 1) -> -2
+ * index_name_pos(&index, "d", 1) -> 1
+ * index_name_pos(&index, "e", 1) -> -3
+ * index_name_pos(&index, "f", 1) -> -3
+ * index_name_pos(&index, "g", 1) -> -5
+ */
extern int index_name_pos(const struct index_state *, const char *name, int namelen);
+
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
#define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */
#define ADD_CACHE_SKIP_DFCHECK 4 /* Ok to skip DF conflict checks */
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* [PATCHv3 2/4] cache.h: document remove_index_entry_at
From: Stefan Beller @ 2017-01-19 3:18 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>
Do this by moving the existing documentation from
read-cache.c to cache.h.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 3 +++
read-cache.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index 1469ddeafe..929474d7a9 100644
--- a/cache.h
+++ b/cache.h
@@ -603,7 +603,10 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
#define ADD_CACHE_KEEP_CACHE_TREE 32 /* Do not invalidate cache-tree */
extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
+
+/* Remove entry, return true if there are more entries to go. */
extern int remove_index_entry_at(struct index_state *, int pos);
+
extern void remove_marked_cache_entries(struct index_state *istate);
extern int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_VERBOSE 1
diff --git a/read-cache.c b/read-cache.c
index 2eca639cce..63a414cdb5 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -503,7 +503,6 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel
return index_name_stage_pos(istate, name, namelen, 0);
}
-/* Remove entry, return true if there are more entries to go.. */
int remove_index_entry_at(struct index_state *istate, int pos)
{
struct cache_entry *ce = istate->cache[pos];
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* [PATCHv3 4/4] documentation: retire unfinished documentation
From: Stefan Beller @ 2017-01-19 3:18 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>
When looking for documentation for a specific function, you may be tempted
to run
git -C Documentation grep index_name_pos
only to find the file technical/api-in-core-index.txt, which doesn't
help for understanding the given function. It would be better to not find
these functions in the documentation, such that people directly dive into
the code instead.
In the previous patches we have documented
* index_name_pos()
* remove_index_entry_at()
* add_[file_]to_index()
in cache.h
We already have documentation for:
* add_index_entry()
* read_index()
Which leaves us with a TODO for:
* cache -> the_index macros
* refresh_index()
* discard_index()
* ie_match_stat() and ie_modified(); how they are different and when to
use which.
* write_index() that was renamed to write_locked_index
* cache_tree_invalidate_path()
* cache_tree_update()
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/technical/api-in-core-index.txt | 21 ---------------------
1 file changed, 21 deletions(-)
delete mode 100644 Documentation/technical/api-in-core-index.txt
diff --git a/Documentation/technical/api-in-core-index.txt b/Documentation/technical/api-in-core-index.txt
deleted file mode 100644
index adbdbf5d75..0000000000
--- a/Documentation/technical/api-in-core-index.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-in-core index API
-=================
-
-Talk about <read-cache.c> and <cache-tree.c>, things like:
-
-* cache -> the_index macros
-* read_index()
-* write_index()
-* ie_match_stat() and ie_modified(); how they are different and when to
- use which.
-* index_name_pos()
-* remove_index_entry_at()
-* remove_file_from_index()
-* add_file_to_index()
-* add_index_entry()
-* refresh_index()
-* discard_index()
-* cache_tree_invalidate_path()
-* cache_tree_update()
-
-(JC, Linus)
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* [PATCHv3 3/4] cache.h: document add_[file_]to_index
From: Stefan Beller @ 2017-01-19 3:18 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/cache.h b/cache.h
index 929474d7a9..12394eb541 100644
--- a/cache.h
+++ b/cache.h
@@ -614,8 +614,18 @@ extern int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_IGNORE_ERRORS 4
#define ADD_CACHE_IGNORE_REMOVAL 8
#define ADD_CACHE_INTENT 16
+/*
+ * These two are used to add the contents of the file at path
+ * to the index, marking the working tree up-to-date by storing
+ * the cached stat info in the resulting cache entry. A caller
+ * that has already run lstat(2) on the path can call
+ * add_to_index(), and all others can call add_file_to_index();
+ * the latter will do necessary lstat(2) internally before
+ * calling the former.
+ */
extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
extern int add_file_to_index(struct index_state *, const char *path, int flags);
+
extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
--
2.11.0.299.g762782ba8a
^ permalink raw reply related
* Re: Git: new feature suggestion
From: Konstantin Khomoutov @ 2017-01-19 6:33 UTC (permalink / raw)
To: Joao Pinto; +Cc: git, Linus Torvalds, CARLOS.PALMINHA@synopsys.com
In-Reply-To: <4817eb00-6efc-e3c0-53d7-46f2509350d3@synopsys.com>
On Wed, 18 Jan 2017 10:40:52 +0000
Joao Pinto <Joao.Pinto@synopsys.com> wrote:
[...]
> I have seen a lot of Linux developers avoid this re-organization
> operations because they would lose the renamed file history, because
> a new log is created for the new file, even if it is a renamed
> version of itself. I am sending you this e-mail to suggest the
> creation of a new feature in Git: when renamed, a file or folder
> should inherit his parent’s log and a “rename: …” would be
> automatically created or have some kind of pointer to its “old” form
> to make history analysis easier.
Git does not record renames because of its stance that what matters is
code _of the whole project_ as opposed to its location in a particular
file.
Hence with regard to renames Git "works backwards" by detecting them
dynamically while traversing the history (such as with `git log`
etc). This detection uses certain heuristics which can be controlled
with knobs pointed to by Stefan Beller.
Still, I welcome you to read the sort-of "reference" post by Linus
Torvalds [1] in which he explains the reasoning behind this approach
implemented in Git. IMO, understanding the reasoning behind the idea
is much better than just mechanically learning how to use it.
The whole thread (esp. Torvalds' replies) is worth reading, but that
particular mail summarizes the whole thing very well.
(The reference link to it used to be [2], but Gmane is not fully
recovered to be able to display it.)
1. http://public-inbox.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/
2. http://thread.gmane.org/gmane.comp.version-control.git/27/focus=217
^ permalink raw reply
* Re: fatal: bad revision 'git rm -r --ignore-unmatch -- folder'
From: jean-christophe manciot @ 2017-01-19 8:32 UTC (permalink / raw)
To: git
In-Reply-To: <CAKcFC3aqjLNUNKPDZ08rO_SBkY84uvHBerCxnSchAe8rH0dnMg@mail.gmail.com>
In case you were wondering whether these files were tracked or not:
git-Games# git ls-files Ubuntu/16.04
Ubuntu/16.04/residualvm_0.3.0~git-1_amd64.deb
Ubuntu/16.04/scummvm-data_1.8.0_all.deb
Ubuntu/16.04/scummvm-data_1.9.0_all.deb
Ubuntu/16.04/scummvm-dbgsym_1.8.2~git20160821.bad86050_amd64.deb
Ubuntu/16.04/scummvm-dbgsym_1.9.0_amd64.deb
Ubuntu/16.04/scummvm_1.8.0_amd64.deb
Ubuntu/16.04/scummvm_1.9.0_amd64.deb
On Tue, Jan 17, 2017 at 4:30 PM, jean-christophe manciot
<actionmystique@gmail.com> wrote:
> Hi there,
>
> I'm trying to purge a complete folder and its files from the
> repository history with:
>
> git-Games# git filter-branch 'git rm -r --ignore-unmatch --
> Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
> fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
>
> git does not find the folder although it's there:
>
> git-Games# ll Ubuntu/16.04/
> total 150316
> drwxr-x--- 2 actionmystique actionmystique 4096 Nov 19 13:40 ./
> drwxr-x--- 4 actionmystique actionmystique 4096 Oct 30 14:02 ../
> -rwxr-x--- 1 actionmystique actionmystique 2190394 May 9 2016
> residualvm_0.3.0~git-1_amd64.deb*
> ...
> -rw-r--r-- 1 actionmystique actionmystique 67382492 Oct 13 09:15
> scummvm-dbgsym_1.9.0_amd64.deb
>
> What is going on?
>
> --
> Jean-Christophe
--
Jean-Christophe
^ permalink raw reply
* Re: fatal: bad revision 'git rm -r --ignore-unmatch -- folder'
From: jean-christophe manciot @ 2017-01-19 8:42 UTC (permalink / raw)
To: git
In-Reply-To: <CAKcFC3ZN+=o_2t4AawCE0c4Tw+t_XJKTHtEq9d7bv-ht5euPbw@mail.gmail.com>
Also some context information:
Ubuntu 16.10 4.8
git 2.11.0
On Thu, Jan 19, 2017 at 9:32 AM, jean-christophe manciot
<actionmystique@gmail.com> wrote:
> In case you were wondering whether these files were tracked or not:
>
> git-Games# git ls-files Ubuntu/16.04
> Ubuntu/16.04/residualvm_0.3.0~git-1_amd64.deb
> Ubuntu/16.04/scummvm-data_1.8.0_all.deb
> Ubuntu/16.04/scummvm-data_1.9.0_all.deb
> Ubuntu/16.04/scummvm-dbgsym_1.8.2~git20160821.bad86050_amd64.deb
> Ubuntu/16.04/scummvm-dbgsym_1.9.0_amd64.deb
> Ubuntu/16.04/scummvm_1.8.0_amd64.deb
> Ubuntu/16.04/scummvm_1.9.0_amd64.deb
>
> On Tue, Jan 17, 2017 at 4:30 PM, jean-christophe manciot
> <actionmystique@gmail.com> wrote:
>> Hi there,
>>
>> I'm trying to purge a complete folder and its files from the
>> repository history with:
>>
>> git-Games# git filter-branch 'git rm -r --ignore-unmatch --
>> Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
>> fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
>>
>> git does not find the folder although it's there:
>>
>> git-Games# ll Ubuntu/16.04/
>> total 150316
>> drwxr-x--- 2 actionmystique actionmystique 4096 Nov 19 13:40 ./
>> drwxr-x--- 4 actionmystique actionmystique 4096 Oct 30 14:02 ../
>> -rwxr-x--- 1 actionmystique actionmystique 2190394 May 9 2016
>> residualvm_0.3.0~git-1_amd64.deb*
>> ...
>> -rw-r--r-- 1 actionmystique actionmystique 67382492 Oct 13 09:15
>> scummvm-dbgsym_1.9.0_amd64.deb
>>
>> What is going on?
>>
>> --
>> Jean-Christophe
>
>
>
> --
> Jean-Christophe
--
Jean-Christophe
^ permalink raw reply
* Re: [PATCH 0/6] loose-object fsck fixes/tightening
From: John Szakmeister @ 2017-01-19 11:18 UTC (permalink / raw)
To: Jeff King; +Cc: Dennis Kaarsemaker, git
In-Reply-To: <20170113175258.e66taigy4wpokohk@sigill.intra.peff.net>
On Fri, Jan 13, 2017 at 12:52 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 13, 2017 at 04:15:42AM -0500, John Szakmeister wrote:
>
>> > I did notice another interesting case when looking at this. Fsck ends up
>> > in fsck_loose(), which has the sha1 and path of the loose object. It
>> > passes the sha1 to fsck_sha1(), and ignores the path entirely!
>> >
>> > So if you have a duplicate copy of the object in a pack, we'd actually
>> > find and check the duplicate. This can happen, e.g., if you had a loose
>> > object and fetched a thin-pack which made a copy of the loose object to
>> > complete the pack).
>> >
>> > Probably fsck_loose() should be more picky about making sure we are
>> > reading the data from the loose version we found.
>>
>> Interesting find! Thanks for the information Peff!
>
> So I figured I would knock this out as a fun morning exercise. But
> sheesh, it turned out to be a slog, because most of the functions rely
> on map_sha1_file() to convert the sha1 to an object path at the lowest
> level.
Yeah, I discovered the same thing when I took a look at it a week or so ago. :-(
> But I finally got something working, so here it is. I found another bug
> on the way, along with a few cleanups. And then I did the trailing
> garbage detection at the end, because by that point I knew right where
> it needed to go. :)
I don't know if my opinion counts for much, but the changes look good to me.
-John
^ permalink raw reply
* [PATCH v5 0/3] nd/log-graph-configurable-colors
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170109103258.25341-1-pclouds@gmail.com>
v5 moves space trimming to color_parse_mem() from read_graph_colors_config,
which is renamed to parse_graph... because the config reading is moved
back to graph_init.
I think it looks better, but we may be pushing the limits of
argv_array's abuse.
Nguyễn Thái Ngọc Duy (3):
color.c: fix color_parse_mem() with value_len == 0
color.c: trim leading spaces in color_parse_mem()
log --graph: customize the graph lines with config log.graphColors
Documentation/config.txt | 4 ++++
color.c | 10 +++++++++-
graph.c | 42 +++++++++++++++++++++++++++++++++++++++---
t/t4202-log.sh | 22 ++++++++++++++++++++++
4 files changed, 74 insertions(+), 4 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH v5 1/3] color.c: fix color_parse_mem() with value_len == 0
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>
In this code we want to match the word "reset". If len is zero,
strncasecmp() will return zero and we incorrectly assume it's "reset" as
a result.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
color.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/color.c b/color.c
index 81c2676..a9eadd1 100644
--- a/color.c
+++ b/color.c
@@ -207,6 +207,9 @@ int color_parse_mem(const char *value, int value_len, char *dst)
struct color fg = { COLOR_UNSPECIFIED };
struct color bg = { COLOR_UNSPECIFIED };
+ if (!len)
+ return -1;
+
if (!strncasecmp(value, "reset", len)) {
xsnprintf(dst, end - dst, GIT_COLOR_RESET);
return 0;
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 2/3] color.c: trim leading spaces in color_parse_mem()
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>
Normally color_parse_mem() is called from config parser which trims the
leading spaces already. The new caller in the next patch won't. Let's be
tidy and trim leading spaces too (we already trim trailing spaces before
comma).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
color.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/color.c b/color.c
index a9eadd1..7bb4a96 100644
--- a/color.c
+++ b/color.c
@@ -207,10 +207,15 @@ int color_parse_mem(const char *value, int value_len, char *dst)
struct color fg = { COLOR_UNSPECIFIED };
struct color bg = { COLOR_UNSPECIFIED };
+ while (len > 0 && isspace(*ptr)) {
+ ptr++;
+ len--;
+ }
+
if (!len)
return -1;
- if (!strncasecmp(value, "reset", len)) {
+ if (!strncasecmp(ptr, "reset", len)) {
xsnprintf(dst, end - dst, GIT_COLOR_RESET);
return 0;
}
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 3/3] log --graph: customize the graph lines with config log.graphColors
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>
If you have a 256 colors terminal (or one with true color support), then
the predefined 12 colors seem limited. On the other hand, you don't want
to draw graph lines with every single color in this mode because the two
colors could look extremely similar. This option allows you to hand pick
the colors you want.
Even with standard terminal, if your background color is neither black
or white, then the graph line may match your background and become
hidden. You can exclude your background color (or simply the colors you
hate) with this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 4 ++++
graph.c | 42 +++++++++++++++++++++++++++++++++++++++---
t/t4202-log.sh | 22 ++++++++++++++++++++++
3 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..33a007b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2003,6 +2003,10 @@ log.follow::
i.e. it cannot be used to follow multiple files and does not work well
on non-linear history.
+log.graphColors::
+ A list of colors, separated by commas, that can be used to draw
+ history lines in `git log --graph`.
+
log.showRoot::
If true, the initial commit will be shown as a big creation event.
This is equivalent to a diff against an empty tree.
diff --git a/graph.c b/graph.c
index dd17201..822d40a 100644
--- a/graph.c
+++ b/graph.c
@@ -4,6 +4,7 @@
#include "graph.h"
#include "diff.h"
#include "revision.h"
+#include "argv-array.h"
/* Internal API */
@@ -62,6 +63,26 @@ enum graph_state {
static const char **column_colors;
static unsigned short column_colors_max;
+static void parse_graph_colors_config(struct argv_array *colors, const char *string)
+{
+ const char *end, *start;
+
+ start = string;
+ end = string + strlen(string);
+ while (start < end) {
+ const char *comma = strchrnul(start, ',');
+ char color[COLOR_MAXLEN];
+
+ if (!color_parse_mem(start, comma - start, color))
+ argv_array_push(colors, color);
+ else
+ warning(_("ignore invalid color '%.*s' in log.graphColors"),
+ (int)(comma - start), start);
+ start = comma + 1;
+ }
+ argv_array_push(colors, GIT_COLOR_RESET);
+}
+
void graph_set_column_colors(const char **colors, unsigned short colors_max)
{
column_colors = colors;
@@ -207,9 +228,24 @@ struct git_graph *graph_init(struct rev_info *opt)
{
struct git_graph *graph = xmalloc(sizeof(struct git_graph));
- if (!column_colors)
- graph_set_column_colors(column_colors_ansi,
- column_colors_ansi_max);
+ if (!column_colors) {
+ struct argv_array ansi_colors = {
+ column_colors_ansi,
+ column_colors_ansi_max + 1
+ };
+ struct argv_array *colors = &ansi_colors;
+ char *string;
+
+ if (!git_config_get_string("log.graphcolors", &string)) {
+ static struct argv_array custom_colors = ARGV_ARRAY_INIT;
+ argv_array_clear(&custom_colors);
+ parse_graph_colors_config(&custom_colors, string);
+ free(string);
+ colors = &custom_colors;
+ }
+ /* graph_set_column_colors takes a max-index, not a count */
+ graph_set_column_colors(colors->argv, colors->argc - 1);
+ }
graph->commit = NULL;
graph->revs = opt;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c..0aeabed 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -313,6 +313,28 @@ test_expect_success 'log --graph with merge' '
test_cmp expect actual
'
+cat > expect.colors <<\EOF
+* Merge branch 'side'
+<BLUE>|<RESET><CYAN>\<RESET>
+<BLUE>|<RESET> * side-2
+<BLUE>|<RESET> * side-1
+* <CYAN>|<RESET> Second
+* <CYAN>|<RESET> sixth
+* <CYAN>|<RESET> fifth
+* <CYAN>|<RESET> fourth
+<CYAN>|<RESET><CYAN>/<RESET>
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge with log.graphColors' '
+ test_config log.graphColors ",, blue,invalid-color, cyan, red , " &&
+ git log --color=always --graph --date-order --pretty=tformat:%s |
+ test_decode_color | sed "s/ *\$//" >actual &&
+ test_cmp expect.colors actual
+'
+
test_expect_success 'log --raw --graph -m with merge' '
git log --raw --graph --oneline -m master | head -n 500 >actual &&
grep "initial" actual
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* The design of refs backends, linked worktrees and submodules
From: Duy Nguyen @ 2017-01-19 11:55 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Git Mailing List
I've started working on fixing the "git gc" issue with multiple
worktrees, which brings me back to this. Just some thoughts. Comments
are really appreciated.
In the current code, files backend has special cases for both
submodules (explicitly) and linked worktrees (hidden behind git_path).
But if a backend has to handle this stuff, all future backends have to
too. Which does not sound great. Imagine we have "something" in
addition to worktrees and submodules in future, then all backends have
to learn about it.
So how about we move worktree and submodule support back to front-end?
The backend deals with refs, period. The file-based backend will be
given a directory where refs live in and it work on that. Backends do
not use git_path(). Backends do not care about $GIT_DIR. Access to odb
(e.g. sha-1 validation) if needed is abstracted out via a set of
callbacks. This allows submodules to give access to submodule's
separate odb. And it's getting close to the "struct repository"
mentioned somewhere in refs "TODO" comments, even though we are
nowhere close to introducing that struct.
How does that sound? In particular, is there anything wrong, or
unrealistic, with that?
For that to work, I'll probably need to add a "composite" ref_store
that combines two file-based backends (for per-repo and per-worktree
refs) to represent a unified ref store. I think your work on ref
iterator makes way for that. A bit worried about transactions though,
because I think per-repo and per-worktree updates will be separated in
two transactions. But that's probably ok because future backends, like
lmdb, will have to go through the same route anyway.
--
Duy
^ permalink raw reply
* Re: [PATCH/RFC 5/4] Redefine core.bare in multiple working tree setting
From: Duy Nguyen @ 2017-01-19 12:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqh953j2q0.fsf@gitster.mtv.corp.google.com>
Thanks. I'll shelve this for now, maybe sleep on it for a while. The
series is complete without this patch by the way, if you want to pick
it up.
On Fri, Jan 13, 2017 at 6:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> With per-worktree configuration in place, core.bare is moved to main
>> worktree's private config file. But it does not really make sense
>> because this is about _repository_. Instead we could leave core.bare in
>> the per-repo config and change/extend its definition from:
>>
>> If true this repository is assumed to be 'bare' and has no working
>> directory associated with it.
>>
>> to
>>
>> If true this repository is assumed to be 'bare' and has no _main_
>> working directory associated with it.
>>
>> In other words, linked worktrees are not covered by core.bare. This
>> definition is the same as before when it comes to single worktree setup.
>
> Up to this point, I think it is not _wrong_ per-se, but it does not
> say anything about secondary worktrees. Some may have their own
> working tree, others may be bare, and there is no way for programs
> to discover if a particular secondary worktree has or lacks its own
> working tree.
>
> Granted, "git worktree" porcelain may be incapable of creating a
> secondary worktree without a working tree, but I think the
> underlying repository layout still is capable of expressing such a
> secondary worktree.
>
> So there still is something else necessary, I suspect, to make the
> definition complete. Perhaps core.bare should be set in
> per-worktree configuration for all worktrees including the primary
> one, and made the definition/explanation of core.bare to be
> "definition of this variable, if done, must be done in per-worktree
> config file. If set to true, the worktree is 'bare' and has no
> working directory associated with it"? That makes things even more
> equal, as there is truly no "special one" at that point.
>
> I dunno.
--
Duy
^ permalink raw reply
* Re: [PATCH v5 0/4] Per-worktree config file support
From: Duy Nguyen @ 2017-01-19 12:09 UTC (permalink / raw)
To: Stefan Beller
Cc: git@vger.kernel.org, Junio C Hamano, Michael J Gruber,
Jens Lehmann, Lars Schneider, Michael Haggerty, Max Kirillov
In-Reply-To: <CAGZ79ka+zkr83tSkg_kJWoN1u3fgu1O3u1-7USEoSM1tj-53vA@mail.gmail.com>
On Wed, Jan 11, 2017 at 12:01 AM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Jan 10, 2017 at 3:25 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> Let's get this rolling again. To refresh your memory because it's half
>> a year since v4 [1], this is about letting each worktree in multi
>> worktree setup has their own config settings. The most prominent ones
>> are core.worktree, used by submodules, and core.sparseCheckout.
>
> Thanks for getting this rolling again.
>
>>
>> This time I'm not touching submodules at all. I'm leaving it in the
>> good hands of "submodule people". All I'm providing is mechanism. How
>> you use it is up to you. So the series benefits sparse checkout users
>> only.
>
> As one of the "submodule people", I have no complaints here.
>
>>
>> Not much has changed from v4, except that the migration to new config
>> layout is done automatically _update_ a config variable with "git
>> config --worktree".
>>
>> I think this one is more or less ready. I have an RFC follow-up patch
>> about core.bare, but that could be handled separately.
>
> I have read through the series and think the design is sound for worktrees
> (though I have little knowledge about them).
Submodules and multi worktrees start to look very similar, the more I
think about it. Well, except that multi worktree does not separate odb
and config files, maybe. And we have already seen both have a need to
share code (like the moving .git dir operation). I suspect I'll learn
more about submodules along the way, and you worktrees ;-)
> Now even further:
>
> So to build on top of this series, I'd like to make submodules usable
> with worktrees (i.e. shared object store, only clone/fetch once and
> all worktrees
> benefit from it), the big question is how to get the underlying data
> model right.
>
> Would a submodule go into the superprojects
>
> .git/worktrees/<worktree-name>/modules/<submodule-name>/
>
> or rather
>
> .git/modules<submodule-name>/worktrees/<worktree-name>
>
> Or both (one of them being a gitlink file pointing at the other?)
>
> I have not made up my mind, as I haven't laid out all cases that are
> relevant here.
I would go with a conservative step first, keep submodules
per-worktree. After it's sorted out. You can change the layout (e.g.
as a config extension). The latter probably has some complication (but
yeah sharing would be a big plus).
--
Duy
^ permalink raw reply
* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Duy Nguyen @ 2017-01-19 12:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, git, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <xmqqvatouwsh.fsf@gitster.mtv.corp.google.com>
On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Christian Couder <christian.couder@gmail.com> writes:
>>>
>>>> So what should we do if freshen_file() returns 0 which means that the
>>>> freshening failed?
>>>
>>> You tell me ;-) as you are the one who is proposing this feature.
>>
>> My answer is, we are not worse than freshening loose objects case
>> (especially since I took the idea from there).
>
> I do not think so, unfortunately. Loose object files with stale
> timestamps are not removed as long as objects are still reachable.
But there are plenty of unreachable loose objects, added in index,
then got replaced with new versions. cache-tree can create loose trees
too and it's been run more often, behind user's back, to take
advantage of the shortcut in unpack-trees.
> For the base/shared index file, the timestamp is the only thing that
> protects them from pruning, unless it is serving as the base file
> for the currently active $GIT_DIR/index that is split.
--
Duy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox