git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* What's cooking in git.git (Jan 2014, #01; Mon, 6)
@ 2014-01-06 22:36 Junio C Hamano
  2014-01-06 23:16 ` Francesco Pretto
  2014-01-07 17:49 ` Jens Lehmann
  0 siblings, 2 replies; 35+ messages in thread
From: Junio C Hamano @ 2014-01-06 22:36 UTC (permalink / raw
  To: git

Welcome to the first issue of "What's cooking" report for the new
year.

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

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"]

* fc/remote-helper-fixes (2013-12-26) 5 commits
  (merged to 'next' on 2013-12-26 at ce5f872)
 + remote-hg: test 'shared_path' in a moved clone
  (merged to 'next' on 2013-12-17 at aa4dc07)
 + remote-hg: add tests for special filenames
 + remote-hg: fix 'shared path' path
 + remote-helpers: add extra safety checks
 + remote-hg: avoid buggy strftime()


* jc/push-refmap (2013-12-04) 3 commits
  (merged to 'next' on 2013-12-12 at 71e358f)
 + push: also use "upstream" mapping when pushing a single ref
 + push: use remote.$name.push as a refmap
 + builtin/push.c: use strbuf instead of manual allocation

 Make "git push origin master" update the same ref that would be
 updated by our 'master' when "git push origin" (no refspecs) is run
 while the 'master' branch is checked out, which makes "git push"
 more symmetric to "git fetch" and more usable for the triangular
 workflow.


* jk/cat-file-regression-fix (2013-12-12) 2 commits
  (merged to 'next' on 2013-12-13 at 3713e01)
 + cat-file: handle --batch format with missing type/size
 + cat-file: pass expand_data to print_object_or_die

 "git cat-file --batch=", an admittedly useless command, did not
 behave very well.


* jk/name-pack-after-byte-representation (2013-12-16) 3 commits
  (merged to 'next' on 2013-12-17 at 0bc385c)
 + pack-objects doc: treat output filename as opaque
  (merged to 'next' on 2013-12-09 at 247b2d0)
 + pack-objects: name pack files after trailer hash
 + sha1write: make buffer const-correct
 (this branch is tangled with jk/pack-bitmap.)

 Two packfiles that contain the same set of objects have
 traditionally been named identically, but that made repacking a
 repository that is already fully packed without any cruft with a
 different packing parameter cumbersome. Update the convention to
 name the packfile after the bytestream representation of the data,
 not after the set of objects in it.


* jk/pull-rebase-using-fork-point (2013-12-10) 2 commits
  (merged to 'next' on 2013-12-13 at 1862c12)
 + rebase: use reflog to find common base with upstream
 + pull: use merge-base --fork-point when appropriate


* jk/rev-parse-double-dashes (2013-12-09) 2 commits
  (merged to 'next' on 2013-12-13 at d26bac7)
 + rev-parse: be more careful with munging arguments
 + rev-parse: correctly diagnose revision errors before "--"

 "git rev-parse <revs> -- <paths>" did not implement the usual
 disambiguation rules the commands in the "git log" family used in
 the same way.


* js/gnome-keyring (2013-12-16) 1 commit
  (merged to 'next' on 2013-12-17 at 422fd61)
 + contrib/git-credential-gnome-keyring.c: small stylistic cleanups

 Style fix.


* tg/diff-no-index-refactor (2013-12-16) 4 commits
  (merged to 'next' on 2013-12-17 at 009d8d8)
 + diff: avoid some nesting
 + diff: add test for --no-index executed outside repo
  (merged to 'next' on 2013-12-13 at 523f7c4)
 + diff: don't read index when --no-index is given
 + diff: move no-index detection to builtin/diff.c

 "git diff ../else/where/A ../else/where/B" when ../else/where is
 clearly outside the repository, and "git diff --no-index A B", do
 not have to look at the index at all, but we used to read the index
 unconditionally.


* zk/difftool-counts (2013-12-16) 2 commits
  (merged to 'next' on 2013-12-16 at 0e0d235)
 + diff.c: fix some recent whitespace style violations
  (merged to 'next' on 2013-12-12 at ba35694)
 + difftool: display the number of files in the diff queue in the prompt

 Show the total number of paths and the number of paths shown so far
 when "git difftool" prompts to launch an external diff tool, which
 would give users some sense of progress.

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

* ta/format-user-manual-as-an-article (2014-01-06) 1 commit
  (merged to 'next' on 2014-01-06 at 37858f6)
 + user-manual: improve html and pdf formatting

 Update the way the user-manual is formatted via AsciiDoc to save
 trees.

 Will merge to 'master'.


* bm/merge-base-octopus-dedup (2013-12-30) 2 commits
  (merged to 'next' on 2014-01-06 at 355d62b)
 + merge-base --octopus: reduce the result from get_octopus_merge_bases()
 + merge-base: separate "--independent" codepath into its own helper

 "git merge-base --octopus" used to leave cleaning up suboptimal
 result to the caller, but now it does the clean-up itself.

 Will merge to 'master'.


* jk/test-framework-updates (2014-01-02) 3 commits
  (merged to 'next' on 2014-01-06 at f81f373)
 + t0000: drop "known breakage" test
 + t0000: simplify HARNESS_ACTIVE hack
 + t0000: set TEST_OUTPUT_DIRECTORY for sub-tests

 The basic test used to leave unnecessary trash directories in the
 t/ directory.

 Will merge to 'master'.


* js/lift-parent-count-limit (2013-12-27) 1 commit
  (merged to 'next' on 2014-01-06 at b74133c)
 + Remove the line length limit for graft files

 There is no reason to have a hardcoded upper limit of the number of
 parents for an octopus merge, created via the graft mechanism.

 Will merge to 'master'.


* ap/remote-hg-skip-null-bookmarks (2014-01-02) 1 commit
 - remote-hg: do not fail on invalid bookmarks

 Reported to break tests ($gmane/240005)
 Expecting a reroll.


* bs/mirbsd (2014-01-02) 1 commit
  (merged to 'next' on 2014-01-06 at d5cecbb)
 + Add MirBSD support to the build system.

 Will merge to 'master'.


* jk/credential-plug-leak (2014-01-02) 1 commit
  (merged to 'next' on 2014-01-06 at 88e29a3)
 + Revert "prompt: clean up strbuf usage"

 An earlier "clean-up" introduced an unnecessary memory leak.

 Will merge to 'master'.


* jk/http-auth-tests-robustify (2014-01-02) 1 commit
  (merged to 'next' on 2014-01-06 at 7e87bba)
 + use distinct username/password for http auth tests

 Using the same username and password during the tests would not
 catch a potential breakage of sending one when we should be sending
 the other.

 Will merge to 'master'.


* km/gc-eperm (2014-01-02) 1 commit
  (merged to 'next' on 2014-01-06 at fe107de)
 + gc: notice gc processes run by other users

 A "gc" process running as a different user should be able to stop a
 new "gc" process from starting.

 Will merge to 'master'.


* rr/completion-branch-config (2014-01-06) 4 commits
  (merged to 'next' on 2014-01-06 at ed9eecc)
 + completion: fix remote.pushdefault
 + completion: fix branch.autosetup(merge|rebase)
 + completion: introduce __gitcomp_nl_append ()
 + zsh completion: find matching custom bash completion

 Two-level configuration variable names in "branch.*" and "remote.*"
 hierarchies whose variables are predominantly three-level where not
 completed by hitting a <TAB> in bash and zsh completions.

 Will merge to 'master'.


* ss/builtin-cleanup (2014-01-06) 3 commits
  (merged to 'next' on 2014-01-06 at ffcac50)
 + builtin/help.c: speed up is_git_command() by checking for builtin commands first
 + builtin/help.c: call load_command_list() only when it is needed
 + git.c: consistently use the term "builtin" instead of "internal command"

 "git help $cmd" unnecessarily enumerated potential command names
 from the filesystem, even when $cmd is known to be a built-in.

 Ideas for further optimization, primarily by killing the use of
 is_in_cmdlist(), were suggested in the discussion, but they can
 come as follow-ups on top of this series.

 Will merge to 'master'.


* ss/safe-create-leading-dir-with-slash (2014-01-06) 1 commit
 - safe_create_leading_directories(): on Windows, \ can separate path components


* vm/octopus-merge-bases-simplify (2014-01-03) 1 commit
  (merged to 'next' on 2014-01-06 at 35df672)
 + get_octopus_merge_bases(): cleanup redundant variable

 Will merge to 'master'.


* fp/submodule-checkout-mode (2014-01-06) 2 commits
 - DONOTMERGE: needs sign-off
 - git-submodule.sh: support 'checkout' as a valid update mode

 Need to pick up a rerolled one.

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

* jc/graph-post-root-gap (2013-12-30) 3 commits
 - WIP: document what we want at the end
 - graph: remove unused code a bit
 - graph: stuff the current commit into graph->columns[]

 This was primarily a RFH ($gmane/239580).


* fc/transport-helper-fixes (2013-12-09) 6 commits
 - remote-bzr: support the new 'force' option
 - test-hg.sh: tests are now expected to pass
 - transport-helper: check for 'forced update' message
 - transport-helper: add 'force' to 'export' helpers
 - transport-helper: don't update refs in dry-run
 - transport-helper: mismerge fix

 Updates transport-helper, fast-import and fast-export to allow the
 ref mapping and ref deletion in a way similar to the natively
 supported transports.

 Reported to break t5541.
 Will hold.


* fc/completion (2013-12-09) 1 commit
 - completion: fix completion of certain aliases

 SZEDER Gábor noticed that this breaks "git -c var=val alias" and
 also suggested a better description of the change.

 Will hold.


* mo/subtree-split-updates (2013-12-10) 3 commits
 - subtree: add --edit option
 - subtree: allow --squash and --message with push
 - subtree: support split --rejoin --squash

 Comments?


* hv/submodule-ignore-fix (2013-12-06) 4 commits
 - disable complete ignorance of submodules for index <-> HEAD diff
 - always show committed submodules in summary after commit
 - teach add -f option for ignored submodules
 - fix 'git add' to skip submodules configured as ignored

 Teach "git add" to be consistent with "git status" when changes to
 submodules are set to be ignored, to avoid surprises after checking
 with "git status" to see there isn't any change to be further added
 and then see that "git add ." adds changes to them.

 I think a reroll is coming, so this may need to be replaced, but I
 needed some practice with heavy conflict resolution.  It conflicts
 with two changes to "git add" that have been scheduled for Git 2.0
 quite badly, and I think I got the resolution right this time.


* kb/fast-hashmap (2014-01-03) 19 commits
 - hashmap.h: make sure map entries are tightly packed
  (merged to 'next' on 2014-01-03 at dc85001)
 + name-hash: retire unused index_name_exists()
 + hashmap.h: Use 'unsigned int' for hash-codes everywhere
  (merged to 'next' on 2013-12-16 at bff99b1)
 + Drop unnecessary #includes from test-hashmap
 + Add test-hashmap to .gitignore
  (merged to 'next' on 2013-12-06 at f90be3d)
 + read-cache.c: fix memory leaks caused by removed cache entries
 + builtin/update-index.c: cleanup update_one
 + fix 'git update-index --verbose --again' output
 + remove old hash.[ch] implementation
 + name-hash.c: remove cache entries instead of marking them CE_UNHASHED
 + name-hash.c: use new hash map implementation for cache entries
 + name-hash.c: remove unreferenced directory entries
 + name-hash.c: use new hash map implementation for directories
 + diffcore-rename.c: use new hash map implementation
 + diffcore-rename.c: simplify finding exact renames
 + diffcore-rename.c: move code around to prepare for the next patch
 + buitin/describe.c: use new hash map implementation
 + add a hashtable implementation that supports O(1) removal
 + submodule: don't access the .gitmodules cache entry after removing it

 Improvements to our hash table to get it to meet the needs of the
 msysgit fscache project, with some nice performance improvements.

 The tip one does not seem to have reached concensus (yet).


* jc/create-directories-microopt (2013-11-11) 1 commit
 - checkout: most of the time we have good leading directories

 Of unknown value until tested on non-Linux platforms (especially
 Windows).

 Will hold.


* jt/commit-fixes-footer (2013-10-30) 1 commit
 - commit: Add -f, --fixes <commit> option to add Fixes: line

 There is an ongoing discussion around this topic; in general I am
 fairly negative on a new feature that is too narrow and prefer a
 more generic solution that can be tailored for specific needs, as
 many people stated in the thread.

 It appears that the discussion stalled.


* np/pack-v4 (2013-09-18) 90 commits
 . packv4-parse.c: add tree offset caching
 . t1050: replace one instance of show-index with verify-pack
 . index-pack, pack-objects: allow creating .idx v2 with .pack v4
 . unpack-objects: decode v4 trees
 . unpack-objects: allow to save processed bytes to a buffer
 - ...

 Nico and Duy advancing the eternal vaporware pack-v4.  This is here
 primarily for wider distribution of the preview edition.

 Temporarily ejected from 'pu', to try out jk/pack-bitmap, which
 this topic conflicts with.


* mf/graph-show-root (2013-10-25) 1 commit
 . graph.c: mark root commit differently

 In a repository with multiple-roots, "log --graph", especially with
 "--oneline", does not give the reader enough visual cue to see
 where one line of history ended and a separate history began.

 This is the version that marks the roots 'x' when they would have
 been marked as '*'; Keshav Kini suggested an alternative of giving
 an extra blank line after every root, which I tend to think is a
 better approach to the problem.


* tg/perf-lib-test-perf-cleanup (2013-09-19) 2 commits
 - perf-lib: add test_perf_cleanup target
 - perf-lib: split starting the test from the execution

 Add test_perf_cleanup shell function to the perf suite, that allows
 the script writers to define a test with a clean-up action.

 Holding until needed.


* yt/shortened-rename (2013-10-18) 2 commits
 - SQUASH??? style fixes and s/omit/shorten/ where appropriate
 - diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible

 Attempts to give more weight on the fact that a filepair represents
 a rename than showing substring of the actual path when diffstat
 lines are not wide enough.

 I am not sure if that is solving a right problem, though.


* rv/send-email-cache-generated-mid (2013-08-21) 2 commits
 - git-send-email: Cache generated message-ids, use them when prompting
 - git-send-email: add optional 'choices' parameter to the ask sub


* rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits
 - ### DONTMERGE: needs better explanation on what config they need
 - pack-refs.c: Add missing call to git_config()
 - show-ref.c: Add missing call to git_config()

 The changes themselves are probably good, but it is unclear what
 basic setting needs to be read for which exact operation.

 Waiting for clarification.
 $gmane/228294


* 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.


* 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.

 Drew Northup volunteered to take a look into this.
 $gmane/226216


* jc/show-branch (2013-06-07) 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

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

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

* bc/log-decoration (2013-12-20) 1 commit
  (merged to 'next' on 2014-01-03 at ff8873f)
 + log: properly handle decorations with chained tags

 "git log --decorate" did not handle a tag pointed by another tag
 nicely.

 Will merge to 'master'.


* jh/rlimit-nofile-fallback (2013-12-18) 1 commit
  (merged to 'next' on 2014-01-03 at b56ae0c)
 + get_max_fd_limit(): fall back to OPEN_MAX upon getrlimit/sysconf failure

 When we figure out how many file descriptors to allocate for
 keeping packfiles open, a system with non-working getrlimit() could
 cause us to die(), but because we make this call only to get a
 rough estimate of how many is available and we do not even attempt
 to use up all file descriptors available ourselves, it is nicer to
 fall back to a reasonable low value rather than dying.

 Will merge to 'master'.


* rt/bfg-ad-in-filter-branch-doc (2013-12-18) 1 commit
  (merged to 'next' on 2014-01-03 at 2a45e3b)
 + docs: add filter-branch notes on The BFG

 Will merge to 'master'.


* sb/diff-orderfile-config (2013-12-18) 3 commits
  (merged to 'next' on 2014-01-03 at 744eba7)
 + diff: add diff.orderfile configuration variable
 + diff: let "git diff -O" read orderfile from any file and fail properly
 + t4056: add new tests for "git diff -O"

 Allow "git diff -O<file>" to be configured with a new configuration
 variable.

 Will merge to 'master'.


* nd/daemon-informative-errors-typofix (2013-12-20) 1 commit
  (merged to 'next' on 2014-01-03 at 1b87648)
 + daemon: be strict at parsing parameters --[no-]informative-errors

 Will merge to 'master'.


* tm/fetch-prune (2014-01-03) 2 commits
  (merged to 'next' on 2014-01-03 at a58c6b4)
 + fetch --prune: Run prune before fetching
 + fetch --prune: always print header url

 Fetching 'frotz' branch with "git fetch", while having
 'frotz/nitfol' remote-tracking branch from an earlier fetch, would
 error out, primarily because the command has not been told to
 remove anything on our side. In such a case, "git fetch --prune"
 can be used to remove 'frotz/nitfol' to make room to fetch and
 store 'frotz' remote-tracking branch.

 Will merge to 'master'.


* jk/oi-delta-base (2013-12-26) 2 commits
  (merged to 'next' on 2014-01-06 at 8cf3ed2)
 + cat-file: provide %(deltabase) batch format
 + sha1_object_info_extended: provide delta base sha1s

 Teach "cat-file --batch" to show delta-base object name for a
 packed object that is represented as a delta.

 Will merge to 'master'.


* jk/sha1write-void (2013-12-26) 1 commit
  (merged to 'next' on 2014-01-06 at d8cd8ff)
 + do not pretend sha1write returns errors

 Code clean-up.

 Will merge to 'master'.


* jl/submodule-recursive-checkout (2013-12-26) 5 commits
 - Teach checkout to recursively checkout submodules
 - submodule: teach unpack_trees() to update submodules
 - submodule: teach unpack_trees() to repopulate submodules
 - submodule: teach unpack_trees() to remove submodule contents
 - submodule: prepare for recursive checkout of submodules

 What is the doneness of this one???


* mh/safe-create-leading-directories (2014-01-06) 17 commits
 - rename_tmp_log(): on SCLD_VANISHED, retry
 - rename_tmp_log(): limit the number of remote_empty_directories() attempts
 - rename_tmp_log(): handle a possible mkdir/rmdir race
 - rename_ref(): extract function rename_tmp_log()
 - remove_dir_recurse(): handle disappearing files and directories
 - remove_dir_recurse(): tighten condition for removing unreadable dir
 - lock_ref_sha1_basic(): if locking fails with ENOENT, retry
 - lock_ref_sha1_basic(): on SCLD_VANISHED, retry
 - safe_create_leading_directories(): add new error value SCLD_VANISHED
 - cmd_init_db(): when creating directories, handle errors conservatively
 - safe_create_leading_directories(): introduce enum for return values
 - safe_create_leading_directories(): always restore slash at end of loop
 - safe_create_leading_directories(): split on first of multiple slashes
 - safe_create_leading_directories(): rename local variable
 - safe_create_leading_directories(): add explicit "slash" pointer
 - safe_create_leading_directories(): reduce scope of local variable
 - safe_create_leading_directories(): fix format of "if" chaining

 Code clean-up and protection against concurrent write access to the
 ref namespace.

 Is ready for 'next', with or without minor nitfix.


* nd/add-empty-fix (2013-12-26) 1 commit
  (merged to 'next' on 2014-01-06 at 88a78c9)
 + add: don't complain when adding empty project root

 "git add -A" (no other arguments) in a totally empty working tree
 used to emit an error.

 Will merge to 'master'.


* nd/commit-tree-constness (2013-12-26) 1 commit
  (merged to 'next' on 2014-01-06 at a177c9f)
 + commit.c: make "tree" a const pointer in commit_tree*()

 Code clean-up.

 Will merge to 'master'.


* jk/pack-bitmap (2013-12-30) 21 commits
 - pack-bitmap: implement optional name_hash cache
 - t/perf: add tests for pack bitmaps
 - t: add basic bitmap functionality tests
 - count-objects: recognize .bitmap in garbage-checking
 - repack: consider bitmaps when performing repacks
 - repack: handle optional files created by pack-objects
 - repack: turn exts array into array-of-struct
 - repack: stop using magic number for ARRAY_SIZE(exts)
 - pack-objects: implement bitmap writing
 - rev-list: add bitmap mode to speed up object lists
 - pack-objects: use bitmaps when packing objects
 - pack-objects: split add_object_entry
 - pack-bitmap: add support for bitmap indexes
 - documentation: add documentation for the bitmap format
 - ewah: compressed bitmap implementation
 - compat: add endianness helpers
 - sha1_file: export `git_open_noatime`
 - revision: allow setting custom limiter function
 - pack-objects: factor out name_hash
 - pack-objects: refactor the packing list
 - revindex: export new APIs

 Borrows the bitmap index into packfiles from JGit to speed up
 enumeration of objects involved in a commit range without having to
 fully traverse the history.

 Will merge to 'next'.


* ap/path-max (2013-12-16) 1 commit
  (merged to 'next' on 2014-01-03 at affc620)
 + Prevent buffer overflows when path is too long

 Will merge to 'master'.


* mh/path-max (2013-12-18) 2 commits
  (merged to 'next' on 2014-01-03 at 5227c9b)
 + builtin/prune.c: use strbuf to avoid having to worry about PATH_MAX
 + prune-packed: use strbuf to avoid having to worry about PATH_MAX

 A few places where we relied on a fixed length buffer to hold
 pathnames in these two programs have been converted to use strbuf.

 Will merge to 'master'.


* nv/commit-gpgsign-config (2013-12-17) 3 commits
  (merged to 'next' on 2014-01-03 at 9780cbb)
 + test the commit.gpgsign config option
 + commit-tree: add and document --no-gpg-sign
 + Add the commit.gpgsign option to sign all commits

 Introduce commit.gpgsign configuration variable to force every
 commit to be GPG signed.  The variable cannot be overriden from the
 command line of some of the commands that create commits except for
 "git commit" and "git commit-tree", but I am not convinced that it
 is a good idea to sprinkle support for --no-gpg-sign everywhere.


* cc/replace-object-info (2013-12-30) 11 commits
  (merged to 'next' on 2014-01-03 at 4473803)
 + replace info: rename 'full' to 'long' and clarify in-code symbols
  (merged to 'next' on 2013-12-17 at aeb9e18)
 + Documentation/git-replace: describe --format option
 + builtin/replace: unset read_replace_refs
 + t6050: add tests for listing with --format
 + builtin/replace: teach listing using short, medium or full formats
 + sha1_file: perform object replacement in sha1_object_info_extended()
 + t6050: show that git cat-file --batch fails with replace objects
 + sha1_object_info_extended(): add an "unsigned flags" parameter
 + sha1_file.c: add lookup_replace_object_extended() to pass flags
 + replace_object: don't check read_replace_refs twice
 + rename READ_SHA1_FILE_REPLACE flag to LOOKUP_REPLACE_OBJECT

 read_sha1_file() that is the workhorse to read the contents given
 an object name honoured object replacements, but there is no
 corresponding mechanism to sha1_object_info() that is used to
 obtain the metainfo (e.g. type & size) about the object, leading
 callers to weird inconsistencies.

 Will merge to 'master'.


* nd/shallow-clone (2014-01-06) 30 commits
  (merged to 'next' on 2014-01-06 at 3dc7fab)
 + shallow: remove unused code
 + send-pack.c: mark a file-local function static
  (merged to 'next' on 2014-01-03 at a776065)
 + git-clone.txt: remove shallow clone limitations
 + prune: clean .git/shallow after pruning objects
 + clone: use git protocol for cloning shallow repo locally
 + send-pack: support pushing from a shallow clone via http
 + receive-pack: support pushing to a shallow clone via http
 + smart-http: support shallow fetch/clone
 + remote-curl: pass ref SHA-1 to fetch-pack as well
 + send-pack: support pushing to a shallow clone
 + receive-pack: allow pushes that update .git/shallow
 + connected.c: add new variant that runs with --shallow-file
 + add GIT_SHALLOW_FILE to propagate --shallow-file to subprocesses
 + receive/send-pack: support pushing from a shallow clone
 + receive-pack: reorder some code in unpack()
 + fetch: add --update-shallow to accept refs that update .git/shallow
 + upload-pack: make sure deepening preserves shallow roots
 + fetch: support fetching from a shallow repository
 + clone: support remote shallow repository
 + fetch-pack.h: one statement per bitfield declaration
 + fetch-pack.c: move shallow update code out of fetch_pack()
 + shallow.c: steps 6 and 7 to select new commits for .git/shallow
 + shallow.c: the 8 steps to select new commits for .git/shallow
 + shallow.c: extend setup_*_shallow() to accept extra shallow commits
 + connect.c: teach get_remote_heads to parse "shallow" lines
 + make the sender advertise shallow commits to the receiver
 + clone: prevent --reference to a shallow repository
 + send-pack: forbid pushing from a shallow repository
 + remote.h: replace struct extra_have_objects with struct sha1_array
 + transport.h: remove send_pack prototype, already defined in send-pack.h

 Fetching from a shallow-cloned repository used to be forbidden,
 primarily because the codepaths involved were not carefully vetted
 and we did not bother supporting such usage. This attempts to allow
 object transfer out of a shallow-cloned repository in a controlled
 way (i.e. the receiver become a shallow repository with truncated
 history).

 Will merge to 'master'.


* jn/git-gui-chmod+x (2013-11-25) 1 commit
 - git-gui: chmod +x po2msg, windows/git-gui.sh

 Parked here until I get the same change back from the upstream
 git-gui tree.


* jn/gitk-chmod+x (2013-11-25) 1 commit
 - gitk: chmod +x po2msg

 Parked here until I get the same change back from the upstream gitk
 tree.


* nd/negative-pathspec (2013-12-06) 3 commits
  (merged to 'next' on 2013-12-12 at 9f340c8)
 + pathspec.c: support adding prefix magic to a pathspec with mnemonic magic
 + Support pathspec magic :(exclude) and its short form :!
 + glossary-content.txt: rephrase magic signature part

 Introduce "negative pathspec" magic, to allow "git log -- . ':!dir'" to
 tell us "I am interested in everything but 'dir' directory".

 Will merge to 'master'.


* cc/starts-n-ends-with-endgame (2013-12-05) 1 commit
 - strbuf: remove prefixcmp() and suffixcmp()

 Endgame for the cc/starts-n-ends-with topic; this needs to be
 evil-merged with other topics that introduce new uses of
 prefix/suffix-cmp functions.

 Will merge to 'next' and cook until Git 2.0.


* gj/push-more-verbose-advice (2013-11-13) 1 commit
  (merged to 'next' on 2013-12-06 at 574b18a)
 + push: switch default from "matching" to "simple"

 Originally merged to 'next' on 2013-11-21

 Explain 'simple' and 'matching' in "git push" advice message; the
 topmost patch is a rebase of jc/push-2.0-default-to-simple on top
 of it.

 Will cook in 'next' until Git 2.0.


* tr/merge-recursive-index-only (2013-10-28) 3 commits
 - merge-recursive: -Xindex-only to leave worktree unchanged
 - merge-recursive: internal flag to avoid touching the worktree
 - merge-recursive: remove dead conditional in update_stages()

 Will hold until using script appears.


* jn/add-2.0-u-A-sans-pathspec (2013-04-26) 1 commit
  (merged to 'next' on 2013-12-06 at ead2ec8)
 + 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
  (merged to 'next' on 2013-12-06 at ae18007)
 + core.statinfo: remove as promised in Git 2.0

 Will cook in 'next' until Git 2.0.


* jc/push-2.0-default-to-simple (2013-06-18) 1 commit
  (merged to 'next' on 2013-12-06 at 6fad61c)
 + 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
  (merged to 'next' on 2013-12-06 at fbaa75a)
 + 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.


* jc/hold-diff-remove-q-synonym-for-no-deletion (2013-07-19) 1 commit
  (merged to 'next' on 2013-12-06 at 083d67c)
 + diff: remove "diff-files -q" in a version of Git in a distant future

 Will cook in 'next' until a distant future.

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

* aa/transport-non-positive-depth-only (2013-11-26) 1 commit
 . transport: catch non positive --depth option value


* cc/remote-remove-redundant-postfixcmp (2013-11-06) 2 commits
 . Rename suffixcmp() to has_suffix() and invert its result
 . builtin/remote: remove postfixcmp() and use suffixcmp() instead


* th/reflog-annotated-tag (2013-10-28) 1 commit
 . reflog: handle lightweight and annotated tags equally

 "git log -g $annotated_tag", when there is no reflog history, should
 have produced a single output entry (i.e. the ref creation event),
 but instead showed the history leading to the tag.

 Broken at the design level.  Any reflog entry that points at a non
 commit needs to be handled with new code that does not exist yet,
 and lifting the "this code handles only commits" without adding
 such code does not solve anything.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)
  2014-01-06 22:36 What's cooking in git.git (Jan 2014, #01; Mon, 6) Junio C Hamano
@ 2014-01-06 23:16 ` Francesco Pretto
  2014-01-06 23:32   ` Junio C Hamano
  2014-01-07 17:49 ` Jens Lehmann
  1 sibling, 1 reply; 35+ messages in thread
From: Francesco Pretto @ 2014-01-06 23:16 UTC (permalink / raw
  To: Junio C Hamano; +Cc: git@vger.kernel.org

2014/1/6 Junio C Hamano <gitster@pobox.com>:
>
>  - git-submodule.sh: support 'checkout' as a valid update mode
>
>  Need to pick up a rerolled one.
>

I resent it, can you see it?

Thank you,
Francesco

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)
  2014-01-06 23:16 ` Francesco Pretto
@ 2014-01-06 23:32   ` Junio C Hamano
  2014-01-06 23:45     ` Francesco Pretto
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2014-01-06 23:32 UTC (permalink / raw
  To: Francesco Pretto; +Cc: git@vger.kernel.org

Francesco Pretto <ceztkoml@gmail.com> writes:

> 2014/1/6 Junio C Hamano <gitster@pobox.com>:
>>
>>  - git-submodule.sh: support 'checkout' as a valid update mode
>>
>>  Need to pick up a rerolled one.
>>
>
> I resent it, can you see it?

I know. I saw it and that is why I left the note to self.

The thing is, it takes a non trivial amount of time to run through a
single day's integration cycle, and any reroll that comes later in a
day once the cycle started may be too late for that day.  Otherwise
I have to discard the the result of earlier merges and tests and
start over from scratch.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)
  2014-01-06 23:32   ` Junio C Hamano
@ 2014-01-06 23:45     ` Francesco Pretto
  0 siblings, 0 replies; 35+ messages in thread
From: Francesco Pretto @ 2014-01-06 23:45 UTC (permalink / raw
  To: Junio C Hamano; +Cc: git@vger.kernel.org

2014/1/7 Junio C Hamano <gitster@pobox.com>:
> The thing is, it takes a non trivial amount of time to run through a
> single day's integration cycle, and any reroll that comes later in a
> day once the cycle started may be too late for that day.  Otherwise
> I have to discard the the result of earlier merges and tests and
> start over from scratch.
>

Got it, thank you.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: What's cooking in git.git (Jan 2014, #01; Mon, 6)
  2014-01-06 22:36 What's cooking in git.git (Jan 2014, #01; Mon, 6) Junio C Hamano
  2014-01-06 23:16 ` Francesco Pretto
@ 2014-01-07 17:49 ` Jens Lehmann
       [not found]   ` <xmqqvbxvekwv.fsf@gitster.dls.corp.google.com>
  1 sibling, 1 reply; 35+ messages in thread
From: Jens Lehmann @ 2014-01-07 17:49 UTC (permalink / raw
  To: Junio C Hamano, git

Am 06.01.2014 23:36, schrieb Junio C Hamano:
> * jl/submodule-recursive-checkout (2013-12-26) 5 commits
>  - Teach checkout to recursively checkout submodules
>  - submodule: teach unpack_trees() to update submodules
>  - submodule: teach unpack_trees() to repopulate submodules
>  - submodule: teach unpack_trees() to remove submodule contents
>  - submodule: prepare for recursive checkout of submodules
> 
>  What is the doneness of this one???

It's still work in progress. Currently I'm working on a test
framework so we can reuse recursive submodule checkout tests
instead of rewriting them for every command that learns the
--recurse-submodule option. Will reroll this series as soon
as I have something presentable.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [WIP/PATCH 0/9] v2 submodule recursive checkout]
       [not found]   ` <xmqqvbxvekwv.fsf@gitster.dls.corp.google.com>
@ 2014-02-03 19:47     ` Jens Lehmann
  2014-02-03 19:48       ` [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules Jens Lehmann
                         ` (8 more replies)
  0 siblings, 9 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:47 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

Am 07.01.2014 18:55, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Am 06.01.2014 23:36, schrieb Junio C Hamano:
>>> * jl/submodule-recursive-checkout (2013-12-26) 5 commits
>>>  - Teach checkout to recursively checkout submodules
>>>  - submodule: teach unpack_trees() to update submodules
>>>  - submodule: teach unpack_trees() to repopulate submodules
>>>  - submodule: teach unpack_trees() to remove submodule contents
>>>  - submodule: prepare for recursive checkout of submodules
>>>
>>>  What is the doneness of this one???
>>
>> It's still work in progress. Currently I'm working on a test
>> framework so we can reuse recursive submodule checkout tests
>> instead of rewriting them for every command that learns the
>> --recurse-submodule option. Will reroll this series as soon
>> as I have something presentable.
> 
> Thanks.

Ok, time for another round. This is still WIP/RFC and not
ready to be merged yet, but I believe this round makes it a
bit more clear where this is heading.

Changes to the first version are:

- Reordered the commits according to Jonathan's proposal
  (this currently makes the checkout tests fail until the
  last commit completes the functionality; this will be
  fixed when the test framework is added)

- Fixed calling parse_fetch_recurse_submodules_arg() where
  parse_update_recurse_submodules_arg() must be used.

- Moved the documentation of the --[no-]recurse-submodule
  option into an include file so different commands can
  reuse it.

- Added the --[no-]recurse-submodule option to bisect, merge
  and reset too.

Tests, documentation and commit messages are not complete yet,
I'll work on them in the next rounds. The wiki page in my Github
repository will describe the current status of this series:

  https://github.com/jlehmann/git-submod-enhancements/wiki/Recursive-submodule-checkout


Jens Lehmann (9):
  submodule: prepare for recursive checkout of submodules
  Teach reset the --[no-]recurse-submodules option
  Teach checkout the --[no-]recurse-submodules option
  Teach merge the --[no-]recurse-submodules option
  Teach bisect--helper the --[no-]recurse-submodules option
  Teach bisect the --[no-]recurse-submodules option
  submodule: teach unpack_trees() to remove submodule contents
  submodule: teach unpack_trees() to repopulate submodules
  submodule: teach unpack_trees() to update submodules

 Documentation/git-bisect.txt                |   5 +
 Documentation/git-checkout.txt              |   2 +
 Documentation/git-merge.txt                 |   2 +
 Documentation/git-reset.txt                 |   4 +
 Documentation/recurse-submodules-update.txt |   8 +
 bisect.c                                    |  33 ++--
 bisect.h                                    |   3 +-
 builtin/bisect--helper.c                    |   9 +-
 builtin/checkout.c                          |  14 ++
 builtin/merge.c                             |  14 ++
 builtin/reset.c                             |  14 ++
 entry.c                                     |  19 ++-
 git-bisect.sh                               |  29 +++-
 submodule.c                                 | 238 +++++++++++++++++++++++++++-
 submodule.h                                 |  12 ++
 t/t2013-checkout-submodule.sh               | 215 ++++++++++++++++++++++++-
 unpack-trees.c                              |  95 +++++++++--
 unpack-trees.h                              |   1 +
 wrapper.c                                   |   3 +
 19 files changed, 676 insertions(+), 44 deletions(-)
 create mode 100644 Documentation/recurse-submodules-update.txt

-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
@ 2014-02-03 19:48       ` Jens Lehmann
  2014-02-03 22:23         ` Junio C Hamano
  2014-02-04  0:01         ` Jonathan Nieder
  2014-02-03 19:49       ` [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option Jens Lehmann
                         ` (7 subsequent siblings)
  8 siblings, 2 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:48 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

This commit adds the functions and files needed for configuration,
documentation, setting the default behavior and determining if a
submodule path should be updated automatically.

It won't really enable recursive submodule update. This will be done
by later commits.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/recurse-submodules-update.txt |  8 +++++
 submodule.c                                 | 50 +++++++++++++++++++++++++++++
 submodule.h                                 |  6 ++++
 3 files changed, 64 insertions(+)
 create mode 100644 Documentation/recurse-submodules-update.txt

diff --git a/Documentation/recurse-submodules-update.txt b/Documentation/recurse-submodules-update.txt
new file mode 100644
index 0000000..e57d452
--- /dev/null
+++ b/Documentation/recurse-submodules-update.txt
@@ -0,0 +1,8 @@
+--[no-]recurse-submodules::
+	Using --recurse-submodules will update the work tree of all
+	initialized submodules according to the commit recorded in the
+	superproject if their update configuration is set to checkout'. If
+	local modifications in a submodule would be overwritten the checkout
+	will fail unless forced. Without this option or with
+	--no-recurse-submodules is, the work trees of submodules will not be
+	updated, only the hash recorded in the superproject will be updated.
diff --git a/submodule.c b/submodule.c
index 613857e..b3eb28d 100644
--- a/submodule.c
+++ b/submodule.c
@@ -16,6 +16,8 @@ static struct string_list config_name_for_path;
 static struct string_list config_fetch_recurse_submodules_for_name;
 static struct string_list config_ignore_for_name;
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
+static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
+static int option_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static struct string_list changed_submodule_paths;
 static int initialized_fetch_ref_tips;
 static struct sha1_array ref_tips_before_fetch;
@@ -382,6 +384,48 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
 	}
 }

+int parse_update_recurse_submodules_arg(const char *opt, const char *arg)
+{
+	switch (git_config_maybe_bool(opt, arg)) {
+	case 1:
+		return RECURSE_SUBMODULES_ON;
+	case 0:
+		return RECURSE_SUBMODULES_OFF;
+	default:
+		if (!strcmp(arg, "checkout"))
+			return RECURSE_SUBMODULES_ON;
+		die("bad %s argument: %s", opt, arg);
+	}
+}
+
+int option_parse_update_submodules(const struct option *opt,
+				   const char *arg, int unset)
+{
+	if (unset) {
+		*(int *)opt->value = RECURSE_SUBMODULES_OFF;
+	} else {
+		if (arg)
+			*(int *)opt->value = parse_update_recurse_submodules_arg(opt->long_name, arg);
+		else
+			*(int *)opt->value = RECURSE_SUBMODULES_ON;
+	}
+	return 0;
+}
+
+int submodule_needs_update(const char *path)
+{
+	struct string_list_item *path_option;
+	path_option = unsorted_string_list_lookup(&config_name_for_path, path);
+	if (!path_option)
+		return 0;
+
+	/* update can't be "none", "merge" or "rebase" */
+
+	if (option_update_recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
+		return 1;
+	return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
+}
+
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
@@ -589,6 +633,12 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
 	return ret;
 }

+void set_config_update_recurse_submodules(int default_value, int option_value)
+{
+	config_update_recurse_submodules = default_value;
+	option_update_recurse_submodules = option_value;
+}
+
 static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
 {
 	int is_present = 0;
diff --git a/submodule.h b/submodule.h
index 7beec48..79b336b 100644
--- a/submodule.h
+++ b/submodule.h
@@ -3,6 +3,7 @@

 struct diff_options;
 struct argv_array;
+struct option;

 enum {
 	RECURSE_SUBMODULES_ON_DEMAND = -1,
@@ -22,12 +23,17 @@ void gitmodules_config(void);
 int parse_submodule_config_option(const char *var, const char *value);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
 int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
+int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
+int option_parse_update_submodules(const struct option *opt,
+		const char *arg, int unset);
+int submodule_needs_update(const char *path);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule, const char *meta,
 		const char *del, const char *add, const char *reset);
 void set_config_fetch_recurse_submodules(int value);
+void set_config_update_recurse_submodules(int default_value, int option_value);
 void check_for_new_submodule_commits(unsigned char new_sha1[20]);
 int fetch_populated_submodules(const struct argv_array *options,
 			       const char *prefix, int command_line_option,
-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
  2014-02-03 19:48       ` [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules Jens Lehmann
@ 2014-02-03 19:49       ` Jens Lehmann
  2014-02-03 22:40         ` Junio C Hamano
  2014-02-03 19:50       ` [WIP/PATCH 3/9] Teach checkout " Jens Lehmann
                         ` (6 subsequent siblings)
  8 siblings, 1 reply; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:49 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

This new option will allow the user to not only reset the work tree of
the superproject but to also update the work tree of all initialized
submodules (so they match the SHA-1 recorded in the superproject) when
used together with --hard or --merge. But this commit only adds the
option without any functionality, that will be added to unpack_trees()
in subsequent commits.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/git-reset.txt |  4 ++++
 builtin/reset.c             | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index f445cb3..8f833f4 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -94,6 +94,10 @@ OPTIONS
 --quiet::
 	Be quiet, only report errors.

+include::recurse-submodules-update.txt[]
++
+This option only makes sense together with `--hard` and `--merge` and is
+ignored when used without these options.

 EXAMPLES
 --------
diff --git a/builtin/reset.c b/builtin/reset.c
index 6004803..adf372e 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -20,6 +20,7 @@
 #include "parse-options.h"
 #include "unpack-trees.h"
 #include "cache-tree.h"
+#include "submodule.h"

 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
@@ -255,6 +256,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
 	int patch_mode = 0, unborn;
+	const char *recurse_submodules_default = "off";
+	int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 	const char *rev;
 	unsigned char sha1[20];
 	struct pathspec pathspec;
@@ -270,13 +273,24 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "keep", &reset_type,
 				N_("reset HEAD but keep local changes"), KEEP),
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
+		{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+			"checkout", "control recursive updating of submodules",
+			PARSE_OPT_OPTARG, option_parse_update_submodules },
+		{ OPTION_STRING, 0, "recurse-submodules-default",
+			&recurse_submodules_default, NULL,
+			"default mode for recursion", PARSE_OPT_HIDDEN },
 		OPT_END()
 	};

+	gitmodules_config();
 	git_config(git_default_config, NULL);

 	argc = parse_options(argc, argv, prefix, options, git_reset_usage,
 						PARSE_OPT_KEEP_DASHDASH);
+	set_config_update_recurse_submodules(
+		parse_update_recurse_submodules_arg("--recurse-submodules-default",
+						    recurse_submodules_default),
+		recurse_submodules);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);

 	unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", sha1);
-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [WIP/PATCH 3/9] Teach checkout the --[no-]recurse-submodules option
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
  2014-02-03 19:48       ` [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules Jens Lehmann
  2014-02-03 19:49       ` [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option Jens Lehmann
@ 2014-02-03 19:50       ` Jens Lehmann
  2014-02-03 22:56         ` Junio C Hamano
  2014-02-03 19:50       ` [WIP/PATCH 4/9] Teach merge " Jens Lehmann
                         ` (5 subsequent siblings)
  8 siblings, 1 reply; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:50 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

This new option will allow the user to not only update the work tree of
the superproject according to the checked out commit but to also update
the work tree of all initialized submodules (so they match the SHA-1
recorded in the superproject). But this commit only adds the option
without any functionality, that will be added to unpack_trees() in
subsequent commits.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/git-checkout.txt |   2 +
 builtin/checkout.c             |  14 +++
 t/t2013-checkout-submodule.sh  | 215 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 228 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 91294f8..6c7d31f 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -225,6 +225,8 @@ This means that you can use `git checkout -p` to selectively discard
 edits from your current working tree. See the ``Interactive Mode''
 section of linkgit:git-add[1] to learn how to operate the `--patch` mode.

+include::recurse-submodules-update.txt[]
+
 <branch>::
 	Branch to checkout; if it refers to a branch (i.e., a name that,
 	when prepended with "refs/heads/", is a valid ref), then that
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5df3837..e4ef0ba 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -21,6 +21,9 @@
 #include "submodule.h"
 #include "argv-array.h"

+static const char *recurse_submodules_default = "off";
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+
 static const char * const checkout_usage[] = {
 	N_("git checkout [options] <branch>"),
 	N_("git checkout [options] [<branch>] -- <file>..."),
@@ -1111,6 +1114,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			 N_("do not limit pathspecs to sparse entries only")),
 		OPT_HIDDEN_BOOL(0, "guess", &dwim_new_local_branch,
 				N_("second guess 'git checkout no-such-branch'")),
+		{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+			    "checkout", "control recursive updating of submodules",
+			    PARSE_OPT_OPTARG, option_parse_update_submodules },
+		{ OPTION_STRING, 0, "recurse-submodules-default",
+			   &recurse_submodules_default, NULL,
+			   "default mode for recursion", PARSE_OPT_HIDDEN },
 		OPT_END(),
 	};

@@ -1132,6 +1141,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
 	}

+	set_config_update_recurse_submodules(
+		parse_update_recurse_submodules_arg("--recurse-submodules-default",
+						    recurse_submodules_default),
+		recurse_submodules);
+
 	if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
 		die(_("-b, -B and --orphan are mutually exclusive"));

diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 06b18f8..bc3e1ca 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -4,17 +4,57 @@ test_description='checkout can handle submodules'

 . ./test-lib.sh

+submodule_creation_must_succeed() {
+	# checkout base ($1)
+	git checkout -f --recurse-submodules $1 &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached $1 &&
+
+	# checkout target ($2)
+	if test -d submodule; then
+		echo change>>submodule/first.t &&
+		test_must_fail git checkout --recurse-submodules $2 &&
+		git checkout -f --recurse-submodules $2
+	else
+		git checkout --recurse-submodules $2
+	fi &&
+	test -e submodule/.git &&
+	test -f submodule/first.t &&
+	test -f submodule/second.t &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached $2
+}
+
+submodule_removal_must_succeed() {
+	# checkout base ($1)
+	git checkout -f --recurse-submodules $1 &&
+	git submodule update -f &&
+	test -e submodule/.git &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached $1 &&
+
+	# checkout target ($2)
+	echo change>>submodule/first.t &&
+	test_must_fail git checkout --recurse-submodules $2 &&
+	git checkout -f --recurse-submodules $2 &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached $2 &&
+	! test -d submodule
+}
+
 test_expect_success 'setup' '
 	mkdir submodule &&
 	(cd submodule &&
 	 git init &&
 	 test_commit first) &&
-	git add submodule &&
+	echo first > file &&
+	git add file submodule &&
 	test_tick &&
 	git commit -m superproject &&
 	(cd submodule &&
 	 test_commit second) &&
-	git add submodule &&
+	echo second > file &&
+	git add file submodule &&
 	test_tick &&
 	git commit -m updated.superproject
 '
@@ -36,7 +76,8 @@ test_expect_success '"checkout <submodule>" updates the index only' '
 	git checkout HEAD^ submodule &&
 	test_must_fail git diff-files --quiet &&
 	git checkout HEAD submodule &&
-	git diff-files --quiet
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD
 '

 test_expect_success '"checkout <submodule>" honors diff.ignoreSubmodules' '
@@ -62,4 +103,172 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
 	! test -s actual
 '

+test_expect_success '"checkout --recurse-submodules" removes deleted submodule' '
+	git config -f .gitmodules submodule.submodule.path submodule &&
+	git config -f .gitmodules submodule.submodule.url submodule.bare &&
+	(cd submodule && git clone --bare . ../submodule.bare) &&
+	echo submodule.bare >>.gitignore &&
+	git config submodule.submodule.ignore none &&
+	git add .gitignore .gitmodules submodule &&
+	git submodule update --init &&
+	git commit -m "submodule registered" &&
+	git checkout -b base &&
+	git checkout -b delete_submodule &&
+	rm -rf submodule &&
+	git rm submodule &&
+	git commit -m "submodule deleted" &&
+	submodule_removal_must_succeed base delete_submodule
+'
+
+test_expect_success '"checkout --recurse-submodules" repopulates submodule' '
+	submodule_creation_must_succeed delete_submodule base
+'
+
+test_expect_success '"checkout --recurse-submodules" repopulates submodule in existing directory' '
+	git checkout --recurse-submodules delete_submodule &&
+	mkdir submodule &&
+	submodule_creation_must_succeed delete_submodule base
+'
+
+test_expect_success '"checkout --recurse-submodules" replaces submodule with files' '
+	git checkout -f base &&
+	git checkout -b replace_submodule_with_dir &&
+	git update-index --force-remove submodule &&
+	rm -rf submodule/.git .gitmodules &&
+	git add .gitmodules submodule/* &&
+	git commit -m "submodule replaced" &&
+	git checkout -f base &&
+	git submodule update -f &&
+	git checkout --recurse-submodules replace_submodule_with_dir &&
+	test -d submodule &&
+	! test -e submodule/.git &&
+	test -f submodule/first.t &&
+	test -f submodule/second.t
+'
+
+test_expect_success '"checkout --recurse-submodules" removes files and repopulates submodule' '
+	submodule_creation_must_succeed replace_submodule_with_dir base
+'
+
+test_expect_failure '"checkout --recurse-submodules" replaces submodule with a file' '
+	git checkout -f base &&
+	git checkout -b replace_submodule_with_file &&
+	git update-index --force-remove submodule &&
+	rm -rf submodule .gitmodules &&
+	echo content >submodule &&
+	git add .gitmodules submodule &&
+	git commit -m "submodule replaced with file" &&
+	git checkout -f base &&
+	git submodule update -f &&
+	git checkout --recurse-submodules replace_submodule_with_file &&
+	test -d submodule &&
+	! test -e submodule/.git &&
+	test -f submodule/first.t &&
+	test -f submodule/second.t
+'
+
+test_expect_success '"checkout --recurse-submodules" removes the file and repopulates submodule' '
+	submodule_creation_must_succeed replace_submodule_with_file base
+'
+
+test_expect_failure '"checkout --recurse-submodules" replaces submodule with a link' '
+	git checkout -f base &&
+	git checkout -b replace_submodule_with_link &&
+	git update-index --force-remove submodule &&
+	rm -rf submodule .gitmodules &&
+	ln -s submodule &&
+	git add .gitmodules submodule &&
+	git commit -m "submodule replaced with link" &&
+	git checkout -f base &&
+	git submodule update -f &&
+	git checkout --recurse-submodules replace_submodule_with_link &&
+	test -d submodule &&
+	! test -e submodule/.git &&
+	test -f submodule/first.t &&
+	test -f submodule/second.t
+'
+
+test_expect_success '"checkout --recurse-submodules" removes the link and repopulates submodule' '
+	submodule_creation_must_succeed replace_submodule_with_link base
+'
+
+test_expect_success '"checkout --recurse-submodules" updates recursively' '
+	git checkout --recurse-submodules base &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD &&
+	git checkout -b updated_submodule &&
+	(cd submodule &&
+	 echo x >>first.t &&
+	 git add first.t &&
+	 test_commit third) &&
+	git add submodule &&
+	test_tick &&
+	git commit -m updated.superproject &&
+	git checkout --recurse-submodules base &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" needs -f to update a modifed submodule commit' '
+	(
+		cd submodule &&
+		git checkout --recurse-submodules HEAD^
+	) &&
+	test_must_fail git checkout --recurse-submodules master &&
+	test_must_fail git diff-files --quiet submodule &&
+	git diff-files --quiet file &&
+	git checkout --recurse-submodules -f master &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" needs -f to update modifed submodule content' '
+	echo modified >submodule/second.t &&
+	test_must_fail git checkout --recurse-submodules HEAD^ &&
+	test_must_fail git diff-files --quiet submodule &&
+	git diff-files --quiet file &&
+	git checkout --recurse-submodules -f HEAD^ &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD &&
+	git checkout --recurse-submodules -f master &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" ignores modified submodule content that would not be changed' '
+	echo modified >expected &&
+	cp expected submodule/first.t &&
+	git checkout --recurse-submodules HEAD^ &&
+	test_cmp expected submodule/first.t &&
+	test_must_fail git diff-files --quiet submodule &&
+	git diff-index --quiet --cached HEAD &&
+	git checkout --recurse-submodules -f master &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD
+'
+
+test_expect_failure '"checkout --recurse-submodules" does not care about untracked submodule content' '
+	echo untracked >submodule/untracked &&
+	git checkout --recurse-submodules master &&
+	git diff-files --quiet --ignore-submodules=untracked &&
+	git diff-index --quiet --cached HEAD &&
+	rm submodule/untracked
+'
+
+test_expect_failure '"checkout --recurse-submodules" needs -f when submodule commit is not present (but does fail anyway)' '
+	git checkout --recurse-submodules -b bogus_commit master &&
+	git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submodule
+	BOGUS_TREE=$(git write-tree) &&
+	BOGUS_COMMIT=$(echo "bogus submodule commit" | git commit-tree $BOGUS_TREE) &&
+	git commit -m "bogus submodule commit" &&
+	git checkout --recurse-submodules -f master &&
+	test_must_fail git checkout --recurse-submodules bogus_commit &&
+	git diff-files --quiet &&
+	test_must_fail git checkout --recurse-submodules -f bogus_commit &&
+	test_must_fail git diff-files --quiet submodule &&
+	git diff-files --quiet file &&
+	git diff-index --quiet --cached HEAD &&
+	git checkout --recurse-submodules -f master
+'
+
 test_done
-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [WIP/PATCH 4/9] Teach merge the --[no-]recurse-submodules option
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
                         ` (2 preceding siblings ...)
  2014-02-03 19:50       ` [WIP/PATCH 3/9] Teach checkout " Jens Lehmann
@ 2014-02-03 19:50       ` Jens Lehmann
  2014-02-03 23:01         ` Junio C Hamano
  2014-02-03 19:51       ` [WIP/PATCH 5/9] Teach bisect--helper " Jens Lehmann
                         ` (4 subsequent siblings)
  8 siblings, 1 reply; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:50 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

This new option will allow the user to not only update the work tree of
the superproject according to the merge result but to also update the
work tree of all initialized submodules (so they match the SHA-1 recorded
in the superproject). But this commit only adds the option without any
functionality, that will be added to unpack_trees() in subsequent commits.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/git-merge.txt |  2 ++
 builtin/merge.c             | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 4395459..9ed1655 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -96,6 +96,8 @@ commit or stash your changes before running 'git merge'.
 'git merge --abort' is equivalent to 'git reset --merge' when
 `MERGE_HEAD` is present.

+include::recurse-submodules-update.txt[]
+
 <commit>...::
 	Commits, usually other branch heads, to merge into our branch.
 	Specifying more than one commit will create a merge with
diff --git a/builtin/merge.c b/builtin/merge.c
index 4941a6c..a0eb665 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -28,6 +28,7 @@
 #include "remote.h"
 #include "fmt-merge-msg.h"
 #include "gpg-interface.h"
+#include "submodule.h"

 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -65,6 +66,8 @@ static int abort_current_merge;
 static int show_progress = -1;
 static int default_to_upstream;
 static const char *sign_commit;
+static const char *recurse_submodules_default = "off";
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;

 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -223,6 +226,12 @@ static struct option builtin_merge_options[] = {
 	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key id"),
 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
+	{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+		"checkout", "control recursive updating of submodules",
+		PARSE_OPT_OPTARG, option_parse_update_submodules },
+	{ OPTION_STRING, 0, "recurse-submodules-default",
+		&recurse_submodules_default, NULL,
+		"default mode for recursion", PARSE_OPT_HIDDEN },
 	OPT_END()
 };

@@ -1113,6 +1122,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	else
 		head_commit = lookup_commit_or_die(head_sha1, "HEAD");

+	gitmodules_config();
 	git_config(git_merge_config, NULL);

 	if (branch_mergeoptions)
@@ -1121,6 +1131,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			builtin_merge_usage, 0);
 	if (shortlog_len < 0)
 		shortlog_len = (merge_log_config > 0) ? merge_log_config : 0;
+	set_config_update_recurse_submodules(
+		parse_update_recurse_submodules_arg("--recurse-submodules-default",
+						    recurse_submodules_default),
+		recurse_submodules);

 	if (verbosity < 0 && show_progress == -1)
 		show_progress = 0;
-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [WIP/PATCH 5/9] Teach bisect--helper the --[no-]recurse-submodules option
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
                         ` (3 preceding siblings ...)
  2014-02-03 19:50       ` [WIP/PATCH 4/9] Teach merge " Jens Lehmann
@ 2014-02-03 19:51       ` Jens Lehmann
  2014-02-03 19:51       ` [WIP/PATCH 6/9] Teach bisect " Jens Lehmann
                         ` (3 subsequent siblings)
  8 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:51 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

This is necessary before we can teach 'git bisect' this option, as that
calls the bisect--helper to do the actual work which then in turn calls
'git checkout'. The helper just passes the option given on the command
line on to checkout. The new recurse_submodules_enum_to_option() is added
to avoid having the helper learn the command line representation of the
different option values himself.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 bisect.c                 | 33 ++++++++++++++++++++++-----------
 bisect.h                 |  3 ++-
 builtin/bisect--helper.c |  9 ++++++++-
 submodule.c              | 21 +++++++++++++++++++++
 submodule.h              |  1 +
 5 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/bisect.c b/bisect.c
index 37200b4..b84e607 100644
--- a/bisect.c
+++ b/bisect.c
@@ -11,13 +11,13 @@
 #include "bisect.h"
 #include "sha1-array.h"
 #include "argv-array.h"
+#include "submodule.h"

 static struct sha1_array good_revs;
 static struct sha1_array skipped_revs;

 static unsigned char *current_bad_sha1;

-static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
 static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
 static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD", NULL, NULL};

@@ -683,22 +683,30 @@ static void mark_expected_rev(char *bisect_rev_hex)
 		die("closing file %s: %s", filename, strerror(errno));
 }

-static int bisect_checkout(char *bisect_rev_hex, int no_checkout)
+static int bisect_checkout(char *bisect_rev_hex, int no_checkout,
+			   const char *recurse_submodules)
 {
 	int res;

 	mark_expected_rev(bisect_rev_hex);

-	argv_checkout[2] = bisect_rev_hex;
 	if (no_checkout) {
 		argv_update_ref[3] = bisect_rev_hex;
 		if (run_command_v_opt(argv_update_ref, RUN_GIT_CMD))
 			die("update-ref --no-deref HEAD failed on %s",
 			    bisect_rev_hex);
 	} else {
-		res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
+		struct argv_array argv = ARGV_ARRAY_INIT;
+		argv_array_push(&argv, "checkout");
+		argv_array_push(&argv, "-q");
+		if (recurse_submodules)
+		    argv_array_push(&argv, recurse_submodules);
+		argv_array_push(&argv, bisect_rev_hex);
+		argv_array_push(&argv, "--");
+		res = run_command_v_opt(argv.argv, RUN_GIT_CMD);
 		if (res)
 			exit(res);
+		argv_array_clear(&argv);
 	}

 	argv_show_branch[1] = bisect_rev_hex;
@@ -771,7 +779,7 @@ static void handle_skipped_merge_base(const unsigned char *mb)
  * - If one is "skipped", we can't know but we should warn.
  * - If we don't know, we should check it out and ask the user to test.
  */
-static void check_merge_bases(int no_checkout)
+static void check_merge_bases(int no_checkout, const char *recurse_submodules)
 {
 	struct commit_list *result;
 	int rev_nr;
@@ -789,7 +797,8 @@ static void check_merge_bases(int no_checkout)
 			handle_skipped_merge_base(mb);
 		} else {
 			printf("Bisecting: a merge base must be tested\n");
-			exit(bisect_checkout(sha1_to_hex(mb), no_checkout));
+			exit(bisect_checkout(sha1_to_hex(mb), no_checkout,
+					     recurse_submodules));
 		}
 	}

@@ -832,7 +841,8 @@ static int check_ancestors(const char *prefix)
  * If a merge base must be tested by the user, its source code will be
  * checked out to be tested by the user and we will exit.
  */
-static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
+static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout,
+					    const char *recurse_submodules)
 {
 	char *filename = git_pathdup("BISECT_ANCESTORS_OK");
 	struct stat st;
@@ -851,7 +861,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)

 	/* Check if all good revs are ancestor of the bad rev. */
 	if (check_ancestors(prefix))
-		check_merge_bases(no_checkout);
+		check_merge_bases(no_checkout, recurse_submodules);

 	/* Create file BISECT_ANCESTORS_OK. */
 	fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
@@ -897,7 +907,8 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
  * If no_checkout is non-zero, the bisection process does not
  * checkout the trial commit but instead simply updates BISECT_HEAD.
  */
-int bisect_next_all(const char *prefix, int no_checkout)
+int bisect_next_all(const char *prefix, int no_checkout,
+		    const char *recurse_submodules)
 {
 	struct rev_info revs;
 	struct commit_list *tried;
@@ -908,7 +919,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
 	if (read_bisect_refs())
 		die("reading bisect refs failed");

-	check_good_are_ancestors_of_bad(prefix, no_checkout);
+	check_good_are_ancestors_of_bad(prefix, no_checkout, recurse_submodules);

 	bisect_rev_setup(&revs, prefix, "%s", "^%s", 1);
 	revs.limited = 1;
@@ -954,7 +965,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
 	       "(roughly %d step%s)\n", nr, (nr == 1 ? "" : "s"),
 	       steps, (steps == 1 ? "" : "s"));

-	return bisect_checkout(bisect_rev_hex, no_checkout);
+	return bisect_checkout(bisect_rev_hex, no_checkout, recurse_submodules);
 }

 static inline int log2i(int n)
diff --git a/bisect.h b/bisect.h
index 2a6c831..5c1ea9c 100644
--- a/bisect.h
+++ b/bisect.h
@@ -22,7 +22,8 @@ struct rev_list_info {
 	const char *header_prefix;
 };

-extern int bisect_next_all(const char *prefix, int no_checkout);
+extern int bisect_next_all(const char *prefix, int no_checkout,
+			   const char *recurse_submodules);

 extern int estimate_bisect_steps(int all);

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3324229..b30087a 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "parse-options.h"
 #include "bisect.h"
+#include "submodule.h"

 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --next-all [--no-checkout]"),
@@ -12,11 +13,16 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	int next_all = 0;
 	int no_checkout = 0;
+	int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+
 	struct option options[] = {
 		OPT_BOOL(0, "next-all", &next_all,
 			 N_("perform 'git bisect next'")),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
+		{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+			"checkout", "control recursive updating of submodules",
+			PARSE_OPT_OPTARG, option_parse_update_submodules },
 		OPT_END()
 	};

@@ -27,5 +33,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_bisect_helper_usage, options);

 	/* next-all */
-	return bisect_next_all(prefix, no_checkout);
+	return bisect_next_all(prefix, no_checkout,
+			       recurse_submodules_enum_to_option(recurse_submodules));
 }
diff --git a/submodule.c b/submodule.c
index b3eb28d..448b645 100644
--- a/submodule.c
+++ b/submodule.c
@@ -44,6 +44,27 @@ static int gitmodules_is_unmerged;
 static int gitmodules_is_modified;


+/*
+ * Convert values defined in the RECURSE_SUBMODULES_* enum to the string
+ * representation usable as command parameter. Returns NULL if no parameter
+ * is necessary.
+ */
+const char *recurse_submodules_enum_to_option(int recurse_submodules)
+{
+	switch(recurse_submodules) {
+	case RECURSE_SUBMODULES_ON_DEMAND:
+		return "--recurse-submodules=on-demand";
+	case RECURSE_SUBMODULES_OFF:
+		return "--no-recurse-submodules";
+	case RECURSE_SUBMODULES_ON:
+		return "--recurse-submodules";
+	case RECURSE_SUBMODULES_DEFAULT:
+		return NULL;
+	default:
+		die("Invalid recurse submodule value: %d", recurse_submodules);
+	}
+}
+
 int is_staging_gitmodules_ok(void)
 {
 	return !gitmodules_is_modified;
diff --git a/submodule.h b/submodule.h
index 79b336b..5958010 100644
--- a/submodule.h
+++ b/submodule.h
@@ -11,6 +11,7 @@ enum {
 	RECURSE_SUBMODULES_DEFAULT = 1,
 	RECURSE_SUBMODULES_ON = 2
 };
+const char *recurse_submodules_enum_to_option(int recurse_submodules);

 int is_staging_gitmodules_ok(void);
 int update_path_in_gitmodules(const char *oldpath, const char *newpath);
-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [WIP/PATCH 6/9] Teach bisect the --[no-]recurse-submodules option
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
                         ` (4 preceding siblings ...)
  2014-02-03 19:51       ` [WIP/PATCH 5/9] Teach bisect--helper " Jens Lehmann
@ 2014-02-03 19:51       ` Jens Lehmann
  2014-02-03 20:04         ` W. Trevor King
  2014-02-03 19:52       ` [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents Jens Lehmann
                         ` (2 subsequent siblings)
  8 siblings, 1 reply; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:51 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

When using this option 'git bisect' will automatically update the work
tree of all initialized submodules (so they match the SHA-1 recorded in
the superproject) in each bisection step. This makes calling 'git
submodule update' eacht time obsolete, which was tedious and error prone.
If the option is given it is stored in the BISECT_RECURSE_SUBMODULES file
in the git directory so that later bisection steps can reuse it. But this
commit only adds the option without any functionality, that will be added
to unpack_trees() in subsequent commits.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/git-bisect.txt |  5 +++++
 git-bisect.sh                | 29 ++++++++++++++++++++++++-----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index f986c5c..c0aaba8 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -276,6 +276,11 @@ does not require a checked out tree.
 +
 If the repository is bare, `--no-checkout` is assumed.

+include::recurse-submodules-update.txt[]
++
+This option is passed to linkgit:git-checkout[1] when checking out the next
+to be tested commit and is ignored when used together with `--no-checkout`.
+
 EXAMPLES
 --------

diff --git a/git-bisect.sh b/git-bisect.sh
index 73b4c14..ba64a21 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -3,7 +3,7 @@
 USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
 	print this long help message.
-git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
+git bisect start [--no-checkout] [--[no-]recurse-submodules[=<mode>]] [<bad> [<good>...]] [--] [<pathspec>...]
 	reset bisect state and start bisection.
 git bisect bad [<rev>]
 	mark <rev> a known-bad revision.
@@ -91,6 +91,12 @@ bisect_start() {
 		--no-checkout)
 			mode=--no-checkout
 			shift ;;
+		--no-recurse-submodules)
+			recurse_submodules="$1"
+			shift ;;
+		--recurse-submodules*)
+			recurse_submodules="$1"
+			shift ;;
 		--*)
 			die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
 		*)
@@ -124,9 +130,13 @@ bisect_start() {
 	then
 		# Reset to the rev from where we started.
 		start_head=$(cat "$GIT_DIR/BISECT_START")
+		if test -s "$GIT_DIR/BISECT_RECURSE_SUBMODULES"
+		then
+			recurse_submodules=$(cat "$GIT_DIR/BISECT_RECURSE_SUBMODULES")
+		fi
 		if test "z$mode" != "z--no-checkout"
 		then
-			git checkout "$start_head" -- ||
+			git checkout ${recurse_submodules:+"$recurse_submodules"} "$start_head" -- ||
 			die "$(eval_gettext "Checking out '\$start_head' failed. Try 'git bisect reset <validbranch>'.")"
 		fi
 	else
@@ -168,7 +178,10 @@ bisect_start() {
 		test "z$mode" != "z--no-checkout" ||
 		git update-ref --no-deref BISECT_HEAD "$start_head"
 	} &&
-	git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
+	git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" && {
+		test -z "$recurse_submodules" ||
+		echo "$recurse_submodules" >"$GIT_DIR/BISECT_RECURSE_SUBMODULES"
+	} &&
 	eval "$eval true" &&
 	echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
 	#
@@ -306,8 +319,13 @@ bisect_next() {
 	bisect_autostart
 	bisect_next_check good

+	if test -f "$GIT_DIR/BISECT_RECURSE_SUBMODULES"
+	then
+		recurse_submodules=$(cat "$GIT_DIR/BISECT_RECURSE_SUBMODULES")
+	fi
+
 	# Perform all bisection computation, display and checkout
-	git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout)
+	git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout) ${recurse_submodules:+"$recurse_submodules"}
 	res=$?

 	# Check if we should exit because bisection is finished
@@ -374,7 +392,7 @@ bisect_reset() {
 		usage ;;
 	esac

-	if ! test -f "$GIT_DIR/BISECT_HEAD" && ! git checkout "$branch" --
+	if ! test -f "$GIT_DIR/BISECT_HEAD" && ! git checkout ${recurse_submodules:+"$recurse_submodules"} "$branch" --
 	then
 		die "$(eval_gettext "Could not check out original HEAD '\$branch'.
 Try 'git bisect reset <commit>'.")"
@@ -397,6 +415,7 @@ bisect_clean_state() {
 	# Cleanup head-name if it got left by an old version of git-bisect
 	rm -f "$GIT_DIR/head-name" &&
 	git update-ref -d --no-deref BISECT_HEAD &&
+	rm -f "$GIT_DIR/BISECT_RECURSE_SUBMODULES" &&
 	# clean up BISECT_START last
 	rm -f "$GIT_DIR/BISECT_START"
 }
-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
                         ` (5 preceding siblings ...)
  2014-02-03 19:51       ` [WIP/PATCH 6/9] Teach bisect " Jens Lehmann
@ 2014-02-03 19:52       ` Jens Lehmann
  2014-02-03 20:10         ` W. Trevor King
  2014-02-03 19:53       ` [WIP/PATCH 8/9] submodule: teach unpack_trees() to repopulate submodules Jens Lehmann
  2014-02-03 19:54       ` [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules Jens Lehmann
  8 siblings, 1 reply; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:52 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

Implement the functionality needed to enable work tree manipulating
commands to that a deleted submodule should not only affect the index
(leaving all the files of the submodule in the work tree) but also to
remove the work tree of the superproject (including any untracked
files).

That will only work properly when the submodule uses a gitfile instead of
a .git directory and no untracked files are present. Otherwise the removal
will fail with a warning (which is just what happened until now).

Extend rmdir_or_warn() to remove the directories of those submodules which
are scheduled for removal. Also teach verify_clean_submodule() to check
that a submodule configured to be removed is not modified before scheduling
it for removal.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 submodule.c    | 37 +++++++++++++++++++++++++++++++++++++
 submodule.h    |  1 +
 unpack-trees.c |  7 ++++---
 wrapper.c      |  3 +++
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/submodule.c b/submodule.c
index 448b645..f292e9e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -447,6 +447,43 @@ int submodule_needs_update(const char *path)
 	return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
 }

+int depopulate_submodule(const char *path)
+{
+	struct strbuf dot_git = STRBUF_INIT;
+	struct child_process cp;
+	const char *argv[] = {"rm", "-rf", path, NULL};
+
+	/* Is it populated? */
+	strbuf_addf(&dot_git, "%s/.git", path);
+	if (!resolve_gitdir(dot_git.buf)) {
+		strbuf_release(&dot_git);
+		return 0;
+	}
+	strbuf_release(&dot_git);
+
+	/* Does it have a .git directory? */
+	if (!submodule_uses_gitfile(path)) {
+		warning(_("cannot remove submodule '%s' because it (or one of "
+			  "its nested submodules) uses a .git directory"),
+			  path);
+		return -1;
+	}
+
+	/* Remove the whole submodule directory */
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 0;
+	cp.no_stdin = 1;
+	if (run_command(&cp)) {
+		warning("Could not remove submodule %s", path);
+		strbuf_release(&dot_git);
+		return -1;
+	}
+
+	return 0;
+}
+
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
diff --git a/submodule.h b/submodule.h
index 5958010..2139e08 100644
--- a/submodule.h
+++ b/submodule.h
@@ -28,6 +28,7 @@ int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
 int option_parse_update_submodules(const struct option *opt,
 		const char *arg, int unset);
 int submodule_needs_update(const char *path);
+int depopulate_submodule(const char *path);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
diff --git a/unpack-trees.c b/unpack-trees.c
index 164354d..82c99eb 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -8,6 +8,7 @@
 #include "progress.h"
 #include "refs.h"
 #include "attr.h"
+#include "submodule.h"

 /*
  * Error messages expected by scripts out of plumbing commands such as
@@ -1266,14 +1267,14 @@ static void invalidate_ce_path(const struct cache_entry *ce,
 /*
  * Check that checking out ce->sha1 in subdir ce->name is not
  * going to overwrite any working files.
- *
- * Currently, git does not checkout subprojects during a superproject
- * checkout, so it is not going to overwrite anything.
  */
 static int verify_clean_submodule(const struct cache_entry *ce,
 				  enum unpack_trees_error_types error_type,
 				  struct unpack_trees_options *o)
 {
+	if (submodule_needs_update(ce->name) &&
+	    is_submodule_modified(ce->name, 0))
+		return 1;
 	return 0;
 }

diff --git a/wrapper.c b/wrapper.c
index 0cc5636..425a3fd 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -2,6 +2,7 @@
  * Various trivial helper wrappers around standard functions
  */
 #include "cache.h"
+#include "submodule.h"

 static void do_nothing(size_t size)
 {
@@ -409,6 +410,8 @@ int unlink_or_warn(const char *file)

 int rmdir_or_warn(const char *file)
 {
+	if (submodule_needs_update(file) && depopulate_submodule(file))
+		return -1;
 	return warn_if_unremovable("rmdir", file, rmdir(file));
 }

-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [WIP/PATCH 8/9] submodule: teach unpack_trees() to repopulate submodules
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
                         ` (6 preceding siblings ...)
  2014-02-03 19:52       ` [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents Jens Lehmann
@ 2014-02-03 19:53       ` Jens Lehmann
  2014-02-03 19:54       ` [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules Jens Lehmann
  8 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:53 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

Implement the functionality needed to enable work tree manipulating
commands so that an added submodule does not only affect the index and
creates an empty directory but it also populates the work tree of any
initialized submodule according to the SHA-1 recorded in the superproject.

That will only work for submodules that store their git directory in the
.git/modules directory of the superproject. Additionally the submodule has
to be found in the .gitmodules file before the work tree update starts as
the git directory is stored under the submodule name, not its path. This
will be fixed when the .gitmodules blob of the tree we are updating to
will be used for the path <-> name mapping in a later commit.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 entry.c        |  4 ++++
 submodule.c    | 44 +++++++++++++++++++++++++++++++++++++++++---
 submodule.h    |  1 +
 unpack-trees.c | 19 +++++++++++++++++++
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/entry.c b/entry.c
index 7b7aa81..d1bf6ec 100644
--- a/entry.c
+++ b/entry.c
@@ -2,6 +2,7 @@
 #include "blob.h"
 #include "dir.h"
 #include "streaming.h"
+#include "submodule.h"

 static void create_directories(const char *path, int path_len,
 			       const struct checkout *state)
@@ -203,6 +204,9 @@ static int write_entry(struct cache_entry *ce,
 			return error("cannot create temporary submodule %s", path);
 		if (mkdir(path, 0777) < 0)
 			return error("cannot create submodule directory %s", path);
+		if (submodule_needs_update(path) &&
+		    populate_submodule(path, ce->sha1, state->force))
+			return error("cannot checkout submodule %s", path);
 		break;
 	default:
 		return error("unknown file mode for %s in index", path);
diff --git a/submodule.c b/submodule.c
index f292e9e..3907034 100644
--- a/submodule.c
+++ b/submodule.c
@@ -447,6 +447,42 @@ int submodule_needs_update(const char *path)
 	return config_update_recurse_submodules != RECURSE_SUBMODULES_OFF;
 }

+int populate_submodule(const char *path, unsigned char sha1[20], int force)
+{
+	struct string_list_item *path_option;
+	const char *name, *real_git_dir;
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process cp;
+	const char *argv[] = {"read-tree", force ? "--reset" : "-m", "-u", NULL, NULL};
+
+	path_option = unsorted_string_list_lookup(&config_name_for_path, path);
+	if (!path_option)
+		return 0;
+
+	name = path_option->util;
+
+	strbuf_addf(&buf, "%s/modules/%s", resolve_gitdir(get_git_dir()), name);
+	real_git_dir = resolve_gitdir(buf.buf);
+	if (!real_git_dir)
+		goto out;
+	connect_work_tree_and_git_dir(path, real_git_dir);
+
+	/* Run read-tree --reset sha1 */
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.dir = path;
+	argv[3] = sha1_to_hex(sha1);
+	if (run_command(&cp))
+		warning(_("Checking out submodule %s failed"), path);
+
+out:
+	strbuf_release(&buf);
+	return 0;
+}
+
 int depopulate_submodule(const char *path)
 {
 	struct strbuf dot_git = STRBUF_INIT;
@@ -1242,6 +1278,7 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
 {
 	struct strbuf file_name = STRBUF_INIT;
 	struct strbuf rel_path = STRBUF_INIT;
+	const char *real_git_dir = xstrdup(real_path(git_dir));
 	const char *real_work_tree = xstrdup(real_path(work_tree));
 	FILE *fp;

@@ -1250,15 +1287,15 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
 	fp = fopen(file_name.buf, "w");
 	if (!fp)
 		die(_("Could not create git link %s"), file_name.buf);
-	fprintf(fp, "gitdir: %s\n", relative_path(git_dir, real_work_tree,
+	fprintf(fp, "gitdir: %s\n", relative_path(real_git_dir, real_work_tree,
 						  &rel_path));
 	fclose(fp);

 	/* Update core.worktree setting */
 	strbuf_reset(&file_name);
-	strbuf_addf(&file_name, "%s/config", git_dir);
+	strbuf_addf(&file_name, "%s/config", real_git_dir);
 	if (git_config_set_in_file(file_name.buf, "core.worktree",
-				   relative_path(real_work_tree, git_dir,
+				   relative_path(real_work_tree, real_git_dir,
 						 &rel_path)))
 		die(_("Could not set core.worktree in %s"),
 		    file_name.buf);
@@ -1266,4 +1303,5 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
 	strbuf_release(&file_name);
 	strbuf_release(&rel_path);
 	free((void *)real_work_tree);
+	free((void *)real_git_dir);
 }
diff --git a/submodule.h b/submodule.h
index 2139e08..a7d09a5 100644
--- a/submodule.h
+++ b/submodule.h
@@ -28,6 +28,7 @@ int parse_update_recurse_submodules_arg(const char *opt, const char *arg);
 int option_parse_update_submodules(const struct option *opt,
 		const char *arg, int unset);
 int submodule_needs_update(const char *path);
+int populate_submodule(const char *path, unsigned char sha1[20], int force);
 int depopulate_submodule(const char *path);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
diff --git a/unpack-trees.c b/unpack-trees.c
index 82c99eb..49d0a67 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1371,6 +1371,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 			      struct unpack_trees_options *o)
 {
 	const struct cache_entry *result;
+	char *name_copy, *separator;

 	/*
 	 * It may be that the 'lstat()' succeeded even though
@@ -1413,6 +1414,24 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
 			return 0;
 	}

+	/*
+	 * If the path lies inside a to be added submodule it
+	 * is ok to remove it.
+	 */
+	name_copy = xstrdup(name);
+	while ((separator = strrchr(name_copy, '/'))) {
+		int i;
+		*separator = '\0';
+		for (i = 0; i < the_index.cache_nr; i++) {
+			struct cache_entry *ce = the_index.cache[i];
+			if (!strcmp(ce->name, name_copy) && S_ISGITLINK(ce->ce_mode)) {
+				free(name_copy);
+				return 0;
+			}
+		}
+	}
+	free(name_copy);
+
 	return o->gently ? -1 :
 		add_rejected_path(o, error_type, name);
 }
-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules
  2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
                         ` (7 preceding siblings ...)
  2014-02-03 19:53       ` [WIP/PATCH 8/9] submodule: teach unpack_trees() to repopulate submodules Jens Lehmann
@ 2014-02-03 19:54       ` Jens Lehmann
  2014-02-03 20:19         ` W. Trevor King
  2014-02-04  0:11         ` Duy Nguyen
  8 siblings, 2 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 19:54 UTC (permalink / raw
  To: Git Mailing List
  Cc: Junio C Hamano, Jonathan Nieder, Heiko Voigt, W. Trevor King

Implement the functionality needed to enable work tree manipulating
commands so that an changed submodule does not only affect the index but
it also updates the work tree of any initialized submodule according to
the SHA-1 recorded in the superproject.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 entry.c        | 15 ++++++++--
 submodule.c    | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 submodule.h    |  3 ++
 unpack-trees.c | 69 ++++++++++++++++++++++++++++++++++++----------
 unpack-trees.h |  1 +
 5 files changed, 157 insertions(+), 17 deletions(-)

diff --git a/entry.c b/entry.c
index d1bf6ec..61a2767 100644
--- a/entry.c
+++ b/entry.c
@@ -265,7 +265,7 @@ int checkout_entry(struct cache_entry *ce,

 	if (!check_path(path, len, &st, state->base_dir_len)) {
 		unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
-		if (!changed)
+		if (!changed && (!S_ISDIR(st.st_mode) || !S_ISGITLINK(ce->ce_mode)))
 			return 0;
 		if (!state->force) {
 			if (!state->quiet)
@@ -280,9 +280,18 @@ int checkout_entry(struct cache_entry *ce,
 		 * just do the right thing)
 		 */
 		if (S_ISDIR(st.st_mode)) {
-			/* If it is a gitlink, leave it alone! */
-			if (S_ISGITLINK(ce->ce_mode))
+			if (S_ISGITLINK(ce->ce_mode)) {
+				if (submodule_needs_update(ce->name)) {
+					if (is_submodule_populated(ce->name)) {
+						if (update_submodule(ce->name, ce->sha1, state->force))
+							return error("cannot checkout submodule %s", path);
+					} else {
+						if (populate_submodule(path, ce->sha1, state->force))
+							return error("cannot populate submodule %s", path);
+					}
+				}
 				return 0;
+			}
 			if (!state->force)
 				return error("%s is a directory", path);
 			remove_subtree(path);
diff --git a/submodule.c b/submodule.c
index 3907034..83e7595 100644
--- a/submodule.c
+++ b/submodule.c
@@ -520,6 +520,42 @@ int depopulate_submodule(const char *path)
 	return 0;
 }

+int update_submodule(const char *path, const unsigned char sha1[20], int force)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process cp;
+	const char *hex_sha1 = sha1_to_hex(sha1);
+	const char *argv[] = {
+		"checkout",
+		force ? "-fq" : "-q",
+		hex_sha1,
+		NULL,
+	};
+	const char *git_dir;
+
+	strbuf_addf(&buf, "%s/.git", path);
+	git_dir = read_gitfile(buf.buf);
+	if (!git_dir)
+		git_dir = buf.buf;
+	if (!is_directory(git_dir)) {
+		strbuf_release(&buf);
+		/* The submodule is not populated, so we can't check it out */
+		return 0;
+	}
+	strbuf_release(&buf);
+
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.dir = path;   /* GIT_WORK_TREE doesn't work for git checkout */
+	if (run_command(&cp))
+		return error("Could not checkout submodule %s", path);
+
+	return 0;
+}
+
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
@@ -961,6 +997,17 @@ out:
 	return result;
 }

+int is_submodule_populated(const char *path)
+{
+	int retval = 0;
+	struct strbuf gitdir = STRBUF_INIT;
+	strbuf_addf(&gitdir, "%s/.git", path);
+	if (resolve_gitdir(gitdir.buf))
+		retval = 1;
+	strbuf_release(&gitdir);
+	return retval;
+}
+
 unsigned is_submodule_modified(const char *path, int ignore_untracked)
 {
 	ssize_t len;
@@ -1110,6 +1157,45 @@ int ok_to_remove_submodule(const char *path)
 	return ok_to_remove;
 }

+unsigned is_submodule_checkout_safe(const char *path, const unsigned char sha1[20])
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process cp;
+	const char *hex_sha1 = sha1_to_hex(sha1);
+	const char *argv[] = {
+		"read-tree",
+		"-n",
+		"-m",
+		"HEAD",
+		hex_sha1,
+		NULL,
+	};
+	const char *git_dir;
+
+	strbuf_addf(&buf, "%s/.git", path);
+	git_dir = read_gitfile(buf.buf);
+	if (!git_dir)
+		git_dir = buf.buf;
+	if (!is_directory(git_dir)) {
+		strbuf_release(&buf);
+		/* The submodule is not populated, it's safe to check it out */
+		/*
+		 * TODO: When git learns to re-populate submodules, a check must be
+		 * added here to assert that no local files will be overwritten.
+		 */
+		return 1;
+	}
+	strbuf_release(&buf);
+
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.dir = path;
+	return run_command(&cp) == 0;
+}
+
 static int find_first_merges(struct object_array *result, const char *path,
 		struct commit *a, struct commit *b)
 {
diff --git a/submodule.h b/submodule.h
index a7d09a5..65f6396 100644
--- a/submodule.h
+++ b/submodule.h
@@ -30,6 +30,7 @@ int option_parse_update_submodules(const struct option *opt,
 int submodule_needs_update(const char *path);
 int populate_submodule(const char *path, unsigned char sha1[20], int force);
 int depopulate_submodule(const char *path);
+int update_submodule(const char *path, const unsigned char sha1[20], int force);
 void show_submodule_summary(FILE *f, const char *path,
 		const char *line_prefix,
 		unsigned char one[20], unsigned char two[20],
@@ -41,9 +42,11 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20]);
 int fetch_populated_submodules(const struct argv_array *options,
 			       const char *prefix, int command_line_option,
 			       int quiet);
+int is_submodule_populated(const char *path);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);
 int submodule_uses_gitfile(const char *path);
 int ok_to_remove_submodule(const char *path);
+unsigned is_submodule_checkout_safe(const char *path, const unsigned char sha1[20]);
 int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
 		    const unsigned char a[20], const unsigned char b[20], int search);
 int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
diff --git a/unpack-trees.c b/unpack-trees.c
index 49d0a67..46b85ac 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -27,6 +27,9 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
 	/* ERROR_NOT_UPTODATE_DIR */
 	"Updating '%s' would lose untracked files in it",

+	/* ERROR_NOT_UPTODATE_SUBMODULE */
+	"Updating submodule '%s' would lose modifications in it",
+
 	/* ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN */
 	"Untracked working tree file '%s' would be overwritten by merge.",

@@ -71,6 +74,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,

 	msgs[ERROR_NOT_UPTODATE_DIR] =
 		"Updating the following directories would lose untracked files in it:\n%s";
+	msgs[ERROR_NOT_UPTODATE_SUBMODULE] =
+		"Updating the following submodules would lose modifications in it:\n%s";

 	if (advice_commit_before_merge)
 		msg = "The following untracked working tree files would be %s by %s:\n%%s"
@@ -1224,17 +1229,15 @@ static int verify_uptodate_1(const struct cache_entry *ce,
 	if (!lstat(ce->name, &st)) {
 		int flags = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE;
 		unsigned changed = ie_match_stat(o->src_index, ce, &st, flags);
-		if (!changed)
-			return 0;
-		/*
-		 * NEEDSWORK: the current default policy is to allow
-		 * submodule to be out of sync wrt the superproject
-		 * index.  This needs to be tightened later for
-		 * submodules that are marked to be automatically
-		 * checked out.
-		 */
-		if (S_ISGITLINK(ce->ce_mode))
-			return 0;
+		if (!changed) {
+			if (!S_ISGITLINK(ce->ce_mode) || !submodule_needs_update(ce->name) ||
+			    (ce_stage(ce) ? is_submodule_checkout_safe(ce->name, ce->sha1)
+			    : !is_submodule_modified(ce->name, 1)))
+				return 0;
+		} else
+			if (S_ISGITLINK(ce->ce_mode) && !submodule_needs_update(ce->name))
+				return 0;
+
 		errno = 0;
 	}
 	if (errno == ENOENT)
@@ -1257,6 +1260,36 @@ static int verify_uptodate_sparse(const struct cache_entry *ce,
 	return verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);
 }

+/*
+ * When a submodule gets turned into an unmerged entry, we want it to be
+ * up-to-date regarding the merge changes.
+ */
+static int verify_uptodate_submodule(const struct cache_entry *old,
+				     const struct cache_entry *new,
+				     struct unpack_trees_options *o)
+{
+	struct stat st;
+
+	if (o->index_only || (!((old->ce_flags & CE_VALID) || ce_skip_worktree(old)) && (o->reset || ce_uptodate(old))))
+		return 0;
+	if (!lstat(old->name, &st)) {
+		unsigned changed = ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
+		if (!changed) {
+			if (!S_ISGITLINK(old->ce_mode) ||
+			    !submodule_needs_update(new->name) ||
+			    is_submodule_checkout_safe(new->name, new->sha1))
+				return 0;
+		} else
+			if (S_ISGITLINK(old->ce_mode) && !submodule_needs_update(new->name))
+				return 0;
+		errno = 0;
+	}
+	if (errno == ENOENT)
+		return 0;
+	return o->gently ? -1 :
+		add_rejected_path(o, ERROR_NOT_UPTODATE_SUBMODULE, old->name);
+}
+
 static void invalidate_ce_path(const struct cache_entry *ce,
 			       struct unpack_trees_options *o)
 {
@@ -1536,9 +1569,17 @@ static int merged_entry(const struct cache_entry *ce,
 			copy_cache_entry(merge, old);
 			update = 0;
 		} else {
-			if (verify_uptodate(old, o)) {
-				free(merge);
-				return -1;
+			if (S_ISGITLINK(old->ce_mode) ||
+			    S_ISGITLINK(merge->ce_mode)) {
+				if (verify_uptodate_submodule(old, merge, o)) {
+					free(merge);
+					return -1;
+				}
+			} else {
+				if (verify_uptodate(old, o)) {
+					free(merge);
+					return -1;
+				}
 			}
 			/* Migrate old flags over */
 			update |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);
diff --git a/unpack-trees.h b/unpack-trees.h
index 36a73a6..bee8740 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -15,6 +15,7 @@ enum unpack_trees_error_types {
 	ERROR_WOULD_OVERWRITE = 0,
 	ERROR_NOT_UPTODATE_FILE,
 	ERROR_NOT_UPTODATE_DIR,
+	ERROR_NOT_UPTODATE_SUBMODULE,
 	ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN,
 	ERROR_WOULD_LOSE_UNTRACKED_REMOVED,
 	ERROR_BIND_OVERLAP,
-- 
1.9.rc0.28.ge3363ff

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 6/9] Teach bisect the --[no-]recurse-submodules option
  2014-02-03 19:51       ` [WIP/PATCH 6/9] Teach bisect " Jens Lehmann
@ 2014-02-03 20:04         ` W. Trevor King
  2014-02-03 20:22           ` Jens Lehmann
  0 siblings, 1 reply; 35+ messages in thread
From: W. Trevor King @ 2014-02-03 20:04 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Junio C Hamano, Jonathan Nieder, Heiko Voigt

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

On Mon, Feb 03, 2014 at 08:51:57PM +0100, Jens Lehmann wrote:
> submodule update' eacht time obsolete, which was tedious and error prone.
                    ^ each

I'm just reading the commit messages this pass ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents
  2014-02-03 19:52       ` [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents Jens Lehmann
@ 2014-02-03 20:10         ` W. Trevor King
  2014-02-07 21:24           ` Jens Lehmann
  0 siblings, 1 reply; 35+ messages in thread
From: W. Trevor King @ 2014-02-03 20:10 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Junio C Hamano, Jonathan Nieder, Heiko Voigt

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

On Mon, Feb 03, 2014 at 08:52:49PM +0100, Jens Lehmann wrote:
> Implement the functionality needed to enable work tree manipulating
> commands to that a deleted submodule should not only affect the index
> (leaving all the files of the submodule in the work tree) but also to
> remove the work tree of the superproject (including any untracked
> files).
> 
> That will only work properly when the submodule uses a gitfile instead of
> a .git directory and no untracked files are present. Otherwise the removal
> will fail with a warning (which is just what happened until now).

I'm having trouble parsing this one.  How about:

  Add a depopulate_submodule helper which removes the submodule
  working directory without touching the index.  This will only work
  properly when the submodule uses a gitfile…

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules
  2014-02-03 19:54       ` [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules Jens Lehmann
@ 2014-02-03 20:19         ` W. Trevor King
  2014-02-07 21:25           ` Jens Lehmann
  2014-02-04  0:11         ` Duy Nguyen
  1 sibling, 1 reply; 35+ messages in thread
From: W. Trevor King @ 2014-02-03 20:19 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Junio C Hamano, Jonathan Nieder, Heiko Voigt

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On Mon, Feb 03, 2014 at 08:54:17PM +0100, Jens Lehmann wrote:
> Implement the functionality needed to enable work tree manipulating
> commands so that an changed submodule does not only affect the index but
> it also updates the work tree of any initialized submodule according to
> the SHA-1 recorded in the superproject.

How about:

  …so that *a* changed submodule ** updates the index and work tree of
  any initialized submodule according to the SHA-1 recorded in the
  superproject.  Before this commit it updated neither; users had to
  run 'submodule update' to propagate gitlink updates into the
  submodule.

I'm pretty sure that's accurate anyway ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 6/9] Teach bisect the --[no-]recurse-submodules option
  2014-02-03 20:04         ` W. Trevor King
@ 2014-02-03 20:22           ` Jens Lehmann
  0 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-03 20:22 UTC (permalink / raw
  To: W. Trevor King
  Cc: Git Mailing List, Junio C Hamano, Jonathan Nieder, Heiko Voigt

Am 03.02.2014 21:04, schrieb W. Trevor King:
> On Mon, Feb 03, 2014 at 08:51:57PM +0100, Jens Lehmann wrote:
>> submodule update' eacht time obsolete, which was tedious and error prone.
>                     ^ each
> 
> I'm just reading the commit messages this pass ;).

Fair enough ;-)

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules
  2014-02-03 19:48       ` [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules Jens Lehmann
@ 2014-02-03 22:23         ` Junio C Hamano
  2014-02-07 21:06           ` Jens Lehmann
  2014-02-04  0:01         ` Jonathan Nieder
  1 sibling, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2014-02-03 22:23 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

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

> This commit adds the functions and files needed for configuration,

Please just say "Add the functions and files needed for ...".

> +++ b/Documentation/recurse-submodules-update.txt
> @@ -0,0 +1,8 @@
> +--[no-]recurse-submodules::
> +	Using --recurse-submodules will update the work tree of all
> +	initialized submodules according to the commit recorded in the
> +	superproject if their update configuration is set to checkout'. If

That single quote does not seem to be closing any matching quote.

The phrase "according to" feels a bit too fuzzy.  Merging the commit
to what is checked out is one possible implementation of "according to".
Applying the diff between the commit and what is checked out to work
tree is another.  Resetting the work tree files to exactly match the
commit would be yet another.

I think "update the work trees to the commit" (i.e. lose the
"according") would be the closest to what you are trying to say
here.

> +	local modifications in a submodule would be overwritten the checkout
> +	will fail unless forced. Without this option or with
> +	--no-recurse-submodules is, the work trees of submodules will not be
> +	updated, only the hash recorded in the superproject will be updated.

It is unclear what happens if their update configuration is set to
something other than 'checkout'.

> diff --git a/submodule.c b/submodule.c
> index 613857e..b3eb28d 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -382,6 +384,48 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
> ...
> +int option_parse_update_submodules(const struct option *opt,
> +				   const char *arg, int unset)
> +{
> +	if (unset) {
> +		*(int *)opt->value = RECURSE_SUBMODULES_OFF;
> +	} else {
> +		if (arg)
> +			*(int *)opt->value = parse_update_recurse_submodules_arg(opt->long_name, arg);
> +		else
> +			*(int *)opt->value = RECURSE_SUBMODULES_ON;
> +	}

You can easily unnest to lose {}

    if (unset)
            value = off;
    else if (arg)
            value = parse...;
    else
            value = on;

Also I suspect that git_config_maybe_bool() natively knows how to
handle arg==NULL, so

    if (unset)
	value = off;
    else
	value = parse...;

is sufficient?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option
  2014-02-03 19:49       ` [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option Jens Lehmann
@ 2014-02-03 22:40         ` Junio C Hamano
  2014-02-07 21:09           ` Jens Lehmann
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2014-02-03 22:40 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

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

> This new option will allow the user to not only reset the work tree of
> the superproject but to also update the work tree of all initialized
> submodules (so they match the SHA-1 recorded in the superproject) when
> used together with --hard or --merge. But this commit only adds the

I agree that --soft and --mixed should not do anything.  I am not
sure why --keep should not do anything to submodule working trees
when asked to recurse, though.

> option without any functionality, that will be added to unpack_trees()
> in subsequent commits.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>  Documentation/git-reset.txt |  4 ++++
>  builtin/reset.c             | 14 ++++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> index f445cb3..8f833f4 100644
> --- a/Documentation/git-reset.txt
> +++ b/Documentation/git-reset.txt
> @@ -94,6 +94,10 @@ OPTIONS
>  --quiet::
>  	Be quiet, only report errors.
>
> +include::recurse-submodules-update.txt[]
> ++
> +This option only makes sense together with `--hard` and `--merge` and is
> +ignored when used without these options.
>
>  EXAMPLES
>  --------
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 6004803..adf372e 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -20,6 +20,7 @@
>  #include "parse-options.h"
>  #include "unpack-trees.h"
>  #include "cache-tree.h"
> +#include "submodule.h"
>
>  static const char * const git_reset_usage[] = {
>  	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
> @@ -255,6 +256,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  {
>  	int reset_type = NONE, update_ref_status = 0, quiet = 0;
>  	int patch_mode = 0, unborn;
> +	const char *recurse_submodules_default = "off";
> +	int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
>  	const char *rev;
>  	unsigned char sha1[20];
>  	struct pathspec pathspec;
> @@ -270,13 +273,24 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  		OPT_SET_INT(0, "keep", &reset_type,
>  				N_("reset HEAD but keep local changes"), KEEP),
>  		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
> +		{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
> +			"checkout", "control recursive updating of submodules",
> +			PARSE_OPT_OPTARG, option_parse_update_submodules },
> +		{ OPTION_STRING, 0, "recurse-submodules-default",
> +			&recurse_submodules_default, NULL,
> +			"default mode for recursion", PARSE_OPT_HIDDEN },
>  		OPT_END()
>  	};
>
> +	gitmodules_config();
>  	git_config(git_default_config, NULL);
>
>  	argc = parse_options(argc, argv, prefix, options, git_reset_usage,
>  						PARSE_OPT_KEEP_DASHDASH);
> +	set_config_update_recurse_submodules(
> +		parse_update_recurse_submodules_arg("--recurse-submodules-default",
> +						    recurse_submodules_default),
> +		recurse_submodules);
>  	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
>
>  	unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", sha1);

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 3/9] Teach checkout the --[no-]recurse-submodules option
  2014-02-03 19:50       ` [WIP/PATCH 3/9] Teach checkout " Jens Lehmann
@ 2014-02-03 22:56         ` Junio C Hamano
  2014-02-07 21:12           ` Jens Lehmann
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2014-02-03 22:56 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

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

> +	set_config_update_recurse_submodules(
> +		parse_update_recurse_submodules_arg("--recurse-submodules-default",
> +						    recurse_submodules_default),
> +		recurse_submodules);

I think I saw these exact lines in another patch.  Perhaps the whole
thing can become a helper function that lets the caller avoid typing
the whole long strings that needs a strange/unfortunate line break? 

> diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
> index 06b18f8..bc3e1ca 100755
> --- a/t/t2013-checkout-submodule.sh
> +++ b/t/t2013-checkout-submodule.sh
> @@ -4,17 +4,57 @@ test_description='checkout can handle submodules'
>
>  . ./test-lib.sh
>
> +submodule_creation_must_succeed() {

Style: SP before (), i.e.

	submodule_creation_must_succeed () {

> +	# checkout base ($1)
> +	git checkout -f --recurse-submodules $1 &&
> +	git diff-files --quiet &&
> +	git diff-index --quiet --cached $1 &&

Please make it a habit to quote a parameter that is intended not to
be split at $IFS (e.g. write these as "$1" not as $1).  Otherwise
the reader has to wonder if this can be called with a "foo bar" and
the expects it to be split into two.

> +	# checkout target ($2)
> +	if test -d submodule; then

Style: no semicolons in standard control structure, i.e.

	if test -d submodule
	then

> +		echo change>>submodule/first.t &&

Style: SP before but not after redirection operator, i.e.

	echo foo >>bar

> +submodule_removal_must_succeed() {

Likewise.

> +	# checkout base ($1)
> +	git checkout -f --recurse-submodules $1 &&

Likewise.

> +	echo first > file &&

Likewise.

> +test_expect_success '"checkout --recurse-submodules" replaces submodule with files' '
> +	git checkout -f base &&
> +	git checkout -b replace_submodule_with_dir &&
> +	git update-index --force-remove submodule &&
> +	rm -rf submodule/.git .gitmodules &&
> +	git add .gitmodules submodule/* &&
> +	git commit -m "submodule replaced" &&
> +	git checkout -f base &&
> +	git submodule update -f &&
> +	git checkout --recurse-submodules replace_submodule_with_dir &&
> +	test -d submodule &&
> +	! test -e submodule/.git &&
> +	test -f submodule/first.t &&
> +	test -f submodule/second.t
> +'

Hmmmm.  Is it sufficient for these files to just exist, or do we
want to make sure they have expected contents?

Thanks.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 4/9] Teach merge the --[no-]recurse-submodules option
  2014-02-03 19:50       ` [WIP/PATCH 4/9] Teach merge " Jens Lehmann
@ 2014-02-03 23:01         ` Junio C Hamano
  2014-02-07 21:23           ` Jens Lehmann
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2014-02-03 23:01 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

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

> This new option will allow the user to not only update the work tree of
> the superproject according to the merge result but to also update the
> work tree of all initialized submodules (so they match the SHA-1 recorded
> in the superproject). But this commit only adds the option without any
> functionality, that will be added to unpack_trees() in subsequent commits.

When the two branches of the superproject being merged wants to put
a submodule project to commit A and B, that conflict needs to be
resolved, but if they agree that the submodule project should be at
C (which is different from what the current superproject HEAD has
for the submodule in its gitlink), then we want a checkout of that
commit to happen in that submodule.  Makes sense.

After resolving such a conflict between A and B, who is responsible
to adjust the working tree state of the submodule involved, by the
way?  "git merge --continue" does not exist and its moral equivalent
to conclude such an interrupted merge is "git commit".  Should it
learn to do "recurse-submodule", or should the user run a separate
"checkout --recurse-submodule"?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules
  2014-02-03 19:48       ` [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules Jens Lehmann
  2014-02-03 22:23         ` Junio C Hamano
@ 2014-02-04  0:01         ` Jonathan Nieder
  2014-02-07 21:01           ` Jens Lehmann
  1 sibling, 1 reply; 35+ messages in thread
From: Jonathan Nieder @ 2014-02-04  0:01 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Junio C Hamano, Heiko Voigt, W. Trevor King

Jens Lehmann wrote:

> This commit adds the functions and files needed for configuration,
> documentation, setting the default behavior and determining if a
> submodule path should be updated automatically.

Yay!

[...]
>  Documentation/recurse-submodules-update.txt |  8 +++++
>  submodule.c                                 | 50 +++++++++++++++++++++++++++++
>  submodule.h                                 |  6 ++++
>  3 files changed, 64 insertions(+)
>  create mode 100644 Documentation/recurse-submodules-update.txt

I like the shared documentation snippet.

Ok, naive questions and overly pedantic nitpicking follow.  Patch with
a couple of suggested changes at the end.

[...]
> --- /dev/null
> +++ b/Documentation/recurse-submodules-update.txt
> @@ -0,0 +1,8 @@
> +--[no-]recurse-submodules::
> +	Using --recurse-submodules will update the work tree of all
> +	initialized submodules according to the commit recorded in the
> +	superproject if their update configuration is set to checkout'. If
> +	local modifications in a submodule would be overwritten the checkout
> +	will fail unless forced. Without this option or with
> +	--no-recurse-submodules is, the work trees of submodules will not be
> +	updated, only the hash recorded in the superproject will be updated.

Tweaks:

 * Spelling out "--no-recurse-submodules, --recurse-submodules" (imitating
   e.g. --decorate in git-log(1))

 * Shortening, using imperative mood
 
 * Skipping description of safety check, since it matches how checkout
   works in general

That would make

	--no-recurse-submodules::
	--recurse-submodules::
		Perform the checkout in submodules, too.  This only affects
		submodules with update strategy `checkout` (which is the
		default update strategy; see `submodule.<name>.update` in
		link:gitmodules[5]).
	+
	The default behavior is to update submodule entries in the superproject
	index and to leave the inside of submodules alone.  That behavior can also
	be requested explicitly with --no-recurse-submodules.

Ideas for further work:

 * The safety check probably deserves a new section where it could be
   described in detail alongside a description of the corresponding check
   for plain checkout.  Then the description of the -f option could
   point to that section.

 * What happens when update = merge, rebase, or !command?  I think
   skipping them for now like suggested above is fine, but:

   - It would be even better to error out when there are changes to carry
     over with update = merge or rebase

   - Better still to perform the rebase when update = rebase

   - I have no idea what update = merge should do for non-fast-forward
     moves

> --- a/submodule.c
> +++ b/submodule.c
> @@ -16,6 +16,8 @@ static struct string_list config_name_for_path;
>  static struct string_list config_fetch_recurse_submodules_for_name;
>  static struct string_list config_ignore_for_name;
>  static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
> +static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
> +static int option_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;

Confusingly, config_update_recurse_submodules is set using the
--recurse-submodules-default option, not configuration.  There's
precedent for that in fetch.recurseSubmodules handling, but perhaps
a comment would help --- something like

	/*
	 * When no --recurse-submodules option was passed, should git fetch
	 * from submodules where submodule.<name>.fetchRecurseSubmodules
	 * doesn't indicate what to do?
	 *
	 * Controlled by fetch.recurseSubmodules.  The default is determined by
	 * the --recurse-submodules-default option, which propagates
	 * --recurse-submodules from the parent git process when recursing.
	 */
	static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;

	/*
	 * When no --recurse-submodules option was passed, should git update
	 * the index and worktree within submodules (and in turn their
	 * submodules, etc)?
	 *
	 * Controlled by the --recurse-submodules-default option, which
	 * propagates --recurse-submodules from the parent git process
	 * when recursing.
	 */
	static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;

[...]
> @@ -382,6 +384,48 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
>  	}
>  }
> 
> +int parse_update_recurse_submodules_arg(const char *opt, const char *arg)
> +{
> +	switch (git_config_maybe_bool(opt, arg)) {
> +	case 1:
> +		return RECURSE_SUBMODULES_ON;
> +	case 0:
> +		return RECURSE_SUBMODULES_OFF;
> +	default:
> +		if (!strcmp(arg, "checkout"))
> +			return RECURSE_SUBMODULES_ON;

Hm, is this arg == checkout case futureproofing for when
--recurse-submodules learns to handle submodules without
'update = checkout', too?

Is it safe to leave it out for now?

[...]
> +int submodule_needs_update(const char *path)

Return value convention: 1 means "do update"; 0 means "don't update".

Some day later I suppose 2 or -1 could mean "error out".  Ok.

Naming nit: needs_update sounds like it's checking if there was a
change at that path.  How about something like submodule_should_update(),
!submodule_ignore_for_update(), or update_should_recurse_into_submodule()?

[...]
> @@ -589,6 +633,12 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
>  	return ret;
>  }
> 
> +void set_config_update_recurse_submodules(int default_value, int option_value)
> +{
> +	config_update_recurse_submodules = default_value;
> +	option_update_recurse_submodules = option_value;
> +}

Could option_parse_update_submodules set
option_update_recurse_submodules directly?  Alternatively, could this
function examine option_value so that submodule.c would only need one
variable?

	if (option_value == RECURSE_SUBMODULES_DEFAULT)
		update_recurse_submodules = default_value;
	else
		update_recurse_submodules = option_value;

If .gitmodules some day grows a submodule.<name>.checkoutRecurseSubmodules
option then it would be convenient to have the option that overrides and
the default tracked separately.  Is that the idea here?

I might try writing a dummy command to test this basic --recurse-submodules
option handling as a separate patch.

Thanks,
Jonathan

diff --git i/Documentation/recurse-submodules-update.txt w/Documentation/recurse-submodules-update.txt
index e57d452..eae376d 100644
--- i/Documentation/recurse-submodules-update.txt
+++ w/Documentation/recurse-submodules-update.txt
@@ -1,8 +1,10 @@
---[no-]recurse-submodules::
-	Using --recurse-submodules will update the work tree of all
-	initialized submodules according to the commit recorded in the
-	superproject if their update configuration is set to checkout'. If
-	local modifications in a submodule would be overwritten the checkout
-	will fail unless forced. Without this option or with
-	--no-recurse-submodules is, the work trees of submodules will not be
-	updated, only the hash recorded in the superproject will be updated.
+--no-recurse-submodules::
+--recurse-submodules::
+	Perform the checkout in submodules, too.  This only affects
+	submodules with update strategy `checkout` (which is the
+	default update strategy; see `submodule.<name>.update` in
+	linkgit:gitmodules[5]).
++
+The default behavior is to update submodule entries in the superproject
+index and to leave the inside of submodules alone.  That behavior can
+also be requested explicitly with `--no-recurse-submodules`.
diff --git i/submodule.c w/submodule.c
index b3eb28d..f88bf70 100644
--- i/submodule.c
+++ w/submodule.c
@@ -12,11 +12,30 @@
 #include "argv-array.h"
 #include "blob.h"
 
+/*
+ * When no --recurse-submodules option was passed, should git fetch
+ * from submodules where submodule.<name>.fetchRecurseSubmodules doesn't
+ * indicate what to do?
+ *
+ * Controlled by fetch.recurseSubmodules.  The default is determined by
+ * the --recurse-submodules-default option, which propagates
+ * --recurse-submodules from the parent git process when recursing.
+ */
+static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
+
+/*
+ * When no --recurse-submodules option was passed, should git update the
+ * index and worktree within submodules (and in turn their submodules,
+ * etc)?
+ *
+ * Controlled by the --recurse-submodules-default option, which propagates
+ * --recurse-submodules from the parent git process when recursing.
+ */
+static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
+
 static struct string_list config_name_for_path;
 static struct string_list config_fetch_recurse_submodules_for_name;
 static struct string_list config_ignore_for_name;
-static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
-static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
 static int option_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static struct string_list changed_submodule_paths;
 static int initialized_fetch_ref_tips;
@@ -392,8 +411,6 @@ int parse_update_recurse_submodules_arg(const char *opt, const char *arg)
 	case 0:
 		return RECURSE_SUBMODULES_OFF;
 	default:
-		if (!strcmp(arg, "checkout"))
-			return RECURSE_SUBMODULES_ON;
 		die("bad %s argument: %s", opt, arg);
 	}
 }

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules
  2014-02-03 19:54       ` [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules Jens Lehmann
  2014-02-03 20:19         ` W. Trevor King
@ 2014-02-04  0:11         ` Duy Nguyen
  2014-02-07 21:32           ` Jens Lehmann
  1 sibling, 1 reply; 35+ messages in thread
From: Duy Nguyen @ 2014-02-04  0:11 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Junio C Hamano, Jonathan Nieder, Heiko Voigt,
	W. Trevor King

On Tue, Feb 4, 2014 at 2:54 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Implement the functionality needed to enable work tree manipulating
> commands so that an changed submodule does not only affect the index but
> it also updates the work tree of any initialized submodule according to
> the SHA-1 recorded in the superproject.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>  entry.c        | 15 ++++++++--
>  submodule.c    | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  submodule.h    |  3 ++
>  unpack-trees.c | 69 ++++++++++++++++++++++++++++++++++++----------
>  unpack-trees.h |  1 +
>  5 files changed, 157 insertions(+), 17 deletions(-)
>
> diff --git a/entry.c b/entry.c
> index d1bf6ec..61a2767 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -265,7 +265,7 @@ int checkout_entry(struct cache_entry *ce,
>
>         if (!check_path(path, len, &st, state->base_dir_len)) {
>                 unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
> -               if (!changed)
> +               if (!changed && (!S_ISDIR(st.st_mode) || !S_ISGITLINK(ce->ce_mode)))
>                         return 0;

Should we report something when ce is a gitlink, but path is not a
directory, instead of siliently exit?

> diff --git a/submodule.c b/submodule.c
> index 3907034..83e7595 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -520,6 +520,42 @@ int depopulate_submodule(const char *path)
>         return 0;
>  }
>
> +int update_submodule(const char *path, const unsigned char sha1[20], int force)
> +{
> +       struct strbuf buf = STRBUF_INIT;
> +       struct child_process cp;
> +       const char *hex_sha1 = sha1_to_hex(sha1);
> +       const char *argv[] = {
> +               "checkout",
> +               force ? "-fq" : "-q",

respect "state->quiet" in checkout_entry() as well?

> +               hex_sha1,
> +               NULL,
> +       };
> +       const char *git_dir;
> +
> +       strbuf_addf(&buf, "%s/.git", path);
> +       git_dir = read_gitfile(buf.buf);
> +       if (!git_dir)
> +               git_dir = buf.buf;
> +       if (!is_directory(git_dir)) {
> +               strbuf_release(&buf);
> +               /* The submodule is not populated, so we can't check it out */
> +               return 0;
> +       }
> +       strbuf_release(&buf);
> +
> +       memset(&cp, 0, sizeof(cp));
> +       cp.argv = argv;
> +       cp.env = local_repo_env;
> +       cp.git_cmd = 1;
> +       cp.no_stdin = 1;
> +       cp.dir = path;   /* GIT_WORK_TREE doesn't work for git checkout */

And if we do respect --quiet and it's not specified, paths printed by
this process is relative to "dir", not to user cwd. Could be
confusing.

> +       if (run_command(&cp))
> +               return error("Could not checkout submodule %s", path);
> +
> +       return 0;
> +}
> +
-- 
Duy

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules
  2014-02-04  0:01         ` Jonathan Nieder
@ 2014-02-07 21:01           ` Jens Lehmann
  0 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-07 21:01 UTC (permalink / raw
  To: Jonathan Nieder
  Cc: Git Mailing List, Junio C Hamano, Heiko Voigt, W. Trevor King

Am 04.02.2014 01:01, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
>> --- /dev/null
>> +++ b/Documentation/recurse-submodules-update.txt
>> @@ -0,0 +1,8 @@
>> +--[no-]recurse-submodules::
>> +	Using --recurse-submodules will update the work tree of all
>> +	initialized submodules according to the commit recorded in the
>> +	superproject if their update configuration is set to checkout'. If
>> +	local modifications in a submodule would be overwritten the checkout
>> +	will fail unless forced. Without this option or with
>> +	--no-recurse-submodules is, the work trees of submodules will not be
>> +	updated, only the hash recorded in the superproject will be updated.
> 
> Tweaks:
> 
>  * Spelling out "--no-recurse-submodules, --recurse-submodules" (imitating
>    e.g. --decorate in git-log(1))
> 
>  * Shortening, using imperative mood
>  
>  * Skipping description of safety check, since it matches how checkout
>    works in general
> 
> That would make
> 
> 	--no-recurse-submodules::
> 	--recurse-submodules::
> 		Perform the checkout in submodules, too.  This only affects
> 		submodules with update strategy `checkout` (which is the
> 		default update strategy; see `submodule.<name>.update` in
> 		link:gitmodules[5]).
> 	+
> 	The default behavior is to update submodule entries in the superproject
> 	index and to leave the inside of submodules alone.  That behavior can also
> 	be requested explicitly with --no-recurse-submodules.

Much better, thanks!

> Ideas for further work:
> 
>  * The safety check probably deserves a new section where it could be
>    described in detail alongside a description of the corresponding check
>    for plain checkout.  Then the description of the -f option could
>    point to that section.

Good idea.

>  * What happens when update = merge, rebase, or !command?  I think
>    skipping them for now like suggested above is fine, but:
> 
>    - It would be even better to error out when there are changes to carry
>      over with update = merge or rebase

In the first round I'd rather do nothing (just like we do now) for merge
or rebase. These two should be tackled in a follow up series (especially
as I currently do not think everybody agrees on the desired behavior when
the branch config is set yet)

>    - Better still to perform the rebase when update = rebase
> 
>    - I have no idea what update = merge should do for non-fast-forward
>      moves

The same it does for checkout when we would overwrite local changes:
error out before doing anything and let the user sort things out?

>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -16,6 +16,8 @@ static struct string_list config_name_for_path;
>>  static struct string_list config_fetch_recurse_submodules_for_name;
>>  static struct string_list config_ignore_for_name;
>>  static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
>> +static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
>> +static int option_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> 
> Confusingly, config_update_recurse_submodules is set using the
> --recurse-submodules-default option, not configuration.  There's
> precedent for that in fetch.recurseSubmodules handling, but perhaps
> a comment would help --- something like
> 
> 	/*
> 	 * When no --recurse-submodules option was passed, should git fetch
> 	 * from submodules where submodule.<name>.fetchRecurseSubmodules
> 	 * doesn't indicate what to do?
> 	 *
> 	 * Controlled by fetch.recurseSubmodules.  The default is determined by
> 	 * the --recurse-submodules-default option, which propagates
> 	 * --recurse-submodules from the parent git process when recursing.
> 	 */
> 	static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
> 
> 	/*
> 	 * When no --recurse-submodules option was passed, should git update
> 	 * the index and worktree within submodules (and in turn their
> 	 * submodules, etc)?
> 	 *
> 	 * Controlled by the --recurse-submodules-default option, which
> 	 * propagates --recurse-submodules from the parent git process
> 	 * when recursing.
> 	 */
> 	static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;

Makes lots of sense.

> [...]
>> @@ -382,6 +384,48 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
>>  	}
>>  }
>>
>> +int parse_update_recurse_submodules_arg(const char *opt, const char *arg)
>> +{
>> +	switch (git_config_maybe_bool(opt, arg)) {
>> +	case 1:
>> +		return RECURSE_SUBMODULES_ON;
>> +	case 0:
>> +		return RECURSE_SUBMODULES_OFF;
>> +	default:
>> +		if (!strcmp(arg, "checkout"))
>> +			return RECURSE_SUBMODULES_ON;
> 
> Hm, is this arg == checkout case futureproofing for when
> --recurse-submodules learns to handle submodules without
> 'update = checkout', too?

Right.

> Is it safe to leave it out for now?

Yes it is.

> [...]
>> +int submodule_needs_update(const char *path)
> 
> Return value convention: 1 means "do update"; 0 means "don't update".
> 
> Some day later I suppose 2 or -1 could mean "error out".  Ok.
> 
> Naming nit: needs_update sounds like it's checking if there was a
> change at that path.  How about something like submodule_should_update(),
> !submodule_ignore_for_update(), or update_should_recurse_into_submodule()?

Good point, will do.

> [...]
>> @@ -589,6 +633,12 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
>>  	return ret;
>>  }
>>
>> +void set_config_update_recurse_submodules(int default_value, int option_value)
>> +{
>> +	config_update_recurse_submodules = default_value;
>> +	option_update_recurse_submodules = option_value;
>> +}
> 
> Could option_parse_update_submodules set
> option_update_recurse_submodules directly?  Alternatively, could this
> function examine option_value so that submodule.c would only need one
> variable?
> 
> 	if (option_value == RECURSE_SUBMODULES_DEFAULT)
> 		update_recurse_submodules = default_value;
> 	else
> 		update_recurse_submodules = option_value;
> 
> If .gitmodules some day grows a submodule.<name>.checkoutRecurseSubmodules
> option then it would be convenient to have the option that overrides and
> the default tracked separately.  Is that the idea here?

Correct. I intend to add a global and per-submodule "autoupdate" setting
just like those we have for fetch.

> I might try writing a dummy command to test this basic --recurse-submodules
> option handling as a separate patch.

Hmm, I haven't thought of that. So far I was testing this in the regular
test cases and intended to add that to the test framework. Will think
about that.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules
  2014-02-03 22:23         ` Junio C Hamano
@ 2014-02-07 21:06           ` Jens Lehmann
  0 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-07 21:06 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

Am 03.02.2014 23:23, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> This commit adds the functions and files needed for configuration,
> 
> Please just say "Add the functions and files needed for ...".

Roger that.

>> +++ b/Documentation/recurse-submodules-update.txt
>> @@ -0,0 +1,8 @@
>> +--[no-]recurse-submodules::
>> +	Using --recurse-submodules will update the work tree of all
>> +	initialized submodules according to the commit recorded in the
>> +	superproject if their update configuration is set to checkout'. If
> 
> That single quote does not seem to be closing any matching quote.
> 
> The phrase "according to" feels a bit too fuzzy.  Merging the commit
> to what is checked out is one possible implementation of "according to".
> Applying the diff between the commit and what is checked out to work
> tree is another.  Resetting the work tree files to exactly match the
> commit would be yet another.
> 
> I think "update the work trees to the commit" (i.e. lose the
> "according") would be the closest to what you are trying to say
> here.
> 
>> +	local modifications in a submodule would be overwritten the checkout
>> +	will fail unless forced. Without this option or with
>> +	--no-recurse-submodules is, the work trees of submodules will not be
>> +	updated, only the hash recorded in the superproject will be updated.
> 
> It is unclear what happens if their update configuration is set to
> something other than 'checkout'.

Jonathan already proposed a better description, will use that in the next
round.

>> diff --git a/submodule.c b/submodule.c
>> index 613857e..b3eb28d 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -382,6 +384,48 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
>> ...
>> +int option_parse_update_submodules(const struct option *opt,
>> +				   const char *arg, int unset)
>> +{
>> +	if (unset) {
>> +		*(int *)opt->value = RECURSE_SUBMODULES_OFF;
>> +	} else {
>> +		if (arg)
>> +			*(int *)opt->value = parse_update_recurse_submodules_arg(opt->long_name, arg);
>> +		else
>> +			*(int *)opt->value = RECURSE_SUBMODULES_ON;
>> +	}
> 
> You can easily unnest to lose {}
> 
>     if (unset)
>             value = off;
>     else if (arg)
>             value = parse...;
>     else
>             value = on;

Yeah, that's better.

> Also I suspect that git_config_maybe_bool() natively knows how to
> handle arg==NULL, so
> 
>     if (unset)
> 	value = off;
>     else
> 	value = parse...;
> 
> is sufficient?

Will try.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option
  2014-02-03 22:40         ` Junio C Hamano
@ 2014-02-07 21:09           ` Jens Lehmann
  0 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-07 21:09 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

Am 03.02.2014 23:40, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> This new option will allow the user to not only reset the work tree of
>> the superproject but to also update the work tree of all initialized
>> submodules (so they match the SHA-1 recorded in the superproject) when
>> used together with --hard or --merge. But this commit only adds the
> 
> I agree that --soft and --mixed should not do anything.  I am not
> sure why --keep should not do anything to submodule working trees
> when asked to recurse, though.

Correct, I missed that option. I think it should update submodules
too.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 3/9] Teach checkout the --[no-]recurse-submodules option
  2014-02-03 22:56         ` Junio C Hamano
@ 2014-02-07 21:12           ` Jens Lehmann
  0 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-07 21:12 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

Am 03.02.2014 23:56, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> +	set_config_update_recurse_submodules(
>> +		parse_update_recurse_submodules_arg("--recurse-submodules-default",
>> +						    recurse_submodules_default),
>> +		recurse_submodules);
> 
> I think I saw these exact lines in another patch.  Perhaps the whole
> thing can become a helper function that lets the caller avoid typing
> the whole long strings that needs a strange/unfortunate line break? 

Right, that'd be better.

>> diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
>> index 06b18f8..bc3e1ca 100755
>> --- a/t/t2013-checkout-submodule.sh
>> +++ b/t/t2013-checkout-submodule.sh
>> @@ -4,17 +4,57 @@ test_description='checkout can handle submodules'
>>
>>  . ./test-lib.sh
>>
>> +submodule_creation_must_succeed() {
> 
> Style: SP before (), i.e.
> 
> 	submodule_creation_must_succeed () {
> 
>> +	# checkout base ($1)
>> +	git checkout -f --recurse-submodules $1 &&
>> +	git diff-files --quiet &&
>> +	git diff-index --quiet --cached $1 &&
> 
> Please make it a habit to quote a parameter that is intended not to
> be split at $IFS (e.g. write these as "$1" not as $1).  Otherwise
> the reader has to wonder if this can be called with a "foo bar" and
> the expects it to be split into two.
> 
>> +	# checkout target ($2)
>> +	if test -d submodule; then
> 
> Style: no semicolons in standard control structure, i.e.
> 
> 	if test -d submodule
> 	then
> 
>> +		echo change>>submodule/first.t &&
> 
> Style: SP before but not after redirection operator, i.e.
> 
> 	echo foo >>bar
> 
>> +submodule_removal_must_succeed() {
> 
> Likewise.
> 
>> +	# checkout base ($1)
>> +	git checkout -f --recurse-submodules $1 &&
> 
> Likewise.
> 
>> +	echo first > file &&
> 
> Likewise.
> 
>> +test_expect_success '"checkout --recurse-submodules" replaces submodule with files' '
>> +	git checkout -f base &&
>> +	git checkout -b replace_submodule_with_dir &&
>> +	git update-index --force-remove submodule &&
>> +	rm -rf submodule/.git .gitmodules &&
>> +	git add .gitmodules submodule/* &&
>> +	git commit -m "submodule replaced" &&
>> +	git checkout -f base &&
>> +	git submodule update -f &&
>> +	git checkout --recurse-submodules replace_submodule_with_dir &&
>> +	test -d submodule &&
>> +	! test -e submodule/.git &&
>> +	test -f submodule/first.t &&
>> +	test -f submodule/second.t
>> +'
> 
> Hmmmm.  Is it sufficient for these files to just exist, or do we
> want to make sure they have expected contents?

Thanks, will consider all you remarks above in the ongoing work for
testing framework which should replace these tests.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 4/9] Teach merge the --[no-]recurse-submodules option
  2014-02-03 23:01         ` Junio C Hamano
@ 2014-02-07 21:23           ` Jens Lehmann
  2014-02-07 22:00             ` Junio C Hamano
  0 siblings, 1 reply; 35+ messages in thread
From: Jens Lehmann @ 2014-02-07 21:23 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

Am 04.02.2014 00:01, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> This new option will allow the user to not only update the work tree of
>> the superproject according to the merge result but to also update the
>> work tree of all initialized submodules (so they match the SHA-1 recorded
>> in the superproject). But this commit only adds the option without any
>> functionality, that will be added to unpack_trees() in subsequent commits.
> 
> When the two branches of the superproject being merged wants to put
> a submodule project to commit A and B, that conflict needs to be
> resolved, but if they agree that the submodule project should be at
> C (which is different from what the current superproject HEAD has
> for the submodule in its gitlink), then we want a checkout of that
> commit to happen in that submodule.  Makes sense.
> 
> After resolving such a conflict between A and B, who is responsible
> to adjust the working tree state of the submodule involved, by the
> way?  "git merge --continue" does not exist and its moral equivalent
> to conclude such an interrupted merge is "git commit".  Should it
> learn to do "recurse-submodule", or should the user run a separate
> "checkout --recurse-submodule"?

I think the user needs to sort things out, just like she has to do
when a file has a merge conflict. But unfortunately we cannot use
conflict markers here, so I'd propose the following:

* When merge proposes a merge resolution (which it does today by
  telling the user "Found a possible merge resolution for the
  submodule ... [use] git update-index --cacheinfo 160000 ...")
  that commit should be checked out in the submodule but not
  staged. Then the user can simply add and commit.

* If the merge resolution is not obvious to merge, it leaves the
  submodule in an unmerged state, the local commit still being
  checked out. The user has to manually do the merge in the
  submodule and commits that in the superproject.

Does that make sense?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents
  2014-02-03 20:10         ` W. Trevor King
@ 2014-02-07 21:24           ` Jens Lehmann
  0 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-07 21:24 UTC (permalink / raw
  To: W. Trevor King
  Cc: Git Mailing List, Junio C Hamano, Jonathan Nieder, Heiko Voigt

Am 03.02.2014 21:10, schrieb W. Trevor King:
> On Mon, Feb 03, 2014 at 08:52:49PM +0100, Jens Lehmann wrote:
>> Implement the functionality needed to enable work tree manipulating
>> commands to that a deleted submodule should not only affect the index
>> (leaving all the files of the submodule in the work tree) but also to
>> remove the work tree of the superproject (including any untracked
>> files).
>>
>> That will only work properly when the submodule uses a gitfile instead of
>> a .git directory and no untracked files are present. Otherwise the removal
>> will fail with a warning (which is just what happened until now).
> 
> I'm having trouble parsing this one.  How about:
> 
>   Add a depopulate_submodule helper which removes the submodule
>   working directory without touching the index.  This will only work
>   properly when the submodule uses a gitfile…

Thanks, that's better.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules
  2014-02-03 20:19         ` W. Trevor King
@ 2014-02-07 21:25           ` Jens Lehmann
  0 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-07 21:25 UTC (permalink / raw
  To: W. Trevor King
  Cc: Git Mailing List, Junio C Hamano, Jonathan Nieder, Heiko Voigt

Am 03.02.2014 21:19, schrieb W. Trevor King:
> On Mon, Feb 03, 2014 at 08:54:17PM +0100, Jens Lehmann wrote:
>> Implement the functionality needed to enable work tree manipulating
>> commands so that an changed submodule does not only affect the index but
>> it also updates the work tree of any initialized submodule according to
>> the SHA-1 recorded in the superproject.
> 
> How about:
> 
>   …so that *a* changed submodule ** updates the index and work tree of
>   any initialized submodule according to the SHA-1 recorded in the
>   superproject.  Before this commit it updated neither; users had to
>   run 'submodule update' to propagate gitlink updates into the
>   submodule.
> 
> I'm pretty sure that's accurate anyway ;).

And I like it better ;-)

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules
  2014-02-04  0:11         ` Duy Nguyen
@ 2014-02-07 21:32           ` Jens Lehmann
  0 siblings, 0 replies; 35+ messages in thread
From: Jens Lehmann @ 2014-02-07 21:32 UTC (permalink / raw
  To: Duy Nguyen
  Cc: Git Mailing List, Junio C Hamano, Jonathan Nieder, Heiko Voigt,
	W. Trevor King

Am 04.02.2014 01:11, schrieb Duy Nguyen:
> On Tue, Feb 4, 2014 at 2:54 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Implement the functionality needed to enable work tree manipulating
>> commands so that an changed submodule does not only affect the index but
>> it also updates the work tree of any initialized submodule according to
>> the SHA-1 recorded in the superproject.
>>
>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>> ---
>>  entry.c        | 15 ++++++++--
>>  submodule.c    | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  submodule.h    |  3 ++
>>  unpack-trees.c | 69 ++++++++++++++++++++++++++++++++++++----------
>>  unpack-trees.h |  1 +
>>  5 files changed, 157 insertions(+), 17 deletions(-)
>>
>> diff --git a/entry.c b/entry.c
>> index d1bf6ec..61a2767 100644
>> --- a/entry.c
>> +++ b/entry.c
>> @@ -265,7 +265,7 @@ int checkout_entry(struct cache_entry *ce,
>>
>>         if (!check_path(path, len, &st, state->base_dir_len)) {
>>                 unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
>> -               if (!changed)
>> +               if (!changed && (!S_ISDIR(st.st_mode) || !S_ISGITLINK(ce->ce_mode)))
>>                         return 0;
> 
> Should we report something when ce is a gitlink, but path is not a
> directory, instead of siliently exit?

Good point.

>> diff --git a/submodule.c b/submodule.c
>> index 3907034..83e7595 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -520,6 +520,42 @@ int depopulate_submodule(const char *path)
>>         return 0;
>>  }
>>
>> +int update_submodule(const char *path, const unsigned char sha1[20], int force)
>> +{
>> +       struct strbuf buf = STRBUF_INIT;
>> +       struct child_process cp;
>> +       const char *hex_sha1 = sha1_to_hex(sha1);
>> +       const char *argv[] = {
>> +               "checkout",
>> +               force ? "-fq" : "-q",
> 
> respect "state->quiet" in checkout_entry() as well?

See below.

>> +               hex_sha1,
>> +               NULL,
>> +       };
>> +       const char *git_dir;
>> +
>> +       strbuf_addf(&buf, "%s/.git", path);
>> +       git_dir = read_gitfile(buf.buf);
>> +       if (!git_dir)
>> +               git_dir = buf.buf;
>> +       if (!is_directory(git_dir)) {
>> +               strbuf_release(&buf);
>> +               /* The submodule is not populated, so we can't check it out */
>> +               return 0;
>> +       }
>> +       strbuf_release(&buf);
>> +
>> +       memset(&cp, 0, sizeof(cp));
>> +       cp.argv = argv;
>> +       cp.env = local_repo_env;
>> +       cp.git_cmd = 1;
>> +       cp.no_stdin = 1;
>> +       cp.dir = path;   /* GIT_WORK_TREE doesn't work for git checkout */
> 
> And if we do respect --quiet and it's not specified, paths printed by
> this process is relative to "dir", not to user cwd. Could be
> confusing.

That's the reason I'm currently always passing -q to checkout. While
checkout would have to learn a "--prefix=" option to be able to print
the path relative to the superproject, some (most?) users don't want
to see this detailed information from inside the submodule. After all
git status and diff currently also only show a condensed view of the
submodule state and don't print any detailed information about files
inside the submodule. We might want to add means to enable that later,
and then we'd have to conditionally provide --quiet (and --prefix)
here.

>> +       if (run_command(&cp))
>> +               return error("Could not checkout submodule %s", path);
>> +
>> +       return 0;
>> +}
>> +

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [WIP/PATCH 4/9] Teach merge the --[no-]recurse-submodules option
  2014-02-07 21:23           ` Jens Lehmann
@ 2014-02-07 22:00             ` Junio C Hamano
  2014-02-07 22:08               ` W. Trevor King
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2014-02-07 22:00 UTC (permalink / raw
  To: Jens Lehmann
  Cc: Git Mailing List, Jonathan Nieder, Heiko Voigt, W. Trevor King

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

> I think the user needs to sort things out, just like she has to do
> when a file has a merge conflict. But unfortunately we cannot use
> conflict markers here, so I'd propose the following:
>
> * When merge proposes a merge resolution (which it does today by
>   telling the user "Found a possible merge resolution for the
>   submodule ... [use] git update-index --cacheinfo 160000 ...")
>   that commit should be checked out in the submodule but not
>   staged. Then the user can simply add and commit.
>
> * If the merge resolution is not obvious to merge, it leaves the
>   submodule in an unmerged state, the local commit still being
>   checked out. The user has to manually do the merge in the
>   submodule and commits that in the superproject.
>
> Does that make sense?

The latter one does not worry me too much.

For the former, "add and commit" at the top-level makes perfect
sense, and the "commit should be checked out in the submodule" is a
necessary step to sanity-check and prepare for that "add and commit"
step, but what does "checked out in the submodule" exactly mean?  Do
we detach the HEAD at the commit?  Do we advance the tip of the
branch of the submodule to that commit?  Do we know/require/care if
such a move always fast-forwards?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Re: [WIP/PATCH 4/9] Teach merge the --[no-]recurse-submodules option
  2014-02-07 22:00             ` Junio C Hamano
@ 2014-02-07 22:08               ` W. Trevor King
  0 siblings, 0 replies; 35+ messages in thread
From: W. Trevor King @ 2014-02-07 22:08 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Jens Lehmann, Git Mailing List, Jonathan Nieder, Heiko Voigt

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]

On Fri, Feb 07, 2014 at 02:00:23PM -0800, Junio C Hamano wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> > I think the user needs to sort things out, just like she has to do
> > when a file has a merge conflict. But unfortunately we cannot use
> > conflict markers here, so I'd propose the following:
> >
> > * When merge proposes a merge resolution (which it does today by
> >   telling the user "Found a possible merge resolution for the
> >   submodule ... [use] git update-index --cacheinfo 160000 ...")
> >   that commit should be checked out in the submodule but not
> >   staged. Then the user can simply add and commit.
> > …
> …
> 
> For the former, "add and commit" at the top-level makes perfect
> sense, …

This still works if the merge issue is in a grandchild submodule, but
it's going to be a bit tedious if the user has to add-and-commit at
each level from the troublesome sub-sub-…-module on up to the
top-level superproject.  I can't think of a cleaner solution though.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2014-02-07 22:09 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-06 22:36 What's cooking in git.git (Jan 2014, #01; Mon, 6) Junio C Hamano
2014-01-06 23:16 ` Francesco Pretto
2014-01-06 23:32   ` Junio C Hamano
2014-01-06 23:45     ` Francesco Pretto
2014-01-07 17:49 ` Jens Lehmann
     [not found]   ` <xmqqvbxvekwv.fsf@gitster.dls.corp.google.com>
2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
2014-02-03 19:48       ` [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules Jens Lehmann
2014-02-03 22:23         ` Junio C Hamano
2014-02-07 21:06           ` Jens Lehmann
2014-02-04  0:01         ` Jonathan Nieder
2014-02-07 21:01           ` Jens Lehmann
2014-02-03 19:49       ` [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option Jens Lehmann
2014-02-03 22:40         ` Junio C Hamano
2014-02-07 21:09           ` Jens Lehmann
2014-02-03 19:50       ` [WIP/PATCH 3/9] Teach checkout " Jens Lehmann
2014-02-03 22:56         ` Junio C Hamano
2014-02-07 21:12           ` Jens Lehmann
2014-02-03 19:50       ` [WIP/PATCH 4/9] Teach merge " Jens Lehmann
2014-02-03 23:01         ` Junio C Hamano
2014-02-07 21:23           ` Jens Lehmann
2014-02-07 22:00             ` Junio C Hamano
2014-02-07 22:08               ` W. Trevor King
2014-02-03 19:51       ` [WIP/PATCH 5/9] Teach bisect--helper " Jens Lehmann
2014-02-03 19:51       ` [WIP/PATCH 6/9] Teach bisect " Jens Lehmann
2014-02-03 20:04         ` W. Trevor King
2014-02-03 20:22           ` Jens Lehmann
2014-02-03 19:52       ` [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents Jens Lehmann
2014-02-03 20:10         ` W. Trevor King
2014-02-07 21:24           ` Jens Lehmann
2014-02-03 19:53       ` [WIP/PATCH 8/9] submodule: teach unpack_trees() to repopulate submodules Jens Lehmann
2014-02-03 19:54       ` [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules Jens Lehmann
2014-02-03 20:19         ` W. Trevor King
2014-02-07 21:25           ` Jens Lehmann
2014-02-04  0:11         ` Duy Nguyen
2014-02-07 21:32           ` Jens Lehmann

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).