What's cooking in git.git (May 2013, #09; Wed, 29)

What's cooking in git.git (May 2013, #09; Wed, 29)

[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'.

Usually I avoid issuing "What's cooking" back-to-back, but because I
will be offline for a few days, here is a snapshot of tonight's
status.

Some topics that have been cooking in 'next' from the previous cycle
have now graduated to 'master', so the RelNotes have been updated.

Nothing new in 'next', and it has not been rewound yet, which will
hopefully happen this weekend, and after that post 1.8.3 cycle
really starts.

Also a rather serious regression on path-exclusion logic (most
notably seen in .gitignore) has been found and quickly patched (it
hasn't been merged to 'master' yet, though).

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

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

* as/check-ignore (2013-04-29) 6 commits
  (merged to 'next' on 2013-04-30 at 646931f)
 + t0008: use named pipe (FIFO) to test check-ignore streaming
  (merged to 'next' on 2013-04-21 at 7515aa8)
 + Documentation: add caveats about I/O buffering for check-{attr,ignore}
 + check-ignore: allow incremental streaming of queries via --stdin
 + check-ignore: move setup into cmd_check_ignore()
 + check-ignore: add -n / --non-matching option
 + t0008: remove duplicated test fixture data

 Enhance "check-ignore" (1.8.2 update) to work more like "check-attr"
 over bidi-pipes.


* fc/transport-helper-error-reporting (2013-05-10) 12 commits
  (merged to 'next' on 2013-05-10 at 2a9af4b)
 + transport-helper: fix remote helper namespace regression
 + test: remote-helper: add missing and
  (merged to 'next' on 2013-04-25 at 3358f1a)
 + t5801: "VAR=VAL shell_func args" is forbidden
  (merged to 'next' on 2013-04-22 at 5ba6467)
 + transport-helper: update remote helper namespace
 + transport-helper: trivial code shuffle
 + transport-helper: warn when refspec is not used
 + transport-helper: clarify pushing without refspecs
 + transport-helper: update refspec documentation
 + transport-helper: clarify *:* refspec
 + transport-helper: improve push messages
 + transport-helper: mention helper name when it dies
 + transport-helper: report errors properly
 (this branch is tangled with js/transport-helper-error-reporting-fix.)

 Update transport helper to report errors and maintain ref hierarchy
 used to keep track of remote helper state better.


* jc/prune-all (2013-04-25) 4 commits
  (merged to 'next' on 2013-04-26 at 97a7387)
 + prune: introduce OPT_EXPIRY_DATE() and use it
  (merged to 'next' on 2013-04-22 at b00ccf6)
 + api-parse-options.txt: document "no-" for non-boolean options
 + git-gc.txt, git-reflog.txt: document new expiry options
 + date.c: add parse_expiry_date()
 (this branch is used by mh/packed-refs-various.)

 We used the approxidate() parser for "--expire=<timestamp>" options
 of various commands, but it is better to treat --expire=all and
 --expire=now a bit more specially than using the current timestamp.
 Update "git gc" and "git reflog" with a new parsing function for
 expiry dates.


* jh/checkout-auto-tracking (2013-04-21) 8 commits
  (merged to 'next' on 2013-04-22 at 2356700)
 + glossary: Update and rephrase the definition of a remote-tracking branch
 + branch.c: Validate tracking branches with refspecs instead of refs/remotes/*
 + t9114.2: Don't use --track option against "svn-remote"-tracking branches
 + t7201.24: Add refspec to keep --track working
 + t3200.39: tracking setup should fail if there is no matching refspec.
 + checkout: Use remote refspecs when DWIMming tracking branches
 + t2024: Show failure to use refspec when DWIMming remote branch names
 + t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'

 Updates "git checkout foo" that DWIMs the intended "upstream" and
 turns it into "git checkout -t -b foo remotes/origin/foo" to
 correctly take existing remote definitions into account.  The
 remote "origin" may be what uniquely map its own branch to
 remotes/some/where/foo but that some/where may not be "origin".


* jk/lookup-object-prefer-latest (2013-05-02) 1 commit
  (merged to 'next' on 2013-05-06 at cc59dcc)
 + lookup_object: prioritize recently found objects

 Optimizes object lookup when the object hashtable starts to become
 crowded.


* jk/subtree-do-not-push-if-split-fails (2013-05-01) 1 commit
  (merged to 'next' on 2013-05-06 at 81bdf37)
 + contrib/subtree: don't delete remote branches if split fails

 "git subtree" (in contrib/) had one codepath with loose error
 checks to lose data at the remote side.


* jk/test-output (2013-05-06) 3 commits
  (merged to 'next' on 2013-05-06 at 7c03af3)
 + t/Makefile: don't define TEST_RESULTS_DIRECTORY recursively
  (merged to 'next' on 2013-05-01 at 63827c9)
 + test output: respect $TEST_OUTPUT_DIRECTORY
 + t/Makefile: fix result handling with TEST_OUTPUT_DIRECTORY

 When TEST_OUTPUT_DIRECTORY setting is used, it was handled somewhat
 inconsistently between the test framework and t/Makefile, and logic
 to summarize the results looked at a wrong place.


* jn/config-ignore-inaccessible (2013-04-15) 1 commit
  (merged to 'next' on 2013-05-07 at 4f09e24)
 + config: allow inaccessible configuration under $HOME

 When $HOME is misconfigured to point at an unreadable directory, we
 used to complain and die. This loosens the check.


* js/transport-helper-error-reporting-fix (2013-04-28) 13 commits
  (merged to 'next' on 2013-04-29 at 8cc4bb8)
 + git-remote-testgit: build it to run under $SHELL_PATH
 + git-remote-testgit: further remove some bashisms
 + git-remote-testgit: avoid process substitution
  (merged to 'next' on 2013-04-25 at 3358f1a)
 + t5801: "VAR=VAL shell_func args" is forbidden
  (merged to 'next' on 2013-04-22 at 5ba6467)
 + transport-helper: update remote helper namespace
 + transport-helper: trivial code shuffle
 + transport-helper: warn when refspec is not used
 + transport-helper: clarify pushing without refspecs
 + transport-helper: update refspec documentation
 + transport-helper: clarify *:* refspec
 + transport-helper: improve push messages
 + transport-helper: mention helper name when it dies
 + transport-helper: report errors properly
 (this branch is tangled with fc/transport-helper-error-reporting.)

 Finishing touches to fc/transport-helper-error-reporting topic.


* mh/fetch-into-shallow (2013-05-02) 2 commits
  (merged to 'next' on 2013-05-03 at 3fadc61)
 + t5500: add test for fetching with an unknown 'shallow'
  (merged to 'next' on 2013-04-29 at a167d3e)
 + upload-pack: ignore 'shallow' lines with unknown obj-ids


* mh/packed-refs-various (2013-05-01) 33 commits
  (merged to 'next' on 2013-05-01 at e527153)
 + refs: handle the main ref_cache specially
 + refs: change do_for_each_*() functions to take ref_cache arguments
 + pack_one_ref(): do some cheap tests before a more expensive one
 + pack_one_ref(): use write_packed_entry() to do the writing
 + pack_one_ref(): use function peel_entry()
 + refs: inline function do_not_prune()
 + pack_refs(): change to use do_for_each_entry()
 + refs: use same lock_file object for both ref-packing functions
 + pack_one_ref(): rename "path" parameter to "refname"
 + pack-refs: merge code from pack-refs.{c,h} into refs.{c,h}
 + pack-refs: rename handle_one_ref() to pack_one_ref()
 + refs: extract a function write_packed_entry()
 + repack_without_ref(): write peeled refs in the rewritten file
 + t3211: demonstrate loss of peeled refs if a packed ref is deleted
 + refs: change how packed refs are deleted
 + search_ref_dir(): return an index rather than a pointer
 + repack_without_ref(): silence errors for dangling packed refs
 + t3210: test for spurious error messages for dangling packed refs
 + refs: change the internal reference-iteration API
 + refs: extract a function peel_entry()
 + peel_ref(): fix return value for non-peelable, not-current reference
 + peel_object(): give more specific information in return value
 + refs: extract function peel_object()
 + refs: extract a function ref_resolves_to_object()
 + repack_without_ref(): use function get_packed_ref()
 + peel_ref(): use function get_packed_ref()
 + get_packed_ref(): return a ref_entry
 + do_for_each_ref_in_dirs(): remove dead code
 + refs: define constant PEELED_LINE_LENGTH
 + refs: document how current_ref is used
 + refs: document do_for_each_ref() and do_one_ref()
 + refs: document the fields of struct ref_value
 + refs: document flags constants REF_*
 (this branch uses jc/prune-all.)

 Updates reading and updating packed-refs file, correcting corner
 case bugs.


* rj/sparse (2013-04-28) 10 commits
  (merged to 'next' on 2013-05-01 at 649e16c)
 + sparse: Fix mingw_main() argument number/type errors
 + compat/mingw.c: Fix some sparse warnings
 + compat/win32mmap.c: Fix some sparse warnings
 + compat/poll/poll.c: Fix a sparse warning
 + compat/win32/pthread.c: Fix a sparse warning
 + compat/unsetenv.c: Fix a sparse warning
 + compat/nedmalloc: Fix compiler warnings on linux
 + compat/nedmalloc: Fix some sparse warnings
 + compat/fnmatch/fnmatch.c: Fix a sparse error
 + compat/regex/regexec.c: Fix some sparse warnings

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

* ap/diff-ignore-blank-lines (2013-05-29) 1 commit
 - diff: add --ignore-blank-lines option

 "git diff" learned a mode that ignores hunks whose change consists
 only of additions and removals of blank lines, which is the same as
 "diff -B" (ignore blank lines) of GNU diff.


* fc/show-branch-in-rebase-am (2013-05-29) 1 commit
 - prompt: fix for simple rebase

 The bash prompt code (in contrib/) displayed the name of the branch
 being rebased when "rebase -i/-m/-p" modes are in use, but not the
 plain vanilla "rebase".

 Will merge to 'next'.


* kb/status-ignored-optim-2 (2013-05-29) 1 commit
 - dir.c: fix ignore processing within not-ignored directories

 Fix 1.8.3 regressions in the .gitignore path exclusion logic.


* ks/difftool-dir-diff-copy-fix (2013-05-29) 1 commit
 - difftool --dir-diff: allow changing any clean working tree file

 "difftool --dir-diff" did not copy back changes made by the
 end-user in the diff tool backend to the working tree in some
 cases.


* rr/push-head (2013-05-29) 3 commits
 - push: make push.default = current use resolved HEAD
 - push: fail early with detached HEAD and current
 - push: factor out the detached HEAD error message

 "git push $there HEAD:branch" did not resolve HEAD early enough, so
 it was easy to flip it around while push is still going on and push
 out a branch that the user did not originally intended when the
 command was started.

 Will merge to 'next'.


* sb/archive-zip-double-assignment-fix (2013-05-29) 1 commit
 - archive-zip:write_zip_entry: Remove second reset of size variable to zero.

 Will merge to 'next'.

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

* mg/more-textconv (2013-05-10) 7 commits
  (merged to 'next' on 2013-05-21 at 701cdb7)
 + grep: honor --textconv for the case rev:path
 + grep: allow to use textconv filters
 + t7008: demonstrate behavior of grep with textconv
 + cat-file: do not die on --textconv without textconv filters
 + show: honor --textconv for blobs
 + diff_opt: track whether flags have been set explicitly
 + t4030: demonstrate behavior of show with textconv

 Make "git grep" and "git show" pay attention to --textconv when
 dealing with blob objects.

 I thought this was pretty well designed and executed, but it seems
 there are some doubts on the list.


* mh/multimail (2013-04-21) 1 commit
 - git-multimail: a replacement for post-receive-email

 Waiting for the initial history to pull from.
 $gmane/223564


* jc/format-patch (2013-04-22) 2 commits
 - format-patch: --inline-single
 - format-patch: rename "no_inline" field

 A new option to send a single patch to the standard output to be
 appended at the bottom of a message.  I personally have no need for
 this, but it was easy enough to cobble together.  Tests, docs and
 stripping out more MIMEy stuff are left as exercises to interested
 parties.

 Not ready for inclusion.

 Will discard unless we hear from anybody who is interested in
 tying its loose ends.


* jk/gitweb-utf8 (2013-04-08) 4 commits
 - gitweb: Fix broken blob action parameters on blob/commitdiff pages
 - gitweb: Don't append ';js=(0|1)' to external links
 - gitweb: Make feed title valid utf8
 - gitweb: Fix utf8 encoding for blob_plain, blobdiff_plain, commitdiff_plain, and patch

 Various fixes to gitweb.

 Waiting for a reroll after a review.
 Will discard unless we hear from anybody who is interested in
 tying its loose ends.

* jk/commit-info-slab (2013-04-19) 3 commits
 - commit-slab: introduce a macro to define a slab for new type
 - commit-slab: avoid large realloc
 - commit: allow associating auxiliary info on-demand
 (this branch is used by jc/show-branch.)

 Technology demonstration to show a way we could use unbound number
 of flag bits on commit objects.

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

* rj/mingw-cygwin (2013-05-08) 2 commits
 - cygwin: Remove the CYGWIN_V15_WIN32API build variable
 - mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE

 Update build for Cygwin 1.[57].  Torsten Bögershausen reports that
 this is fine with Cygwin 1.7 ($gmane/225824) so let's try moving it
 ahead.
 
 Will merge to 'next'.


* rr/rebase-autostash (2013-05-29) 7 commits
 - rebase: implement --[no-]autostash and rebase.autostash
 - rebase --merge: return control to caller, for housekeeping
 - rebase -i: return control to caller, for housekeeping
 - am: return control to caller, for housekeeping
 - rebase: prepare to do generic housekeeping
 - rebase -i: don't error out if $state_dir already exists
 - am: tighten a conditional that checks for $dotest

 Will merge to 'next'.


* nd/urls-doc-no-file-hyperlink-fix (2013-05-24) 1 commit
 - urls.txt: avoid auto converting to hyperlink

 Will merge to 'next'.


* cb/log-follow-with-combined (2013-05-28) 1 commit
 - fix segfault with git log -c --follow

 Will merge to 'next'.


* fc/cleanups (2013-05-28) 3 commits
 - test: rebase: fix --interactive test
 - test: trivial cleanups
 - remote: trivial style cleanup

 Will merge to 'next'.


* fc/makefile (2013-05-26) 5 commits
 - build: do not install git-remote-testpy
 - build: add NO_INSTALL variable
 - build: cleanup using $<
 - build: cleanup using $^
 - build: trivial simplification
 (this branch is used by fc/remote-helpers-use-specified-python.)

 Will merge to 'next'.


* fc/remote-helpers-use-specified-python (2013-05-28) 4 commits
 - remote-helpers: add exec-path links
 - remote-helpers: allow direct test execution
 - remote-helpers: rename tests
 - remote-helpers: generate scripts
 (this branch uses fc/makefile.)

 I do not particularly think the second from the bottom is a good
 change, but it takes the remainder of the series hostage.
 Will hopefully be rerolled without it.


* fc/send-email-chainreplyto-warning (2013-05-28) 1 commit
 - send-email: remove warning about unset chainreplyto

 An overdue removal od "behaviour changed at 1.7.0; if you were
 living in a cave, here is what you can adjust to it" message.

 Will merge to 'next'.


* ks/difftool-dirdiff-copy-all (2013-05-28) 1 commit
 - difftool --dir-diff: always use identical working tree file

 "difftool --dir-diff" populates a temporary directory with files,
 inviting the user to modify them in place, but in some cases we
 weren't expecting the user to modify them and did not take the
 change back to the working tree.

 Will merge to 'next'.


* nd/prune-packed-dryrun-verbose (2013-05-28) 1 commit
 - prune-packed: avoid implying "1" is DRY_RUN in prune_packed_objects()

 Will merge to 'next'.


* rj/mingw-compat-st-mode-bits (2013-05-28) 1 commit
 - path: Fix a sparse warning

 Will merge to 'next'.


* rs/commit-m-no-edit (2013-05-28) 1 commit
 - commit: don't start editor if empty message is given with -m

 Will merge to 'next'.


* xq/credential-osxkeychain (2013-05-28) 1 commit
 - credential-osxkeychain: support more protocols

 Will merge to 'next'.


* jc/core-checkstat (2013-05-06) 1 commit
 - deprecate core.statinfo at Git 2.0 boundary
 (this branch is used by jc/core-checkstat-2.0.)

 Will merge to 'next'.


* tr/line-log (2013-04-22) 13 commits
  (merged to 'next' on 2013-04-22 at 8f2c1de)
 + git-log(1): remove --full-line-diff description
  (merged to 'next' on 2013-04-21 at cd92620)
 + line-log: fix documentation formatting
  (merged to 'next' on 2013-04-15 at 504559e)
 + log -L: improve comments in process_all_files()
 + log -L: store the path instead of a diff_filespec
 + log -L: test merge of parallel modify/rename
 + t4211: pass -M to 'git log -M -L...' test
  (merged to 'next' on 2013-04-05 at 5afb00c)
 + log -L: fix overlapping input ranges
 + log -L: check range set invariants when we look it up
  (merged to 'next' on 2013-04-01 at 5be920c)
 + Speed up log -L... -M
 + log -L: :pattern:file syntax to find by funcname
 + Implement line-history search (git log -L)
 + Export rewrite_parents() for 'log -L'
 + Refactor parse_loc

 Will merge to 'master'.


* mh/reflife (2013-05-28) 25 commits
 - refs: document the lifetime of the args passed to each_ref_fn
 - register_ref(): make a copy of the bad reference SHA-1
 - exclude_existing(): set existing_refs.strdup_strings
 - string_list_add_refs_by_glob(): add a comment about memory management
 - string_list_add_one_ref(): rename first parameter to "refname"
 - show_head_ref(): rename first parameter to "refname"
 - show_head_ref(): do not shadow name of argument
 - add_existing(): do not retain a reference to sha1
 - do_fetch(): clean up existing_refs before exiting
 - do_fetch(): reduce scope of peer_item
 - object_array_entry: fix memory handling of the name field
 - find_first_merges(): remove unnecessary code
 - find_first_merges(): initialize merges variable using initializer
 - fsck: don't put a void*-shaped peg in a char*-shaped hole
 - object_array_remove_duplicates(): rewrite to reduce copying
 - revision: use object_array_filter() in implementation of gc_boundary()
 - object_array: add function object_array_filter()
 - revision: split some overly-long lines
 - cmd_diff(): make it obvious which cases are exclusive of each other
 - cmd_diff(): rename local variable "list" -> "entry"
 - cmd_diff(): use an object_array for holding trees
 - builtin_diff_tree(): make it obvious that function wants two entries
 - add_rev_cmdline(): make a copy of the name argument
 - fetch: make own copies of refnames
 - describe: make own copy of refname

 Define memory ownership and lifetime rules for what for-each-ref
 feeds to its callbacks (in short, "you do not own it, so make a
 copy if you want to keep it").

 Will merge to 'next'.


* th/bisect-skip-report-range-fix (2013-05-22) 1 commit
 - bisect: Fix log output for multi-parent skip ranges

 Fix for an additional bisect log comments.

 Will merge to 'next'.


* mm/mediawiki-https-fail-message (2013-05-29) 1 commit
 - git-remote-mediawiki: better error message when HTTP(S) access fails

 Hint users when https:// connection failed to check the
 certificate; it is a good hint if we assumie that it is common
 error for the end users to make.

 Will merge to 'next'.


* tg/maint-zsh-svn-remote-prompt (2013-05-22) 1 commit
 - prompt: fix show upstream with svn and zsh

 zsh prompt script that borrowed from bash prompt script did not
 work due to slight differences in array variable notation between
 these two shells.

 Will merge to 'next'.


* tr/push-no-verify-doc (2013-05-23) 1 commit
 - Document push --no-verify

 "git push --[no-]verify" was not documented.

 Will merge to 'next'.


* dm/unbash-subtree (2013-05-21) 1 commit
 - contrib/git-subtree: Use /bin/sh interpreter instead of /bin/bash

 It turns out that git-subtree script does not have to be run with
 bash.

 Will merge to 'next'.


* fc/transport-helper-no-refspec (2013-05-21) 2 commits
 - transport-helper: check if the dry-run is supported
 - transport-helper: barf when user tries old:new

 With "export" remote-helper protocol, (1) a push that tries to
 update a remote ref whose name is different from the pushing side
 does not work yet, and (2) the helper may not know how to do
 --dry-run, so detect such problematic cases and disable them for
 now.

 Will merge to 'next'.


* rr/die-on-missing-upstream (2013-05-22) 2 commits
 - sha1_name: fix error message for @{<N>}, @{<date>}
 - sha1_name: fix error message for @{u}

 When a reflog notation is used for implicit "current branch", we
 did not say which branch and worse said "branch ''".

 Will merge to 'next'.


* jc/show-branch (2013-05-21) 5 commits
 - show-branch: use commit slab to represent bitflags of arbitrary width
 - show-branch.c: remove "all_mask"
 - show-branch.c: abstract out "flags" operation
 - show-branch.c: lift all_mask/all_revs to a global static
 - show-branch.c: update comment style
 (this branch uses jk/commit-info-slab.)

 Waiting for the final step to lift the hard-limit before sending it out.


* fc/remote-bzr (2013-05-28) 8 commits
 - remote-bzr: add fallback check for a partial clone
 - remote-bzr: reorganize the way 'wanted' works
 - remote-bzr: trivial cleanups
 - remote-bzr: change global repo
 - remote-bzr: delay cloning/pulling
 - remote-bzr: simplify get_remote_branch()
 - remote-bzr: fix for files with spaces
 - remote-bzr: recover from failed clones

 Will merge to 'next'.


* jx/clean-interactive (2013-05-22) 15 commits
 - test: add t7301 for git-clean--interactive
 - git-clean: add documentation for interactive git-clean
 - git-clean: add ask each interactive action
 - git-clean: add select by numbers interactive action
 - git-clean: add filter by pattern interactive action
 - git-clean: use a git-add-interactive compatible UI
 - git-clean: add colors to interactive git-clean
 - git-clean: show items of del_list in columns
 - git-clean: add support for -i/--interactive
 - git-clean: refactor git-clean into two phases
 - Refactor write_name_quoted_relative, remove unused params
 - Refactor quote_path_relative, remove unused params
 - quote.c: remove path_relative, use relative_path instead
 - path.c: refactor relative_path(), not only strip prefix
 - test: add test cases for relative_path

 Still under discussion, e.g. $gmane/225139.


* tr/test-v-and-v-subtest-only (2013-05-16) 6 commits
 - test-lib: support running tests under valgrind in parallel
 - test-lib: allow prefixing a custom string before "ok N" etc.
 - test-lib: valgrind for only tests matching a pattern
 - test-lib: verbose mode for only tests matching a pattern
 - test-lib: refactor $GIT_SKIP_TESTS matching
 - test-lib: enable MALLOC_* for the actual tests

 Allows N instances of tests run in parallel, each running 1/N parts
 of the test suite under Valgrind, to speed things up.

 The tip one may be useful in practice but is a tad ugly ;-)

 There seem to be some miscounting by toggling the verbose/valgrind
 mode at wrong places?  Cf. $gmane/225735


* rh/merge-options-doc-fix (2013-05-16) 1 commit
  (merged to 'next' on 2013-05-21 at f76baf3)
 + Documentation/merge-options.txt: restore `-e` option

 Will merge to 'master'.


* rr/zsh-color-prompt (2013-05-17) 3 commits
 - prompt: colorize ZSH prompt
 - prompt: factor out gitstring coloring logic
 - prompt: introduce GIT_PS1_STATESEPARATOR

 Will merge to 'next'.


* an/diff-index-doc (2013-05-20) 1 commit
  (merged to 'next' on 2013-05-21 at 6e1459b)
 + Documentation/diff-index: mention two modes of operation

 Will merge to 'master'.


* fc/contrib-related (2013-05-20) 1 commit
 - Add new git-related helper to contrib

 Waiting the design review for this earliest part to settle.


* mc/describe-first-parent (2013-05-20) 1 commit
  (merged to 'next' on 2013-05-21 at 2e02ffc)
 + describe: Add --first-parent option

 Will merge to 'master'.


* rs/tar-tests (2013-05-20) 6 commits
  (merged to 'next' on 2013-05-21 at aed12a7)
 + t5000: test long filenames
 + t5000: simplify tar-tree tests
 + t5000: use check_tar for prefix test
 + t5000: factor out check_tar
 + t5000, t5003: create directories for extracted files lazily
 + t5000: integrate export-subst tests into regular tests

 Will merge to 'master'.


* da/darwin (2013-05-21) 4 commits
  (merged to 'next' on 2013-05-21 at 0497edb)
 + imap-send: eliminate HMAC deprecation warnings on Mac OS X
 + cache.h: eliminate SHA-1 deprecation warnings on Mac OS X
 + Makefile: add support for Apple CommonCrypto facility
 + Makefile: fix default regex settings on Darwin

 Newer MacOS X encourages the programs to compile and link with
 their CommonCrypto, not with OpenSSL.

 Will merge to 'master'.


* fc/macos-x-clipped-write (2013-05-17) 1 commit
  (merged to 'next' on 2013-05-21 at 510b0dd)
 + compate/clipped-write.c: large write(2) fails on Mac OS X/XNU

 Mac OS X does not like to write(2) more than INT_MAX number of
 bytes.

 Will merge to 'master'.


* fc/remote-hg (2013-05-28) 50 commits
 - remote-hg: add support for --force
 - remote-hg: add support for --dry-run
 - remote-hg: check if a fetch is needed
 - remote-hg: trivial cleanup
 - remote-helpers: improve marks usage
 - remote-hg: add check_push() helper
 - remote-hg: add setup_big_push() helper
 - remote-hg: remove files before modifications
 - remote-hg: improve lightweight tag author
 - remote-hg: use remote 'default' not local one
 - remote-hg: improve branch listing
 - remote-hg: simplify branch_tip()
 - remote-hg: check diverged bookmarks
 - remote-hg: pass around revision refs
 - remote-hg: implement custom checkheads()
 - remote-hg: implement custom push()
 - remote-hg: only update necessary revisions
 - remote-hg: force remote bookmark push selectively
 - remote-hg: reorganize bookmark handling
 - remote-hg: add test for failed double push
 - remote-hg: add test for big push
 - remote-hg: add test for new bookmark special
 - remote-hg: add test for bookmark diverge
 - remote-hg: add test for diverged push
 - remote-hg: add test to push new bookmark
 - remote-hg: add remote tests
 - remote-hg: update bookmarks when using a remote
 - remote-hg: add check_bookmark() test helper
 - remote-bzr: simplify test checks
 - remote-hg: add tests for 'master' bookmark
 - remote-hg: always point HEAD to master
 - remote-hg: improve progress calculation
 - remote-hg: trivial cleanups
 - remote-hg: ensure remote rebasing works
 - remote-hg: upgrade version 1 marks
 - remote-hg: switch from revisions to SHA-1 noteids
 - remote-hg: add version checks to the marks
 - remote-hg: improve node traversing
 - remote-hg: shuffle some code
 - remote-hg: use a shared repository store
 - remote-hg: load all extensions
 - remote-hg: test: simplify previous branch checkout
 - remote-helpers: test: simplify remote URLs
 - remote-helpers: tests: general improvements
 - remote-helpers: test: cleanup style
 - remote-helpers: test: cleanup white-spaces
 - remote-hg: trivial reorganization
 - remote-hg: test: be a little more quiet
 - remote-hg: tests: fix hg merge
 - remote-helpers: tests: use python directly

 Will merge to 'next'.


* hv/config-from-blob (2013-05-12) 5 commits
 - do not die when error in config parsing of buf occurs
 - teach config --blob option to parse config from database
 - config: make parsing stack struct independent from actual data source
 - config: drop cf validity check in get_next_char()
 - config: factor out config file stack management

 Will see a resubmit.
 $gmane/223964


* jc/t5551-posix-sed-bre (2013-05-12) 1 commit
  (merged to 'next' on 2013-05-21 at d597ca1)
 + t5551: do not use unportable sed '\+'

 POSIX fix for a test script.

 Will merge to 'master'.


* jk/fetch-always-update-tracking (2013-05-28) 5 commits
  (merged to 'next' on 2013-05-28 at c6abf3f)
 + fetch: don't try to update unfetched tracking refs
  (merged to 'next' on 2013-05-21 at f7e4b16)
 + fetch: opportunistically update tracking refs
 + refactor "ref->merge" flag
 + fetch/pull doc: untangle meaning of bare <ref>
 + t5510: start tracking-ref tests from a known state

 "git fetch origin master" unlike "git fetch origin" or "git fetch"
 does not update "refs/remotes/origin/master" and it was an early
 design decision to keep the update of remote tracking branches
 predictable, but in practice it turns out that people find it more
 convenient to opportunisticly update them whenever we have a
 chance, and we have been updating them when we run "git push" which
 already breaks the original "predictability" anyway.

 Will merge to 'master'.


* nd/clone-connectivity-shortcut (2013-05-28) 4 commits
 - clone: open a shortcut for connectivity check
 - index-pack: remove dead code (it should never happen)
 - fetch-pack: prepare updated shallow file before fetching the pack
 - clone: let the user know when check_everything_connected is run

 Will merge to 'next'.


* kb/ancestry-path-threedots (2013-05-16) 2 commits
  (merged to 'next' on 2013-05-21 at 5d75bd3)
 + revision.c: treat A...B merge bases as if manually specified
 + t6019: demonstrate --ancestry-path A...B breakage
 (this branch is used by kb/full-history-compute-treesame-carefully-2.)

 "git log --ancestry-path A...B" did not work as expected, as it did
 not pay attention to the fact that the merge base between A and B
 was the bottom of the range being specified.

 Will merge to 'master'.


* kb/full-history-compute-treesame-carefully-2 (2013-05-16) 15 commits
 - revision.c: make default history consider bottom commits
 - revision.c: don't show all merges for --parents
 - revision.c: discount side branches when computing TREESAME
 - revision.c: add BOTTOM flag for commits
 - simplify-merges: drop merge from irrelevant side branch
 - simplify-merges: never remove all TREESAME parents
 - t6012: update test for tweaked full-history traversal
 - revision.c: Make --full-history consider more merges
 - Documentation: avoid "uninteresting"
 - rev-list-options.txt: correct TREESAME for P
 - t6111: add parents to tests
 - t6111: allow checking the parents as well
 - t6111: new TREESAME test set
 - t6019: test file dropped in -s ours merge
 - decorate.c: compact table when growing
 (this branch uses kb/ancestry-path-threedots.)

 Major update to a very core part of the system to improve culling
 of irrelevant parents while traversing a mergy history.

 Will not be a 1.8.3 material, but is an important topic.

 Will merge to 'next'.


* jc/strbuf-branchname-fix (2013-05-16) 1 commit
  (merged to 'next' on 2013-05-21 at 5db8872)
 + strbuf_branchname(): do not double-expand @{-1}~22

 "git merge @{-1}~22" was rewritten to "git merge frotz@{1}~22"
 incorrectly when your previous branch was "frotz" (it should be
 rewritten to "git merge frotz~22" instead).

 Will merge to 'master'.


* mk/combine-diff-context-horizon-fix (2013-05-15) 1 commit
  (merged to 'next' on 2013-05-21 at f462c02)
 + combine-diff.c: Fix output when changes are exactly 3 lines apart

 "git diff -c -p" was not showing a deleted line from a hunk when
 another hunk immediately begins where the earlier one ends.

 Will merge to 'master'.


* mm/color-auto-default (2013-05-15) 2 commits
 - make color.ui default to 'auto'
 - config: refactor management of color.ui's default value

 Flip the default for color.ui to 'auto', which is what many
 tutorials recommend new users to do.  The updated code claims the
 switch happened at Git 2.0 in the past tense, but we might want to
 expedite it, as this change is not all that important to deserve a
 major version bump.

 I'd vote for merging this without waiting for 2.0.  Comments?
 Waiting for a reroll.


* tr/coverage (2013-05-13) 4 commits
  (merged to 'next' on 2013-05-21 at 8480007)
 + coverage: build coverage-untested-functions by default
 + coverage: set DEFAULT_TEST_TARGET to avoid using prove
 + coverage: do not delete .gcno files before building
 + coverage: split build target into compile and test

 Update the test coverage support that was left to bitrot for some
 time.

 Will merge to 'master'.


* jh/shorten-refname (2013-05-07) 4 commits
 - t1514: refname shortening is done after dereferencing symbolic refs
 - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin
 - t1514: Demonstrate failure to correctly shorten "refs/remotes/origin/HEAD"
 - t1514: Add tests of shortening refnames in strict/loose mode

 When remotes/origin/HEAD is not a symbolic ref, "rev-parse
 --abbrev-ref remotes/origin/HEAD" ought to show "origin", not
 "origin/HEAD", which is fixed with this series (if it is a symbolic
 ref that points at remotes/origin/something, then it should show
 "origin/something" and it already does).

 Expecting a reroll, as an early part of a larger series.


* nd/clone-local-with-colon (2013-05-07) 1 commit
  (merged to 'next' on 2013-05-21 at 3e293bd)
 + clone: allow cloning local paths with colons in them

 "git clone foo/bar:baz" cannot be a request to clone from a remote
 over git-over-ssh specified in the scp style.  Detect this case and
 clone from a local repository at "foo/bar:baz".

 Will merge to 'master'.


* nd/warn-ambiguous-object-name (2013-05-29) 1 commit
 - get_sha1: warn about full or short object names that look like refs

 "git cmd <name>", when <name> happens to be a 40-hex string,
 directly uses the 40-hex string as an object name, even if a ref
 "refs/<some hierarchy>/<name>" exists.  This disambiguation order
 is unlikely to change, but we should warn about the ambiguity just
 like we warn when more than one refs/ hierachies share the same
 name.


* fc/zsh-leftover-bits (2013-05-29) 3 commits
  (merged to 'next' on 2013-05-29 at b367b4e)
 + completion: zsh: improve bash script loading
  (merged to 'next' on 2013-05-21 at 1b84ec1)
 + completion: synchronize zsh wrapper
 + completion: cleanup zsh wrapper

 Will merge to 'master'.


* rs/empty-archive (2013-05-09) 2 commits
  (merged to 'next' on 2013-05-21 at 3e9e288)
 + t5004: resurrect original empty tar archive test
 + t5004: avoid using tar for checking emptiness of archive

 Fixes tests added in 1.8.2 era that are broken on BSDs.

 Will merge to 'master'.


* vv/help-unknown-ref (2013-05-08) 2 commits
  (merged to 'next' on 2013-05-21 at 77a2941)
 + merge: use help_unknown_ref()
 + help: add help_unknown_ref()

 Detect "git merge foo" that might have meant "git merge origin/foo"
 and give an error message that is more specific than "foo is not
 something we can merge".

 Will merge to 'master'.


* fc/fast-export-persistent-marks (2013-05-07) 3 commits
  (merged to 'next' on 2013-05-21 at 10b8214)
 + fast-{import,export}: use get_sha1_hex() to read from marks file
 + fast-export: don't parse commits while reading marks file
 + fast-export: do not parse non-commit objects while reading marks file

 Optimization for fast-export by avoiding unnecessarily resolving
 arbitrary object name and parsing object when only presence and
 type information is necessary, etc.

 Will merge to 'master'.


* jk/packed-refs-race (2013-05-06) 4 commits
 - for_each_ref: load all loose refs before packed refs
 - get_packed_refs: reload packed-refs file when it changes
 - add a stat_validity struct
 - resolve_ref: close race condition for packed refs

 What is the status of this thing?


* fc/at-head (2013-05-08) 13 commits
 - sha1_name: compare variable with constant, not constant with variable
 - Add new @ shortcut for HEAD
 - sha1_name: refactor reinterpret()
 - sha1_name: check @{-N} errors sooner
 - sha1_name: reorganize get_sha1_basic()
 - sha1_name: don't waste cycles in the @-parsing loop
 - sha1_name: remove unnecessary braces
 - sha1_name: remove no-op
 - tests: at-combinations: @{N} versus HEAD@{N}
 - tests: at-combinations: increase coverage
 - tests: at-combinations: improve nonsense()
 - tests: at-combinations: check ref names directly
 - tests: at-combinations: simplify setup

 Instead of typing four capital letters "HEAD", you can say "@"
 instead.

 Will merge to 'next'.


* fc/completion (2013-04-27) 9 commits
  (merged to 'next' on 2013-05-21 at aba6ef1)
 + completion: remove __git_index_file_list_filter()
 + completion: add space after completed filename
 + completion: add hack to enable file mode in bash < 4
 + completion: refactor __git_complete_index_file()
 + completion: refactor diff_index wrappers
 + completion: use __gitcompadd for __gitcomp_file
 + completion; remove unuseful comments
 + completion: document tilde expansion failure in tests
 + completion: add file completion tests

 Will merge to 'master'.


* jk/submodule-subdirectory-ok (2013-04-24) 3 commits
  (merged to 'next' on 2013-04-24 at 6306b29)
 + submodule: fix quoting in relative_path()
  (merged to 'next' on 2013-04-22 at f211e25)
 + submodule: drop the top-level requirement
 + rev-parse: add --prefix option

 Allow various subcommands of "git submodule" to be run not from the
 top of the working tree of the superproject.

 Will be commented by Jens.


* jl/submodule-mv (2013-04-23) 5 commits
  (merged to 'next' on 2013-04-23 at c04f574)
 + submodule.c: duplicate real_path's return value
  (merged to 'next' on 2013-04-19 at 45ae3c9)
 + rm: delete .gitmodules entry of submodules removed from the work tree
 + Teach mv to update the path entry in .gitmodules for moved submodules
 + Teach mv to move submodules using a gitfile
 + Teach mv to move submodules together with their work trees

 "git mv A B" when moving a submodule A does "the right thing",
 inclusing relocating its working tree and adjusting the paths in
 the .gitmodules file.

 Will be commented by Jens.


* jn/add-2.0-u-A-sans-pathspec (2013-04-26) 1 commit
 - git add: -u/-A now affects the entire working tree

 Will cook in 'next' until Git 2.0.


* jc/core-checkstat-2.0 (2013-05-06) 1 commit
 - core.statinfo: remove as promised in Git 2.0
 (this branch uses jc/core-checkstat.)

 Will cook in 'next' until Git 2.0.


* jc/push-2.0-default-to-simple (2013-04-03) 1 commit
 - push: switch default from "matching" to "simple"

 Will cook in 'next' until Git 2.0.


* jc/add-2.0-ignore-removal (2013-04-22) 1 commit
 - git add <pathspec>... defaults to "-A"

 Updated endgame for "git add <pathspec>" that defaults to "--all"
 aka "--no-ignore-removal".

 Will cook in 'next' until Git 2.0.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Thomas Rast]

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

> * rr/die-on-missing-upstream (2013-05-22) 2 commits
>  - sha1_name: fix error message for @{<N>}, @{<date>}
>  - sha1_name: fix error message for @{u}
>
>  When a reflog notation is used for implicit "current branch", we
>  did not say which branch and worse said "branch ''".
>
>  Will merge to 'next'.

This interacts with the tests from

> * fc/at-head (2013-05-08) 13 commits

to fail valgrind in t1508 like so:

  ==22927== Invalid read of size 1
  ==22927==    at 0x4C2C71B: __GI_strnlen (mc_replace_strmem.c:377)
  ==22927==    by 0x567FF6B: vfprintf (in /lib64/libc-2.17.so)
  ==22927==    by 0x56AC194: vsnprintf (in /lib64/libc-2.17.so)
  ==22927==    by 0x4EAC39: vreportf (usage.c:12)
  ==22927==    by 0x4EACA4: die_builtin (usage.c:36)
  ==22927==    by 0x4EAEE0: die (usage.c:103)
  ==22927==    by 0x4D8C51: get_sha1_1 (sha1_name.c:542)
  ==22927==    by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
  ==22927==    by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
  ==22927==    by 0x4D99E1: get_sha1 (sha1_name.c:1124)
  ==22927==    by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
  ==22927==    by 0x4051B3: handle_internal_command (git.c:284)
  ==22927==  Address 0x5bebccb is 11 bytes inside a block of size 22 free'd
  ==22927==    at 0x4C2ACDA: free (vg_replace_malloc.c:468)
  ==22927==    by 0x4D8C37: get_sha1_1 (sha1_name.c:541)
  ==22927==    by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
  ==22927==    by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
  ==22927==    by 0x4D99E1: get_sha1 (sha1_name.c:1124)
  ==22927==    by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
  ==22927==    by 0x4051B3: handle_internal_command (git.c:284)
  ==22927==    by 0x4053E7: main (git.c:492)

I think it's enough to squash this little change; leaking some memory
immediately before die() is not too bad, especially if we're going to
pass real_ref+11 into die()...

diff --git i/sha1_name.c w/sha1_name.c
index 5ea16ff..a07558d 100644
--- i/sha1_name.c
+++ w/sha1_name.c
@@ -538,7 +538,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 					"back to %s.", len, str,
 					show_date(co_time, co_tz, DATE_RFC2822));
 			else {
-				free(real_ref);
 				die("Log for '%.*s' only has %d entries.",
 				    len, str, co_cnt);
 			}


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

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Ramkumar Ramachandra]

Thomas Rast wrote:
> diff --git i/sha1_name.c w/sha1_name.c
> index 5ea16ff..a07558d 100644
> --- i/sha1_name.c
> +++ w/sha1_name.c
> @@ -538,7 +538,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>                                         "back to %s.", len, str,
>                                         show_date(co_time, co_tz, DATE_RFC2822));
>                         else {
> -                               free(real_ref);
>                                 die("Log for '%.*s' only has %d entries.",
>                                     len, str, co_cnt);
>                         }

Ah, good catch.  Thanks.

Sorry I didn't notice this while writing the patch.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Jens Lehmann]

Am 30.05.2013 01:58, schrieb Junio C Hamano:
> * jl/submodule-mv (2013-04-23) 5 commits
>   (merged to 'next' on 2013-04-23 at c04f574)
>  + submodule.c: duplicate real_path's return value
>   (merged to 'next' on 2013-04-19 at 45ae3c9)
>  + rm: delete .gitmodules entry of submodules removed from the work tree
>  + Teach mv to update the path entry in .gitmodules for moved submodules
>  + Teach mv to move submodules using a gitfile
>  + Teach mv to move submodules together with their work trees
> 
>  "git mv A B" when moving a submodule A does "the right thing",
>  inclusing relocating its working tree and adjusting the paths in
>  the .gitmodules file.

There are only two issues I'm aware of:

*) When the .gitmodules file is already modified but unchanged
   running rm or mv on a submodule will stage those changes too.

*) There is a harmless but unnecessary double invocation of strlen()
   in the function (fixed by the diff below).

I plan to fix the first issue in another patch which would also get
rid of the second issue, as exactly that code would have to be touched
anyways.

Does that make sense?

----------8<-----------------
diff --git a/submodule.c b/submodule.c
index edfc23c..4670af7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -102,7 +102,7 @@ void stage_updated_gitmodules(void)
        struct cache_entry *ce;
        int namelen = strlen(".gitmodules");

-       pos = cache_name_pos(".gitmodules", strlen(".gitmodules"));
+       pos = cache_name_pos(".gitmodules", namelen);
        if (pos < 0) {
                warning(_("could not find .gitmodules in index"));
                return;

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Jens Lehmann]

Am 30.05.2013 01:58, schrieb Junio C Hamano:
> * jk/submodule-subdirectory-ok (2013-04-24) 3 commits
>   (merged to 'next' on 2013-04-24 at 6306b29)
>  + submodule: fix quoting in relative_path()
>   (merged to 'next' on 2013-04-22 at f211e25)
>  + submodule: drop the top-level requirement
>  + rev-parse: add --prefix option
> 
>  Allow various subcommands of "git submodule" to be run not from the
>  top of the working tree of the superproject.

The summary and status commands are looking good in this version
(they are now showing the submodule directory paths relative to
the current directory). Apart from that my other remarks from
gmane $221575 still seem to apply. And this series has only tests
for status, summary and add (and that just with an absolute URL),
I'd rather like to see a test for each submodule command (and a
relative add to) to document the desired behavior.

But I'm not sure if it's better to have another iteration of this
series or to address the open issues a follow-up series. Having
status, summary and add - at least with absolute URLs - lose the
toplevel requirement is already a huge improvement IMO. Opinions?

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Øystein Walle]

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

> * kb/status-ignored-optim-2 (2013-05-29) 1 commit
>  - dir.c: fix ignore processing within not-ignored directories
> 
>  Fix 1.8.3 regressions in the .gitignore path exclusion logic.

Hi,

I see that the Tested-by line in kb/status-ignored-optim-2 (3973cbd)
doesn't contain my e-mail address. I personally have no particular
preference whether it's there or not, but I noticed that it usually is
on Tested-by lines, so I'm wondering if I have slipped on some standard
practice. Would be useful to know in case I get the chance to help out
more :)

Thanks,
Øsse

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[John Keeping]

On Thu, May 30, 2013 at 09:23:40PM +0200, Jens Lehmann wrote:
> Am 30.05.2013 01:58, schrieb Junio C Hamano:
> > * jk/submodule-subdirectory-ok (2013-04-24) 3 commits
> >   (merged to 'next' on 2013-04-24 at 6306b29)
> >  + submodule: fix quoting in relative_path()
> >   (merged to 'next' on 2013-04-22 at f211e25)
> >  + submodule: drop the top-level requirement
> >  + rev-parse: add --prefix option
> > 
> >  Allow various subcommands of "git submodule" to be run not from the
> >  top of the working tree of the superproject.
> 
> The summary and status commands are looking good in this version
> (they are now showing the submodule directory paths relative to
> the current directory). Apart from that my other remarks from
> gmane $221575 still seem to apply. And this series has only tests
> for status, summary and add (and that just with an absolute URL),
> I'd rather like to see a test for each submodule command (and a
> relative add to) to document the desired behavior.

To summarize what I think are the outstanding issues from your email:

* Should '$sm_path' be relative in "submodule foreach"?
* "submodule add" with a relative path
* "submodule init" initializes all submodules
* Tests

The current version does make '$sm_path' relative in "submodule
foreach", although it's hard to spot because we have to leave doing so
until right before the "eval".

I'm not sure what you mean about "submodule add" - the new version
treats the "path" argument as relative (providing it is not an absolute
path).  The "repository" argument is not changed by running from a
subdirectory but I think that's correct since it is documented as being
relative to the superproject's origin repository.

"submodule init" is behaving in the same way as "deinit" - if you say
"submodule init ." then it will only initialize submodules below the
current directory.  The difference is that "deinit" dies if it is not
given any arguments whereas "init" will initialize everything from the
top level down.  I'm not sure whether to change this; given the
direction "git add -u" is heading in for 2.0 I think the current
behaviour is the most consistent with the rest of Git.

> But I'm not sure if it's better to have another iteration of this
> series or to address the open issues a follow-up series. Having
> status, summary and add - at least with absolute URLs - lose the
> toplevel requirement is already a huge improvement IMO. Opinions?

I think the only thing outstanding is tests.  I'm happy to add those as
a follow-up or in a re-roll.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Junio C Hamano]

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

> Am 30.05.2013 01:58, schrieb Junio C Hamano:
>> * jl/submodule-mv (2013-04-23) 5 commits
>>   (merged to 'next' on 2013-04-23 at c04f574)
>>  + submodule.c: duplicate real_path's return value
>>   (merged to 'next' on 2013-04-19 at 45ae3c9)
>>  + rm: delete .gitmodules entry of submodules removed from the work tree
>>  + Teach mv to update the path entry in .gitmodules for moved submodules
>>  + Teach mv to move submodules using a gitfile
>>  + Teach mv to move submodules together with their work trees
>> 
>>  "git mv A B" when moving a submodule A does "the right thing",
>>  inclusing relocating its working tree and adjusting the paths in
>>  the .gitmodules file.
>
> There are only two issues I'm aware of:
>
> *) When the .gitmodules file is already modified but unchanged
>    running rm or mv on a submodule will stage those changes too.
>
> *) There is a harmless but unnecessary double invocation of strlen()
>    in the function (fixed by the diff below).
>
> I plan to fix the first issue in another patch which would also get
> rid of the second issue, as exactly that code would have to be touched
> anyways.
>
> Does that make sense?

In general I think whatever you think that makes sense in this area
would make sense ;-).

I do not feel confident that I am reading what you mean by "already
modified but unchanged" right.  Do you mean the working tree version
is different from both HEAD and the index (HEAD and the index may or
may not match and that does not change the situation)?  Assuming
that is the case, i.e. the situation you are worried about is:

    When ".gitmodules" has a local modification the user chose not
    to "add" yet.  Then the user does "git rm/mv" on a submodule
    that is unrelated to the submodule whose setting the user has
    changes for.

I am curious what the plan is to "fix" this.  An obviously safe
thing to do is to error out with a "You have local modification;
please 'git add .gitmodules' first." but if that advice/suggestion
is always the right course of action for the user, it invites "then
why doesn't Git do so for me?", which would in turn support that it
is not an issue in the first place (it deserves to be mentioned in a
warning, "adding your local modifications together with change
needed for rm/mv", though).

I think in the ideal world, you may want to apply the change needed
for rm/mv to the version in the index, and then update the working
tree version by doing a 3-way merge. We already know that eventually
we would need a merge driver that is specific to the file format
that git-config uses, possibly even taking an advantage of the
knowledge of not just the file format but also the semantics of
individual variables [*1*], so we may want to keep it in mind that a
three-way merge would be the eventual goal, while settling on an
"error out on local mod" just like "git checkout anotherbranch" used
to always stop (before we taught the "-m" option to it) when a local
change needed a 3-way merge to be carried along to the new branch.

So my gut feeling of the "fix" at this point in the evolution of the
program may be to error out with a message like "You have local
changes to .gitmodules; please stash it before moving or removing".

[Footnote]

*1* I think all the variables in .gitmodules are single-valued, so
    the original submodule.dir.path's value was "dir", the local
    modification by the user was to make it "folder", and rm wants
    to delete an unrelated submodule.mod.* altogether, we can apply
    the usual 3-way merge policy per variable basis to update
    submodule.dir.pah to "folder".

    A more general merge driver to handle git-config format files
    would have a way to be told that some variables are additive
    with the -X<driver-specific-option> mechanism.  When variable
    foo.bar is specified as a multi-valued set of variables, the
    original has a single foo.bar="xyzzy", one side adds a
    foo.bar="frotz while the other side modifies the original to
    foo.bar="nitfol", the ideal way for such a merge driver to
    operate is to leave two definitions (xyzzy will be gone and
    frotz and nitfol remain).

    But I highly suspect that would need a much larger change to the
    configuration file parser and writer that is totally out of
    scope with this change, and that is why my recommendation at
    this point is just to follow the example of pre-"-m" era of "git
    checkout anotherbranch".


> ----------8<-----------------
> diff --git a/submodule.c b/submodule.c
> index edfc23c..4670af7 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -102,7 +102,7 @@ void stage_updated_gitmodules(void)
>         struct cache_entry *ce;
>         int namelen = strlen(".gitmodules");
>
> -       pos = cache_name_pos(".gitmodules", strlen(".gitmodules"));
> +       pos = cache_name_pos(".gitmodules", namelen);
>         if (pos < 0) {
>                 warning(_("could not find .gitmodules in index"));
>                 return;

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Junio C Hamano]

Thomas Rast <trast@inf.ethz.ch> writes:

> This interacts with the tests from
>
>> * fc/at-head (2013-05-08) 13 commits
>
> to fail valgrind in t1508 like so:
>
>   ==22927== Invalid read of size 1
>   ==22927==    at 0x4C2C71B: __GI_strnlen (mc_replace_strmem.c:377)
>   ==22927==    by 0x567FF6B: vfprintf (in /lib64/libc-2.17.so)
>   ==22927==    by 0x56AC194: vsnprintf (in /lib64/libc-2.17.so)
>   ==22927==    by 0x4EAC39: vreportf (usage.c:12)
>   ==22927==    by 0x4EACA4: die_builtin (usage.c:36)
>   ==22927==    by 0x4EAEE0: die (usage.c:103)
>   ==22927==    by 0x4D8C51: get_sha1_1 (sha1_name.c:542)
>   ==22927==    by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
>   ==22927==    by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
>   ==22927==    by 0x4D99E1: get_sha1 (sha1_name.c:1124)
>   ==22927==    by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
>   ==22927==    by 0x4051B3: handle_internal_command (git.c:284)
>   ==22927==  Address 0x5bebccb is 11 bytes inside a block of size 22 free'd
>   ==22927==    at 0x4C2ACDA: free (vg_replace_malloc.c:468)
>   ==22927==    by 0x4D8C37: get_sha1_1 (sha1_name.c:541)
>   ==22927==    by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
>   ==22927==    by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
>   ==22927==    by 0x4D99E1: get_sha1 (sha1_name.c:1124)
>   ==22927==    by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
>   ==22927==    by 0x4051B3: handle_internal_command (git.c:284)
>   ==22927==    by 0x4053E7: main (git.c:492)
>
> I think it's enough to squash this little change; leaking some memory
> immediately before die() is not too bad, especially if we're going to
> pass real_ref+11 into die()...

Good catch, thanks.  when !len and real_ref is the current branch,
str just points into real_ref that is geting freed.

>
> diff --git i/sha1_name.c w/sha1_name.c
> index 5ea16ff..a07558d 100644
> --- i/sha1_name.c
> +++ w/sha1_name.c
> @@ -538,7 +538,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>  					"back to %s.", len, str,
>  					show_date(co_time, co_tz, DATE_RFC2822));
>  			else {
> -				free(real_ref);
>  				die("Log for '%.*s' only has %d entries.",
>  				    len, str, co_cnt);
>  			}

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[John Keeping]

On Fri, May 31, 2013 at 08:40:51PM +0100, John Keeping wrote:
> On Thu, May 30, 2013 at 09:23:40PM +0200, Jens Lehmann wrote:
> > Am 30.05.2013 01:58, schrieb Junio C Hamano:
> > > * jk/submodule-subdirectory-ok (2013-04-24) 3 commits
> > >   (merged to 'next' on 2013-04-24 at 6306b29)
> > >  + submodule: fix quoting in relative_path()
> > >   (merged to 'next' on 2013-04-22 at f211e25)
> > >  + submodule: drop the top-level requirement
> > >  + rev-parse: add --prefix option
> > > 
> > >  Allow various subcommands of "git submodule" to be run not from the
> > >  top of the working tree of the superproject.
> > 
> > The summary and status commands are looking good in this version
> > (they are now showing the submodule directory paths relative to
> > the current directory). Apart from that my other remarks from
> > gmane $221575 still seem to apply. And this series has only tests
> > for status, summary and add (and that just with an absolute URL),
> > I'd rather like to see a test for each submodule command (and a
> > relative add to) to document the desired behavior.
> 
> To summarize what I think are the outstanding issues from your email:
> 
> * Should '$sm_path' be relative in "submodule foreach"?
> * "submodule add" with a relative path
> * "submodule init" initializes all submodules
> * Tests
> 
> The current version does make '$sm_path' relative in "submodule
> foreach", although it's hard to spot because we have to leave doing so
> until right before the "eval".
> 
> I'm not sure what you mean about "submodule add" - the new version
> treats the "path" argument as relative (providing it is not an absolute
> path).  The "repository" argument is not changed by running from a
> subdirectory but I think that's correct since it is documented as being
> relative to the superproject's origin repository.
> 
> "submodule init" is behaving in the same way as "deinit" - if you say
> "submodule init ." then it will only initialize submodules below the
> current directory.  The difference is that "deinit" dies if it is not
> given any arguments whereas "init" will initialize everything from the
> top level down.  I'm not sure whether to change this; given the
> direction "git add -u" is heading in for 2.0 I think the current
> behaviour is the most consistent with the rest of Git.
> 
> > But I'm not sure if it's better to have another iteration of this
> > series or to address the open issues a follow-up series. Having
> > status, summary and add - at least with absolute URLs - lose the
> > toplevel requirement is already a huge improvement IMO. Opinions?
> 
> I think the only thing outstanding is tests.  I'm happy to add those as
> a follow-up or in a re-roll.

I started looking at this over the weekend but didn't get time to get
something ready to be submitted.  I did find a couple of issues in
cmd_foreach that make me think this topic should be dropped when "next"
is rewound and held in pu waiting for a re-roll.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> I started looking at this over the weekend but didn't get time to get
> something ready to be submitted.  I did find a couple of issues in
> cmd_foreach that make me think this topic should be dropped when "next"
> is rewound and held in pu waiting for a re-roll.

Thanks for a heads-up.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Jens Lehmann]

Am 02.06.2013 20:50, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Am 30.05.2013 01:58, schrieb Junio C Hamano:
>>> * jl/submodule-mv (2013-04-23) 5 commits
>>>   (merged to 'next' on 2013-04-23 at c04f574)
>>>  + submodule.c: duplicate real_path's return value
>>>   (merged to 'next' on 2013-04-19 at 45ae3c9)
>>>  + rm: delete .gitmodules entry of submodules removed from the work tree
>>>  + Teach mv to update the path entry in .gitmodules for moved submodules
>>>  + Teach mv to move submodules using a gitfile
>>>  + Teach mv to move submodules together with their work trees
>>>
>>>  "git mv A B" when moving a submodule A does "the right thing",
>>>  inclusing relocating its working tree and adjusting the paths in
>>>  the .gitmodules file.

<detailed discussion snipped>

> So my gut feeling of the "fix" at this point in the evolution of the
> program may be to error out with a message like "You have local
> changes to .gitmodules; please stash it before moving or removing".

Yeah, me too thinks that this is a sane short term solution (even
though a "git submodule add" currently happily stages any unstaged
modifications to the .gitmodules file too, that should not stop us
from doing better for rm and mv ;-).

And I also agree that in the long run the the git-config aware merge
driver together with the 3-way merge of a modified .gitmodules file
you described is the best solution. But I'd really like to complete
the recursive update before tackling that, so for now I just added
these two to the to-do list on my GitHub wiki page.

I'll resubmit this series with the strlen() fix and the erroring out
in case of unstaged modifications of the .gitmodules file as soon as
I find some time.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Jens Lehmann]

Am 31.05.2013 21:40, schrieb John Keeping:
> On Thu, May 30, 2013 at 09:23:40PM +0200, Jens Lehmann wrote:
>> Am 30.05.2013 01:58, schrieb Junio C Hamano:
>>> * jk/submodule-subdirectory-ok (2013-04-24) 3 commits
>>>   (merged to 'next' on 2013-04-24 at 6306b29)
>>>  + submodule: fix quoting in relative_path()
>>>   (merged to 'next' on 2013-04-22 at f211e25)
>>>  + submodule: drop the top-level requirement
>>>  + rev-parse: add --prefix option
>>>
>>>  Allow various subcommands of "git submodule" to be run not from the
>>>  top of the working tree of the superproject.
>>
>> The summary and status commands are looking good in this version
>> (they are now showing the submodule directory paths relative to
>> the current directory). Apart from that my other remarks from
>> gmane $221575 still seem to apply. And this series has only tests
>> for status, summary and add (and that just with an absolute URL),
>> I'd rather like to see a test for each submodule command (and a
>> relative add to) to document the desired behavior.
> 
> To summarize what I think are the outstanding issues from your email:
> 
> * Should '$sm_path' be relative in "submodule foreach"?
> * "submodule add" with a relative path
> * "submodule init" initializes all submodules
> * Tests
> 
> The current version does make '$sm_path' relative in "submodule
> foreach", although it's hard to spot because we have to leave doing so
> until right before the "eval".

Yes. If I read the code correctly the submodule is cd'ed in before
the foreach command is executed, so $sm_path should only be used for
displaying info about where the command is executed anyway. Looks
like your code is doing the right thing adjusting $sm_path to be
relative to the directory the user is in. But a test showing that
would really be nice ;-)

> I'm not sure what you mean about "submodule add" - the new version
> treats the "path" argument as relative (providing it is not an absolute
> path).  The "repository" argument is not changed by running from a
> subdirectory but I think that's correct since it is documented as being
> relative to the superproject's origin repository.

Sorry, I should have been more specific here. I saw that you did some
changes to make "submodule add" do the right thing with relative paths,
but the following change to t7406 does not work like I believe it
should but instead makes the test fail:
-------------------8<---------------------
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index a4ffea0..9766b9e 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
 test_expect_success 'submodule add places git-dir in superprojects git-dir' '
        (cd super &&
         mkdir deeper &&
-        git submodule add ../submodule deeper/submodule &&
+        (cd deeper &&
+         git submodule add ../../submodule submodule
+        ) &&
         (cd deeper/submodule &&
          git log > ../../expected
         ) &&
-------------------8<---------------------

> "submodule init" is behaving in the same way as "deinit" - if you say
> "submodule init ." then it will only initialize submodules below the
> current directory.  The difference is that "deinit" dies if it is not
> given any arguments whereas "init" will initialize everything from the
> top level down.  I'm not sure whether to change this; given the
> direction "git add -u" is heading in for 2.0 I think the current
> behaviour is the most consistent with the rest of Git.

I meant that both commands still print the submodule names from the
top-level directory, not the one the user is in.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[John Keeping]

On Mon, Jun 03, 2013 at 11:47:23PM +0200, Jens Lehmann wrote:
> Am 31.05.2013 21:40, schrieb John Keeping:
> > The current version does make '$sm_path' relative in "submodule
> > foreach", although it's hard to spot because we have to leave doing so
> > until right before the "eval".
> 
> Yes. If I read the code correctly the submodule is cd'ed in before
> the foreach command is executed, so $sm_path should only be used for
> displaying info about where the command is executed anyway. Looks
> like your code is doing the right thing adjusting $sm_path to be
> relative to the directory the user is in. But a test showing that
> would really be nice ;-)

Agreed.  I've also noticed that the legacy "path" variable hasn't been
adjusted and the printing of the module paths does not make them
relative.  I'll fix them in the next version.

> > I'm not sure what you mean about "submodule add" - the new version
> > treats the "path" argument as relative (providing it is not an absolute
> > path).  The "repository" argument is not changed by running from a
> > subdirectory but I think that's correct since it is documented as being
> > relative to the superproject's origin repository.
> 
> Sorry, I should have been more specific here. I saw that you did some
> changes to make "submodule add" do the right thing with relative paths,
> but the following change to t7406 does not work like I believe it
> should but instead makes the test fail:
> -------------------8<---------------------
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index a4ffea0..9766b9e 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
>  test_expect_success 'submodule add places git-dir in superprojects git-dir' '
>         (cd super &&
>          mkdir deeper &&
> -        git submodule add ../submodule deeper/submodule &&
> +        (cd deeper &&
> +         git submodule add ../../submodule submodule
> +        ) &&
>          (cd deeper/submodule &&
>           git log > ../../expected
>          ) &&
> -------------------8<---------------------

Ah, ok.  I think this case is problematic because the repository
argument is either relative to "remote.origin.url" or to the top of the
working tree if there is no "origin" remote.  I wonder if we should just
die when a relative path is given for the repository and we're not at
the top of the working tree.

> > "submodule init" is behaving in the same way as "deinit" - if you say
> > "submodule init ." then it will only initialize submodules below the
> > current directory.  The difference is that "deinit" dies if it is not
> > given any arguments whereas "init" will initialize everything from the
> > top level down.  I'm not sure whether to change this; given the
> > direction "git add -u" is heading in for 2.0 I think the current
> > behaviour is the most consistent with the rest of Git.
> 
> I meant that both commands still print the submodule names from the
> top-level directory, not the one the user is in.

Will fix.

Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Heiko Voigt]

On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote:
> > Sorry, I should have been more specific here. I saw that you did some
> > changes to make "submodule add" do the right thing with relative paths,
> > but the following change to t7406 does not work like I believe it
> > should but instead makes the test fail:
> > -------------------8<---------------------
> > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> > index a4ffea0..9766b9e 100755
> > --- a/t/t7406-submodule-update.sh
> > +++ b/t/t7406-submodule-update.sh
> > @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
> >  test_expect_success 'submodule add places git-dir in superprojects git-dir' '
> >         (cd super &&
> >          mkdir deeper &&
> > -        git submodule add ../submodule deeper/submodule &&
> > +        (cd deeper &&
> > +         git submodule add ../../submodule submodule
> > +        ) &&
> >          (cd deeper/submodule &&
> >           git log > ../../expected
> >          ) &&
> > -------------------8<---------------------
> 
> Ah, ok.  I think this case is problematic because the repository
> argument is either relative to "remote.origin.url" or to the top of the
> working tree if there is no "origin" remote.  I wonder if we should just
> die when a relative path is given for the repository and we're not at
> the top of the working tree.

Why not behave as if we are at the top of the working tree for relative
paths? If there is an origin remote thats fine. If there is no origin
remote you could warn that the path used is taken relative from the root
of the superproject during add. What do you think?

Cheers Heiko

Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[John Keeping]

On Tue, Jun 04, 2013 at 03:29:51PM +1000, Heiko Voigt wrote:
> On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote:
> > > Sorry, I should have been more specific here. I saw that you did some
> > > changes to make "submodule add" do the right thing with relative paths,
> > > but the following change to t7406 does not work like I believe it
> > > should but instead makes the test fail:
> > > -------------------8<---------------------
> > > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> > > index a4ffea0..9766b9e 100755
> > > --- a/t/t7406-submodule-update.sh
> > > +++ b/t/t7406-submodule-update.sh
> > > @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
> > >  test_expect_success 'submodule add places git-dir in superprojects git-dir' '
> > >         (cd super &&
> > >          mkdir deeper &&
> > > -        git submodule add ../submodule deeper/submodule &&
> > > +        (cd deeper &&
> > > +         git submodule add ../../submodule submodule
> > > +        ) &&
> > >          (cd deeper/submodule &&
> > >           git log > ../../expected
> > >          ) &&
> > > -------------------8<---------------------
> > 
> > Ah, ok.  I think this case is problematic because the repository
> > argument is either relative to "remote.origin.url" or to the top of the
> > working tree if there is no "origin" remote.  I wonder if we should just
> > die when a relative path is given for the repository and we're not at
> > the top of the working tree.
> 
> Why not behave as if we are at the top of the working tree for relative
> paths? If there is an origin remote thats fine. If there is no origin
> remote you could warn that the path used is taken relative from the root
> of the superproject during add. What do you think?

That's what the patch currently queued on "pu" does, which Jens wants to
change, isn't it?

Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Heiko Voigt]

On Tue, Jun 04, 2013 at 09:10:45AM +0100, John Keeping wrote:
> On Tue, Jun 04, 2013 at 03:29:51PM +1000, Heiko Voigt wrote:
> > On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote:
> > > > Sorry, I should have been more specific here. I saw that you did some
> > > > changes to make "submodule add" do the right thing with relative paths,
> > > > but the following change to t7406 does not work like I believe it
> > > > should but instead makes the test fail:
> > > > -------------------8<---------------------
> > > > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> > > > index a4ffea0..9766b9e 100755
> > > > --- a/t/t7406-submodule-update.sh
> > > > +++ b/t/t7406-submodule-update.sh
> > > > @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
> > > >  test_expect_success 'submodule add places git-dir in superprojects git-dir' '
> > > >         (cd super &&
> > > >          mkdir deeper &&
> > > > -        git submodule add ../submodule deeper/submodule &&
> > > > +        (cd deeper &&
> > > > +         git submodule add ../../submodule submodule
> > > > +        ) &&
> > > >          (cd deeper/submodule &&
> > > >           git log > ../../expected
> > > >          ) &&
> > > > -------------------8<---------------------
> > > 
> > > Ah, ok.  I think this case is problematic because the repository
> > > argument is either relative to "remote.origin.url" or to the top of the
> > > working tree if there is no "origin" remote.  I wonder if we should just
> > > die when a relative path is given for the repository and we're not at
> > > the top of the working tree.
> > 
> > Why not behave as if we are at the top of the working tree for relative
> > paths? If there is an origin remote thats fine. If there is no origin
> > remote you could warn that the path used is taken relative from the root
> > of the superproject during add. What do you think?
> 
> That's what the patch currently queued on "pu" does, which Jens wants to
> change, isn't it?

True I did not realize this when reading it the first time. But I think
we should still not die when in a subdirectory. After all this series is
trying to archive that the submodule command works in subdirectories
seamlessly right? So you probably want to translate a relative path
without "origin" remote given from a subdirectory to the superproject
level and use that. Then you do not have to die.

Cheers Heiko

Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[John Keeping]

On Tue, Jun 04, 2013 at 09:17:17PM +1000, Heiko Voigt wrote:
> On Tue, Jun 04, 2013 at 09:10:45AM +0100, John Keeping wrote:
> > On Tue, Jun 04, 2013 at 03:29:51PM +1000, Heiko Voigt wrote:
> > > On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote:
> > > > > Sorry, I should have been more specific here. I saw that you did some
> > > > > changes to make "submodule add" do the right thing with relative paths,
> > > > > but the following change to t7406 does not work like I believe it
> > > > > should but instead makes the test fail:
> > > > > -------------------8<---------------------
> > > > > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> > > > > index a4ffea0..9766b9e 100755
> > > > > --- a/t/t7406-submodule-update.sh
> > > > > +++ b/t/t7406-submodule-update.sh
> > > > > @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
> > > > >  test_expect_success 'submodule add places git-dir in superprojects git-dir' '
> > > > >         (cd super &&
> > > > >          mkdir deeper &&
> > > > > -        git submodule add ../submodule deeper/submodule &&
> > > > > +        (cd deeper &&
> > > > > +         git submodule add ../../submodule submodule
> > > > > +        ) &&
> > > > >          (cd deeper/submodule &&
> > > > >           git log > ../../expected
> > > > >          ) &&
> > > > > -------------------8<---------------------
> > > > 
> > > > Ah, ok.  I think this case is problematic because the repository
> > > > argument is either relative to "remote.origin.url" or to the top of the
> > > > working tree if there is no "origin" remote.  I wonder if we should just
> > > > die when a relative path is given for the repository and we're not at
> > > > the top of the working tree.
> > > 
> > > Why not behave as if we are at the top of the working tree for relative
> > > paths? If there is an origin remote thats fine. If there is no origin
> > > remote you could warn that the path used is taken relative from the root
> > > of the superproject during add. What do you think?
> > 
> > That's what the patch currently queued on "pu" does, which Jens wants to
> > change, isn't it?
> 
> True I did not realize this when reading it the first time. But I think
> we should still not die when in a subdirectory. After all this series is
> trying to archive that the submodule command works in subdirectories
> seamlessly right? So you probably want to translate a relative path
> without "origin" remote given from a subdirectory to the superproject
> level and use that. Then you do not have to die.

The problem is that sometimes you do want to adjust the path and
sometimes you don't.  Reading git-submodule(1), it says:

     This may be either an absolute URL, or (if it begins with ./ or
     ../), the location relative to the superproject’s origin
     repository.
     [snip]
     If the superproject doesn’t have an origin configured the
     superproject is its own authoritative upstream and the current
     working directory is used instead.

So I think it's quite reasonable to have a server layout that looks like
this:

    project
    |- libs
    |  |- libA
    |  `- libB
    |- core.git

and with only core.git on your local system do:

    cd core/libs
    git submodule add ../libs/libB

expecting that to point to libB.  But if we adjust the path then the
user has to do:

    git submodule add ../../libs/libB

However, it is also perfectly reasonable to have no remote configured
and the library next to the repository itself.  In which case we do want
to specify the additional "../" so that shell completion works in the
natural way.

The only way I can see to resolve the ambiguity is to die when we hit
this particular case.  This should be acceptable because people
shouldn't be adding new submodules anywhere near as often as they
perform other submodule operations and it doesn't affect absolute URLs.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Jens Lehmann]

Am 04.06.2013 14:48, schrieb John Keeping:
> On Tue, Jun 04, 2013 at 09:17:17PM +1000, Heiko Voigt wrote:
>> On Tue, Jun 04, 2013 at 09:10:45AM +0100, John Keeping wrote:
>>> On Tue, Jun 04, 2013 at 03:29:51PM +1000, Heiko Voigt wrote:
>>>> On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote:
>>>>>> Sorry, I should have been more specific here. I saw that you did some
>>>>>> changes to make "submodule add" do the right thing with relative paths,
>>>>>> but the following change to t7406 does not work like I believe it
>>>>>> should but instead makes the test fail:
>>>>>> -------------------8<---------------------
>>>>>> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
>>>>>> index a4ffea0..9766b9e 100755
>>>>>> --- a/t/t7406-submodule-update.sh
>>>>>> +++ b/t/t7406-submodule-update.sh
>>>>>> @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
>>>>>>  test_expect_success 'submodule add places git-dir in superprojects git-dir' '
>>>>>>         (cd super &&
>>>>>>          mkdir deeper &&
>>>>>> -        git submodule add ../submodule deeper/submodule &&
>>>>>> +        (cd deeper &&
>>>>>> +         git submodule add ../../submodule submodule
>>>>>> +        ) &&
>>>>>>          (cd deeper/submodule &&
>>>>>>           git log > ../../expected
>>>>>>          ) &&
>>>>>> -------------------8<---------------------
>>>>>
>>>>> Ah, ok.  I think this case is problematic because the repository
>>>>> argument is either relative to "remote.origin.url" or to the top of the
>>>>> working tree if there is no "origin" remote.  I wonder if we should just
>>>>> die when a relative path is given for the repository and we're not at
>>>>> the top of the working tree.
>>>>
>>>> Why not behave as if we are at the top of the working tree for relative
>>>> paths? If there is an origin remote thats fine. If there is no origin
>>>> remote you could warn that the path used is taken relative from the root
>>>> of the superproject during add. What do you think?
>>>
>>> That's what the patch currently queued on "pu" does, which Jens wants to
>>> change, isn't it?
>>
>> True I did not realize this when reading it the first time. But I think
>> we should still not die when in a subdirectory. After all this series is
>> trying to archive that the submodule command works in subdirectories
>> seamlessly right? So you probably want to translate a relative path
>> without "origin" remote given from a subdirectory to the superproject
>> level and use that. Then you do not have to die.
> 
> The problem is that sometimes you do want to adjust the path and
> sometimes you don't.  Reading git-submodule(1), it says:
> 
>      This may be either an absolute URL, or (if it begins with ./ or
>      ../), the location relative to the superproject’s origin
>      repository.
>      [snip]
>      If the superproject doesn’t have an origin configured the
>      superproject is its own authoritative upstream and the current
>      working directory is used instead.
> 
> So I think it's quite reasonable to have a server layout that looks like
> this:
> 
>     project
>     |- libs
>     |  |- libA
>     |  `- libB
>     |- core.git
> 
> and with only core.git on your local system do:
> 
>     cd core/libs
>     git submodule add ../libs/libB
> 
> expecting that to point to libB.  But if we adjust the path then the
> user has to do:
> 
>     git submodule add ../../libs/libB
> 
> However, it is also perfectly reasonable to have no remote configured
> and the library next to the repository itself.  In which case we do want
> to specify the additional "../" so that shell completion works in the
> natural way.

Exactly.

> The only way I can see to resolve the ambiguity is to die when we hit
> this particular case.

Hmm, I'm not so sure about that. Don't the first three lines in
resolve_relative_url() show how to distinguish between these two
cases?

resolve_relative_url ()
{
	remote=$(get_default_remote)
	remoteurl=$(git config "remote.$remote.url") ||
		remoteurl=$(pwd) # the repository is its own authoritative upstream
...

And this looks like a central place to handle this issue too (but I
admit I only glanced over the call sites of resolve_relative_url(),
so I might be missing something here).

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[John Keeping]

On Tue, Jun 04, 2013 at 11:39:25PM +0200, Jens Lehmann wrote:
> Am 04.06.2013 14:48, schrieb John Keeping:
> > The problem is that sometimes you do want to adjust the path and
> > sometimes you don't.  Reading git-submodule(1), it says:
> > 
> >      This may be either an absolute URL, or (if it begins with ./ or
> >      ../), the location relative to the superproject’s origin
> >      repository.
> >      [snip]
> >      If the superproject doesn’t have an origin configured the
> >      superproject is its own authoritative upstream and the current
> >      working directory is used instead.
> > 
> > So I think it's quite reasonable to have a server layout that looks like
> > this:
> > 
> >     project
> >     |- libs
> >     |  |- libA
> >     |  `- libB
> >     |- core.git
> > 
> > and with only core.git on your local system do:
> > 
> >     cd core/libs
> >     git submodule add ../libs/libB
> > 
> > expecting that to point to libB.  But if we adjust the path then the
> > user has to do:
> > 
> >     git submodule add ../../libs/libB
> > 
> > However, it is also perfectly reasonable to have no remote configured
> > and the library next to the repository itself.  In which case we do want
> > to specify the additional "../" so that shell completion works in the
> > natural way.
> 
> Exactly.
> 
> > The only way I can see to resolve the ambiguity is to die when we hit
> > this particular case.
> 
> Hmm, I'm not so sure about that. Don't the first three lines in
> resolve_relative_url() show how to distinguish between these two
> cases?
>
> resolve_relative_url ()
> {
> 	remote=$(get_default_remote)
> 	remoteurl=$(git config "remote.$remote.url") ||
> 		remoteurl=$(pwd) # the repository is its own authoritative upstream
> ...

If it's this simple, yes.  But I think there's also a third possibility
that combines both of these: what if the local directory structure is
the same as that on the "origin" remote?  Then "origin" exists but we
still want to adjust for the subdirectory.

The risk is that I can't see a behaviour that doesn't seem to choose
whether to convert the given path or not arbitrarily.  Even knowing the
rules, I expect that I could end up being surprised by this if I create
a new repository and haven't set up "origin" yet.

Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Phil Hord]

On Tue, Jun 4, 2013 at 8:48 AM, John Keeping <john@keeping.me.uk> wrote:
> On Tue, Jun 04, 2013 at 09:17:17PM +1000, Heiko Voigt wrote:
>> On Tue, Jun 04, 2013 at 09:10:45AM +0100, John Keeping wrote:
>> > On Tue, Jun 04, 2013 at 03:29:51PM +1000, Heiko Voigt wrote:
>> > > On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote:
>> > > > > Sorry, I should have been more specific here. I saw that you did some
>> > > > > changes to make "submodule add" do the right thing with relative paths,
>> > > > > but the following change to t7406 does not work like I believe it
>> > > > > should but instead makes the test fail:
>> > > > > -------------------8<---------------------
>> > > > > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
>> > > > > index a4ffea0..9766b9e 100755
>> > > > > --- a/t/t7406-submodule-update.sh
>> > > > > +++ b/t/t7406-submodule-update.sh
>> > > > > @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
>> > > > >  test_expect_success 'submodule add places git-dir in superprojects git-dir' '
>> > > > >         (cd super &&
>> > > > >          mkdir deeper &&
>> > > > > -        git submodule add ../submodule deeper/submodule &&
>> > > > > +        (cd deeper &&
>> > > > > +         git submodule add ../../submodule submodule
>> > > > > +        ) &&
>> > > > >          (cd deeper/submodule &&
>> > > > >           git log > ../../expected
>> > > > >          ) &&
>> > > > > -------------------8<---------------------
>> > > >
>> > > > Ah, ok.  I think this case is problematic because the repository
>> > > > argument is either relative to "remote.origin.url" or to the top of the
>> > > > working tree if there is no "origin" remote.  I wonder if we should just
>> > > > die when a relative path is given for the repository and we're not at
>> > > > the top of the working tree.
>> > >
>> > > Why not behave as if we are at the top of the working tree for relative
>> > > paths? If there is an origin remote thats fine. If there is no origin
>> > > remote you could warn that the path used is taken relative from the root
>> > > of the superproject during add. What do you think?
>> >
>> > That's what the patch currently queued on "pu" does, which Jens wants to
>> > change, isn't it?
>>
>> True I did not realize this when reading it the first time. But I think
>> we should still not die when in a subdirectory. After all this series is
>> trying to archive that the submodule command works in subdirectories
>> seamlessly right? So you probably want to translate a relative path
>> without "origin" remote given from a subdirectory to the superproject
>> level and use that. Then you do not have to die.
>
> The problem is that sometimes you do want to adjust the path and
> sometimes you don't.  Reading git-submodule(1), it says:
>
>      This may be either an absolute URL, or (if it begins with ./ or
>      ../), the location relative to the superproject’s origin
>      repository.
>      [snip]
>      If the superproject doesn’t have an origin configured the
>      superproject is its own authoritative upstream and the current
>      working directory is used instead.
>
> So I think it's quite reasonable to have a server layout that looks like
> this:
>
>     project
>     |- libs
>     |  |- libA
>     |  `- libB
>     |- core.git
>
> and with only core.git on your local system do:
>
>     cd core/libs
>     git submodule add ../libs/libB
>
> expecting that to point to libB.  But if we adjust the path then the
> user has to do:
>
>     git submodule add ../../libs/libB
>
> However, it is also perfectly reasonable to have no remote configured
> and the library next to the repository itself.  In which case we do want
> to specify the additional "../" so that shell completion works in the
> natural way.

In submodule add, the leading '../' prefix on the repository url has
always meant that the url is relative to the url of the current repo.
The given repo-url is precisely what ends up in .gitmodules'
submodule.$name.url.  It works this way whether there is a remote
configured or not.

It does seem like we need to be cautious around this change.

> The only way I can see to resolve the ambiguity is to die when we hit
> this particular case.  This should be acceptable because people
> shouldn't be adding new submodules anywhere near as often as they
> perform other submodule operations and it doesn't affect absolute URLs.

I don't think I like that.  But I don't know if I like anything I
dreamed up, either.

P

Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[John Keeping]

On Tue, Jun 04, 2013 at 06:57:34PM -0400, Phil Hord wrote:
> On Tue, Jun 4, 2013 at 8:48 AM, John Keeping <john@keeping.me.uk> wrote:
> > The problem is that sometimes you do want to adjust the path and
> > sometimes you don't.  Reading git-submodule(1), it says:
> >
> >      This may be either an absolute URL, or (if it begins with ./ or
> >      ../), the location relative to the superproject’s origin
> >      repository.
> >      [snip]
> >      If the superproject doesn’t have an origin configured the
> >      superproject is its own authoritative upstream and the current
> >      working directory is used instead.
> >
> > So I think it's quite reasonable to have a server layout that looks like
> > this:
> >
> >     project
> >     |- libs
> >     |  |- libA
> >     |  `- libB
> >     |- core.git
> >
> > and with only core.git on your local system do:
> >
> >     cd core/libs
> >     git submodule add ../libs/libB
> >
> > expecting that to point to libB.  But if we adjust the path then the
> > user has to do:
> >
> >     git submodule add ../../libs/libB
> >
> > However, it is also perfectly reasonable to have no remote configured
> > and the library next to the repository itself.  In which case we do want
> > to specify the additional "../" so that shell completion works in the
> > natural way.
> 
> In submodule add, the leading '../' prefix on the repository url has
> always meant that the url is relative to the url of the current repo.
> The given repo-url is precisely what ends up in .gitmodules'
> submodule.$name.url.  It works this way whether there is a remote
> configured or not.
> 
> It does seem like we need to be cautious around this change.
> 
> > The only way I can see to resolve the ambiguity is to die when we hit
> > this particular case.  This should be acceptable because people
> > shouldn't be adding new submodules anywhere near as often as they
> > perform other submodule operations and it doesn't affect absolute URLs.
> 
> I don't think I like that.  But I don't know if I like anything I
> dreamed up, either.

I've decided that I will make it die (unless anyone comes up with an
obviously correct solution before I get around to sending the reroll)
because it's a lot easier to loosen that in the future than to change it
if we get the behaviour wrong now.  I don't want to hold every other
subcommand hostage to this one case.

Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Junio C Hamano]

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

> Am 02.06.2013 20:50, schrieb Junio C Hamano:
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> 
>>> Am 30.05.2013 01:58, schrieb Junio C Hamano:
>>>> * jl/submodule-mv (2013-04-23) 5 commits
>>>>   (merged to 'next' on 2013-04-23 at c04f574)
>>>>  + submodule.c: duplicate real_path's return value
>>>>   (merged to 'next' on 2013-04-19 at 45ae3c9)
>>>>  + rm: delete .gitmodules entry of submodules removed from the work tree
>>>>  + Teach mv to update the path entry in .gitmodules for moved submodules
>>>>  + Teach mv to move submodules using a gitfile
>>>>  + Teach mv to move submodules together with their work trees
>>>>
>>>>  "git mv A B" when moving a submodule A does "the right thing",
>>>>  inclusing relocating its working tree and adjusting the paths in
>>>>  the .gitmodules file.
>
> <detailed discussion snipped>
>
>> So my gut feeling of the "fix" at this point in the evolution of the
>> program may be to error out with a message like "You have local
>> changes to .gitmodules; please stash it before moving or removing".
>
> Yeah, me too thinks that this is a sane short term solution (even
> though a "git submodule add" currently happily stages any unstaged
> modifications to the .gitmodules file too, that should not stop us
> from doing better for rm and mv ;-).
>
> And I also agree that in the long run the the git-config aware merge
> driver together with the 3-way merge of a modified .gitmodules file
> you described is the best solution. But I'd really like to complete
> the recursive update before tackling that, so for now I just added
> these two to the to-do list on my GitHub wiki page.
>
> I'll resubmit this series with the strlen() fix and the erroring out
> in case of unstaged modifications of the .gitmodules file as soon as
> I find some time.

Ping?

No need to hurry, but just to make sure this didn't disappear from
everybody's radar.

Thanks.