* [PATCH v2] travis-ci: fix Perforce install on macOS
From: Lars Schneider @ 2017-01-22 22:55 UTC (permalink / raw)
To: git; +Cc: gitster
The `perforce` and `perforce-server` package were moved from brew [1][2]
to cask [3]. Teach TravisCI the new location.
Perforce updates their binaries without version bumps. That made the
brew install (legitimately!) fail due to checksum mismatches. The
workaround is not necessary anymore as Cask [4] allows to disable the
checksum test for individual formulas.
[1] https://github.com/Homebrew/homebrew-binary/commit/1394e42de04d07445f82f9512627e864ff4ca4c6
[2] https://github.com/Homebrew/homebrew-binary/commit/f8da22d6b8dbcfcfdb2dfa9ac1a5e5d8e05aac2b
[3] https://github.com/caskroom/homebrew-cask/pull/29180
[4] https://caskroom.github.io/
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Hi,
this small update removes one more unnecessary line and makes the
formula name lower case (no functional reason - just looks better ;-).
@Junio: Do you prefer such a v2 as "--in-reply-to" to v1 or as separate
thread? What eases your workflow?
Thanks,
Lars
Notes:
Base Commit: 787f75f056 (787f75f0567aa8c7347544c65e9d3bc6640a27d4)
Diff on Web: https://github.com/larsxschneider/git/commit/a8ec423355
Checkout: git fetch https://github.com/larsxschneider/git travisci/brew-perforce-fix-v2 && git checkout a8ec423355
Interdiff (v1..v2):
diff --git a/.travis.yml b/.travis.yml
index c6ba8c8ec5..9c63c8c3f6 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -76,12 +76,11 @@ before_install:
;;
osx)
brew update --quiet
- brew tap homebrew/binary --quiet
# Uncomment this if you want to run perf tests:
# brew install gnu-time
brew install git-lfs gettext
brew link --force gettext
- brew install Caskroom/cask/perforce
+ brew install caskroom/cask/perforce
;;
esac;
echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)";
.travis.yml | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 3843967a69..9c63c8c3f6 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -75,20 +75,12 @@ before_install:
popd
;;
osx)
- brew_force_set_latest_binary_hash () {
- FORMULA=$1
- SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 2)
- sed -E -i.bak "s/sha256 \"[0-9a-f]{64}\"/sha256 \"$SHA\"/g" \
- "$(brew --repository homebrew/homebrew-binary)/$FORMULA.rb"
- }
brew update --quiet
- brew tap homebrew/binary --quiet
- brew_force_set_latest_binary_hash perforce
- brew_force_set_latest_binary_hash perforce-server
# Uncomment this if you want to run perf tests:
# brew install gnu-time
- brew install git-lfs perforce-server perforce gettext
+ brew install git-lfs gettext
brew link --force gettext
+ brew install caskroom/cask/perforce
;;
esac;
echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)";
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2 3/4] show-ref: Optimize show_ref a bit
From: Junio C Hamano @ 2017-01-22 22:55 UTC (permalink / raw)
To: Vladimir Panteleev; +Cc: git
In-Reply-To: <xmqqa8aivhik.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Having to do this change probably is an indication that the division
> of labour between show_ref() and show_one() up to this step needs to
> be rethought.
>
> Conceptually, "git show-ref" works in two ways:
>
> * When in --verify mode, the end user gives which refnames to
> consider showing.
>
> * Otherwise the end user gives pattern and the command infers which
> refnames to consider showing using the pattern.
>
> And for the refnames that are considered for showing, we may do
> various things, like -d to deref and --quiet to be silent. We want
> this actual "output" step to be the same between two modes of
> operation.
... also "error out if the named object did not exist" can be part
of this, which means ...
> So a better division of labour would be:
>
> * Make show_ref() about "using pattern, enumerate what refs to
> show" and call show_one().
>
> * Update show_one() and teach _it_ to handle quiet and deref_tags.
... "if (!has_sha1_file(oid->hash)) die()" in show_ref() would
probably want to be part of this update.
^ permalink raw reply
* Re: [PATCH v2 3/4] show-ref: Optimize show_ref a bit
From: Junio C Hamano @ 2017-01-22 22:47 UTC (permalink / raw)
To: Vladimir Panteleev; +Cc: git
In-Reply-To: <20170121010821.25046-4-git@thecybershadow.net>
Vladimir Panteleev <git@thecybershadow.net> writes:
> The inner `if (verify)' check was not being used before the preceding
> commit, as show_ref was never being called from a code path where
> verify was non-zero.
>
> Adding a `!verify' check to the outer if skips an unnecessary string
> comparison when verify is non-zero, and show_ref is already called
> with a reference exactly matching pattern.
>
> Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
> ---
> builtin/show-ref.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/builtin/show-ref.c b/builtin/show-ref.c
> index bcdc1a95e..3cf344d47 100644
> --- a/builtin/show-ref.c
> +++ b/builtin/show-ref.c
> @@ -43,7 +43,7 @@ static int show_ref(const char *refname, const struct object_id *oid,
> if (!match)
> return 0;
> }
> - if (pattern) {
> + if (pattern && !verify) {
> int reflen = strlen(refname);
> const char **p = pattern, *m;
> while ((m = *p++) != NULL) {
> @@ -54,9 +54,6 @@ static int show_ref(const char *refname, const struct object_id *oid,
> continue;
> if (len == reflen)
> goto match;
> - /* "--verify" requires an exact match */
> - if (verify)
> - continue;
> if (refname[reflen - len - 1] == '/')
> goto match;
> }
Having to do this change probably is an indication that the division
of labour between show_ref() and show_one() up to this step needs to
be rethought.
Conceptually, "git show-ref" works in two ways:
* When in --verify mode, the end user gives which refnames to
consider showing.
* Otherwise the end user gives pattern and the command infers which
refnames to consider showing using the pattern.
And for the refnames that are considered for showing, we may do
various things, like -d to deref and --quiet to be silent. We want
this actual "output" step to be the same between two modes of
operation.
So a better division of labour would be:
* Make show_ref() about "using pattern, enumerate what refs to
show" and call show_one().
* Update show_one() and teach _it_ to handle quiet and deref_tags.
* From cmd_show_ref(), in --verify mode, make sure the ref exists
and call show_one(), because we do not do the "using pattern,
enumerate what refs to show" at all.
And from that point of view, I think 4/4 is going in the wrong
direction.
I also think that "git show-ref --verify HEAD" should work; it is OK
that the command accepts "--head" in that case, but when in --verify
mode, the end user gives which refnames to consider showing, and
HEAD given from the command line is a signal enough that that
psuedo-ref is what the end user wants to be shown. "--head" is
about not filtering in "enumerate the ones that match the given
patterns" mode, and it feels unnecessary to require it in "--verify"
mode.
Thanks.
^ permalink raw reply
* Re: Does it make sense to show tips on blame?
From: Junio C Hamano @ 2017-01-22 22:25 UTC (permalink / raw)
To: Edmundo Carmona Antoranz; +Cc: Git List
In-Reply-To: <CAOc6etZ9QeJAf1CKh9Eraigwy45yR6eO-7Th+bZRnTa71BoPew@mail.gmail.com>
Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:
> Output is like this (from README.txt, taken from difflame on git
> itself, sorry if it's too wide):
It is not just too wide but it is line-wrapped and cannot see what
you wanted to say out of it.
What does the word "tip" mean in this context? The word is often
used to mean the commits directly pointed at by branches (i.e. the
tip of history), but I do not think that is what you are interested
in showing in this butchered "diff" output.
> 2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)
> 405d7f4af fast-import: properly fanout notes when tree is imported
> +405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2228) if (!root->tree)
> +405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2229)
> load_tree(root);
> ...
Do you mean the one-line summary for the commit? The commits that
are shown will not be at the tip most of the time (the "+" ones may
be if you happen to be running "git show" on the tip of a branch,
but that is minority if you also want to do "git log -p"), so I am
not sure it makes sense to call them "tips" in the above output.
If I were doing the above, I would probably do them as footnotes to
keep the body of the patch text (slightly) more readable. E.g.
@@ -l,k +m,n @@
2a113aee9b
+405d7f4af6 if (!root-tree)
+405d7f4af6 load_tree(root)
...
#2a113aee9b "fast-import: Proper notes tree manipulation", 2009-12-07
#405d7f4af6 "fast-import: properly fanout notes when tree is imported", 2016-12-21
> The question is if you think it makes sense to add this option to
> git-blame itself.
>
> Something like:
> git blame --tips README.txt
I do not think I would use it personally, as it would make it hard
to pipe the output of "git blame" to less and then /search things,
as extra cruft added by the option will get in the way. IOW, I do
not think we want it for human-oriented output.
On the other hand, when output from "blame" is used by scripts (or
GUI frontends), having the one-line summary would be very useful.
But I think the authors of "blame" already thought about that
usecase and included the "summary" field in the "--porcelain" output
that are meant to be consumed by scripts, so...
^ permalink raw reply
* Re: interaction between git-diff-index and git-apply
From: Junio C Hamano @ 2017-01-22 22:06 UTC (permalink / raw)
To: Ariel Davis; +Cc: git
In-Reply-To: <C45218A3-8A81-4307-AADB-FBA4F51EAC51@icloud.com>
Ariel Davis <ariel.z.davis@icloud.com> writes:
> I have noticed an interesting interaction between git-diff-index and git-apply.
> Essentially, it seems that if we start with a clean working tree, then
> git-apply a patch, then git-apply the reverse of that patch, git-diff-index
> still thinks files are modified. But then, if we git-status, git-diff-index
> seems to "realize" the files are actually not modified.
That is perfectly normal and you are making it too complex. You do
not need to involve "git apply" at all.
$ git init
$ echo hello >file && git add file && git commit -m initial
$ git diff-index HEAD
$ echo hello >file
$ git diff-index HEAD
:100644 100644 ce013625030ba8dba906f756967f9e9ca394464a 0000000000000000000000000000000000000000 M file
$ git update-index --refresh
$ git diff-index HEAD
$ exit
A few things about Git that are involved in the above are:
* There are plumbing commands that are geared more towards
scripting Git efficiently and there are end-user facing Porcelain
commands.
* Git uses cached "(l)stat" information to avoid having to inspect
the contents of the file all the time. The idea is that Git
remembers certain attributes (like size and last modified time)
of a file when the contents of the file and the blob object in
the index are the same (e.g. when you did "git add file" in the
above sequence), and it can tell a file was edited/modified if
these attributes are different from those recorded in the index
without comparing the contents of the file with the blob.
* The plumbing commands trust the cached "(l)stat" information for
efficiency and whoever uses the plumbing commands are responsible
for culling the false positives when cached "(l)stat" information
is used to see which paths are modified. They (typically these
are scripted commands) do so with "update-index --refresh".
* The Porcelain commands sacrifice the efficiency and internally do
an equivalent of "update-index --refresh" at the beginning to
hide the false positive.
After your reverse application of the patch with "git apply" (or the
second "echo hello" into the file in the above example), the
contents of the file is equivalent to what is in the index, but the
last modified timestamp (among others) is different because you
wrote into the file. If you do not do "update-index --refresh"
before running "diff-index" again, "diff-index" will notice and
report the fact that you touched that file. If you run "git status",
"git diff", etc., they internally do "update-index --refresh" and
then after that until you touch the file on the filesystem, the
cached "(l)stat" information will match and you will stop seeing the
false positive.
^ permalink raw reply
* Re: [PATCH] blame: add option to print tips (--tips)
From: Edmundo Carmona Antoranz @ 2017-01-22 21:39 UTC (permalink / raw)
To: Git List; +Cc: Edmundo Carmona Antoranz
In-Reply-To: <20170122212855.25924-1-eantoranz@gmail.com>
Hello, everybody!
So, this is a draft of what I mean by "adding tips to blame".
Example output (sample from builtin/blame.c):
15:32 $ ./git blame --tips -L 1934,1960 builtin/blame.c
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1934)
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1935) static void emit_other(struct scoreboard *sb,
struct blame_entry *ent, int opt)
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1936) {
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1937) int cnt;
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1938) const char *cp;
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1939) struct origin *suspect = ent->suspect;
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1940) struct commit_info ci;
d59f765ac9: use sha1_to_hex_r() instead of strcpy
d59f765ac9 builtin/blame.c (Jeff King 2015-09-24
17:08:03 -0400 1941) char hex[GIT_SHA1_HEXSZ + 1];
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1942) int show_raw_time = !!(opt &
OUTPUT_RAW_TIMESTAMP);
f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1943) int revision_length = (opt &
OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1944)
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1945) get_commit_info(suspect->commit, &ci, 1);
f2fd0760f6: Convert struct object to object_id
f2fd0760f6 builtin/blame.c (brian m. carlson 2015-11-10
02:22:28 +0000 1946) sha1_to_hex_r(hex,
suspect->commit->object.oid.hash);
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1947)
f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1948) if (opt & OUTPUT_SHOW_TIPS)
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1949) printf("\t%.*s: %s\n", revision_length,
hex, ci.summary.buf);
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1950)
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1951) cp = nth_line(sb, ent->lno);
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1952) for (cnt = 0; cnt < ent->num_lines; cnt++) {
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1953) char ch;
f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1954) int length = revision_length;
b11121d9e3: git-blame: show lines attributed to boundary
commits differently.
b11121d9e3 builtin-blame.c (Junio C Hamano 2006-12-01
20:45:45 -0800 1955)
b11121d9e3 builtin-blame.c (Junio C Hamano 2006-12-01
20:45:45 -0800 1956) if (suspect->commit->object.flags &
UNINTERESTING) {
e68989a739: annotate: fix for cvsserver.
e68989a739 builtin-blame.c (Junio C Hamano 2007-02-06
01:52:04 -0800 1957) if (blank_boundary)
e68989a739 builtin-blame.c (Junio C Hamano 2007-02-06
01:52:04 -0800 1958) memset(hex, ' ',
length);
7ceacdffc5: "blame -c" should be compatible with "annotate"
7ceacdffc5 builtin-blame.c (Junio C Hamano 2008-09-05
00:57:35 -0700 1959) else if (!(opt &
OUTPUT_ANNOTATE_COMPAT)) {
4c10a5caa7: blame: -b (blame.blankboundary) and --root (blame.showroot)
4c10a5caa7 builtin-blame.c (Junio C Hamano 2006-12-18
14:04:38 -0800 1960) length--;
Does it look "worthy"? And if so, would it be better to think of
something like an "aggregate" option (or something like that) that
would include the common information as tips, something like:
15:32 $ ./git blame --tips -L 1934,1960 builtin/blame.c
cee7f245dc: builtin-pickaxe.c (Junio C Hamano
2006-10-19 16:00:04 -0700) git-pickaxe: blame rewritten.
1934
1935 static void emit_other(struct scoreboard *sb, struct blame_entry
*ent, int opt)
1936 {
1937 int cnt;
1938 const char *cp;
1939 struct origin *suspect = ent->suspect;
1940 struct commit_info ci;
d59f765ac9: builtin/blame.c (Jeff King
2015-09-24 17:08:03 -0400) use sha1_to_hex_r() instead of strcpy
1941 char hex[GIT_SHA1_HEXSZ + 1];
cee7f245dc: builtin-pickaxe.c (Junio C Hamano
2006-10-19 16:00:04 -0700) git-pickaxe: blame rewritten.
1942 int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
f2aea1696f: builtin/blame.c (Edmundo Carmona Antoranz
2017-01-22 15:23:31 -0600) blame: add option to print tips (--tips)
1943 int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ?
GIT_SHA1_HEXSZ : abbrev;
cee7f245dc: builtin-pickaxe.c (Junio C Hamano
2006-10-19 16:00:04 -0700) git-pickaxe: blame rewritten.
1944
1945 get_commit_info(suspect->commit, &ci, 1);
Best regards!
On Sun, Jan 22, 2017 at 3:28 PM, Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
> ---
> builtin/blame.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 126b8c9e5..4bc449f40 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
> #define OUTPUT_NO_AUTHOR 0200
> #define OUTPUT_SHOW_EMAIL 0400
> #define OUTPUT_LINE_PORCELAIN 01000
> +#define OUTPUT_SHOW_TIPS 02000
>
> static void emit_porcelain_details(struct origin *suspect, int repeat)
> {
> @@ -1939,14 +1940,18 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
> struct commit_info ci;
> char hex[GIT_SHA1_HEXSZ + 1];
> int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
> + int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
>
> get_commit_info(suspect->commit, &ci, 1);
> sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
>
> + if (opt & OUTPUT_SHOW_TIPS)
> + printf("\t%.*s: %s\n", revision_length, hex, ci.summary.buf);
> +
> cp = nth_line(sb, ent->lno);
> for (cnt = 0; cnt < ent->num_lines; cnt++) {
> char ch;
> - int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
> + int length = revision_length;
>
> if (suspect->commit->object.flags & UNINTERESTING) {
> if (blank_boundary)
> @@ -2609,6 +2614,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
> { OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
> { OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
> OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
> + OPT_BIT(0, "tips", &output_option, N_("Show tips before content lines"), OUTPUT_SHOW_TIPS),
> OPT__ABBREV(&abbrev),
> OPT_END()
> };
> --
> 2.11.0.rc1
>
^ permalink raw reply
* [PATCH] blame: add option to print tips (--tips)
From: Edmundo Carmona Antoranz @ 2017-01-22 21:28 UTC (permalink / raw)
To: git; +Cc: Edmundo Carmona Antoranz
---
builtin/blame.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..4bc449f40 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
#define OUTPUT_NO_AUTHOR 0200
#define OUTPUT_SHOW_EMAIL 0400
#define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_SHOW_TIPS 02000
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -1939,14 +1940,18 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
struct commit_info ci;
char hex[GIT_SHA1_HEXSZ + 1];
int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
+ int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
get_commit_info(suspect->commit, &ci, 1);
sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+ if (opt & OUTPUT_SHOW_TIPS)
+ printf("\t%.*s: %s\n", revision_length, hex, ci.summary.buf);
+
cp = nth_line(sb, ent->lno);
for (cnt = 0; cnt < ent->num_lines; cnt++) {
char ch;
- int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
+ int length = revision_length;
if (suspect->commit->object.flags & UNINTERESTING) {
if (blank_boundary)
@@ -2609,6 +2614,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
{ OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
+ OPT_BIT(0, "tips", &output_option, N_("Show tips before content lines"), OUTPUT_SHOW_TIPS),
OPT__ABBREV(&abbrev),
OPT_END()
};
--
2.11.0.rc1
^ permalink raw reply related
* interaction between git-diff-index and git-apply
From: Ariel Davis @ 2017-01-22 19:09 UTC (permalink / raw)
To: git
Hello,
I have noticed an interesting interaction between git-diff-index and git-apply.
Essentially, it seems that if we start with a clean working tree, then
git-apply a patch, then git-apply the reverse of that patch, git-diff-index
still thinks files are modified. But then, if we git-status, git-diff-index
seems to "realize" the files are actually not modified.
Below are steps to reproduce, as well as information about my machine, which I
discovered this behavior on.
Reproduction instructions:
$ mkdir -p test/repo
$ cd test/repo
$ git init -q
$ echo 1 > file
$ git add file
$ git commit -qm 1
$ echo 2 >> file
$ git add file
$ git commit -qm 2
$ git diff @~ > ../patch
$ git apply --reverse ../patch
$ git diff-index --name-only @
file
$ git apply ../patch
$ git diff-index --name-only @
file
$ git status
On branch master
nothing to commit, working tree clean
$ git diff-index --name-only @
$ echo hmm
hmm
System information:
$ uname -a
Darwin azdavis-mbp 16.3.0 Darwin Kernel Version 16.3.0: Thu Nov 17 20:23:58 PST
2016; root:xnu-3789.31.2~1/RELEASE_X86_64 x86_64 i386 MacBookPro12,1 Darwin
$ sw_vers
ProductName: Mac OS X
ProductVersion: 10.12.2
BuildVersion: 16C67
$ git --version
git version 2.11.0
Thank you for your time.
Ariel Davis
^ permalink raw reply
* [PATCH v3 4/4] git gui: allow for a long recentrepo list
From: Philip Oakley @ 2017-01-22 19:53 UTC (permalink / raw)
To: GitList; +Cc: Self
In-Reply-To: <20170122195301.1784-1-philipoakley@iee.org>
The gui.recentrepo list may be longer than the maxrecent setting.
Allow extra space to show any extra entries.
In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outwith the gui, or the maxrecent setting changed
to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
lib/choose_repository.tcl | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index f39636f..80f5a59 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -142,6 +142,10 @@ constructor pick {} {
-label [mc "Recent Repositories"]
}
+ if {[set lenrecent [llength $sorted_recent]] < $maxrecent} {
+ set lenrecent $maxrecent
+ }
+
${NS}::label $w_body.space
${NS}::label $w_body.recentlabel \
-anchor w \
@@ -153,7 +157,7 @@ constructor pick {} {
-background [get_bg_color $w_body.recentlabel] \
-wrap none \
-width 50 \
- -height $maxrecent
+ -height $lenrecent
$w_recentlist tag conf link \
-foreground blue \
-underline 1
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply related
* [PATCH v3 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo
From: Philip Oakley @ 2017-01-22 19:52 UTC (permalink / raw)
To: GitList; +Cc: Self
In-Reply-To: <20170122195301.1784-1-philipoakley@iee.org>
The git gui's recent repo list may become contaminated with duplicate
entries. The git gui would barf when attempting to remove one entry.
Remove them all - there is no option within 'git config' to selectively
remove one of the entries.
This issue was reported on the 'Git User' list
(https://groups.google.com/forum/#!topic/git-users/msev4KsQGFc,
Warning: gui.recentrepo has multiply values while executing).
And also by zosrothko as a Git-for-Windows issue
https://github.com/git-for-windows/git/issues/1014.
On startup the gui checks that entries in the recentrepo list are still
valid repos and deletes thoses that are not. If duplicate entries are
present the 'git config --unset' will barf and this prevents the gui
from starting.
Subsequent patches fix other parts of recentrepo logic used for syncing
internal lists with the external .gitconfig.
Reported-by: Alexey Astakhov <asstv7@gmail.com>
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
lib/choose_repository.tcl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index 75d1da8..133ca0a 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -247,7 +247,7 @@ proc _get_recentrepos {} {
proc _unset_recentrepo {p} {
regsub -all -- {([()\[\]{}\.^$+*?\\])} $p {\\\1} p
- git config --global --unset gui.recentrepo "^$p\$"
+ git config --global --unset-all gui.recentrepo "^$p\$"
load_config 1
}
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply related
* [PATCH v3 3/4] git gui: de-dup selected repo from recentrepo history
From: Philip Oakley @ 2017-01-22 19:53 UTC (permalink / raw)
To: GitList; +Cc: Self
In-Reply-To: <20170122195301.1784-1-philipoakley@iee.org>
When the gui/user selects a repo for display, that repo is brought to
the end of the recentrepo config list. The logic can fail if there are
duplicate old entries for the repo (you cannot unset a single config
entry when duplicates are present).
Similarly, the maxrecentrepo logic could fail if older duplicate entries
are present.
The first commit of this series ({this}~2) fixed the config unsetting
issue. Rather than manipulating a local copy of the $recent list (one
cannot know how many entries were removed), simply re-read it.
We must also catch the error when the attempt to remove the second copy
from the re-read list is performed.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
lib/choose_repository.tcl | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index aa87bcc..f39636f 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -247,7 +247,7 @@ proc _get_recentrepos {} {
proc _unset_recentrepo {p} {
regsub -all -- {([()\[\]{}\.^$+*?\\])} $p {\\\1} p
- git config --global --unset-all gui.recentrepo "^$p\$"
+ catch {git config --global --unset-all gui.recentrepo "^$p\$"}
load_config 1
}
@@ -262,12 +262,11 @@ proc _append_recentrepos {path} {
set i [lsearch $recent $path]
if {$i >= 0} {
_unset_recentrepo $path
- set recent [lreplace $recent $i $i]
}
- lappend recent $path
git config --global --add gui.recentrepo $path
load_config 1
+ set recent [get_config gui.recentrepo]
if {[set maxrecent [get_config gui.maxrecentrepo]] eq {}} {
set maxrecent 10
@@ -275,7 +274,7 @@ proc _append_recentrepos {path} {
while {[llength $recent] > $maxrecent} {
_unset_recentrepo [lindex $recent 0]
- set recent [lrange $recent 1 end]
+ set recent [get_config gui.recentrepo]
}
}
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply related
* [PATCH v3 2/4] git gui: cope with duplicates in _get_recentrepo
From: Philip Oakley @ 2017-01-22 19:52 UTC (permalink / raw)
To: GitList; +Cc: Self
In-Reply-To: <20170122195301.1784-1-philipoakley@iee.org>
_get_recentrepo will fail if duplicate invalid entries are present
in the recentrepo config list. The previous commit fixed the
'git config' limitations in _unset_recentrepo by unsetting all config
entries, however this code would fail on the second attempt to unset it.
Refactor the code to pre-sort and de-duplicate the recentrepo list to
avoid a potential second unset attempt.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
lib/choose_repository.tcl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index 133ca0a..aa87bcc 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -235,14 +235,14 @@ method _invoke_next {} {
proc _get_recentrepos {} {
set recent [list]
- foreach p [get_config gui.recentrepo] {
+ foreach p [lsort -unique [get_config gui.recentrepo]] {
if {[_is_git [file join $p .git]]} {
lappend recent $p
} else {
_unset_recentrepo $p
}
}
- return [lsort $recent]
+ return $recent
}
proc _unset_recentrepo {p} {
--
2.9.0.windows.1.323.g0305acf
^ permalink raw reply related
* [PATCH v3 0/4] git gui: allow for a long recentrepo list
From: Philip Oakley @ 2017-01-22 19:52 UTC (permalink / raw)
To: GitList
Cc: Self, Junio C Hamano, Pat Thoyts, Eric Sunshine, Alexey Astakhov,
Johannes Schindelin
Way back in December 2015 I made a couple of attempts to patch up
the git-gui's recentrepo list in the face of duplicate entries in
the .gitconfig
The series end at http://public-inbox.org/git/9731888BD4C348F5BFC82FA96D978034@PhilipOakley/
A similar problem was reported recently on the Git for Windows list
https://github.com/git-for-windows/git/issues/1014 were a full
recentrepo list also stopped the gui working.
This series applies to Pat Thoyt's upstream Github repo as a merge
ready Pull Request https://github.com/patthoyts/git-gui/pull/10.
Hence if applied here it should be into the sub-tree git/git-gui.
I believe I've covered the points raised by Junio in the last review
and allowed for cases where the recentrepo list is now longer than the
maximum (which can be configured, both up and down).
I've cc'd just the cover letter to those involved previously, rather
than the series to avoid spamming.
There are various other low hanging fruit that the eager could look at
which are detailed at the end of the GfW/issues/104 referenced above.
Philip Oakley (4):
git-gui: remove duplicate entries from .gitconfig's gui.recentrepo
git gui: cope with duplicates in _get_recentrepo
git gui: de-dup selected repo from recentrepo history
git gui: allow for a long recentrepo list
lib/choose_repository.tcl | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
--
2.9.0.windows.1.323.g0305acf
---
cc: Junio C Hamano <gitster@pobox.com>
cc: Pat Thoyts <patthoyts@users.sourceforge.net>,
cc: Eric Sunshine <sunshine@sunshineco.com>,
cc: Alexey Astakhov <asstv7@gmail.com>
cc: Johannes Schindelin <johannes.schindelin@gmx.de>
^ permalink raw reply
* [PATCH 3/3] git-prompt.sh: fix for submodule 'dirty' indicator
From: Benjamin Fuchs @ 2017-01-22 19:30 UTC (permalink / raw)
To: git; +Cc: szeder.dev, sbeller, email
Fixing wrong git diff line.
---
contrib/completion/git-prompt.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c44b9a2..43b28e9 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -306,9 +306,9 @@ __git_ps1_submodule ()
local submodule_name="$(basename "$git_dir")"
if [ "$submodule_name" != ".git" ] && [ "$submodule_name" != "." ]; then
local parent_top="${git_dir%.git*}"
- local submodule_top="${git_dir#*modules}"
+ local submodule_top="${git_dir#*modules/}"
local status=""
- git diff -C "$parent_top" --no-ext-diff --ignore-submodules=dirty --quiet -- "$submodule_top" 2>/dev/null || status="+"
+ git -C "$parent_top" diff --no-ext-diff --ignore-submodules=dirty --quiet -- "$submodule_top" 2>/dev/null || status="+"
printf "$status$submodule_name:"
fi
}
@@ -544,7 +544,7 @@ __git_ps1 ()
local sub=""
if [ -n "${GIT_PS1_SHOWSUBMODULE}" ]; then
- sub="$(__git_ps1_submodule $g)"
+ sub="$(__git_ps1_submodule "$g")"
fi
local f="$w$i$s$u"
--
2.7.4
^ permalink raw reply related
* Re: [RFC] Case insensitive Git attributes
From: Dakota Hawkins @ 2017-01-22 19:25 UTC (permalink / raw)
To: Junio C Hamano, Duy Nguyen
Cc: Johannes Schindelin, Stefan Beller, Lars Schneider, git
Apologies for the delayed bump. I think because we're talking about
affecting the behavior of .gitattributes that it would be better to
have a distinct .gitattributes option, whether or not you also have a
similar config option.
Since .gitattributes is versioned and config options are not, I think
this takes it out of the realm of personal preference. It's already
too easy for somebody to screw up and not have a "required" filter
driver (e.g. git lfs) configured and damage a repo by getting
unnoticed and unfiltered content committed.
I would love a .gitconfig option I could set that would let me stop
manually ignoring case in globs for git commands, but this might
actually make things worse for people if it were included as a config
option only -- suddenly attributes could be applied to different files
for different people.
Of course, if git supported a subset of config options you could
actually version and ensure everybody else has, this wouldn't be a
problem ;)
So, I think the correct (for today) solution is to have two options.
One for .gitattributes globs and one for everything else. I realize
that this might be somewhat controversial, so I wanted to see what
everybody thought.
^ permalink raw reply
* Re: [RFC] what content should go in https://git-scm.com/doc/ext
From: Christian Couder @ 2017-01-22 19:13 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170121135545.gi6crct66msylhpr@sigill.intra.peff.net>
On Sat, Jan 21, 2017 at 2:55 PM, Jeff King <peff@peff.net> wrote:
> I'm wondering if anybody has opinions on:
>
> https://github.com/git/git-scm.com/pull/924
>
> (and I suspect most people in this community do not read pull requests
> there, hence this post).
Yeah, thanks for posting it here.
> Basically, we maintain a list of links to outside documentation, as well
> as to books. Somebody has requested a link to their paid tutorial
> course. How should we handle it?
I think it depends if you are ready and willing to handle more changes.
> If the resource is valuable, then it may make sense to link to it, even
> if it costs money. We already do this with book links, and my policy has
> been to link any relevant book that is requested (I don't read them for
> quality, so I have no opinions).
>
> Should we do the same for tutorial content like this?
It could make sense to link to tutorial content, and then it could
also make sense to link to trainings (online and maybe offline too),
but my fear is that you could then have many people who want to
advertise their tutorials and trainings, and that it might be
difficult to draw a line while keeping the playing field even.
But maybe it's worth trying anyway.
^ permalink raw reply
* [DEMO][PATCH v2 6/5] compat: add a qsort_s() implementation based on GNU's qsort_r(1)
From: René Scharfe @ 2017-01-22 18:02 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano, Johannes Schindelin
In-Reply-To: <67ac53cd-3fc0-8bd0-30f4-129281c3090f@web.de>
Implement qsort_s() as a wrapper to the GNU version of qsort_r(1) and
use it on Linux. Performance increases slightly:
Test HEAD^ HEAD
--------------------------------------------------------------------
0071.2: sort(1) 0.10(0.20+0.02) 0.10(0.21+0.01) +0.0%
0071.3: string_list_sort() 0.17(0.15+0.01) 0.16(0.15+0.01) -5.9%
Additionally the unstripped size of compat/qsort_s.o falls from 24576
to 16544 bytes in my build.
IMHO these savings aren't worth the increased complexity of having to
support two implementations.
---
Makefile | 6 ++++++
compat/qsort_s.c | 18 ++++++++++++++++++
config.mak.uname | 1 +
3 files changed, 25 insertions(+)
diff --git a/Makefile b/Makefile
index 53ecc84e28..46db1c773f 100644
--- a/Makefile
+++ b/Makefile
@@ -282,6 +282,9 @@ all::
# Define HAVE_ISO_QSORT_S if your platform provides a qsort_s() that's
# compatible with the one described in C11 Annex K.
#
+# Define HAVE_GNU_QSORT_R if your platform provides a qsort_r() that's
+# compatible with the one introduced with glibc 2.8.
+#
# Define UNRELIABLE_FSTAT if your system's fstat does not return the same
# information on a not yet closed file that lstat would return for the same
# file after it was closed.
@@ -1426,6 +1429,9 @@ ifdef HAVE_ISO_QSORT_S
else
COMPAT_OBJS += compat/qsort_s.o
endif
+ifdef HAVE_GNU_QSORT_R
+ COMPAT_CFLAGS += -DHAVE_GNU_QSORT_R
+endif
ifdef RUNTIME_PREFIX
COMPAT_CFLAGS += -DRUNTIME_PREFIX
endif
diff --git a/compat/qsort_s.c b/compat/qsort_s.c
index 52d1f0a73d..763ee1faae 100644
--- a/compat/qsort_s.c
+++ b/compat/qsort_s.c
@@ -1,5 +1,21 @@
#include "../git-compat-util.h"
+#if defined HAVE_GNU_QSORT_R
+
+int git_qsort_s(void *b, size_t n, size_t s,
+ int (*cmp)(const void *, const void *, void *), void *ctx)
+{
+ if (!n)
+ return 0;
+ if (!b || !cmp)
+ return -1;
+
+ qsort_r(b, n, s, cmp, ctx);
+ return 0;
+}
+
+#else
+
/*
* A merge sort implementation, simplified from the qsort implementation
* by Mike Haertel, which is a part of the GNU C Library.
@@ -67,3 +83,5 @@ int git_qsort_s(void *b, size_t n, size_t s,
}
return 0;
}
+
+#endif
diff --git a/config.mak.uname b/config.mak.uname
index 447f36ac2e..a1858f54ff 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -37,6 +37,7 @@ ifeq ($(uname_S),Linux)
NEEDS_LIBRT = YesPlease
HAVE_GETDELIM = YesPlease
SANE_TEXT_GREP=-a
+ HAVE_GNU_QSORT_R = YesPlease
endif
ifeq ($(uname_S),GNU/kFreeBSD)
HAVE_ALLOCA_H = YesPlease
--
2.11.0
^ permalink raw reply related
* [PATCH v2 2/5] add QSORT_S
From: René Scharfe @ 2017-01-22 17:52 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano, Johannes Schindelin
In-Reply-To: <67ac53cd-3fc0-8bd0-30f4-129281c3090f@web.de>
Add the macro QSORT_S, a convenient wrapper for qsort_s() that infers
the size of the array elements and dies on error.
Basically all possible errors are programming mistakes (passing NULL as
base of a non-empty array, passing NULL as comparison function,
out-of-bounds accesses), so terminating the program should be acceptable
for most callers.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
git-compat-util.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index f706744e6a..f46d40e4a4 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -994,6 +994,11 @@ int git_qsort_s(void *base, size_t nmemb, size_t size,
#define qsort_s git_qsort_s
#endif
+#define QSORT_S(base, n, compar, ctx) do { \
+ if (qsort_s((base), (n), sizeof(*(base)), compar, ctx)) \
+ die("BUG: qsort_s() failed"); \
+} while (0)
+
#ifndef REG_STARTEND
#error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
#endif
--
2.11.0
^ permalink raw reply related
* [PATCH v2 5/5] ref-filter: use QSORT_S in ref_array_sort()
From: René Scharfe @ 2017-01-22 17:58 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano, Johannes Schindelin
In-Reply-To: <67ac53cd-3fc0-8bd0-30f4-129281c3090f@web.de>
Pass the array of sort keys to compare_refs() via the context parameter
of qsort_s() instead of using a global variable; that's cleaner and
simpler. If ref_array_sort() is to be called from multiple parallel
threads then care still needs to be taken that the global variable
used_atom is not modified concurrently.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
ref-filter.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1a978405e6..3975022c88 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1589,8 +1589,7 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
return (s->reverse) ? -cmp : cmp;
}
-static struct ref_sorting *ref_sorting;
-static int compare_refs(const void *a_, const void *b_)
+static int compare_refs(const void *a_, const void *b_, void *ref_sorting)
{
struct ref_array_item *a = *((struct ref_array_item **)a_);
struct ref_array_item *b = *((struct ref_array_item **)b_);
@@ -1606,8 +1605,7 @@ static int compare_refs(const void *a_, const void *b_)
void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
{
- ref_sorting = sorting;
- QSORT(array->items, array->nr, compare_refs);
+ QSORT_S(array->items, array->nr, compare_refs, sorting);
}
static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state)
--
2.11.0
^ permalink raw reply related
* [PATCH v2 4/5] string-list: use QSORT_S in string_list_sort()
From: René Scharfe @ 2017-01-22 17:57 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano, Johannes Schindelin
In-Reply-To: <67ac53cd-3fc0-8bd0-30f4-129281c3090f@web.de>
Pass the comparison function to cmp_items() via the context parameter of
qsort_s() instead of using a global variable. That allows calling
string_list_sort() from multiple parallel threads.
Our qsort_s() in compat/ is slightly slower than qsort(1) from glibc
2.24 for sorting lots of lines:
Test HEAD^ HEAD
---------------------------------------------------------------------
0071.2: sort(1) 0.10(0.22+0.01) 0.09(0.21+0.00) -10.0%
0071.3: string_list_sort() 0.16(0.15+0.01) 0.17(0.15+0.00) +6.3%
GNU sort(1) version 8.26 is significantly faster because it uses
multiple parallel threads; with the unportable option --parallel=1 it
becomes slower:
Test HEAD^ HEAD
--------------------------------------------------------------------
0071.2: sort(1) 0.21(0.18+0.01) 0.20(0.18+0.01) -4.8%
0071.3: string_list_sort() 0.16(0.13+0.02) 0.17(0.15+0.01) +6.3%
There is some instability -- the numbers for the sort(1) check shouldn't
be affected by this patch. Anyway, the performance of our qsort_s()
implementation is apparently good enough, at least for this test.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
string-list.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/string-list.c b/string-list.c
index 8c83cac189..45016ad86d 100644
--- a/string-list.c
+++ b/string-list.c
@@ -211,21 +211,18 @@ struct string_list_item *string_list_append(struct string_list *list,
list->strdup_strings ? xstrdup(string) : (char *)string);
}
-/* Yuck */
-static compare_strings_fn compare_for_qsort;
-
-/* Only call this from inside string_list_sort! */
-static int cmp_items(const void *a, const void *b)
+static int cmp_items(const void *a, const void *b, void *ctx)
{
+ compare_strings_fn cmp = ctx;
const struct string_list_item *one = a;
const struct string_list_item *two = b;
- return compare_for_qsort(one->string, two->string);
+ return cmp(one->string, two->string);
}
void string_list_sort(struct string_list *list)
{
- compare_for_qsort = list->cmp ? list->cmp : strcmp;
- QSORT(list->items, list->nr, cmp_items);
+ QSORT_S(list->items, list->nr, cmp_items,
+ list->cmp ? list->cmp : strcmp);
}
struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
--
2.11.0
^ permalink raw reply related
* [PATCH v2 3/5] perf: add basic sort performance test
From: René Scharfe @ 2017-01-22 17:53 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano, Johannes Schindelin
In-Reply-To: <67ac53cd-3fc0-8bd0-30f4-129281c3090f@web.de>
Add a sort command to test-string-list that reads lines from stdin,
stores them in a string_list and then sorts it. Use it in a simple
perf test script to measure the performance of string_list_sort().
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
t/helper/test-string-list.c | 25 +++++++++++++++++++++++++
t/perf/p0071-sort.sh | 26 ++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
create mode 100755 t/perf/p0071-sort.sh
diff --git a/t/helper/test-string-list.c b/t/helper/test-string-list.c
index 4a68967bd1..c502fa16d3 100644
--- a/t/helper/test-string-list.c
+++ b/t/helper/test-string-list.c
@@ -97,6 +97,31 @@ int cmd_main(int argc, const char **argv)
return 0;
}
+ if (argc == 2 && !strcmp(argv[1], "sort")) {
+ struct string_list list = STRING_LIST_INIT_NODUP;
+ struct strbuf sb = STRBUF_INIT;
+ struct string_list_item *item;
+
+ strbuf_read(&sb, 0, 0);
+
+ /*
+ * Split by newline, but don't create a string_list item
+ * for the empty string after the last separator.
+ */
+ if (sb.buf[sb.len - 1] == '\n')
+ strbuf_setlen(&sb, sb.len - 1);
+ string_list_split_in_place(&list, sb.buf, '\n', -1);
+
+ string_list_sort(&list);
+
+ for_each_string_list_item(item, &list)
+ puts(item->string);
+
+ string_list_clear(&list, 0);
+ strbuf_release(&sb);
+ return 0;
+ }
+
fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
argv[1] ? argv[1] : "(there was none)");
return 1;
diff --git a/t/perf/p0071-sort.sh b/t/perf/p0071-sort.sh
new file mode 100755
index 0000000000..7c9a35a646
--- /dev/null
+++ b/t/perf/p0071-sort.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description='Basic sort performance tests'
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+test_expect_success 'setup' '
+ git ls-files --stage "*.[ch]" "*.sh" |
+ cut -f2 -d" " |
+ git cat-file --batch >unsorted
+'
+
+test_perf 'sort(1)' '
+ sort <unsorted >expect
+'
+
+test_perf 'string_list_sort()' '
+ test-string-list sort <unsorted >actual
+'
+
+test_expect_success 'string_list_sort() sorts like sort(1)' '
+ test_cmp_bin expect actual
+'
+
+test_done
--
2.11.0
^ permalink raw reply related
* [PATCH v2 1/5] compat: add qsort_s()
From: René Scharfe @ 2017-01-22 17:51 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano, Johannes Schindelin
In-Reply-To: <67ac53cd-3fc0-8bd0-30f4-129281c3090f@web.de>
The function qsort_s() was introduced with C11 Annex K; it provides the
ability to pass a context pointer to the comparison function, supports
the convention of using a NULL pointer for an empty array and performs a
few safety checks.
Add an implementation based on compat/qsort.c for platforms that lack a
native standards-compliant qsort_s() (i.e. basically everyone). It
doesn't perform the full range of possible checks: It uses size_t
instead of rsize_t and doesn't check nmemb and size against RSIZE_MAX
because we probably don't have the restricted size type defined. For
the same reason it returns int instead of errno_t.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Makefile | 8 +++++++
compat/qsort_s.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
git-compat-util.h | 6 +++++
3 files changed, 83 insertions(+)
create mode 100644 compat/qsort_s.c
diff --git a/Makefile b/Makefile
index 27afd0f378..53ecc84e28 100644
--- a/Makefile
+++ b/Makefile
@@ -279,6 +279,9 @@ all::
# is a simplified version of the merge sort used in glibc. This is
# recommended if Git triggers O(n^2) behavior in your platform's qsort().
#
+# Define HAVE_ISO_QSORT_S if your platform provides a qsort_s() that's
+# compatible with the one described in C11 Annex K.
+#
# Define UNRELIABLE_FSTAT if your system's fstat does not return the same
# information on a not yet closed file that lstat would return for the same
# file after it was closed.
@@ -1418,6 +1421,11 @@ ifdef INTERNAL_QSORT
COMPAT_CFLAGS += -DINTERNAL_QSORT
COMPAT_OBJS += compat/qsort.o
endif
+ifdef HAVE_ISO_QSORT_S
+ COMPAT_CFLAGS += -DHAVE_ISO_QSORT_S
+else
+ COMPAT_OBJS += compat/qsort_s.o
+endif
ifdef RUNTIME_PREFIX
COMPAT_CFLAGS += -DRUNTIME_PREFIX
endif
diff --git a/compat/qsort_s.c b/compat/qsort_s.c
new file mode 100644
index 0000000000..52d1f0a73d
--- /dev/null
+++ b/compat/qsort_s.c
@@ -0,0 +1,69 @@
+#include "../git-compat-util.h"
+
+/*
+ * A merge sort implementation, simplified from the qsort implementation
+ * by Mike Haertel, which is a part of the GNU C Library.
+ * Added context pointer, safety checks and return value.
+ */
+
+static void msort_with_tmp(void *b, size_t n, size_t s,
+ int (*cmp)(const void *, const void *, void *),
+ char *t, void *ctx)
+{
+ char *tmp;
+ char *b1, *b2;
+ size_t n1, n2;
+
+ if (n <= 1)
+ return;
+
+ n1 = n / 2;
+ n2 = n - n1;
+ b1 = b;
+ b2 = (char *)b + (n1 * s);
+
+ msort_with_tmp(b1, n1, s, cmp, t, ctx);
+ msort_with_tmp(b2, n2, s, cmp, t, ctx);
+
+ tmp = t;
+
+ while (n1 > 0 && n2 > 0) {
+ if (cmp(b1, b2, ctx) <= 0) {
+ memcpy(tmp, b1, s);
+ tmp += s;
+ b1 += s;
+ --n1;
+ } else {
+ memcpy(tmp, b2, s);
+ tmp += s;
+ b2 += s;
+ --n2;
+ }
+ }
+ if (n1 > 0)
+ memcpy(tmp, b1, n1 * s);
+ memcpy(b, t, (n - n2) * s);
+}
+
+int git_qsort_s(void *b, size_t n, size_t s,
+ int (*cmp)(const void *, const void *, void *), void *ctx)
+{
+ const size_t size = st_mult(n, s);
+ char buf[1024];
+
+ if (!n)
+ return 0;
+ if (!b || !cmp)
+ return -1;
+
+ if (size < sizeof(buf)) {
+ /* The temporary array fits on the small on-stack buffer. */
+ msort_with_tmp(b, n, s, cmp, buf, ctx);
+ } else {
+ /* It's somewhat large, so malloc it. */
+ char *tmp = xmalloc(size);
+ msort_with_tmp(b, n, s, cmp, tmp, ctx);
+ free(tmp);
+ }
+ return 0;
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index 87237b092b..f706744e6a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -988,6 +988,12 @@ static inline void sane_qsort(void *base, size_t nmemb, size_t size,
qsort(base, nmemb, size, compar);
}
+#ifndef HAVE_ISO_QSORT_S
+int git_qsort_s(void *base, size_t nmemb, size_t size,
+ int (*compar)(const void *, const void *, void *), void *ctx);
+#define qsort_s git_qsort_s
+#endif
+
#ifndef REG_STARTEND
#error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
#endif
--
2.11.0
^ permalink raw reply related
* [PATCH v2 0/5] string-list: make string_list_sort() reentrant
From: René Scharfe @ 2017-01-22 17:47 UTC (permalink / raw)
To: Git List; +Cc: Jeff King, Junio C Hamano, Johannes Schindelin
Use qsort_s() from C11 Annex K to make string_list_sort() safer, in
particular when called from parallel threads.
Changes from v1:
* Renamed HAVE_QSORT_S to HAVE_ISO_QSORT_S in Makefile to disambiguate.
* Added basic perf test (patch 3).
* Converted a second caller to QSORT_S, in ref-filter.c (patch 5).
compat: add qsort_s()
add QSORT_S
perf: add basic sort performance test
string-list: use QSORT_S in string_list_sort()
ref-filter: use QSORT_S in ref_array_sort()
Makefile | 8 ++++++
compat/qsort_s.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
git-compat-util.h | 11 ++++++++
ref-filter.c | 6 ++--
string-list.c | 13 ++++-----
t/helper/test-string-list.c | 25 ++++++++++++++++
t/perf/p0071-sort.sh | 26 +++++++++++++++++
7 files changed, 146 insertions(+), 12 deletions(-)
create mode 100644 compat/qsort_s.c
create mode 100755 t/perf/p0071-sort.sh
--
2.11.0
^ permalink raw reply
* RE: [PATCH] git-p4: Fix git-p4.mapUser on Windows
From: George Vanburgh @ 2017-01-22 14:14 UTC (permalink / raw)
To: 'Luke Diamand'
Cc: 'Lars Schneider', 'Git mailing list'
In-Reply-To: <CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com>
> On 22 Jan 2017, at 13:05, Luke Diamand <luke@diamand.org>
>
> I'm confused....see below.
That now makes two of us!
I think I've figured out where I messed up, see below.
>
> On 21 January 2017 at 15:21, George Vanburgh <george@vanburgh.me>
> wrote:
> >> On 21 Jan 2017, at 13:33, Lars Schneider <larsxschneider@gmail.com>
> >> > On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me>
> >> wrote:
> >> >
> >> > From: George Vanburgh <gvanburgh@bloomberg.net>
> >> >
> >> > When running git-p4 on Windows, with multiple git-p4.mapUser
> >> > entries in git config - no user mappings are applied to the
> >> > generated
> > repository.
> >> >
> >> > Reproduction Steps:
> >> >
> >> > 1. Add multiple git-p4.mapUser entries to git config on a Windows
> >> > machine
> >> > 2. Attempt to clone a p4 repository
> >> >
> >> > None of the user mappings will be applied.
> >> >
> >> > This issue is caused by the fact that gitConfigList, uses
> >> > split(os.linesep) to convert the output of git config --get-all
> >> > into a list.
> >> >
> >> > On Windows, os.linesep is equal to '\r\n' - however git.exe returns
> >> > configuration with a line seperator of '\n'. This leads to the list
> >> > returned by gitConfigList containing only one element - which
> >> > contains the full output of git config --get-all in string form.
> >> > This causes problems for the code introduced to
> >> > getUserMapFromPerforceServer in 10d08a1.
> >> >
> >> > This issue should be caught by the test introduced in 10d08a1, and
> >> > would require running on Windows to reproduce. When running inside
> >> > MinGW/Cygwin, however, os.linesep correctly returns '\n', and
> >> > everything works as expected.
> >>
> >> This surprises me. I would expect `\r\n` in a MinGW env...
> >> Nevertheless, I wouldn't have caught that as I don't run the git-p4
> >> tests
> > on
> >> Windows...
> >
> > It appears I was mistaken - the successful tests I ran were actually
> > under the Ubuntu subsystem for Windows, which (obviously) passed.
> >
> > Just did a quick experiment:
> >
> > Git Bash (MinGW):
> > georg@TEMPEST MINGW64 ~
> > $ python -c "import os
> > print(repr(os.linesep))"
> > '\r\n'
> >
> > Powershell:
> > PS C:\Users\georg> python -c "import os
> >>> print(repr(os.linesep))"
> > '\r\n'
> >
> > Ubuntu subsystem for Windows:
> > george@TEMPEST:~$ python -c "import os print(repr(os.linesep))"
> > '\n'
> >
> > So this issue applies to git-p4 running under both PowerShell and MinGW.
> >
> >>
> >>
> >> > The simplest fix for this issue would be to convert the line split
> >> > logic inside gitConfigList to use splitlines(), which splits on any
> >> > standard line delimiter. However, this function was only introduced
> >> > in Python 2.7, and would mean a bump in the minimum required
> >> > version of Python required to run git-p4. The alternative fix,
> >> > implemented here, is to use '\n' as a delimiter, which git.exe
> >> > appears to output consistently on Windows anyway.
>
> Have you tried a 2.6 Python with splitlines? I just tried this code and it seems
> fine.
>
> > val = s.strip().splitlines()
> > print("splitlines:", val)
>
> Tried with 2.7 and 2.6, and bot print out an array of strings returned from
> read_pipe().
>
> And 'grep -r splitlines' on the Python2.6 source has lots of hits.
Ah - it appears I was looking in the wrong part of the Python
documentation - which is what lead me to think this.
From the Python 2.4 documentation:
https://docs.python.org/release/2.4/lib/string-methods.html
Splitlines is a built-in method, not part of the string class.
> george@TEMPEST:~# /home/george/.pyenv/versions/2.4.1/bin/python -c 'import string
> print("test1\ntest2\r\ntest3".splitlines())'
> ['test1', 'test2', 'test3']
>
> >>
> >> Well, that also means if we ever use splitlines() then your fix below
> > would
> >> brake the code, right?
> >>
> >> Python 2.7 was released 7 years ago in 2010.
> >
> > Now I feel old...
>
> :-)
>
> >
> >> Therefore, I would vote to
> >> bump the minimum version. But that's just my opinion :-)
> >
> > I feel like splitlines is the better/safer fix - but figured bumping
> > the minimum Python version was probably part of a wider discussion. If
> > it's something people are comfortable with - I'd be happy to rework
> > the fix to use splitlines.
> > Luke - do you have any thoughts on this?
>
> I agree that we have to stop supporting 2.6 at some point (and start
> supporting 3.x, but that's another world of hurt). But does 2.6 really not have
> splitlines?
The minimum supported version is currently Python 2.4, no?
>
> If you send a patch with splitlines I can try it out (although I guess it could be
> broken under Windows).
Given that there doesn't _actually_ seem to be an issue with using splitlines in
2.4 (sorry for the confusion!) - I'll test out a patch using splitlines on
Windows, and resubmit =]
>
> Luke
^ permalink raw reply
* Re: [PATCH] git-p4: Fix git-p4.mapUser on Windows
From: Luke Diamand @ 2017-01-22 13:04 UTC (permalink / raw)
To: George Vanburgh; +Cc: Lars Schneider, Git mailing list
In-Reply-To: <000f01d273fa$05358ee0$0fa0aca0$@vanburgh.me>
I'm confused....see below.
On 21 January 2017 at 15:21, George Vanburgh <george@vanburgh.me> wrote:
>> On 21 Jan 2017, at 13:33, Lars Schneider <larsxschneider@gmail.com>
>> > On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me>
>> wrote:
>> >
>> > From: George Vanburgh <gvanburgh@bloomberg.net>
>> >
>> > When running git-p4 on Windows, with multiple git-p4.mapUser entries
>> > in git config - no user mappings are applied to the generated
> repository.
>> >
>> > Reproduction Steps:
>> >
>> > 1. Add multiple git-p4.mapUser entries to git config on a Windows
>> > machine
>> > 2. Attempt to clone a p4 repository
>> >
>> > None of the user mappings will be applied.
>> >
>> > This issue is caused by the fact that gitConfigList, uses
>> > split(os.linesep) to convert the output of git config --get-all into a
>> > list.
>> >
>> > On Windows, os.linesep is equal to '\r\n' - however git.exe returns
>> > configuration with a line seperator of '\n'. This leads to the list
>> > returned by gitConfigList containing only one element - which contains
>> > the full output of git config --get-all in string form. This causes
>> > problems for the code introduced to getUserMapFromPerforceServer in
>> > 10d08a1.
>> >
>> > This issue should be caught by the test introduced in 10d08a1, and
>> > would require running on Windows to reproduce. When running inside
>> > MinGW/Cygwin, however, os.linesep correctly returns '\n', and
>> > everything works as expected.
>>
>> This surprises me. I would expect `\r\n` in a MinGW env...
>> Nevertheless, I wouldn't have caught that as I don't run the git-p4 tests
> on
>> Windows...
>
> It appears I was mistaken - the successful tests I ran were actually under
> the Ubuntu subsystem for Windows, which (obviously) passed.
>
> Just did a quick experiment:
>
> Git Bash (MinGW):
> georg@TEMPEST MINGW64 ~
> $ python -c "import os
> print(repr(os.linesep))"
> '\r\n'
>
> Powershell:
> PS C:\Users\georg> python -c "import os
>>> print(repr(os.linesep))"
> '\r\n'
>
> Ubuntu subsystem for Windows:
> george@TEMPEST:~$ python -c "import os
> print(repr(os.linesep))"
> '\n'
>
> So this issue applies to git-p4 running under both PowerShell and MinGW.
>
>>
>>
>> > The simplest fix for this issue would be to convert the line split
>> > logic inside gitConfigList to use splitlines(), which splits on any
>> > standard line delimiter. However, this function was only introduced in
>> > Python 2.7, and would mean a bump in the minimum required version of
>> > Python required to run git-p4. The alternative fix, implemented here,
>> > is to use '\n' as a delimiter, which git.exe appears to output
>> > consistently on Windows anyway.
Have you tried a 2.6 Python with splitlines? I just tried this code
and it seems fine.
> val = s.strip().splitlines()
> print("splitlines:", val)
Tried with 2.7 and 2.6, and bot print out an array of strings returned
from read_pipe().
And 'grep -r splitlines' on the Python2.6 source has lots of hits.
>>
>> Well, that also means if we ever use splitlines() then your fix below
> would
>> brake the code, right?
>>
>> Python 2.7 was released 7 years ago in 2010.
>
> Now I feel old...
:-)
>
>> Therefore, I would vote to
>> bump the minimum version. But that's just my opinion :-)
>
> I feel like splitlines is the better/safer fix - but figured bumping the
> minimum
> Python version was probably part of a wider discussion. If it's something
> people
> are comfortable with - I'd be happy to rework the fix to use splitlines.
> Luke - do you have any thoughts on this?
I agree that we have to stop supporting 2.6 at some point (and start
supporting 3.x, but that's another world of hurt). But does 2.6 really
not have splitlines?
If you send a patch with splitlines I can try it out (although I guess
it could be broken under Windows).
Luke
^ 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