* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Jacob Keller @ 2016-10-19 23:36 UTC (permalink / raw)
To: Jeff King; +Cc: Git mailing list
In-Reply-To: <20161019224211.k4anavgqrhmunz6p@sigill.intra.peff.net>
On Wed, Oct 19, 2016 at 3:42 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Oct 19, 2016 at 03:26:18PM -0700, Jacob Keller wrote:
>
>> I recently (and in the past) had an issue where I was using git add
>> --interactive and accidentally did something like the following:
>>
>> # hack lots of randmo changes, then begin trying to commit then separately
>> git add -i
>> # set only the changes I want
>> # accidentally add <file> to the commit
>> $git commit -s <file>
>> # type up a long commit message
>> # notice that I committed everything
>>
>> At this point I'd like to be able to do something like:
>> $git unstage -i
>> # select each hunk to unstage
>
> I'd usually do one of:
>
> # undo selectively
> git reset -p HEAD^
> git commit --amend
AHA! I knew about git reset -p but I didn't know about git reset -p
allowed passing a treeish. Does this reset modify my local files at
all? I think it doesn't, right?
>
> or:
>
> # roll back the whole commit
> git reset HEAD
> # do it right this time
> git add -p
> # and steal the commit message from the previous attempt
> git commit -c HEAD@{1}
>
> -Peff
Also nice to know about git commit -c
Thanks a lot! This should save me some headaches.
I still think it's worth while to add a check for git-commit which
does something like check when we say "git commit <files>" and if the
index already has those files marked as being changed, compare them
with the current contents of the file as in the checkout and quick
saying "please don't do that" so as to avoid the problem in the first
place.
A naive approach would just be "if index already has staged
differences dont allow path selection" but that doesn't let me do
something like "git add -p <files>" "git commit <other files>"
We could even make it work so that "commit --only" doesn't run this so
that way people can easily override, and we can give suggestions for
how to fix it in the output of the message. I can't really think if a
reasonable objection to such a change. I'll try to code something up
in the next few days when I can find some spare time.
Regards,
Jake
^ permalink raw reply
* [ANNOUNCE] git-log-compact v1.0
From: Kyle J. McKay @ 2016-10-20 0:13 UTC (permalink / raw)
To: Git mailing list; +Cc: Jakub Narebski, Christian Couder
> NOTE: If you read the excellent Git Rev News [1], then you
> already know all about git-log-compact :)
The git-log-compact script provides a compact alternative to the
`git log --oneline` output that includes dates, times and author
and/or committer initials in a space efficient output format.
`git-log-compact` is intended to fill the gap between the single line
`--oneline` log output format and the multiline `--pretty=short` /
`--pretty=medium` output formats.
It is not strictly a one line per commit output format (but almost) and
while it only shows the title of each commit (like the short format) it
does include author and date information (similarly to the medium format)
except that timestamps are shown very compactly and author/committer
names are shown as initials.
This allows one to get a complete view of repository activity in a very
compact output format that can show many commits per screen full and is
fully compatible with the `--graph` option.
Simple example output from the Git repository:
git log-compact --graph --date-order --decorate --no-merges -n 5 v2.5.3
=== 2015-09-17 ===
* ee6ad5f4 12:16 jch (tag: v2.5.3) Git 2.5.3
=== 2015-09-09 ===
* b9d66899 14:22 js am --skip/--abort: merge HEAD/ORIG_HEAD tree into index
| === 2015-09-04 ===
| * 27ea6f85 10:46 jch (tag: v2.5.2) Git 2.5.2
* 74b67638 10:36 jch (tag: v2.4.9) Git 2.4.9
..........
* ecad27cf 10:32 jch (tag: v2.3.9) Git 2.3.9
I have been wanting a compact one line output format that included dates,
times and initials for some time that is compatible with --graph, clearly
shows root commits and eliminates confusion over whether or not two adjacent
lines in the output are related as parent/child (the --show-linear-break
option does not work with --graph).
The git-log-compact utility is the result. Except for --notes, --pretty and
--format options (which would make the output a non-oneline format) any
other `git log` option may be used (including things like --cherry-mark,
--patch, --raw, --stat, --summary, --show-linear-break etc.),
There are a few new options specific to git-log-compact which are described
in the README and the `git-log-compact -h` output that can be used to alter
the dates, times and/or initials displayed.
The project page with detailed help and many screen shots is located at:
https://mackyle.github.io/git-log-compact/
Alternatively the repository can be cloned from:
https://github.com/mackyle/git-log-compact.git
Or the script file itself (which is really all you need) can be
viewed/fetched from:
https://github.com/mackyle/git-log-compact/blob/HEAD/git-log-compact
The git-log-compact script should work with any version of Git released
in the last several years.
--Kyle
[1] https://git.github.io/rev_news/2016/10/19/edition-20/
^ permalink raw reply
* Re: Problems with "git svn clone"
From: Eric Wong @ 2016-10-20 0:49 UTC (permalink / raw)
To: K Richard Pixley; +Cc: git
In-Reply-To: <dea3ab60-be6b-ebcc-1047-6ab28a25979b@zebra.com>
K Richard Pixley <cnp637@zebra.com> wrote:
> On 10/19/16 13:41 , Eric Wong wrote:
> >K Richard Pixley <cnp637@zebra.com> wrote:
> >>error: git-svn died of signal 11
> >>
> >>This seems awfully early and blatant for a core dump. What can I do to
> >>get this running?
> >Can you show us a backtrace? Thanks.
> There is none. I ran it in gdb and bt produced no results. Nor did the
> thread send bt command.
You probably need to install the debug package(s) for subversion.
In case it's a different bug, maybe the debug package for Perl, too.
> >>Initially discovered on git-2.7.4, (ubuntu-16.04), but also reproduced
> >>on freshly built top of tree git-2.10.1.445.g3cdd5d1.
> >This could be a problem with the SVN Perl libraries, and should
> >be fixed in newer versions (not sure if it's made it to distros,
> >yet):
> >
> >https://public-inbox.org/git/0BCA1E695085C645B9CD4A27DD59F6FA39AAD5CF@GBWGCEUHUBD0101.rbsres07.net/T/
> >
> >Seems like it is fixed in latest Debian, maybe it needs to trickle
> >into Ubuntu: https://bugs.debian.org/780246
> Thanks. I'll try adding that.
>
> Er... which debian would that be? testing? Or sid?
It looks like it (1.9.4-3) has trickled into stretch (testing)
by now:
https://packages.debian.org/src:subversion
^ permalink raw reply
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Jeff King @ 2016-10-20 2:13 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xprKV1Y7VShLR9uNgcpVdZk39xoTfkwiin1bVQYTe_TAA@mail.gmail.com>
On Wed, Oct 19, 2016 at 04:36:36PM -0700, Jacob Keller wrote:
> > # undo selectively
> > git reset -p HEAD^
> > git commit --amend
>
> AHA! I knew about git reset -p but I didn't know about git reset -p
> allowed passing a treeish. Does this reset modify my local files at
> all? I think it doesn't, right?
Correct. If you wanted to modify the working tree, too, use "git
checkout -p HEAD^".
> I still think it's worth while to add a check for git-commit which
> does something like check when we say "git commit <files>" and if the
> index already has those files marked as being changed, compare them
> with the current contents of the file as in the checkout and quick
> saying "please don't do that" so as to avoid the problem in the first
> place.
>
> A naive approach would just be "if index already has staged
> differences dont allow path selection" but that doesn't let me do
> something like "git add -p <files>" "git commit <other files>"
I suspect both of those would complain about legitimate workflows.
I dunno. I do not ever use "git commit <file>" myself. I almost
invariably use one of "git add -p" (to review changes as I add them) or
"git add -u" (when I know everything is in good shape, such as after
merge resolution; I'll sometimes just "git commit -a", too).
But it sounds like you want a third mode besides "--include" and
"--only". Basically "commit what has been staged already, but restrict
the commit to the paths I mentioned". Something like "--only-staged" or
something. I do not think we would want to change the default from
--only, but I could see a config option or something to select that
behavior.
I suspect nobody has really asked for such a thing before because
separate staging and "git commit <file>" are really two distinct
workflows. Your pain comes from mix-and-matching them.
-Peff
^ permalink raw reply
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Jacob Keller @ 2016-10-20 5:53 UTC (permalink / raw)
To: Jeff King; +Cc: Git mailing list
In-Reply-To: <20161020021323.tav5glu7xy4u7mtj@sigill.intra.peff.net>
On Wed, Oct 19, 2016 at 7:13 PM, Jeff King <peff@peff.net> wrote:
> I suspect both of those would complain about legitimate workflows.
>
> I dunno. I do not ever use "git commit <file>" myself. I almost
> invariably use one of "git add -p" (to review changes as I add them) or
> "git add -u" (when I know everything is in good shape, such as after
> merge resolution; I'll sometimes just "git commit -a", too).
>
> But it sounds like you want a third mode besides "--include" and
> "--only". Basically "commit what has been staged already, but restrict
> the commit to the paths I mentioned". Something like "--only-staged" or
> something. I do not think we would want to change the default from
> --only, but I could see a config option or something to select that
> behavior.
>
> I suspect nobody has really asked for such a thing before because
> separate staging and "git commit <file>" are really two distinct
> workflows. Your pain comes from mix-and-matching them.
>
> -Peff
No. What I want is to *prevent* mix-and-match from happening.
Basically, sometimes I use "git add -p" to stage changes. But if I
just did a "git diff" and I know all the changes that I want are in
the file I will just do "git commit <file>" or "git commit -a". The
problem is that sometimes I stage stuff, forget or just make a brain
mistake, and I go ahead and use "git commit <file>"
What I want is to make it so that when you run "git commit <file>"
that it is smart enough to go "hey! You already staged something from
that file in the index and it doesn't match what you're asking me to
commit now, so I'm going to stop and make sure you either reset, don't
run "git commit <file>" or run "git commit --only <file>" or similar.
It's just about making it so that if I happen to make the mistake in
the future it doesn't generate a commit and instead tells me that I
was being an idiot. I don't want this check to just be "you can't
stage with the index and then tell me to commit -a or commit <files>"
because I think that's too restrictive and might make people complain.
Essentially, I want the tool to become smart enough to make it so that
an obvious mistake is caught early before I have to undo things.
That being said, it's much less of a pain point now that I know I can
go "git reset -p HEAD^". It never occurred to me that git reset -p
would work that way, so I didn't even try it.
Thanks,
Jake
^ permalink raw reply
* [PATCH 0/7] stop blind fallback to ".git"
From: Jeff King @ 2016-10-20 6:15 UTC (permalink / raw)
To: git
This is a follow-on to the series in:
http://public-inbox.org/git/20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net/
That series avoided looking at ".git/config" when we haven't discovered
whether we are in a git repo. This takes that further and avoids ever
looking at ".git" as a fallback.
Patches 1-6 just teach various low-level code to detect and handle the
case when we are not in a configure repository (along with associated
cleanups). The final patch actually disallows this case (and the early
fixes were found by running the test suite with just the final patch).
I think this is a step in the right direction, both in fixing bugs, but
also in making our setup and repository-access code easier to reason
about. However, it does carry a risk of regression, if there are more
"fixes" that I missed. If we wanted to be really conservative, we could
hold back the 7th patch as a separate topic and cook it in "next" for an
extra release cycle or something. I'm not all that worried, though.
I built this on top of jc/diff-unique-abbrev-comments, as it refactors
that same function (and it didn't seem like a bad topic to be held
hostage by).
[1/7]: read info/{attributes,exclude} only when in repository
[2/7]: test-*-cache-tree: setup git dir
[3/7]: find_unique_abbrev: use 4-buffer ring
[4/7]: diff_unique_abbrev: rename to diff_aligned_abbrev
[5/7]: diff_aligned_abbrev: use "struct oid"
[6/7]: diff: handle sha1 abbreviations outside of repository
[7/7]: setup_git_env: avoid blind fall-back to ".git"
attr.c | 6 +++++-
builtin/merge.c | 11 +++++-----
builtin/receive-pack.c | 16 ++++++---------
cache.h | 4 ++--
combine-diff.c | 6 +++---
diff.c | 43 +++++++++++++++++++++++++---------------
diff.h | 6 +++++-
dir.c | 12 +++++------
environment.c | 5 ++++-
sha1_name.c | 4 +++-
t/helper/test-dump-cache-tree.c | 1 +
t/helper/test-scrap-cache-tree.c | 1 +
12 files changed, 68 insertions(+), 47 deletions(-)
^ permalink raw reply
* [PATCH 1/7] read info/{attributes,exclude} only when in repository
From: Jeff King @ 2016-10-20 6:16 UTC (permalink / raw)
To: git
In-Reply-To: <20161020061536.6fqh23xb2nhxodpa@sigill.intra.peff.net>
The low-level attribute and gitignore code will try to look
in $GIT_DIR/info for any repo-level configuration files,
even if we have not actually determined that we are in a
repository (e.g., running "git grep --no-index"). In such a
case they end up looking for ".git/info/attributes", etc.
This is generally harmless, as such a file is unlikely to
exist outside of a repository, but it's still conceptually
the wrong thing to do.
Let's detect this situation explicitly and skip reading the
file (i.e., the same behavior we'd get if we were in a
repository and the file did not exist).
Signed-off-by: Jeff King <peff@peff.net>
---
attr.c | 6 +++++-
dir.c | 12 ++++++------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/attr.c b/attr.c
index eec5d7d..1fcf042 100644
--- a/attr.c
+++ b/attr.c
@@ -531,7 +531,11 @@ static void bootstrap_attr_stack(void)
debug_push(elem);
}
- elem = read_attr_from_file(git_path_info_attributes(), 1);
+ if (startup_info->have_repository)
+ elem = read_attr_from_file(git_path_info_attributes(), 1);
+ else
+ elem = NULL;
+
if (!elem)
elem = xcalloc(1, sizeof(*elem));
elem->origin = NULL;
diff --git a/dir.c b/dir.c
index 3bad1ad..6cde773 100644
--- a/dir.c
+++ b/dir.c
@@ -2195,8 +2195,6 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
void setup_standard_excludes(struct dir_struct *dir)
{
- const char *path;
-
dir->exclude_per_dir = ".gitignore";
/* core.excludefile defaulting to $XDG_HOME/git/ignore */
@@ -2207,10 +2205,12 @@ void setup_standard_excludes(struct dir_struct *dir)
dir->untracked ? &dir->ss_excludes_file : NULL);
/* per repository user preference */
- path = git_path_info_exclude();
- if (!access_or_warn(path, R_OK, 0))
- add_excludes_from_file_1(dir, path,
- dir->untracked ? &dir->ss_info_exclude : NULL);
+ if (startup_info->have_repository) {
+ const char *path = git_path_info_exclude();
+ if (!access_or_warn(path, R_OK, 0))
+ add_excludes_from_file_1(dir, path,
+ dir->untracked ? &dir->ss_info_exclude : NULL);
+ }
}
int remove_path(const char *name)
--
2.10.1.619.g16351a7
^ permalink raw reply related
* [PATCH 2/7] test-*-cache-tree: setup git dir
From: Jeff King @ 2016-10-20 6:16 UTC (permalink / raw)
To: git
In-Reply-To: <20161020061536.6fqh23xb2nhxodpa@sigill.intra.peff.net>
These test helper programs access the index, but do not ever
setup_git_directory(), meaning we just blindly looked in
".git/index". This happened to work for the purposes of our
tests (which do not run from subdirectories, nor in
non-repos), but it's a bad habit.
Signed-off-by: Jeff King <peff@peff.net>
---
t/helper/test-dump-cache-tree.c | 1 +
t/helper/test-scrap-cache-tree.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/t/helper/test-dump-cache-tree.c b/t/helper/test-dump-cache-tree.c
index 44f3290..7af116d 100644
--- a/t/helper/test-dump-cache-tree.c
+++ b/t/helper/test-dump-cache-tree.c
@@ -58,6 +58,7 @@ int cmd_main(int ac, const char **av)
{
struct index_state istate;
struct cache_tree *another = cache_tree();
+ setup_git_directory();
if (read_cache() < 0)
die("unable to read index file");
istate = the_index;
diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c
index 5b2fd09..27fe040 100644
--- a/t/helper/test-scrap-cache-tree.c
+++ b/t/helper/test-scrap-cache-tree.c
@@ -7,6 +7,7 @@ static struct lock_file index_lock;
int cmd_main(int ac, const char **av)
{
+ setup_git_directory();
hold_locked_index(&index_lock, 1);
if (read_cache() < 0)
die("unable to read index file");
--
2.10.1.619.g16351a7
^ permalink raw reply related
* [PATCH 3/7] find_unique_abbrev: use 4-buffer ring
From: Jeff King @ 2016-10-20 6:19 UTC (permalink / raw)
To: git
In-Reply-To: <20161020061536.6fqh23xb2nhxodpa@sigill.intra.peff.net>
Some code paths want to format multiple abbreviated sha1s in
the same output line. Because we use a single static buffer
for our return value, they have to either break their output
into several calls or allocate their own arrays and use
find_unique_abbrev_r().
Intead, let's mimic sha1_to_hex() and use a ring of several
buffers, so that the return value stays valid through
multiple calls. This shortens some of the callers, and makes
it harder to for them to make a silly mistake.
Signed-off-by: Jeff King <peff@peff.net>
---
It feels a little funny in these callers to be moving from a reentrant
function to one that relies on a static buffer. But I feel like the
result is more obvious and less error-prone, and the "ring of buffers"
concept has proven useful and safe in sha1_to_hex().
My ulterior motive is that while refactoring one of the later patches, I
just assumed that we did have a ring of buffers, and introduced a subtle
bug.
builtin/merge.c | 11 +++++------
builtin/receive-pack.c | 16 ++++++----------
cache.h | 4 ++--
sha1_name.c | 4 +++-
4 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index a8b57c7..b65eeaa 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1374,12 +1374,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct commit *commit;
if (verbosity >= 0) {
- char from[GIT_SHA1_HEXSZ + 1], to[GIT_SHA1_HEXSZ + 1];
- find_unique_abbrev_r(from, head_commit->object.oid.hash,
- DEFAULT_ABBREV);
- find_unique_abbrev_r(to, remoteheads->item->object.oid.hash,
- DEFAULT_ABBREV);
- printf(_("Updating %s..%s\n"), from, to);
+ printf(_("Updating %s..%s\n"),
+ find_unique_abbrev(head_commit->object.oid.hash,
+ DEFAULT_ABBREV),
+ find_unique_abbrev(remoteheads->item->object.oid.hash,
+ DEFAULT_ABBREV));
}
strbuf_addstr(&msg, "Fast-forward");
if (have_message)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 04ed38e..680759d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1163,10 +1163,6 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
struct string_list_item *item;
struct command *dst_cmd;
unsigned char sha1[GIT_SHA1_RAWSZ];
- char cmd_oldh[GIT_SHA1_HEXSZ + 1],
- cmd_newh[GIT_SHA1_HEXSZ + 1],
- dst_oldh[GIT_SHA1_HEXSZ + 1],
- dst_newh[GIT_SHA1_HEXSZ + 1];
int flag;
strbuf_addf(&buf, "%s%s", get_git_namespace(), cmd->ref_name);
@@ -1197,14 +1193,14 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
dst_cmd->skip_update = 1;
- find_unique_abbrev_r(cmd_oldh, cmd->old_sha1, DEFAULT_ABBREV);
- find_unique_abbrev_r(cmd_newh, cmd->new_sha1, DEFAULT_ABBREV);
- find_unique_abbrev_r(dst_oldh, dst_cmd->old_sha1, DEFAULT_ABBREV);
- find_unique_abbrev_r(dst_newh, dst_cmd->new_sha1, DEFAULT_ABBREV);
rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
" its target '%s' (%s..%s)",
- cmd->ref_name, cmd_oldh, cmd_newh,
- dst_cmd->ref_name, dst_oldh, dst_newh);
+ cmd->ref_name,
+ find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV),
+ find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV),
+ dst_cmd->ref_name,
+ find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV),
+ find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
cmd->error_string = dst_cmd->error_string =
"inconsistent aliased update";
diff --git a/cache.h b/cache.h
index 05ecb88..6e06311 100644
--- a/cache.h
+++ b/cache.h
@@ -901,8 +901,8 @@ extern char *sha1_pack_index_name(const unsigned char *sha1);
* The result will be at least `len` characters long, and will be NUL
* terminated.
*
- * The non-`_r` version returns a static buffer which will be overwritten by
- * subsequent calls.
+ * The non-`_r` version returns a static buffer which remains valid until 4
+ * more calls to find_unique_abbrev are made.
*
* The `_r` variant writes to a buffer supplied by the caller, which must be at
* least `GIT_SHA1_HEXSZ + 1` bytes. The return value is the number of bytes
diff --git a/sha1_name.c b/sha1_name.c
index 4092836..36ce9b9 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -472,7 +472,9 @@ int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
const char *find_unique_abbrev(const unsigned char *sha1, int len)
{
- static char hex[GIT_SHA1_HEXSZ + 1];
+ static int bufno;
+ static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
+ char *hex = hexbuffer[3 & ++bufno];
find_unique_abbrev_r(hex, sha1, len);
return hex;
}
--
2.10.1.619.g16351a7
^ permalink raw reply related
* [PATCH 4/7] diff_unique_abbrev: rename to diff_aligned_abbrev
From: Jeff King @ 2016-10-20 6:19 UTC (permalink / raw)
To: git
In-Reply-To: <20161020061536.6fqh23xb2nhxodpa@sigill.intra.peff.net>
The word "align" describes how the function actually differs
from find_unique_abbrev, and will make it less confusing
when we add more diff-specific abbrevation functions that do
not do this alignment.
Since this is a globally available function, let's also move
its descriptive comment to the header file, where we
typically document function interfaces.
Signed-off-by: Jeff King <peff@peff.net>
---
combine-diff.c | 6 +++---
diff.c | 10 +++-------
diff.h | 6 +++++-
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 8e2a577..b36c2d1 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1203,9 +1203,9 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
/* Show sha1's */
for (i = 0; i < num_parent; i++)
- printf(" %s", diff_unique_abbrev(p->parent[i].oid.hash,
- opt->abbrev));
- printf(" %s ", diff_unique_abbrev(p->oid.hash, opt->abbrev));
+ printf(" %s", diff_aligned_abbrev(p->parent[i].oid.hash,
+ opt->abbrev));
+ printf(" %s ", diff_aligned_abbrev(p->oid.hash, opt->abbrev));
}
if (opt->output_format & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS)) {
diff --git a/diff.c b/diff.c
index f5d6d7e..2d8b74b 100644
--- a/diff.c
+++ b/diff.c
@@ -4136,11 +4136,7 @@ void diff_free_filepair(struct diff_filepair *p)
free(p);
}
-/*
- * This is different from find_unique_abbrev() in that
- * it stuffs the result with dots for alignment.
- */
-const char *diff_unique_abbrev(const unsigned char *sha1, int len)
+const char *diff_aligned_abbrev(const unsigned char *sha1, int len)
{
int abblen;
const char *abbrev;
@@ -4188,9 +4184,9 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
fprintf(opt->file, "%s", diff_line_prefix(opt));
if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) {
fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode,
- diff_unique_abbrev(p->one->oid.hash, opt->abbrev));
+ diff_aligned_abbrev(p->one->oid.hash, opt->abbrev));
fprintf(opt->file, "%s ",
- diff_unique_abbrev(p->two->oid.hash, opt->abbrev));
+ diff_aligned_abbrev(p->two->oid.hash, opt->abbrev));
}
if (p->score) {
fprintf(opt->file, "%c%03d%c", p->status, similarity_index(p),
diff --git a/diff.h b/diff.h
index 25ae60d..f2b04b6 100644
--- a/diff.h
+++ b/diff.h
@@ -340,7 +340,11 @@ extern void diff_warn_rename_limit(const char *varname, int needed, int degraded
#define DIFF_STATUS_FILTER_AON '*'
#define DIFF_STATUS_FILTER_BROKEN 'B'
-extern const char *diff_unique_abbrev(const unsigned char *, int);
+/*
+ * This is different from find_unique_abbrev() in that
+ * it stuffs the result with dots for alignment.
+ */
+extern const char *diff_aligned_abbrev(const unsigned char *sha1, int);
/* do not report anything on removed paths */
#define DIFF_SILENT_ON_REMOVED 01
--
2.10.1.619.g16351a7
^ permalink raw reply related
* [PATCH 5/7] diff_aligned_abbrev: use "struct oid"
From: Jeff King @ 2016-10-20 6:20 UTC (permalink / raw)
To: git
In-Reply-To: <20161020061536.6fqh23xb2nhxodpa@sigill.intra.peff.net>
Since we're modifying this function anyway, it's a good time
to update it to the more modern "struct oid". We can also
drop some of the magic numbers in favor of GIT_SHA1_HEXSZ,
along with some descriptive comments.
Signed-off-by: Jeff King <peff@peff.net>
---
combine-diff.c | 4 ++--
diff.c | 20 +++++++++++---------
diff.h | 2 +-
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index b36c2d1..59501db 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1203,9 +1203,9 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
/* Show sha1's */
for (i = 0; i < num_parent; i++)
- printf(" %s", diff_aligned_abbrev(p->parent[i].oid.hash,
+ printf(" %s", diff_aligned_abbrev(&p->parent[i].oid,
opt->abbrev));
- printf(" %s ", diff_aligned_abbrev(p->oid.hash, opt->abbrev));
+ printf(" %s ", diff_aligned_abbrev(&p->oid, opt->abbrev));
}
if (opt->output_format & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS)) {
diff --git a/diff.c b/diff.c
index 2d8b74b..8f0f309 100644
--- a/diff.c
+++ b/diff.c
@@ -4136,14 +4136,15 @@ void diff_free_filepair(struct diff_filepair *p)
free(p);
}
-const char *diff_aligned_abbrev(const unsigned char *sha1, int len)
+const char *diff_aligned_abbrev(const struct object_id *oid, int len)
{
int abblen;
const char *abbrev;
- if (len == 40)
- return sha1_to_hex(sha1);
- abbrev = find_unique_abbrev(sha1, len);
+ if (len == GIT_SHA1_HEXSZ)
+ return oid_to_hex(oid);
+
+ abbrev = find_unique_abbrev(oid->hash, len);
abblen = strlen(abbrev);
/*
@@ -4165,15 +4166,16 @@ const char *diff_aligned_abbrev(const unsigned char *sha1, int len)
* the automatic sizing is supposed to give abblen that ensures
* uniqueness across all objects (statistically speaking).
*/
- if (abblen < 37) {
- static char hex[41];
+ if (abblen < GIT_SHA1_HEXSZ - 3) {
+ static char hex[GIT_SHA1_HEXSZ + 1];
if (len < abblen && abblen <= len + 2)
xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, "..");
else
xsnprintf(hex, sizeof(hex), "%s...", abbrev);
return hex;
}
- return sha1_to_hex(sha1);
+
+ return oid_to_hex(oid);
}
static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
@@ -4184,9 +4186,9 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
fprintf(opt->file, "%s", diff_line_prefix(opt));
if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) {
fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode,
- diff_aligned_abbrev(p->one->oid.hash, opt->abbrev));
+ diff_aligned_abbrev(&p->one->oid, opt->abbrev));
fprintf(opt->file, "%s ",
- diff_aligned_abbrev(p->two->oid.hash, opt->abbrev));
+ diff_aligned_abbrev(&p->two->oid, opt->abbrev));
}
if (p->score) {
fprintf(opt->file, "%c%03d%c", p->status, similarity_index(p),
diff --git a/diff.h b/diff.h
index f2b04b6..01afc70 100644
--- a/diff.h
+++ b/diff.h
@@ -344,7 +344,7 @@ extern void diff_warn_rename_limit(const char *varname, int needed, int degraded
* This is different from find_unique_abbrev() in that
* it stuffs the result with dots for alignment.
*/
-extern const char *diff_aligned_abbrev(const unsigned char *sha1, int);
+extern const char *diff_aligned_abbrev(const struct object_id *sha1, int);
/* do not report anything on removed paths */
#define DIFF_SILENT_ON_REMOVED 01
--
2.10.1.619.g16351a7
^ permalink raw reply related
* [PATCH 6/7] diff: handle sha1 abbreviations outside of repository
From: Jeff King @ 2016-10-20 6:21 UTC (permalink / raw)
To: git
In-Reply-To: <20161020061536.6fqh23xb2nhxodpa@sigill.intra.peff.net>
When generating diffs outside a repository (e.g., with "diff
--no-index"), we may write abbreviated sha1s as part of
"--raw" output or the "index" lines of "--patch" output.
Since we have no object database, we never find any
collisions, and these sha1s get whatever static abbreviation
length is configured (typically 7).
However, we do blindly look in ".git/objects" to see if any
objects exist, even though we know we are not in a
repository. This is usually harmless because such a
directory is unlikely to exist, but could be wrong in rare
circumstances.
Let's instead notice when we are not in a repository and
behave as if the object database is empty (i.e., just use
the default abbrev length). It would perhaps make sense to
be conservative and show full sha1s in that case, but
showing the default abbreviation is what we've always done
(and is certainly less ugly).
Note that this does mean that:
cd /not/a/repo
GIT_OBJECT_DIRECTORY=/some/real/objdir git diff --no-index ...
used to look for collisions in /some/real/objdir but now
does not. This could be considered either a bugfix (we do
not look at objects if we have no repository) or a
regression, but it seems unlikely that anybody would care
much either way.
Signed-off-by: Jeff King <peff@peff.net>
---
diff.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/diff.c b/diff.c
index 8f0f309..ef11001 100644
--- a/diff.c
+++ b/diff.c
@@ -3049,6 +3049,19 @@ static int similarity_index(struct diff_filepair *p)
return p->score * 100 / MAX_SCORE;
}
+static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
+{
+ if (startup_info->have_repository)
+ return find_unique_abbrev(oid->hash, abbrev);
+ else {
+ char *hex = oid_to_hex(oid);
+ if (abbrev < 0 || abbrev > GIT_SHA1_HEXSZ)
+ die("BUG: oid abbreviation out of range: %d", abbrev);
+ hex[abbrev] = '\0';
+ return hex;
+ }
+}
+
static void fill_metainfo(struct strbuf *msg,
const char *name,
const char *other,
@@ -3107,9 +3120,9 @@ static void fill_metainfo(struct strbuf *msg,
(!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
abbrev = 40;
}
- strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
- find_unique_abbrev(one->oid.hash, abbrev));
- strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);
+ strbuf_addf(msg, "%s%sindex %s..%s", line_prefix, set,
+ diff_abbrev_oid(&one->oid, abbrev),
+ diff_abbrev_oid(&two->oid, abbrev));
if (one->mode == two->mode)
strbuf_addf(msg, " %06o", one->mode);
strbuf_addf(msg, "%s\n", reset);
@@ -4144,7 +4157,7 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len)
if (len == GIT_SHA1_HEXSZ)
return oid_to_hex(oid);
- abbrev = find_unique_abbrev(oid->hash, len);
+ abbrev = diff_abbrev_oid(oid, len);
abblen = strlen(abbrev);
/*
--
2.10.1.619.g16351a7
^ permalink raw reply related
* (no subject)
From: Jordan DE GEA @ 2016-10-20 6:22 UTC (permalink / raw)
To: git
unsubscribe alsa-devel
^ permalink raw reply
* [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"
From: Jeff King @ 2016-10-20 6:24 UTC (permalink / raw)
To: git
In-Reply-To: <20161020061536.6fqh23xb2nhxodpa@sigill.intra.peff.net>
When we default to ".git" without having done any kind of
repository setup, the results quite often do what the user
expects. But this has also historically been the cause of
some poorly behaved corner cases. These cases can be hard to
find, because they happen at the conjunction of two
relatively rare circumstances:
1. We are running some code which assumes there's a
repository present, but there isn't necessarily one
(e.g., low-level diff code triggered by "git diff
--no-index" might try to look at some repository data).
2. We have an unusual setup, like being in a subdirectory
of the working tree, or we have a .git file (rather
than a directory), or we are running a tool like "init"
or "clone" which may operate on a repository in a
different directory.
Our test scripts often cover (1), but miss doing (2) at the
same time, and so the fallback appears to work but has
lurking bugs. We can flush these bugs out by refusing to do
the fallback entirely., This makes potential problems a lot
more obvious by complaining even for "usual" setups.
This passes the test suite (after the adjustments in the
previous patches), but there's a risk of regression for any
cases where the fallback usually works fine but the code
isn't exercised by the test suite. So by itself, this
commit is a potential step backward, but lets us take two
steps forward once we've identified and fixed any such
instances.
Signed-off-by: Jeff King <peff@peff.net>
---
environment.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/environment.c b/environment.c
index cd5aa57..b1743e6 100644
--- a/environment.c
+++ b/environment.c
@@ -164,8 +164,11 @@ static void setup_git_env(void)
const char *replace_ref_base;
git_dir = getenv(GIT_DIR_ENVIRONMENT);
- if (!git_dir)
+ if (!git_dir) {
+ if (!startup_info->have_repository)
+ die("BUG: setup_git_env called without repository");
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+ }
gitfile = read_gitfile(git_dir);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
if (get_common_dir(&sb, git_dir))
--
2.10.1.619.g16351a7
^ permalink raw reply related
* Re: [PATCH 6/7] diff: handle sha1 abbreviations outside of repository
From: Jeff King @ 2016-10-20 6:31 UTC (permalink / raw)
To: git
In-Reply-To: <20161020062125.3iqej3bpdoitr3fz@sigill.intra.peff.net>
On Thu, Oct 20, 2016 at 02:21:25AM -0400, Jeff King wrote:
> diff --git a/diff.c b/diff.c
> index 8f0f309..ef11001 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3049,6 +3049,19 @@ static int similarity_index(struct diff_filepair *p)
> return p->score * 100 / MAX_SCORE;
> }
>
> +static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
> +{
> + if (startup_info->have_repository)
> + return find_unique_abbrev(oid->hash, abbrev);
> + else {
> + char *hex = oid_to_hex(oid);
> + if (abbrev < 0 || abbrev > GIT_SHA1_HEXSZ)
> + die("BUG: oid abbreviation out of range: %d", abbrev);
> + hex[abbrev] = '\0';
> + return hex;
> + }
> +}
Note that this function has a semantic (but not textual) conflict with
lt/auto-abbrev in 'next', as it sets DEFAULT_ABBREV to -1.
The resolution is:
diff --git a/diff.c b/diff.c
index cab811e..4c09314 100644
--- a/diff.c
+++ b/diff.c
@@ -3102,7 +3102,9 @@ static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
return find_unique_abbrev(oid->hash, abbrev);
else {
char *hex = oid_to_hex(oid);
- if (abbrev < 0 || abbrev > GIT_SHA1_HEXSZ)
+ if (abbrev < 0)
+ abbrev = FALLBACK_DEFAULT_ABBREV;
+ if (abbrev > GIT_SHA1_HEXSZ)
die("BUG: oid abbreviation out of range: %d", abbrev);
hex[abbrev] = '\0';
return hex;
This logic could be pushed down into the find_unique_abbrev() code
(where it _would_ just cause a textual conflict). I preferred to keep it
up here because other callers could conceivably want to handle the
non-repo case in some different way (e.g., by not abbreviating at all).
-Peff
^ permalink raw reply related
* Re: [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Torsten Bögershausen @ 2016-10-20 6:51 UTC (permalink / raw)
To: Dennis Kaarsemaker, git
Cc: jacob.e.keller, stefanbeller, peff, j6t, jacob.keller
In-Reply-To: <20161019210448.aupphybw5qar6mqe@hurricane>
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
> index 3e752ce..a2acff3 100755
> --- a/t/t6000-rev-list-misc.sh
> +++ b/t/t6000-rev-list-misc.sh
> @@ -100,4 +100,11 @@ test_expect_success '--bisect and --first-parent can not be combined' '
> test_must_fail git rev-list --bisect --first-parent HEAD
> '
>
> +test_expect_success '--header shows a NUL after each commit' '
> + touch expect &&
> + printf "\0" > expect &&
Micronit: No ' ' after '>'
(And why do we need the touch ?)
+ printf "\0" >expect &&
> + git rev-list --header --max-count=1 HEAD | tail -n1 >actual &&
> + test_cmp_bin expect actual
> +'
> +
> test_done
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Johannes Schindelin @ 2016-10-20 10:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqbmygmehv.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Wed, 19 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > What I can also say for certain is that the version from yesterday (commit
> > 4ef44ce) was the first one in a week that built successfully and hence
> > reached the test phase, and it was the first version of `pu` ever to time
> > out after running for 3h.
>
> I am sympathetic, but I'd be lying if I said I can feel your pain.
Obviously.
> Admittedly I do not run _all_ the tests (I certainly know that I
> exclude the ones behind EXPENSIVE prerequisite), but after every
> rebuilding of 'jch' and 'pu', I run the testsuite (and also rebuild
> docs) before pushing them out, and "make test && make doc && make
> install install-doc" run sequentially for the four integration
> branches finishes within 15 minutes, even when I run them after
> "make clean".
You have the luxury of a system that runs shell scripts fast.
Or maybe I should put it differently: you set up the test suite in such a
way that it runs fast on your system, exploiting the fact that shell
scripts run fast there.
If you want to know just how harmful this reliance on shell scripting is
to our goal of keeping Git portable: already moving from Linux to MacOSX
costs you roughly 3x as long to run the build & test (~12mins vs ~36mins
for GCC, according to https://travis-ci.org/git/git/builds/159125647).
Again, I have to repeat myself: this is not good.
> Perhaps the recent change to run the tests in parallel from slower
> to faster under prove may be helping my case.
No, you misunderstand. The tests are *already* run in parallel. And
running them from slower to faster would only make a difference if the
last tests were not requiring roughly the same time to run.
Also, I cannot use prove(1) because it proved to be unreliable in my setup
(it does hang from time to time, for no good reason whatsoever, which
would only make my situation worse, of course).
> > Maybe we should start optimizing the tests...
>
> Yup, two things that come to mind are to identify long ones and see
> if each of them can be split into two halves that can be run in
> parallel, and to go through the tests with fine toothed comb and
> remove the ones that test exactly the same thing as another test.
> The latter would be very time consuming, though.
Again, you misunderstand.
The problem is not whether or not to run the tests in parallel.
The problem is that our tests take an insanely long time to run. That is a
big problem, it is no good, tests are only useful if they are cheap enough
to run all the time.
So the only thing that would really count as an improvement would be to
change the test suite in such a manner that it relies more on helpers in
t/helper/ and less on heavy-duty shell scripting.
Of course, if you continue to resist (because the problem is obviously not
affecting you personally, so why would you care), I won't even try to find
the time to start on that project.
Ciao,
Dscho
P.S.: After increasing the time-out to 240 minutes, the test suite still
times out. I investigated a little bit and it appears that
t6030-bisect-porcelain.sh now hangs in the `git bisect next` call of its
"2 - bisect starts with only one bad" test case. This is specific to
Windows, I cannot reproduce that problem on Linux, and I will keep you
posted on my progress.
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Johannes Schindelin @ 2016-10-20 10:50 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20161019205638.m3ytxozzmeh47ml2@sigill.intra.peff.net>
Hi peff,
On Wed, 19 Oct 2016, Jeff King wrote:
> On Wed, Oct 19, 2016 at 10:32:12AM -0700, Junio C Hamano wrote:
>
> > > Maybe we should start optimizing the tests...
> >
> > Yup, two things that come to mind are to identify long ones and see if
> > each of them can be split into two halves that can be run in parallel,
> > and to go through the tests with fine toothed comb and remove the ones
> > that test exactly the same thing as another test. The latter would be
> > very time consuming, though.
>
> FWIW, I have made attempts at "split long ones into two" before, and
> didn't come up with much. There _are_ some tests that are much longer
> than others[1], but they are not longer than the whole suite takes to
> run. So running in slow-to-fast order means they start first, are run in
> parallel with the other tests, and the CPUs stay relatively full through
> the whole run.
That reflects my findings, too. I want to add that I found preciously
little difference between running slow-to-fast and running in numeric
order, so I gave up on optimizing on that front.
> 43.216765165329 t3404-rebase-interactive.sh
> 30.6568658351898 t3421-rebase-topology-linear.sh
> 27.92564702034 t9001-send-email.sh
> 15.5906939506531 t9500-gitweb-standalone-no-errors.sh
> 15.4882569313049 t6030-bisect-porcelain.sh
> 14.487174987793 t7610-mergetool.sh
> 13.8276169300079 t3425-rebase-topology-merges.sh
> 12.7450480461121 t3426-rebase-submodule.sh
> 12.4915001392365 t3415-rebase-autosquash.sh
> 11.8122401237488 t5572-pull-submodule.sh
That looks very similar to what I found: t3404 on top, followed by t3421.
Further, I found that the Subversion tests (which run at the end) are so
close in their running time that running the tests in parallel with -j5
does not result in any noticeable improvement when reordered.
I guess I will have to bite into the sour apple and try to profile, say,
t3404 somehow, including all the shell scripting stuff, to identify where
exactly all that time is lost. My guess is that it boils down to
gazillions of calls to programs like expr.exe or merely subshells.
Ciao,
Dscho
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Duy Nguyen @ 2016-10-20 11:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.DEB.2.20.1610191049040.3847@virtualbox>
On Wed, Oct 19, 2016 at 4:18 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Junio,
>
> I know you are a fan of testing things thoroughly in the test suite, but I
> have to say that it is getting out of hand, in particular due to our
> over-use of shell script idioms (which really only run fast on Linux, not
> a good idea for a portable software).
>
> My builds of `pu` now time out, after running for 3h straight in the VM
> dedicated to perform the daily routine of building and testing the git.git
> branches in Git for Windows' SDK. For comparison, `next` passes build &
> tests in 2.6h. That is quite the jump.
I'm just curious, will running git.exe from WSL [1] help speed things
up a bit (or, hopefully, a lot)? I'm assuming that shell's speed in
WSL is quite fast.
I'm pretty sure the test suite would need some adaptation, but if the
speedup is significant, maybe it's worth spending time on.
[1] https://news.ycombinator.com/item?id=12748395
--
Duy
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Jeff King @ 2016-10-20 11:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1610201218060.3264@virtualbox>
On Thu, Oct 20, 2016 at 12:50:32PM +0200, Johannes Schindelin wrote:
> That reflects my findings, too. I want to add that I found preciously
> little difference between running slow-to-fast and running in numeric
> order, so I gave up on optimizing on that front.
Interesting. It makes a 10-15% difference here.
I also point "--root" at a ram disk. The tests are very I/O heavy and
sometimes fsync; even on a system with an SSD, this saves another ~10%.
I know that's small potatoes compared to the Windows vs Linux times, but
it might be worth exploring.
> Further, I found that the Subversion tests (which run at the end) are so
> close in their running time that running the tests in parallel with -j5
> does not result in any noticeable improvement when reordered.
I normally don't run the Subversion tests at all. Installing cvs, cvsps,
subversion, and libsvn-perl nearly doubles the runtime of the test suite
for me (I imagine adding p4 to the mix would bump it further). While
it's certainly possible to break them with a change in core git, it
doesn't seem like a good tradeoff if I'm not touching them often.
As the GfW maintainer, you probably should be running them, at least
before a release. But cutting them might be a good way to speed up your
day-to-day runs.
I also use -j16 on a quad-core (+hyperthreads) machine, which I arrived
at experimentally. At least on Linux, it's definitely worth having more
threads than processors, to keep the processors busy.
> I guess I will have to bite into the sour apple and try to profile, say,
> t3404 somehow, including all the shell scripting stuff, to identify where
> exactly all that time is lost. My guess is that it boils down to
> gazillions of calls to programs like expr.exe or merely subshells.
I'm not so sure it isn't gazillions of calls to git. It is testing
rebase, after all, which is itself a shell script. GIT_TRACE_PERFORMANCE
gives sort of a crude measure; it reports only builtins (so it will
underestimate the total time spent in git), but it also doesn't make
clear which programs call which, so some times are double-counted (if a
builtin shells out to another builtin). But:
$ export GIT_TRACE_PERFORMANCE=/tmp/foo.out
$ rm /tmp/foo.out
$ time ./t3404-rebase-interactive.sh
real 0m29.755s
user 0m1.444s
sys 0m2.268s
$ perl -lne '
/performance: ([0-9.]+)/ and $total += $1;
END { print $total }
' /tmp/foo.out
32.851352624
Clearly that's not 100% accurate, as it claims we spent longer in git
than the script actually took to run. Given the caveats above, I'm not
even sure if it is in the right ballpark. But there are 11,000 git
builtins run as part of that script. Even at 2ms each, that's still most
of the time going to git.
And obviously the fix involves converting git-rebase, which you're
already working on. But it's not clear to me that the test
infrastructure or shell scripts are the primary cause of the slowness in
this particular case.
-Peff
^ permalink raw reply
* [REQUEST PULL] git-gui 0.20.0 to 0.21.0
From: Pat Thoyts @ 2016-10-20 11:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GitList
The following changes since commit 4498b3a50a0e839788682f672df267cbc1ba9292:
git-gui: set version 0.20 (2015-04-18 12:15:32 +0100)
are available in the git repository at:
git://repo.or.cz/git-gui.git tags/gitgui-0.21.0
for you to fetch changes up to ccc985126f23ff5d9ac610cb820bca48405ff5ef:
git-gui: set version 0.21 (2016-10-20 11:19:43 +0100)
----------------------------------------------------------------
git-gui 0.21.0
----------------------------------------------------------------
Alex Riesen (2):
git-gui: support for $FILENAMES in tool definitions
git-gui: ensure the file in the diff pane is in the list of selected files
Alexander Shopov (2):
git-gui i18n: Updated Bulgarian translation (565,0f,0u)
git-gui: Mark 'All' in remote.tcl for translation
Dimitriy Ryazantcev (1):
git-gui: Update Russian translation
Elia Pinto (1):
git-gui/po/glossary/txt-to-pot.sh: use the $( ... ) construct for command substitution
Johannes Schindelin (1):
git-gui: respect commit.gpgsign again
Karsten Blees (2):
git-gui: unicode file name support on windows
git-gui: handle the encoding of Git's output correctly
Olaf Hering (1):
git-gui: sort entries in tclIndex
Orgad Shaneh (1):
git-gui: Do not reset author details on amend
Pat Thoyts (19):
Allow keyboard control to work in the staging widgets.
Amend tab ordering and text widget border and highlighting.
git-gui: fix detection of Cygwin
git-gui (Windows): use git-gui.exe in `Create Desktop Shortcut`
Merge branch 'sy/i18n' into pu
Merge branch 'js/commit-gpgsign' into pu
Merge branch 'rs/use-modern-git-merge-syntax' into pu
Merge branch 'va/i18n' into pu
Merge branch 'patches' into pu
Merge branch 'pt/git4win-mods' into pu
Merge branch 'pt/non-mouse-usage' into pu
Merge branch 'va/i18n_2' into pu
git-gui: maintain backwards compatibility for merge syntax
Merge branch 'dr/ru' into pu
git-gui: avoid persisting modified author identity
Merge branch 'kb/unicode' into pu
Merge branch 'os/preserve-author' into pu
Merge branch 'as/bulgarian' into pu
git-gui: set version 0.21
René Scharfe (1):
git-gui: stop using deprecated merge syntax
Satoshi Yasushima (6):
git-gui: consistently use the same word for "remote" in Japanese
git-gui: consistently use the same word for "blame" in Japanese
git-gui: apply po template to Japanese translation
git-gui: add Japanese language code
git-gui: update Japanese translation
git-gui: update Japanese information
Vasco Almeida (6):
git-gui i18n: mark strings for translation
git-gui: l10n: add Portuguese translation
git-gui i18n: internationalize use of colon punctuation
git-gui i18n: mark "usage:" strings for translation
git-gui: fix incorrect use of Tcl append command
git-gui i18n: mark string in lib/error.tcl for translation
yaras (1):
git-gui: fix initial git gui message encoding
GIT-VERSION-GEN | 2 +-
Makefile | 2 +-
git-gui.sh | 154 +-
lib/blame.tcl | 2 +-
lib/branch_checkout.tcl | 2 +-
lib/branch_create.tcl | 2 +-
lib/branch_delete.tcl | 4 +-
lib/branch_rename.tcl | 2 +-
lib/browser.tcl | 6 +-
lib/commit.tcl | 39 +-
lib/database.tcl | 4 +-
lib/diff.tcl | 14 +-
lib/error.tcl | 8 +-
lib/index.tcl | 12 +-
lib/merge.tcl | 18 +-
lib/option.tcl | 8 +-
lib/remote.tcl | 8 +-
lib/remote_add.tcl | 2 +-
lib/remote_branch_delete.tcl | 2 +-
lib/shortcut.tcl | 17 +-
lib/themed.tcl | 87 +-
lib/tools.tcl | 3 +
lib/tools_dlg.tcl | 6 +-
lib/transport.tcl | 2 +-
po/bg.po | 3543 ++++++++++++++++++++++--------------------
po/glossary/pt_pt.po | 293 ++++
po/glossary/txt-to-pot.sh | 4 +-
po/ja.po | 3077 ++++++++++++++++++------------------
po/pt_pt.po | 2716 ++++++++++++++++++++++++++++++++
po/ru.po | 680 +++-----
30 files changed, 6957 insertions(+), 3762 deletions(-)
create mode 100644 po/glossary/pt_pt.po
create mode 100644 po/pt_pt.po
^ permalink raw reply
* Re: [PATCH v3 14/25] sequencer: introduce a helper to read files written by scripts
From: Johannes Schindelin @ 2016-10-20 12:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <xmqqpomxr6t6.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 18 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > In the meantime, I'd be happy to just add a comment that this function is
> > intended for oneliners, but that it will also read multi-line files and
> > only strip off the EOL marker from the last line.
> >
> > Would that work for you?
>
> That would be ideal, I would think.
Done,
Dscho
^ permalink raw reply
* Re: [PATCH v4 05/25] sequencer: eventually release memory allocated for the option values
From: Johannes Schindelin @ 2016-10-20 12:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <xmqq1szdnnvc.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 18 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> > To remedy that, we now take custody of the option values in question,
> >> > requiring those values to be malloc()ed or strdup()ed
> >>
> >> That is the approach this patch takes, so "eventually release" in
> >> the title is no longer accurate, I would think.
> >
> > To the contrary, we now free() things in remove_state(), so we still
> > "eventually release" the memory.
>
> OK. We call a change to teach remove_state() to free the resource
> does more commonly as "plug leaks"; the word "eventually" gave me an
> impression that we are emphasizing the fact that we do not free(3)
> immediately but lazily do so at the end, hence my response.
I changed the commit subject, hopefully to your liking,
Dscho
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Jeff King @ 2016-10-20 12:31 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1610201154070.3264@virtualbox>
On Thu, Oct 20, 2016 at 12:17:33PM +0200, Johannes Schindelin wrote:
> If you want to know just how harmful this reliance on shell scripting is
> to our goal of keeping Git portable: already moving from Linux to MacOSX
> costs you roughly 3x as long to run the build & test (~12mins vs ~36mins
> for GCC, according to https://travis-ci.org/git/git/builds/159125647).
Wait, shell scripts are slow on MacOS now?
Perhaps, but it seems more likely that one or more of the following is
true:
- setup of the OS X VM takes longer (it does; if you click-through to
the test results, you'll see that the "make test" step goes from
647s on Linux to 1108s on MacOS. That's much worse, but not even
twice as slow, let alone 3x).
- Travis Linux and OSX VMs do not have identical hardware. Looking at
https://docs.travis-ci.com/user/ci-environment/, it appears that
Linux containers get twice as many cores.
- Git performance on Linux may be better than MacOS. The test suite is
very filesystem-heavy because it creates and destroys a lot of files
and repositories. If the kernel vfs performance is worse, it's
likely to show up in the test suite (especially if the issue is
latency and you aren't doing it massively in parallel).
I don't have a real way to measure that, but it seems like a
plausible factor.
So that sucks that the MacOS Travis build takes a half hour to run. But
I don't think that shell scripting is the culprit.
> So the only thing that would really count as an improvement would be to
> change the test suite in such a manner that it relies more on helpers in
> t/helper/ and less on heavy-duty shell scripting.
>
> Of course, if you continue to resist (because the problem is obviously not
> affecting you personally, so why would you care), I won't even try to find
> the time to start on that project.
I'm not sure what you mean by "resist". The tests suite has been a set
of shell scripts for over a decade. As far as I know there is not
currently a viable alternative. If you have patches that make it faster
without negatively impact the ease of writing tests, I'd be happy to see
them. If you have more t/helper programs that can eliminate expensive
bits of the shell scripts and speed up the test run, great. If you have
some other proposal entirely, I'd love to hear it. But I do not see
that there is any proposal to "resist" at this point.
I'm also not entirely convinced that the test suite being a shell script
is the main culprit for its slowness. We run git a lot of times, and
that's inherent in testing it. I ran the whole test suite under
"strace -f -e execve". There are ~335K execs. Here's the breakdown of
the top ones:
$ perl -lne '/execve\("(.*?)"/ and print $1' /tmp/foo.out | sort | uniq -c | sort -rn | head
152271 /home/peff/compile/git/git
57340 /home/peff/compile/git/t/../bin-wrappers/git
16865 /bin/sed
12650 /bin/rm
11257 /bin/cat
9326 /home/peff/compile/git/git-sh-i18n--envsubst
9079 /usr/bin/diff
8013 /usr/bin/wc
5924 /bin/mv
4566 /bin/grep
Almost half are running git itself. Let's assume that can't be changed.
That leaves ~180K of shell-related overhead (versus the optimal case,
that the entire test suite becomes one monolithic program ;) ).
Close to 1/3 of those processes are just invoking the bin-wrapper
script to set up the EXEC_PATH, etc. I imagine it would not be too hard
to just do that in the test script. In fact, it looks like:
make prefix=/wherever install
GIT_TEST_INSTALLED=/wherever/bin make test
might give you an immediate speedup by skipping bin-wrappers entirely.
The rest of it is harder. I think you'd have to move the test suite to a
language like perl that can do more of that as builtins (I'm sure you'd
enjoy the portability implications of _that_). It would almost be
easier to build a variant of the shell that has sed, rm, cat, and a few
others compiled in.
-Peff
PS I haven't kept up with all of this POSIX-layer stuff that's been
announced in Windows the past few months. Is it a viable path forward
that would have better performance (obviously not in the short term,
but where we may arrive in a few years)?
^ permalink raw reply
* [PATCH] doc: remove reference to the traditional layout in git-tag.txt
From: Younes Khoudli @ 2016-10-20 13:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Younes Khoudli
This is the only place in the documentation that the traditional layout
is mentioned, and it is confusing. Remove it.
* Documentation/git-tag.txt: Here.
Signed-off-by: Younes Khoudli <younes.khoudli@gmail.com>
---
Documentation/git-tag.txt | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7ecca8e..80019c5 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -253,9 +253,8 @@ On Automatic following
~~~~~~~~~~~~~~~~~~~~~~
If you are following somebody else's tree, you are most likely
-using remote-tracking branches (`refs/heads/origin` in traditional
-layout, or `refs/remotes/origin/master` in the separate-remote
-layout). You usually want the tags from the other end.
+using remote-tracking branches (eg. `refs/remotes/origin/master`).
+You usually want the tags from the other end.
On the other hand, if you are fetching because you would want a
one-shot merge from somebody else, you typically do not want to
--
2.10.0
^ 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