What's cooking in git.git (Sep 2012, #03; Mon, 10)

What's cooking in git.git (Sep 2012, #03; Mon, 10)

[Junio C Hamano]

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

The fifth batch of topics have started graduating to 'master'.

I'm planning to keep this cycle reasonably short and aim for tagging
the result as 1.8.0 at the end of 9th week, on October 21st, after
which I'd disappear for a few weeks.  http://tinyurl.com/gitCal is
where you can always find my rough tagging schedule at.

You can find the changes described here in the integration branches of the
repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

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

* jc/ll-merge-binary-ours (2012-09-08) 2 commits
 - attr: "binary" attribute should choose built-in "binary" merge driver
 - merge: teach -Xours/-Xtheirs to binary ll-merge driver

"git merge -Xtheirs" did not help content-level merge of binary
files; it should just take their version.  Also "*.jpg binary" in
the attributes did not imply they should use the binary ll-merge
driver.

* jc/mailinfo-RE (2012-09-09) 1 commit
 - mailinfo: strip "RE: " prefix

We strip the prefix from "Re: subject" and also from a less common
"re: subject", but left even less common "RE: subject" intact.

* js/compat-mkdir (2012-09-08) 1 commit
 - Document MKDIR_WO_TRAILING_SLASH in Makefile

Finishing touches to recently added wrapper for mkdir() that do not
want to see trailing slashes.

Will merge to 'next'.

* mh/string-list (2012-09-10) 6 commits
 - api-string-list.txt: initialize the string_list the easy way
 - string_list: add a function string_list_longest_prefix()
 - string_list: add a new function, string_list_remove_duplicates()
 - string_list: add a new function, filter_string_list()
 - string_list: add two new functions for splitting strings
 - string_list: add function string_list_append_nodup()
 (this branch is used by mh/fetch-filter-refs.)

--------------------------------------------------
[Graduated to "master"]

* cn/branch-set-upstream-to (2012-08-30) 3 commits
  (merged to 'next' on 2012-08-31 at d550ecd)
 + branch: deprecate --set-upstream and show help if we detect possible mistaken use
 + branch: add --unset-upstream option
 + branch: introduce --set-upstream-to

"git branch --set-upstream origin/master" is a common mistake to
create a local branch 'origin/master' and set it to integrate with
the current branch.  With a plan to deprecate this option, introduce
"git branch (-u|--set-upstream-to) origin/master" that sets the
current branch to integrate with 'origin/master' remote tracking
branch.

* jk/maint-quiet-is-synonym-to-s-in-log (2012-08-28) 1 commit
  (merged to 'next' on 2012-08-31 at 06f6953)
 + log: fix --quiet synonym for -s

We tried to bend backwards to allow "--quiet" to be a synonym as
"-s" when given as e.g. "git show --quiet", but did not quite
succeed.

* mz/cherry-pick-cmdline-order (2012-08-30) 3 commits
  (merged to 'next' on 2012-08-31 at fc8eec4)
 + cherry-pick/revert: respect order of revisions to pick
 + demonstrate broken 'git cherry-pick three one two'
 + teach log --no-walk=unsorted, which avoids sorting

"git cherry-pick A C B" used to replay changes in A and then B and
then C if these three commits had committer timestamps in that
order, which is not what the user who said "A C B" naturally expects.

* ph/credential-gnome-keyring (2012-08-24) 1 commit
  (merged to 'next' on 2012-08-31 at 6f3b1de)
 + contrib: add credential helper for GnomeKeyring
 (this branch is used by ph/credential-refactor.)

The later refactoring of the shared code in the original series may
not be worth the trouble, so it is split into a separate topic that
builds on top of this one, which independently should be useful.

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

* ph/credential-refactor (2012-09-02) 5 commits
 - wincred: port to generic credential helper
 - Merge branch 'ef/win32-cred-helper' into ph/credential-refactor
 - osxkeychain: port to generic credential helper implementation
 - gnome-keyring: port to generic helper implementation
 - contrib: add generic credential helper

Attempts to refactor to share code among OSX keychain, Gnome keyring
and Win32 credential helpers.

* jc/maint-name-rev (2012-09-04) 7 commits
 - describe --contains: use "name-rev --weight"
 - name-rev --weight: tests and documentation
 - name-rev --weight: cache the computed weight in notes
 - name-rev --weight: trivial optimization
 - name-rev: --weight option
 - name_rev: clarify the logic to assign a new tip-name to a commit
 - name-rev: lose unnecessary typedef

"git name-rev" names the given revision based on a ref that can be
reached in the smallest number of steps from the rev, but that is
not useful when the caller wants to know which tag is the oldest one
that contains the rev.  This teaches a new mode to the command that
uses the oldest ref among those which contain the rev.

I am not sure if this is worth it; for one thing, even with the help
from notes-cache, it seems to make the "describe --contains" even
slower. Also the command will be unusably slow for a user who does
not have a write access (hence unable to create or update the
notes-cache).

Needs another round to at least find a better name for the option,
and possibly a cheaper but still better than the current "close to
the tip" heuristics.

* ms/contrib-thunderbird-updates (2012-08-31) 2 commits
 - [SQUASH] minimum fixup
 - Thunderbird: fix appp.sh format problems

Update helper to send out format-patch output using Thunderbird.
Seems to have design regression for silent users.

* as/check-ignore (2012-09-02) 10 commits
 . fixup: decl-after-stmt etc.
 . Add git-check-ignore
 . Provide free_directory() for reclaiming dir_struct memory
 . Extract some useful pathspec handling code from builtin/add.c into a library
 . For each exclude pattern, store information about where it came from
 . dir.c: refactor excluded() and path_excluded()
 . dir.c: refactor excluded_from_list()
 . dir.c: rename cryptic 'which' variable to more consistent name
 . Improve documentation and comments regarding directory traversal API
 . Update directory listing API doc to match code

Will be rerolled.

* jx/test-real-path (2012-08-27) 1 commit
 - test: set the realpath of CWD as TRASH_DIRECTORY

Running tests with the "trash" directory elsewhere with the "--root"
option did not work well if the directory was specified by a symbolic
link pointing at it.

Seems broken as it makes $(pwd) and TRASH_DIRECTORY inconsistent.
Needs rerolling.

* jc/maint-push-refs-all (2012-08-27) 2 commits
 - get_fetch_map(): tighten checks on dest refs
 - [BROKEN] fetch/push: allow refs/*:refs/*

Allows pushing and fetching everything including refs/stash.
This is broken (see the log message there).

* er/doc-fast-import-done (2012-08-22) 1 commit
 - fast-import: document the --done option

Parked in 'pu' in case ESR responds with "Sorry, forgot to sign-off".

* jc/add-delete-default (2012-08-13) 1 commit
 - git add: notice removal of tracked paths by default

"git add dir/" updated modified files and added new files, but does
not notice removed files, which may be "Huh?" to some users.  They
can of course use "git add -A dir/", but why should they?

Resurrected from graveyard, as I thought it was a worthwhile thing
to do in the longer term; waiting for comments.

* tx/relative-in-the-future (2012-08-16) 2 commits
 - date: show relative dates in the future
 - date: refactor the relative date logic from presentation

Not my itch; rewritten an earlier submission by Tom Xue into
somewhat more maintainable form, though it breaks existing i18n.

Anybody interested in fixing it up?  Otherwise may discard.

* tg/index-v5 (2012-08-17) 13 commits
 . p0002-index.sh: add perf test for the index formats
 . update-index.c: rewrite index when index-version is given
 . Write resolve-undo data for index-v5
 . Write index-v5 cache-tree data
 . Write index-v5
 . Read cache-tree in index-v5
 . Read resolve-undo data
 . Read index-v5
 . Make in-memory format aware of stat_crc
 . Add documentation of the index-v5 file format
 . t2104: Don't fail for index versions other than [23]
 . read-cache.c: Re-read index if index file changed
 . Move index v2 specific functions to their own file

A GSoC project.  Was waiting for comments from mentors and
stakeholders, but nothing seems to be happening, other than breakage
fixes on Cygwin.  May discard.

* mz/rebase-range (2012-07-18) 7 commits
 . rebase (without -p): correctly calculate patches to rebase
 . rebase -p: don't request --left-right only to ignore left side
 . rebase -p: use --cherry-mark for todo file
 . git-rebase--interactive.sh: look up subject in add_pick_line
 . git-rebase--interactive: group all $preserve_merges code
 . git-rebase--interactive.sh: extract function for adding "pick" line
 . git-rebase--am.sh: avoid special-casing --keep-empty

Expecting a reroll.

Performance concerns from Windows folks.  Also the series lacks
proper sign-offs.

* mb/remote-default-nn-origin (2012-07-11) 6 commits
 - Teach get_default_remote to respect remote.default.
 - Test that plain "git fetch" uses remote.default when on a detached HEAD.
 - Teach clone to set remote.default.
 - Teach "git remote" about remote.default.
 - Teach remote.c about the remote.default configuration setting.
 - Rename remote.c's default_remote_name static variables.

When the user does not specify what remote to interact with, we
often attempt to use 'origin'.  This can now be customized via a
configuration variable.

Expecting a reroll.

"The first remote becomes the default" bit is better done as a
separate step.

* jc/split-blob (2012-04-03) 6 commits
 - chunked-object: streaming checkout
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - packfile: use varint functions

Not ready.

I finished the streaming checkout codepath, but as explained in
127b177 (bulk-checkin: support chunked-object encoding, 2011-11-30),
these are still early steps of a long and painful journey. At least
pack-objects and fsck need to learn the new encoding for the series
to be usable locally, and then index-pack/unpack-objects needs to
learn it to be used remotely.

Given that I heard a lot of noise that people want large files, and
that I was asked by somebody at GitTogether'11 privately for an
advice on how to pay developers (not me) to help adding necessary
support, I am somewhat dissapointed that the original patch series
that was sent long time ago still remains here without much comments
and updates from the developer community. I even made the interface
to the logic that decides where to split chunks easily replaceable,
and I deliberately made the logic in the original patch extremely
stupid to entice others, especially the "bup" fanbois, to come up
with a better logic, thinking that giving people an easy target to
shoot for, they may be encouraged to help out. The plan is not
working :-<.

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

* pw/p4-submit-conflicts (2012-09-10) 12 commits
 - git-p4: add submit --conflict option and config varaiable
 - git p4: add submit --prepare-p4-only option
 - git p4: add submit --dry-run option
 - git p4: accept -v for --verbose
 - git p4: revert deleted files after submit cancel
 - git p4: rearrange submit template construction
 - git p4: test clean-up after failed submit, fix added files
 - git p4: standardize submit cancel due to unchanged template
 - git p4: move conflict prompt into run, add [q]uit input
 - git p4: remove submit failure options [a]pply and [w]rite
 - git p4: gracefully fail if some commits could not be applied
 - git p4 test: remove bash-ism of combined export/assignment

Rerolled.

Waiting for comments.

* kd/cvsimport-avoid-invalid-tag (2012-09-06) 1 commit
 - cvsimport: strip all inappropriate tag strings

"cvsimport" tried to create a tag taken from CVS without
sufficiently sanitizing it, causing the import to fail when an
invalid character in the tagname made underlying "git tag" to fail.

Will merge to 'next'.

* mh/abspath (2012-09-10) 9 commits
 - t0060: split absolute path test in two to exercise some of it on Windows
 - t0060: verify that real_path() removes extra slashes
 - real_path(): properly handle nonexistent top-level paths
 - t0060: verify that real_path() works correctly with absolute paths
 - real_path(): reject the empty string
 - t0060: verify that real_path() fails if passed the empty string
 - absolute_path(): reject the empty string
 - t0060: verify that absolute_path() fails if passed the empty string
 - t0060: move tests of real_path() from t0000 to here

Will merge to 'next'.

* nd/i18n-status (2012-09-06) 1 commit
 - status: remove i18n legos

Will merge to 'next'.

* nd/log-n-doc (2012-09-06) 1 commit
 - doc: move rev-list option -<n> from git-log.txt to rev-list-options.txt

Will merge to 'next'.

* nd/maint-remote-remove (2012-09-06) 1 commit
 - remote: prefer subcommand name 'remove' to 'rm'

Will merge to 'next'.

* sb/send-email-reconfirm-fix (2012-09-06) 1 commit
 - send-email: initial_to and initial_reply_to are both optional

Will merge to 'next'.

* sn/ls-remote-get-url-doc (2012-09-07) 1 commit
 - ls-remote: document the '--get-url' option

Will merge to 'next'.

* dj/fetch-all-tags (2012-09-07) 1 commit
 - fetch --all: pass --tags/--no-tags through to each remote
 (this branch uses jk/argv-array.)

"git fetch --all", when passed "--no-tags", did not honor the
"--no-tags" option while fetching from individual remotes (the same
issue existed with "--tags", but combination "--all --tags" makes
much less sense than "--all --no-tags").

Will merge to 'next'.

* jc/maint-ident-missing-human-name (2012-08-31) 1 commit
  (merged to 'next' on 2012-09-07 at 0e99b20)
 + split_ident_line(): make best effort when parsing author/committer line

"git show --format='%ci'" did not give timestamp correctly for
commits created without human readable name on "committer" line.

Will merge to 'master' as part of the fifth batch.

* jk/argv-array (2012-09-02) 4 commits
  (merged to 'next' on 2012-09-07 at 98dbd14)
 + submodule: use argv_array instead of hand-building arrays
 + fetch: use argv_array instead of hand-building arrays
 + argv-array: fix bogus cast when freeing array
 + argv-array: add pop function
 (this branch is used by dj/fetch-all-tags.)

Use argv-array API in "git fetch" implementation.

Will merge to 'master' as part of the fifth batch.

* rj/tap-fix (2012-09-02) 6 commits
 - test-lib.sh: Suppress the "passed all ..." message if no tests run
 - test-lib.sh: Add check for invalid use of 'skip_all' facility
 - test-lib.sh: Fix some shell coding style violations
 - t4016-*.sh: Skip all tests rather than each test
 - t3902-*.sh: Skip all tests rather than each test
 - t3300-*.sh: Fix a TAP parse error

Will merge to 'next'.

* rj/test-regex (2012-09-02) 1 commit
  (merged to 'next' on 2012-09-07 at e7e3527)
 + test-regex: Add a test to check for a bug in the regex routines

Git ships with a fall-back regexp implementation for platforms with
buggy regexp library; give people a tool to see if they should be
using it on their platform.

Will merge to 'master' as part of the fifth batch.

* jc/maint-checkout-fileglob-doc (2012-09-10) 3 commits
 - gitcli: contrast wildcard given to shell and to git
 - gitcli: formatting fix
 - Document file-glob for "git checkout -- '*.c'"

Updated with help from Peff.

Will merge to 'next'.

* jc/xprm-generation (2012-09-04) 1 commit
 - test-generation: compute generation numbers and clock skews

* rj/path-cleanup (2012-09-04) 5 commits
 - Call mkpathdup() rather than xstrdup(mkpath(...))
 - Call git_pathdup() rather than xstrdup(git_path("..."))
 - path.c: Use vsnpath() in the implementation of git_path()
 - path.c: Don't discard the return value of vsnpath()
 - path.c: Remove the 'git_' prefix from a file scope function

Will merge to 'next'.

* rs/archive-zip-utf8 (2012-09-04) 1 commit
 - archive-zip: support UTF-8 paths

Will merge to 'next'.

* nd/i18n-index-pack (2012-08-31) 1 commit
  (merged to 'next' on 2012-09-07 at bbcece1)
 + i18n: mark more index-pack strings for translation

Will merge to 'master' as part of the fifth batch.

* nd/checkout-option-parsing-fix (2012-09-07) 4 commits
 - fixup! checkout: reorder option handling
 - checkout: reorder option handling
 - checkout: move more parameters to struct checkout_opts
 - checkout: pass "struct checkout_opts *" as const pointer

The option parsing of "git checkout" had error checking, dwim and
defaulting missing options, all mixed in the code, and issuing an
appropriate error message with useful context was getting harder.
Reorganize the code and allow giving a proper diagnosis when the
user says "git checkout -b -t foo bar" (e.g. "-t" is not a good name
for a branch).

Will merge to 'next' after squashing the tip two.

* js/compat-itimer (2012-09-08) 1 commit
 - Add a no-op setitimer() wrapper

Pieces to support compilation on __TANDEM.

Will merge to 'next'.

* mh/fetch-filter-refs (2012-09-10) 14 commits
 - fetch-pack: eliminate spurious error messages
 - cmd_fetch_pack(): simplify computation of return value
 - fetch-pack: report missing refs even if no existing refs were received
 - cmd_fetch_pack(): return early if finish_connect() fails
 - filter_refs(): simplify logic
 - filter_refs(): build refs list as we go
 - filter_refs(): delete matched refs from sought list
 - fetch_pack(): update sought->nr to reflect number of unique entries
 - filter_refs(): do not check the same sought_pos twice
 - Change fetch_pack() and friends to take string_list arguments
 - fetch_pack(): reindent function decl and defn
 - Rename static function fetch_pack() to http_fetch_pack()
 - t5500: add tests of fetch-pack --all --depth=N $URL $REF
 - t5500: add tests of error output for missing refs
 (this branch uses mh/string-list.)

Code simplification and clarification.

Waiting for the mh/string-list to settle.

* jc/merge-bases (2012-08-31) 9 commits
  (merged to 'next' on 2012-09-07 at ab0974d)
 + reduce_heads(): reimplement on top of remove_redundant()
 + merge-base: "--is-ancestor A B"
 + get_merge_bases_many(): walk from many tips in parallel
 + in_merge_bases(): use paint_down_to_common()
 + merge_bases_many(): split out the logic to paint history
 + in_merge_bases(): omit unnecessary redundant common ancestor reduction
 + http-push: use in_merge_bases() for fast-forward check
 + receive-pack: use in_merge_bases() for fast-forward check
 + in_merge_bases(): support only one "other" commit

Optimise the "merge-base" computation a bit, and also update its
users that do not need the full merge-base information to call a
cheaper subset.

Will merge to 'master' as part of the fifth batch.

* jl/submodule-rm (2012-08-27) 1 commit
 - Teach rm to remove submodules unless they contain a git directory

"git rm submodule" cannot blindly remove a submodule directory as
its working tree may have local changes, and worse yet, it may even
have its repository embedded in it.  Teach it some special cases
where it is safe to remove a submodule, specifically, when there is
no local changes in the submodule working tree, and its repository
is not embedded in its working tree but is elsewhere and uses the
gitfile mechanism to point at it.

I lost track; what is the doneness of the discussion on this patch?

* fa/remote-svn (2012-08-28) 16 commits
 - Add a test script for remote-svn
 - remote-svn: add marks-file regeneration
 - Add a svnrdump-simulator replaying a dump file for testing
 - remote-svn: add incremental import
 - remote-svn: Activate import/export-marks for fast-import
 - Create a note for every imported commit containing svn metadata
 - vcs-svn: add fast_export_note to create notes
 - Allow reading svn dumps from files via file:// urls
 - remote-svn, vcs-svn: Enable fetching to private refs
 - When debug==1, start fast-import with "--stats" instead of "--quiet"
 - Add documentation for the 'bidi-import' capability of remote-helpers
 - Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability
 - Add argv_array_detach and argv_array_free_detached
 - Add svndump_init_fd to allow reading dumps from arbitrary FDs
 - Add git-remote-testsvn to Makefile and .gitignore
 - Implement a remote helper for svn in C
 (this branch is used by fa/vcs-svn.)

A GSoC project.  Looked promising.
Waiting for comments from mentors and stakeholders.

* fa/vcs-svn (2012-08-28) 4 commits
 - vcs-svn: remove repo_tree
 - vcs-svn/svndump: rewrite handle_node(), begin|end_revision()
 - vcs-svn/svndump: restructure node_ctx, rev_ctx handling
 - svndump: move struct definitions to .h
 (this branch uses fa/remote-svn.)

A GSoC project.  Looked promising.
Waiting for comments from mentors and stakeholders.

* jk/no-more-pre-exec-callback (2012-06-05) 1 commit
 - pager: drop "wait for output to run less" hack

(Originally merged to 'next' on 2012-07-23)

Will defer until the end of the 2012.
while waiting for older "less" to go extinct.

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

* jc/sanitize-nkd-lazy-iconv-open (2012-07-31) 1 commit
 . macos: lazily initialize iconv

Teach the code that works around NKD/NKC gotcha on MacOS to call
iconv_open() only when it is necessary, in the hope of avoiding
set-up overhead.  It turns out that there was no noticeable
improvements.

* nd/checkout-branch-name-check (2012-08-27) 1 commit
 . checkout: verify new branch name's validity early

"git checkout -b --opt y" errors out saying that creating a new
branch to check it out and grabbing contents for paths out of a
commit are incompatible operations.  While it is technically correct
(the command line wants to create a new branch whose name is "--opt"
and check it out, and there shouldn't be anything else left on the
command line, but there is "y"), "--opt" is not a valid name of the
branch to begin with, so even without "y", the command will not
succeed.  Treat this case specially to complain that "--opt" is not
a valid branch name.

[PATCH v3] Teach rm to remove submodules unless they contain a git directory

[Jens Lehmann]

Currently using "git rm" on a submodule - populated or not - fails with
this error:
	fatal: git rm: '<submodule path>': Is a directory
This made sense in the past as there was no way to remove a submodule
without possibly removing unpushed parts of the submodule's history
contained in its .git directory too, so erroring out here protected the
user from possible loss of data.

But submodules cloned with a recent git version do not contain the .git
directory anymore, they use a gitfile to point to their git directory
which is safely stored inside the superproject's .git directory. The work
tree of these submodules can safely be removed without loosing history, so
let's teach git to do so.

Using rm on an unpopulated submodule now removes the empty directory from
the work tree and the gitlink from the index. If the submodule's directory
is missing from the work tree, it will still be removed from the index.

Using rm on a populated submodule using a gitfile will apply the usual
checks for work tree modification adapted to submodules (unless forced).
For a submodule that means that the HEAD is the same as recorded in the
index, no tracked files are modified and no untracked files that aren't
ignored are present in the submodules work tree (ignored files are deemed
expendable and won't stop a submodule's work tree from being removed).
That logic has to be applied in all nested submodules too.

Using rm on a submodule which has its .git directory inside the work trees
top level directory will just error out like it did before to protect the
repository, even when forced. In the future git could either provide a
message informing the user to convert the submodule to use a gitfile or
even attempt to do the conversion itself, but that is not part of this
change.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 11.09.2012 01:55, schrieb Junio C Hamano:
> * jl/submodule-rm (2012-08-27) 1 commit
>  - Teach rm to remove submodules unless they contain a git directory
> 
> "git rm submodule" cannot blindly remove a submodule directory as
> its working tree may have local changes, and worse yet, it may even
> have its repository embedded in it.  Teach it some special cases
> where it is safe to remove a submodule, specifically, when there is
> no local changes in the submodule working tree, and its repository
> is not embedded in its working tree but is elsewhere and uses the
> gitfile mechanism to point at it.
> 
> I lost track; what is the doneness of the discussion on this patch?

The review of v2 revealed that in case of submodule merge conflicts
the necessary checks weren't done. This (and the minor issues raised
in http://permalink.gmane.org/gmane.comp.version-control.git/204370)
is fixed in this version.


 Documentation/git-rm.txt |  15 +++
 builtin/rm.c             | 107 ++++++++++++++---
 submodule.c              |  80 +++++++++++++
 submodule.h              |   2 +
 t/t3600-rm.sh            | 291 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 480 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 5d31860..882cb11 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -107,6 +107,21 @@ as well as modifications of existing paths.
 Typically you would first remove all tracked files from the working
 tree using this command:

+Submodules
+~~~~~~~~~~
+Only submodules using a gitfile (which means they were cloned
+with a git version 1.7.8 or newer) will be removed from the work
+tree, as their repository lives inside the .git directory of the
+superproject. If a submodule (or one of those nested inside it)
+still uses a .git directory, `git rm` will fail - no matter if forced
+or not - to protect the submodule's history.
+
+A submodule is considered up-to-date when the HEAD is the same as
+recorded in the index, no tracked files are modified and no untracked
+files that aren't ignored are present in the submodules work tree.
+Ignored files are deemed expendable and won't stop a submodule's work
+tree from being removed.
+
 ----------------
 git ls-files -z | xargs -0 rm -f
 ----------------
diff --git a/builtin/rm.c b/builtin/rm.c
index b384c4c..0e7ea7c 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -9,6 +9,7 @@
 #include "cache-tree.h"
 #include "tree-walk.h"
 #include "parse-options.h"
+#include "submodule.h"

 static const char * const builtin_rm_usage[] = {
 	N_("git rm [options] [--] <file>..."),
@@ -17,9 +18,43 @@ static const char * const builtin_rm_usage[] = {

 static struct {
 	int nr, alloc;
-	const char **name;
+	struct {
+		const char *name;
+		char is_submodule;
+	} *entry;
 } list;

+static int check_submodules_use_gitfiles(void)
+{
+	int i;
+	int errs = 0;
+
+	for (i = 0; i < list.nr; i++) {
+		const char *name = list.entry[i].name;
+		int pos;
+		struct cache_entry *ce;
+		struct stat st;
+
+		pos = cache_name_pos(name, strlen(name));
+		if (pos < 0)
+			pos = -pos-1;
+		ce = active_cache[pos];
+
+		if (!S_ISGITLINK(ce->ce_mode) ||
+		    (lstat(ce->name, &st) < 0) ||
+		    is_empty_dir(name))
+			continue;
+
+		if (!submodule_uses_gitfile(name))
+			errs = error(_("submodule '%s' (or one of its nested "
+				     "submodules) uses a .git directory\n"
+				     "(use 'rm -rf' if you really want to remove "
+				     "it including all of its history)"), name);
+	}
+
+	return errs;
+}
+
 static int check_local_mod(unsigned char *head, int index_only)
 {
 	/*
@@ -37,15 +72,23 @@ static int check_local_mod(unsigned char *head, int index_only)
 		struct stat st;
 		int pos;
 		struct cache_entry *ce;
-		const char *name = list.name[i];
+		const char *name = list.entry[i].name;
 		unsigned char sha1[20];
 		unsigned mode;
 		int local_changes = 0;
 		int staged_changes = 0;

 		pos = cache_name_pos(name, strlen(name));
-		if (pos < 0)
-			continue; /* removing unmerged entry */
+		if (pos < 0) {
+			/*
+			 * Skip unmerged entries except for populated submodules
+			 * that could loose history when removed.
+			 */
+			pos = -pos-1;
+			if (!S_ISGITLINK(active_cache[pos]->ce_mode) ||
+			    is_empty_dir(name))
+				continue;
+		}
 		ce = active_cache[pos];

 		if (lstat(ce->name, &st) < 0) {
@@ -58,9 +101,10 @@ static int check_local_mod(unsigned char *head, int index_only)
 			/* if a file was removed and it is now a
 			 * directory, that is the same as ENOENT as
 			 * far as git is concerned; we do not track
-			 * directories.
+			 * directories unless they are submodules.
 			 */
-			continue;
+			if (!S_ISGITLINK(ce->ce_mode))
+				continue;
 		}

 		/*
@@ -80,8 +124,11 @@ static int check_local_mod(unsigned char *head, int index_only)

 		/*
 		 * Is the index different from the file in the work tree?
+		 * If it's a submodule, is its work tree modified?
 		 */
-		if (ce_match_stat(ce, &st, 0))
+		if (ce_match_stat(ce, &st, 0) ||
+		    (S_ISGITLINK(ce->ce_mode) &&
+		     !ok_to_remove_submodule(ce->name)))
 			local_changes = 1;

 		/*
@@ -115,10 +162,18 @@ static int check_local_mod(unsigned char *head, int index_only)
 				errs = error(_("'%s' has changes staged in the index\n"
 					     "(use --cached to keep the file, "
 					     "or -f to force removal)"), name);
-			if (local_changes)
-				errs = error(_("'%s' has local modifications\n"
-					     "(use --cached to keep the file, "
-					     "or -f to force removal)"), name);
+			if (local_changes) {
+				if (S_ISGITLINK(ce->ce_mode) &&
+				    !submodule_uses_gitfile(name)) {
+					errs = error(_("submodule '%s' (or one of its nested "
+						     "submodules) uses a .git directory\n"
+						     "(use 'rm -rf' if you really want to remove "
+						     "it including all of its history)"), name);
+				} else
+					errs = error(_("'%s' has local modifications\n"
+						     "(use --cached to keep the file, "
+						     "or -f to force removal)"), name);
+			}
 		}
 	}
 	return errs;
@@ -173,8 +228,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		struct cache_entry *ce = active_cache[i];
 		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
 			continue;
-		ALLOC_GROW(list.name, list.nr + 1, list.alloc);
-		list.name[list.nr++] = ce->name;
+		ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
+		list.entry[list.nr].name = ce->name;
+		list.entry[list.nr++].is_submodule = S_ISGITLINK(ce->ce_mode);
 	}

 	if (pathspec) {
@@ -215,6 +271,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			hashclr(sha1);
 		if (check_local_mod(sha1, index_only))
 			exit(1);
+	} else if (!index_only) {
+		if (check_submodules_use_gitfiles())
+			exit(1);
 	}

 	/*
@@ -222,7 +281,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 * the index unless all of them succeed.
 	 */
 	for (i = 0; i < list.nr; i++) {
-		const char *path = list.name[i];
+		const char *path = list.entry[i].name;
 		if (!quiet)
 			printf("rm '%s'\n", path);

@@ -244,7 +303,25 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	if (!index_only) {
 		int removed = 0;
 		for (i = 0; i < list.nr; i++) {
-			const char *path = list.name[i];
+			const char *path = list.entry[i].name;
+			if (list.entry[i].is_submodule) {
+				if (is_empty_dir(path)) {
+					if (!rmdir(path)) {
+						removed = 1;
+						continue;
+					}
+				} else {
+					struct strbuf buf = STRBUF_INIT;
+					strbuf_addstr(&buf, path);
+					if (!remove_dir_recursively(&buf, 0)) {
+						removed = 1;
+						strbuf_release(&buf);
+						continue;
+					}
+					strbuf_release(&buf);
+					/* Fallthrough and let remove_path() fail. */
+				}
+			}
 			if (!remove_path(path)) {
 				removed = 1;
 				continue;
diff --git a/submodule.c b/submodule.c
index 19dc6a6..acb2fe0 100644
--- a/submodule.c
+++ b/submodule.c
@@ -758,6 +758,86 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
 	return dirty_submodule;
 }

+int submodule_uses_gitfile(const char *path)
+{
+	struct child_process cp;
+	const char *argv[] = {
+		"submodule",
+		"foreach",
+		"--quiet",
+		"--recursive",
+		"test -f .git",
+		NULL,
+	};
+	struct strbuf buf = STRBUF_INIT;
+	const char *git_dir;
+
+	strbuf_addf(&buf, "%s/.git", path);
+	git_dir = read_gitfile(buf.buf);
+	if (!git_dir) {
+		strbuf_release(&buf);
+		return 0;
+	}
+	strbuf_release(&buf);
+
+	/* Now test that all nested submodules use a gitfile too */
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.no_stderr = 1;
+	cp.no_stdout = 1;
+	cp.dir = path;
+	if (run_command(&cp))
+		return 0;
+
+	return 1;
+}
+
+int ok_to_remove_submodule(const char *path)
+{
+	struct stat st;
+	ssize_t len;
+	struct child_process cp;
+	const char *argv[] = {
+		"status",
+		"--porcelain",
+		"-u",
+		"--ignore-submodules=none",
+		NULL,
+	};
+	struct strbuf buf = STRBUF_INIT;
+	int ok_to_remove = 1;
+
+	if ((lstat(path, &st) < 0) || is_empty_dir(path))
+		return 1;
+
+	if (!submodule_uses_gitfile(path))
+		return 0;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.out = -1;
+	cp.dir = path;
+	if (start_command(&cp))
+		die("Could not run 'git status --porcelain -uall --ignore-submodules=none' in submodule %s", path);
+
+	len = strbuf_read(&buf, cp.out, 1024);
+	if (len > 2)
+		ok_to_remove = 0;
+	close(cp.out);
+
+	if (finish_command(&cp))
+		die("'git status --porcelain -uall --ignore-submodules=none' failed in submodule %s", path);
+
+	strbuf_release(&buf);
+	return ok_to_remove;
+}
+
 static int find_first_merges(struct object_array *result, const char *path,
 		struct commit *a, struct commit *b)
 {
diff --git a/submodule.h b/submodule.h
index e105b0e..9c0f6a4 100644
--- a/submodule.h
+++ b/submodule.h
@@ -27,6 +27,8 @@ int fetch_populated_submodules(int num_options, const char **options,
 			       const char *prefix, int command_line_option,
 			       int quiet);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);
+int submodule_uses_gitfile(const char *path);
+int ok_to_remove_submodule(const char *path);
 int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
 		    const unsigned char a[20], const unsigned char b[20], int search);
 int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 9fd28bc..61ef529 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -262,4 +262,295 @@ test_expect_success 'rm removes subdirectories recursively' '
 	! test -d dir
 '

+cat >expect <<EOF
+D  submod
+EOF
+
+cat >expect.modified <<EOF
+ M submod
+EOF
+
+test_expect_success 'rm removes empty submodules from work tree' '
+	mkdir submod &&
+	git update-index --add --cacheinfo 160000 $(git rev-parse HEAD) submod &&
+	git config -f .gitmodules submodule.sub.url ./. &&
+	git config -f .gitmodules submodule.sub.path submod &&
+	git submodule init &&
+	git add .gitmodules &&
+	git commit -m "add submodule" &&
+	git rm submod &&
+	test ! -e submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm removes removed submodule from index' '
+	git reset --hard &&
+	git submodule update &&
+	rm -rf submod &&
+	git rm submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm removes work tree of unmodified submodules' '
+	git reset --hard &&
+	git submodule update &&
+	git rm submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated submodule with different HEAD fails unless forced' '
+	git reset --hard &&
+	git submodule update &&
+	(cd submod &&
+		git checkout HEAD^
+	) &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -f submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.modified actual &&
+	git rm -f submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated submodule with modifications fails unless forced' '
+	git reset --hard &&
+	git submodule update &&
+	(cd submod &&
+		echo X >empty
+	) &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -f submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.modified actual &&
+	git rm -f submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated submodule with untracked files fails unless forced' '
+	git reset --hard &&
+	git submodule update &&
+	(cd submod &&
+		echo X >untracked
+	) &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -f submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.modified actual &&
+	git rm -f submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'setup submodule conflict' '
+	git reset --hard &&
+	git submodule update &&
+	git checkout -b branch1 &&
+	echo 1 >nitfol &&
+	git add nitfol &&
+	git commit -m "added nitfol 1" &&
+	git checkout -b branch2 master &&
+	echo 2 >nitfol &&
+	git add nitfol &&
+	git commit -m "added nitfol 2" &&
+	git checkout -b conflict1 master &&
+	(cd submod &&
+		git fetch &&
+		git checkout branch1
+	) &&
+	git add submod &&
+	git commit -m "submod 1" &&
+	git checkout -b conflict2 master &&
+	(cd submod &&
+		git checkout branch2
+	) &&
+	git add submod &&
+	git commit -m "submod 2"
+'
+
+cat >expect.conflict <<EOF
+UU submod
+EOF
+
+test_expect_success 'rm of a conflicted populated submodule fails unless forced' '
+	git checkout conflict1 &&
+	git reset --hard &&
+	git submodule update &&
+	test_must_fail git merge conflict2 &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -f submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.conflict actual &&
+	git rm -f submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a conflicted populated submodule with a .git directory fails even when forced' '
+	git checkout conflict1 &&
+	git reset --hard &&
+	git submodule update &&
+	(cd submod &&
+		rm .git &&
+		cp -a ../.git/modules/sub .git &&
+		GIT_WORK_TREE=. git config --unset core.worktree
+	) &&
+	test_must_fail git merge conflict2 &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -d submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.conflict actual &&
+	test_must_fail git rm -f submod &&
+	test -d submod &&
+	test -d submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.conflict actual &&
+	git merge --abort &&
+	rm -rf submod
+'
+
+test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
+	git checkout conflict1 &&
+	git reset --hard &&
+	test_must_fail git merge conflict2 &&
+	git rm submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated submodule with a .git directory fails even when forced' '
+	git checkout -f master &&
+	git reset --hard &&
+	git submodule update &&
+	(cd submod &&
+		rm .git &&
+		cp -a ../.git/modules/sub .git &&
+		GIT_WORK_TREE=. git config --unset core.worktree
+	) &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -d submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	! test -s actual &&
+	test_must_fail git rm -f submod &&
+	test -d submod &&
+	test -d submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	! test -s actual &&
+	rm -rf submod
+'
+
+cat >expect.deepmodified <<EOF
+ M submod/subsubmod
+EOF
+
+test_expect_success 'setup subsubmodule' '
+	git reset --hard &&
+	git submodule update &&
+	(cd submod &&
+		git update-index --add --cacheinfo 160000 $(git rev-parse HEAD) subsubmod &&
+		git config -f .gitmodules submodule.sub.url ../. &&
+		git config -f .gitmodules submodule.sub.path subsubmod &&
+		git submodule init &&
+		git add .gitmodules &&
+		git commit -m "add subsubmodule" &&
+		git submodule update subsubmod
+	) &&
+	git commit -a -m "added deep submodule"
+'
+
+test_expect_success 'rm recursively removes work tree of unmodified submodules' '
+	git rm submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated nested submodule with different nested HEAD fails unless forced' '
+	git reset --hard &&
+	git submodule update --recursive &&
+	(cd submod/subsubmod &&
+		git checkout HEAD^
+	) &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -f submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.modified actual &&
+	git rm -f submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated nested submodule with nested modifications fails unless forced' '
+	git reset --hard &&
+	git submodule update --recursive &&
+	(cd submod/subsubmod &&
+		echo X >empty
+	) &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -f submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.modified actual &&
+	git rm -f submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated nested submodule with nested untracked files fails unless forced' '
+	git reset --hard &&
+	git submodule update --recursive &&
+	(cd submod/subsubmod &&
+		echo X >untracked
+	) &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -f submod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect.modified actual &&
+	git rm -f submod &&
+	test ! -d submod &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated nested submodule with a nested .git directory fails even when forced' '
+	git reset --hard &&
+	git submodule update --recursive &&
+	(cd submod/subsubmod &&
+		rm .git &&
+		cp -a ../../.git/modules/sub/modules/sub .git &&
+		GIT_WORK_TREE=. git config --unset core.worktree
+	) &&
+	test_must_fail git rm submod &&
+	test -d submod &&
+	test -d submod/subsubmod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	! test -s actual &&
+	test_must_fail git rm -f submod &&
+	test -d submod &&
+	test -d submod/subsubmod/.git &&
+	git status -s -uno --ignore-submodules=none > actual &&
+	! test -s actual &&
+	rm -rf submod
+'
+
 test_done
-- 
1.7.12.316.gac65367

Re: [PATCH v3] Teach rm to remove submodules unless they contain a git directory

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

>> * jl/submodule-rm (2012-08-27) 1 commit
>>  - Teach rm to remove submodules unless they contain a git directory
>> 
>> "git rm submodule" cannot blindly remove a submodule directory as
>> its working tree may have local changes, and worse yet, it may even
>> have its repository embedded in it.  Teach it some special cases
>> where it is safe to remove a submodule, specifically, when there is
>> no local changes in the submodule working tree, and its repository
>> is not embedded in its working tree but is elsewhere and uses the
>> gitfile mechanism to point at it.
>> 
>> I lost track; what is the doneness of the discussion on this patch?
>
> The review of v2 revealed that in case of submodule merge conflicts
> the necessary checks weren't done. This (and the minor issues raised
> in http://permalink.gmane.org/gmane.comp.version-control.git/204370)
> is fixed in this version.

Thanks.  I wish all others paid attention to "What's cooking" like
you did here.

And if it is hard to do so for whatever reason, suggest a better way
for me to publish "What's cooking" or an equivalent (I am interested
in finding the least bureaucratic way to help people and keep the
balls rolling).

> +static int check_submodules_use_gitfiles(void)
> +{
> +	int i;
> +	int errs = 0;
> +
> +	for (i = 0; i < list.nr; i++) {
> +		const char *name = list.entry[i].name;
> +		int pos;
> +		struct cache_entry *ce;
> +		struct stat st;
> +
> +		pos = cache_name_pos(name, strlen(name));
> +		if (pos < 0)
> +			pos = -pos-1;
> +		ce = active_cache[pos];
> +
> +		if (!S_ISGITLINK(ce->ce_mode) ||
> +		    (lstat(ce->name, &st) < 0) ||
> +		    is_empty_dir(name))
> +			continue;

If the name doesn't exist in the index (i.e. "list" has names that
do not exist in the index for whatever reason), a negative pos is
returned to tell you where it _would_ be inserted if you said "git
add" the path.  But these names in the "list" are guaranteed to
exist in the index in _some_ form, so for a negative pos, (-pos-1)
will have the conflicted entry at the lowest stage (typically the
common ancestor's version).  I am not sure checking only that one is
sufficient, though.  Wouldn't you want to at least check stage #2
(ours, which should most resemble the working tree)?  If this were
"common ancestor had it as a submodule, our side removed it and
created something else, their side updated the submodule" conflict,
the stage #2 would not be a gitlink (it would be a blob if that
something else is a file, or may be missing if the submodule was
replaced with a directory), and the path ce->name would definitely
not be a submodule.

> +		if (!submodule_uses_gitfile(name))
> +			errs = error(_("submodule '%s' (or one of its nested "
> +				     "submodules) uses a .git directory\n"
> +				     "(use 'rm -rf' if you really want to remove "
> +				     "it including all of its history)"), name);
> +	}
> +
> +	return errs;
> +}
> +
>  static int check_local_mod(unsigned char *head, int index_only)
>  {
>  	/*
> @@ -37,15 +72,23 @@ static int check_local_mod(unsigned char *head, int index_only)
>  		struct stat st;
>  		int pos;
>  		struct cache_entry *ce;
> -		const char *name = list.name[i];
> +		const char *name = list.entry[i].name;
>  		unsigned char sha1[20];
>  		unsigned mode;
>  		int local_changes = 0;
>  		int staged_changes = 0;
>
>  		pos = cache_name_pos(name, strlen(name));
> -		if (pos < 0)
> -			continue; /* removing unmerged entry */
> +		if (pos < 0) {
> +			/*
> +			 * Skip unmerged entries except for populated submodules
> +			 * that could loose history when removed.

s/loose/lose/

> +			 */
> +			pos = -pos-1;
> +			if (!S_ISGITLINK(active_cache[pos]->ce_mode) ||
> +			    is_empty_dir(name))
> +				continue;
> +		}

Lilewise.  It may make sense to introduce a helper function to tell
if it is a submodule on our side by checking only the stage #2 entry
when you see a nagetive pos returned from cache_name_pos() and call
it "is_ours_submodule?()" or something.

Suggestions for "What's cooking"

[Jens Lehmann]

Am 11.09.2012 21:41, schrieb Junio C Hamano:
> Thanks.  I wish all others paid attention to "What's cooking" like
> you did here.
> 
> And if it is hard to do so for whatever reason, suggest a better way
> for me to publish "What's cooking" or an equivalent (I am interested
> in finding the least bureaucratic way to help people and keep the
> balls rolling).

I think "What's cooking" makes lots of sense in its current form
as one gets a very good overview over current development tracks.

Maybe in addition it would be nice to email the author(s) of a
series when the state changes or new comments are added (and to
only include the relevant part from "What's cooking" there). For
me it's not a big problem as I just have to grep for "submodule"
to get the bits I care about, but I suspect others might have to
invest much more time to check the current state of their series
and may appreciate being mailed directly when something happens.
Opinions?

Re: [PATCH v3] Teach rm to remove submodules unless they contain a git directory

[Jens Lehmann]

Am 11.09.2012 21:41, schrieb Junio C Hamano:
> Lilewise.  It may make sense to introduce a helper function to tell
> if it is a submodule on our side by checking only the stage #2 entry
> when you see a nagetive pos returned from cache_name_pos() and call
> it "is_ours_submodule?()" or something.

Thanks, will do so.

Re: Suggestions for "What's cooking"

[Jeff King]

On Wed, Sep 12, 2012 at 09:21:46PM +0200, Jens Lehmann wrote:

> Am 11.09.2012 21:41, schrieb Junio C Hamano:
> > Thanks.  I wish all others paid attention to "What's cooking" like
> > you did here.
> > 
> > And if it is hard to do so for whatever reason, suggest a better way
> > for me to publish "What's cooking" or an equivalent (I am interested
> > in finding the least bureaucratic way to help people and keep the
> > balls rolling).
> 
> I think "What's cooking" makes lots of sense in its current form
> as one gets a very good overview over current development tracks.
>
> Maybe in addition it would be nice to email the author(s) of a
> series when the state changes or new comments are added (and to
> only include the relevant part from "What's cooking" there).

Yeah, in general I think the current system is fine. It might be
slightly more convenient to send out the update email, but finding the
right thread is more work for Junio (I guess you could just ignore that
and start a new thread, but for readers it is nice if it is connected to
the original series thread).

And I personally think it is nice to read through the whole list of
topics and their current status occasionally. I sometimes end up
commenting on a topic that I probably would not have otherwise seen that
way.  Of course, I likely have a lot more git time than most people, so
the effort of skimming "what's cooking" is not too high for me.

-Peff

Re: Suggestions for "What's cooking"

[Dan Johnson]

On Wed, Sep 12, 2012 at 3:21 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 11.09.2012 21:41, schrieb Junio C Hamano:
>> Thanks.  I wish all others paid attention to "What's cooking" like
>> you did here.
>>
>> And if it is hard to do so for whatever reason, suggest a better way
>> for me to publish "What's cooking" or an equivalent (I am interested
>> in finding the least bureaucratic way to help people and keep the
>> balls rolling).
>
> I think "What's cooking" makes lots of sense in its current form
> as one gets a very good overview over current development tracks.
>
> Maybe in addition it would be nice to email the author(s) of a
> series when the state changes or new comments are added (and to
> only include the relevant part from "What's cooking" there). For
> me it's not a big problem as I just have to grep for "submodule"
> to get the bits I care about, but I suspect others might have to
> invest much more time to check the current state of their series
> and may appreciate being mailed directly when something happens.
> Opinions?

I was thinking about this earlier. I wondered if it might even be
worth it just to CC the authors of all topics whose status has changed
since the last what's cooking, to make sure that they see updates
pertinent to them. I know that I at least have filters which catch
emails which CC me and promote them to my inbox, so I would see them
more readily.

My normal mode of operation is that when I have a patch in I check all
the "What's cooking" messages as if I was F5-ing a webpage, to follow
its status. Were I CCd on the message, I would be updated whenever the
mail was sent, which I would appreciate. This also has the nice side
effect of updating patch authors who are not subscribed to the list.

On the other hand, its possible some people would find that this
generated lots of noise, and it might also cause unrelated replies to
the "What's Cooking" message to CC all authors.
-- 
-Dan

Re: Suggestions for "What's cooking"

[Junio C Hamano]

Dan Johnson <computerdruid@gmail.com> writes:

> I was thinking about this earlier. I wondered if it might even be
> worth it just to CC the authors of all topics whose status has changed
> since the last what's cooking, to make sure that they see updates
> pertinent to them. I know that I at least have filters which catch
> emails which CC me and promote them to my inbox, so I would see them
> more readily.

I've done that a few times per release cycle, usually before we go
into the pre-release freeze, but doing so manually is very time
consuming.  It's the kind of bureaucratic overhead I'd rather avoid.
If somebody volunteers to write a script that takes something like

    git diff whats-cooking.txt

in a checkout of the 'todo' branch and figure out whom to Cc, and do
so reliably, it may be an option.

Thanks.

Re: Suggestions for "What's cooking"

[Philip Oakley]

From: "Jens Lehmann" <Jens.Lehmann@web.de>
Sent: Wednesday, September 12, 2012 8:21 PM
> Am 11.09.2012 21:41, schrieb Junio C Hamano:
>> Thanks.  I wish all others paid attention to "What's cooking" like
>> you did here.
>>
>> And if it is hard to do so for whatever reason, suggest a better way
>> for me to publish "What's cooking" or an equivalent (I am interested
>> in finding the least bureaucratic way to help people and keep the
>> balls rolling).
>
> I think "What's cooking" makes lots of sense in its current form
> as one gets a very good overview over current development tracks.
>
> Maybe in addition it would be nice to email the author(s) of a
> series when the state changes or new comments are added (and to
> only include the relevant part from "What's cooking" there). For
> me it's not a big problem as I just have to grep for "submodule"
> to get the bits I care about, but I suspect others might have to
> invest much more time to check the current state of their series
> and may appreciate being mailed directly when something happens.
> Opinions?

My comment, as a simple reader, is that I misread the order of the 
items, in that I miss-associate the description paragraph with the * 
title _below_. That is, I see the description first and then read on...

Thinking about it, if the description paragraph was indented by one 
space then the * title  would create that obvious content indent that (I 
am) would be expected.

Obviously only a useful suggestion if it's easy to implement...

Philip 

Re: Suggestions for "What's cooking"

[Andrew Ardill]

(sorry about double replying - html sub-part creeped in!)
On 13 September 2012 08:49, Philip Oakley <philipoakley@iee.org> wrote:
>
> From: "Jens Lehmann" <Jens.Lehmann@web.de>
> Sent: Wednesday, September 12, 2012 8:21 PM
>
>> Am 11.09.2012 21:41, schrieb Junio C Hamano:
>>>
>>> Thanks.  I wish all others paid attention to "What's cooking" like
>>> you did here.
>>>
>>> And if it is hard to do so for whatever reason, suggest a better way
>>> for me to publish "What's cooking" or an equivalent (I am interested
>>> in finding the least bureaucratic way to help people and keep the
>>> balls rolling).
>>
>>
>> I think "What's cooking" makes lots of sense in its current form
>> as one gets a very good overview over current development tracks.
>>
>> Maybe in addition it would be nice to email the author(s) of a
>> series when the state changes or new comments are added (and to
>> only include the relevant part from "What's cooking" there). For
>> me it's not a big problem as I just have to grep for "submodule"
>> to get the bits I care about, but I suspect others might have to
>> invest much more time to check the current state of their series
>> and may appreciate being mailed directly when something happens.
>> Opinions?
>
>
> My comment, as a simple reader, is that I misread the order of the items, in that I miss-associate the description paragraph with the * title _below_. That is, I see the description first and then read on...
>
> Thinking about it, if the description paragraph was indented by one space then the * title  would create that obvious content indent that (I am) would be expected.
>
> Obviously only a useful suggestion if it's easy to implement...


I can attest to the fact that the format can be at times difficult to
parse, and I often find myself rereading sections to make sure I
understood what each was referring to.

As a casual reader, interested in the development that is going on,
the things I am interested in for each branch/topic are like:
 - Branch/Topic description
 - Current integration status
 - Next steps required
 - Notes and memoranda

I understand that references to where the branch is found (it's name)
and what it includes (commit list) are important too, but these are
less important for me.

Currently, the output for each branch looks something like:
* <branch-name> (<creation-date>) <number-of-commits>
  (<merge-status>)
 [list-of-commits]
  (<branch-usage>)
<long-description>
<notes-and-memoranda>
<next-steps>

and these are grouped by current integration status (new, graduated,
stalled etc)

A format that would make this information easier for me to parse would
be something like:

<short-branch-description>
  <long-branch-description>
  <notes>
  <next-steps>
  * <branch-name> (<creation-date>) <number-of-commits>
    (<merge-status>)
   [list-of-commits]
    (<branch-usage>)

Essentially, shifting the details of the branch to the bottom, and
adding a short description for the entire branch. Indent everything
after the short description to make it clear that they belong
together.

The only real 'new' information required is the short description, but
that could be replaced with the topic name if short description is not
available (or the topic name is self explanatory).

Most of the parsing benefit would come from the indentation, but
having the 'summary' information near the top would let me skip things
I am not interested in without having to scan the list of commits and
other details.

Regards,

Andrew Ardill

Re: Suggestions for "What's cooking"

[Michael Haggerty]

On 09/13/2012 12:49 AM, Philip Oakley wrote:
> My comment, as a simple reader, is that I misread the order of the 
> items, in that I miss-associate the description paragraph with the * 
> title _below_. That is, I see the description first and then read on...
> 
> Thinking about it, if the description paragraph was indented by one 
> space then the * title  would create that obvious content indent that (I 
> am) would be expected.

+1.  When I started reading the "What's cooking", I found it hard to
tell/remember whether text comments apply to the list of patches above
them or below them.  If you don't want to make bigger changes to the
format, then even an extra blank line between the section about each
patch series would remove the ambiguity.

Otherwise, I think that "What's cooking" emails are a great service that
you provide to the community.  They help mitigate the inconvenience of
using emails rather than pull requests for exchanging and managing
patches :-)

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: Suggestions for "What's cooking"

[Philip Oakley]

From: "Philip Oakley" <philipoakley@iee.org>
Sent: Wednesday, September 12, 2012 11:49 PM
> From: "Jens Lehmann" <Jens.Lehmann@web.de>
> Sent: Wednesday, September 12, 2012 8:21 PM
>> Am 11.09.2012 21:41, schrieb Junio C Hamano:
>>> Thanks.  I wish all others paid attention to "What's cooking" like
>>> you did here.
>>>
>>> And if it is hard to do so for whatever reason, suggest a better way
>>> for me to publish "What's cooking" or an equivalent (I am interested
>>> in finding the least bureaucratic way to help people and keep the
>>> balls rolling).
>>
>> I think "What's cooking" makes lots of sense in its current form
>> as one gets a very good overview over current development tracks.
>>
>> Maybe in addition it would be nice to email the author(s) of a
>> series when the state changes or new comments are added (and to
>> only include the relevant part from "What's cooking" there). For
>> me it's not a big problem as I just have to grep for "submodule"
>> to get the bits I care about, but I suspect others might have to
>> invest much more time to check the current state of their series
>> and may appreciate being mailed directly when something happens.
>> Opinions?
>
> My comment, as a simple reader, is that I misread the order of the 
> items, in that I miss-associate the description paragraph with the * 
> title _below_. That is, I see the description first and then read 
> on...
>
> Thinking about it, if the description paragraph was indented by one 
> space then the * title  would create that obvious content indent that 
> (I am) would be expected.
>
> Obviously only a useful suggestion if it's easy to implement...
>
> Philip
Thinking overnight. One very simple option is to just add a double line 
spacing between items to give a clearer break.

 i.e.
previous item ends.LF
LF
LF
* Next Item 

Re: Suggestions for "What's cooking"

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

>> Thinking about it, if the description paragraph was indented by one
>> space then the * title  would create that obvious content indent
>> that (I am) would be expected.
>>
>> Obviously only a useful suggestion if it's easy to implement...
>>
>> Philip
> Thinking overnight. One very simple option is to just add a double
> line spacing between items to give a clearer break.

I've played with both and have prepared patches to Reintegrate and
cook (both in the 'todo' branch).  Will play with the changes a bit
more and then decide.

Thanks.

Re: Suggestions for "What's cooking"

[Junio C Hamano]

Andrew Ardill <andrew.ardill@gmail.com> writes:

> Currently, the output for each branch looks something like:
> * <branch-name> (<creation-date>) <number-of-commits>
>   (<merge-status>)
>  [list-of-commits]
>   (<branch-usage>)
> <long-description>
> <notes-and-memoranda>
> <next-steps>
>
> and these are grouped by current integration status (new, graduated,
> stalled etc)

Yes.  Thanks for a concise summary.

> A format that would make this information easier for me to parse would
> be something like:
>
> <short-branch-description>
>   <long-branch-description>
>   <notes>
>   <next-steps>
>   * <branch-name> (<creation-date>) <number-of-commits>
>     (<merge-status>)
>    [list-of-commits]
>     (<branch-usage>)

I do not see how it makes any sense to have the "This is where the
section begins with, and its name is this" line in the middle of a
block indented in such a way.  Care to explain?

I can see some people may care more about the description than the
list of commits [*1*], though.


[Footnote]

*1* It however is an indication that the title of each commit needs
to be improved to convey enough information so that I do not have to
write the branch description myself for them.

Re: Suggestions for "What's cooking"

[Andrew Ardill]

On 14 September 2012 04:06, Junio C Hamano <gitster@pobox.com> wrote:
> Andrew Ardill <andrew.ardill@gmail.com> writes:
>
>> Currently, the output for each branch looks something like:
>> * <branch-name> (<creation-date>) <number-of-commits>
>>   (<merge-status>)
>>  [list-of-commits]
>>   (<branch-usage>)
>> <long-description>
>> <notes-and-memoranda>
>> <next-steps>
>>
>> and these are grouped by current integration status (new, graduated,
>> stalled etc)
>
> Yes.  Thanks for a concise summary.
>
>> A format that would make this information easier for me to parse would
>> be something like:
>>
>> <short-branch-description>
>>   <long-branch-description>
>>   <notes>
>>   <next-steps>
>>   * <branch-name> (<creation-date>) <number-of-commits>
>>     (<merge-status>)
>>    [list-of-commits]
>>     (<branch-usage>)
>
> I do not see how it makes any sense to have the "This is where the
> section begins with, and its name is this" line in the middle of a
> block indented in such a way.  Care to explain?

I'm not quite sure what aspect you are referring to, so let me just
expand my reasoning a little bit and hopefully that clears things up.

First of all, I didn't spend that much time thinking through the
layout, merely re-arranged things so that what I considered most
important was at the start of each listing. I kept everything else the
same, with an extra level of indentation for everything except the
first line of each listing. Perhaps modifying the existing indentation
to better fit in this layout is in order, but that is in some ways
orthogonal to the ideas I was trying to present.

I am not against changing how each listing is laid out in a more
disruptive way, this was just a first attempt at making it easier to
parse.

I like the idea proposed by others to increase whitespace between
listings to make each stand out, however I think indentation is a
better method.
* Increased whitespace between listings lengthens the entire list,
requiring more scrolling and decreasing the amount of information on
each page. Simply indenting most lines by a few columns of whitespace
may cause some lines to wrap, but in general will not lengthen the
listing or decrease information density. [edit] I realised after
writing this that the addition of a <short-branch-description> does
actually increase the length of the listing, however it does not
decrease information density as much as a blank line.
* The visual difference between two blank lines and one is
significant, but not as distinct as the presence (or not) of a
character in the first column of text. In scanning a long document, I
propose that finding a line that starts in the first column of text is
easier than finding the next line which is preceded by two blank
lines. Similarly jumping forwards or backwards a listing would be
easier.

This is all a little academic though, so lets compare both versions
with an excerpt from the most recent "What's cooking"!

First, the extra blank line

-- >8 --
* jc/maint-blame-no-such-path (2012-09-10) 1 commit
 - blame $path: avoid getting fooled by case insensitive filesystems

"git blame MAKEFILE" run in a history that has "Makefile" but not
"MAKEFILE" should say "No such file MAKEFILE in HEAD", but got
confused on a case insensitive filesystem.


* sl/autoconf (2012-09-11) 2 commits
 - build: don't duplicate substitution of make variables
 - build: improve GIT_CONF_SUBST signature


* cn/branch-set-upstream-to (2012-09-11) 2 commits
 - completion: complete branch name for "branch --set-upstream-to="
 - completion: add --set-upstream-to and --unset-upstream

Will merge to 'next'.

--------------------------------------------------
[Graduated to "master"]

* jc/maint-ident-missing-human-name (2012-08-31) 1 commit
  (merged to 'next' on 2012-09-07 at 0e99b20)
 + split_ident_line(): make best effort when parsing author/committer line

"git show --format='%ci'" did not give timestamp correctly for
commits created without human readable name on "committer" line.


* jc/merge-bases (2012-08-31) 9 commits
  (merged to 'next' on 2012-09-07 at ab0974d)
 + reduce_heads(): reimplement on top of remove_redundant()
 + merge-base: "--is-ancestor A B"
 + get_merge_bases_many(): walk from many tips in parallel
 + in_merge_bases(): use paint_down_to_common()
 + merge_bases_many(): split out the logic to paint history
 + in_merge_bases(): omit unnecessary redundant common ancestor reduction
 + http-push: use in_merge_bases() for fast-forward check
 + receive-pack: use in_merge_bases() for fast-forward check
 + in_merge_bases(): support only one "other" commit

Optimise the "merge-base" computation a bit, and also update its
users that do not need the full merge-base information to call a
cheaper subset.

-- 8< --

Now, the extra indentation and re-organised contents

-- >8 --
jc/maint-blame-no-such-path
  "git blame MAKEFILE" run in a history that has "Makefile" but not
  "MAKEFILE" should say "No such file MAKEFILE in HEAD", but got
  confused on a case insensitive filesystem.

  * jc/maint-blame-no-such-path (2012-09-10) 1 commit
   - blame $path: avoid getting fooled by case insensitive filesystems

sl/autoconf
  * sl/autoconf (2012-09-11) 2 commits
   - build: don't duplicate substitution of make variables
   - build: improve GIT_CONF_SUBST signature

cn/branch-set-upstream-to
  Will merge to 'next'.

  * cn/branch-set-upstream-to (2012-09-11) 2 commits
   - completion: complete branch name for "branch --set-upstream-to="
   - completion: add --set-upstream-to and --unset-upstream

--------------------------------------------------
[Graduated to "master"]

jc/maint-ident-missing-human-name
  "git show --format='%ci'" did not give timestamp correctly for
  commits created without human readable name on "committer" line.

  * jc/maint-ident-missing-human-name (2012-08-31) 1 commit
    (merged to 'next' on 2012-09-07 at 0e99b20)
   + split_ident_line(): make best effort when parsing author/committer line

jc/merge-bases
  Optimise the "merge-base" computation a bit, and also update its
  users that do not need the full merge-base information to call a
  cheaper subset.

  * jc/merge-bases (2012-08-31) 9 commits
    (merged to 'next' on 2012-09-07 at ab0974d)
   + reduce_heads(): reimplement on top of remove_redundant()
   + merge-base: "--is-ancestor A B"
   + get_merge_bases_many(): walk from many tips in parallel
   + in_merge_bases(): use paint_down_to_common()
   + merge_bases_many(): split out the logic to paint history
   + in_merge_bases(): omit unnecessary redundant common ancestor reduction
   + http-push: use in_merge_bases() for fast-forward check
   + receive-pack: use in_merge_bases() for fast-forward check
   + in_merge_bases(): support only one "other" commit
-- 8< --

I personally find the second much more useful, but perhaps the
comparison will help other people evaluate them both.

>
> I can see some people may care more about the description than the
> list of commits [*1*], though.
>
>
> [Footnote]
>
> *1* It however is an indication that the title of each commit needs
> to be improved to convey enough information so that I do not have to
> write the branch description myself for them.

I remember something about including topic descriptions being
described when signed tag pull requests were being designed, could
that information potentially be coerced if available?

Regards,

Andrew Ardill

Re: Suggestions for "What's cooking"

[Junio C Hamano]

Andrew Ardill <andrew.ardill@gmail.com> writes:

> On 14 September 2012 04:06, Junio C Hamano <gitster@pobox.com> wrote:
>> Andrew Ardill <andrew.ardill@gmail.com> writes:
>>
>>> <short-branch-description>
>>>   <long-branch-description>
>>>   <notes>
>>>   <next-steps>
>>>   * <branch-name> (<creation-date>) <number-of-commits>
>>>     (<merge-status>)
>>>    [list-of-commits]
>>>     (<branch-usage>)
>>
>> I do not see how it makes any sense to have the "This is where the
>> section begins with, and its name is this" line in the middle of a
>> block indented in such a way.  Care to explain?
>
> I'm not quite sure what aspect you are referring to,...

Just this part, as I do not have much time.  Here is your reordered
one I will reject:

  A > jc/maint-blame-no-such-path
    >   "git blame MAKEFILE" run in a history that has "Makefile" but not
    >   "MAKEFILE" should say "No such file MAKEFILE in HEAD", but got
    >   confused on a case insensitive filesystem.
    >
  B >   * jc/maint-blame-no-such-path (2012-09-10) 1 commit
    >    - blame $path: avoid getting fooled by case insensitive filesystems

I was noting that B which *is* formatted as a header line (it EVEN
has a leading asterisk to make it clear that it begins something
new) is in the middle, and you added a redundant A that is not even
marked clearly as a header line.

Re: Suggestions for "What's cooking"

[Andrew Ardill]

On 14 September 2012 12:29, Junio C Hamano <gitster@pobox.com> wrote:
> Andrew Ardill <andrew.ardill@gmail.com> writes:
>
>> On 14 September 2012 04:06, Junio C Hamano <gitster@pobox.com> wrote:
>>> Andrew Ardill <andrew.ardill@gmail.com> writes:
>>>
>>>> <short-branch-description>
>>>>   <long-branch-description>
>>>>   <notes>
>>>>   <next-steps>
>>>>   * <branch-name> (<creation-date>) <number-of-commits>
>>>>     (<merge-status>)
>>>>    [list-of-commits]
>>>>     (<branch-usage>)
>>>
>>> I do not see how it makes any sense to have the "This is where the
>>> section begins with, and its name is this" line in the middle of a
>>> block indented in such a way.  Care to explain?
>>
>> I'm not quite sure what aspect you are referring to,...
>
> Just this part, as I do not have much time.  Here is your reordered
> one I will reject:
>
>   A > jc/maint-blame-no-such-path
>     >   "git blame MAKEFILE" run in a history that has "Makefile" but not
>     >   "MAKEFILE" should say "No such file MAKEFILE in HEAD", but got
>     >   confused on a case insensitive filesystem.
>     >
>   B >   * jc/maint-blame-no-such-path (2012-09-10) 1 commit
>     >    - blame $path: avoid getting fooled by case insensitive filesystems
>
> I was noting that B which *is* formatted as a header line (it EVEN
> has a leading asterisk to make it clear that it begins something
> new) is in the middle, and you added a redundant A that is not even
> marked clearly as a header line.

The leading asterisk is actually not as useful to me, as indicating a
header line, as the 'out-denting' I am proposing. I think this is due
to the similarities between the asterisk and the other symbols used to
indicate commits. This is maybe just a typographic issue, but I think
in general the contrast between letters and spaces appearing in the
first columns of text is stronger than either of characters and
letters, or spaces and characters. A quick comparison of all three:

--Letters and Spaces--
jc/maint-ident-missing-human-name
  "git show --format='%ci'" did not give timestamp correctly for...
   + split_ident_line(): make best effort when parsing author/committer line

--Characters and Letters--
* jc/maint-ident-missing-human-name
"git show --format='%ci'" did not give timestamp correctly for...
 + split_ident_line(): make best effort when parsing author/committer line

--Characters and Spaces--
* jc/maint-ident-missing-human-name
  "git show --format='%ci'" did not give timestamp correctly for
   + split_ident_line(): make best effort when parsing author/committer line

My preference would be first for letters and spaces, or if that is not
good enough then characters and spaces.


With regards to the comment that the old header line appears in the
middle of the output, as I said earlier that was a consequence of
reordering and indenting everything but otherwise leaving it as is.
This should be changed, so how about:

<branch-name> (<creation-date>)
  <branch-description?>
  <notes-and-memoranda?>
  <next-steps?>

  <#-commits> (<merge-status?>)
   [list-of-commits]
  (<branch-usage?>)

eg:
jc/maint-ident-missing-human-name (2012-08-31)
  "git show --format='%ci'" did not give timestamp correctly for
  commits created without human readable name on "committer" line.

  1 commit (merged to 'next' on 2012-09-07 at 0e99b20)
   + split_ident_line(): make best effort when parsing author/committer line


with no description:
sl/autoconf (2012-09-11)
  2 commits
   - build: don't duplicate substitution of make variables
   - build: improve GIT_CONF_SUBST signature


Hopefully that makes more sense and addresses the concerns you raised.
Adding an asterisk at the start is ok by me, if that is something you
think is needed.

One thing I did think about, when leaving the asterisk in the middle
of the listing in the first version, was how machine readable the
format was. I'm not sure if that is important, but the asterisk was a
clear signal that what followed was a listing of commits. In any case,
the new and revised format is perhaps slightly less machine readable
as a result.


I feel a little bit like I might be bikeshedding this, however I do
think an improvement to the formatting of "What's cooking" is a
meaningful one for the project!

Regards,

Andrew Ardill

Re: Suggestions for "What's cooking"

[Junio C Hamano]

Andrew Ardill <andrew.ardill@gmail.com> writes:

> I feel a little bit like I might be bikeshedding this...

Yes you are.

Re: Suggestions for "What's cooking"

[Junio C Hamano]

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

> I've played with both and have prepared patches to Reintegrate and
> cook (both in the 'todo' branch).  Will play with the changes a bit
> more and then decide.

So here is how tonight's "What's cooking" may look like with extra
indentation and blank lines.

The tools that read this file to help my workflow have been
minimally adjusted.  I am hoping that the updates to them I made
were enough to make the format tweak not to negatively affect me,
and so far things are going smoothly, but I may find some corner
cases later. Knock wood...

-- >8 --

To: git@vger.kernel.org
Bcc: lwn@lwn.net
Subject: What's cooking in git.git (Sep 2012, #05; Thu, 13)
X-master-at: ce5cf6ffc6feb9fb4f9a50cdfa2f527fa119c94f
X-next-at: dd7cb6d65b94d88f3bfb9efefabd5818614bf587

What's cooking in git.git (Sep 2012, #05; Thu, 13)
--------------------------------------------------

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

***BLURB***

I'm planning to keep this cycle reasonably short and aim for tagging
the result as 1.8.0 at the end of 9th week, on October 21st, after
which I'd disappear for a few weeks.  http://tinyurl.com/gitCal is
where you can always find my rough tagging schedule at.

You can find the changes described here in the integration branches of the
repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

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

* jw/doc-commit-title (2012-09-13) 1 commit
 - Documentation: describe subject more precisely

 Update parts of document that talked about "first line of commit
 log" to say "title of commit" with definition of what that "title"
 is.

 Will merge to 'next' after eyeballing.


* nd/maint-diffstat-summary (2012-09-13) 1 commit
 - Revert diffstat summary back to English

 Earlier we made the diffstat summary line that shows the number of
 lines added/deleted localizable, but it was found irritating having
 to see them in various languages on a list whose discussion language
 is English.

 Breaks many tests which need to be fixed before moving forward.


* nd/fetch-status-alignment (2012-09-12) 2 commits
 - [FIXUP] %.*s width must be int, not size_t
 - fetch: align per-ref summary report in UTF-8 locales

 Will merge to 'next' after squashing the fix-up.


* mv/cherry-pick-s (2012-09-13) 1 commit
 . cherry-pick: don't forget -s on failure


* jc/maint-log-grep-all-match (2012-09-13) 3 commits
 - log: document use of multiple commit limiting options
 - log --grep/--author: honor --all-match honored for multiple --grep patterns
 - grep: teach --debug option to dump the parse tree

 Fix a long-standing bug in "git log --grep" when multiple "--grep"
 are used together with "--all-match" and "--author" or "--committer".

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

* ph/credential-refactor (2012-09-02) 5 commits
 - wincred: port to generic credential helper
 - Merge branch 'ef/win32-cred-helper' into ph/credential-refactor
 - osxkeychain: port to generic credential helper implementation
 - gnome-keyring: port to generic helper implementation
 - contrib: add generic credential helper

 Attempts to refactor to share code among OSX keychain, Gnome keyring
 and Win32 credential helpers.


* jc/maint-name-rev (2012-09-04) 7 commits
 - describe --contains: use "name-rev --weight"
 - name-rev --weight: tests and documentation
 - name-rev --weight: cache the computed weight in notes
 - name-rev --weight: trivial optimization
 - name-rev: --weight option
 - name_rev: clarify the logic to assign a new tip-name to a commit
 - name-rev: lose unnecessary typedef

 "git name-rev" names the given revision based on a ref that can be
 reached in the smallest number of steps from the rev, but that is
 not useful when the caller wants to know which tag is the oldest one
 that contains the rev.  This teaches a new mode to the command that
 uses the oldest ref among those which contain the rev.

 I am not sure if this is worth it; for one thing, even with the help
 from notes-cache, it seems to make the "describe --contains" even
 slower. Also the command will be unusably slow for a user who does
 not have a write access (hence unable to create or update the
 notes-cache).

 Needs another round to at least find a better name for the option,
 and possibly a cheaper but still better than the current "close to
 the tip" heuristics.


* ms/contrib-thunderbird-updates (2012-08-31) 2 commits
 - [SQUASH] minimum fixup
 - Thunderbird: fix appp.sh format problems

 Update helper to send out format-patch output using Thunderbird.
 Seems to have design regression for silent users.


* as/check-ignore (2012-09-02) 10 commits
 . fixup: decl-after-stmt etc.
 . Add git-check-ignore
 . Provide free_directory() for reclaiming dir_struct memory
 . Extract some useful pathspec handling code from builtin/add.c into a library
 . For each exclude pattern, store information about where it came from
 . dir.c: refactor excluded() and path_excluded()
 . dir.c: refactor excluded_from_list()
 . dir.c: rename cryptic 'which' variable to more consistent name
 . Improve documentation and comments regarding directory traversal API
 . Update directory listing API doc to match code

 Will be rerolled.


* jx/test-real-path (2012-08-27) 1 commit
 - test: set the realpath of CWD as TRASH_DIRECTORY

 Running tests with the "trash" directory elsewhere with the "--root"
 option did not work well if the directory was specified by a symbolic
 link pointing at it.

 Seems broken as it makes $(pwd) and TRASH_DIRECTORY inconsistent.
 Needs rerolling.


* jc/maint-push-refs-all (2012-08-27) 2 commits
 - get_fetch_map(): tighten checks on dest refs
 - [BROKEN] fetch/push: allow refs/*:refs/*

 Allows pushing and fetching everything including refs/stash.
 This is broken (see the log message there).

 Not ready.


* er/doc-fast-import-done (2012-08-22) 1 commit
 - fast-import: document the --done option

 Parked in 'pu' in case ESR responds with "Sorry, forgot to sign-off".


* jc/add-delete-default (2012-08-13) 1 commit
 - git add: notice removal of tracked paths by default

 "git add dir/" updated modified files and added new files, but does
 not notice removed files, which may be "Huh?" to some users.  They
 can of course use "git add -A dir/", but why should they?

 Resurrected from graveyard, as I thought it was a worthwhile thing
 to do in the longer term.

 Waiting for comments.


* tx/relative-in-the-future (2012-08-16) 2 commits
 - date: show relative dates in the future
 - date: refactor the relative date logic from presentation

 Not my itch; rewritten an earlier submission by Tom Xue into
 somewhat more maintainable form, though it breaks existing i18n.

 Waiting for a voluteer to fix it up.
 Otherwise may discard.


* tg/index-v5 (2012-08-17) 13 commits
 . p0002-index.sh: add perf test for the index formats
 . update-index.c: rewrite index when index-version is given
 . Write resolve-undo data for index-v5
 . Write index-v5 cache-tree data
 . Write index-v5
 . Read cache-tree in index-v5
 . Read resolve-undo data
 . Read index-v5
 . Make in-memory format aware of stat_crc
 . Add documentation of the index-v5 file format
 . t2104: Don't fail for index versions other than [23]
 . read-cache.c: Re-read index if index file changed
 . Move index v2 specific functions to their own file

 A GSoC project.  Was waiting for comments from mentors and
 stakeholders, but nothing seems to be happening, other than breakage
 fixes on Cygwin.  May discard.


* mz/rebase-range (2012-07-18) 7 commits
 . rebase (without -p): correctly calculate patches to rebase
 . rebase -p: don't request --left-right only to ignore left side
 . rebase -p: use --cherry-mark for todo file
 . git-rebase--interactive.sh: look up subject in add_pick_line
 . git-rebase--interactive: group all $preserve_merges code
 . git-rebase--interactive.sh: extract function for adding "pick" line
 . git-rebase--am.sh: avoid special-casing --keep-empty

 Expecting a reroll.

 Performance concerns from Windows folks.  Also the series lacks
 proper sign-offs.


* mb/remote-default-nn-origin (2012-07-11) 6 commits
 - Teach get_default_remote to respect remote.default.
 - Test that plain "git fetch" uses remote.default when on a detached HEAD.
 - Teach clone to set remote.default.
 - Teach "git remote" about remote.default.
 - Teach remote.c about the remote.default configuration setting.
 - Rename remote.c's default_remote_name static variables.

 When the user does not specify what remote to interact with, we
 often attempt to use 'origin'.  This can now be customized via a
 configuration variable.

 Expecting a reroll.

 "The first remote becomes the default" bit is better done as a
 separate step.


* jc/split-blob (2012-04-03) 6 commits
 - chunked-object: streaming checkout
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - packfile: use varint functions

 Not ready.

 I finished the streaming checkout codepath, but as explained in
 127b177 (bulk-checkin: support chunked-object encoding, 2011-11-30),
 these are still early steps of a long and painful journey. At least
 pack-objects and fsck need to learn the new encoding for the series
 to be usable locally, and then index-pack/unpack-objects needs to
 learn it to be used remotely.

 Given that I heard a lot of noise that people want large files, and
 that I was asked by somebody at GitTogether'11 privately for an
 advice on how to pay developers (not me) to help adding necessary
 support, I am somewhat dissapointed that the original patch series
 that was sent long time ago still remains here without much comments
 and updates from the developer community. I even made the interface
 to the logic that decides where to split chunks easily replaceable,
 and I deliberately made the logic in the original patch extremely
 stupid to entice others, especially the "bup" fanbois, to come up
 with a better logic, thinking that giving people an easy target to
 shoot for, they may be encouraged to help out. The plan is not
 working :-<.

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

* dg/run-command-child-cleanup (2012-09-11) 1 commit
  (merged to 'next' on 2012-09-12 at aa5f9e2)
 + run-command.c: fix broken list iteration in clear_child_for_cleanup

 The code to wait for subprocess and remove it from our internal queue
 wasn't quite right.

 Will merge to 'master' as part of the seventh batch.


* jc/maint-blame-no-such-path (2012-09-11) 2 commits
 - blame: allow "blame file" in the middle of a conflicted merge
 - blame $path: avoid getting fooled by case insensitive filesystems

 "git blame MAKEFILE" run in a history that has "Makefile" but not
 "MAKEFILE" should say "No such file MAKEFILE in HEAD", but got
 confused on a case insensitive filesystem and failed to do so.

 Even during a conflicted merge, "git blame $path" always meant to
 blame uncommitted changes to the "working tree" version; make it
 more useful by showing cleanly merged parts as coming from the other
 branch that is being merged.

 Will merge to 'next'.


* sl/autoconf (2012-09-11) 2 commits
  (merged to 'next' on 2012-09-12 at 6ebe199)
 + build: don't duplicate substitution of make variables
 + build: improve GIT_CONF_SUBST signature

 Reduces repetition in configure.ac.

 Will merge to 'master' as part of the seventh batch.


* cn/branch-set-upstream-to (2012-09-11) 2 commits
  (merged to 'next' on 2012-09-12 at e162318)
 + completion: complete branch name for "branch --set-upstream-to="
 + completion: add --set-upstream-to and --unset-upstream

 Finishing touches to the recently graduated topic to introduce
 "git branch --set-upstream-to" option.

 Will merge to 'master' as part of the seventh batch.


* jc/ll-merge-binary-ours (2012-09-12) 3 commits
  (merged to 'next' on 2012-09-12 at 9a7a6b3)
 + ll-merge: warn about inability to merge binary files only when we can't
 + attr: "binary" attribute should choose built-in "binary" merge driver
 + merge: teach -Xours/-Xtheirs to binary ll-merge driver

 "git merge -Xtheirs" did not help content-level merge of binary
 files; it should just take their version.  Also "*.jpg binary" in
 the attributes did not imply they should use the binary ll-merge
 driver.

 Will merge to 'master' as part of the seventh batch.


* jc/mailinfo-RE (2012-09-09) 1 commit
  (merged to 'next' on 2012-09-12 at 131edbf)
 + mailinfo: strip "RE: " prefix

 We strip the prefix from "Re: subject" and also from a less common
 "re: subject", but left even less common "RE: subject" intact.

 Will merge to 'master' as part of the seventh batch.


* mh/string-list (2012-09-12) 6 commits
 - api-string-list.txt: initialize the string_list the easy way
 - string_list: add a function string_list_longest_prefix()
 - string_list: add a new function, string_list_remove_duplicates()
 - string_list: add a new function, filter_string_list()
 - string_list: add two new functions for splitting strings
 - string_list: add function string_list_append_nodup()
 (this branch is used by mh/fetch-filter-refs.)

 Will merge to 'next'.


* pw/p4-submit-conflicts (2012-09-10) 12 commits
 - git-p4: add submit --conflict option and config varaiable
 - git p4: add submit --prepare-p4-only option
 - git p4: add submit --dry-run option
 - git p4: accept -v for --verbose
 - git p4: revert deleted files after submit cancel
 - git p4: rearrange submit template construction
 - git p4: test clean-up after failed submit, fix added files
 - git p4: standardize submit cancel due to unchanged template
 - git p4: move conflict prompt into run, add [q]uit input
 - git p4: remove submit failure options [a]pply and [w]rite
 - git p4: gracefully fail if some commits could not be applied
 - git p4 test: remove bash-ism of combined export/assignment

 Waiting for comments.


* mh/abspath (2012-09-10) 9 commits
  (merged to 'next' on 2012-09-11 at 5e29b53)
 + t0060: split absolute path test in two to exercise some of it on Windows
 + t0060: verify that real_path() removes extra slashes
 + real_path(): properly handle nonexistent top-level paths
 + t0060: verify that real_path() works correctly with absolute paths
 + real_path(): reject the empty string
 + t0060: verify that real_path() fails if passed the empty string
 + absolute_path(): reject the empty string
 + t0060: verify that absolute_path() fails if passed the empty string
 + t0060: move tests of real_path() from t0000 to here

 Will merge to 'master' as part of the sixth batch.


* nd/i18n-status (2012-09-06) 1 commit
  (merged to 'next' on 2012-09-11 at 7cfa224)
 + status: remove i18n legos

 Will merge to 'master' as part of the sixth batch.


* sn/ls-remote-get-url-doc (2012-09-07) 1 commit
  (merged to 'next' on 2012-09-11 at 9d09780)
 + ls-remote: document the '--get-url' option

 Will merge to 'master' as part of the sixth batch.


* dj/fetch-all-tags (2012-09-07) 1 commit
  (merged to 'next' on 2012-09-11 at 083a029)
 + fetch --all: pass --tags/--no-tags through to each remote

 "git fetch --all", when passed "--no-tags", did not honor the
 "--no-tags" option while fetching from individual remotes (the same
 issue existed with "--tags", but combination "--all --tags" makes
 much less sense than "--all --no-tags").

 Will merge to 'master' as part of the sixth batch.


* rj/tap-fix (2012-09-02) 6 commits
  (merged to 'next' on 2012-09-11 at 4104358)
 + test-lib.sh: Suppress the "passed all ..." message if no tests run
 + test-lib.sh: Add check for invalid use of 'skip_all' facility
 + test-lib.sh: Fix some shell coding style violations
 + t4016-*.sh: Skip all tests rather than each test
 + t3902-*.sh: Skip all tests rather than each test
 + t3300-*.sh: Fix a TAP parse error

 Will merge to 'master' as part of the sixth batch.


* jc/xprm-generation (2012-09-04) 1 commit
 - test-generation: compute generation numbers and clock skews


* rj/path-cleanup (2012-09-04) 5 commits
  (merged to 'next' on 2012-09-11 at 9e8da84)
 + Call mkpathdup() rather than xstrdup(mkpath(...))
 + Call git_pathdup() rather than xstrdup(git_path("..."))
 + path.c: Use vsnpath() in the implementation of git_path()
 + path.c: Don't discard the return value of vsnpath()
 + path.c: Remove the 'git_' prefix from a file scope function

 Will merge to 'master' as part of the sixth batch.


* rs/archive-zip-utf8 (2012-09-04) 1 commit
  (merged to 'next' on 2012-09-11 at 3b1f071)
 + archive-zip: support UTF-8 paths

 Need help from people on platforms on which Zip matters to see
 compatiblity with other people's zip implementations.

 Will merge to 'master' as part of the sixth batch.


* nd/checkout-option-parsing-fix (2012-09-11) 3 commits
  (merged to 'next' on 2012-09-11 at 3d3ef13)
 + checkout: reorder option handling
 + checkout: move more parameters to struct checkout_opts
 + checkout: pass "struct checkout_opts *" as const pointer

 The option parsing of "git checkout" had error checking, dwim and
 defaulting missing options, all mixed in the code, and issuing an
 appropriate error message with useful context was getting harder.
 Reorganize the code and allow giving a proper diagnosis when the
 user says "git checkout -b -t foo bar" (e.g. "-t" is not a good name
 for a branch).

 Will merge to 'master' as part of the sixth batch.


* mh/fetch-filter-refs (2012-09-12) 14 commits
 - fetch-pack: eliminate spurious error messages
 - cmd_fetch_pack(): simplify computation of return value
 - fetch-pack: report missing refs even if no existing refs were received
 - cmd_fetch_pack(): return early if finish_connect() fails
 - filter_refs(): simplify logic
 - filter_refs(): build refs list as we go
 - filter_refs(): delete matched refs from sought list
 - fetch_pack(): update sought->nr to reflect number of unique entries
 - filter_refs(): do not check the same sought_pos twice
 - Change fetch_pack() and friends to take string_list arguments
 - fetch_pack(): reindent function decl and defn
 - Rename static function fetch_pack() to http_fetch_pack()
 - t5500: add tests of fetch-pack --all --depth=N $URL $REF
 - t5500: add tests of error output for missing refs
 (this branch uses mh/string-list.)

 Code simplification and clarification.

 Will merge to 'next'.


* jl/submodule-rm (2012-08-27) 1 commit
 - Teach rm to remove submodules unless they contain a git directory

 "git rm submodule" cannot blindly remove a submodule directory as
 its working tree may have local changes, and worse yet, it may even
 have its repository embedded in it.  Teach it some special cases
 where it is safe to remove a submodule, specifically, when there is
 no local changes in the submodule working tree, and its repository
 is not embedded in its working tree but is elsewhere and uses the
 gitfile mechanism to point at it.

 Replacement sent but was still iffy around conflicted merge cases.


* fa/remote-svn (2012-08-28) 16 commits
 - Add a test script for remote-svn
 - remote-svn: add marks-file regeneration
 - Add a svnrdump-simulator replaying a dump file for testing
 - remote-svn: add incremental import
 - remote-svn: Activate import/export-marks for fast-import
 - Create a note for every imported commit containing svn metadata
 - vcs-svn: add fast_export_note to create notes
 - Allow reading svn dumps from files via file:// urls
 - remote-svn, vcs-svn: Enable fetching to private refs
 - When debug==1, start fast-import with "--stats" instead of "--quiet"
 - Add documentation for the 'bidi-import' capability of remote-helpers
 - Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability
 - Add argv_array_detach and argv_array_free_detached
 - Add svndump_init_fd to allow reading dumps from arbitrary FDs
 - Add git-remote-testsvn to Makefile and .gitignore
 - Implement a remote helper for svn in C
 (this branch is used by fa/vcs-svn.)

 A GSoC project.  Looked promising.
 Waiting for comments from mentors and stakeholders.


* fa/vcs-svn (2012-08-28) 4 commits
 - vcs-svn: remove repo_tree
 - vcs-svn/svndump: rewrite handle_node(), begin|end_revision()
 - vcs-svn/svndump: restructure node_ctx, rev_ctx handling
 - svndump: move struct definitions to .h
 (this branch uses fa/remote-svn.)

 A GSoC project.  Looked promising.
 Waiting for comments from mentors and stakeholders.


* jk/no-more-pre-exec-callback (2012-06-05) 1 commit
 - pager: drop "wait for output to run less" hack

 (Originally merged to 'next' on 2012-07-23)

 Will defer until the end of the 2012.
 while waiting for older "less" to go extinct.

Re: Suggestions for "What's cooking"

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Friday, September 14, 2012 3:29 AM
> Andrew Ardill <andrew.ardill@gmail.com> writes:
>
>> On 14 September 2012 04:06, Junio C Hamano <gitster@pobox.com> wrote:
>>> Andrew Ardill <andrew.ardill@gmail.com> writes:
>>>
>>>> <short-branch-description>
>>>>   <long-branch-description>
>>>>   <notes>
>>>>   <next-steps>
>>>>   * <branch-name> (<creation-date>) <number-of-commits>
>>>>     (<merge-status>)
>>>>    [list-of-commits]
>>>>     (<branch-usage>)
>>>
>>> I do not see how it makes any sense to have the "This is where the
>>> section begins with, and its name is this" line in the middle of a
>>> block indented in such a way.  Care to explain?
>>
>> I'm not quite sure what aspect you are referring to,...
>
> Just this part, as I do not have much time.  Here is your reordered
> one I will reject:
>
>  A > jc/maint-blame-no-such-path
>    >   "git blame MAKEFILE" run in a history that has "Makefile" but 
> not
>    >   "MAKEFILE" should say "No such file MAKEFILE in HEAD", but got
>    >   confused on a case insensitive filesystem.
>    >
>  B >   * jc/maint-blame-no-such-path (2012-09-10) 1 commit
>    >    - blame $path: avoid getting fooled by case insensitive 
> filesystems
>
> I was noting that B which *is* formatted as a header line (it EVEN
> has a leading asterisk to make it clear that it begins something
> new) is in the middle, and you added a redundant A that is not even
> marked clearly as a header line.
>
Are we all working with Black text on a White background? (or is it vice 
versa) as this changes which bits of emphasis the eye will pick up. I'm 
reading the emails as black text against a white background.

I find that for black text, in a block format, that one does not notice 
any special inital character, such as the '*', when it is part of a 
rectangular block. In fact I feel I tend to, if anything, down grade 
text begining with special characters as being bullet points below some 
main block text. Hence my suggestion to have either a visual break 
(extra line above), or a block indent (extra left hand space).

Changing the contrast to white text on a black background totally 
changes what the eye/brain will see/notice [$dayjob is electro-optic 
vision systems where contrast inversion is a standard requirement for 
that reason]. It maybe that we are seeing different personal effects 
because of our set-ups.

Re: Suggestions for "What's cooking"

[Michael Haggerty]

On 09/14/2012 06:47 AM, Junio C Hamano wrote:
> So here is how tonight's "What's cooking" may look like with extra
> indentation and blank lines. [...]

I find this much more readable than the old format.  Thanks!

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: Suggestions for "What's cooking"

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Friday, September 14, 2012 5:47 AM
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> I've played with both and have prepared patches to Reintegrate and
>> cook (both in the 'todo' branch).  Will play with the changes a bit
>> more and then decide.
> 
> So here is how tonight's "What's cooking" may look like with extra
> indentation and blank lines.
> 
> The tools that read this file to help my workflow have been
> minimally adjusted.  I am hoping that the updates to them I made
> were enough to make the format tweak not to negatively affect me,
> and so far things are going smoothly, but I may find some corner
> cases later. Knock wood...
> 
> -- >8 --
<snip>

+1. It looks good even when printed with a proportional width font.