Git development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] Add Git::config_path()
From: Jakub Narebski @ 2011-10-07 20:32 UTC (permalink / raw)
  To: Cord Seele; +Cc: Matthieu Moy, git, Junio C Hamano, Eric Wong, Cord Seele
In-Reply-To: <1317379945-9355-2-git-send-email-cowose@gmail.com>

Cord Seele <cowose@googlemail.com> writes:

> Use --path option when calling 'git config' thus allow for pathname
> expansion, e.g. a tilde.
> 
> Signed-off-by: Cord Seele <cowose@gmail.com>
> ---
>  perl/Git.pm |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)

I think the following minimal test should be squashed in:

---
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 3787186..7558f0c 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -43,7 +43,9 @@ test_expect_success \
      git config --add test.booltrue true &&
      git config --add test.boolfalse no &&
      git config --add test.boolother other &&
-     git config --add test.int 2k
+     git config --add test.int 2k &&
+     git config --add test.path "~/foo" &&
+     git config --add test.pathexpanded "$HOME/foo"
      '
 
 # The external test will outputs its own plan
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 13ba96e..ce9340c 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -33,6 +33,8 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
 is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
 ok($r->config_bool("test.booltrue"), "config_bool: true");
 ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
+is($r->config_path("test.path"), $r->config("test.pathexpanded"),
+   "config_path: ~/foo expansion");
 our $ansi_green = "\x1b[32m";
 is($r->get_color("color.test.slot1", "red"), $ansi_green, "get_color");
 # Cannot test $r->get_colorbool("color.foo")) because we do not

^ permalink raw reply related

* What's cooking in git.git (Oct 2011, #03; Fri, 7)
From: Junio C Hamano @ 2011-10-07 20:28 UTC (permalink / raw)
  To: git

News flash: git://git.kernel.org/pub/scm/git/git.git is back.
---

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

The first wave of topics have been graduated to 'master'.  I've rebuilt
'next' while kicking some topics back to 'pu'. Also some topics have been
discarded.

Here are the repositories that have my integration branches:

With maint, master, next, pu, todo, html and man:

	git://git.kernel.org/pub/scm/git/git.git
	git://repo.or.cz/alt-git.git
	https://code.google.com/p/git-core/
	https://github.com/git/git

With only maint, master, html and man:

	git://git.sourceforge.jp/gitroot/git-core/git.git
	git://git-core.git.sourceforge.net/gitroot/git-core/git-core

With all the topics and integration branches but not todo, html or man:

	https://github.com/gitster/git

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

* jc/signed-commit (2011-10-05) 4 commits
 - commit: teach --gpg-sign option
 - Split GPG interface into its own helper library
 - rename "match_refs()" to "match_push_refs()"
 - send-pack: typofix error message
 (this branch is tangled with jc/signed-push and jc/signed-push-3.)

This is to replace the earlier "signed push" experiments.

* js/maint-merge-one-file-osx-expr (2011-10-06) 1 commit
  (merged to 'next' on 2011-10-07 at fbb28a2)
 + merge-one-file: fix "expr: non-numeric argument"

Will merge to 'master' in the third wave.

* tm/completion-commit-fixup-squash (2011-10-06) 2 commits
 - completion: commit --fixup and --squash
 - completion: unite --reuse-message and --reedit-message handling

Looked reasonable but completion is not exactly in my area so I'd like to
see others the test these in the real life and give opinions first.

* tm/completion-push-set-upstream (2011-10-06) 1 commit
 - completion: push --set-upstream

Looked reasonable but completion is not exactly in my area so I'd like to
see others the test these in the real life and give opinions first.

* js/no-cherry-pick-head-after-punted (2011-10-06) 1 commit
 - Merge branch 'js/maint-no-cherry-pick-head-after-punted' into js/no-cherry-pick-head-after-punted
 (this branch uses js/maint-no-cherry-pick-head-after-punted.)

Looked reasonable.
Will merge to 'next'.

* js/maint-no-cherry-pick-head-after-punted (2011-10-06) 2 commits
 - cherry-pick: do not give irrelevant advice when cherry-pick punted
 - revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
 (this branch is used by js/no-cherry-pick-head-after-punted.)

Looked reasonable.
Will merge to 'next'.

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

* jk/http-auth-keyring (2011-09-28) 22 commits
 - credential-cache: don't cache items without context
 - check_expirations: don't copy over same element
 - t0300: add missing EOF terminator for <<
 - credential-store: use a better storage format
 - t0300: make alternate username tests more robust
 - t0300: make askpass tests a little more robust
 - credential-cache: fix expiration calculation corner cases
 - docs: minor tweaks to credentials API
 - credentials: make credential_fill_gently() static
 - 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
 (this branch is used by js/cred-macos-x-keychain-2.)

Kicked back to 'pu' to allow design level discussions to continue.

* js/cred-macos-x-keychain-2 (2011-10-06) 1 commit
 - contrib: add a pair of credential helpers for Mac OS X's keychain
 (this branch uses jk/http-auth-keyring.)

Kicked back to 'pu' to allow design level discussions to continue.

* hv/submodule-merge-search (2011-08-26) 5 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
 - push: teach --recurse-submodules the on-demand option
 (this branch is tangled with fg/submodule-auto-push.)

The second from the bottom one needs to be replaced with a properly
written commit log message.

* fg/submodule-auto-push (2011-09-11) 2 commits
 - submodule.c: make two functions static
 - push: teach --recurse-submodules the on-demand option
 (this branch is tangled with hv/submodule-merge-search.)

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

* 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

Perhaps 281eee4 (revision: keep track of the end-user input from the
command line, 2011-08-25) would help.

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

* sp/smart-http-failure (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at 02f9982)
 + remote-curl: Fix warning after HTTP failure

Will merge to 'master' in the second wave.

* cb/do-not-pretend-to-hijack-long-help (2011-10-05) 1 commit
  (merged to 'next' on 2011-10-06 at 46851fe)
 + use -h for synopsis and --help for manpage consistently

Will merge to 'master' in the second wave.

* cp/git-web-browse-browsers (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at da42ad0)
 + git-web--browse: avoid the use of eval

Will merge to 'master' in the third wave.

* il/archive-err-signal (2011-10-05) 1 commit
  (merged to 'next' on 2011-10-06 at 7e3083f)
 + Support ERR in remote archive like in fetch/push

* nd/daemon-log-sock-errors (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 5f3630f)
 + daemon: log errors if we could not use some sockets

* nd/document-err-packet (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 0c5f5d0)
 + pack-protocol: document "ERR" line

Will merge to 'master' in the second wave.

* nd/git-daemon-error-msgs (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 209126d)
 + daemon: return "access denied" if a service is not allowed

* jc/is-url-simplify (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at d6c6741)
 + url.c: simplify is_url()

Will merge to 'master' in the third wave.

* jn/ident-from-etc-mailname (2011-10-06) 2 commits
  (merged to 'next' on 2011-10-06 at a68770d)
 + ident: do not retrieve default ident when unnecessary
 + ident: check /etc/mailname if email is unknown

* jn/no-g-plus-s-on-bsd (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 3d85674)
 + Makefile: do not set setgid bit on directories on GNU/kFreeBSD

Will merge to 'master' in the third wave.

* js/log-show-children (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at de8f6f2)
 + log --children

Will merge to 'master' in the third wave.

* rs/name-rev-usage (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at e51878e)
 + name-rev: split usage string

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* rs/test-ctype (2011-10-03) 2 commits
  (merged to 'next' on 2011-10-06 at b8c26d2)
 + test-ctype: add test for is_pathspec_magic
 + test-ctype: macrofy

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* rs/pending (2011-10-03) 8 commits
  (merged to 'next' on 2011-10-06 at 998462b)
 + commit: factor out clear_commit_marks_for_object_array
 + checkout: use leak_pending flag
 + bundle: use leak_pending flag
 + bisect: use leak_pending flag
 + revision: add leak_pending flag
 + checkout: use add_pending_{object,sha1} in orphan check
 + revision: factor out add_pending_sha1
 + checkout: check for "Previous HEAD" notice in t2020

* ph/transport-with-gitfile (2011-10-04) 4 commits
  (merged to 'next' on 2011-10-06 at 891b8b6)
 + Add test showing git-fetch groks gitfiles
 + Teach transport about the gitfile mechanism
 + Learn to handle gitfiles in enter_repo
 + enter_repo: do not modify input

* jc/grep-untracked-exclude (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at b16cffe)
 + Merge branch 'jc/maint-grep-untracked-exclude' into jc/grep-untracked-exclude
 (this branch uses bw/grep-no-index-no-exclude and jc/maint-grep-untracked-exclude.)

* jc/maint-grep-untracked-exclude (2011-10-04) 1 commit
 + grep: teach --untracked and --exclude-standard options
 (this branch is used by jc/grep-untracked-exclude; uses bw/grep-no-index-no-exclude.)

* dm/tree-walk (2011-09-28) 2 commits
  (merged to 'next' on 2011-10-06 at 76e90c3)
 + tree-walk: micro-optimization in tree_entry_interesting
 + tree-walk: drop unused parameter from match_dir_prefix

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* cs/perl-config-path-send-email (2011-09-30) 2 commits
  (merged to 'next' on 2011-10-06 at 93c00f0)
 + use new Git::config_path() for aliasesfile
 + Add Git::config_path()

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* jc/checkout-from-tree-keep-local-changes (2011-09-30) 1 commit
  (merged to 'next' on 2011-10-06 at 64061aa)
 + checkout $tree $path: do not clobber local changes in $path not in $tree

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the fourth wave.

* jc/apply-blank-at-eof-fix (2011-09-26) 1 commit
  (merged to 'next' on 2011-10-06 at a9dfd8f)
 + apply --whitespace=error: correctly report new blank lines at end

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the third wave.

* nd/sparse-doc (2011-09-26) 1 commit
  (merged to 'next' on 2011-10-06 at f6b8355)
 + git-read-tree.txt: update sparse checkout examples

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* jp/get-ref-dir-unsorted (2011-09-30) 2 commits
  (merged to 'next' on 2011-10-06 at 69fe65d)
 + refs: Use binary search to lookup refs faster
 + Don't sort ref_list too early

Will merge to 'master' in the third wave.

* jc/parse-options-boolean (2011-09-28) 5 commits
  (merged to 'next' on 2011-10-06 at dd4936c)
 + apply: use OPT_NOOP_NOARG
 + revert: use OPT_NOOP_NOARG
 + parseopt: add OPT_NOOP_NOARG
 + archive.c: use OPT_BOOL()
 + parse-options: deprecate OPT_BOOLEAN

Will merge to 'master' in the second wave.

* mh/maint-notes-merge-pathbuf-fix (2011-09-27) 1 commit
  (merged to 'next' on 2011-10-06 at 0af69bb)
 + notes_merge_commit(): do not pass temporary buffer to other function

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* ph/push-to-delete-nothing (2011-09-30) 1 commit
  (merged to 'next' on 2011-10-06 at 33ac777)
 + receive-pack: don't pass non-existent refs to post-{receive,update} hooks

Will merge to 'master' in the fourth wave.

* ps/gitweb-js-with-lineno (2011-09-27) 1 commit
  (merged to 'next' on 2011-10-06 at 9236f5e)
 + gitweb: Fix links to lines in blobs when javascript-actions are enabled

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* zj/send-email-authen-sasl (2011-09-29) 1 commit
  (merged to 'next' on 2011-10-06 at 78b31cd)
 + send-email: auth plain/login fix

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* jc/maint-diffstat-numstat-context (2011-09-22) 1 commit
  (merged to 'next' on 2011-10-06 at 36c972d)
 + diff: teach --stat/--numstat to honor -U$num

"diff" is allowed to match the common lines differently depending on how
many context lines it is showing, so running --(num)stat with 0 lines of
context internally gives a result that may be surprising to some people.

Originally merged to 'next' on 2011-09-26.
Will merge to 'master' in the second wave.

* nd/maint-sparse-errors (2011-09-22) 2 commits
  (merged to 'next' on 2011-10-06 at e3cbb90)
 + Add explanation why we do not allow to sparse checkout to empty working tree
 + sparse checkout: show error messages when worktree shaping fails

Originally merged to 'next' on 2011-09-22.
Will merge to 'master' in the third wave.

* rs/diff-cleanup-records-fix (2011-10-03) 2 commits
  (merged to 'next' on 2011-10-06 at 91f035f)
 + diff: resurrect XDF_NEED_MINIMAL with --minimal
 + Revert removal of multi-match discard heuristic in 27af01

Will merge to 'master' in the third wave.

* di/fast-import-empty-tag-note-fix (2011-09-22) 2 commits
  (merged to 'next' on 2011-10-06 at 3a01ef1)
 + fast-import: don't allow to note on empty branch
 + fast-import: don't allow to tag empty branch

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the fourth wave.

* bw/grep-no-index-no-exclude (2011-09-15) 2 commits
  (merged to 'next' on 2011-10-06 at 325270b)
 + grep --no-index: don't use git standard exclusions
 + grep: do not use --index in the short usage output
 (this branch is used by jc/grep-untracked-exclude and jc/maint-grep-untracked-exclude.)

Originally merged to 'next' on 2011-09-26.
Will merge to 'master' in the third wave.

* mh/check-ref-format-3 (2011-10-05) 23 commits
  (merged to 'next' on 2011-10-06 at c277498)
 + add_ref(): verify that the refname is formatted correctly
 + resolve_ref(): expand documentation
 + resolve_ref(): also treat a too-long SHA1 as invalid
 + resolve_ref(): emit warnings for improperly-formatted references
 + resolve_ref(): verify that the input refname has the right format
 + remote: avoid passing NULL to read_ref()
 + remote: use xstrdup() instead of strdup()
 + resolve_ref(): do not follow incorrectly-formatted symbolic refs
 + resolve_ref(): extract a function get_packed_ref()
 + resolve_ref(): turn buffer into a proper string as soon as possible
 + resolve_ref(): only follow a symlink that contains a valid, normalized refname
 + resolve_ref(): use prefixcmp()
 + resolve_ref(): explicitly fail if a symlink is not readable
 + Change check_refname_format() to reject unnormalized refnames
 + Inline function refname_format_print()
 + Make collapse_slashes() allocate memory for its result
 + Do not allow ".lock" at the end of any refname component
 + Refactor check_refname_format()
 + Change check_ref_format() to take a flags argument
 + Change bad_ref_char() to return a boolean value
 + git check-ref-format: add options --allow-onelevel and --refspec-pattern
 + t1402: add some more tests
 + get_sha1_hex(): do not read past a NUL character

* js/bisect-no-checkout (2011-09-21) 1 commit
  (merged to 'next' on 2011-10-06 at 0354e94)
 + bisect: fix exiting when checkout failed in bisect_start()

Originally merged to 'next' on 2011-09-21.
Will merge to 'master' in the third wave.

* jc/request-pull-show-head-4 (2011-10-07) 8 commits
  (merged to 'next' on 2011-10-07 at fcaeca0)
 + fmt-merge-msg: use branch.$name.description
  (merged to 'next' on 2011-10-06 at fa5e0fe)
 + request-pull: use the branch description
 + request-pull: state what commit to expect
 + request-pull: modernize style
 + branch: teach --edit-description option
 + format-patch: use branch description in cover letter
 + branch: add read_branch_desc() helper function
 + Merge branch 'bk/ancestry-path' into jc/branch-desc

Will merge to 'master' in the fourth wave.

* jm/mergetool-pathspec (2011-09-26) 2 commits
  (merged to 'next' on 2011-10-06 at b8e830f)
 + mergetool: no longer need to save standard input
 + mergetool: Use args as pathspec to unmerged files

Originally merged to 'next' on 2011-09-26.
Will merge to 'master' in the second wave.

* nd/maint-autofix-tag-in-head (2011-09-18) 4 commits
  (merged to 'next' on 2011-10-06 at c083e69)
 + Accept tags in HEAD or MERGE_HEAD
 + merge: remove global variable head[]
 + merge: use return value of resolve_ref() to determine if HEAD is invalid
 + merge: keep stash[] a local variable

Originally merged to 'next' on 2011-09-27.
Will merge to 'master' in the third wave.

* bc/attr-ignore-case (2011-10-06) 5 commits
 - attr.c: respect core.ignorecase when matching attribute patterns
 - attr: read core.attributesfile from git_default_core_config
 - builtin/mv.c: plug miniscule memory leak
 - cleanup: use internal memory allocation wrapper functions everywhere
 - attr.c: avoid inappropriate access to strbuf "buf" member

Re-rolled.

* mz/remote-rename (2011-09-11) 4 commits
  (merged to 'next' on 2011-10-06 at 96db20d)
 + remote: only update remote-tracking branch if updating refspec
 + remote rename: warn when refspec was not updated
 + remote: "rename o foo" should not rename ref "origin/bar"
 + remote: write correct fetch spec when renaming remote 'remote'

Originally merged to 'next' on 2011-09-26.
Will merge to 'master' in the second wave.

* cb/common-prefix-unification (2011-09-12) 3 commits
  (merged to 'next' on 2011-10-06 at 8349bca)
 + rename pathspec_prefix() to common_prefix() and move to dir.[ch]
 + consolidate pathspec_prefix and common_prefix
 + remove prefix argument from pathspec_prefix

Originally merged to 'next' on 2011-09-14.
Will merge to 'master' in the second wave.

* jn/maint-http-error-message (2011-09-06) 2 commits
  (merged to 'next' on 2011-10-06 at 668a706)
 + http: avoid empty error messages for some curl errors
 + http: remove extra newline in error message

Originally merged to 'next' on 2011-09-12.
Will merge to 'master' in the second wave.

* mh/iterate-refs (2011-09-11) 7 commits
  (merged to 'next' on 2011-10-06 at c7a33e5)
 + refs.c: make create_cached_refs() static
 + 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()

Originally merged to 'next' on 2011-09-27.
Will merge to 'master' in the second wave.

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

Originally merged to 'next' on 2011-08-24.
Will merge to 'master' in the second wave.

* jc/lookup-object-hash (2011-08-11) 6 commits
 - 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.

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

Originally merged to 'next' on 2011-08-23.
Will merge to 'master' in the second wave.

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

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

Discarded, expecting a fresh re-roll.

* jc/make-static (2011-09-14) 4 commits
 . exec_cmd.c: prepare_git_cmd() is sometimes used
 . environment.c: have_git_dir() has users on Cygwin
 . vcs-svn: remove unused functions and make some static
 . make-static: master

* jk/add-i-hunk-filter (2011-07-27) 5 commits
 . 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

* jc/signed-push (2011-10-05) 7 commits
 . push -s: support pre-receive-signature hook
 . push -s: receiving end
 . push -s: send signed push certificate
 . push -s: skeleton
 - Split GPG interface into its own helper library
 - rename "match_refs()" to "match_push_refs()"
 - send-pack: typofix error message
 (this branch is tangled with jc/signed-commit and jc/signed-push-3.)

This was the v2 that updated notes tree on the receiving end.

* jc/signed-push-3 (2011-10-05) 4 commits
 . push -s: signed push
 - Split GPG interface into its own helper library
 - rename "match_refs()" to "match_push_refs()"
 - send-pack: typofix error message
 (this branch is tangled with jc/signed-commit and jc/signed-push.)

This is the third edition, that moves the preparation of the notes tree to
the sending end.

* 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

Discarded, expecting another round to separate per-pack and per-session
limits.

* jk/generation-numbers (2011-09-11) 8 commits
 . metadata-cache.c: make two functions static
 . 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

* 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

Incomplete with respect to backslash processing in prefix_filename(), and
also loses the ability to escape glob specials.

^ permalink raw reply

* Re: How pretty is pretty? git cat-file -p inconsistency
From: Michael J Gruber @ 2011-10-07 20:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v62k0wudg.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 07.10.2011 20:04:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> That is, "cat file -p" pretty prints dates for tag objects but not for
>> commit objects. In fact, "-p" on commit objects does not prettify at all
>> compared to the raw content. Is that intentional?
> 
> "cat-file -p" is an ill-conceived half-ass afterthought, and I do not
> think anybody sane considers it as part of the "plumbing" ultra stable
> interface for machine consumption. See a0f15fa (Pretty-print tagger
> dates., 2006-03-01).

Uh, I see. "git cat-file -p tagname" is a bit like the the missing "git
tag show tagname" or "git show tagname" without the commit.

> 
>> I'd suggest
>> prettifying dates with "-p" for commit objects also.
> 
> Please make it so. It is your choice to do a patch to update this single
> thing first, or to discuss the output with "-p" for all the other object
> types at the same time to get the list concensus before proceeding.

I never knew how ugly the output of "git tag-file tree sha1" is. I guess
it's the type of object whose format I don't know... We don't have an
object format description in Doc/technical, do we? tree.c doesn't tell
me much.

Looking at how "cat-file -p" for tags is done makes me not want to do it
for commits ;) We do have pretty "git show" for all types of objects,
though "git cat-file -p treeobject" is more informative than "git show
treeobject". I guess I have to make up my mind about what direction to go.

Michael

^ permalink raw reply

* Re: [PATCH v2] post-receive-email: explicitly set Content-Type header
From: Jonathan Nieder @ 2011-10-07 20:19 UTC (permalink / raw)
  To: Alexey Shumkin
  Cc: Johannes Sixt, Fabian Emmes, Alexander Gerasiov, git,
	Junio C Hamano
In-Reply-To: <20111007165209.595834f2@ashu.dyn.rarus.ru>

Alexey Shumkin wrote:

> In development process under Windows non-UTF-8
> encoding is used (cp1251 in my case). So, filenames have this encoding,
> and as we know Git stores their names as is - in cp1251 - without a
> conversion. And filenames are also used in diff-stat (with
> core.quotepath= false

Yes, when a person sets [core] quotepath to false, I think it's fair to
assume for now that the filenames are in the same encoding as
everything else (whether that's UTF-8 or something else).  Maybe it
would be possible in a separate patch to add a configurable list of
encodings to try out when formatting paths for display.

Jonathan

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Jonathan Nieder @ 2011-10-07 20:12 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <4E8EED39.1060607@drmicha.warpmail.net>

Hi again,

Michael J Gruber wrote:

> I really haven't seen any convincing argument against yet. The details
> (note attached to refname object, or literal refanmes in the notes tree
> as per Jeff) should be discussed further, of course, but if branch
> descriptions aren't "notesy" then I don't know what is.

As mentioned before, I don't want to debate how Junio should spend his
time (better for each person to provide relevant information and
improvements to help out or to spend time on the alternatives one is
interested in), but as a general question, this statement looked
interesting to me.

"git notes" has a funny name, but deep down, as you know, it's about
attaching additional versioned text to commit objects without changing
their names.  Branch descriptions are not per commit object (and as a
mapping, the _keys_ are not shared), and personally I don't think they
should be versioned any more than the branch names are.

I wanted to emphasize this because

	"git notes" is not the best tool for all annotations!

This ends this public service announcement.

> Alternatively, one could store the description in a blob and refer to
> that directly, of course. I.e., have
>
> refs/description/foo
>
> point to a blob whose content is the description of the ref
>
> ref/foo

Sure, that would be a sane alternative design.  It has the advantage
of having the pumbing for fetching and pulling already set up, as you
mention.  The only disadvantages I know of are

 - "git branch -m" and "git remote rename" don't know about it yet
 - there's not one flat file you can edit to run a search/replace on
   all branch descriptions

and those aren't very serious problems.

>> I personally would prefer my branch descriptions to be non-versioned,
>> though I realize that is a matter of taste.
>
> Do you prefer you commit notes to be non-versioned?

No, I like them versioned.  If I didn't, why wouldn't I have sent a
patch to change that?  Maybe some day there will be a "git notes log"
tool to track the history of a note, taking changes in fanout into
account.

Hope that clarifies a little.

^ permalink raw reply

* Re: [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
From: Junio C Hamano @ 2011-10-07 20:09 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Phil Hord, Sitaram Chamarty, git
In-Reply-To: <20111006181522.GA2936@sita-lt.atc.tcs.com>

Sitaram Chamarty <sitaramc@gmail.com> writes:

> This is useful if you forgot to restrict the diff to the paths you want
> to see, or selecting precisely the ones you want is too much typing.
>
> Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>
> ---
>
> On Thu, Oct 06, 2011 at 10:36:40AM -0700, Junio C Hamano wrote:
>
>> Thanks. It is clear from the subject and the patch text that you are
>> changing "hit return to unconditionally launch" into "launch it if you
>> want to", but can you give justification why a choice not to launch is
>> needed in the log message?
>
> OK; done.

Looks OK from a cursory viewing. Do we want some additional tests?

For that matter, have you run the test suite with this patch applied (I
haven't)?

>  git-difftool--helper.sh |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
> index 8452890..0468446 100755
> --- a/git-difftool--helper.sh
> +++ b/git-difftool--helper.sh
> @@ -38,15 +38,16 @@ launch_merge_tool () {
>  
>  	# $LOCAL and $REMOTE are temporary files so prompt
>  	# the user with the real $MERGED name before launching $merge_tool.
> +	ans=y
>  	if should_prompt
>  	then
>  		printf "\nViewing: '$MERGED'\n"
>  		if use_ext_cmd
>  		then
> -			printf "Hit return to launch '%s': " \
> +			printf "Launch '%s' [Y/n]: " \
>  				"$GIT_DIFFTOOL_EXTCMD"
>  		else
> -			printf "Hit return to launch '%s': " "$merge_tool"
> +			printf "Launch '%s' [Y/n]: " "$merge_tool"
>  		fi
>  		read ans
>  	fi
> @@ -54,9 +55,9 @@ launch_merge_tool () {
>  	if use_ext_cmd
>  	then
>  		export BASE
> -		eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
> +		test "$ans" != "n" && eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
>  	else
> -		run_merge_tool "$merge_tool"
> +		test "$ans" != "n" && run_merge_tool "$merge_tool"
>  	fi
>  }

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Junio C Hamano @ 2011-10-07 19:59 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Michael J Gruber, git, Jeff King
In-Reply-To: <20111007195023.GA29712@elie.hsd1.il.comcast.net>

Jonathan Nieder <jrnieder@gmail.com> writes:

> I am surprised that you seem to have missed what I meant by "branch
> names are local"....
> This matters, a lot, because there is no easy way to partition a
> namespace of names descriptive for humans without a central authority
> to negotiate conflicts.

I think we are in total agreement.  The branch names are local, but the
users may want to annotate to describe _the history_ they intend to build
on it.

Various ways to convey the description when the end product (i.e. the
history built on it) is shiped were outlined in the series, so that the
shipper can help the receiver understand the history. The information in
the annotation (i.e. the _value_ of branch.$name.description) is something
the shipper wants to share with the receiver, but the mapping between the
local name of the branch the shipper used to build that history (i.e. the
key "$name" in branch.$name.description) is immaterial in the end result.

I do not think there is much more for me to add to this topic, as I think
you covered all the important bases already.

^ permalink raw reply

* Re: [PATCH] fmt-merge-msg: use branch.$name.description
From: Jonathan Nieder @ 2011-10-07 19:50 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <4E8EED39.1060607@drmicha.warpmail.net>

Michael J Gruber wrote:
> Jonathan Nieder venit, vidit, dixit 07.10.2011 12:06:

>> [1] http://thread.gmane.org/gmane.comp.version-control.git/181953/focus=182000
>
> Funny to point me at a thread I participated in, when I'm saying this
> thread should have continued there ;)

Ah, sorry, I wasn't trying to initiate a debate.  I provided the
pointer to save time for others who haven't looked up the thread
you were referring to yet.

I am surprised that you seem to have missed what I meant by "branch
names are local".  I meant that what I call "master" is likely to be
very different from what you call "master".  Yes, we share commits and
fetch them from each other, and I might even _decide_ that what I call
"master" will be similar to what you call "master".  Luckily, in
practice people don't seem to feel constrained to decide so, and there
are many workflows where my "master" is not your "master" and does not
even contain commits that would be acceptable for your "master".

This matters, a lot, because there is no easy way to partition a
namespace of names descriptive for humans without a central authority
to negotiate conflicts.

^ permalink raw reply

* Re: [PATCH] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-07 19:40 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <CAG+J_DxrQCS8zn5KJ8HnpqShVbMw=zCbqDVa=w08EEibw=tsAA@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> The other inconsistencies I'm aware of between "merge --no-commit &&
> commit" vs "merge" on a clean merge are:

Perhaps you would want to add these to a list of todo items when gitwiki
comes back.

^ permalink raw reply

* Re: [PATCH] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-07 19:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vobxsvd69.fsf@alter.siamese.dyndns.org>

On Fri, Oct 7, 2011 at 3:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Think about it. What I suggested does no way make the situation
> worse. Your patch _does_ make it worse by changing the hook behaviour
> between "merge -m 'foo'" vs "merge -m 'foo' -e"

I think it's arguable how -e should behave. With -e opening my editor,
now I really feel like I'm making a commit and would be surprised by
not having the various commit hooks run.

j.

^ permalink raw reply

* Re: [PATCH] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-07 19:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <CAG+J_Dz7-tTdgT=cqoKhK+fAhmESLnp93yHyxOF_NOY5Wx01+w@mail.gmail.com>

On Fri, Oct 7, 2011 at 2:01 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> I actually think a better choice would be to remove commit_tree() from
> merge and always have it run commit externally. I'm not seriously
> suggesting that be done, but it would make git more consistent. But
> I'm not going to send in a patch which makes the situation worse.

The other inconsistencies I'm aware of between "merge --no-commit &&
commit" vs "merge" on a clean merge are:

* reflog
  - merge uses either "Merge made by the '...' strategy." OR "In-index merge"
  - commit uses "commit (merge) <subject>"

* hooks
  - merge calls
    1) "prepare-commit-msg MERGE_MSG merge"
    2) "post-merge [0|1]" where [0|1] indicates a squash or not.
  - commit calls
    1) "pre-commit"
    2) "prepare-commit-msg COMMIT_EDITMSG merge"
    3) "commit-msg COMMIT_EDITMSG"
    4) "post-commit"

* gc
  - merge calls "git gc --auto" after a successful merge unless
--squash was used
  - commit does not call "git gc --auto"

* diffstat: merge shows it, commit does not

j.

^ permalink raw reply

* Re: [PATCH] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-07 19:01 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <CAG+J_Dz7-tTdgT=cqoKhK+fAhmESLnp93yHyxOF_NOY5Wx01+w@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> I actually think a better choice would be to remove commit_tree() from
> merge and always have it run commit externally. I'm not seriously
> suggesting that be done, but it would make git more consistent. But
> I'm not going to send in a patch which makes the situation worse.

Think about it. What I suggested does no way make the situation
worse. Your patch _does_ make it worse by changing the hook behaviour
between "merge -m 'foo'" vs "merge -m 'foo' -e".

^ permalink raw reply

* Re: Scalable reference handling
From: Martin Fick @ 2011-10-07 18:51 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git
In-Reply-To: <4E8F2012.90108@alum.mit.edu>

On Friday, October 07, 2011 09:51:46 am Michael Haggerty 
wrote:
> I can't write more now, but Martin, if you have time to
> benchmark 9944c7faf903a95d4ed9de284ace32debe21cdc1
> against your repository, I would be very interested to
> learn the results.

The fetch no longer seems to suffer from the large 
regression, it is now faster (~7m) than 1.7.7 (which was 
+15m).


As a quick note, if I comment out the 
invalidate_cached_refs() call in write_ref_sha1() on line  
2065 (on top of 9944c7), it is still much faster, only ~2m.  
Perhaps growing the array on the fly with many refs is still 
be too inefficient?


-Martin

-- 
Employee of Qualcomm Innovation Center, Inc. which is a 
member of Code Aurora Forum

^ permalink raw reply

* Re: git remote doesn't show remotes from .git/remotes
From: Junio C Hamano @ 2011-10-07 18:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Kirill Likhodedov, git
In-Reply-To: <20111007150423.GA2076@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Thu, Oct 06, 2011 at 07:33:23PM +0400, Kirill Likhodedov wrote:
>
>> It seems that 'git remote' doesn't display remotes registered not in
>> .git/config but in .git/remotes/.
>> Is it a bug?
>
> It seems to have been lost in 211c896 (Make git-remote a builtin,
> 2008-02-29).

Sad.

> I don't think there is a specific plan. They're kept for backwards
> compatibility. But really, there is no reason to be using them at all at
> this point.

I've been thinking about making a list of deprecations/deletions for Git
2.0. The only two requirements needed to be added to the list are that it
gets list concensus and it is backed by a solid patch (or patch series).

^ permalink raw reply

* Re: How pretty is pretty? git cat-file -p inconsistency
From: Junio C Hamano @ 2011-10-07 18:04 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <4E8EBC00.90909@drmicha.warpmail.net>

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

> That is, "cat file -p" pretty prints dates for tag objects but not for
> commit objects. In fact, "-p" on commit objects does not prettify at all
> compared to the raw content. Is that intentional?

"cat-file -p" is an ill-conceived half-ass afterthought, and I do not
think anybody sane considers it as part of the "plumbing" ultra stable
interface for machine consumption. See a0f15fa (Pretty-print tagger
dates., 2006-03-01).

> I'd suggest
> prettifying dates with "-p" for commit objects also.

Please make it so. It is your choice to do a patch to update this single
thing first, or to discuss the output with "-p" for all the other object
types at the same time to get the list concensus before proceeding.

Thanks.

^ permalink raw reply

* "Use of uninitialized value" running "git svn clone" when having svn tag branches
From: Luiz-Otavio Zorzella @ 2011-10-07 18:01 UTC (permalink / raw)
  To: git

I'm trying to convert a project (hosted in googlecode.com) from svn to
git, using the "git svn clone" command, and I'm getting an "Use of
uninitialized value" error. Here's the truncated output:

$ git svn clone https://test-libraries-for-java.googlecode.com/svn
--no-metadata -A ~/tmp/authors.txt -t tags -b branches -T trunk
test-libraries-for-java
 r1 = c3adafa93a420f19b1bcfb6765fe0eb90aaa751c (refs/remotes/trunk)
       A       .classpath
       A       .project
       A       COPYING
       A       build.properties
       A       build.xml
W: +empty_dir: trunk/src
[...]
r10 = 8d5d7fdebdb7f822388fd3e4f4061abbfd1fb0cf (refs/remotes/trunk)
       M       test/com/google/common/testing/junit3/JUnitAssertsTest.java
r11 = 4c8a77660bf353ed55c9d583b39e263203c685a4 (refs/remotes/trunk)
Found possible branch point:
https://test-libraries-for-java.googlecode.com/svn/trunk =>
https://test-libraries-for-java.googlecode.com/svn/tags/release-1.0,
11
Use of uninitialized value $u in substitution (s///) at
/usr/lib/git-core/git-svn line 1731.
Use of uninitialized value $u in concatenation (.) or string at
/usr/lib/git-core/git-svn line 1731.
refs/remotes/trunk:
'https://test-libraries-for-java.googlecode.com/svn' not found in ''

For completeness, here's the authors.txt file I'm using:

$ cat ~/tmp/authors.txt
zorzella = Luiz-Otavio 'Z' Zorzella <zorzella@gmail.com>
(no author) = Luiz-Otavio 'Z' Zorzella <zorzella@gmail.com>

**************

The same command works fine if I don't use the "-t tags" part, but
then it does not create the tag branches in my converted git.

Thanks in advance,

Z

^ permalink raw reply

* Re: [PATCH] Teach merge the '[-e|--edit]' option
From: Jay Soffian @ 2011-10-07 18:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk48gwvyd.fsf@alter.siamese.dyndns.org>

On Fri, Oct 7, 2011 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> Implement "git merge [-e|--edit]" as "git merge --no-commit && git commit"
>> as a convenience for the user.
>>
>> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
>> ---
>> ...
>> @@ -1447,6 +1457,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>>       }
>>
>>       if (merge_was_ok) {
>> +             if (option_edit) {
>> +                     const char *args[] = {"commit", "-e", NULL};
>> +                     return run_command_v_opt(args, RUN_GIT_CMD);
>> +             }
>>               fprintf(stderr, _("Automatic merge went well; "
>>                       "stopped before committing as requested\n"));
>>               return 0;
>
>
> I wanted to like this approach, thinking this approach might be safer and
> with the least chance of breaking other codepaths, but this feels like an
> ugly hack.
>
> Are we still honoring all the hooks "git merge" honors?  More importantly,
> isn't this make it impossible for future maintainers of this command to
> enhance the command by adding other hooks after the commit is made?

Git is already inconsistent with respect to which hooks are called
when. Shouldn't post-merge be called on a merge commit regardless of
whether you use --no-commit or not? Well, it isn't, it's only called
when merge performs the commit internally. The post-merge hook was
probably a mistake -- git calls the post-commit hook passing the
context as an argument, so probably merge should just call post-commit
"merge". But that ship has sailed.

See also 65969d43d1 (merge: honor prepare-commit-msg hook, 2011-02-14).

> If we wanted to do this properly, we should update builtin/merge.c to call
> launch_editor() before it runs commit_tree(), in a way similar to how
> prepare_to_commit() in builtin/commit.c does so when e.g. "commit -m foo -e"
> is run. An editmsg is prepared (you already have it in MERGE_MSG), the
> editor is allowed to update it, and then the original code before such a
> patch will run using the updated contents of MERGE_MSG. That way, the _only_
> change in behaviour when "-e" is used is to let the user update the message
> used in the commit log, and everything else would run exactly the same way
> as if no "-e" was given, including the invocation of hooks.

I find git very difficult to reason about (and inconsistent in its
behavior) due to piecemeal hoisting of some functionality into
porcelain commands (another example, revert.c building in the
recursive merge strategy but not any others).

I actually think a better choice would be to remove commit_tree() from
merge and always have it run commit externally. I'm not seriously
suggesting that be done, but it would make git more consistent. But
I'm not going to send in a patch which makes the situation worse.

j.

^ permalink raw reply

* Re: [PATCH WIP 0/3] git log --exclude
From: Junio C Hamano @ 2011-10-07 17:57 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, Jonathan Nieder
In-Reply-To: <CACsJy8DJs_cmCZegyPk=tB-bxWp4izrsTn8T=xeV1sU4fS5oTQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> There may be a solution to mix depth limit and negative pathspec. I
> haven't thought carefully about that because I lean towards a simpler
> behaviour that only allows one negation level: a file is in if it
> matches any positive pathspecs and none of negative ones.

I tend to think it probably is acceptable solution to define "struct
pathspec" to have one positive and one negative set, traversing and
declaring a match on what matches something from the positive set only if
it does not match anything in the negative set.

That is similar to how we define revision ranges in the sense that the
range notation to have two ranges (A..B C..D) does not mean union of two
sets (A..B + C..D), but is difference between two unions ((B D)-(A C)).

> This is enough if narrow clones consist of positive pathspec only. I
> really doubt if users would want to make a narrow clone that "contains
> A but not A/B, storage-wise".

And if you define "struct pathspec" to have one positive and one negative
set, you do not have to limit narrow clone only to positive. The repository
specific narrow pathspec will have one such "struct pathspec", but the
user may give us from the command line another "struct pathspec". At
runtime, we would merge them to form into one negative and one positive
set, and apply the same rule: nothing that matches any negative will
appear in the traversal or in the output.

^ permalink raw reply

* unexpected behavior with `git log --skip filename`
From: Andrew McNabb @ 2011-10-07 17:15 UTC (permalink / raw)
  To: git

The "--skip" option to "git log" did not behave as I expected, but I'm
not sure whether this was user error, unclear documentation, or a bug.
Specifically, I ran the following, intending to find the previous
revision of a given file:

git log --skip=1 -n 1 --oneline some-filename

My expectation was that this would behave the same as:

git log -n 2 --oneline some-filename |tail -n 1

Instead, the --skip=1 parameter seemed to be ignored.  After I tried
several different values, it appears that the commits are skipped before
path matching with "some-filename".

Is this the intended behavior?  If so, should the documentation be
clarified by changing "Note that they are applied before commit ordering
and formatting options, such as --reverse" to something like "Note that
they are applied before path matching, commit ordering, and formatting
options, such as --reverse"?

--
Andrew McNabb
http://www.mcnabbs.org/andrew/
PGP Fingerprint: 8A17 B57C 6879 1863 DE55  8012 AB4D 6098 8826 6868

^ permalink raw reply

* Re: [PATCH] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-07 17:30 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <1318001347-11347-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> Implement "git merge [-e|--edit]" as "git merge --no-commit && git commit"
> as a convenience for the user.
>
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> ---
> ...
> @@ -1447,6 +1457,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  	}
>  
>  	if (merge_was_ok) {
> +		if (option_edit) {
> +			const char *args[] = {"commit", "-e", NULL};
> +			return run_command_v_opt(args, RUN_GIT_CMD);
> +		}
>  		fprintf(stderr, _("Automatic merge went well; "
>  			"stopped before committing as requested\n"));
>  		return 0;


I wanted to like this approach, thinking this approach might be safer and
with the least chance of breaking other codepaths, but this feels like an
ugly hack.

Are we still honoring all the hooks "git merge" honors?  More importantly,
isn't this make it impossible for future maintainers of this command to
enhance the command by adding other hooks after the commit is made?

If we wanted to do this properly, we should update builtin/merge.c to call
launch_editor() before it runs commit_tree(), in a way similar to how
prepare_to_commit() in builtin/commit.c does so when e.g. "commit -m foo -e"
is run. An editmsg is prepared (you already have it in MERGE_MSG), the
editor is allowed to update it, and then the original code before such a
patch will run using the updated contents of MERGE_MSG. That way, the _only_
change in behaviour when "-e" is used is to let the user update the message
used in the commit log, and everything else would run exactly the same way
as if no "-e" was given, including the invocation of hooks.

^ permalink raw reply

* Re: [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
From: Carlos Martín Nieto @ 2011-10-07 16:40 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <20111007163319.GC4399@sigill.intra.peff.net>

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

On Fri, 2011-10-07 at 12:33 -0400, Jeff King wrote:
> On Thu, Oct 06, 2011 at 11:21:47PM +0200, Carlos Martín Nieto wrote:
> 
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index b937d71..94b2bd3 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,
> >  		free_refs(ref_map);
> >  		return 1;
> >  	}
> > -	if (prune)
> > +	if (prune) {
> > +		/* If --tags was specified, we need to tell prune_refs
> > +		 * that we're filtering the refs from the remote */
> > +		if (tags == TAGS_SET) {
> > +			const char * tags_refspec = "refs/tags/*:refs/tags/*";
> > +			refs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));
> > +			refs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);
> > +			ref_count++;
> > +		}
> >  		prune_refs(transport, refs, ref_count, ref_map);
> > +	}
> 
> I don't think we can realloc refs here. It's passed into do_fetch. When
> we realloc it, the old pointer value will be invalid. But when we return
> from do_fetch, the caller (fetch_one) will still have that old value,
> and will call free() on it.

Yes, you're right. I guess it's been working by luck and generous amount
of memory.

> 
> Instead, you have to make a whole new list, copy the old values in, add
> your new one, and then free the result.

Will do.

   cmn


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs
From: Jeff King @ 2011-10-07 16:39 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <1318005433.4579.5.camel@centaur.lab.cmartin.tk>

On Fri, Oct 07, 2011 at 06:37:13PM +0200, Carlos Martín Nieto wrote:

> > I assume you mean s/tag/branch/ in the last line?
> 
> Yeah, maybe ref would be better?

Yeah, agreed.

> > Tests?
> 
> Good point.

It sounds like you already have a reproduction recipe for this, and for
the --tags thing in the next commit.

> OK, so take a step back and figure out what we want the rules to be
> before we call get_stale_heads. It does sound like a more elegant
> approach. I was trying to disrupt the callers as little as possible, but
> then again, there's only two. Will change.

Yeah. Sometimes we try hard to make a minimal patch, because it makes it
easier to review. At the same time, I think it's more important to make
the code clean if it needs it. Especially when there aren't many callers
to disrupt.

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs
From: Carlos Martín Nieto @ 2011-10-07 16:37 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <20111007162625.GB4399@sigill.intra.peff.net>

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

On Fri, 2011-10-07 at 12:26 -0400, Jeff King wrote:
> On Thu, Oct 06, 2011 at 11:21:46PM +0200, Carlos Martín Nieto wrote:
> 
> > If the user gave us refspecs on the command line, we should use those
> > when deciding whether to prune a ref instead of relying on the
> > refspecs in the config.
> > 
> > Previously, running
> > 
> >     git fetch --prune origin refs/heads/master:refs/remotes/origin/master
> > 
> > would delete every other tag under the origin namespace because we
> 
> I assume you mean s/tag/branch/ in the last line?

Yeah, maybe ref would be better?

> 
> > ---
> >  builtin/fetch.c  |    6 ++--
> >  builtin/remote.c |    2 +-
> >  remote.c         |   78 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  remote.h         |    3 +-
> >  4 files changed, 77 insertions(+), 12 deletions(-)
> 
> Tests?

Good point.

> 
> >  static int get_stale_heads_cb(const char *refname,
> >  	const unsigned char *sha1, int flags, void *cb_data)
> >  {
> >  	struct stale_heads_info *info = cb_data;
> >  	struct refspec refspec;
> > +	int ret;
> >  	memset(&refspec, 0, sizeof(refspec));
> >  	refspec.dst = (char *)refname;
> > -	if (!remote_find_tracking(info->remote, &refspec)) {
> > -		if (!((flags & REF_ISSYMREF) ||
> > -		    string_list_has_string(info->ref_names, refspec.src))) {
> > -			struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
> > -			hashcpy(ref->new_sha1, sha1);
> > -		}
> > +
> > +	/*
> > +	 * If the user speicified refspecs on the command line, we
> > +	 * should only use those to check. Otherwise, look in the
> > +	 * remote's configuration for the branch.
> > +	 */
> > +	if (info->ref_count)
> > +		ret = find_in_refs(info->refs, info->ref_count, &refspec);
> > +	else
> > +		ret = remote_find_tracking(info->remote, &refspec);
> 
> Minor typo in the comment. But more importantly, this feels like a very
> low-level place to be thinking about things like what the user gave us
> on the command line.
> 
> Shouldn't get_stale_heads not get a remote at all, and just get a set of
> refspecs? Those should be the minimal information it needs to get its
> answer, right?

OK, so take a step back and figure out what we want the rules to be
before we call get_stale_heads. It does sound like a more elegant
approach. I was trying to disrupt the callers as little as possible, but
then again, there's only two. Will change.

   cmn


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH 3/3] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
From: Jeff King @ 2011-10-07 16:33 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <1317936107-1230-4-git-send-email-cmn@elego.de>

On Thu, Oct 06, 2011 at 11:21:47PM +0200, Carlos Martín Nieto wrote:

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index b937d71..94b2bd3 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -699,8 +699,17 @@ static int do_fetch(struct transport *transport,
>  		free_refs(ref_map);
>  		return 1;
>  	}
> -	if (prune)
> +	if (prune) {
> +		/* If --tags was specified, we need to tell prune_refs
> +		 * that we're filtering the refs from the remote */
> +		if (tags == TAGS_SET) {
> +			const char * tags_refspec = "refs/tags/*:refs/tags/*";
> +			refs = xrealloc(refs, (ref_count + 1) * sizeof(struct refspec));
> +			refs[ref_count] = *parse_fetch_refspec(1, &tags_refspec);
> +			ref_count++;
> +		}
>  		prune_refs(transport, refs, ref_count, ref_map);
> +	}

I don't think we can realloc refs here. It's passed into do_fetch. When
we realloc it, the old pointer value will be invalid. But when we return
from do_fetch, the caller (fetch_one) will still have that old value,
and will call free() on it.

Instead, you have to make a whole new list, copy the old values in, add
your new one, and then free the result.

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] fetch: honor the user-provided refspecs when pruning refs
From: Jeff King @ 2011-10-07 16:26 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano, mathstuf
In-Reply-To: <1317936107-1230-3-git-send-email-cmn@elego.de>

On Thu, Oct 06, 2011 at 11:21:46PM +0200, Carlos Martín Nieto wrote:

> If the user gave us refspecs on the command line, we should use those
> when deciding whether to prune a ref instead of relying on the
> refspecs in the config.
> 
> Previously, running
> 
>     git fetch --prune origin refs/heads/master:refs/remotes/origin/master
> 
> would delete every other tag under the origin namespace because we

I assume you mean s/tag/branch/ in the last line?

> ---
>  builtin/fetch.c  |    6 ++--
>  builtin/remote.c |    2 +-
>  remote.c         |   78 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>  remote.h         |    3 +-
>  4 files changed, 77 insertions(+), 12 deletions(-)

Tests?

>  static int get_stale_heads_cb(const char *refname,
>  	const unsigned char *sha1, int flags, void *cb_data)
>  {
>  	struct stale_heads_info *info = cb_data;
>  	struct refspec refspec;
> +	int ret;
>  	memset(&refspec, 0, sizeof(refspec));
>  	refspec.dst = (char *)refname;
> -	if (!remote_find_tracking(info->remote, &refspec)) {
> -		if (!((flags & REF_ISSYMREF) ||
> -		    string_list_has_string(info->ref_names, refspec.src))) {
> -			struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
> -			hashcpy(ref->new_sha1, sha1);
> -		}
> +
> +	/*
> +	 * If the user speicified refspecs on the command line, we
> +	 * should only use those to check. Otherwise, look in the
> +	 * remote's configuration for the branch.
> +	 */
> +	if (info->ref_count)
> +		ret = find_in_refs(info->refs, info->ref_count, &refspec);
> +	else
> +		ret = remote_find_tracking(info->remote, &refspec);

Minor typo in the comment. But more importantly, this feels like a very
low-level place to be thinking about things like what the user gave us
on the command line.

Shouldn't get_stale_heads not get a remote at all, and just get a set of
refspecs? Those should be the minimal information it needs to get its
answer, right?

-Peff

^ permalink raw reply


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