Git development
 help / color / mirror / Atom feed
* Re: [PATCH] commit: allow {--amend|-c foo} when {HEAD|foo} has empty message
From: Junio C Hamano @ 2012-02-28 21:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Thomas Rast, Thomas Rast, git
In-Reply-To: <20120228195931.GE11260@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Yeah, I agree that treating it like an empty body is reasonable
> (possibly with a warning). But given that nobody has actually seen this
> in the wild, maybe it is simpler to mark it with fsck, and to just die()
> when we see it. That would hopefully alert the author of the broken tool
> early, before the tools is made public. If it turns out that such
> commits do end up in the wild, then we can relax the behavior then.

Yeah, it is not like we would want to encourage a commit with empty body,
be it preceded with "\n\n" or just a "\n", in the first place.

We would need to locate all the places that expect that strstr("\n\n")
will find something, and add die("commit made with a broken git") at these
places anyway, in addition to the change to fsck. Given that, I suspect
that the extra amount of the work needed to tweak the code to tolerate
such a commit and keep going might not be so big, and going that route
would avoid punishing the users of broken versions of git (or broken
imitations of git, for that matter), so...

^ permalink raw reply

* Re: [PATCH] Documentation: use {asterisk} in rev-list-options.txt when needed
From: Junio C Hamano @ 2012-02-28 21:18 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: Jeff King, git
In-Reply-To: <1330460423.691.15.camel@centaur.lab.cmartin.tk>

Carlos Martín Nieto <cmn@elego.de> writes:

> On Tue, 2012-02-28 at 14:45 -0500, Jeff King wrote:
>> On Tue, Feb 28, 2012 at 04:35:48PM +0100, Carlos Martín Nieto wrote:
>> 
>> > Text between to '*' is emphasized in AsciiDoc which made the
>> 
>> s/to/two/
>
> Oops. Thanks. Can you squash that in, Junio?

Thanks; done.

^ permalink raw reply

* Re: [PATCH 4/3] parse-options: disallow --no-no-sth
From: Junio C Hamano @ 2012-02-28 21:15 UTC (permalink / raw)
  To: René Scharfe
  Cc: Thomas Rast, git, Bert Wesarg, Geoffrey Irving,
	Johannes Schindelin, Pierre Habouzit, Jeff King
In-Reply-To: <4F4D3545.6060704@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Now that options whose definition starts with "no-" can be negated
> by removing said "no-", there is no need anymore to allow them to
> be negated by adding a second "no-", which just looks silly.

Thanks.  But accepting them silently and do what the user would have
expected, especially if we do not advertise it, would not hurt anybody,
no?

^ permalink raw reply

* Re: Stash during incomplete merge
From: Neal Kreitzinger @ 2012-02-28 22:24 UTC (permalink / raw)
  Cc: Phil Hord, git@vger.kernel.org, Phil Hord
In-Reply-To: <4F4D377B.2000206@gmail.com>

On 2/28/2012 2:22 PM, Neal Kreitzinger wrote:
>
> You may also want to consider the --keep-index option on your "git
> stash save" if your "testing" workflow doesn't involve adds or
> commits before the git stash apply/pop.
>
the very limited case I had in mind (and probably incorrect assumption)
about your "testing" workflow was:

hack merge-conflicts
$ git add conflict-resolution  (conflict-resolution is in worktree and
index)
hack conflict-resolution with extra stuff (original conflict-resolution
is still in index)
uh-oh, i got carried away and started doing extra stuff (evil merge) and
forgot to finish testing just the conflict-resolutions (pure merge-commit)
$ git stash --keep-index (conflict-resolution is still in index and now
back in wokrtree)
finish testing just the conflict-resolutions (merge-commit-to-be)
(conflict resolutions worked (I knew they would))
$ git stash pop (original conflict resolution is still in index, but
extra-stuff is back in worktree)
$ git commit (commit the conflict-resolutions/merge-commit)
$ git add foo
$ git commit (new foo stuff committed after merge commit)

v/r,
neal

^ permalink raw reply

* Re: git-subtree Ready #2
From: Avery Pennarun @ 2012-02-28 22:42 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Jeff King, Junio C Hamano, David A. Greene, git
In-Reply-To: <m3ehtfvhkv.fsf@localhost.localdomain>

On Mon, Feb 27, 2012 at 9:04 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Jeff King <peff@peff.net> writes:
>> Yeah, I don't see much point in rewriting. If parts of the history suck,
>> then so be it.  It's probably not that big to store. And while it's
>> sometimes easier to fix bad commit messages when they are recent and in
>> your memory (rather than trying to remember later what you meant to
>> say), I think it is already too late for that. Any archaeology you do
>> now to make good commit messages could probably just as easily be done
>> if and when somebody actually needs the commit message later (emphasis
>> on the "if" -- it's likely that nobody will care about most of the
>> commit messages later at all).
>
> Anyway we already have subtree merges if subsystem with bad error
> messages -- see gitweb.

So be it then!  May my lame commit messages persist forever!  As if I
don't have enough embarrassing stuff on the Internet.

(Personally I think the vast majority of the commit messages are
perfectly fine, and the ones that aren't generally describe boring
commits anyway, like changes to the 'todo' file.)

Have fun,

Avery

^ permalink raw reply

* Re: Announcing 3 git docs: Best Practices, fixing mistakes, post-production editing
From: Jeff King @ 2012-02-28 22:52 UTC (permalink / raw)
  To: Seth Robertson; +Cc: git
In-Reply-To: <201202281304.q1SD4U8W018223@no.baka.org>

On Tue, Feb 28, 2012 at 08:04:30AM -0500, Seth Robertson wrote:

> Commit Often, Perfect Later, Publish Once: Git Best Practices
> ----------------------------------------------------------------------
> http://sethrobertson.github.com/GitBestPractices

I have only read the first of the three so far, but it looks very nice.
I did notice a few things which were worth commenting on (I'll quote
directly from the page in question below).

> [section Don't Panic, subsection Lost and Found]
>
> Dangling Commit
>
> These are the most likely candidates for finding lost data. A dangling
> commit is a commit no longer reachable by any branch or tag. This can
> happen due to resets and rebases and are normal. git show SHA will let
> you inspect them.

Resets and rebases record the commits in the reflog (at the very least
in the HEAD reflog), and should generally not be the cause of dangling
commits (the objects should usually expire in the same "git gc" that
expires the reflog entries). I suspect a more common cause is deleting
branches, which leaves no reflog (the commits may be in the HEAD reflog
if they were ever checked out, though).

It's somewhat minor; the overall advice ("do not worry about dangling
commits") holds. But it might be worth pointing out that the method for
recovering an accidentally deleted branch is usually:

  1. look in the HEAD reflog

  2. if you can't find it there, try dangling commits

> [section Do make useful commit messages]

This talks about formatting, but not about content. I have long wanted
to write a nice essay on what should go into a good commit message, but
when I've tried it ends up very specific to the project, the type of
commit, and the individual change. I wonder if anybody knows of
something good you could link to.

> [section On Sausage Making]
>
> Some people like to hide the sausage making, or in other words pretend to
> the outside world that their commits sprung full-formed in utter
> perfection into their git repository. Certain large public projects
> demand this, others demand smushing all work into one large commit, and
> still others do not care.
>
> A good reason to hide the sausage making is if you feel you may be
> cherry-picking commits a lot (though this too is often a sign of bad
> workflow). Having one or a small number of commits to pick is much
> easier than having to find one commit here, one there, and half of this
> other one. The latter approach makes your problem much much harder and
> typically will lead to merge conflicts when the donor branch is finally
> merged in.
>
> Another good reason is to ensure each commit compiles and/or passes
> regression tests, and represents a different easily understood concept
> (important for archeology). The former allows git-bisect to chose any
> commit and have a good chance of that commit doing something useful, and
> the latter allows for easy change review, understanding, and
> cherry-picking.

This is a nice overview of the motivation, but I think it misses one
of the main reasons we clean up patches in git.git: code review. When
you publish patches, you have several audiences. One of those audiences
is the end user who will compile release v1.5. They only care about the
end result of the patches. Another is people bisecting, who care that
everything intermediate builds and is reasonable; you can satisfy that
by checking each commit against a test suite. Another is people reading
the logs later to find out what happened; it's OK for them to see that a
bug was in the initial version, and then fixed 5 minutes later.

But yet another audience is reviewers who will read your changes and
decide they should be applied, rejected, or re-worked. For those people,
it is much harder to review a series that introduces a bug in patch 1,
but fixes it in patch 5. The reviewer may also notice the bug, take time
thinking about and writing an analysis, and then get frustrated to find
that their work was wasted when they get to patch 5.

The alternative is that they stop thinking about individual patches and
consider the whole series (e.g., when they see patch 1 has a bug, stop
reading and look through the other patches for a fix). But that makes
review much harder, because you have to think about a much larger series
of changes.

By cleaning up patches into single, logical changes that build on one
another, and which don't individually regress (i.e., they are always
moving towards some desirable common endpoint), the author is writing a
chronological story not of what happened, but what _should_ happen, with
the intent that the audience (i.e., reviewers) are convinced that the
change is the right thing to do.

I really liked your movie analogy. Patch series are really just
documentaries about a change, arranged for greatest impact on the
viewer. :)

> [Do periodic maintenance]
>
> Compact your repo (git gc --aggressive)
>
> This will removed outdated dangling objects (after the two+ week grace
> period). It will also compress any loose objects git has added since
> your last gc. git will run gc automatically after certain commands, but
> doing a manual --aggressive will save space and speed git operations.

Most people shouldn't be using "--aggressive". Unless you have an
existing pack that is poorly packed (e.g., because you did a
fast-import that did not do much delta searching), you are not going to
see much benefit, and it will take a lot longer. Basically the three
levels of "gc" are:

  1. git gc --auto; if there are too many loose objects, they will all
     go into a new incremental pack. If there are already too many
     packs, all of the existing packs will get re-packed together.

     If we are making an incremental pack, this is by far the fastest,
     because the speed is independent of the existing history. If we
     pack everything together, it should be more or less the same as (2)
     below.

  2. git gc; this packs everything into a single pack. It does not use
     high window and depth parameters, but more importantly, it reuses
     existing deltas. That makes the delta compression phase _much_
     faster, and it often makes the writing phase faster (because for
     older objects, we are primarily streaming them right out of the
     existing pack). On a big repo, though, it does do a lot of I/O,
     because it has to rewrite the whole pack.

  3. git gc --aggressive; this is often way slower than the above
     because we throw out all of the existing deltas and recompute them
     from scratch. The higher window parameter means it will spend a bit
     more time computing, and it may end up with a smaller pack.

In practical applications, I would expect (2) to achieve similar results to (3). If
that isn't the case, then I think we should be tuning up the default
window and depth parameters for non-aggressive "git gc" a bit.

> [section Miscellaneous "don't"s]
>
> create very large repositories (when possible)
>
> Git can be slow in the face of large repositories. There are
> git-config options that can help. pack.threads=1 pack.deltaCacheSize=1
> pack.windowMemory=512m core.packedGitWindowSize=16m
> core.packedGitLimit=128m. Other likely ones exist.

It might help to qualify "big" here. To some people, 10,000 files,
50,000 commits, and a 200M packfile is big. But that's a fraction of
linux-2.6, which most people use. I think big here is probably getting
into 100K-200K files (where the time to stat() files becomes noticeable,
commits are probably not relevant (because git is usually good at only
looking at recent bits of history for most operations), and packfiles
above 1G or so start to get cumbersome (mostly because of the I/O on a
full repack; but then you should consider marking a pack as .keep).

But those numbers are just pulled out of a hat based on the last few
years. Your OS, your hardware, and your expectations make a huge
difference in what seems reasonable.

Your config recommendations seem mostly related to relieving memory
pressure for packing (at the expense of making the pack a lot slower).
Dropping --aggressive from your gc might help a lot with that, too. It
might be worth noting that you should only start twiddling these options
if you are running out of memory during a repack. They will not affect
git performance for day-to-day commands.

I don't think you should need to adjust core.packedGitWindowSize or
core.packedGitLimit at all. Those files are mmap'd, so it is up to the
OS to be reasonable about faulting in or releasing the memory. The main
motivation of pack windows is not memory _usage_, but rather getting a
large contiguous chunk of the address space. mmap-ing a 4G packfile on a
32-bit system just doesn't work. But the defaults are set to reasonable
values for each architecture.

-Peff

^ permalink raw reply

* Re: [PATCH] Documentation: use {asterisk} in rev-list-options.txt when needed
From: Thomas Rast @ 2012-02-28 23:03 UTC (permalink / raw)
  To: Jeff King; +Cc: Carlos Martín Nieto, git
In-Reply-To: <20120228194551.GC11725@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Anyway, that is not a problem with your patch. :) I confirmed that the
> bug happens in my version of the toolchain, and your fix works (I also
> tried using `*`, but backtick does not suppress markup.

Actually it would, if it weren't for our use of '-a no-inline-literal'.
Which we are using because the "backticks do not interpret {stuff}"
feature was introduced in a backwards-incompatible way in asciidoc
8.4.1, see 71c020c (Disable asciidoc 8.4.1+ semantics for `{plus}` and
friends, 2009-07-25).

Some googling tells me the distros are currently at

  openSuSE         8.4.5   (local)
  Debian (stable)  8.5.2   packages.debian.org
  Fedora           8.4.5   admin.fedoraproject.org/pkgdb/
                           (but I can't discern whether it's actually in there...)
  Arch             8.6.6   www.archlinux.org/packages/
  Ubuntu 11.10     8.6.4   packages.ubuntu.com

so perhaps the time has come to remove that switch?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* [PATCH 1/2] am -3: allow nonstandard -p<num> option
From: Junio C Hamano @ 2012-02-28 23:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1330471495-12013-1-git-send-email-gitster@pobox.com>

When falling back to 3-way merge, we run "git apply" to synthesize the
fake ancestor tree by parsing the incoming patch, and another "git apply"
to apply the patch to the fake ancestor tree.  Both invocation need to
be aware of the custom -p<num> setting to parse patches that were prepared
with non-standard src/dst prefix.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-am.sh |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 1c13b13..d5d168f 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -127,15 +127,18 @@ fall_back_3way () {
     mkdir "$dotest/patch-merge-tmp-dir"
 
     # First see if the patch records the index info that we can use.
-    git apply --build-fake-ancestor "$dotest/patch-merge-tmp-index" \
-	"$dotest/patch" &&
+    cmd="git apply $git_apply_opt --build-fake-ancestor" &&
+    cmd="$cmd "'"$dotest/patch-merge-tmp-index" "$dotest/patch"' &&
+    eval "$cmd" &&
     GIT_INDEX_FILE="$dotest/patch-merge-tmp-index" \
     git write-tree >"$dotest/patch-merge-base+" ||
     cannot_fallback "$(gettext "Repository lacks necessary blobs to fall back on 3-way merge.")"
 
     say Using index info to reconstruct a base tree...
-    if GIT_INDEX_FILE="$dotest/patch-merge-tmp-index" \
-	git apply --cached <"$dotest/patch"
+
+    cmd='GIT_INDEX_FILE="$dotest/patch-merge-tmp-index"'
+    cmd="$cmd git apply --cached $git_apply_opt"' <"$dotest/patch"'
+    if eval "$cmd"
     then
 	mv "$dotest/patch-merge-base+" "$dotest/patch-merge-base"
 	mv "$dotest/patch-merge-tmp-index" "$dotest/patch-merge-index"
-- 
1.7.9.2.344.g3e8c86

^ permalink raw reply related

* [PATCH 0/2] "am -3" and "--no-prefix" does not work well
From: Junio C Hamano @ 2012-02-28 23:24 UTC (permalink / raw)
  To: git

I wrote these a few days ago when I tried to review somebody's patch
on this list that was made with "format-patch --no-prefix".  The regular
patch application without the three-way fallback was prepared to honor a
custom -p<num>, but the code to synthesize the fake ancestor tree forgot
to pay attention to -p<num>, so "am -3 -p0" was failing.

Junio C Hamano (2):
  am -3: allow nonstandard -p<num> option
  test: "am -3" can accept non-standard -p<num>

 git-am.sh     |   11 +++++++----
 t/t4150-am.sh |   15 +++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

-- 
1.7.9.2.344.g3e8c86

^ permalink raw reply

* [PATCH 2/2] test: "am -3" can accept non-standard -p<num>
From: Junio C Hamano @ 2012-02-28 23:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1330471495-12013-1-git-send-email-gitster@pobox.com>

This adds a test for the previous one to make sure that "am -3 -p0" can
read patches created with the --no-prefix option.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4150-am.sh |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index d7d9ccc..e1d381c 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -123,6 +123,7 @@ test_expect_success setup '
 	git commit -m "added another file" &&
 
 	git format-patch --stdout master >lorem-move.patch &&
+	git format-patch --no-prefix --stdout master >lorem-zero.patch &&
 
 	git checkout -b rename &&
 	git mv file renamed &&
@@ -276,6 +277,20 @@ test_expect_success 'am -3 falls back to 3-way merge' '
 	git diff --exit-code lorem
 '
 
+test_expect_success 'am -3 -p0 can read --no-prefix patch' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout -b lorem3 master2 &&
+	sed -n -e "3,\$p" msg >file &&
+	head -n 9 msg >>file &&
+	git add file &&
+	test_tick &&
+	git commit -m "copied stuff" &&
+	git am -3 -p0 lorem-zero.patch &&
+	! test -d .git/rebase-apply &&
+	git diff --exit-code lorem
+'
+
 test_expect_success 'am can rename a file' '
 	grep "^rename from" rename.patch &&
 	rm -fr .git/rebase-apply &&
-- 
1.7.9.2.344.g3e8c86

^ permalink raw reply related

* [PATCH] fsck: --no-dangling omits "dangling object" information
From: Junio C Hamano @ 2012-02-28 23:25 UTC (permalink / raw)
  To: git; +Cc: Clemens Buchacher, Jeff King
In-Reply-To: <7vhayc55a7.fsf@alter.siamese.dyndns.org>

The default output from "fsck" is often overwhelmed by informational
message on dangling objects, especially if you do not repack often, and a
real error can easily be buried.

Add "--no-dangling" option to omit them, and update the user manual to
demonstrate its use.

Based on a patch by Clemens Buchacher, but reverted the part to change
the default to --no-dangling, which is unsuitable for the first patch.
The usual three-step procedure to break the backward compatibility over
time needs to happen on top of this, if we were to go in that direction.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I butchered Clemens's patch and added "--no-dangling", and reverted the
   part that flips the default.  I also took the authorship blame, as I do
   not share the final goal of fliping the default (yet), but our first
   steps should be in the same direction, which is this patch.

 Documentation/git-fsck.txt    |    7 ++++++-
 Documentation/git-repack.txt  |    2 +-
 Documentation/user-manual.txt |   15 +++++++--------
 builtin/fsck.c                |    7 +++++--
 t/t1450-fsck.sh               |    6 +-----
 5 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index 6c47395..47e2f19 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]
 	 [--[no-]full] [--strict] [--verbose] [--lost-found]
-	 [--[no-]progress] [<object>*]
+	 [--[no-]dangling] [--[no-]progress] [<object>*]
 
 DESCRIPTION
 -----------
@@ -30,6 +30,11 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless
 	Print out objects that exist but that aren't reachable from any
 	of the reference nodes.
 
+--dangling::
+--no-dangling::
+	Print objects that exist but that are never 'directly' used (default).
+	`--no-dangling` can be used to squech this information from the output.
+
 --root::
 	Report root nodes.
 
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 40af321..4c1aff6 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -34,7 +34,7 @@ OPTIONS
 	Especially useful when packing a repository that is used
 	for private development. Use
 	with '-d'.  This will clean up the objects that `git prune`
-	leaves behind, but `git fsck --full` shows as
+	leaves behind, but `git fsck --full --dangling` shows as
 	dangling.
 +
 Note that users fetching over dumb protocols will have to fetch the
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index f13a846..6c7fee7 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1582,7 +1582,7 @@ Checking the repository for corruption
 
 The linkgit:git-fsck[1] command runs a number of self-consistency checks
 on the repository, and reports on any problems.  This may take some
-time.  The most common warning by far is about "dangling" objects:
+time.
 
 -------------------------------------------------
 $ git fsck
@@ -1597,9 +1597,11 @@ dangling tree b24c2473f1fd3d91352a624795be026d64c8841f
 ...
 -------------------------------------------------
 
-Dangling objects are not a problem.  At worst they may take up a little
-extra disk space.  They can sometimes provide a last-resort method for
-recovering lost work--see <<dangling-objects>> for details.
+You will see informational messages on dangling objects. They are objects
+that still exist in the repository but are no longer referenced by any of
+your branches, and can (and will) be removed after a while with "gc".
+You can run `git fsck --no-dangling` to supress these messages, and still
+view real errors.
 
 [[recovering-lost-changes]]
 Recovering lost changes
@@ -3295,15 +3297,12 @@ it is with linkgit:git-fsck[1]; this may be time-consuming.
 Assume the output looks like this:
 
 ------------------------------------------------
-$ git fsck --full
+$ git fsck --full --no-dangling
 broken link from    tree 2d9263c6d23595e7cb2a21e5ebbb53655278dff8
               to    blob 4b9458b3786228369c63936db65827de3cc06200
 missing blob 4b9458b3786228369c63936db65827de3cc06200
 ------------------------------------------------
 
-(Typically there will be some "dangling object" messages too, but they
-aren't interesting.)
-
 Now you know that blob 4b9458b3 is missing, and that the tree 2d9263c6
 points to it.  If you could find just one copy of that missing blob
 object, possibly in some other repository, you could move it into
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 8c479a7..67eb553 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -29,6 +29,7 @@ static int errors_found;
 static int write_lost_and_found;
 static int verbose;
 static int show_progress = -1;
+static int show_dangling = 1;
 #define ERROR_OBJECT 01
 #define ERROR_REACHABLE 02
 #define ERROR_PACK 04
@@ -221,8 +222,9 @@ static void check_unreachable_object(struct object *obj)
 	 * start looking at, for example.
 	 */
 	if (!obj->used) {
-		printf("dangling %s %s\n", typename(obj->type),
-		       sha1_to_hex(obj->sha1));
+		if (show_dangling)
+			printf("dangling %s %s\n", typename(obj->type),
+			       sha1_to_hex(obj->sha1));
 		if (write_lost_and_found) {
 			char *filename = git_path("lost-found/%s/%s",
 				obj->type == OBJ_COMMIT ? "commit" : "other",
@@ -614,6 +616,7 @@ static char const * const fsck_usage[] = {
 static struct option fsck_opts[] = {
 	OPT__VERBOSE(&verbose, "be verbose"),
 	OPT_BOOLEAN(0, "unreachable", &show_unreachable, "show unreachable objects"),
+	OPT_BOOL(0, "dangling", &show_dangling, "show dangling objects"),
 	OPT_BOOLEAN(0, "tags", &show_tags, "report tags"),
 	OPT_BOOLEAN(0, "root", &show_root, "report root nodes"),
 	OPT_BOOLEAN(0, "cache", &keep_cache_objects, "make index objects head nodes"),
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 5b8ebd8..5b79c51 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -27,12 +27,8 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 		git init &&
 		echo ../../../.git/objects >.git/objects/info/alternates &&
 		test_commit C fileC one &&
-		git fsck >../out 2>&1
+		git fsck --no-dangling >../actual 2>&1
 	) &&
-	{
-		grep -v dangling out >actual ||
-		:
-	} &&
 	test_cmp empty actual
 '
 
-- 
1.7.9.2.344.g3e8c86

^ permalink raw reply related

* [PATCH v2 0/2] "log --regexp-ignore-case -S/-G<string>"
From: Junio C Hamano @ 2012-02-29  0:20 UTC (permalink / raw)
  To: git

This is a re-roll of the previous attempt to teach pickaxe to match
case insensitively.

Junio C Hamano (2):
  grep: use static trans-case table
  pickaxe: allow -i to search in patch case-insensitively

 cache.h                |    3 ++
 ctype.c                |   36 +++++++++++++++
 diff.h                 |    1 +
 diffcore-pickaxe.c     |    9 +++-
 grep.c                 |   11 ++---
 revision.c             |    1 +
 t/t4209-log-pickaxe.sh |  119 ++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 170 insertions(+), 10 deletions(-)
 create mode 100755 t/t4209-log-pickaxe.sh

-- 
1.7.9.2.344.g3e8c86

^ permalink raw reply

* [PATCH v2 1/2] grep: use static trans-case table
From: Junio C Hamano @ 2012-02-29  0:20 UTC (permalink / raw)
  To: git
In-Reply-To: <1330474831-9030-1-git-send-email-gitster@pobox.com>

In order to prepare the kwset machinery for a case-insensitive search, we
used to use a static table of 256 elements and filled it every time before
calling kwsalloc().  Because the kwset machinery will never modify this
table, just allocate a single instance globally and fill it at the compile
time.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h |    3 +++
 ctype.c |   36 ++++++++++++++++++++++++++++++++++++
 grep.c  |   11 +++--------
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/cache.h b/cache.h
index 79c612f..79dc305 100644
--- a/cache.h
+++ b/cache.h
@@ -1258,4 +1258,7 @@ extern struct startup_info *startup_info;
 /* builtin/merge.c */
 int checkout_fast_forward(const unsigned char *from, const unsigned char *to);
 
+/* in ctype.c, for kwset users */
+extern const char tolower_trans_tbl[256];
+
 #endif /* CACHE_H */
diff --git a/ctype.c b/ctype.c
index b5d856f..7c14d85 100644
--- a/ctype.c
+++ b/ctype.c
@@ -25,3 +25,39 @@ unsigned char sane_ctype[256] = {
 	A, A, A, A, A, A, A, A, A, A, A, R, R, 0, P, 0,		/* 112..127 */
 	/* Nothing in the 128.. range */
 };
+
+/* For case-insensitive kwset */
+const char tolower_trans_tbl[256] = {
+	0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+	0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+	0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+	0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+	 ' ',  '!',  '"',  '#',  '$',  '%',  '&', 0x27,
+	 '(',  ')',  '*',  '+',  ',',  '-',  '.',  '/',
+	 '0',  '1',  '2',  '3',  '4',  '5',  '6',  '7',
+	 '8',  '9',  ':',  ';',  '<',  '=',  '>',  '?',
+	 '@',  'a',  'b',  'c',  'd',  'e',  'f',  'g',
+	 'h',  'i',  'j',  'k',  'l',  'm',  'n',  'o',
+	 'p',  'q',  'r',  's',  't',  'u',  'v',  'w',
+	 'x',  'y',  'z',  '[', 0x5c,  ']',  '^',  '_',
+	 '`',  'a',  'b',  'c',  'd',  'e',  'f',  'g',
+	 'h',  'i',  'j',  'k',  'l',  'm',  'n',  'o',
+	 'p',  'q',  'r',  's',  't',  'u',  'v',  'w',
+	 'x',  'y',  'z',  '{',  '|',  '}',  '~', 0x7f,
+	0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87,
+	0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f,
+	0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97,
+	0x98, 0x99, 0x9a, 0x9b, 0x9c, 0x9d, 0x9e, 0x9f,
+	0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7,
+	0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf,
+	0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5, 0xb6, 0xb7,
+	0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbd, 0xbe, 0xbf,
+	0xc0, 0xc1, 0xc2, 0xc3, 0xc4, 0xc5, 0xc6, 0xc7,
+	0xc8, 0xc9, 0xca, 0xcb, 0xcc, 0xcd, 0xce, 0xcf,
+	0xd0, 0xd1, 0xd2, 0xd3, 0xd4, 0xd5, 0xd6, 0xd7,
+	0xd8, 0xd9, 0xda, 0xdb, 0xdc, 0xdd, 0xde, 0xdf,
+	0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xe7,
+	0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef,
+	0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
+	0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff,
+};
diff --git a/grep.c b/grep.c
index b29d09c..1030f38 100644
--- a/grep.c
+++ b/grep.c
@@ -168,15 +168,10 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
 		p->fixed = 0;
 
 	if (p->fixed) {
-		if (opt->regflags & REG_ICASE || p->ignore_case) {
-			static char trans[256];
-			int i;
-			for (i = 0; i < 256; i++)
-				trans[i] = tolower(i);
-			p->kws = kwsalloc(trans);
-		} else {
+		if (opt->regflags & REG_ICASE || p->ignore_case)
+			p->kws = kwsalloc(tolower_trans_tbl);
+		else
 			p->kws = kwsalloc(NULL);
-		}
 		kwsincr(p->kws, p->pattern, p->patternlen);
 		kwsprep(p->kws);
 		return;
-- 
1.7.9.2.344.g3e8c86

^ permalink raw reply related

* [PATCH v2 2/2] pickaxe: allow -i to search in patch case-insensitively
From: Junio C Hamano @ 2012-02-29  0:20 UTC (permalink / raw)
  To: git
In-Reply-To: <1330474831-9030-1-git-send-email-gitster@pobox.com>

"git log -S<string>" is a useful way to find the last commit in the
codebase that touched the <string>. As it was designed to be used by a
porcelain script to dig the history starting from a block of text that
appear in the starting commit, it never had to look for anything but an
exact match.

When used by an end user who wants to look for the last commit that
removed a string (e.g. name of a variable) that he vaguely remembers,
however, it is useful to support case insensitive match.

When given the "--regexp-ignore-case" (or "-i") option, which originally
was designed to affect case sensitivity of the search done in the commit
log part, e.g. "log --grep", the matches made with -S/-G pickaxe search is
done case insensitively now.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.h                 |    1 +
 diffcore-pickaxe.c     |    9 +++-
 revision.c             |    1 +
 t/t4209-log-pickaxe.sh |  119 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 128 insertions(+), 2 deletions(-)
 create mode 100755 t/t4209-log-pickaxe.sh

diff --git a/diff.h b/diff.h
index 0c51724..436b574 100644
--- a/diff.h
+++ b/diff.h
@@ -80,6 +80,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)
 #define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)
 #define DIFF_OPT_FUNCCONTEXT         (1 << 29)
+#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 380a837..ed23eb4 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -138,8 +138,12 @@ static void diffcore_pickaxe_grep(struct diff_options *o)
 {
 	int err;
 	regex_t regex;
+	int cflags = REG_EXTENDED | REG_NEWLINE;
 
-	err = regcomp(&regex, o->pickaxe, REG_EXTENDED | REG_NEWLINE);
+	if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
+		cflags |= REG_ICASE;
+
+	err = regcomp(&regex, o->pickaxe, cflags);
 	if (err) {
 		char errbuf[1024];
 		regerror(err, &regex, errbuf, 1024);
@@ -237,7 +241,8 @@ static void diffcore_pickaxe_count(struct diff_options *o)
 		}
 		regexp = &regex;
 	} else {
-		kws = kwsalloc(NULL);
+		kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)
+			       ? tolower_trans_tbl : NULL);
 		kwsincr(kws, needle, len);
 		kwsprep(kws);
 	}
diff --git a/revision.c b/revision.c
index 8764dde..971b7dc 100644
--- a/revision.c
+++ b/revision.c
@@ -1559,6 +1559,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->grep_filter.regflags |= REG_EXTENDED;
 	} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
 		revs->grep_filter.regflags |= REG_ICASE;
+		DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
 	} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
 		revs->grep_filter.fixed = 1;
 	} else if (!strcmp(arg, "--all-match")) {
diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
new file mode 100755
index 0000000..eed7273
--- /dev/null
+++ b/t/t4209-log-pickaxe.sh
@@ -0,0 +1,119 @@
+#!/bin/sh
+
+test_description='log --grep/--author/--regexp-ignore-case/-S/-G'
+. ./test-lib.sh
+
+test_expect_success setup '
+	>file &&
+	git add file &&
+	test_tick &&
+	git commit -m initial &&
+
+	echo Picked >file &&
+	test_tick &&
+	git commit -a --author="Another Person <another@example.com>" -m second
+'
+
+test_expect_success 'log --grep' '
+	git log --grep=initial --format=%H >actual &&
+	git rev-parse --verify HEAD^ >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --grep --regexp-ignore-case' '
+	git log --regexp-ignore-case --grep=InItial --format=%H >actual &&
+	git rev-parse --verify HEAD^ >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --grep -i' '
+	git log -i --grep=InItial --format=%H >actual &&
+	git rev-parse --verify HEAD^ >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --author --regexp-ignore-case' '
+	git log --regexp-ignore-case --author=person --format=%H >actual &&
+	git rev-parse --verify HEAD >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --author -i' '
+	git log -i --author=person --format=%H >actual &&
+	git rev-parse --verify HEAD >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -G (nomatch)' '
+	git log -Gpicked --format=%H >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -G (match)' '
+	git log -GPicked --format=%H >actual &&
+	git rev-parse --verify HEAD >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -G --regexp-ignore-case (nomatch)' '
+	git log --regexp-ignore-case -Gpickle --format=%H >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -G -i (nomatch)' '
+	git log -i -Gpickle --format=%H >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -G --regexp-ignore-case (match)' '
+	git log --regexp-ignore-case -Gpicked --format=%H >actual &&
+	git rev-parse --verify HEAD >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -G -i (match)' '
+	git log -i -Gpicked --format=%H >actual &&
+	git rev-parse --verify HEAD >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -S (nomatch)' '
+	git log -Spicked --format=%H >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -S (match)' '
+	git log -SPicked --format=%H >actual &&
+	git rev-parse --verify HEAD >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -S --regexp-ignore-case (match)' '
+	git log --regexp-ignore-case -Spicked --format=%H >actual &&
+	git rev-parse --verify HEAD >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -S -i (match)' '
+	git log -i -Spicked --format=%H >actual &&
+	git rev-parse --verify HEAD >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -S --regexp-ignore-case (nomatch)' '
+	git log --regexp-ignore-case -Spickle --format=%H >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log -S -i (nomatch)' '
+	git log -i -Spickle --format=%H >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+test_done
-- 
1.7.9.2.344.g3e8c86

^ permalink raw reply related

* Re: Announcing 3 git docs: Best Practices, fixing mistakes, post-production editing
From: Junio C Hamano @ 2012-02-29  1:00 UTC (permalink / raw)
  To: Seth Robertson; +Cc: git
In-Reply-To: <201202281304.q1SD4U8W018223@no.baka.org>

Just a few I noticed that are dubious to be in a document that is meant to
describe "best practices".

"Do commit early and often"
---------------------------

* "Personally ... history of this repository!".  That looks somewhat out
  of place when you are trying to document "best practices".


"Don't panic"
-------------

* As we never "auto-stash", anything that is on stash is by definition
  what the user deliberately placed, just like a commit on a branch that
  the user may have forgotten.  So it is strange to count it as one of the
  three places that "lost" commit may be hiding.  If you make it four and
  add "a branch you might have forgotten" to the mix, it would make a bit
  more sense, though.

* The example command line for gitk passes --all and also everything from
  "log -g" output, which should be OK for toy history, but wouldn't be
  such a good idea when you can expect tons of data from "log -g".

  Doesn't "gitk" itself accept -g these days?

* Lost and found

  Why "git ls-tree -r"?  Doesn't "git show" work eqully well?

  Also, the name of the hash we happen to use to produce the "object name"
  is "SHA-1", so either of these two are fine, but do not say "SHA"
  (throughout the document).


"On Sausage Making"
-------------------

* The desription of "downside" shows a bias against efforts to strive for
  useful history, and also shows ignorance of the true motivation behind
  such discipline. It is _not_ blame or ego. It is all about leaving a
  history others can later use to understand _why_ the code became the way
  it is now, to make it less likely for others to break it.

  If I were writing this, I would either remove that one paragraph
  altogether, or tone it down dramatically.  There is a short-term
  downside that you would be spending time on perfecting the history
  instead of advancing the tip of the branch, especially when you know the
  tree at the tip of the perfected history will be identical to the tip of
  the messy history you currently have.  If you plan to leave the project
  in a month or so and will never look back, that is totally wasted effort
  as maintaining the result will be other people's problem.  But if you
  are planning to be involved in the project for a longer haul, the time
  and effort is worth spending to make less-than-useful history into
  useful one.


"Do keep up to date"
--------------------

* You explained in "Do choose a workflow" section that different workflows
  suite different projects.  It would read better to rephrase this
  paragraph in which you are admitting that not everybody agrees with your
  "pull --rebase".  Instead of saying "but they should agree with me", it
  would be more useful to say in what workflow and the workflow elements
  such as "pull --rebase" you advocate in this section are suited (you do
  not have to say in what other workflow they are inappropriate).

I stopped reading at this point, but will look at the rest some other day.
Thanks for a fun reading.

^ permalink raw reply

* Re: Tilde spec - befuzzled
From: Andrew Ardill @ 2012-02-29  1:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, Andreas Ericsson, Luke Diamand, Git List
In-Reply-To: <7vaa42zrw8.fsf@alter.siamese.dyndns.org>

On 29 February 2012 06:20, Junio C Hamano <gitster@pobox.com> wrote:
> Thomas Rast <trast@inf.ethz.ch> writes:
>
>>>> '<rev>{tilde}<n>', e.g. 'master{tilde}3'::
>>>> A suffix '{tilde}<n>' to a revision parameter means the commit
>>>> object that is the <n>th generation grand-parent of the named
>>>> commit object, following only the first parents.
>>>>
>>>> Hang on, *grand*-parents?
>>>> ...
>>
>> Perhaps we should reword it as "n-th first-parent ancestor"?  Barring
>> confusion about the position of the dashes, that leaves little room for
>> error.
>
> I think we could either go "easier to read but not precise"
>
>        ... that is the <n>th generation (grand-)parent of ...
>
> or "may sound scary but correct"
>
>        the ancestor reached by walking the first-parent chain <n> times
>
> I am not sure which bucket "n-th first-parent ancestor" falls into.

The terms might be too technical, however my understanding was that
HEAD^n takes <n> steps along a breadth-first traversal of the commit
tree rooted at HEAD, while HEAD~n uses a depth-first traversal.

A better form for the description might come from that formulation of
the process, rather than the 'generational' formulation.

Regards,

Andrew Ardill

^ permalink raw reply

* Re: [PATCH v2] Perform cheaper connectivity check when pack is used as medium
From: Nguyen Thai Ngoc Duy @ 2012-02-29  1:41 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Junio C Hamano
In-Reply-To: <4F4CF58A.9090502@viscovery.net>

2012/2/28 Johannes Sixt <j.sixt@viscovery.net>:
> Am 2/28/2012 14:18, schrieb Nguyễn Thái Ngọc Duy:
>> Without the patch:
>> $ time git fetch file:///home/pclouds/w/git/.git
>> remote: Counting objects: 125638, done.
>> remote: Compressing objects: 100% (33201/33201), done.
> ...
>> With the patch:
>> $ time git fetch file:///home/pclouds/w/git/.git
>> remote: Counting objects: 125647, done.
>> remote: Compressing objects: 100% (33209/33209), done.
>
> It is a bit irritating that the number are different when they should be
> identical...

~/w/git is my working directory so objects may change a little bit.
Should have fetched from a clone instead, sorry.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v7 02/10] Stop starting pager recursively
From: Nguyen Thai Ngoc Duy @ 2012-02-29  1:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Ramsay Jones
In-Reply-To: <7vipiqzsd8.fsf@alter.siamese.dyndns.org>

2012/2/29 Junio C Hamano <gitster@pobox.com>:
>> A natural question that may come after reading only the above is if "git
>> column" is the only one that has this problem.  In other words, is the
>> undesirable behaviour you observed caused by a bug in setup_pager() that
>> needs to be fixed, or should it be fixed in "git column"?
>
> Put another way, if there is another git command X that can be used as a
> filter to the output of a git command Y, do you suffer from the same issue
> to when you abuse the GIT_PAGER mechanism to pipe the output from Y to X?
> That is a sure sign that the pager mechanism needs improvement (obviously,
> an alternative answer could be "don't do that then", though).
>
> For example, shortlog is designed to be X for Y=log, i.e.
>
>        $ git log v1.0.0.. | git shortlog
>
> is a perfectly valid way to use the command.  I could imagine that this
> patch may improve the situation if you abuse GIT_PAGER mechanism to
> implement the above pipeline, i.e.
>
>        $ GIT_PAGER="git -p shortlog" git log v1.0.0..
>
> Although I never tried it.

It does improve that situation, or at least it stops git from forking
frantically.
-- 
Duy

^ permalink raw reply

* filtering out mode-change-only changes
From: Neal Kreitzinger @ 2012-02-29  2:31 UTC (permalink / raw)
  To: git

What is the best way to filter out the "mode change only" entries from a 
"name-status diff result" listing of changed files?

Reason-for-this:
get a list of files whose content changed and feed that list into a 
gui-diff-tool for visual review of "merge" (rebase) results.

Partial-Solutions:
--name-status does not have mode-change info.
--raw has mode change info but I would have to parse it out and compare it 
myself.
--summary has that info, but not the content modification info.

Before I write a script to discern "mode-change-only changes" and remove 
them from the list of changed files (thus leaving only content-changed files 
in the list), I'd like to see if there is some way that git 
already-does-this-for-you, or if someone already has a script that does 
this.

Thanks in advance for any tips.

v/r,
neal 

^ permalink raw reply

* Re: [PATCH 1/2] am -3: allow nonstandard -p<num> option
From: Jeff King @ 2012-02-29  2:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1330471495-12013-2-git-send-email-gitster@pobox.com>

On Tue, Feb 28, 2012 at 03:24:54PM -0800, Junio C Hamano wrote:

> When falling back to 3-way merge, we run "git apply" to synthesize the
> fake ancestor tree by parsing the incoming patch, and another "git apply"
> to apply the patch to the fake ancestor tree.  Both invocation need to
> be aware of the custom -p<num> setting to parse patches that were prepared
> with non-standard src/dst prefix.

Makes sense. One question:

> diff --git a/git-am.sh b/git-am.sh
> index 1c13b13..d5d168f 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -127,15 +127,18 @@ fall_back_3way () {
>      mkdir "$dotest/patch-merge-tmp-dir"
>  
>      # First see if the patch records the index info that we can use.
> -    git apply --build-fake-ancestor "$dotest/patch-merge-tmp-index" \
> -	"$dotest/patch" &&
> +    cmd="git apply $git_apply_opt --build-fake-ancestor" &&
> +    cmd="$cmd "'"$dotest/patch-merge-tmp-index" "$dotest/patch"' &&
> +    eval "$cmd" &&

$git_apply_opt can have other stuff in it, too (from my cursory reading,
it looks like --whitespace, --directory, --exclude, -C, --reject,
--ignore-whitespace, and --ignore-space-change).  Those options are now
passed, too.

Naively, I don't think it should be a problem. Many of them will do
nothing (because the patch _should_ apply cleanly to the blobs it
mentions). Some seem like an obvious improvement (e.g., "--directory"
should be just as necessary as "-p", I would think). For something like
"--whitespace=error", I would think we would have errored out already
when we first tried to apply the patch. Or maybe not. I didn't test.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] am -3: allow nonstandard -p<num> option
From: Junio C Hamano @ 2012-02-29  3:36 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20120229025842.GA3585@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Feb 28, 2012 at 03:24:54PM -0800, Junio C Hamano wrote:
>
>> When falling back to 3-way merge, we run "git apply" to synthesize the
>> fake ancestor tree by parsing the incoming patch, and another "git apply"
>> to apply the patch to the fake ancestor tree.  Both invocation need to
>> be aware of the custom -p<num> setting to parse patches that were prepared
>> with non-standard src/dst prefix.
>
> Makes sense. One question:
> ...
> $git_apply_opt can have other stuff in it, too (from my cursory reading,
> it looks like --whitespace, --directory, --exclude, -C, --reject,
> --ignore-whitespace, and --ignore-space-change).  Those options are now
> passed, too.
>
> Naively, I don't think it should be a problem. Many of them will do
> nothing (because the patch _should_ apply cleanly to the blobs it
> mentions). Some seem like an obvious improvement (e.g., "--directory"
> should be just as necessary as "-p", I would think). For something like
> "--whitespace=error", I would think we would have errored out already
> when we first tried to apply the patch. Or maybe not. I didn't test.

An honest answer is that I didn't think deeply if they matter ;-).

Certainly we would want to honor the original settings for whitespace
errors by propagating the option, so that we would reject or adjust when
synthesizing the fake ancestor tree the same way as we deal with them when
apply the patch for real.

^ permalink raw reply

* Re: [PATCH v7 02/10] Stop starting pager recursively
From: Junio C Hamano @ 2012-02-29  3:37 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, Ramsay Jones
In-Reply-To: <CACsJy8DJM5hwWnTs2rkhCa7a=vKRRsLMjZGBotmDDpAaGG+Sxw@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> It does improve that situation, or at least it stops git from forking
> frantically.

That "at least" sounds suspicious ;-) Do you mean that there is other
breakages in that mode of operation?

^ permalink raw reply

* Re: filtering out mode-change-only changes
From: Junio C Hamano @ 2012-02-29  3:40 UTC (permalink / raw)
  To: Neal Kreitzinger; +Cc: git
In-Reply-To: <jik2le$2lb$1@dough.gmane.org>

"Neal Kreitzinger" <neal@rsss.com> writes:

> What is the best way to filter out the "mode change only" entries from a 
> "name-status diff result" listing of changed files?
>
> Reason-for-this:
> get a list of files whose content changed and feed that list into a 
> gui-diff-tool for visual review of "merge" (rebase) results.

Later I have some words on this.

> ... I'd like to see if there is some way that git 
> already-does-this-for-you, or if someone already has a script that does 
> this.

I do not know about random scripts people write, but there is nothing
built-in.

But *if* the _real_ reason you want to do this is because you do not want
to see unnecessary mode changes caused by your filesystem that screws up
file modes for whatever breakage, perhaps setting core.filemode to false
so that mode changes made to the working tree files by your mode breaking
filesystem might be the real solution.

Of course I do not know if that is the real reason you are asking for that
or not.

^ permalink raw reply

* Re: [PATCH v7 02/10] Stop starting pager recursively
From: Nguyen Thai Ngoc Duy @ 2012-02-29  3:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Ramsay Jones
In-Reply-To: <7vr4xewbrz.fsf@alter.siamese.dyndns.org>

On Wed, Feb 29, 2012 at 10:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> It does improve that situation, or at least it stops git from forking
>> frantically.
>
> That "at least" sounds suspicious ;-) Do you mean that there is other
> breakages in that mode of operation?

Well, "shortlog -p" did not run $PAGER (i.e. less). I haven't had time
to dig further.
-- 
Duy

^ permalink raw reply

* Re: filtering out mode-change-only changes
From: Junio C Hamano @ 2012-02-29  3:52 UTC (permalink / raw)
  To: Neal Kreitzinger; +Cc: git
In-Reply-To: <7vmx82wbmr.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> "Neal Kreitzinger" <neal@rsss.com> writes:
>
>> What is the best way to filter out the "mode change only" entries from a 
>> "name-status diff result" listing of changed files?
>> ...
> I do not know about random scripts people write, but there is nothing
> built-in.

Having said that, if we _were_ to do this built-in, an obvious logical
place to do so is to define a new DIFF_OPT_IGNORE_EXECUTABLE_BIT, teach
"--ignore-executable-bit" command line option to diff_opt_parse(), and
then teach diff_resolve_rename_copy() to consider this bit when the code
originally set DIFF_STATUS_MODIFIED.  Instead, the updated code that is
working under --ignore-executable-bit option would drop such a filepair
from diff_queued_diff.

I do not know if such a change is worth doing, though.  It depends on the
real reason why do you have so many "mode change only" changes that would
make rebasing or cherry-picking too troublesome.

^ permalink raw reply


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