* Re: git clone failing when used through bundler on Docker for Windows with a shared volume
From: Philip Oakley @ 2017-01-12 20:51 UTC (permalink / raw)
To: Omar Qureshi, git
In-Reply-To: <CA+-cb7TuPd-n-HZO-60cKAysmtTaVMcviC2W+bhxz7hikbY-RA@mail.gmail.com>
From: "Omar Qureshi" <omar@omarqureshi.net>
> Hi there, I'm not sure this is the best place for this, but, this
> seems to be an issue with Git when used through Docker on Windows when
> there is a shared volume.
>
> The issue is documented at
> https://github.com/bundler/bundler/issues/5322 and I've provided a git
I think this was
7814fbe ("normalize_path_copy(): fix pushing to //server/share/dir on
Windows", 2016-12-14)
I've added a longer comment to the github issue (didn't have email at the
time)
> repository that allows you to simulate the issue, for this the
> requirements are Docker for Windows with the Docker client installed
> on WSL as well as docker-compose installed via pip.
>
> Docker for Windows will need to be configured to have a shared drive
>
> Also, it makes no difference if a tag is provided or not
>
--
Philip
^ permalink raw reply
* Re: [PATCH v2] diff: add interhunk context config option
From: Junio C Hamano @ 2017-01-12 20:56 UTC (permalink / raw)
To: Vegard Nossum; +Cc: git, René Scharfe, Pranit Bauva
In-Reply-To: <1484223671-5476-1-git-send-email-vegard.nossum@oracle.com>
Vegard Nossum <vegard.nossum@oracle.com> writes:
> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
> index 58f4bd6..d8cd854 100644
> --- a/Documentation/diff-config.txt
> +++ b/Documentation/diff-config.txt
> @@ -60,6 +60,12 @@ diff.context::
> Generate diffs with <n> lines of context instead of the default
> of 3. This value is overridden by the -U option.
>
> +diff.interHunkContext::
> + Show the context between diff hunks, up to <n> lines, thereby
> + fusing the hunks that are close to each other. The default is 0,
> + meaning no fusing will occur. This value is overridden by the
> + --inter-hunk-context option.
This is good if it were a description for
"--inter-hunk-context=<n>", but the text needs to be adjusted if it
were to be used for "diff.interHunkContext". It is unclear how the
'<n>' the description refers to comes from.
I suspect that it would be sufficient to just revert the first
sentence to exactly match the way --inter-hunk-context=<lines> is
described.
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index e6215c3..a219aa2 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -511,6 +511,8 @@ endif::git-format-patch[]
> --inter-hunk-context=<lines>::
> Show the context between diff hunks, up to the specified number
> of lines, thereby fusing hunks that are close to each other.
> + Defaults to `diff.interHunkContext` or 0 if the config option
> + is unset.
This one is good, but then "The default is 0, meaning no fusing will
occur." in "diff.interHunkContext" is misleading and unnecessary.
When "diff.interHunkContext" is not set, it simply is not set (as
opposed to having a default value of 0).
> diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh
> index e4e3e28..d9ac9d1 100755
> --- a/t/t4032-diff-inter-hunk-context.sh
> +++ b/t/t4032-diff-inter-hunk-context.sh
> @@ -16,11 +16,15 @@ f() {
> }
>
> t() {
> + use_config=""
It is more customary to just say
use_config=
All of the above are micronits that I can locally touch-up. For
now, I'll queue the following.
-- >8 --
From: Vegard Nossum <vegard.nossum@oracle.com>
Date: Thu, 12 Jan 2017 13:21:11 +0100
Subject: [PATCH] diff: add interhunk context config option
The --inter-hunk-context= option was added in commit 6d0e674a5754
("diff: add option to show context between close hunks"). This patch
allows configuring a default for this option.
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/diff-config.txt | 6 ++++++
Documentation/diff-options.txt | 2 ++
diff.c | 8 ++++++++
t/t4032-diff-inter-hunk-context.sh | 27 ++++++++++++++++++++++++++-
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index d8570f2a75..15521f5191 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -60,6 +60,12 @@ diff.context::
Generate diffs with <n> lines of context instead of the default
of 3. This value is overridden by the -U option.
+diff.interHunkContext::
+ Show the context between diff hunks, up to the specified number
+ of lines, thereby fusing the hunks that are close to each other.
+ This value serves as the default for the `--inter-hunk-context`
+ command line option.
+
diff.external::
If this config variable is set, diff generation is not
performed using the internal diff machinery, but using the
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index e6215c372c..a219aa2907 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -511,6 +511,8 @@ endif::git-format-patch[]
--inter-hunk-context=<lines>::
Show the context between diff hunks, up to the specified number
of lines, thereby fusing hunks that are close to each other.
+ Defaults to `diff.interHunkContext` or 0 if the config option
+ is unset.
-W::
--function-context::
diff --git a/diff.c b/diff.c
index e2eb6d66a9..f08cd8e033 100644
--- a/diff.c
+++ b/diff.c
@@ -32,6 +32,7 @@ static int diff_rename_limit_default = 400;
static int diff_suppress_blank_empty;
static int diff_use_color_default = -1;
static int diff_context_default = 3;
+static int diff_interhunk_context_default;
static const char *diff_word_regex_cfg;
static const char *external_diff_cmd_cfg;
static const char *diff_order_file_cfg;
@@ -239,6 +240,12 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
return -1;
return 0;
}
+ if (!strcmp(var, "diff.interhunkcontext")) {
+ diff_interhunk_context_default = git_config_int(var, value);
+ if (diff_interhunk_context_default < 0)
+ return -1;
+ return 0;
+ }
if (!strcmp(var, "diff.renames")) {
diff_detect_rename_default = git_config_rename(var, value);
return 0;
@@ -3362,6 +3369,7 @@ void diff_setup(struct diff_options *options)
options->rename_limit = -1;
options->dirstat_permille = diff_dirstat_permille_default;
options->context = diff_context_default;
+ options->interhunkcontext = diff_interhunk_context_default;
options->ws_error_highlight = ws_error_highlight_default;
DIFF_OPT_SET(options, RENAME_EMPTY);
diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh
index e4e3e28fc7..bada0cbd32 100755
--- a/t/t4032-diff-inter-hunk-context.sh
+++ b/t/t4032-diff-inter-hunk-context.sh
@@ -16,11 +16,15 @@ f() {
}
t() {
+ use_config=
+ git config --unset diff.interHunkContext
+
case $# in
4) hunks=$4; cmd="diff -U$3";;
5) hunks=$5; cmd="diff -U$3 --inter-hunk-context=$4";;
+ 6) hunks=$5; cmd="diff -U$3"; git config diff.interHunkContext $4; use_config="(diff.interHunkContext=$4) ";;
esac
- label="$cmd, $1 common $2"
+ label="$use_config$cmd, $1 common $2"
file=f$1
expected=expected.$file.$3.$hunks
@@ -89,4 +93,25 @@ t 9 lines 3 2
t 9 lines 3 2 2
t 9 lines 3 3 1
+# use diff.interHunkContext?
+t 1 line 0 0 2 config
+t 1 line 0 1 1 config
+t 1 line 0 2 1 config
+t 9 lines 3 3 1 config
+t 2 lines 0 0 2 config
+t 2 lines 0 1 2 config
+t 2 lines 0 2 1 config
+t 3 lines 1 0 2 config
+t 3 lines 1 1 1 config
+t 3 lines 1 2 1 config
+t 9 lines 3 2 2 config
+t 9 lines 3 3 1 config
+
+test_expect_success 'diff.interHunkContext invalid' '
+ git config diff.interHunkContext asdf &&
+ test_must_fail git diff &&
+ git config diff.interHunkContext -1 &&
+ test_must_fail git diff
+'
+
test_done
--
2.11.0-559-ge2476dcca1
^ permalink raw reply related
* Re: [PATCH 2/2] Use 'env' to find perl instead of fixed path
From: Pat Pannuto @ 2017-01-12 21:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <xmqqbmvcj9le.fsf@gitster.mtv.corp.google.com>
I'm happy to let this drop.
I've filed the missing perl library as a homebrew issue [1], so
hopefully this won't be an issue for folks in the future.
You may still want the 1/2 patch in this series, just to make things
internally consistent with "-w" vs "use warnings;" inside git's perl
scripts.
-Pat
[1] https://github.com/Homebrew/homebrew-core/issues/8783
On Thu, Jan 12, 2017 at 3:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> So if this patch would make it into upstream Git, I would *have* to revert
>> it in Git for Windows, adding to my already considerable maintenance burden.
>
> I do not think we want "#!/usr/bin/env $language" in the upstream,
> either.
>
^ permalink raw reply
* Re: Bug report: Git pull hang occasionally
From: Junio C Hamano @ 2017-01-12 21:12 UTC (permalink / raw)
To: Kai Zhang; +Cc: git
In-Reply-To: <E9196161-995C-4575-9560-D7E7B9A6B43D@netskope.com>
Kai Zhang <kai@netskope.com> writes:
>> On Dec 21, 2016, at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Junio C Hamano <gitster@pobox.com> writes:
>> ...
>>
>> I wonder if the latter is solved by recent patch 296b847c0d
>> ("remote-curl: don't hang when a server dies before any output",
>> 2016-11-18) on the client side.
>> ...
> After apply this patch, hanging did not happen again.
Thanks for confirming.
> Would this patch go to release in near future?
I see 296b847c0d in the message you are responding to (by the way,
do not top-post to this list).
Let's check it together.
$ git log master..296b847c0d
$ git merge-base master 296b847c0d
296b847c0d6de63353e236cfbf94163d24155529
Yup, it already is in master.
Using a third-party script "when-merged" [*1*], we can easily find
that it was merged a few days ago to 'master', after cooking in
'next' for a handful of weeks:
$ git when-merged 296b847c0d next
refs/heads/next 3ea70d01afc6305b88d33b8585f1fc41c486a182
$ git when-merged 296b847c0d master
refs/heads/master d984592043aec3c9f5b1955560a133896ca115b5
$ git show -s --format='%cI' 3ea70d01af d984592043
2016-12-05T11:38:03-08:00
2017-01-10T15:24:25-08:00
Unless people find regressions caused by this change (in which case
we may have to revert it), this will be included in the release at
the end of this cycle. http://tinyurl.com/gitCal tells us that the
current cycle is expected to complete early February.
[Footnote]
*1* git://github.com/mhagger/git-when-merged
^ permalink raw reply
* Re: Bug report: Git pull hang occasionally
From: Kai Zhang @ 2017-01-12 21:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqy3yghtio.fsf@gitster.mtv.corp.google.com>
> On Jan 12, 2017, at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Kai Zhang <kai@netskope.com> writes:
>
>>> On Dec 21, 2016, at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>> ...
>>>
>>> I wonder if the latter is solved by recent patch 296b847c0d
>>> ("remote-curl: don't hang when a server dies before any output",
>>> 2016-11-18) on the client side.
>>> ...
>
>> After apply this patch, hanging did not happen again.
>
> Thanks for confirming.
>
>> Would this patch go to release in near future?
>
> I see 296b847c0d in the message you are responding to (by the way,
> do not top-post to this list).
>
> Let's check it together.
>
> $ git log master..296b847c0d
> $ git merge-base master 296b847c0d
> 296b847c0d6de63353e236cfbf94163d24155529
>
> Yup, it already is in master.
>
> Using a third-party script "when-merged" [*1*], we can easily find
> that it was merged a few days ago to 'master', after cooking in
> 'next' for a handful of weeks:
>
> $ git when-merged 296b847c0d next
> refs/heads/next 3ea70d01afc6305b88d33b8585f1fc41c486a182
> $ git when-merged 296b847c0d master
> refs/heads/master d984592043aec3c9f5b1955560a133896ca115b5
> $ git show -s --format='%cI' 3ea70d01af d984592043
> 2016-12-05T11:38:03-08:00
> 2017-01-10T15:24:25-08:00
>
> Unless people find regressions caused by this change (in which case
> we may have to revert it), this will be included in the release at
> the end of this cycle. http://tinyurl.com/gitCal tells us that the
> current cycle is expected to complete early February.
>
>
> [Footnote]
>
> *1* git://github.com/mhagger/git-when-merged
Thank you for your help!
^ permalink raw reply
* [PATCH 0/3] updates to gitk & git-gui doc now gitview has gone
From: Philip Oakley @ 2017-01-12 21:32 UTC (permalink / raw)
To: GitList; +Cc: Junio C Hamano
gitview was removed recently.
> Sent: Tuesday, January 10, 2017 11:48 PM
> Subject: What's cooking in git.git (Jan 2017, #01; Tue, 10)
> * sb/remove-gitview (2017-01-07) 1 commit
> (merged to 'next' on 2017-01-10 at dcb3abd146)
> + contrib: remove gitview
> Will merge to 'master'.
Lets remove the reference in the gitk man page, and do some other
fixups while in the area.
Philip Oakley (3):
doc: gitk: remove gitview reference
doc: gitk: add the upstream repo location
doc: git-gui browser does not default to HEAD
Documentation/git-gui.txt | 2 +-
Documentation/gitk.txt | 14 ++++++++------
2 files changed, 9 insertions(+), 7 deletions(-)
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply
* [PATCH 2/3] doc: gitk: add the upstream repo location
From: Philip Oakley @ 2017-01-12 21:32 UTC (permalink / raw)
To: GitList; +Cc: Junio C Hamano, Paul Mackerras
In-Reply-To: <20170112213240.7972-1-philipoakley@iee.org>
Match the 'git gui' information regarding the graphical browser
and its upstream location.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
cc: Paul Mackerras <paulus@ozlabs.org>
---
Documentation/gitk.txt | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 73c02b9..9244379 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -178,9 +178,15 @@ used by default. If '$XDG_CONFIG_HOME' is not set it defaults to
History
-------
Gitk was the first graphical repository browser. It's written in
-tcl/tk and started off in a separate repository but was later merged
-into the main Git repository.
+tcl/tk.
+'gitk' is actually maintained as an independent project, but stable
+versions are distributed as part of the Git suite for the convenience
+of end users.
+
+gitk-git/ comes from Paul Mackerras's gitk project:
+
+ git://ozlabs.org/~paulus/gitk
SEE ALSO
--------
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply related
* [PATCH 3/3] doc: git-gui browser does not default to HEAD
From: Philip Oakley @ 2017-01-12 21:32 UTC (permalink / raw)
To: GitList; +Cc: Junio C Hamano, Shawn O . Pearce, Pat Thoyts
In-Reply-To: <20170112213240.7972-1-philipoakley@iee.org>
37cd4f7 ("Document git-gui, git-citool as mainporcelain manual pages",
2007-06-21) documented the default, but was shortly followed by c52c945
("git-gui: Allow blame/browser subcommands on bare repositories",
2007-07-17) which, it would appear, as a side effect, removed that default.
Finally document that change.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
cc: Shawn O. Pearce <spearce@spearce.org>
cc: Pat Thoyts <patthoyts@users.sourceforge.net>
---
Documentation/git-gui.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-gui.txt b/Documentation/git-gui.txt
index c1a3e8b..5f93f80 100644
--- a/Documentation/git-gui.txt
+++ b/Documentation/git-gui.txt
@@ -35,7 +35,7 @@ blame::
browser::
Start a tree browser showing all files in the specified
- commit (or `HEAD` by default). Files selected through the
+ commit. Files selected through the
browser are opened in the blame viewer.
citool::
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply related
* [PATCH 1/3] doc: gitk: remove gitview reference
From: Philip Oakley @ 2017-01-12 21:32 UTC (permalink / raw)
To: GitList; +Cc: Junio C Hamano, Paul Mackerras
In-Reply-To: <20170112213240.7972-1-philipoakley@iee.org>
contrib/gitview has been removed. Remove the reference.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
cc: Paul Mackerras <paulus@ozlabs.org>
---
Documentation/gitk.txt | 4 ----
1 file changed, 4 deletions(-)
diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index e382dd9..73c02b9 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -187,10 +187,6 @@ SEE ALSO
'qgit(1)'::
A repository browser written in C++ using Qt.
-'gitview(1)'::
- A repository browser written in Python using Gtk. It's based on
- 'bzrk(1)' and distributed in the contrib area of the Git repository.
-
'tig(1)'::
A minimal repository browser and Git tool output highlighter written
in C using Ncurses.
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply related
* Re: [PATCHv2 4/4] unpack-trees: support super-prefix option
From: Junio C Hamano @ 2017-01-12 21:40 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, novalis
In-Reply-To: <20170112001253.27679-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> This is only patchv4 that is rerolled, patches 1-3 remain as is.
Good timing, as I was about to send a reminder to suggest rerolling
4/4 alone ;-)
> +static const char *super_prefixed(const char *path)
> +{
There used to be a comment that explains why we keep two static
buffers around here. Even though it is in the log message, the
in-code comment would save people trouble of having to go through
"git blame" output.
I'd say something like
/*
* It is necessary and sufficient to have two static buffers
* as the return value of this function is fed to error()
* using the unpack_*_errors[] templates we can see above.
*/
perhaps.
> + static struct strbuf buf[2] = {STRBUF_INIT, STRBUF_INIT};
> + static int super_prefix_len = -1;
> + static unsigned idx = 0;
> +
If we initialize this to 1 (or even better, "ARRAY_SIZE(buf) - 1"),
then we would use buf[0] first and then buf[1], which feels more
natural to me.
Other than that, this looks OK. Will queue.
Thanks.
> + if (super_prefix_len < 0) {
> + if (!get_super_prefix())
> + super_prefix_len = 0;
> + else {
> + int i;
> +
> + super_prefix_len = strlen(get_super_prefix());
> + for (i = 0; i < ARRAY_SIZE(buf); i++)
> + strbuf_addstr(&buf[i], get_super_prefix());
> + }
> + }
> +
> + if (!super_prefix_len)
> + return path;
> +
> + if (++idx >= ARRAY_SIZE(buf))
> + idx = 0;
> +
> + strbuf_setlen(&buf[idx], super_prefix_len);
> + strbuf_addstr(&buf[idx], path);
> +
> + return buf[idx].buf;
> +}
> +
> void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
> const char *cmd)
> {
> @@ -172,7 +202,7 @@ static int add_rejected_path(struct unpack_trees_options *o,
> const char *path)
> {
> if (!o->show_all_errors)
> - return error(ERRORMSG(o, e), path);
> + return error(ERRORMSG(o, e), super_prefixed(path));
>
> /*
> * Otherwise, insert in a list for future display by
> @@ -196,7 +226,7 @@ static void display_error_msgs(struct unpack_trees_options *o)
> something_displayed = 1;
> for (i = 0; i < rejects->nr; i++)
> strbuf_addf(&path, "\t%s\n", rejects->items[i].string);
> - error(ERRORMSG(o, e), path.buf);
> + error(ERRORMSG(o, e), super_prefixed(path.buf));
> strbuf_release(&path);
> }
> string_list_clear(rejects, 0);
> @@ -1918,7 +1948,9 @@ int bind_merge(const struct cache_entry * const *src,
> o->merge_size);
> if (a && old)
> return o->gently ? -1 :
> - error(ERRORMSG(o, ERROR_BIND_OVERLAP), a->name, old->name);
> + error(ERRORMSG(o, ERROR_BIND_OVERLAP),
> + super_prefixed(a->name),
> + super_prefixed(old->name));
> if (!a)
> return keep_entry(old, o);
> else
^ permalink raw reply
* Re: [PATCH 2/2] Use 'env' to find perl instead of fixed path
From: Junio C Hamano @ 2017-01-12 21:49 UTC (permalink / raw)
To: Pat Pannuto; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <CAAnLKaGvz4Wzs36gMSdoYCg+tzx6KFCe59FNnk5zNQ-L58ww1g@mail.gmail.com>
Pat Pannuto <pat.pannuto@gmail.com> writes:
> You may still want the 1/2 patch in this series, just to make things
> internally consistent with "-w" vs "use warnings;" inside git's perl
> scripts.
I do not think so. 1/2 is justified as a prerequisite of 2/2 and
needs a different log message, so cannot be used as is.
I vaguely recall hearing arguments for and against the choice
between "#!path-to-perl -w" vs "use warnings;" but do not recall the
details to have a strong opinion either way, so we might even end up
wanting to be "internally consistent" by going the other way. Please
prepare a standalone patch with an update message to convince people
why "use warnings;" is more preferable than "#!path-to-perl -w" and
explaining that we are making things consistently use the more
preferable form, if you want to go in that direction.
Thanks.
^ permalink raw reply
* Re: [PATCH 5/5] describe: teach describe negative pattern matches
From: Junio C Hamano @ 2017-01-12 22:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jacob Keller, git, Jacob Keller
In-Reply-To: <alpine.DEB.2.20.1701121041450.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Jake,
>
> On Wed, 11 Jan 2017, Jacob Keller wrote:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Teach git-describe the `--discard` option which will allow specifying
>> a glob pattern of tags to ignore.
>
> IMHO "discard" is the wrong word, it almost sounds as if the matching tags
> would be *deleted*.
>
> Maybe `--exclude` or `--unmatch` instead?
Yeah, as j6t mentions, I think --exclude would be a good choice.
^ permalink raw reply
* Re: [PATCHv2 4/4] unpack-trees: support super-prefix option
From: Stefan Beller @ 2017-01-12 22:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Brandon Williams, David Turner
In-Reply-To: <xmqqtw94hs8f.fsf@gitster.mtv.corp.google.com>
On Thu, Jan 12, 2017 at 1:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> This is only patchv4 that is rerolled, patches 1-3 remain as is.
>
> Good timing, as I was about to send a reminder to suggest rerolling
> 4/4 alone ;-)
>
>> +static const char *super_prefixed(const char *path)
>> +{
>
> There used to be a comment that explains why we keep two static
> buffers around here. Even though it is in the log message, the
> in-code comment would save people trouble of having to go through
> "git blame" output.
>
> I'd say something like
>
> /*
> * It is necessary and sufficient to have two static buffers
> * as the return value of this function is fed to error()
> * using the unpack_*_errors[] templates we can see above.
> */
>
> perhaps.
If you think it helps, I can reroll with such a comment.
At the time of fixing up for v4 I felt like it is overly verbose.
Such a comment is only useful in understanding the choice of 2.
I thought it was sufficient in the log as once you're interested in
that function you'd read the output of blame anyway.
On the other hand having statically allocated arrays of fixed size is
dangerous in terms of maintainability, i.e. down the road someone
thinks this is a good function to reuse and then they may run into
subtle bugs as they will not be aware of the internally static buffer
that is overwritten after a certain time.
>
>> + static struct strbuf buf[2] = {STRBUF_INIT, STRBUF_INIT};
>> + static int super_prefix_len = -1;
>> + static unsigned idx = 0;
>> +
>
> If we initialize this to 1 (or even better, "ARRAY_SIZE(buf) - 1"),
> then we would use buf[0] first and then buf[1], which feels more
> natural to me.
Yes I agree, though I overcomplicating things just so that they feel
more natural seems wrong as well.
At the time I assumed that having a static variable initialized to 0
was slightly cheaper, as the init code just memsets all of .bss to 0
unlike the .data segment that has to be crafted manually.
But to get that variable into the .bss section reliably we'd have
to omit the "=0". (My compiler recognized that it can be put into
.bss because it is 0)
>
> Other than that, this looks OK. Will queue.
>
> Thanks.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 1/3] doc: gitk: remove gitview reference
From: Stefan Beller @ 2017-01-12 22:25 UTC (permalink / raw)
To: Philip Oakley; +Cc: GitList, Junio C Hamano, Paul Mackerras
In-Reply-To: <20170112213240.7972-2-philipoakley@iee.org>
On Thu, Jan 12, 2017 at 1:32 PM, Philip Oakley <philipoakley@iee.org> wrote:
> contrib/gitview has been removed. Remove the reference.
Thanks for this cleanup.
^ permalink raw reply
* Re: [PATCH/RFC 5/4] Redefine core.bare in multiple working tree setting
From: Junio C Hamano @ 2017-01-12 23:08 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <20170110113320.13119-1-pclouds@gmail.com>
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.
^ permalink raw reply
* Bug report: Documentation error in git-bisect man description
From: Manuel Ullmann @ 2017-01-12 23:02 UTC (permalink / raw)
To: git
Hi,
there is a mistake in the git-bisect description.
The second paragraph of it says ‘the terms "old" and "new" can be used
in place of "good" and "bad"’. So from a logical point of view the
description part stating the usage syntax should be:
git bisect (bad|good) [<rev>]
git bisect (old|new) [<rev>...]
instead of
git bisect (bad|new) [<rev>]
git bisect (good|old) [<rev>...]
Checked man page version of 2.11.0, but it is in my local 2.10.2 git as well.
Best regards,
Manuel
^ permalink raw reply
* [PATCH] worktree: fix an 'using plain integer as NULL pointer' warning
From: Ramsay Jones @ 2017-01-12 23:18 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Duy,
If you need to re-roll your 'nd/worktree-move' branch, could you
please squash this into the relevant patch. (commit 62985f75c1
"worktree move: refuse to move worktrees with submodules", 08-01-2017).
[BTW, this is a sparse warning, just in case you were wondering!]
Thanks!
ATB,
Ramsay Jones
builtin/worktree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 339c622e2..a1c91f154 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -528,7 +528,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
static void validate_no_submodules(const struct worktree *wt)
{
- struct index_state istate = {0};
+ struct index_state istate = { NULL };
int i, found_submodules = 0;
if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
--
2.11.0
^ permalink raw reply related
* Re: Bug report: Documentation error in git-bisect man description
From: Junio C Hamano @ 2017-01-12 23:32 UTC (permalink / raw)
To: Manuel Ullmann; +Cc: git, Christian Couder
In-Reply-To: <87r347swz1.fsf@sonnengebleicht.fritz.box>
Manuel Ullmann <ullman.alias@posteo.de> writes:
> Hi,
>
> there is a mistake in the git-bisect description.
> The second paragraph of it says ‘the terms "old" and "new" can be used
> in place of "good" and "bad"’. So from a logical point of view the
> description part stating the usage syntax should be:
>
> git bisect (bad|good) [<rev>]
> git bisect (old|new) [<rev>...]
>
> instead of
>
> git bisect (bad|new) [<rev>]
> git bisect (good|old) [<rev>...]
>
> Checked man page version of 2.11.0, but it is in my local 2.10.2 git as well.
Hmmm, I tend to agree, modulo a minor fix.
If the description were in a context inside a paragraph like this:
When you want to tell 'git bisect' that a <rev> belongs to
the newer half of the history, you say
git bisect (bad|new) [<rev>]
On the other hand, when you want to tell 'git bisect' that a
<rev> belongs to the older half of the history, you can say
git bisect (good|old) [<rev>]
then the pairing we see in the current text makes quite a lot of
sense.
But in the early part of the description section, listing the
information that logically belongs to the synopsis section, I think
the current one is misleading. You are painting commits with two
colors, and if you are from the "older vs newer" school, you say
either 'old' or 'new' as the names of these two colors, and do not
use 'bad' or 'good'. A line with "git bisect (old|new) [<rev>]" in
the list would make more sense.
Similarly, if you are from the "still good vs already bad" school,
you would either say 'good' or 'bad' so you would want to see a line
with "git bisect (good|bad) [<rev>]" in the list (not "bad|good" in
that order, but opposite).
Christian, am I talking nonsense?
^ permalink raw reply
* Re: Bug report: Documentation error in git-bisect man description
From: Junio C Hamano @ 2017-01-12 23:42 UTC (permalink / raw)
To: Manuel Ullmann; +Cc: git, Christian Couder
In-Reply-To: <xmqqd1frj1lt.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Manuel Ullmann <ullman.alias@posteo.de> writes:
>
> Hmmm, I tend to agree, modulo a minor fix.
>
> If the description were in a context inside a paragraph like this:
>
> When you want to tell 'git bisect' that a <rev> belongs to
> the newer half of the history, you say
>
> git bisect (bad|new) [<rev>]
>
> On the other hand, when you want to tell 'git bisect' that a
> <rev> belongs to the older half of the history, you can say
>
> git bisect (good|old) [<rev>]
>
> then the pairing we see in the current text makes quite a lot of
> sense.
Actually, the above is _exactly_ what was intended. I misread the
current documentation when I made the comment, and I think that the
current one _IS_ correct. The latter half of the above is not about
a single rev. You can paint multiple commits with the "older half"
color, i.e.
On the other hand, when you want to tell 'git bisect' that
one or more <rev>s belong to the older half of the history,
you can say
git bisect (good|old) [<rev>...]
In contrast, you can mark only one <rev> as newer (or "already
bad"). So pairing (bad|good) and (new|old) like you suggested
breaks the correctness of the command line description.
If (bad|new) and (good|old) bothers you because they may mislead the
readers to think bad is an opposite of new (and good is an opposite
of old), the only solution I can think of to that problem is to
expand these two lines into four and list them like this:
git bisect bad [<rev>]
git bisect good [<rev>...]
git bisect new [<rev>]
git bisect old [<rev>...]
^ permalink raw reply
* [PATCH 20/27] attr: change validity check for attribute names to use positive logic
From: Brandon Williams @ 2017-01-12 23:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pclouds, sbeller, Brandon Williams
In-Reply-To: <20170112235354.153403-1-bmwill@google.com>
From: Junio C Hamano <gitster@pobox.com>
Convert 'invalid_attr_name()' to 'attr_name_valid()' and use positive
logic for the return value. In addition create a helper function that
prints out an error message when an invalid attribute name is used.
We could later update the message to exactly spell out what the
rules for a good attribute name are, etc.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
attr.c | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/attr.c b/attr.c
index e58fa340c..5399e1cb3 100644
--- a/attr.c
+++ b/attr.c
@@ -74,23 +74,33 @@ static unsigned hash_name(const char *name, int namelen)
return val;
}
-static int invalid_attr_name(const char *name, int namelen)
+static int attr_name_valid(const char *name, size_t namelen)
{
/*
* Attribute name cannot begin with '-' and must consist of
* characters from [-A-Za-z0-9_.].
*/
if (namelen <= 0 || *name == '-')
- return -1;
+ return 0;
while (namelen--) {
char ch = *name++;
if (! (ch == '-' || ch == '.' || ch == '_' ||
('0' <= ch && ch <= '9') ||
('a' <= ch && ch <= 'z') ||
('A' <= ch && ch <= 'Z')) )
- return -1;
+ return 0;
}
- return 0;
+ return 1;
+}
+
+static void report_invalid_attr(const char *name, size_t len,
+ const char *src, int lineno)
+{
+ struct strbuf err = STRBUF_INIT;
+ strbuf_addf(&err, _("%.*s is not a valid attribute name"),
+ (int) len, name);
+ fprintf(stderr, "%s: %s:%d\n", err.buf, src, lineno);
+ strbuf_release(&err);
}
static struct git_attr *git_attr_internal(const char *name, int len)
@@ -105,7 +115,7 @@ static struct git_attr *git_attr_internal(const char *name, int len)
return a;
}
- if (invalid_attr_name(name, len))
+ if (!attr_name_valid(name, len))
return NULL;
FLEX_ALLOC_MEM(a, name, name, len);
@@ -196,17 +206,15 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
cp++;
len--;
}
- if (invalid_attr_name(cp, len)) {
- fprintf(stderr,
- "%.*s is not a valid attribute name: %s:%d\n",
- len, cp, src, lineno);
+ if (!attr_name_valid(cp, len)) {
+ report_invalid_attr(cp, len, src, lineno);
return NULL;
}
} else {
/*
* As this function is always called twice, once with
* e == NULL in the first pass and then e != NULL in
- * the second pass, no need for invalid_attr_name()
+ * the second pass, no need for attr_name_valid()
* check here.
*/
if (*cp == '-' || *cp == '!') {
@@ -258,10 +266,8 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
name += strlen(ATTRIBUTE_MACRO_PREFIX);
name += strspn(name, blank);
namelen = strcspn(name, blank);
- if (invalid_attr_name(name, namelen)) {
- fprintf(stderr,
- "%.*s is not a valid attribute name: %s:%d\n",
- namelen, name, src, lineno);
+ if (!attr_name_valid(name, namelen)) {
+ report_invalid_attr(name, namelen, src, lineno);
goto fail_return;
}
}
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH 19/27] attr: pass struct attr_check to collect_some_attrs
From: Brandon Williams @ 2017-01-12 23:53 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, gitster, pclouds, sbeller
In-Reply-To: <20170112235354.153403-1-bmwill@google.com>
The old callchain used to take an array of attr_check_item items.
Instead pass the 'attr_check' container object to 'collect_some_attrs()'
and access the fields in the data structure directly.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
attr.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/attr.c b/attr.c
index da727e3fd..e58fa340c 100644
--- a/attr.c
+++ b/attr.c
@@ -777,9 +777,7 @@ static int macroexpand_one(int nr, int rem)
* check_all_attr. If num is non-zero, only attributes in check[] are
* collected. Otherwise all attributes are collected.
*/
-static void collect_some_attrs(const char *path, int num,
- struct attr_check_item *check)
-
+static void collect_some_attrs(const char *path, struct attr_check *check)
{
struct attr_stack *stk;
int i, pathlen, rem, dirlen;
@@ -802,17 +800,18 @@ static void collect_some_attrs(const char *path, int num,
prepare_attr_stack(path, dirlen);
for (i = 0; i < attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
- if (num && !cannot_trust_maybe_real) {
+ if (check->check_nr && !cannot_trust_maybe_real) {
rem = 0;
- for (i = 0; i < num; i++) {
- if (!check[i].attr->maybe_real) {
+ for (i = 0; i < check->check_nr; i++) {
+ const struct git_attr *a = check->check[i].attr;
+ if (!a->maybe_real) {
struct attr_check_item *c;
- c = check_all_attr + check[i].attr->attr_nr;
+ c = check_all_attr + a->attr_nr;
c->value = ATTR__UNSET;
rem++;
}
}
- if (rem == num)
+ if (rem == check->check_nr)
return;
}
@@ -821,18 +820,17 @@ static void collect_some_attrs(const char *path, int num,
rem = fill(path, pathlen, basename_offset, stk, rem);
}
-static int git_check_attrs(const char *path, int num,
- struct attr_check_item *check)
+int git_check_attr(const char *path, struct attr_check *check)
{
int i;
- collect_some_attrs(path, num, check);
+ collect_some_attrs(path, check);
- for (i = 0; i < num; i++) {
- const char *value = check_all_attr[check[i].attr->attr_nr].value;
+ for (i = 0; i < check->check_nr; i++) {
+ const char *value = check_all_attr[check->check[i].attr->attr_nr].value;
if (value == ATTR__UNKNOWN)
value = ATTR__UNSET;
- check[i].value = value;
+ check->check[i].value = value;
}
return 0;
@@ -843,7 +841,7 @@ void git_all_attrs(const char *path, struct attr_check *check)
int i;
attr_check_reset(check);
- collect_some_attrs(path, check->check_nr, check->check);
+ collect_some_attrs(path, check);
for (i = 0; i < attr_nr; i++) {
const char *name = check_all_attr[i].attr->name;
@@ -856,11 +854,6 @@ void git_all_attrs(const char *path, struct attr_check *check)
}
}
-int git_check_attr(const char *path, struct attr_check *check)
-{
- return git_check_attrs(path, check->check_nr, check->check);
-}
-
struct attr_check *attr_check_alloc(void)
{
return xcalloc(1, sizeof(struct attr_check));
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH 18/27] attr: retire git_check_attrs() API
From: Brandon Williams @ 2017-01-12 23:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pclouds, sbeller, Brandon Williams
In-Reply-To: <20170112235354.153403-1-bmwill@google.com>
From: Junio C Hamano <gitster@pobox.com>
Since nobody uses the old API, make it file-scope static, and update
the documentation to describe the new API.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/technical/api-gitattributes.txt | 86 +++++++++++++++++----------
attr.c | 3 +-
attr.h | 1 -
3 files changed, 58 insertions(+), 32 deletions(-)
diff --git a/Documentation/technical/api-gitattributes.txt b/Documentation/technical/api-gitattributes.txt
index 260266867..82f5130e7 100644
--- a/Documentation/technical/api-gitattributes.txt
+++ b/Documentation/technical/api-gitattributes.txt
@@ -16,10 +16,15 @@ Data Structure
of no interest to the calling programs. The name of the
attribute can be retrieved by calling `git_attr_name()`.
-`struct git_attr_check`::
+`struct attr_check_item`::
- This structure represents a set of attributes to check in a call
- to `git_check_attr()` function, and receives the results.
+ This structure represents one attribute and its value.
+
+`struct attr_check`::
+
+ This structure represents a collection of `attr_check_item`.
+ It is passed to `git_check_attr()` function, specifying the
+ attributes to check, and receives their values.
Attribute Values
@@ -27,7 +32,7 @@ Attribute Values
An attribute for a path can be in one of four states: Set, Unset,
Unspecified or set to a string, and `.value` member of `struct
-git_attr_check` records it. There are three macros to check these:
+attr_check_item` records it. There are three macros to check these:
`ATTR_TRUE()`::
@@ -48,49 +53,51 @@ value of the attribute for the path.
Querying Specific Attributes
----------------------------
-* Prepare an array of `struct git_attr_check` to define the list of
- attributes you would want to check. To populate this array, you would
- need to define necessary attributes by calling `git_attr()` function.
+* Prepare `struct attr_check` using attr_check_initl()
+ function, enumerating the names of attributes whose values you are
+ interested in, terminated with a NULL pointer. Alternatively, an
+ empty `struct attr_check` can be prepared by calling
+ `attr_check_alloc()` function and then attributes you want to
+ ask about can be added to it with `attr_check_append()`
+ function.
* Call `git_check_attr()` to check the attributes for the path.
-* Inspect `git_attr_check` structure to see how each of the attribute in
- the array is defined for the path.
+* Inspect `attr_check` structure to see how each of the
+ attribute in the array is defined for the path.
Example
-------
-To see how attributes "crlf" and "indent" are set for different paths.
+To see how attributes "crlf" and "ident" are set for different paths.
-. Prepare an array of `struct git_attr_check` with two elements (because
- we are checking two attributes). Initialize their `attr` member with
- pointers to `struct git_attr` obtained by calling `git_attr()`:
+. Prepare a `struct attr_check` with two elements (because
+ we are checking two attributes):
------------
-static struct git_attr_check check[2];
+static struct attr_check *check;
static void setup_check(void)
{
- if (check[0].attr)
+ if (check)
return; /* already done */
- check[0].attr = git_attr("crlf");
- check[1].attr = git_attr("ident");
+ check = attr_check_initl("crlf", "ident", NULL);
}
------------
-. Call `git_check_attr()` with the prepared array of `struct git_attr_check`:
+. Call `git_check_attr()` with the prepared `struct attr_check`:
------------
const char *path;
setup_check();
- git_check_attr(path, ARRAY_SIZE(check), check);
+ git_check_attr(path, check);
------------
-. Act on `.value` member of the result, left in `check[]`:
+. Act on `.value` member of the result, left in `check->check[]`:
------------
- const char *value = check[0].value;
+ const char *value = check->check[0].value;
if (ATTR_TRUE(value)) {
The attribute is Set, by listing only the name of the
@@ -109,20 +116,39 @@ static void setup_check(void)
}
------------
+To see how attributes in argv[] are set for different paths, only
+the first step in the above would be different.
+
+------------
+static struct attr_check *check;
+static void setup_check(const char **argv)
+{
+ check = attr_check_alloc();
+ while (*argv) {
+ struct git_attr *attr = git_attr(*argv);
+ attr_check_append(check, attr);
+ argv++;
+ }
+}
+------------
+
Querying All Attributes
-----------------------
To get the values of all attributes associated with a file:
-* Call `git_all_attrs()`, which returns an array of `git_attr_check`
- structures.
+* Prepare an empty `attr_check` structure by calling
+ `attr_check_alloc()`.
+
+* Call `git_all_attrs()`, which populates the `attr_check`
+ with the attributes attached to the path.
-* Iterate over the `git_attr_check` array to examine the attribute
- names and values. The name of the attribute described by a
- `git_attr_check` object can be retrieved via
- `git_attr_name(check[i].attr)`. (Please note that no items will be
- returned for unset attributes, so `ATTR_UNSET()` will return false
- for all returned `git_array_check` objects.)
+* Iterate over the `attr_check.check[]` array to examine
+ the attribute names and values. The name of the attribute
+ described by a `attr_check.check[]` object can be retrieved via
+ `git_attr_name(check->check[i].attr)`. (Please note that no items
+ will be returned for unset attributes, so `ATTR_UNSET()` will return
+ false for all returned `attr_check.check[]` objects.)
-* Free the `git_array_check` array.
+* Free the `attr_check` struct by calling `attr_check_free()`.
diff --git a/attr.c b/attr.c
index d2eaa0410..da727e3fd 100644
--- a/attr.c
+++ b/attr.c
@@ -821,7 +821,8 @@ static void collect_some_attrs(const char *path, int num,
rem = fill(path, pathlen, basename_offset, stk, rem);
}
-int git_check_attrs(const char *path, int num, struct attr_check_item *check)
+static int git_check_attrs(const char *path, int num,
+ struct attr_check_item *check)
{
int i;
diff --git a/attr.h b/attr.h
index 971bb9a38..3db9893ef 100644
--- a/attr.h
+++ b/attr.h
@@ -52,7 +52,6 @@ extern void attr_check_free(struct attr_check *check);
*/
extern const char *git_attr_name(const struct git_attr *);
-int git_check_attrs(const char *path, int, struct attr_check_item *);
extern int git_check_attr(const char *path, struct attr_check *check);
/*
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH 17/27] attr: convert git_check_attrs() callers to use the new API
From: Brandon Williams @ 2017-01-12 23:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pclouds, sbeller, Brandon Williams
In-Reply-To: <20170112235354.153403-1-bmwill@google.com>
From: Junio C Hamano <gitster@pobox.com>
The remaining callers are all simple "I have N attributes I am
interested in. I'll ask about them with various paths one by one".
After this step, no caller to git_check_attrs() remains. After
removing it, we can extend "struct attr_check" struct with data
that can be used in optimizing the query for the specific N
attributes it contains.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
archive.c | 24 ++++++------------------
builtin/pack-objects.c | 19 +++++--------------
convert.c | 17 ++++++-----------
ll-merge.c | 33 ++++++++++++++-------------------
userdiff.c | 19 ++++++++-----------
ws.c | 19 ++++++-------------
6 files changed, 45 insertions(+), 86 deletions(-)
diff --git a/archive.c b/archive.c
index b76bd4691..3591f7d55 100644
--- a/archive.c
+++ b/archive.c
@@ -87,19 +87,6 @@ void *sha1_file_to_archive(const struct archiver_args *args,
return buffer;
}
-static void setup_archive_check(struct attr_check_item *check)
-{
- static struct git_attr *attr_export_ignore;
- static struct git_attr *attr_export_subst;
-
- if (!attr_export_ignore) {
- attr_export_ignore = git_attr("export-ignore");
- attr_export_subst = git_attr("export-subst");
- }
- check[0].attr = attr_export_ignore;
- check[1].attr = attr_export_subst;
-}
-
struct directory {
struct directory *up;
struct object_id oid;
@@ -120,10 +107,10 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
void *context)
{
static struct strbuf path = STRBUF_INIT;
+ static struct attr_check *check;
struct archiver_context *c = context;
struct archiver_args *args = c->args;
write_archive_entry_fn_t write_entry = c->write_entry;
- struct attr_check_item check[2];
const char *path_without_prefix;
int err;
@@ -137,11 +124,12 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
strbuf_addch(&path, '/');
path_without_prefix = path.buf + args->baselen;
- setup_archive_check(check);
- if (!git_check_attrs(path_without_prefix, ARRAY_SIZE(check), check)) {
- if (ATTR_TRUE(check[0].value))
+ if (!check)
+ check = attr_check_initl("export-ignore", "export-subst", NULL);
+ if (!git_check_attr(path_without_prefix, check)) {
+ if (ATTR_TRUE(check->check[0].value))
return 0;
- args->convert = ATTR_TRUE(check[1].value);
+ args->convert = ATTR_TRUE(check->check[1].value);
}
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8b8fbd814..ff8b3c12d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -894,24 +894,15 @@ static void write_pack_file(void)
written, nr_result);
}
-static void setup_delta_attr_check(struct attr_check_item *check)
-{
- static struct git_attr *attr_delta;
-
- if (!attr_delta)
- attr_delta = git_attr("delta");
-
- check[0].attr = attr_delta;
-}
-
static int no_try_delta(const char *path)
{
- struct attr_check_item check[1];
+ static struct attr_check *check;
- setup_delta_attr_check(check);
- if (git_check_attrs(path, ARRAY_SIZE(check), check))
+ if (!check)
+ check = attr_check_initl("delta", NULL);
+ if (git_check_attr(path, check))
return 0;
- if (ATTR_FALSE(check->value))
+ if (ATTR_FALSE(check->check[0].value))
return 1;
return 0;
}
diff --git a/convert.c b/convert.c
index 1b9829279..affd8ce9b 100644
--- a/convert.c
+++ b/convert.c
@@ -1085,24 +1085,19 @@ struct conv_attrs {
int ident;
};
-static const char *conv_attr_name[] = {
- "crlf", "ident", "filter", "eol", "text",
-};
-#define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name)
-
static void convert_attrs(struct conv_attrs *ca, const char *path)
{
- int i;
- static struct attr_check_item ccheck[NUM_CONV_ATTRS];
+ static struct attr_check *check;
- if (!ccheck[0].attr) {
- for (i = 0; i < NUM_CONV_ATTRS; i++)
- ccheck[i].attr = git_attr(conv_attr_name[i]);
+ if (!check) {
+ check = attr_check_initl("crlf", "ident", "filter",
+ "eol", "text", NULL);
user_convert_tail = &user_convert;
git_config(read_convert_config, NULL);
}
- if (!git_check_attrs(path, NUM_CONV_ATTRS, ccheck)) {
+ if (!git_check_attr(path, check)) {
+ struct attr_check_item *ccheck = check->check;
ca->crlf_action = git_path_check_crlf(ccheck + 4);
if (ca->crlf_action == CRLF_UNDEFINED)
ca->crlf_action = git_path_check_crlf(ccheck + 0);
diff --git a/ll-merge.c b/ll-merge.c
index 198f07aca..3a4227a1c 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -336,15 +336,6 @@ static const struct ll_merge_driver *find_ll_merge_driver(const char *merge_attr
return &ll_merge_drv[LL_TEXT_MERGE];
}
-static int git_path_check_merge(const char *path, struct attr_check_item check[2])
-{
- if (!check[0].attr) {
- check[0].attr = git_attr("merge");
- check[1].attr = git_attr("conflict-marker-size");
- }
- return git_check_attrs(path, 2, check);
-}
-
static void normalize_file(mmfile_t *mm, const char *path)
{
struct strbuf strbuf = STRBUF_INIT;
@@ -362,7 +353,7 @@ int ll_merge(mmbuffer_t *result_buf,
mmfile_t *theirs, const char *their_label,
const struct ll_merge_options *opts)
{
- static struct attr_check_item check[2];
+ static struct attr_check *check;
static const struct ll_merge_options default_opts;
const char *ll_driver_name = NULL;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
@@ -376,10 +367,14 @@ int ll_merge(mmbuffer_t *result_buf,
normalize_file(ours, path);
normalize_file(theirs, path);
}
- if (!git_path_check_merge(path, check)) {
- ll_driver_name = check[0].value;
- if (check[1].value) {
- marker_size = atoi(check[1].value);
+
+ if (!check)
+ check = attr_check_initl("merge", "conflict-marker-size", NULL);
+
+ if (!git_check_attr(path, check)) {
+ ll_driver_name = check->check[0].value;
+ if (check->check[1].value) {
+ marker_size = atoi(check->check[1].value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
@@ -398,13 +393,13 @@ int ll_merge(mmbuffer_t *result_buf,
int ll_merge_marker_size(const char *path)
{
- static struct attr_check_item check;
+ static struct attr_check *check;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
- if (!check.attr)
- check.attr = git_attr("conflict-marker-size");
- if (!git_check_attrs(path, 1, &check) && check.value) {
- marker_size = atoi(check.value);
+ if (!check)
+ check = attr_check_initl("conflict-marker-size", NULL);
+ if (!git_check_attr(path, check) && check->check[0].value) {
+ marker_size = atoi(check->check[0].value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
diff --git a/userdiff.c b/userdiff.c
index b0b44467a..109d4b9fc 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -262,25 +262,22 @@ struct userdiff_driver *userdiff_find_by_name(const char *name) {
struct userdiff_driver *userdiff_find_by_path(const char *path)
{
- static struct git_attr *attr;
- struct attr_check_item check;
-
- if (!attr)
- attr = git_attr("diff");
- check.attr = attr;
+ static struct attr_check *check;
+ if (!check)
+ check = attr_check_initl("diff", NULL);
if (!path)
return NULL;
- if (git_check_attrs(path, 1, &check))
+ if (git_check_attr(path, check))
return NULL;
- if (ATTR_TRUE(check.value))
+ if (ATTR_TRUE(check->check[0].value))
return &driver_true;
- if (ATTR_FALSE(check.value))
+ if (ATTR_FALSE(check->check[0].value))
return &driver_false;
- if (ATTR_UNSET(check.value))
+ if (ATTR_UNSET(check->check[0].value))
return NULL;
- return userdiff_find_by_name(check.value);
+ return userdiff_find_by_name(check->check[0].value);
}
struct userdiff_driver *userdiff_get_textconv(struct userdiff_driver *driver)
diff --git a/ws.c b/ws.c
index fbd876e84..7556adbd0 100644
--- a/ws.c
+++ b/ws.c
@@ -71,24 +71,17 @@ unsigned parse_whitespace_rule(const char *string)
return rule;
}
-static void setup_whitespace_attr_check(struct attr_check_item *check)
-{
- static struct git_attr *attr_whitespace;
-
- if (!attr_whitespace)
- attr_whitespace = git_attr("whitespace");
- check[0].attr = attr_whitespace;
-}
-
unsigned whitespace_rule(const char *pathname)
{
- struct attr_check_item attr_whitespace_rule;
+ static struct attr_check *attr_whitespace_rule;
+
+ if (!attr_whitespace_rule)
+ attr_whitespace_rule = attr_check_initl("whitespace", NULL);
- setup_whitespace_attr_check(&attr_whitespace_rule);
- if (!git_check_attrs(pathname, 1, &attr_whitespace_rule)) {
+ if (!git_check_attr(pathname, attr_whitespace_rule)) {
const char *value;
- value = attr_whitespace_rule.value;
+ value = attr_whitespace_rule->check[0].value;
if (ATTR_TRUE(value)) {
/* true (whitespace) */
unsigned all_rule = ws_tab_width(whitespace_rule_cfg);
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH 15/27] attr: (re)introduce git_check_attr() and struct attr_check
From: Brandon Williams @ 2017-01-12 23:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pclouds, sbeller, Brandon Williams
In-Reply-To: <20170112235354.153403-1-bmwill@google.com>
From: Junio C Hamano <gitster@pobox.com>
A common pattern to check N attributes for many paths is to
(1) prepare an array A of N attr_check_item items;
(2) call git_attr() to intern the N attribute names and fill A;
(3) repeatedly call git_check_attrs() for path with N and A;
A look-up for these N attributes for a single path P scans the
entire attr_stack, starting from the .git/info/attributes file and
then .gitattributes file in the directory the path P is in, going
upwards to find .gitattributes file found in parent directories.
An earlier commit 06a604e6 (attr: avoid heavy work when we know the
specified attr is not defined, 2014-12-28) tried to optimize out
this scanning for one trivial special case: when the attribute being
sought is known not to exist, we do not have to scan for it. While
this may be a cheap and effective heuristic, it would not work well
when N is (much) more than 1.
What we would want is a more customized way to skip irrelevant
entries in the attribute stack, and the definition of irrelevance
is tied to the set of attributes passed to git_check_attrs() call,
i.e. the set of attributes being sought. The data necessary for
this optimization needs to live alongside the set of attributes, but
a simple array of git_attr_check_elem simply does not have any place
for that.
Introduce "struct attr_check" that contains N, the number of
attributes being sought, and A, the array that holds N
attr_check_item items, and a function git_check_attr() that
takes a path P and this structure as its parameters. This structure
can later be extended to hold extra data necessary for optimization.
Also, to make it easier to write the first two steps in common
cases, introduce git_attr_check_initl() helper function, which takes
a NULL-terminated list of attribute names and initialize this
structure.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
attr.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
attr.h | 17 +++++++++++++++
2 files changed, 91 insertions(+)
diff --git a/attr.c b/attr.c
index 2f180d609..be9e398e9 100644
--- a/attr.c
+++ b/attr.c
@@ -865,6 +865,80 @@ int git_all_attrs(const char *path, int *num, struct attr_check_item **check)
return 0;
}
+struct attr_check *attr_check_alloc(void)
+{
+ return xcalloc(1, sizeof(struct attr_check));
+}
+
+int git_check_attr(const char *path, struct attr_check *check)
+{
+ return git_check_attrs(path, check->check_nr, check->check);
+}
+
+struct attr_check *attr_check_initl(const char *one, ...)
+{
+ struct attr_check *check;
+ int cnt;
+ va_list params;
+ const char *param;
+
+ va_start(params, one);
+ for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
+ ;
+ va_end(params);
+
+ check = attr_check_alloc();
+ check->check_nr = cnt;
+ check->check_alloc = cnt;
+ check->check = xcalloc(cnt, sizeof(struct attr_check_item));
+
+ check->check[0].attr = git_attr(one);
+ va_start(params, one);
+ for (cnt = 1; cnt < check->check_nr; cnt++) {
+ struct git_attr *attr;
+ param = va_arg(params, const char *);
+ if (!param)
+ die("BUG: counted %d != ended at %d",
+ check->check_nr, cnt);
+ attr = git_attr(param);
+ if (!attr)
+ die("BUG: %s: not a valid attribute name", param);
+ check->check[cnt].attr = attr;
+ }
+ va_end(params);
+ return check;
+}
+
+struct attr_check_item *attr_check_append(struct attr_check *check,
+ const struct git_attr *attr)
+{
+ struct attr_check_item *item;
+
+ ALLOC_GROW(check->check, check->check_nr + 1, check->check_alloc);
+ item = &check->check[check->check_nr++];
+ item->attr = attr;
+ return item;
+}
+
+void attr_check_reset(struct attr_check *check)
+{
+ check->check_nr = 0;
+}
+
+void attr_check_clear(struct attr_check *check)
+{
+ free(check->check);
+ check->check = NULL;
+ check->check_alloc = 0;
+ check->check_nr = 0;
+}
+
+void attr_check_free(struct attr_check *check)
+{
+ attr_check_clear(check);
+ free(check);
+}
+
void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
{
enum git_attr_direction old = direction;
diff --git a/attr.h b/attr.h
index efc7bb3b3..459347f4b 100644
--- a/attr.h
+++ b/attr.h
@@ -29,6 +29,22 @@ struct attr_check_item {
const char *value;
};
+struct attr_check {
+ int check_nr;
+ int check_alloc;
+ struct attr_check_item *check;
+};
+
+extern struct attr_check *attr_check_alloc(void);
+extern struct attr_check *attr_check_initl(const char *, ...);
+
+extern struct attr_check_item *attr_check_append(struct attr_check *check,
+ const struct git_attr *attr);
+
+extern void attr_check_reset(struct attr_check *check);
+extern void attr_check_clear(struct attr_check *check);
+extern void attr_check_free(struct attr_check *check);
+
/*
* Return the name of the attribute represented by the argument. The
* return value is a pointer to a null-delimited string that is part
@@ -37,6 +53,7 @@ struct attr_check_item {
extern const char *git_attr_name(const struct git_attr *);
int git_check_attrs(const char *path, int, struct attr_check_item *);
+extern int git_check_attr(const char *path, struct attr_check *check);
/*
* Retrieve all attributes that apply to the specified path. *num
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH 16/27] attr: convert git_all_attrs() to use "struct attr_check"
From: Brandon Williams @ 2017-01-12 23:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pclouds, sbeller, Brandon Williams
In-Reply-To: <20170112235354.153403-1-bmwill@google.com>
From: Junio C Hamano <gitster@pobox.com>
This updates the other two ways the attribute check is done via an
array of "struct attr_check_item" elements. These two niches
appear only in "git check-attr".
* The caller does not know offhand what attributes it wants to ask
about and cannot use attr_check_initl() to prepare the
attr_check structure.
* The caller may not know what attributes it wants to ask at all,
and instead wants to learn everything that the given path has.
Such a caller can call attr_check_alloc() to allocate an empty
attr_check, and then call attr_check_append() to add attribute names
one by one.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
attr.c | 38 ++++++++++++---------------------
attr.h | 9 +++-----
builtin/check-attr.c | 60 ++++++++++++++++++++++++++--------------------------
3 files changed, 47 insertions(+), 60 deletions(-)
diff --git a/attr.c b/attr.c
index be9e398e9..d2eaa0410 100644
--- a/attr.c
+++ b/attr.c
@@ -837,42 +837,32 @@ int git_check_attrs(const char *path, int num, struct attr_check_item *check)
return 0;
}
-int git_all_attrs(const char *path, int *num, struct attr_check_item **check)
+void git_all_attrs(const char *path, struct attr_check *check)
{
- int i, count, j;
+ int i;
- collect_some_attrs(path, 0, NULL);
+ attr_check_reset(check);
+ collect_some_attrs(path, check->check_nr, check->check);
- /* Count the number of attributes that are set. */
- count = 0;
- for (i = 0; i < attr_nr; i++) {
- const char *value = check_all_attr[i].value;
- if (value != ATTR__UNSET && value != ATTR__UNKNOWN)
- ++count;
- }
- *num = count;
- ALLOC_ARRAY(*check, count);
- j = 0;
for (i = 0; i < attr_nr; i++) {
+ const char *name = check_all_attr[i].attr->name;
const char *value = check_all_attr[i].value;
- if (value != ATTR__UNSET && value != ATTR__UNKNOWN) {
- (*check)[j].attr = check_all_attr[i].attr;
- (*check)[j].value = value;
- ++j;
- }
+ struct attr_check_item *item;
+ if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
+ continue;
+ item = attr_check_append(check, git_attr(name));
+ item->value = value;
}
-
- return 0;
}
-struct attr_check *attr_check_alloc(void)
+int git_check_attr(const char *path, struct attr_check *check)
{
- return xcalloc(1, sizeof(struct attr_check));
+ return git_check_attrs(path, check->check_nr, check->check);
}
-int git_check_attr(const char *path, struct attr_check *check)
+struct attr_check *attr_check_alloc(void)
{
- return git_check_attrs(path, check->check_nr, check->check);
+ return xcalloc(1, sizeof(struct attr_check));
}
struct attr_check *attr_check_initl(const char *one, ...)
diff --git a/attr.h b/attr.h
index 459347f4b..971bb9a38 100644
--- a/attr.h
+++ b/attr.h
@@ -56,13 +56,10 @@ int git_check_attrs(const char *path, int, struct attr_check_item *);
extern int git_check_attr(const char *path, struct attr_check *check);
/*
- * Retrieve all attributes that apply to the specified path. *num
- * will be set to the number of attributes on the path; **check will
- * be set to point at a newly-allocated array of git_attr_check
- * objects describing the attributes and their values. *check must be
- * free()ed by the caller.
+ * Retrieve all attributes that apply to the specified path.
+ * check holds the attributes and their values.
*/
-int git_all_attrs(const char *path, int *num, struct attr_check_item **check);
+void git_all_attrs(const char *path, struct attr_check *check);
enum git_attr_direction {
GIT_ATTR_CHECKIN,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 889264a5b..3d4704be5 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -24,12 +24,13 @@ static const struct option check_attr_options[] = {
OPT_END()
};
-static void output_attr(int cnt, struct attr_check_item *check,
- const char *file)
+static void output_attr(struct attr_check *check, const char *file)
{
int j;
+ int cnt = check->check_nr;
+
for (j = 0; j < cnt; j++) {
- const char *value = check[j].value;
+ const char *value = check->check[j].value;
if (ATTR_TRUE(value))
value = "set";
@@ -42,36 +43,38 @@ static void output_attr(int cnt, struct attr_check_item *check,
printf("%s%c" /* path */
"%s%c" /* attrname */
"%s%c" /* attrvalue */,
- file, 0, git_attr_name(check[j].attr), 0, value, 0);
+ file, 0,
+ git_attr_name(check->check[j].attr), 0, value, 0);
} else {
quote_c_style(file, NULL, stdout, 0);
- printf(": %s: %s\n", git_attr_name(check[j].attr), value);
+ printf(": %s: %s\n",
+ git_attr_name(check->check[j].attr), value);
}
-
}
}
static void check_attr(const char *prefix,
- int cnt, struct attr_check_item *check,
+ struct attr_check *check,
+ int collect_all,
const char *file)
{
char *full_path =
prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
- if (check != NULL) {
- if (git_check_attrs(full_path, cnt, check))
- die("git_check_attrs died");
- output_attr(cnt, check, file);
+
+ if (collect_all) {
+ git_all_attrs(full_path, check);
} else {
- if (git_all_attrs(full_path, &cnt, &check))
- die("git_all_attrs died");
- output_attr(cnt, check, file);
- free(check);
+ if (git_check_attr(full_path, check))
+ die("git_check_attr died");
}
+ output_attr(check, file);
+
free(full_path);
}
static void check_attr_stdin_paths(const char *prefix,
- int cnt, struct attr_check_item *check)
+ struct attr_check *check,
+ int collect_all)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf unquoted = STRBUF_INIT;
@@ -85,7 +88,7 @@ static void check_attr_stdin_paths(const char *prefix,
die("line is badly quoted");
strbuf_swap(&buf, &unquoted);
}
- check_attr(prefix, cnt, check, buf.buf);
+ check_attr(prefix, check, collect_all, buf.buf);
maybe_flush_or_die(stdout, "attribute to stdout");
}
strbuf_release(&buf);
@@ -100,7 +103,7 @@ static NORETURN void error_with_usage(const char *msg)
int cmd_check_attr(int argc, const char **argv, const char *prefix)
{
- struct attr_check_item *check;
+ struct attr_check *check;
int cnt, i, doubledash, filei;
if (!is_bare_repository())
@@ -160,28 +163,25 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
error_with_usage("No file specified");
}
- if (all_attrs) {
- check = NULL;
- } else {
- check = xcalloc(cnt, sizeof(*check));
+ check = attr_check_alloc();
+ if (!all_attrs) {
for (i = 0; i < cnt; i++) {
- const char *name;
- struct git_attr *a;
- name = argv[i];
- a = git_attr(name);
+ struct git_attr *a = git_attr(argv[i]);
if (!a)
return error("%s: not a valid attribute name",
- name);
- check[i].attr = a;
+ argv[i]);
+ attr_check_append(check, a);
}
}
if (stdin_paths)
- check_attr_stdin_paths(prefix, cnt, check);
+ check_attr_stdin_paths(prefix, check, all_attrs);
else {
for (i = filei; i < argc; i++)
- check_attr(prefix, cnt, check, argv[i]);
+ check_attr(prefix, check, all_attrs, argv[i]);
maybe_flush_or_die(stdout, "attribute to stdout");
}
+
+ attr_check_free(check);
return 0;
}
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
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