Git development
 help / color / mirror / Atom feed
* What's cooking in git.git (Sep 2011, #01; Fri, 2)
From: Junio C Hamano @ 2011-09-02 22:06 UTC (permalink / raw)
  To: git

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

Some of the recent patches are fixes to new feature topics already merged
to "master" and are being fast tracked for 1.7.7-rc1, but otherwise
patches for new features and fixes to longstanding issues will stay in
'pu' or 'next' until the 1.7.7 final.

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

* mh/check-ref-format-print-normalize (2011-08-27) 2 commits
  (merged to 'next' on 2011-08-30 at e827410)
 + Forbid DEL characters in reference names
 + check-ref-format --print: Normalize refnames that start with slashes

Will merge to "master".

* bk/ancestry-path (2011-08-25) 3 commits
  (merged to 'next' on 2011-09-02 at d05ba5d)
 + revision: do not include sibling history in --ancestry-path output
 + revision: keep track of the end-user input from the command line
 + rev-list: Demonstrate breakage with --ancestry-path --all

The topic came up a bit too late in the cycle.
Will cook for a while.

* mg/branch-list (2011-08-28) 5 commits
  (merged to 'next' on 2011-09-02 at b818eae)
 + branch: allow pattern arguments
 + branch: introduce --list option
 + git-branch: introduce missing long forms for the options
 + git-tag: introduce long forms for the options
 + t6040: test branch -vv

Not urgent; the topic came up a bit too late in the cycle.

* mm/rebase-i-exec-edit (2011-08-26) 2 commits
  (merged to 'next' on 2011-09-02 at e75b1b9)
 + rebase -i: notice and warn if "exec $cmd" modifies the index or the working tree
 + rebase -i: clean error message for --continue after failed exec

Not urgent; the topic came up a bit too late in the cycle.

* jk/default-attr (2011-08-26) 1 commit
 - attr: map builtin userdiff drivers to well-known extensions

Not urgent; I fixed up the test breakage just for fun.

* hv/submodule-merge-search (2011-08-26) 4 commits
 - submodule: Search for merges only at end of recursive merge
 - allow multiple calls to submodule merge search for the same path
 - submodule: Demonstrate known breakage during recursive merge
 - push: Don't push a repository with unpushed submodules
 (this branch uses fg/submodule-auto-push.)

Not urgent; the topic came up a bit too late in the cycle.
The bottom one needs to be replaced with a properly written commit log message.

* mm/mediawiki-as-a-remote (2011-09-01) 2 commits
 - git-remote-mediawiki: allow push to set MediaWiki metadata
 - Add a remote helper to interact with mediawiki (fetch & push)

Fun.
Not urgent; the topic came up a bit too late in the cycle.

* nd/maint-autofix-tag-in-head (2011-08-26) 3 commits
 - Accept tags in HEAD or MERGE_HEAD
 - merge: remove global variable head[]
 - merge: keep stash[] a local variable

Probably needs a re-roll to aim a bit higher.
Not urgent; will not be in 1.7.7.

* bc/unstash-clean-crufts (2011-08-27) 4 commits
  (merged to 'next' on 2011-09-02 at 7bfd66f)
 + git-stash: remove untracked/ignored directories when stashed
 + t/t3905: add missing '&&' linkage
 + git-stash.sh: fix typo in error message
 + t/t3905: use the name 'actual' for test output, swap arguments to test_cmp

Not urgent; the topic came up a bit too late in the cycle.

* cb/maint-ls-files-error-report (2011-08-28) 1 commit
  (merged to 'next' on 2011-08-30 at b606e19)
 + t3005: do not assume a particular order of stdout and stderr of git-ls-files

Will merge to "master".

* da/make-auto-header-dependencies (2011-08-30) 1 commit
  (merged to 'next' on 2011-09-02 at e04a4af)
 + Makefile: Improve compiler header dependency check
 (this branch uses fk/make-auto-header-dependencies.)

Not urgent; will not be in 1.7.7.

* gb/am-hg-patch (2011-08-29) 1 commit
  (merged to 'next' on 2011-09-02 at 3edfe4c)
 + am: preliminary support for hg patches

Not urgent; the topic came up a bit too late in the cycle.

* jc/diff-index-unpack (2011-08-29) 3 commits
  (merged to 'next' on 2011-09-02 at 4206bd9)
 + diff-index: pass pathspec down to unpack-trees machinery
 + unpack-trees: allow pruning with pathspec
 + traverse_trees(): allow pruning with pathspec

Will cook for a while.

* jn/remote-helpers-doc (2011-09-01) 3 commits
 - (short) documentation for the testgit remote helper
 - Documentation/git-remote-helpers: explain how import works with multiple refs
 - Documentation/remote-helpers: explain capabilities first

Not urgent.

* nm/grep-object-sha1-lock (2011-08-30) 1 commit
  (merged to 'next' on 2011-09-02 at 336f57d)
 + grep: Fix race condition in delta_base_cache

Not urgent; the topic came up a bit too late in the cycle.

* tr/mergetool-valgrind (2011-08-30) 1 commit
  (merged to 'next' on 2011-09-02 at f5f2c61)
 + Symlink mergetools scriptlets into valgrind wrappers

Not urgent; the topic came up a bit too late in the cycle.

* fg/submodule-auto-push (2011-08-20) 1 commit
  (merged to 'next' on 2011-08-24 at 398e764)
 + push: teach --recurse-submodules the on-demand option
 (this branch is used by hv/submodule-merge-search.)

What the topic aims to achieve may make sense, but the implementation
looked somewhat suboptimal.

* jc/fetch-verify (2011-09-01) 4 commits
 - NEEDSWORK: rev-list --verify-objects: run fsck_objects() on all objects listed
 - fetch: verify we have everything we need before updating our ref
 - rev-list --verify-object
 - list-objects: pass callback data to show_objects()
 (this branch uses jc/traverse-commit-list.)

Make "git fetch" verify what it received a bit more carefully before
updating the refs in the receiving end.

Not urgent; the topic came up a bit too late in the cycle.

* rc/histogram-diff (2011-08-31) 1 commit
  (merged to 'next' on 2011-09-02 at be4d245)
 + xdiff/xprepare: initialise xdlclassifier_t cf in xdl_prepare_env()

Will merge to "master".

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

* ac/describe-dirty-refresh (2011-08-11) 1 commit
  (merged to 'next' on 2011-08-23 at b873611)
 + describe: Refresh the index when run with --dirty

* bc/bisect-test-use-shell-path (2011-08-30) 1 commit
  (merged to 'next' on 2011-08-30 at c090151)
 + t6030: use $SHELL_PATH to invoke user's preferred shell instead of bare sh

* bg/t5540-osx-grep (2011-08-28) 1 commit
  (merged to 'next' on 2011-08-30 at 894af05)
 + t5540-http-test: shorten grep pattern

* bw/doc-repo-layout (2011-08-23) 2 commits
  (merged to 'next' on 2011-08-24 at 605c730)
 + Mark http-fetch without -a as deprecated
 + Documentation: Grammar correction, wording fixes and cleanup

* ci/forbid-unwanted-current-branch-update (2011-08-22) 2 commits
  (merged to 'next' on 2011-08-24 at 1e93b67)
 + Show interpreted branch name in error messages
 + Prevent force-updating of the current branch

* da/difftool-mergtool-refactor (2011-08-19) 4 commits
  (merged to 'next' on 2011-08-23 at a1cc3be)
 + mergetools/meld: Use '--output' when available
 + mergetool--lib: Refactor tools into separate files
 + mergetool--lib: Make style consistent with git
 + difftool--helper: Make style consistent with git

* di/fast-import-blob-tweak (2011-08-22) 2 commits
  (merged to 'next' on 2011-08-24 at 52eef2a)
 + fast-import: treat cat-blob as a delta base hint for next blob
 + fast-import: count and report # of calls to diff_delta in stats

* di/fast-import-deltified-tree (2011-08-14) 2 commits
  (merged to 'next' on 2011-08-23 at ee30265)
 + fast-import: prevent producing bad delta
 + fast-import: add a test for tree delta base corruption

* di/fast-import-doc (2011-08-17) 1 commit
  (merged to 'next' on 2011-08-23 at dab4088)
 + doc/fast-import: document feature import-marks-if-exists

* di/fast-import-ident (2011-08-11) 5 commits
  (merged to 'next' on 2011-08-23 at 9b86391)
 + fsck: improve committer/author check
 + fsck: add a few committer name tests
 + fast-import: check committer name more strictly
 + fast-import: don't fail on omitted committer name
 + fast-import: add input format tests

* di/fast-import-tagging (2011-08-23) 2 commits
  (merged to 'next' on 2011-08-24 at 67e0937)
 + fast-import: allow to tag newly created objects
 + fast-import: add tests for tagging blobs

* di/parse-options-split (2011-08-11) 2 commits
  (merged to 'next' on 2011-08-23 at 6cd667f)
 + Reduce parse-options.o dependencies
 + parse-options: export opterr, optbug

* en/merge-recursive-2 (2011-08-14) 57 commits
  (merged to 'next' on 2011-08-23 at ba6ad0d)
 + merge-recursive: Don't re-sort a list whose order we depend upon
 + merge-recursive: Fix virtual merge base for rename/rename(1to2)/add-dest
 + t6036: criss-cross + rename/rename(1to2)/add-dest + simple modify
 + merge-recursive: Avoid unnecessary file rewrites
 + t6022: Additional tests checking for unnecessary updates of files
 + merge-recursive: Fix spurious 'refusing to lose untracked file...' messages
 + t6022: Add testcase for spurious "refusing to lose untracked" messages
 + t3030: fix accidental success in symlink rename
 + merge-recursive: Fix working copy handling for rename/rename/add/add
 + merge-recursive: add handling for rename/rename/add-dest/add-dest
 + merge-recursive: Have conflict_rename_delete reuse modify/delete code
 + merge-recursive: Make modify/delete handling code reusable
 + merge-recursive: Consider modifications in rename/rename(2to1) conflicts
 + merge-recursive: Create function for merging with branchname:file markers
 + merge-recursive: Record more data needed for merging with dual renames
 + merge-recursive: Defer rename/rename(2to1) handling until process_entry
 + merge-recursive: Small cleanups for conflict_rename_rename_1to2
 + merge-recursive: Fix rename/rename(1to2) resolution for virtual merge base
 + merge-recursive: Introduce a merge_file convenience function
 + merge-recursive: Fix modify/delete resolution in the recursive case
 + merge-recursive: When we detect we can skip an update, actually skip it
 + merge-recursive: Provide more info in conflict markers with file renames
 + merge-recursive: Cleanup and consolidation of rename_conflict_info
 + merge-recursive: Consolidate process_entry() and process_df_entry()
 + merge-recursive: Improve handling of rename target vs. directory addition
 + merge-recursive: Add comments about handling rename/add-source cases
 + merge-recursive: Make dead code for rename/rename(2to1) conflicts undead
 + merge-recursive: Fix deletion of untracked file in rename/delete conflicts
 + merge-recursive: Split update_stages_and_entry; only update stages at end
 + merge-recursive: Allow make_room_for_path() to remove D/F entries
 + string-list: Add API to remove an item from an unsorted list
 + merge-recursive: Split was_tracked() out of would_lose_untracked()
 + merge-recursive: Save D/F conflict filenames instead of unlinking them
 + merge-recursive: Fix code checking for D/F conflicts still being present
 + merge-recursive: Fix sorting order and directory change assumptions
 + merge-recursive: Fix recursive case with D/F conflict via add/add conflict
 + merge-recursive: Avoid working directory changes during recursive case
 + merge-recursive: Remember to free generated unique path names
 + merge-recursive: Consolidate different update_stages functions
 + merge-recursive: Mark some diff_filespec struct arguments const
 + merge-recursive: Correct a comment
 + merge-recursive: Make BUG message more legible by adding a newline
 + t6022: Add testcase for merging a renamed file with a simple change
 + t6022: New tests checking for unnecessary updates of files
 + t6022: Remove unnecessary untracked files to make test cleaner
 + t6036: criss-cross + rename/rename(1to2)/add-source + modify/modify
 + t6036: criss-cross w/ rename/rename(1to2)/modify+rename/rename(2to1)/modify
 + t6036: tests for criss-cross merges with various directory/file conflicts
 + t6036: criss-cross with weird content can fool git into clean merge
 + t6036: Add differently resolved modify/delete conflict in criss-cross test
 + t6042: Add failing testcases for rename/rename/add-{source,dest} conflicts
 + t6042: Ensure rename/rename conflicts leave index and workdir in sane state
 + t6042: Add tests for content issues with modify/rename/directory conflicts
 + t6042: Add a testcase where undetected rename causes silent file deletion
 + t6042: Add a pair of cases where undetected renames cause issues
 + t6042: Add failing testcase for rename/modify/add-source conflict
 + t6042: Add a testcase where git deletes an untracked file

I've taken advantage of the end-of-summer lull and reproduced all 16000+
two-head merges in the Linux kernel repository with and without this
series to make sure they get the identical results (either cleanly merged,
or getting conflicted the same way).

* fg/submodule-ff-check-before-push (2011-08-20) 2 commits
  (merged to 'next' on 2011-08-24 at 398e764)
 + push: Don't push a repository with unpushed submodules
 (this branch is used by hv/submodule-merge-search.)

This is only the "check" part, which was sensible.  The "auto-push" patch
has been moved to its own topic.

* fk/use-kwset-pickaxe-grep-f (2011-08-28) 6 commits
  (merged to 'next' on 2011-08-28 at 3be2039)
 + obstack: Fix portability issues
  (merged to 'next' on 2011-08-23 at 93ba509)
 + Use kwset in grep
 + Use kwset in pickaxe
 + Adapt the kwset code to Git
 + Add string search routines from GNU grep
 + Add obstack.[ch] from EGLIBC 2.10

* gb/maint-am-patch-format-error-message (2011-08-29) 1 commit
  (merged to 'next' on 2011-08-30 at ed01a1d)
 + am: format is in $patch_format, not parse_patch

* gb/maint-am-stgit-author-to-from-fix (2011-08-29) 1 commit
  (merged to 'next' on 2011-08-30 at 0740010)
 + am: fix stgit patch mangling

* jc/clean-exclude-doc (2011-08-28) 1 commit
  (merged to 'next' on 2011-08-30 at 79dd173)
 + Documentation: clarify "git clean -e <pattern>"

* jc/combine-diff-callback (2011-08-20) 1 commit
  (merged to 'next' on 2011-08-24 at 9f9b42d)
 + combine-diff: support format_callback
 (this branch is used by fg/submodule-ff-check-before-push and hv/submodule-merge-search.)

* jc/maint-autofix-tag-in-head (2011-08-19) 1 commit
  (merged to 'next' on 2011-08-23 at 18cee02)
 + commit: reduce use of redundant global variables

* jc/maint-clone-alternates (2011-08-23) 2 commits
  (merged to 'next' on 2011-08-23 at 7280deb)
 + clone: clone from a repository with relative alternates
 + clone: allow more than one --reference

* jc/merge-reword (2011-05-25) 1 commit
  (merged to 'next' on 2011-08-24 at aa5cf7b)
 + merge: reword the final message

* jk/color-and-pager (2011-08-19) 10 commits
  (merged to 'next' on 2011-08-23 at cbb9495)
 + want_color: automatically fallback to color.ui
 + diff: don't load color config in plumbing
 + config: refactor get_colorbool function
 + color: delay auto-color decision until point of use
 + git_config_colorbool: refactor stdout_is_tty handling
 + diff: refactor COLOR_DIFF from a flag into an int
 + setup_pager: set GIT_PAGER_IN_USE
 + t7006: use test_config helpers
 + test-lib: add helper functions for config
 + t7006: modernize calls to unset
 (this branch is used by jk/pager-with-alias and jk/pager-with-external-command.)

* jk/pager-with-external-command (2011-08-19) 1 commit
  (merged to 'next' on 2011-08-24 at 083f5da)
 + support pager.* for external commands
 (this branch is used by jk/pager-with-alias and jk/pager-with-alias; uses jk/color-and-pager.)

* jn/plug-empty-tree-leak (2011-08-16) 2 commits
  (merged to 'next' on 2011-08-23 at aee2184)
 + merge-recursive: take advantage of hardcoded empty tree
 + revert: plug memory leak in "cherry-pick root commit" codepath

* js/i18n-scripts (2011-08-08) 5 commits
  (merged to 'next' on 2011-08-23 at a1b5529)
 + submodule: take advantage of gettextln and eval_gettextln.
 + stash: take advantage of eval_gettextln
 + pull: take advantage of eval_gettextln
 + git-am: take advantage of gettextln and eval_gettextln.
 + gettext: add gettextln, eval_gettextln to encode common idiom

* js/i18n-scripts-2 (2011-08-30) 1 commit
  (merged to 'next' on 2011-08-30 at 5a144a2)
 + bisect: take advantage of gettextln, eval_gettextln.

* mg/branch-set-upstream-previous (2011-08-19) 1 commit
  (merged to 'next' on 2011-08-23 at acef0b6)
 + branch.c: use the parsed branch name

* mg/maint-notes-C-doc (2011-08-25) 1 commit
  (merged to 'next' on 2011-08-30 at 6f3281a)
 + git-notes.txt: clarify -C vs. copy and -F

* mh/attr (2011-08-14) 7 commits
  (merged to 'next' on 2011-08-23 at 22faa6e)
 + Unroll the loop over passes
 + Change while loop into for loop
 + Determine the start of the states outside of the pass loop
 + Change parse_attr() to take a pointer to struct attr_state
 + Increment num_attr in parse_attr_line(), not parse_attr()
 + Document struct match_attr
 + Add a file comment

* ms/daemon-timeout-is-in-seconds (2011-08-28) 1 commit
  (merged to 'next' on 2011-08-30 at 2462eb0)
 + git-daemon.txt: specify --timeout in seconds

* nd/decorate-grafts (2011-08-25) 6 commits
  (merged to 'next' on 2011-08-25 at 44967e7)
 + log: Do not decorate replacements with --no-replace-objects
  (merged to 'next' on 2011-08-23 at 475d27e)
 + log: decorate "replaced" on to replaced commits
 + log: decorate grafted commits with "grafted"
 + Move write_shallow_commits to fetch-pack.c
 + Add for_each_commit_graft() to iterate all grafts
 + decoration: do not mis-decorate refs with same prefix

* nd/maint-clone-gitdir (2011-08-22) 2 commits
  (merged to 'next' on 2011-08-24 at cbf052b)
 + clone: allow to clone from .git file
 + read_gitfile_gently(): rename misnamed function to read_gitfile()

* nk/branch-v-abbrev (2011-07-01) 1 commit
  (merged to 'next' on 2011-08-24 at e9152cf)
 + branch -v: honor core.abbrev
 (this branch is used by mg/branch-list.)

* rc/diff-cleanup-records (2011-08-17) 2 commits
  (merged to 'next' on 2011-08-23 at b8414f5)
 + Merge branch 'rc/histogram-diff' into HEAD
 + xdiff/xprepare: improve O(n*m) performance in xdl_cleanup_records()

* tr/maint-format-patch-empty-output (2011-08-29) 4 commits
  (merged to 'next' on 2011-08-30 at 078c522)
 + Document negated forms of format-patch --to --cc --add-headers
 + t4014: "no-add-headers" is actually called "no-add-header"
 + t4014: invoke format-patch with --stdout where intended
 + t4014: check for empty files from git format-patch --stdout

* tr/maint-ident-to-git-memmove (2011-08-29) 1 commit
  (merged to 'next' on 2011-08-30 at 9395a9b)
 + Use memmove in ident_to_git

* tr/maint-strbuf-grow-nul-termination (2011-08-29) 1 commit
  (merged to 'next' on 2011-08-30 at dc87192)
 + strbuf_grow(): maintain nul-termination even for new buffer

* tr/maint-t3903-misquoted-command (2011-08-30) 1 commit
  (merged to 'next' on 2011-08-30 at f533857)
 + t3903: fix misquoted rev-parse invocation

* va/p4-branch-import (2011-08-22) 4 commits
  (merged to 'next' on 2011-08-24 at f67f8af)
 + git-p4: Add simple test case for branch import
 + git-p4: Allow branch definition with git config
 + git-p4: Allow filtering Perforce branches by user
 + git-p4: Correct branch base depot path detection
 (this branch uses va/p4-rename-copy.)

* va/p4-branch-import-test-update (2011-08-29) 1 commit
  (merged to 'next' on 2011-08-30 at 5c54fb3)
 + git-p4: simple branch tests edits

* va/p4-rename-copy (2011-08-22) 5 commits
  (merged to 'next' on 2011-08-24 at f1faa94)
 + git-p4: Process detectCopiesHarder with --bool
 + git-p4: Add test case for copy detection
 + git-p4: Add test case for rename detection
 + git-p4: Add description of rename/copy detection options
 + git-p4: Allow setting rename/copy detection threshold
 (this branch is used by va/p4-branch-import.)

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

* jk/add-i-hunk-filter (2011-07-27) 5 commits
  (merged to 'next' on 2011-08-11 at 8ff9a56)
 + add--interactive: add option to autosplit hunks
 + add--interactive: allow negatation of hunk filters
 + add--interactive: allow hunk filtering on command line
 + add--interactive: factor out regex error handling
 + add--interactive: refactor patch mode argument processing

Needs documentation updates, tests, and integration with the higher level
callers, e.g. "git add -p".

* jh/receive-count-limit (2011-05-23) 10 commits
 - receive-pack: Allow server to refuse pushes with too many objects
 - pack-objects: Estimate pack size; abort early if pack size limit is exceeded
 - send-pack/receive-pack: Allow server to refuse pushing too large packs
 - pack-objects: Allow --max-pack-size to be used together with --stdout
 - send-pack/receive-pack: Allow server to refuse pushes with too many commits
 - pack-objects: Teach new option --max-commit-count, limiting #commits in pack
 - receive-pack: Prepare for addition of the new 'limit-*' family of capabilities
 - Tighten rules for matching server capabilities in server_supports()
 - send-pack: Attempt to retrieve remote status even if pack-objects fails
 - Update technical docs to reflect side-band-64k capability in receive-pack

Would need another round to separate per-pack and per-session limits.

* jm/mergetool-pathspec (2011-06-22) 2 commits
 - mergetool: Don't assume paths are unmerged
 - mergetool: Add tests for filename with whitespace

I think this is a good idea, but it probably needs a re-roll.
Cf. $gmane/176254, 176255, 166256

* jk/generation-numbers (2011-07-14) 7 commits
 - limit "contains" traversals based on commit generation
 - check commit generation cache validity against grafts
 - pretty: support %G to show the generation number of a commit
 - commit: add commit_generation function
 - add metadata-cache infrastructure
 - decorate: allow storing values instead of pointers
 - Merge branch 'jk/tag-contains-ab' (early part) into HEAD

The initial "tag --contains" de-pessimization without need for generation
numbers is already in; backburnered.

* sr/transport-helper-fix-rfc (2011-07-19) 2 commits
 - t5800: point out that deleting branches does not work
 - t5800: document inability to push new branch with old content

* po/cygwin-backslash (2011-08-05) 2 commits
 - On Cygwin support both UNIX and DOS style path-names
 - git-compat-util: add generic find_last_dir_sep that respects is_dir_sep

I think a further refactoring (no, not my suggestion) was offered?

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

* jc/traverse-commit-list (2011-08-22) 3 commits
  (merged to 'next' on 2011-08-24 at df50dd7)
 + revision.c: update show_object_with_name() without using malloc()
 + revision.c: add show_object_with_name() helper function
 + rev-list: fix finish_object() call
 (this branch is used by jc/fetch-verify.)

Not urgent; will not be in 1.7.7.

* fk/make-auto-header-dependencies (2011-08-18) 1 commit
  (merged to 'next' on 2011-08-24 at 3da2c25)
 + Makefile: Use computed header dependencies if the compiler supports it
 (this branch is used by da/make-auto-header-dependencies.)

Not urgent; will not be in 1.7.7.

* mh/iterate-refs (2011-08-14) 6 commits
 - Retain caches of submodule refs
 - Store the submodule name in struct cached_refs
 - Allocate cached_refs objects dynamically
 - Change the signature of read_packed_refs()
 - Access reference caches only through new function get_cached_refs()
 - Extract a function clear_cached_refs()

I did not see anything fundamentally wrong with this series, but it was
unclear what the benefit of these changes are.  If the series were to read
parts of the ref hierarchy (like refs/heads/) lazily, the story would
have been different, though.

Not urgent; will not be in 1.7.7.

* hv/submodule-update-none (2011-08-11) 2 commits
  (merged to 'next' on 2011-08-24 at 5302fc1)
 + add update 'none' flag to disable update of submodule by default
 + submodule: move update configuration variable further up

Not urgent; will not be in 1.7.7.

* jc/lookup-object-hash (2011-08-11) 6 commits
  (merged to 'next' on 2011-08-24 at 5825411)
 + object hash: replace linear probing with 4-way cuckoo hashing
 + object hash: we know the table size is a power of two
 + object hash: next_size() helper for readability
 + pack-objects --count-only
 + object.c: remove duplicated code for object hashing
 + object.c: code movement for readability

I do not think there is anything fundamentally wrong with this series, but
the risk of breakage far outweighs observed performance gain in one
particular workload. Will keep it in 'next' at least for one cycle.

Not urgent; will not be in 1.7.7.

* fg/submodule-git-file-git-dir (2011-08-22) 2 commits
  (merged to 'next' on 2011-08-23 at 762194e)
 + Move git-dir for submodules
 + rev-parse: add option --resolve-git-dir <path>

I do not think there is anything fundamentally wrong with this series, but
the risk of breakage outweighs any benefit for having this new
feature. Will keep it in 'next' at least for one cycle.

Not urgent; will not be in 1.7.7.

* jk/http-auth-keyring (2011-08-03) 13 commits
  (merged to 'next' on 2011-08-03 at b06e80e)
 + credentials: add "getpass" helper
 + credentials: add "store" helper
 + credentials: add "cache" helper
 + docs: end-user documentation for the credential subsystem
 + http: use hostname in credential description
 + allow the user to configure credential helpers
 + look for credentials in config before prompting
 + http: use credential API to get passwords
 + introduce credentials API
 + http: retry authentication failures for all http requests
 + remote-curl: don't retry auth failures with dumb protocol
 + improve httpd auth tests
 + url: decode buffers that are not NUL-terminated

Looked mostly reasonable except for the limitation that it is not clear
how to deal with a site at which a user needs to use different passwords 
for different repositories. Will keep it in "next" at least for one cycle,
until we start hearing real-world success reports on the list.

Not urgent; will not be in 1.7.7.

* rr/revert-cherry-pick-continue (2011-08-08) 18 commits
  (merged to 'next' on 2011-08-24 at 712c115)
 + revert: Propagate errors upwards from do_pick_commit
 + revert: Introduce --continue to continue the operation
 + revert: Don't implicitly stomp pending sequencer operation
 + revert: Remove sequencer state when no commits are pending
 + reset: Make reset remove the sequencer state
 + revert: Introduce --reset to remove sequencer state
 + revert: Make pick_commits functionally act on a commit list
 + revert: Save command-line options for continuing operation
 + revert: Save data for continuing after conflict resolution
 + revert: Don't create invalid replay_opts in parse_args
 + revert: Separate cmdline parsing from functional code
 + revert: Introduce struct to keep command-line options
 + revert: Eliminate global "commit" variable
 + revert: Rename no_replay to record_origin
 + revert: Don't check lone argument in get_encoding
 + revert: Simplify and inline add_message_to_msg
 + config: Introduce functions to write non-standard file
 + advice: Introduce error_resolve_conflict

Will keep it in 'next' at least for one cycle.
Not urgent; will not be in 1.7.7.

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

* jk/pager-with-alias (2011-08-19) 1 commit
 . support pager.* for aliases

^ permalink raw reply

* Re: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo
From: Sam Vilain @ 2011-09-02 21:30 UTC (permalink / raw)
  To: Bryan Jacobs; +Cc: git, Eric Wong
In-Reply-To: <20110902154206.331b80e9@robyn.woti.com>

On 9/2/11 12:42 PM, Bryan Jacobs wrote:
> On Fri, 02 Sep 2011 12:01:09 -0700
> Sam Vilain<sam@vilain.net>  wrote:
>
>> That's one way to do it; in fact, if the trees match you don't need
>> to do anything complicated like cherry-pick.
>>
>> ie, say you're committing
>>
>>      r1---A---B---C---D
>>
>> and it blows up at
>>
>>      r1--r2--r3--C---D
>>
>> So long as the tree from the fetched r3 == the tree from B, then you
>> can just go ahead and write out new commits for C and D without doing
>> any merging (ie cherry-pick or rebase).  You could also put merge
>> commits back the way they were, too.
> When you say "write out new commits" you mean create a commit object
> with the same contents, but a different parent? Does git-svn do this
> somewhere already?

I guess it doesn't, but if it did it would certainly make this easier.  
I'm not sure why it would need to modify the WC at all.  Eric, is this 
just historical or is there a better reason for that?

Sam

^ permalink raw reply

* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Drew Northup @ 2011-09-02 21:18 UTC (permalink / raw)
  To: Anatol Pomozov; +Cc: git
In-Reply-To: <1314997486-29996-1-git-send-email-anatol.pomozov@gmail.com>

On Fri, 2011-09-02 at 14:04 -0700, Anatol Pomozov wrote:

> -	linkgit:git-config[1].
> +	linkgit:git-config[1]. Note that if this option is specified
> +	then only tags are fetched, refs under refs/heads/* stay unchanged.

Anatol,
Looks like a sane change to me. You might want to check
Documentation/SubmittingPatches in the git repository if you want Junio
to include your patch. It is probably also worth CC-ing those whom did
the previous work on that documentation and the tags code to make sure
your change is correct.

-- 
-Drew Northup
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* [PATCH] Clarify that '--tags' fetches tags only
From: Anatol Pomozov @ 2011-09-02 21:04 UTC (permalink / raw)
  To: git; +Cc: Anatol Pomozov

---
 Documentation/fetch-options.txt |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 39d326a..37d2316 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -63,7 +63,8 @@ ifndef::git-pull[]
 	flag lets all tags and their associated objects be
 	downloaded. The default behavior for a remote may be
 	specified with the remote.<name>.tagopt setting. See
-	linkgit:git-config[1].
+	linkgit:git-config[1]. Note that if this option is specified
+	then only tags are fetched, refs under refs/heads/* stay unchanged.
 
 --recurse-submodules[=yes|on-demand|no]::
 	This option controls if and under what conditions new commits of
-- 
1.7.7.rc0.72.g4b5ea.dirty

^ permalink raw reply related

* Re: [PULL] various git-svn updates
From: Junio C Hamano @ 2011-09-02 19:43 UTC (permalink / raw)
  To: Eric Wong; +Cc: Bryan Jacobs, Sam Vilain, H Krishnan, Ray Chen, git
In-Reply-To: <20110901220006.GA18321@dcvr.yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> Please pull from "master" on git://bogomips.org/git-svn.git
>
>   Bryan Jacobs (1):
>         git-svn: Teach dcommit --mergeinfo to handle multiple lines
>
>   Eric Wong (1):
>         git-svn: fix fetch with moved path when using rewriteRoot
>
>   Ray Chen (1):
>         git-svn: New flag to emulate empty directories
>
> commit 85f022e9c124ffeda31a50cab878e1418d694d87 (fix fetch with moved path)
> is also suitable for cherry-picking into maint.  All tests pass on
> Debian 6.0 (x86_64).

Thanks, pulled but not yet pushed out.

^ permalink raw reply

* Re: [spf:guess,iffy] Re: [spf:guess,iffy] [PATCH] git-svn: teach git-svn to populate svn:mergeinfo
From: Bryan Jacobs @ 2011-09-02 19:42 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, Eric Wong
In-Reply-To: <4E6127F5.5070009@vilain.net>

On Fri, 02 Sep 2011 12:01:09 -0700
Sam Vilain <sam@vilain.net> wrote:

> That's one way to do it; in fact, if the trees match you don't need
> to do anything complicated like cherry-pick.
> 
> ie, say you're committing
> 
>     r1---A---B---C---D
> 
> and it blows up at
> 
>     r1--r2--r3--C---D
> 
> So long as the tree from the fetched r3 == the tree from B, then you
> can just go ahead and write out new commits for C and D without doing
> any merging (ie cherry-pick or rebase).  You could also put merge
> commits back the way they were, too.

When you say "write out new commits" you mean create a commit object
with the same contents, but a different parent? Does git-svn do this
somewhere already?

> If they don't match, then something went wrong with the push really,
> or there is something weird going on.  I'd try to avoid using cherry
> pick automatically in situations like this.  There are too many error
> modes, and if it only happens when you don't know what's going on,
> it's not a good idea to try to fix that.  If it /is/ a sufficiently
> unlikely error (ie, the trees not matching as above), then it would
> be better to simply bomb out and provide two commands:
> 
> * a 'git reset' command to restore to previous state (ie, before the 
> dcommit)
> * a 'git rebase' command to attempt to put the new history on top of
> the new upstream.  Rebase doesn't work with merges of course but it
> still should help the user figure out what to do.
> 
> Another benefit of this approach is that you don't need to muck with
> the WC + index at all, no matter what happens.

All of the above sounds good to me. I haven't taken the time to
understand how git-svn sends changesets upstream (I only know it mucks
with the WC from empirical experience) so I don't know how easy it would
be to change the methodology, though.

Would this also mean we could dcommit from a dirty checkout? Having to
stash/unstash is a nuisance.

> Sam
> 

^ permalink raw reply

* Re: rev-list --cherry-pick and context lines
From: Junio C Hamano @ 2011-09-02 19:13 UTC (permalink / raw)
  To: Stefan Haller; +Cc: Michael J Gruber, git
In-Reply-To: <1k6zcbk.ov5qevxc1a91M%lists@haller-berlin.de>

lists@haller-berlin.de (Stefan Haller) writes:

> Is there a reason why the hard-coded default is 3 in the current code?
> It seems to me that 1 would be a better choice; it would mean "patches
> are equal if their added/removed lines are the same, and they could be
> cherry-picked without conflicts."

Even if two patches have the same added/deleted lines, depending on where
in the preimage they are applied (which is given by the context lines),
the meaning of the patches can be and are very different. So if the
default for showing uses 3-line contect (hence applying, as e-mailed
patches are usually generated with 3-line context), it makes sense for the
default to generate patch id to match it.

As Michael hinted, I think it would make sense to reduce the number of
context lines to generate patch-id the same way if/when you reduce the
context to smaller number of lines for the purpose of patch application,
but it does not make sense to use zero-line context as default.

^ permalink raw reply

* Re: [spf:guess,iffy] Re: [spf:guess,iffy] [PATCH] git-svn: teach git-svn to populate svn:mergeinfo
From: Sam Vilain @ 2011-09-02 19:01 UTC (permalink / raw)
  To: Bryan Jacobs; +Cc: git, Eric Wong
In-Reply-To: <20110902144922.383ed0f1@robyn.woti.com>

On 9/2/11 11:49 AM, Bryan Jacobs wrote:
> I could do a pass through all the commits which are about to be sent
> out to SVN to check if this is going to happen, yes. But I think a
> better solution would be to change how the changes are replayed by
> git-svn dcommit: right now, all changes are applied to the WC, then it
> sequentially does an add+dcommit for each patch? Right? I think it might
> be better to reset --hard to the parent, then pick each change into the
> WC+index before committing. That way if you abort early, cleaning up
> just consists of rebasing the stack onto the last change you sent
> upstream.

That's one way to do it; in fact, if the trees match you don't need to 
do anything complicated like cherry-pick.

ie, say you're committing

    r1---A---B---C---D

and it blows up at

    r1--r2--r3--C---D

So long as the tree from the fetched r3 == the tree from B, then you can 
just go ahead and write out new commits for C and D without doing any 
merging (ie cherry-pick or rebase).  You could also put merge commits 
back the way they were, too.

If they don't match, then something went wrong with the push really, or 
there is something weird going on.  I'd try to avoid using cherry pick 
automatically in situations like this.  There are too many error modes, 
and if it only happens when you don't know what's going on, it's not a 
good idea to try to fix that.  If it /is/ a sufficiently unlikely error 
(ie, the trees not matching as above), then it would be better to simply 
bomb out and provide two commands:

* a 'git reset' command to restore to previous state (ie, before the 
dcommit)
* a 'git rebase' command to attempt to put the new history on top of the 
new upstream.  Rebase doesn't work with merges of course but it still 
should help the user figure out what to do.

Another benefit of this approach is that you don't need to muck with the 
WC + index at all, no matter what happens.

Sam

^ permalink raw reply

* Re: [spf:guess,iffy] [PATCH] git-svn: teach git-svn to populate svn:mergeinfo
From: Bryan Jacobs @ 2011-09-02 18:49 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, Eric Wong
In-Reply-To: <4E612319.7030006@vilain.net>

On Fri, 02 Sep 2011 11:40:25 -0700
Sam Vilain <sam@vilain.net> wrote:

> On 9/2/11 11:07 AM, Bryan Jacobs wrote:
> > For this particular case, it works well: svn:mergeinfo is populated
> > in such a way that the local merge history is recreated when
> > another git-svn user pulls down the repository. This patch thus
> > allows to git users to exchange branching and merging development
> > through a central SVN server without loss of fidelity and without
> > explicitly manipulating the mergeinfo property by hand.
> 
> Whee!  That's what I was intending when I wrote the original change.
> I might have written it myself back in 2008 or whenever it was, but I 
> found I didn't actually have any SVN projects I was sending commits
> to, let alone merges.  git-svn is a project with a continually
> atrophying userbase :-).  Thanks for picking it up.
> 

Glad to hear it. I think there's still work to be done, mostly because
I'm not very familiar with the git codebase and the "right" way to do
things, but I want this to work.

> > r1 --- r3 -- r4
> >   \
> >    r2 --- E
> >
> > F is lost and cannot be cherry-picked back onto the WC, as any
> > files created in E are already present but untracked locally.
> 
> Are r1 and r2 supposed to be on the same SVN branch?

No, different SVN branches.

> 
> Overall, I could believe that.  Perhaps it is simpler to detect those 
> situations in advance and insist the user dcommits them
> independently, although it appears to me that it would apply to any
> dcommit which failed for any reason part way through.  So perhaps
> there is a wider justification for fixing that.

I could do a pass through all the commits which are about to be sent
out to SVN to check if this is going to happen, yes. But I think a
better solution would be to change how the changes are replayed by
git-svn dcommit: right now, all changes are applied to the WC, then it
sequentially does an add+dcommit for each patch? Right? I think it might
be better to reset --hard to the parent, then pick each change into the
WC+index before committing. That way if you abort early, cleaning up
just consists of rebasing the stack onto the last change you sent
upstream.

If I get around to making git-svn put its stuff into notes, this would
be a lot easier since you could just reset --hard back to the original
HEAD, since none of the earlier commits would have been mangled. But of
course everyone who already imported a repo would be SOL if the new
version relied on that Hippocratic behavior...

> Sam

^ permalink raw reply

* Re: rev-list --cherry-pick and context lines
From: Stefan Haller @ 2011-09-02 18:45 UTC (permalink / raw)
  To: Vijay Lakshminarayanan; +Cc: Michael J Gruber, git
In-Reply-To: <87ty8udd68.fsf@gmail.com>

Vijay Lakshminarayanan <laksvij@gmail.com> wrote:

> lists@haller-berlin.de (Stefan Haller) writes:
> 
> > Is there a reason why the hard-coded default is 3 in the current code?
> 
> 3 is the default context for GNU diff (and possibly other diff
> implementations also).

I think you misunderstood my question; I realize that 3 is a useful
default for "git show" or "git log -p" et al, for the reason you give.

My question was why 3 is the hard-coded value for calculating patch-ids.
For that case, 1 seems to make more sense to me.


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

^ permalink raw reply

* Re: [spf:guess,iffy] [PATCH] git-svn: teach git-svn to populate svn:mergeinfo
From: Sam Vilain @ 2011-09-02 18:40 UTC (permalink / raw)
  To: Bryan Jacobs; +Cc: git, Eric Wong
In-Reply-To: <20110902140702.066a4668@robyn.woti.com>

On 9/2/11 11:07 AM, Bryan Jacobs wrote:
> For this particular case, it works well: svn:mergeinfo is populated in such a way that the local merge history is recreated when another git-svn user pulls down the repository. This patch thus allows to git users to exchange branching and merging development through a central SVN server without loss of fidelity and without explicitly manipulating the mergeinfo property by hand.

Whee!  That's what I was intending when I wrote the original change.  I 
might have written it myself back in 2008 or whenever it was, but I 
found I didn't actually have any SVN projects I was sending commits to, 
let alone merges.  git-svn is a project with a continually atrophying 
userbase :-).  Thanks for picking it up.

> If the user has made commits to two local branches, merges them with --no-ff, and attempts to dcommit the result, the tool will error out with a message stating that all parents must reside in the SVN repository before their merge can be committed.
>
> I was unable to discover a way to clean way to restore the repository state in the case where this happens before reaching the HEAD commit. For example:
>
> r1 --- B --- C -- F
>   \               /
>    r2 --- E ------
>
> If r1 and r2 already reside in SVN, git-svn will dcommit B and C, then error when it tries to dcommit F (since the SVN revision number for E has not yet been set; the user should dcommit E before dcommitting F). The repository SHOULD be set to the following state after this happens:
>
> r1 -- r3 -- r4 -- F
>   \               /
>    r2 --- E ------
>
> ... but git-svn seems to put all the changes from all objects that it will be dcommitting into the working copy, which means that cherry-picking F atop the state the WC is in fails (due to conflicts with "untracked" objects already added). So in this patch if you try the above, you actually end up in this state:
>
> r1 --- r3 -- r4
>   \
>    r2 --- E
>
> F is lost and cannot be cherry-picked back onto the WC, as any files created in E are already present but untracked locally.

Are r1 and r2 supposed to be on the same SVN branch?

Overall, I could believe that.  Perhaps it is simpler to detect those 
situations in advance and insist the user dcommits them independently, 
although it appears to me that it would apply to any dcommit which 
failed for any reason part way through.  So perhaps there is a wider 
justification for fixing that.

Sam

^ permalink raw reply

* Re: [PATCH v4 2/2] push: teach --recurse-submodules the on-demand option
From: Junio C Hamano @ 2011-09-02 18:21 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: git, hvoigt, jens.lehmann
In-Reply-To: <1313791728-11328-3-git-send-email-iveqy@iveqy.com>

Fredrik Gustafsson <iveqy@iveqy.com> writes:

> diff --git a/submodule.c b/submodule.c
> index 45f508c..dc95498 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -8,7 +8,10 @@
>  #include "diffcore.h"
>  #include "refs.h"
>  #include "string-list.h"
> +#include "transport.h"
>  
> +typedef int (*needs_push_func_t)(const char *path, const unsigned char sha1[20],
> +		void *data);
>  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;
> @@ -308,21 +311,24 @@ void set_config_fetch_recurse_submodules(int value)
>  	config_fetch_recurse_submodules = value;
>  }
>  
> +typedef int (*module_func_t)(const char *path, const unsigned char sha1[20], void *data);
> +
>  static int has_remote(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
>  {
>  	return 1;
>  }
>  
> -static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])
> +int submodule_needs_pushing(const char *path, const unsigned char sha1[20], void *data)
>  {
> +	int *needs_pushing = data;
> +
>  	if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
> -		return 0;
> +		return 1;
>
>  	if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
>  		struct child_process cp;
>  		const char *argv[] = {"rev-list", NULL, "--not", "--remotes", "-n", "1" , NULL};
>  		struct strbuf buf = STRBUF_INIT;
> -		int needs_pushing = 0;
>  
>  		argv[1] = sha1_to_hex(sha1);
>  		memset(&cp, 0, sizeof(cp));
> @@ -336,41 +342,74 @@ static int submodule_needs_pushing(const char *path, const unsigned char sha1[20
>  			die("Could not run 'git rev-list %s --not --remotes -n 1' command in submodule %s",
>  				sha1_to_hex(sha1), path);
>  		if (strbuf_read(&buf, cp.out, 41))
> -			needs_pushing = 1;
> +			*needs_pushing = 1;
>  		finish_command(&cp);
>  		close(cp.out);
>  		strbuf_release(&buf);
> -		return needs_pushing;
> +		return !*needs_pushing;
>  	}
> -	return 0;
> +	return 1;
> +}

It appears to me that this patch is flipping the meaning of the function,
and the returned value from here is no longer "do we know that this
submodule needs to be pushed (yes/no)?".  The function needs to be renamed
to describe what it does better.

Also you would need to give a comment before the function to describe the
semantics of these two return values (one from the function, the other
from the value placed via the callback data pointer).

The latter is especially important because the caller that gets 1 from
this function would not be able to tell if the value in the callback data
pointer is valid (only happens if "rev-list" said something) or undefined
(no assignment is ever done via *needs_pushing pointer to zero it when
"rev-list" is silent, or if no submodule is checked out at path).

> +int push_submodule(const char *path, const unsigned char sha1[20], void *data)
> +{
> +	if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
> +		return 1;
> +
> +	if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
> +		struct child_process cp;
> +		const char *argv[] = {"push", NULL};
> +
> +		memset(&cp, 0, sizeof(cp));
> +		cp.argv = argv;
> +		cp.env = local_repo_env;
> +		cp.git_cmd = 1;
> +		cp.no_stdin = 1;
> +		cp.out = -1;

Is this correct? Nobody seems to read from this pipe from the "git push"
output. Don't you either want to send it to the end user, or squelch it by
sending it to /dev/null? You could of course read its output between the
following run_command() and close() and do something intelligent depending
on what the command tells you, if you wanted to, but I somehow doubt that
is what you had in mind here...

> +		cp.dir = path;
> +		if (run_command(&cp))
> +			die("Could not run 'git push' command in submodule %s", path);
> +		close(cp.out);
> +	}
> +
> +	return 1;
>  }

Do you really want to "die" here? You would definitely do if the failure
was due to corruption of your submodule repository, but wouldn't you want
to continue pushing other submodules if you couldn't push this submodule
due to non-fast-forward (i.e. somebody else pushed there first), for
example?

> +struct collect_submodules_data {
> +	module_func_t func;
> +	void *data;
> +	int ret;
> +};

What are the meaning of these fields? Document them.

Do you really need a double indirection like this, I wonder...

>  static void collect_submodules_from_diff(struct diff_queue_struct *q,
>  					 struct diff_options *options,
>  					 void *data)
>  {
>  	int i;
> -	int *needs_pushing = data;
> +	struct collect_submodules_data *me = data;
>  
>  	for (i = 0; i < q->nr; i++) {
>  		struct diff_filepair *p = q->queue[i];
>  		if (!S_ISGITLINK(p->two->mode))
>  			continue;
> -		if (submodule_needs_pushing(p->two->path, p->two->sha1)) {
> -			*needs_pushing = 1;
> +		if (!(me->ret = me->func(p->two->path, p->two->sha1, me->data)))
>  			break;
> -		}
>  	}
>  }
>  
> -
> -static void commit_need_pushing(struct commit *commit, struct commit_list *parent, int *needs_pushing)
> +static int commit_need_pushing(struct commit *commit, struct commit_list *parent,
> +	module_func_t func, void *data)
>  {
>  	const unsigned char (*parents)[20];
>  	unsigned int i, n;
>  	struct rev_info rev;
>  
> +	struct collect_submodules_data cb;
> +	cb.func = func;

Just a style thing, but because we do not allow decl-after-statement, it
is customary to have the blank line _after_ the last decl, not in between
the declarations.

> +	cb.data = data;
> +	cb.ret = 1;
> +
>  	n = commit_list_count(parent);
>  	parents = xmalloc(n * sizeof(*parents));
>  
> @@ -382,21 +421,23 @@ static void commit_need_pushing(struct commit *commit, struct commit_list *paren
>  	init_revisions(&rev, NULL);
>  	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
>  	rev.diffopt.format_callback = collect_submodules_from_diff;
> -	rev.diffopt.format_callback_data = needs_pushing;
> +	rev.diffopt.format_callback_data = &cb;
>  	diff_tree_combined(commit->object.sha1, parents, n, 1, &rev);
>  
>  	free(parents);
> +	return cb.ret;
>  }
>  
> -int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remotes_name)
> +static int inspect_superproject_commits(unsigned char new_sha1[20], const char *remotes_name,
> +	module_func_t func, void *data)

Contrast your new name with "check-submodule-needs-pushing".  "inspect"
(or "check" for that matter) is a poor word to use in function names, as
the word by itself does not convey what aspect of the object of the verb
is being inspected or checked. The old name was fine because other words
in the name described what it was checking. The new name does not tell us
anything useful. First try to explain to yourself at high level what the
function does in a few lines, and then a more appropriate name would come
to you.

>  {
>  	struct rev_info rev;
>  	struct commit *commit;
>  	const char *argv[] = {NULL, NULL, "--not", "NULL", NULL};

What is this string "NULL" doing here???

>  	int argc = ARRAY_SIZE(argv) - 1;
>  	char *sha1_copy;
> -	int needs_pushing = 0;
>  	struct strbuf remotes_arg = STRBUF_INIT;
> +	int do_continue = 1;
>  
>  	strbuf_addf(&remotes_arg, "--remotes=%s", remotes_name);
>  	init_revisions(&rev, NULL);
> @@ -407,13 +448,25 @@ int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remote
>  	if (prepare_revision_walk(&rev))
>  		die("revision walk setup failed");
>  
> -	while ((commit = get_revision(&rev)) && !needs_pushing)
> -		commit_need_pushing(commit, commit->parents, &needs_pushing);
> +	while ((commit = get_revision(&rev)) && do_continue)
> +		do_continue = commit_need_pushing(commit, commit->parents, func, data);

A funny way to write

	white ((commit = get_revision(&rev)) != NULL) {
               if (!commit_need_pushing(commit, commit->parents, func, data))
			break;
	}

or even:

	white ((commit = get_revision(&rev)) != NULL &&
        	commit_need_pushing(commit, commit->parents, func, data))
		 ; /* nothing */

No caller of this function uses its return value (one caller uses its
return value left in "data" pointer), so I do not think you would need the
"do_continue" variable, which is misnamed (the name makes sense only as
the loop control inside this function, but does not make any sense as the
return value from this function---it does not tell the caller to continue).

As there is only this calling site of commit_need_pushing(), I wonder why
the function needs to be able to take commit and commit->parents as
separate parameters. Does it even make sense in other contexts to compare
a commit with list of commits that are not its parents and decide if the
commit needs pushing based on that comparison?

>  
>  	free(sha1_copy);
>  	strbuf_release(&remotes_arg);
>  
> -	return needs_pushing;
> +	return do_continue;
> +}
> +
> +int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remotes_name)
> +{
> +	int needs_push = 0;
> +	inspect_superproject_commits(new_sha1, remotes_name, submodule_needs_pushing, &needs_push);
> +	return needs_push;
> +}
> +
> +void push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name)
> +{
> +	inspect_superproject_commits(new_sha1, remotes_name, push_submodule, NULL);
>  }

Again the called function is misnamed as the primary purpose it is used is not
to inspect, but to cause effects. But more importantly...

What does this do, given the loop structure of inspect_superproject_commits()? 
If your superproject is three commits ahead of the remote, the get_revision()
loop may run three times, calling commit_need_pushing() and have it inspect
these superproject commits, and may find that you bound different commits
from the same submodule multiple times during these three superproject commits.
Don't you end up running "git push" multiple times?

I have to say the overall code struction of this patch is simply broken.

How about doing it this way instead?

 - Update check-submodule-needs-pushing that used to stop at the first
   submodule that are not up-to-date not to do that. Instead, loop over
   all the submodules, find and collect which ones needs pushing, and
   return it as a list of submodules. Make sure you have the same
   submodule appear at most once in the result.  You may want to rename it
   to reflect the new role of the function (i.e. collecting submodules
   that needs to be pushed). Perhaps collect_stale_submodules() or
   something.

   For this, I do not think you need to touch the implementation of the
   submodule_needs_pushing() function at all. You do not need to introduce
   the indirection such as module_func_t and collect_submodules_data
   either. The only change needed is to collect_submodules_from_diff()
   that would treat the callback data not as a pointer to int
   (needs-pushing), but as a pointer to the structure to collect the names
   of submodules that need to be pushed (e.g. "struct string_list"), and
   make it not break the loop upon the first submodule that is stale.

 - Instead of adding a call to push-unpushed-submodules before
   check-submodule-needs-pushing in transport_push(), first call
   check-submodule-needs-pushing when on-demand or check is in effect.

   When in check mode, if check-submodule-needs-pushing returned a
   non-empty list, report which ones are stale and die.

   If in on-demand mode, you have a list of submodules you need to run
   "git push" in. Iterate over that list and do your push_submodule().
   You may want to reconsider your "die()" there, though.

Hmm?

^ permalink raw reply

* Re: rev-list --cherry-pick and context lines
From: Vijay Lakshminarayanan @ 2011-09-02 18:14 UTC (permalink / raw)
  To: Stefan Haller; +Cc: Michael J Gruber, git
In-Reply-To: <1k6zcbk.ov5qevxc1a91M%lists@haller-berlin.de>

lists@haller-berlin.de (Stefan Haller) writes:

> Is there a reason why the hard-coded default is 3 in the current code?

3 is the default context for GNU diff (and possibly other diff
implementations also).

> Thanks,
>    Stefan

-- 
Cheers
~vijay

^ permalink raw reply

* [PATCH] git-svn: teach git-svn to populate svn:mergeinfo
From: Bryan Jacobs @ 2011-09-02 18:07 UTC (permalink / raw)
  To: git; +Cc: Sam Vilain, Eric Wong

>From a74814d4cd91627098e5be0209da30a2bc23904e Mon Sep 17 00:00:00 2001
From: Bryan Jacobs <bjacobs@woti.com>
Date: Thu, 1 Sep 2011 16:53:17 -0400
Subject: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo

Allow git-svn to populate the svn:mergeinfo property automatically in
a narrow range of circumstances. Specifically, when dcommitting a
revision with multiple parents, each of which have been already
committed to SVN.

In this case, the merge info is the union of that given by each of the
parents, plus all changes introduced to the first parent by the other
parents.

In all other cases where a revision has multiple parents, cause "git
svn dcommit" to raise an error rather than completing the commit and
potentially losing history information in the upstream SVN repository.

This behavior is disabled by default, and can be enabled by setting
the svn.pushmergeinfo config option.

Signed-off-by: Bryan Jacobs <bjacobs@woti.com>
---

Per previous discussion re: svn:mergeinfo handling, I am submitting for your consideration a patch which allows git-svn to write mergeinfo as well as read it.

This has several limitations. Specifically, it does not handle any cases concerning fast-forward merges, nor any with cherry-picks. It exclusively deals with --no-ff merges where both parents are already committed to SVN.

For this particular case, it works well: svn:mergeinfo is populated in such a way that the local merge history is recreated when another git-svn user pulls down the repository. This patch thus allows to git users to exchange branching and merging development through a central SVN server without loss of fidelity and without explicitly manipulating the mergeinfo property by hand.

If the user has made commits to two local branches, merges them with --no-ff, and attempts to dcommit the result, the tool will error out with a message stating that all parents must reside in the SVN repository before their merge can be committed.

I was unable to discover a way to clean way to restore the repository state in the case where this happens before reaching the HEAD commit. For example:

r1 --- B --- C -- F
 \               /
  r2 --- E ------

If r1 and r2 already reside in SVN, git-svn will dcommit B and C, then error when it tries to dcommit F (since the SVN revision number for E has not yet been set; the user should dcommit E before dcommitting F). The repository SHOULD be set to the following state after this happens:

r1 -- r3 -- r4 -- F
 \               /
  r2 --- E ------

... but git-svn seems to put all the changes from all objects that it will be dcommitting into the working copy, which means that cherry-picking F atop the state the WC is in fails (due to conflicts with "untracked" objects already added). So in this patch if you try the above, you actually end up in this state:

r1 --- r3 -- r4
 \
  r2 --- E

F is lost and cannot be cherry-picked back onto the WC, as any files created in E are already present but untracked locally.

I would appreciate any help anyone can give with what the proper way to "partially reset" a git-svn commit which gets halfway through is. My attempt is the commented-out code in merge_commit_fail below. So far as I can tell, if *any* multi-revision dcommit of a merge is aborted partway through, it may end with an unclean WC and make reflog recovery necessary (even before my changes).

The other corner case I am unsure about is when two branches are merged, then the merge result is itself merged into a third. Like so:

branch1: r1 ----- r4
                 /  \
branch2:   r2 ---    \
                      \
branch3: r3 --------- r5

This has the effect of branch3's mergeinfo containing r2 as part of both branch1 and branch2 (branch2:2 from the merge history set in r4, and branch1:2 from the changes being introduced by the second parent of r5). I wasn't sure whether the mergeinfo should be "deduplicated" and only contain r2 on branch2. Not sure what the stock SVN client does here.

Finally, this makes NO EFFORT to handle svn:mergeinfo set on subdirectories of the branch root folder. The SVN red book advises against using mergeinfo on anything other than a branch top-level folder, so I think this is acceptable.

All new behavior is disabled by default, and all non-error-handling behavior is tested/demonstrated.

 Documentation/git-svn.txt         |    8 +
 git-svn.perl                      |  230 ++++++++++++++++++++++-
 t/t9160-git-svn-mergeinfo-push.sh |   97 ++++++++++
 t/t9160/branches.dump             |  374 +++++++++++++++++++++++++++++++++++++
 4 files changed, 707 insertions(+), 2 deletions(-)
 create mode 100755 t/t9160-git-svn-mergeinfo-push.sh
 create mode 100644 t/t9160/branches.dump

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index ed5eca1..2bf5703 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -213,6 +213,14 @@ discouraged.
 	store this information (as a property), and svn clients starting from
 	version 1.5 can make use of it. 'git svn' currently does not use it
 	and does not set it automatically.
++
+[verse]
+config key: svn.pushmergeinfo
++
+This option will cause git-svn to attempt to automatically populate the
+svn:mergeinfo property in the SVN repository when possible. Currently, this can
+only be done when dcommitting non-fast-forward merges where all parents have
+already been pushed into SVN.
 
 'branch'::
 	Create a branch in the SVN repository.
diff --git a/git-svn.perl b/git-svn.perl
index 89f83fd..80df8a0 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -497,6 +497,216 @@ sub cmd_set_tree {
 	unlink $gs->{index};
 }
 
+sub split_merge_info_range {
+	my ($range) = @_;
+	if ($range =~ /(\d+)-(\d+)/o) {
+		return (int($1), int($2));
+	} else {
+		return (int($range), int($range));
+	}
+}
+
+sub combine_ranges {
+	my ($in) = @_;
+
+	my @fnums = ();
+	my @arr = split(/,/o, $in);
+	for my $element (@arr) {
+		my ($start, $end) = split_merge_info_range($element);
+		push @fnums, $start;
+	}
+
+	my @sorted = @arr [ sort {
+		$fnums[$a] <=> $fnums[$b]
+	} 0..$#arr ];
+
+	my @return = ();
+	my $last = -1;
+	my $first = -1;
+	for my $element (@sorted) {
+		my ($start, $end) = split_merge_info_range($element);
+
+		if ($last == -1) {
+			$first = $start;
+			$last = $end;
+			next;
+		}
+		if ($start <= $last+1) {
+			if ($end > $last) {
+				$last = $end;
+			}
+			next;
+		}
+		if ($first == $last) {
+			push @return, "$first";
+		} else {
+			push @return, "$first-$last";
+		}
+		$first = $start;
+		$last = $end;
+	}
+
+	if ($first != -1) {
+		if ($first == $last) {
+			push @return, "$first";
+		} else {
+			push @return, "$first-$last";
+		}
+	}
+
+	return join(',', @return);
+}
+
+sub merge_revs_into_hash {
+	my ($hash, $minfo) = @_;
+	my @lines = split(' ', $minfo);
+
+	for my $line (@lines) {
+		my ($branchpath, $revs) = split(/:/o, $line);
+
+		if (exists($hash->{$branchpath})) {
+			# Merge the two revision sets
+			my $combined = "$hash->{$branchpath},$revs";
+			$hash->{$branchpath} = combine_ranges($combined);
+		} else {
+			# Just do range combining for consolidation
+			$hash->{$branchpath} = combine_ranges($revs);
+		}
+	}
+}
+
+sub merge_merge_info {
+	my ($mergeinfo_one, $mergeinfo_two) = @_;
+	my %result_hash = ();
+
+	merge_revs_into_hash(\%result_hash, $mergeinfo_one);
+	merge_revs_into_hash(\%result_hash, $mergeinfo_two);
+
+	my $result = '';
+	# Sort below is for consistency's sake
+	for my $branchname (sort keys(%result_hash)) {
+		my $revlist = $result_hash{$branchname};
+		$result .= "$branchname:$revlist\n"
+	}
+	return $result;
+}
+
+sub merge_commit_fail {
+	my ($gs, $linear_refs, $d) = @_;
+	#while (1) {
+	#	my $cs = shift @$linear_refs or last;
+	#	command_noisy(qw/cherry-pick/, $cs);
+	#}
+	#command_noisy(qw/cherry-pick -m/, '1', $d);
+	fatal "Aborted after failed dcommit of merge revision";
+}
+
+sub populate_merge_info {
+	my ($d, $gs, $uuid, $linear_refs) = @_;
+
+	my %parentshash;
+	read_commit_parents(\%parentshash, $d);
+	my @parents = @{$parentshash{$d}};
+	if ($#parents > 0) {
+		# Merge commit
+		my $all_parents_ok = 1;
+		my $aggregate_mergeinfo = '';
+		my $rooturl = $gs->repos_root;
+		foreach my $parent (@parents) {
+			my ($branchurl, $svnrev, $paruuid) =
+				cmt_metadata($parent);
+
+			unless (defined $paruuid) {
+				# A parent is missing SVN annotations...
+				# abort the whole operation.
+				print "$parent is merged into revision $d, "
+					 ."but does not have git-svn metadata. "
+					 ."Either dcommit the branch or use a "
+					 ."local cherry-pick, FF merge, or rebase "
+					 ."instead of an explicit merge commit.\n";
+				merge_commit_fail($gs, $linear_refs, $d);
+			}
+
+			unless ($paruuid eq $uuid) {
+				# Parent has SVN metadata from different repository
+				print "merge parent $parent for change $d has "
+					 ."git-svn uuid $paruuid, while current change "
+					 ."has uuid $uuid!\n";
+				merge_commit_fail($gs, $linear_refs, $d);
+			}
+
+			unless ($branchurl =~ /^$rooturl(.*)/) {
+				# This branch is very strange indeed.
+				print "merge parent $parent for $d is on branch "
+					 ."$branchurl, which is not under the "
+					 ."git-svn root $rooturl!\n";
+				merge_commit_fail($gs, $linear_refs, $d);
+			}
+			my $branchpath = $1;
+
+			my $ra = Git::SVN::Ra->new($branchurl);
+			my (undef, undef, $props) =
+				$ra->get_dir(canonicalize_path("."), $svnrev);
+			my $par_mergeinfo = $props->{'svn:mergeinfo'};
+			unless (defined $par_mergeinfo) {
+				$par_mergeinfo = '';
+			}
+			# Merge previous mergeinfo values
+			$aggregate_mergeinfo =
+				merge_merge_info($aggregate_mergeinfo,
+								 $par_mergeinfo, 0);
+
+			next if $parent eq $parents[0]; # Skip first parent
+			# Add new changes being placed in tree by merge
+			my @cmd = (qw/rev-list --reverse/,
+					   $parent, qw/--not/);
+			foreach my $par (@parents) {
+				unless ($par eq $parent) {
+					push @cmd, $par;
+				}
+			}
+			my @revsin = ();
+			my ($revlist, $ctx) = command_output_pipe(@cmd);
+			while (<$revlist>) {
+				my $irev = $_;
+				chomp $irev;
+				my (undef, $csvnrev, undef) =
+					cmt_metadata($irev);
+				unless (defined $csvnrev) {
+					# A child is missing SVN annotations...
+					# this might be OK, or might not be.
+					warn "W:child $irev is merged into revision "
+						 ."$d but does not have git-svn metadata. "
+						 ."This means git-svn cannot determine the "
+						 ."svn revision numbers to place into the "
+						 ."svn:mergeinfo property. You must ensure "
+						 ."a branch is entirely committed to "
+						 ."SVN before merging it in order for "
+						 ."svn:mergeinfo population to function "
+						 ."properly";
+				}
+				push @revsin, $csvnrev;
+			}
+			command_close_pipe($revlist, $ctx);
+
+			last unless $all_parents_ok;
+
+			# We now have a list of all SVN revnos which are
+			# merged by this particular parent. Integrate them.
+			next if $#revsin == -1;
+			my $newmergeinfo = "$branchpath:" . join(',', @revsin);
+			$aggregate_mergeinfo =
+				merge_merge_info($aggregate_mergeinfo,
+								 $newmergeinfo, 1);
+		}
+		if ($all_parents_ok and $aggregate_mergeinfo) {
+			return $aggregate_mergeinfo;
+		}
+	}
+
+	return undef;
+}
+
 sub cmd_dcommit {
 	my $head = shift;
 	command_noisy(qw/update-index --refresh/);
@@ -547,6 +757,14 @@ sub cmd_dcommit {
 		     "without --no-rebase may be required."
 	}
 	my $expect_url = $url;
+
+	my $push_merge_info = eval {
+		command_oneline(qw/config --get svn.pushmergeinfo/) };
+	if ($push_merge_info eq "false" or $push_merge_info eq "no"
+			or $push_merge_info eq "never") {
+		$push_merge_info = 0;
+	}
+
 	Git::SVN::remove_username($expect_url);
 	while (1) {
 		my $d = shift @$linear_refs or last;
@@ -561,6 +779,12 @@ sub cmd_dcommit {
 			print "diff-tree $d~1 $d\n";
 		} else {
 			my $cmt_rev;
+
+
+			unless (defined $_merge_info or not $push_merge_info) {
+				$_merge_info = populate_merge_info($d, $gs, $uuid, $linear_refs);
+			}
+
 			my %ed_opts = ( r => $last_rev,
 			                log => get_commit_entry($d)->{log},
 			                ra => Git::SVN::Ra->new($url),
@@ -3341,8 +3565,10 @@ sub find_extra_svn_parents {
 		       );
 
 		if ( @incomplete ) {
-			warn "W:svn cherry-pick ignored ($spec) - missing "
-				.@incomplete." commit(s) (eg $incomplete[0])\n";
+			warn "W:svn mergeinfo ignored ($spec) - "
+				.@incomplete." commit(s) have not been merged into "
+				."$merge_base (eg $incomplete[0]). This change "
+				."represents a cherry-pick.\n";
 		} else {
 			warn
 				"Found merge parent (svn:mergeinfo prop): ",
diff --git a/t/t9160-git-svn-mergeinfo-push.sh b/t/t9160-git-svn-mergeinfo-push.sh
new file mode 100755
index 0000000..740adef
--- /dev/null
+++ b/t/t9160-git-svn-mergeinfo-push.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+#
+# Copyright (c) 2007, 2009 Sam Vilain
+#
+
+test_description='git-svn svn mergeinfo propagation'
+
+. ./lib-git-svn.sh
+
+test_expect_success 'load svn dump' "
+	svnadmin load -q '$rawsvnrepo' \
+	  < '$TEST_DIRECTORY/t9160/branches.dump' &&
+	git svn init --minimize-url -R svnmerge \
+	  -T trunk -b branches '$svnrepo' &&
+	git svn fetch --all
+	"
+
+test_expect_success 'propagate merge information' '
+	git config svn.pushmergeinfo yes &&
+	git checkout svnb1 &&
+	git merge --no-ff svnb2 &&
+	git svn dcommit
+	'
+
+test_expect_success 'check svn:mergeinfo' '
+	mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1)
+	echo "$mergeinfo"
+	test "$mergeinfo" = "/branches/svnb2:3,8"
+	'
+
+test_expect_success 'merge another branch' '
+	git merge --no-ff svnb3 &&
+	git svn dcommit
+	'
+
+test_expect_success 'check primary parent mergeinfo respected' '
+	mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1)
+	test "$mergeinfo" = "/branches/svnb2:3,8
+/branches/svnb3:4,9"
+	'
+
+test_expect_success 'merge existing merge' '
+	git merge --no-ff svnb4 &&
+	git svn dcommit
+	'
+
+test_expect_success "check both parents' mergeinfo respected" '
+	mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1)
+	test "$mergeinfo" = "/branches/svnb2:3,8
+/branches/svnb3:4,9
+/branches/svnb4:5-6,10-12
+/branches/svnb5:6,11"
+	'
+
+test_expect_success 'make further commits to branch' '
+	git checkout svnb2 &&
+	touch newb2file &&
+	git add newb2file &&
+	git commit -m "later b2 commit" &&
+	touch newb2file-2 &&
+	git add newb2file-2 &&
+	git commit -m "later b2 commit 2" &&
+	git svn dcommit
+	'
+
+test_expect_success 'second forward merge' '
+	git checkout svnb1 &&
+	git merge --no-ff svnb2 &&
+	git svn dcommit
+	'
+
+test_expect_success "check new mergeinfo added" '
+	mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1)
+	echo "$mergeinfo"
+	test "$mergeinfo" = "/branches/svnb2:3,8,16-17
+/branches/svnb3:4,9
+/branches/svnb4:5-6,10-12
+/branches/svnb5:6,11"
+	'
+
+test_expect_success 'reintegration merge' '
+	git checkout svnb4 &&
+	git merge --no-ff svnb1 &&
+	git svn dcommit
+	'
+
+test_expect_success "check reintegration mergeinfo" '
+	mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb4)
+	echo "$mergeinfo"
+	test "$mergeinfo" = "/branches/svnb1:2-4,7-9,13-18
+/branches/svnb2:3,8,16-17
+/branches/svnb3:4,9
+/branches/svnb4:5-6,10-12
+/branches/svnb5:6,11"
+	'
+
+test_done
diff --git a/t/t9160/branches.dump b/t/t9160/branches.dump
new file mode 100644
index 0000000..e61c3e7
--- /dev/null
+++ b/t/t9160/branches.dump
@@ -0,0 +1,374 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 1ef08553-f2d1-45df-b38c-19af6b7c926d
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2011-09-02T16:08:02.941384Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 114
+Content-length: 114
+
+K 7
+svn:log
+V 12
+Base commit
+
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:08:27.205062Z
+PROPS-END
+
+Node-path: branches
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
+Node-path: trunk
+Node-kind: dir
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+
+Revision-number: 2
+Prop-content-length: 121
+Content-length: 121
+
+K 7
+svn:log
+V 19
+Create branch svnb1
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:09:43.628137Z
+PROPS-END
+
+Node-path: branches/svnb1
+Node-kind: dir
+Node-action: add
+Node-copyfrom-rev: 1
+Node-copyfrom-path: trunk
+
+
+Revision-number: 3
+Prop-content-length: 121
+Content-length: 121
+
+K 7
+svn:log
+V 19
+Create branch svnb2
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:09:46.339930Z
+PROPS-END
+
+Node-path: branches/svnb2
+Node-kind: dir
+Node-action: add
+Node-copyfrom-rev: 1
+Node-copyfrom-path: trunk
+
+
+Revision-number: 4
+Prop-content-length: 121
+Content-length: 121
+
+K 7
+svn:log
+V 19
+Create branch svnb3
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:09:49.394515Z
+PROPS-END
+
+Node-path: branches/svnb3
+Node-kind: dir
+Node-action: add
+Node-copyfrom-rev: 1
+Node-copyfrom-path: trunk
+
+
+Revision-number: 5
+Prop-content-length: 121
+Content-length: 121
+
+K 7
+svn:log
+V 19
+Create branch svnb4
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:09:54.114607Z
+PROPS-END
+
+Node-path: branches/svnb4
+Node-kind: dir
+Node-action: add
+Node-copyfrom-rev: 1
+Node-copyfrom-path: trunk
+
+
+Revision-number: 6
+Prop-content-length: 121
+Content-length: 121
+
+K 7
+svn:log
+V 19
+Create branch svnb5
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:09:58.602623Z
+PROPS-END
+
+Node-path: branches/svnb5
+Node-kind: dir
+Node-action: add
+Node-copyfrom-rev: 1
+Node-copyfrom-path: trunk
+
+
+Revision-number: 7
+Prop-content-length: 110
+Content-length: 110
+
+K 7
+svn:log
+V 9
+b1 commit
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:10:20.292369Z
+PROPS-END
+
+Node-path: branches/svnb1/b1file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 10
+
+PROPS-END
+
+
+Revision-number: 8
+Prop-content-length: 110
+Content-length: 110
+
+K 7
+svn:log
+V 9
+b2 commit
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:10:38.429199Z
+PROPS-END
+
+Node-path: branches/svnb2/b2file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 10
+
+PROPS-END
+
+
+Revision-number: 9
+Prop-content-length: 110
+Content-length: 110
+
+K 7
+svn:log
+V 9
+b3 commit
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:10:52.843023Z
+PROPS-END
+
+Node-path: branches/svnb3/b3file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 10
+
+PROPS-END
+
+
+Revision-number: 10
+Prop-content-length: 110
+Content-length: 110
+
+K 7
+svn:log
+V 9
+b4 commit
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:11:17.489870Z
+PROPS-END
+
+Node-path: branches/svnb4/b4file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 10
+
+PROPS-END
+
+
+Revision-number: 11
+Prop-content-length: 110
+Content-length: 110
+
+K 7
+svn:log
+V 9
+b5 commit
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:11:32.277404Z
+PROPS-END
+
+Node-path: branches/svnb5/b5file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 10
+
+PROPS-END
+
+
+Revision-number: 12
+Prop-content-length: 192
+Content-length: 192
+
+K 7
+svn:log
+V 90
+Merge remote-tracking branch 'svnb5' into HEAD
+
+* svnb5:
+  b5 commit
+  Create branch svnb5
+K 10
+svn:author
+V 7
+bjacobs
+K 8
+svn:date
+V 27
+2011-09-02T16:11:54.274722Z
+PROPS-END
+
+Node-path: branches/svnb4
+Node-kind: dir
+Node-action: change
+Prop-content-length: 56
+Content-length: 56
+
+K 13
+svn:mergeinfo
+V 21
+/branches/svnb5:6,11
+
+PROPS-END
+
+
+Node-path: branches/svnb4/b5file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 0
+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709
+Content-length: 10
+
+PROPS-END
+
+
-- 
1.7.7.rc0.73.gc42335.dirty

^ permalink raw reply related

* Re: [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Jeff King @ 2011-09-02 17:53 UTC (permalink / raw)
  To: Michał Górny; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20110902193931.42593338@pomiocik.lan>

On Fri, Sep 02, 2011 at 07:39:26PM +0200, Michał Górny wrote:

> > > +	/* When having a signed tag without body, format_subject()
> > > +	 * will start to eat the signature. */
> > > +	if (buf > *signature)
> > > +		*body = *signature;
> > > +	else /* - 1 to get a trailing newline to strip */
> > > +		*body = buf - 1;
> > 
> > This last line is wrong if there is no trailing newline, no? Running
> > even the existing tests in t6300 against your new patch, I get:
> [...]
> 
> Any suggestion how to strip trailing newlines?

Just looking at your patch, it might work to do:

  else if (*buf == '\n')
          *body = buf - 1;
  else
          *body = buf;

But there may be other corner cases.  I need to read through the code
more carefully, which I should have time to do later today.

-Peff

^ permalink raw reply

* Re: [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Michał Górny @ 2011-09-02 17:39 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <20110902163903.GA21768@sigill.intra.peff.net>

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

On Fri, 2 Sep 2011 12:39:03 -0400
Jeff King <peff@peff.net> wrote:

> On Thu, Sep 01, 2011 at 06:50:01PM +0200, Michał Górny wrote:
> 
> > -	buf = strchr(buf, '\n');
> > -	if (!buf) {
> > -		*body = "";
> > -		return; /* no body */
> > -	}
> > -	while (*buf == '\n')
> > -		buf++; /* skip blank between subject and body */
> > -	*body = buf;
> > +	buf = format_subject(NULL, buf, NULL);
> > +
> > +	/* When having a signed tag without body, format_subject()
> > +	 * will start to eat the signature. */
> > +	if (buf > *signature)
> > +		*body = *signature;
> > +	else /* - 1 to get a trailing newline to strip */
> > +		*body = buf - 1;
> 
> This last line is wrong if there is no trailing newline, no? Running
> even the existing tests in t6300 against your new patch, I get:
> 
> expecting success:
>   git for-each-ref --format='%(subject)' refs/heads/master >actual &&
>   test_cmp expected actual
> 
> --- expected    2011-09-02 16:36:38.306058729 +0000
> +++ actual      2011-09-02 16:36:38.318058729 +0000
> @@ -1 +1 @@
> -Initial
> +Initia
> not ok - 28 basic atom: head subject

Any suggestion how to strip trailing newlines?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

^ permalink raw reply

* Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?
From: Junio C Hamano @ 2011-09-02 16:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Michael J Gruber, git
In-Reply-To: <20110902162524.GC19690@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> It coincides with my idea too, but it might be a very limited set. For
>> example, there may be a good "suggested by upstream" default for LHS of
>> fetch refspecs (e.g. somebody may have 47 topics published but majority of
>> people are expected to follow only his "master" branch), but it is up to
>> the user of that suggestion what the RHS would be.
>
> Yeah. That leads to synthesizing local keys based on what remote keys
> say. Which is pretty straightforward if you are just fetching the
> remote's config during clone, and then copying or creating local keys
> based on that in your own .git/config (e.g., by creating full refspecs
> with upstream's idea of the LHS, and our idea of the RHS).
> ...
> I do worry that could quickly get complex, and people would start
> wanting a Turing-complete config language. :)

My real point was that more often than not the settings of configuration
variables are inherently per repository not per project, and even though
we may hear people want shared configs, possibly in-tree, distributed as
part of the projects, such a set-up would not be very useful, before you
even consider merging updates but just taking them as suggested initial
values. The choice to take, ignore, or tweak the suggestions all depend
on the semantics of each variable, and it becomes more so once you start
talking about merging updates.

^ permalink raw reply

* Re: [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Jeff King @ 2011-09-02 16:39 UTC (permalink / raw)
  To: Michał Górny; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <1314895801-21147-1-git-send-email-mgorny@gentoo.org>

On Thu, Sep 01, 2011 at 06:50:01PM +0200, Michał Górny wrote:

> -	buf = strchr(buf, '\n');
> -	if (!buf) {
> -		*body = "";
> -		return; /* no body */
> -	}
> -	while (*buf == '\n')
> -		buf++; /* skip blank between subject and body */
> -	*body = buf;
> +	buf = format_subject(NULL, buf, NULL);
> +
> +	/* When having a signed tag without body, format_subject()
> +	 * will start to eat the signature. */
> +	if (buf > *signature)
> +		*body = *signature;
> +	else /* - 1 to get a trailing newline to strip */
> +		*body = buf - 1;

This last line is wrong if there is no trailing newline, no? Running
even the existing tests in t6300 against your new patch, I get:

expecting success:
  git for-each-ref --format='%(subject)' refs/heads/master >actual &&
  test_cmp expected actual

--- expected    2011-09-02 16:36:38.306058729 +0000
+++ actual      2011-09-02 16:36:38.318058729 +0000
@@ -1 +1 @@
-Initial
+Initia
not ok - 28 basic atom: head subject

-Peff

^ permalink raw reply

* Re: rev-list --cherry-pick and context lines
From: Stefan Haller @ 2011-09-02 16:33 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <4E60F707.40708@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> wrote:

> Stefan Haller venit, vidit, dixit 02.09.2011 12:35:
> > Consider two commits on different branches, one with this patch:
> > 
> >     diff --git a/file.txt b/file.txt
> >     index 704fa27..2f7e74c 100644
> >     --- a/file.txt
> >     +++ b/file.txt
> >     @@ -1,3 +1,3 @@
> >      old_context
> >      
> >     -foo
> >     +bar
> > 
> > and the other with this patch:
> > 
> >     diff --git a/file.txt b/file.txt
> >     index f35051b..8c7de32 100644
> >     --- a/file.txt
> >     +++ b/file.txt
> >     @@ -1,3 +1,3 @@
> >      new_context
> >      
> >     -foo
> >     +bar
> > 
> > If I run "git rev-list --cherry-pick --left-right branch1...branch2", it
> > reports both commits as being genuine commits on their respective
> > branch, even though I consider their patches to be the same.
> > 
> > I guess for my purpose I would like to have patch-ids that ignore
> > context (or that use only one line of context, I'm not sure which).
> > 
> > In fact, if I do "git show <commit> -U1 | git patch-id", both commits
> > show the same id.
> > 
> > So, would it make sense to have a parameter for git-rev-list (and
> > git-cherry) that lets you specify how much context to be used for the
> > patch ids?
> 
> It would be a bit like the patch below. "git log" accepts diff options already.
> But:
> [...]

Thanks a lot.  I can't contribute much to answering your "But:"
questions; I can only add more questions myself. :-)

Is there a reason why the hard-coded default is 3 in the current code?
It seems to me that 1 would be a better choice; it would mean "patches
are equal if their added/removed lines are the same, and they could be
cherry-picked without conflicts."

Now, I'm in a situation where I'll be stuck with git 1.7.1 for quite a
while, so no patch is going to help me.  It looks like the only way to
get the behaviour I want is to reimplement git-rev-list --cherry-pick
myself, feeding each patch to git-patch-id, right? (Horribly
inefficient, but might be good enough for my purpose. Just wondering if
I'm missing a smarter way to solve it.)

Thanks,
   Stefan


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

^ permalink raw reply

* Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?
From: Jeff King @ 2011-09-02 16:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, git
In-Reply-To: <7v4o0uncq0.fsf@alter.siamese.dyndns.org>

On Fri, Sep 02, 2011 at 09:14:15AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > [1] My idea of "limited" would be an allow-known-good list of harmless
> > config keys which we would respect when they came from the remote, with
> > the option for the user to whitelist or blacklist more keys if they
> > wanted.
> 
> It coincides with my idea too, but it might be a very limited set. For
> example, there may be a good "suggested by upstream" default for LHS of
> fetch refspecs (e.g. somebody may have 47 topics published but majority of
> people are expected to follow only his "master" branch), but it is up to
> the user of that suggestion what the RHS would be.

Yeah. That leads to synthesizing local keys based on what remote keys
say. Which is pretty straightforward if you are just fetching the
remote's config during clone, and then copying or creating local keys
based on that in your own .git/config (e.g., by creating full refspecs
with upstream's idea of the LHS, and our idea of the RHS).

But it becomes hard to keep your local config in sync with updates on
the remote end. When the remote now adds "next" to the list of
interesting branches, by what mechanism do you fix up your local config?
Certainly we wouldn't want to rewrite the local config without
consulting the user, because they may have reviewed or modified it since
it was created.

One possible solution is that the local config could dynamically depend
on the remote config. E.g., the fetch refspec has something like a
wildcard that matches only the branches that the remote provides to us
via some "interesting branches" config key. Then it's OK for git to
update the "interesting branches" key from the remote. Either the user
is OK with respecting that (because they have left the wildcard in
place), or not (because they have changed the refspec not to use that
wildcard).

I do worry that could quickly get complex, and people would start
wanting a Turing-complete config language. :)

-Peff

^ permalink raw reply

* Re: [PATCH] sha1_file: Remove relative entries limitation
From: Junio C Hamano @ 2011-09-02 16:22 UTC (permalink / raw)
  To: Hui Wang; +Cc: git
In-Reply-To: <4E60A699.9080302@gmail.com>

Hui Wang <jason77.wang@gmail.com> writes:

> Does this patch make sense?

I do not see anything wrong with it per-se, but I'd like to see somebody
made an effort to dig why the restriction is there in the first place. The
check is very much deliberately placed there and there may be a good
reason why we do not resolve relatives of indirect alternates that you are
missing when you wrote the patch (and I am missing when I wrote this
response).

^ permalink raw reply

* Re: Dropping '+' from fetch = +refs/heads/*:refs/remotes/origin/*?
From: Junio C Hamano @ 2011-09-02 16:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Michael J Gruber, git
In-Reply-To: <20110902152947.GB19213@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> [1] My idea of "limited" would be an allow-known-good list of harmless
> config keys which we would respect when they came from the remote, with
> the option for the user to whitelist or blacklist more keys if they
> wanted.

It coincides with my idea too, but it might be a very limited set. For
example, there may be a good "suggested by upstream" default for LHS of
fetch refspecs (e.g. somebody may have 47 topics published but majority of
people are expected to follow only his "master" branch), but it is up to
the user of that suggestion what the RHS would be.

^ permalink raw reply

* Re: [PATCHv2 1/2] remote: write correct fetch spec when renaming remote 'remote'
From: Jeff King @ 2011-09-02 16:03 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <1314924634-12235-1-git-send-email-martin.von.zweigbergk@gmail.com>

On Thu, Sep 01, 2011 at 08:50:33PM -0400, Martin von Zweigbergk wrote:

> +	strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old);
>  	for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
>  		char *ptr;
>  
>  		strbuf_reset(&buf2);
>  		strbuf_addstr(&buf2, oldremote->fetch_refspec[i]);
> -		ptr = strstr(buf2.buf, rename.old);
> +		ptr = strstr(buf2.buf, old_remote_context.buf);
>  		if (ptr)
> -			strbuf_splice(&buf2, ptr-buf2.buf, strlen(rename.old),
> -					rename.new, strlen(rename.new));
> +			strbuf_splice(&buf2,
> +				      ptr-buf2.buf + strlen(":refs/remotes/"),
> +				      strlen(rename.old), rename.new,
> +				      strlen(rename.new));
>  		if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0))
>  			return error("Could not append '%s'", buf.buf);

If we find a refspec that does not match this pattern, what should we
do? This code silently ignores it.

At the very least, I'd like to see a warning indicating that one or more
fetch refspecs were not rewritten, and that the user should update the
config manually.

What should happen if there are multiple refspecs, and one doesn't
match? Should we rewrite the ones we can, or should we abort and not
rewrite anything at all? I think the latter is probably less confusing.

These are corner cases, obviously, and most people will just have the
stock refspecs or something that follows their pattern.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] remote: "rename o foo" should not rename ref "origin/bar"
From: Jeff King @ 2011-09-02 15:55 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.2.00.1109012022110.7267@debian>

On Thu, Sep 01, 2011 at 08:35:01PM -0400, Martin von Zweigbergk wrote:

> > To be totally correct, shouldn't this check each ref against the RHS of
> > the remote's old refspec, and rename it according to the remote's new
> > refspec?
> 
> That's what I thought too, but was planning on leaving it for a
> separate patch. However, after changing patch 1 to only update the
> fetch refspecs from "refs/remotes/$OLD" to "refs/remotes/$NEW", there
> is no other place in the fetchspec where a remote name can occur and
> 'git remote rename' still understands it. So since we're now being
> more conservative about updating refspecs, I guess we need to be
> equally conservative about updating ref names.

Yeah, I think with the change in patch 1, your patch 2 is very
reasonable. It is admitting that "git remote" is really about tweaking
the default remote config, and punting on config that doesn't appear to
follow it. And doing it consistently. I don't think there's nothing
wrong with that.

-Peff

^ permalink raw reply

* Re: rev-list --cherry-pick and context lines
From: Michael J Gruber @ 2011-09-02 15:32 UTC (permalink / raw)
  To: Stefan Haller; +Cc: git
In-Reply-To: <1k6yux4.x1kexb19bkrqpM%lists@haller-berlin.de>

Stefan Haller venit, vidit, dixit 02.09.2011 12:35:
> Consider two commits on different branches, one with this patch:
> 
>     diff --git a/file.txt b/file.txt
>     index 704fa27..2f7e74c 100644
>     --- a/file.txt
>     +++ b/file.txt
>     @@ -1,3 +1,3 @@
>      old_context
>      
>     -foo
>     +bar
> 
> and the other with this patch:
> 
>     diff --git a/file.txt b/file.txt
>     index f35051b..8c7de32 100644
>     --- a/file.txt
>     +++ b/file.txt
>     @@ -1,3 +1,3 @@
>      new_context
>      
>     -foo
>     +bar
> 
> If I run "git rev-list --cherry-pick --left-right branch1...branch2", it
> reports both commits as being genuine commits on their respective
> branch, even though I consider their patches to be the same.
> 
> I guess for my purpose I would like to have patch-ids that ignore
> context (or that use only one line of context, I'm not sure which).
> 
> In fact, if I do "git show <commit> -U1 | git patch-id", both commits
> show the same id.
> 
> So, would it make sense to have a parameter for git-rev-list (and
> git-cherry) that lets you specify how much context to be used for the
> patch ids?

It would be a bit like the patch below. "git log" accepts diff options already.
But:

- Do we want the patch id generation and the patch display (-p) to use the
  same options?

- -U1 implies -p/--patch and there is no --no-patch.

- Which other diff options do we want to pass to the patch id
  generation: --histogram, --patience, ...?

Cheers,
Michael

----

diff --git i/diff.c w/diff.c
index fcc0078..4e82912 100644
--- i/diff.c
+++ w/diff.c
@@ -4103,7 +4103,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
 		}
 
 		xpp.flags = 0;
-		xecfg.ctxlen = 3;
+		xecfg.ctxlen = options->context;
 		xecfg.flags = 0;
 		xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,
 			      &xpp, &xecfg);
diff --git i/revision.c w/revision.c
index 072ddac..5a98ed9 100644
--- i/revision.c
+++ w/revision.c
@@ -601,6 +601,7 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 	left_first = left_count < right_count;
 	init_patch_ids(&ids);
 	ids.diffopts.pathspec = revs->diffopt.pathspec;
+	ids.diffopts.context = revs->diffopt.context;
 
 	/* Compute patch-ids for one side */
 	for (p = list; p; p = p->next) {

^ permalink raw reply related


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