threads / discuss / 26952

What's cooking in git.git (Mar 2011, #06; Thu, 31)

Subject: What's cooking in git.git (Mar 2011, #06; Thu, 31)

## tl;dr

13 messages between Mar 31, 2011 and Jun 13, 2011.

replies: 12people: 4as markdown or json

Junio C Hamano· Mar 31, 2011, 22:26 UTC · lore

Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' while commits prefixed with '+' are in 'next'.

It's been two months since v1.7.4 and -rc0 was way overdue. I'll do a -rc1 sometime next week, hoping that we can finish this cycle by mid April.

Which means that from now on, we wouldn't be interested in topics that are mostly internal clean-ups, new features that is not already in 'master', nor fixes for nontrivial bugs that are not regressions. A bug that existed since v1.7.0 is something we have lived with long enough that it is not worth the risk of introducing other new bugs by trying to fix it before 1.7.5 final).

What we would want to see are regression fixes. There are quite a few new topics that have already graduated in 'master', or have been cooking and should be in 'master' shortly. If they introduce new bugs that didn't exist in v1.7.4, they need to be squashed before anything else.

The criteria is a bit looser for documentation updates. Simple grammar and spelling fixes are usually safe; changes in the structure and markups are riskier.

In any case, please use your best judgement and help polishing v1.7.5 release.

-------------------------------------------------- [New Topics]

* jh/notes-add-ui (2011-03-30) 1 commit
 - Make "git notes add" more user-friendly when there are existing notes
* jk/maint-remote-mirror-safer (2011-03-30) 3 commits
 - remote: deprecate --mirror
 - remote: separate the concept of push and fetch mirrors
 - remote: disallow some nonsensical option combinations
* jk/notes-ui-updates (2011-03-30) 7 commits
 - log/pretty-options: Document --[no-]notes and deprecate old notes options
 - revision.c: make --no-notes reset --notes list
 - revision.c: support --notes command-line option
 - notes: refactor display notes default handling
 - notes: refactor display notes extra refs field
 - revision.c: refactor notes ref expansion
 - notes: make expand_notes_ref globally accessible
* jr/grep-en-config (2011-03-30) 1 commit
  (merged to 'next' on 2011-03-31 at 2a73028)
 + grep: allow -E and -n to be turned on by default via configuration

This is low impact, isolated, and has no risk of breaking the system as a whole. May merge by rc1.

* nd/maint-setup (2011-03-26) 2 commits
  (merged to 'next' on 2011-03-31 at 2c36f6a)
 + Kill off get_relative_cwd()
 + setup: return correct prefix if worktree is '/'

This benefits only the minority who use /.git at the root level of the filesystem, but the changed code is used from many codepaths; will not merge before 1.7.5.

-------------------------------------------------- [Stalled]

* jc/diff-irreversible-delete (2011-02-28) 1 commit
 - git diff -D: omit the preimage of deletes

"diff -B -D" should omit the deleting half of a broken pair from the output. This is low impact, isolated, and has no risk of breaking the system as a whole, but the topic needs documentation and tests.

* gr/cvsimport-alternative-cvspass-location (2011-02-18) 1 commit
 - Look for password in both CVS and CVSNT password files.

It seems that we need separate parsers for these two formats in order not to regress the users of the original cvs.

* jc/index-pack (2011-02-25) 5 commits
 - index-pack --verify: read anomalous offsets from v2 idx file
 - write_idx_file: need_large_offset() helper function
 - index-pack: --verify
 - write_idx_file: introduce a struct to hold idx customization options
 - index-pack: group the delta-base array entries also by type

Still a WIP, and will not be ready for 1.7.5. Need to put histogram output into index-pack --verify to really kill verify-pack.

* jk/tag-contains (2010-07-05) 4 commits
 - Why is "git tag --contains" so slow?
 - default core.clockskew variable to one day
 - limit "contains" traversals based on commit timestamp
 - tag: speed up --contains calculation

The idea of the bottom one is probably Ok, except that the use of object flags needs to be rethought, or at least the helper needs to be moved to builtin/tag.c to make it clear that it should not be used outside the current usage context.

* jk/edit-notes-in-commit-log (2011-03-07) 2 commits
 - [wip] commit: allow editing notes in commit message editor
 - notes: make expand_notes_ref globally accessible
* mg/grep-full-tree (2011-03-01) 2 commits
 - grep: make --full-tree work with pathspecs
 - grep: --full-tree

Do not merge; it would be preferable to use ":/" or whatever magic pathspec that is relative to the root of the working tree.

-------------------------------------------------- [Cooking]

* mz/rebase (2011-02-28) 34 commits
  (merged to 'next' on 2011-03-31 at 3b1343c)
 + rebase: define options in OPTIONS_SPEC
  (merged to 'next' on 2011-02-25 at 52caa7a)
 + Makefile: do not install sourced rebase scripts
  (merged to 'next' on 2011-02-22 at 3219155)
 + rebase: use @{upstream} if no upstream specified
 + rebase -i: remove unnecessary state rebase-root
 + rebase -i: don't read unused variable preserve_merges
 + git-rebase--am: remove unnecessary --3way option
 + rebase -m: don't print exit code 2 when merge fails
 + rebase -m: remember allow_rerere_autoupdate option
 + rebase: remember strategy and strategy options
 + rebase: remember verbose option
 + rebase: extract code for writing basic state
 + rebase: factor out sub command handling
 + rebase: make -v a tiny bit more verbose
 + rebase -i: align variable names
 + rebase: show consistent conflict resolution hint
 + rebase: extract am code to new source file
 + rebase: extract merge code to new source file
 + rebase: remove $branch as synonym for $orig_head
 + rebase -i: support --stat
 + rebase: factor out call to pre-rebase hook
 + rebase: factor out clean work tree check
 + rebase: factor out reference parsing
 + rebase: reorder validation steps
 + rebase -i: remove now unnecessary directory checks
 + rebase: factor out command line option processing
 + rebase: align variable content
 + rebase: align variable names
 + rebase: stricter check of standalone sub command
 + rebase: act on command line outside parsing loop
 + rebase: improve detection of rebase in progress
 + rebase: remove unused rebase state 'prev_head'
 + rebase: read state outside loop
 + rebase: refactor reading of state
 + rebase: clearer names for directory variables

I wanted to wait for an independent Ack or two for the tip one, which was a response to regression concerns raised by J6t, but ended up merging it after giving another look. Will not merge before 1.7.5, as there is no user visible improvements up to this point.

* jc/merge-sans-branch (2011-03-23) 2 commits
  (merged to 'next' on 2011-03-31 at 754a6af)
 + merge: merge with the default upstream branch without argument
 + merge: match the help text with the documentation

Allow running "git merge" without telling it what to merge. It will merge with the "upstream" of the current branch if configured. This is low impact, isolated, and has no risk of major regression. May merge before rc1, but it is Ok to wait.

* jh/gitweb-localtime (2011-03-23) 1 commit
 - gitweb: javascript ability to adjust time based on timezone
* jk/maint-merge-rename-create (2011-03-25) 3 commits
  (merged to 'next' on 2011-03-31 at b9bc9f1)
 + merge: turn on rewrite detection
 + merge: handle renames with replacement content
 + t3030: fix accidental success in symlink rename
May merge before rc1, but it is Ok to wait.
* jk/pull-into-empty (2011-03-25) 2 commits
  (merged to 'next' on 2011-03-31 at d4dd598)
 + pull: do not clobber untracked files on initial pull
 + merge: merge unborn index before setting ref

This is low impact, isolated, and has no risk of major regression. Will merge before rc1.

* mz/maint-rename-unmerged (2011-03-23) 1 commit
  (merged to 'next' on 2011-03-31 at c7b3d9a)
 + diffcore-rename: don't consider unmerged path as source
Will cook until 1.7.5 final.
* nd/struct-pathspec (2011-03-25) 4 commits
  (merged to 'next' on 2011-03-31 at 66cbb7d)
 + Improve tree_entry_interesting() handling code
 + Convert read_tree{,_recursive} to support struct pathspec
 + Reimplement read_tree_recursive() using tree_entry_interesting()
 + Merge branch 'en/object-list-with-pathspec' into 'nd/struct-pathspec'
Will cook until 1.7.5 final.
* jc/add-u-migration (2011-03-22) 3 commits
 - add: make "add -u/-A" update full tree without pathspec (step 3)
 - add: make "add -u/-A" update full tree without pathspec (step 2)
  (merged to 'next' on 2011-03-31 at 962e058)
 + add: make "add -u/-A" update full tree without pathspec

The bottom one is a necessary first step toward the UI clean-up planned for 1.8.0 which we discussed in length in the earlier part of the cycle; the change is low impact, isolated, and has no risk of breaking the system as a whole, but I would wait until the ":/" magic pathspec materializes, as the advice message would have to become different, and the way to get more stable semantics will become more direct.

* jk/progress-with-pager (2011-03-24) 4 commits
 - diff: turn on rename detection progress reporting
 - show: turn on rename detection progress reporting
 - progress: use pager's original_stderr if available
 - pager: save the original stderr when redirecting to pager
Will cook until 1.7.5 final.
* sb/sparse-more (2011-03-21) 1 commit
  (merged to 'next' on 2011-03-23 at 4bec1d1)
 + Makefile: Cover more files with make check
Will merge.
* jc/rename-degrade-cc-to-c (2011-01-06) 4 commits
  (merged to 'next' on 2011-03-31 at 8d685d7)
 + diffcore-rename: fall back to -C when -C -C busts the rename limit
 + diffcore-rename: record filepair for rename src
 + diffcore-rename: refactor "too many candidates" logic
 + builtin/diff.c: remove duplicated call to diff_result_code()
Will hold.
* cn/system-path-tweak (2011-03-17) 1 commit
 - system_path: use a static buffer
* en/merge-recursive (2011-03-17) 4 commits
  (merged to 'next' on 2011-03-18 at a32016b)
 + merge-recursive: tweak magic band-aid
  (merged to 'next' on 2011-03-09 at 3762932)
 + merge-recursive: When we detect we can skip an update, actually skip it
 + t6022: New test checking for unnecessary updates of files in D/F conflicts
 + t6022: New test checking for unnecessary updates of renamed+modified files
I am not happy with these magic band aids.  Will hold.
* nd/init-gitdir (2011-03-19) 2 commits
  (merged to 'next' on 2011-03-31 at 3b8fb40)
 + init, clone: support --separate-git-dir for .git file
 + git-init.txt: move description section up
Will merge.
* jl/submodule-fetch-on-demand (2011-03-06) 7 commits
  (merged to 'next' on 2011-03-20 at a5e452d)
 + fetch/pull: Describe --recurse-submodule restrictions in the BUGS section
 + submodule update: Don't fetch when the submodule commit is already present
 + fetch/pull: Don't recurse into a submodule when commits are already present
 + Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule' option
 + config: teach the fetch.recurseSubmodules option the 'on-demand' value
 + fetch/pull: Add the 'on-demand' value to the --recurse-submodules option
 + fetch/pull: recurse into submodules when necessary
Will merge.
* ab/i18n-st (2011-02-22) 69 commits
  (merged to 'next' on 2011-03-23 at e2732e2)
 + i18n: git-shortlog basic messages
 + i18n: git-revert split up "could not revert/apply" message
 + i18n: git-revert literal "me" messages
 + i18n: git-revert "Your local changes" message
 + i18n: git-revert basic messages
 + i18n: git-notes GIT_NOTES_REWRITE_MODE error message
 + i18n: git-notes basic commands
 + i18n: git-gc "Auto packing the repository" message
 + i18n: git-gc basic messages
 + i18n: git-describe basic messages
 + i18n: git-clean clean.requireForce messages
 + i18n: git-clean basic messages
 + i18n: git-bundle basic messages
 + i18n: git-archive basic messages
 + i18n: git-status "renamed: " message
 + i18n: git-status "Initial commit" message
 + i18n: git-status "Changes to be committed" message
 + i18n: git-status shortstatus messages
 + i18n: git-status "nothing to commit" messages
 + i18n: git-status basic messages
 + i18n: git-push "prevent you from losing" message
 + i18n: git-push basic messages
 + i18n: git-tag tag_template message
 + i18n: git-tag basic messages
 + i18n: git-reset "Unstaged changes after reset" message
 + i18n: git-reset reset_type_names messages
 + i18n: git-reset basic messages
 + i18n: git-rm basic messages
 + i18n: git-mv "bad" messages
 + i18n: git-mv basic messages
 + i18n: git-merge "Wonderful" message
 + i18n: git-merge "You have not concluded your merge" messages
 + i18n: git-merge "Updating %s..%s" message
 + i18n: git-merge basic messages
 + i18n: git-log "--OPT does not make sense" messages
 + i18n: git-log basic messages
 + i18n: git-grep "--open-files-in-pager" message
 + i18n: git-grep basic messages
 + i18n: git-fetch split up "(non-fast-forward)" message
 + i18n: git-fetch update_local_ref messages
 + i18n: git-fetch formatting messages
 + i18n: git-fetch basic messages
 + i18n: git-diff basic messages
 + i18n: git-commit advice messages
 + i18n: git-commit "enter the commit message" message
 + i18n: git-commit print_summary messages
 + i18n: git-commit formatting messages
 + i18n: git-commit "middle of a merge" message
 + i18n: git-commit basic messages
 + i18n: git-checkout "Switched to a .. branch" message
 + i18n: git-checkout "HEAD is now at" message
 + i18n: git-checkout describe_detached_head messages
 + i18n: git-checkout: our/their version message
 + i18n: git-checkout basic messages
 + i18n: git-branch "(no branch)" message
 + i18n: git-branch "git branch -v" messages
 + i18n: git-branch "Deleted branch [...]" message
 + i18n: git-branch "remote branch '%s' not found" message
 + i18n: git-branch basic messages
 + i18n: git-add "Unstaged changes" message
 + i18n: git-add "remove '%s'" message
 + i18n: git-add "did not match any files" message
 + i18n: git-add "The following paths are ignored" message
 + i18n: git-add basic messages
 + i18n: git-clone "Cloning into" message
 + i18n: git-clone "Cloning into" message
 + i18n: git-clone basic messages
 + i18n: git-init "Initialized [...] repository" message
 + i18n: git-init basic messages
Will merge.

-------------------------------------------------- [Discarded]

* jc/diff-dotdot (2011-03-23) 2 commits
 . warn use of "git diff A..B"
 . diff: remove dead code that flips arguments order

This was 1/4 tongue-in-cheek. Now we seem to have a handful of volunteer cluebat bearers, and I wouldn't have to worry about this topic very much.

* jh/merge-sans-branch (2011-02-10) 4 commits
 . merge: add support for merging from upstream by default
 . merge: introduce per-branch-configuration helper function
 . merge: introduce setup_merge_commit helper function
 . merge: update the usage information to be more modern

I've been wanting to move this forward for quite some time but ended up redoing it myself (see jc/merge-sans-branch)

Junio C Hamano· Mar 31, 2011, 22:35 UTC · re: Junio C Hamano · lore

Let's make our cycles shorter

I've been aiming for 6-8 week cycles but the 1.7.5 cycle ended up being way longer than that. I just tagged a -rc0 and it will be mirroring out, and today's "What's cooking" has annotations on topics in flight that I expect to be in the -rc1.

This message is primarily meant to be a reminder to myself and also to clarify my intentions. I'd like one cycle of ours to look roughly like this:

 - 1.7.5 is released.
 - Week 1: post release clean-up.  People are strongly encouraged to give
   the highest priority to the regression fixes for the most recent
   release.
 - Week 2: new features, restructuring, non-regression bugfixes start to
   flow in and graduate in preparation for 1.7.6.  Some may graduate from
   'next' before 1.7.5 to master, some may be newly queued through 'pu' to
   'next'.
 - Week N: 1.7.6-rc0 is tagged.  Examine topics in 'next' that are still
   not in 'master' and decide which way they should go, either included in
   1.7.6-rc1 or wait until the next cycle.
 - Week N+1: 1.7.6-rc1 is tagged with (a subset of) candidate topics we
   decided previous week.
   At this point, people are again strongly encouraged to give the highest
   priority to the regression fixes for the upcoming release.
 - Week N+2: 1.7.6-rc2 is tagged.
 - Week X: 1.7.6 is released.

Historically we have done at least two rc releases, and often three, so I would expect X is at least N+3 but possibly N+4. Since I want to have at most an 8-week cycle, it would mean N=4 or 5, so we have three to four weeks to concentrate the real development for the next release.

We are at "Week N" for this cycle as of today.

This of course does not mean that people are forbidden from working on or discussing anything but regression fixes during the rc and post-release period. It may take longer than a month to stabilize for a large-ish topic to be properly reviewed, discussed and guinea-pigged in 'next'.

So during Weeks N thru X, there may appear new topics in flight and I may end up queueing them in 'pu' or even move some of them to 'next', with an understanding that they will not be part of the current cycle, but are queued merely to make it easier for people interested in the new topic to try out and discuss ideas for the next cycle. Also handling these new topics during the rc period will receive much lower priority and time from me.

I hope all the above sound sensible.  Thanks.
Sebastien Douche· Apr 25, 2011, 00:34 UTC · re: Junio C Hamano · lore

Re: Let's make our cycles shorter

On Fri, Apr 1, 2011 at 00:35, Junio C Hamano <gitster@pobox.com> wrote:

Hi Junio, I was surprised to not read response, it's having sensitive impact on the project. Junio, it's effective now?

-- 
Sebastien Douche <sdouche@gmail.com>
Twitter: @sdouche (agile, lean, python, git, open source)
Junio C Hamano· Apr 25, 2011, 17:03 UTC · re: Sebastien Douche · lore

Re: Let's make our cycles shorter

Sebastien Douche <sdouche@gmail.com> writes:
> On Fri, Apr 1, 2011 at 00:35, Junio C Hamano <gitster@pobox.com> wrote:
> ...
> I was surprised to not read response, it's having sensitive impact on
> the project.
Hmm, what sensitive impact on which project do you have in mind?
> Junio, it's effective now?

The structure of clean-up, development, freeze and then release has always been in effect in this project, but historically the duration of the development stretch varied a lot from cycle to cycle.

I just spelled the structure out for the next cycle, and tried to give some predictable bounds to that elastic development stretch, in order to force myself to stick to a schedule in which we can make measurable progress in reasonable amount of time.

It does not mean that we won't be tackling issues that will take more than N weeks to perfect. Either a topic gets polished enough in a single cycle to graduate to 'master' before -rc0, or it keeps cooking in 'next' during the feature freeze, and will attempt to be in the release after that.

I've tentatively set the following dates on my calendar, based on 9-week cycle:

 - Today is the beginning of week #1 for this cycle.
 - The entire month of May 2011 will be the development stretch (lasting
   up to Week #5 that ends May 29th).
 - Aim to tag 1.7.6-rc0 on June 1st, 2011, -rc1 on 8th, -rc2 on 15th.
 - Either tag 1.7.6 final on 19th or have -rc3 on 22nd and final on 26th
   of June.
If you happen to use Google Calendar, you can paste:
    jfgbl2mrlipp4pb6ieih0qr3so@group.calendar.google.com

in the "Other calendars" box (where a gray "Add a friend's calendar" appears), but you won't be missing much even if you don't (I only have week numbers and the target tagging dates, nothing more interesting than that).

Sebastien Douche· Jun 13, 2011, 00:45 UTC · re: Junio C Hamano · lore

Re: Let's make our cycles shorter

On Mon, Apr 25, 2011 at 19:03, Junio C Hamano <gitster@pobox.com> wrote:
>> I was surprised to not read response, it's having sensitive impact on
>> the project.
>
> Hmm, what sensitive impact on which project do you have in mind?

Short cycle means less time to write code for the next release, the "frame" to add feature is reduced. I'm surely wrong, Git is a stable project and well managed (with 4 branches: unstable, integration, next stable & stable).

> I just spelled the structure out for the next cycle, and tried to give
> some predictable bounds to that elastic development stretch, in order to
> force myself to stick to a schedule in which we can make measurable
> progress in reasonable amount of time.
Cool :).
-- 
Sebastien Douche <sdouche@gmail.com>
Twitter : @sdouche
Michael J Gruber· Apr 1, 2011, 13:31 UTC · re: Junio C Hamano · lore

[PATCH] git diff -D: omit the preimage of deletes

Introduce the -D/--irreversible-delete option which omits the diff for total deletes. It is similar to -M,-C in its output but irreversible in the sense that the resulting patch can not be reversed (-R).

When used in connection with -B, omit the diff of the deletion part of a complete rewrite.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 Documentation/diff-options.txt |   10 +++++++++
 diff.c                         |   14 +++++++++---
 diff.h                         |    1 +
 t/t4022-diff-rewrite.sh        |   43 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index c93124b..4760d7a 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -259,6 +259,16 @@ endif::git-log[]
 	projects, so use it with caution.  Giving more than one
 	`-C` option has the same effect.
 
+-D::
+--irreversible-delete::
+	Omit the preimage for deletes, i.e. print only the header but not
+	the diff between the preimage and `/dev/null`. The resulting patch
+	is irreversible in the sense that it can not be applied in reverse
+	(-R).
++
+When used together with `-B`, omit also the preimage in the deletion part
+of a delete/create pair.
+
 -l<num>::
 	The `-M` and `-C` options require O(n^2) processing time where n
 	is the number of potential rename/copy targets.  This
diff --git a/diff.c b/diff.c
index 5422c43..9ea1de1 100644
--- a/diff.c
+++ b/diff.c
@@ -1943,7 +1943,11 @@ static void builtin_diff(const char *name_a,
 		}
 	}
 
-	if (!DIFF_OPT_TST(o, TEXT) &&
+	if (o->irreversible_delete && lbl[1][0] == '/') {
+		fprintf(o->file, "%s", header.buf);
+		strbuf_reset(&header);
+		goto free_ab_and_return;
+	} else if (!DIFF_OPT_TST(o, TEXT) &&
 	    ( (!textconv_one && diff_filespec_is_binary(one)) ||
 	      (!textconv_two && diff_filespec_is_binary(two)) )) {
 		if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
@@ -1963,8 +1967,7 @@ static void builtin_diff(const char *name_a,
 			fprintf(o->file, "%sBinary files %s and %s differ\n",
 				line_prefix, lbl[0], lbl[1]);
 		o->found_changes = 1;
-	}
-	else {
+	} else {
 		/* Crazy xdl interfaces.. */
 		const char *diffopts = getenv("GIT_DIFF_OPTS");
 		xpparam_t xpp;
@@ -3160,6 +3163,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 			return error("invalid argument to -M: %s", arg+2);
 		options->detect_rename = DIFF_DETECT_RENAME;
 	}
+	else if (!strcmp(arg, "-D") || !strcmp(arg, "--irreversible-delete")) {
+		options->irreversible_delete = 1;
+	}
 	else if (!prefixcmp(arg, "-C") || !prefixcmp(arg, "--find-copies=") ||
 		 !strcmp(arg, "--find-copies")) {
 		if (options->detect_rename == DIFF_DETECT_COPY)
@@ -4205,7 +4211,7 @@ void diffcore_std(struct diff_options *options)
 			diffcore_break(options->break_opt);
 		if (options->detect_rename)
 			diffcore_rename(options);
-		if (options->break_opt != -1)
+		if (options->break_opt != -1 && !options->irreversible_delete)
 			diffcore_merge_broken();
 	}
 	if (options->pickaxe)
diff --git a/diff.h b/diff.h
index 310bd6b..11d13cf 100644
--- a/diff.h
+++ b/diff.h
@@ -104,6 +104,7 @@ struct diff_options {
 	int interhunkcontext;
 	int break_opt;
 	int detect_rename;
+	int irreversible_delete;
 	int skip_stat_unmatch;
 	int line_termination;
 	int output_format;
diff --git a/t/t4022-diff-rewrite.sh b/t/t4022-diff-rewrite.sh
index 2a537a2..c00a94b 100755
--- a/t/t4022-diff-rewrite.sh
+++ b/t/t4022-diff-rewrite.sh
@@ -11,7 +11,9 @@ test_expect_success setup '
 	tr \
 	  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" \
 	  "nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM" \
-	  <"$TEST_DIRECTORY"/../COPYING >test
+	  <"$TEST_DIRECTORY"/../COPYING >test &&
+	echo "to be deleted" >test2 &&
+	git add test2
 
 '
 
@@ -25,5 +27,44 @@ test_expect_success 'detect rewrite' '
 
 '
 
+cat >expect <<EOF
+diff --git a/test2 b/test2
+deleted file mode 100644
+index 4202011..0000000
+--- a/test2
++++ /dev/null
+@@ -1 +0,0 @@
+-to be deleted
+EOF
+test_expect_success 'show deletion diff without -D' '
+
+	rm test2 &&
+	git diff -- test2 >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<EOF
+diff --git a/test2 b/test2
+deleted file mode 100644
+index 4202011..0000000
+EOF
+test_expect_success 'suppress deletion diff with -D' '
+
+	git diff -D -- test2 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'show deletion diff with -B' '
+
+	git diff -B -- test >actual &&
+	grep "Linus Torvalds" actual
+'
+
+test_expect_success 'suppress deletion diff with -B -D' '
+
+	git diff -B -D -- test >actual &&
+	grep -v "Linus Torvalds" actual
+'
+
 test_done
 
-- 
1.7.4.2.668.gba03a4
Junio C Hamano· Apr 1, 2011, 19:26 UTC · re: Michael J Gruber · lore

Re: [PATCH] git diff -D: omit the preimage of deletes

Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 13 quoted lines
> diff --git a/diff.c b/diff.c
> index 5422c43..9ea1de1 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4205,7 +4211,7 @@ void diffcore_std(struct diff_options *options)
>  			diffcore_break(options->break_opt);
>  		if (options->detect_rename)
>  			diffcore_rename(options);
> -		if (options->break_opt != -1)
> +		if (options->break_opt != -1 && !options->irreversible_delete)
>  			diffcore_merge_broken();
>  	}
>  	if (options->pickaxe)
Thanks, but this hunk looks fishy.

What happens to a path that was tentatively broken for the purpose of rename detection with -B -M (break to match with another file) but then found to be with no counterpart after all after running diffcore_rename(), which now needs to get merged back? Such a path is shown as a normal patch when the dissimlarity between the preimage and postimage is not large enough and merge-broken is the step that combines such a broken but unmatched pair back.

I would have expected that the patch relative to jc/diff-irreversible-delete topic would consist only of changes to diff.c:emit_rewrite_diff(), docs and tests.

Junio C Hamano· Apr 3, 2011, 06:23 UTC · re: Junio C Hamano · lore

Re: [PATCH] git diff -D: omit the preimage of deletes

Junio C Hamano <gitster@pobox.com> writes:
> I would have expected that the patch relative to jc/diff-irreversible-delete
> topic would consist only of changes to diff.c:emit_rewrite_diff(), docs
> and tests.

Here is an "in other words" follow-up. Your tests looked reasonable (and pass with this patch on top of what has been queued in 'pu').

 diff.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index 5c66a53..05f443c 100644
--- a/diff.c
+++ b/diff.c
@@ -572,11 +572,14 @@ static void emit_rewrite_diff(const char *name_a,
 		line_prefix, metainfo, a_name.buf, name_a_tab, reset,
 		line_prefix, metainfo, b_name.buf, name_b_tab, reset,
 		line_prefix, fraginfo);
-	print_line_count(o->file, lc_a);
+	if (!o->irreversible_delete)
+		print_line_count(o->file, lc_a);
+	else
+		fprintf(o->file, "?,?");
 	fprintf(o->file, " +");
 	print_line_count(o->file, lc_b);
 	fprintf(o->file, " @@%s\n", reset);
-	if (lc_a)
+	if (lc_a && !o->irreversible_delete)
 		emit_rewrite_lines(&ecbdata, '-', data_one, size_one);
 	if (lc_b)
 		emit_rewrite_lines(&ecbdata, '+', data_two, size_two);
Junio C Hamano· Apr 3, 2011, 06:38 UTC · re: Junio C Hamano · lore

Re: [PATCH] git diff -D: omit the preimage of deletes

Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> I would have expected that the patch relative to jc/diff-irreversible-delete
>> topic would consist only of changes to diff.c:emit_rewrite_diff(), docs
>> and tests.
>
> Here is an "in other words" follow-up.  Your tests looked reasonable (and
> pass with this patch on top of what has been queued in 'pu').

And this is the documentation part, based on your version but somewhat rewritten. Your version said "cannot be applied with -R", but at the mechanical application level, the format is deliberately designed to make `patch` and `git apply` to fail, and I think that should be mentioned together with the reason why such an option exists (i.e. for human eyeball consumption).

I'll squash these two to what is queued in 'pu'. We may want to polish it again after 1.7.5 but I think it is in much better shape now.

Thanks.
 Documentation/diff-options.txt |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index c93124b..30a00d3 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -259,6 +259,19 @@ endif::git-log[]
 	projects, so use it with caution.  Giving more than one
 	`-C` option has the same effect.
 
+-D::
+--irreversible-delete::
+	Omit the preimage for deletes, i.e. print only the header but not
+	the diff between the preimage and `/dev/null`. The resulting patch
+	is not meant to be applied with `patch` nor `git apply`; this is
+	solely for people who want to just concentrate on reviewing the
+	text after the change. In addition, the output obviously lack
+	enough information to apply such a patch in reverse, even manually,
+	hence the name of the option.
++
+When used together with `-B`, omit also the preimage in the deletion part
+of a delete/create pair.
+
 -l<num>::
 	The `-M` and `-C` options require O(n^2) processing time where n
 	is the number of potential rename/copy targets.  This
Michael J Gruber· Apr 3, 2011, 12:51 UTC · re: Junio C Hamano · lore

Re: [PATCH] git diff -D: omit the preimage of deletes

Junio C Hamano venit, vidit, dixit 01.04.2011 21:26:
Show 30 quoted lines
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> diff --git a/diff.c b/diff.c
>> index 5422c43..9ea1de1 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -4205,7 +4211,7 @@ void diffcore_std(struct diff_options *options)
>>  			diffcore_break(options->break_opt);
>>  		if (options->detect_rename)
>>  			diffcore_rename(options);
>> -		if (options->break_opt != -1)
>> +		if (options->break_opt != -1 && !options->irreversible_delete)
>>  			diffcore_merge_broken();
>>  	}
>>  	if (options->pickaxe)
> 
> Thanks, but this hunk looks fishy.
> 
> What happens to a path that was tentatively broken for the purpose of
> rename detection with -B -M (break to match with another file) but then
> found to be with no counterpart after all after running diffcore_rename(),
> which now needs to get merged back?  Such a path is shown as a normal
> patch when the dissimlarity between the preimage and postimage is not
> large enough and merge-broken is the step that combines such a broken but
> unmatched pair back.
> 
> I would have expected that the patch relative to jc/diff-irreversible-delete
> topic would consist only of changes to diff.c:emit_rewrite_diff(), docs
> and tests.
> 

I think I misunderstood what you intended "-B -D" to do (and I even didn't know about -B until -D came up; my understanding of "-B" is still fishy). I just didn't want to let this die before 1.7.5. Thanks for taking this up and clarifying it.

Michael
Jeff King· Apr 1, 2011, 15:26 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2011, #06; Thu, 31)

On Thu, Mar 31, 2011 at 03:26:31PM -0700, Junio C Hamano wrote:
Show 13 quoted lines
> * jk/maint-remote-mirror-safer (2011-03-30) 3 commits
>  - remote: deprecate --mirror
>  - remote: separate the concept of push and fetch mirrors
>  - remote: disallow some nonsensical option combinations
> 
> * jk/notes-ui-updates (2011-03-30) 7 commits
>  - log/pretty-options: Document --[no-]notes and deprecate old notes options
>  - revision.c: make --no-notes reset --notes list
>  - revision.c: support --notes command-line option
>  - notes: refactor display notes default handling
>  - notes: refactor display notes extra refs field
>  - revision.c: refactor notes ref expansion
>  - notes: make expand_notes_ref globally accessible

Both probably post-1.7.5 material. I am of course tempted to get new options in place ASAP (since the sooner they are in, the sooner script writers can say "they are in long enough" and start using them). But I don't think either is critical.

> * jk/edit-notes-in-commit-log (2011-03-07) 2 commits
>  - [wip] commit: allow editing notes in commit message editor
>  - notes: make expand_notes_ref globally accessible

You may want to drop this for now. The bottom one is in jk/notes-ui-updates, which hopefully will go to master soon after 1.7.5, and the top one is going to be rewritten.

Show 7 quoted lines
> * jk/maint-merge-rename-create (2011-03-25) 3 commits
>   (merged to 'next' on 2011-03-31 at b9bc9f1)
>  + merge: turn on rewrite detection
>  + merge: handle renames with replacement content
>  + t3030: fix accidental success in symlink rename
> 
> May merge before rc1, but it is Ok to wait.

I would hold off on this. I _think_ it's fine, but I would really prefer for it to get more exposure in 'next' to shake out any bugs.

Show 7 quoted lines
> * jk/pull-into-empty (2011-03-25) 2 commits
>   (merged to 'next' on 2011-03-31 at d4dd598)
>  + pull: do not clobber untracked files on initial pull
>  + merge: merge unborn index before setting ref
> 
> This is low impact, isolated, and has no risk of major regression. Will
> merge before rc1.
Agreed.
Show 12 quoted lines
> * jc/add-u-migration (2011-03-22) 3 commits
>  - add: make "add -u/-A" update full tree without pathspec (step 3)
>  - add: make "add -u/-A" update full tree without pathspec (step 2)
>   (merged to 'next' on 2011-03-31 at 962e058)
>  + add: make "add -u/-A" update full tree without pathspec
> 
> The bottom one is a necessary first step toward the UI clean-up planned
> for 1.8.0 which we discussed in length in the earlier part of the cycle;
> the change is low impact, isolated, and has no risk of breaking the system
> as a whole, but I would wait until the ":/" magic pathspec materializes,
> as the advice message would have to become different, and the way to get
> more stable semantics will become more direct.

I have been meaning to look closer at this. Were you wanting to get the first stage of the transition into 1.7.5?

Show 7 quoted lines
> * jk/progress-with-pager (2011-03-24) 4 commits
>  - diff: turn on rename detection progress reporting
>  - show: turn on rename detection progress reporting
>  - progress: use pager's original_stderr if available
>  - pager: save the original stderr when redirecting to pager
> 
> Will cook until 1.7.5 final.

I'm not sure if this whole thing should be scrapped. There are potential problems with starting a pager that wants to grab the whole screen (i.e., not less). Maybe it would be enough to have a pager.noprogress option for people who use such a pager.

-Peff
Junio C Hamano· Apr 1, 2011, 17:01 UTC · re: Jeff King · lore

Re: What's cooking in git.git (Mar 2011, #06; Thu, 31)

Jeff King <peff@peff.net> writes:
Show 19 quoted lines
> On Thu, Mar 31, 2011 at 03:26:31PM -0700, Junio C Hamano wrote:
>
> (... parts that I do not disagree with are omitted ...)
>
>> * jc/add-u-migration (2011-03-22) 3 commits
>>  - add: make "add -u/-A" update full tree without pathspec (step 3)
>>  - add: make "add -u/-A" update full tree without pathspec (step 2)
>>   (merged to 'next' on 2011-03-31 at 962e058)
>>  + add: make "add -u/-A" update full tree without pathspec
>> 
>> The bottom one is a necessary first step toward the UI clean-up planned
>> for 1.8.0 which we discussed in length in the earlier part of the cycle;
>> the change is low impact, isolated, and has no risk of breaking the system
>> as a whole, but I would wait until the ":/" magic pathspec materializes,
>> as the advice message would have to become different, and the way to get
>> more stable semantics will become more direct.
>
> I have been meaning to look closer at this. Were you wanting to get the
> first stage of the transition into 1.7.5?

I was tempted to but I think it would be far more pleasant if the first step were to add the warning against "add -u" without pathspec that is ran from a subdirectory to advise "if you meant 'from here', say '.', if you meant 'everywhere', say ':/'---for now we pretend you said '.' to match the traditional behaviour."

It is adding even more confusion to add the "in this repository, 'add -u' is tree-wide" configuration variable without giving people who need to override that in unfamiliar repositories (read: scripts).

Right now, we don't have a good advice to force the tree-wide behaviour other than "cd $(git rev-parse --show-cdup)/. && git add -u", which is quite a mouthful.

We know how the magic "this pathspec is from the root" should work, and I think we even saw "should look like this" patches, but haven't applied to any branch so far yet.

Show 12 quoted lines
>> * jk/progress-with-pager (2011-03-24) 4 commits
>>  - diff: turn on rename detection progress reporting
>>  - show: turn on rename detection progress reporting
>>  - progress: use pager's original_stderr if available
>>  - pager: save the original stderr when redirecting to pager
>> 
>> Will cook until 1.7.5 final.
>
> I'm not sure if this whole thing should be scrapped. There are potential
> problems with starting a pager that wants to grab the whole screen
> (i.e., not less). Maybe it would be enough to have a pager.noprogress
> option for people who use such a pager.

Perhaps. With "Will cook until" I only meant "will not graduate until"; I was not even making any prediction after 1.7.5 in the message.

Thanks.
Jeff King· Apr 1, 2011, 17:06 UTC · re: Junio C Hamano · lore

Re: What's cooking in git.git (Mar 2011, #06; Thu, 31)

On Fri, Apr 01, 2011 at 10:01:42AM -0700, Junio C Hamano wrote:
Show 14 quoted lines
> >> * jc/add-u-migration (2011-03-22) 3 commits
> >>  - add: make "add -u/-A" update full tree without pathspec (step 3)
> >>  - add: make "add -u/-A" update full tree without pathspec (step 2)
> >>   (merged to 'next' on 2011-03-31 at 962e058)
> >>  + add: make "add -u/-A" update full tree without pathspec
> [...]
> > I have been meaning to look closer at this. Were you wanting to get the
> > first stage of the transition into 1.7.5?
> 
> I was tempted to but I think it would be far more pleasant if the first
> step were to add the warning against "add -u" without pathspec that is ran
> from a subdirectory to advise "if you meant 'from here', say '.', if you
> meant 'everywhere', say ':/'---for now we pretend you said '.' to match
> the traditional behaviour."
Yes, I think that is definitely the right first step.
Show 11 quoted lines
> It is adding even more confusion to add the "in this repository, 'add -u'
> is tree-wide" configuration variable without giving people who need to
> override that in unfamiliar repositories (read: scripts).
> 
> Right now, we don't have a good advice to force the tree-wide behaviour
> other than "cd $(git rev-parse --show-cdup)/. && git add -u", which is
> quite a mouthful.
> 
> We know how the magic "this pathspec is from the root" should work, and I
> think we even saw "should look like this" patches, but haven't applied to
> any branch so far yet.

That reasoning makes sense. Let's let the :/ patches develop and cook for post-1.7.5, then, and worry about it in the next cycle when we can build on top of them.

-Peff

← back to recent threads