Git development
 help / color / mirror / Atom feed
* Re: Updated tag 'junio-gpg-pub' ?
From: Junio C Hamano @ 2011-09-30 23:18 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Stefan Näwe, Git List
In-Reply-To: <m2zkhlkgv2.fsf@igel.home>

Andreas Schwab <schwab@linux-m68k.org> writes:

> You might want to update the tag message the next time with
> s/git-/git /.

Heh, good idea ;-).

^ permalink raw reply

* Re: Dealing with rewritten upstream
From: Michael Witten @ 2011-09-30 23:04 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <CAG+J_DwR4vE6iYt475EM7-VDNi4hG3jhdmXWSbJ04Y9fyHeuLw@mail.gmail.com>

On Fri, Sep 30, 2011 at 22:09, Jay Soffian <jaysoffian@gmail.com> wrote:
> I have a repo w/over two years of history whose upstream repo is a
> git-svn mirror.
>
> The upstream folks recently announced they need to retire the existing
> repo and replace it with a new repo. The new repo is identical to the
> old repo tree wise (commit for commit), but some of the commits in the
> old repo had incorrect authorship which is corrected in the new repo,
> so the new repo has different commit IDs than the old.
>
> (i.e., it's as if they've run filter-branch --env-filter on the old repo.)
>
> My repo has many merge points with the old history.
>
> Pictorially:
>
> ---A---B---C---D---E... new-upstream/master
>
> ---a---b---c---d---e... old-upstream/master
>    \       \       \
>     1---2---3---4---5  master
>
> The obvious way do deal with this situation is:
>
> $ git merge -s ours -m "Splice in new-upstream/master" E
>
> Are there any other/better options I'm missing?
>
> (Eventually upstream plans to migrate entirely to git, so I can't just
> run git-svn myself.)

Surely, you'd rather have your master rewritten such that the relevant
commits of new-upstream/master are used IN PLACE of the corresponding
old-upstream/master. Have you considered ways to achieve that?

^ permalink raw reply

* Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain
From: Jay Soffian @ 2011-09-30 22:42 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, John Szakmeister
In-Reply-To: <20110930221111.GB9384@sigill.intra.peff.net>

On Fri, Sep 30, 2011 at 6:11 PM, Jeff King <peff@peff.net> wrote:
> Because security prompts sometimes are out of band. In particular,
> you're already interacting with the user outside of the terminal;
> opening the keychain will get you an "allow git to open the keychain"
> dialog.

Usually it won't. In the default case, the keychain is unlocked and no
permission is needed to add an entry, nor to retrieve that entry by
the application which added it. The prompt will only occur if the
credential helper is not on the entry's ACL, or if the keychain is
locked.

> Another example: if you're running gpg-agent, and you run "git tag -s",
> you'll be prompted for your key passphrase in an out-of-band dialog.
>
> Maybe it doesn't make sense for the actual username/password, though.

Personally, it made sense to me do it at the CLI (obviously). But the
source code in in contrib now. :-)

> I meant running sshd on OS X, and then ssh-ing in from some other box.
> Your credential helper doesn't seem to work at all (I guess because it
> has no access to the keychains). I don't think it's a big deal, though.
> It's the minority case, and if somebody wants to figure out how to make
> it work later, they can.

As far as I know there's no way to make this work. When you login over
ssh, it's a different security context, and there is no access to the
password field of keychain entries.

>> I found it ugly that git's native getpass doesn't echo the username
>> back, and it seems hackish to me for the credential helper to turn
>> back around and invoke it in any case. :-(
>
> Yes, but I think that's a bug that should be fixed in git. :)

Yes it should. :-)

> As far as being hack-ish, the original design was that you could chain
> these things if you wanted. But in practice, I don't know how useful
> that is. In fact, after all of this discussion, I'm wondering how useful
> it is that the helpers are allowed to prompt themselves at all.
>
> The KDE one does it. But would people be happier if git simply did:
>
>  1. ask the helper for a credential. if we get it, done
>
>  2. prompt the user
>
>  3. if the credential is valid, ask the helper to store
>
> That's way less flexible. But it also makes the helpers really simple.

I think that actually makes more sense. There's already an existing
mechanism to customized (2) via GIT_ASKPASS, right? So it overlaps for
the credential helper to do that doesn't it?

> Because the helper might have an alternate way of asking for the
> password (e.g., the KDE helper has its own dialog). Maybe it will never
> be useful. I just wanted to future-proof helpers by giving them sane
> behavior for this case, on the off chance that future versions of git do
> actually do this.
>>
> Don't get me wrong; I think you did the only sane thing. It was more
> "Hmm, I was hoping that the right way to use the various security APIs
> wouldn't need to...". And clearly my hope was wrong. :)
>
> But this is exactly the sort of feedback I was hoping for; the interface
> to the helpers could be better, and we are catching it now.

Okay, the more I think about this, the more I think the existing
design is both too much (asking the credential helper to do anything
other than store/retrieve passwords) and too little (not breaking out
the fields distinctly).

> That's not an unreasonable attitude. I mostly let the browser store
> passwords, but sometimes override it for specific sites. But in this
> case, I think it would be more per-repo. And you can turn off the helper
> for a particular repo (actually, I'm not sure you can, but you probably
> should be able to).

If the credential helper becomes no more than a store/retrieve, it's
git that would do the prompting "Store credentials via
git-osxkeychain?" after logging in successfully with the credentials.

>> It is for security reasons. 99% of users will probably just click

> Anyway, that's neither here nor there. It would be nice if we could set
> the text, but if we can't, then we'll have to live with it.

The C version of the helper will use the name of the helper binary.

j.

^ permalink raw reply

* What's cooking in git.git (Sep 2011, #09; Fri, 30)
From: Junio C Hamano @ 2011-09-30 22:39 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'.

Here are the repositories that have my integration branches:

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

	url = git://repo.or.cz/alt-git.git
	url = https://code.google.com/p/git-core/

With only maint, master, html and man:

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

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

	url = https://github.com/gitster/git

Until kernel.org comes back to life, it might be a good idea to
tentatively have the following in your $HOME/.gitconfig:

    [url "https://github.com/git/git"]
	insteadOf = git://git.kernel.org/pub/scm/git/git.git

I just tagged v1.7.7 final, and expect to rewind and rebuild the 'next'
branch on top of it sometime next week.

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

* dm/tree-walk (2011-09-28) 2 commits
 - tree-walk: micro-optimization in tree_entry_interesting
 - tree-walk: drop unused parameter from match_dir_prefix

* cs/perl-config-path-send-email (2011-09-30) 2 commits
 - use new Git::config_path() for aliasesfile
 - Add Git::config_path()

* jc/checkout-from-tree-keep-local-changes (2011-09-30) 1 commit
 - checkout $tree $path: do not clobber local changes in $path not in $tree

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

* 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
  (merged to 'next' on 2011-08-24 at 398e764)
 + 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.

* jc/signed-push (2011-09-12) 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 uses jc/run-receive-hook-cleanup; is tangled with jc/signed-push-3.)

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

* jc/signed-push-3 (2011-09-12) 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 uses jc/run-receive-hook-cleanup; is tangled with jc/signed-push.)

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

I expect that both of these topics will be discarded.

* 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

Will be dropped.

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

* 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

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

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

* 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. Perhaps drop?

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

* jc/apply-blank-at-eof-fix (2011-09-26) 1 commit
 - apply --whitespace=error: correctly report new blank lines at end

* nd/sparse-doc (2011-09-26) 1 commit
 - git-read-tree.txt: update sparse checkout examples

* jp/get-ref-dir-unsorted (2011-09-30) 2 commits
 - refs: Use binary search to lookup refs faster
 - Don't sort ref_list too early

* jc/grep-untracked-exclude (2011-09-28) 2 commits
 - Merge branch 'jc/maint-grep-untracked-exclude' into jc/grep-untracked-exclude
 - 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-09-28) 3 commits
 - grep: rename --exclude to --exclude-standard
 - grep: --untracked and --exclude tests
 - grep: teach --untracked and --exclude options
 (this branch is used by jc/grep-untracked-exclude; uses bw/grep-no-index-no-exclude.)

* jc/parse-options-boolean (2011-09-28) 5 commits
 - 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

* mh/maint-notes-merge-pathbuf-fix (2011-09-27) 1 commit
 - notes_merge_commit(): do not pass temporary buffer to other function

* ph/push-to-delete-nothing (2011-09-30) 1 commit
 - receive-pack: don't pass non-existent refs to post-{receive,update} hooks

* ps/gitweb-js-with-lineno (2011-09-27) 1 commit
 - gitweb: Fix links to lines in blobs when javascript-actions are enabled

* zj/send-email-authen-sasl (2011-09-29) 1 commit
 - send-email: auth plain/login fix

* jc/maint-diffstat-numstat-context (2011-09-22) 1 commit
  (merged to 'next' on 2011-09-26 at 12539ab)
 + 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.

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

* rs/diff-cleanup-records-fix (2011-09-26) 1 commit
  (merged to 'next' on 2011-09-27 at 3bd75d8)
 + Revert removal of multi-match discard heuristic in 27af01

* di/fast-import-empty-tag-note-fix (2011-09-22) 2 commits
 - fast-import: don't allow to note on empty branch
 - fast-import: don't allow to tag empty branch

Looked reasonable.

* js/check-attr-cached (2011-09-22) 2 commits
  (merged to 'next' on 2011-09-27 at 74d7b66)
 + t0003: remove extra whitespaces
 + Teach '--cached' option to check-attr

* bw/grep-no-index-no-exclude (2011-09-15) 2 commits
  (merged to 'next' on 2011-09-26 at 776f13b)
 + 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.)

* jc/want-commit (2011-09-15) 1 commit
  (merged to 'next' on 2011-09-26 at 5841512)
 + Allow git merge ":/<pattern>"

* jc/ls-remote-short-help (2011-09-16) 1 commit
  (merged to 'next' on 2011-09-26 at e24a27a)
 + ls-remote: a lone "-h" is asking for help

* jc/maint-bundle-too-quiet (2011-09-19) 1 commit
  (merged to 'next' on 2011-09-26 at ba140d4)
 + Teach progress eye-candy to fetch_refs_from_bundle()

* jk/filter-branch-require-clean-work-tree (2011-09-15) 1 commit
  (merged to 'next' on 2011-09-26 at 206a74a)
 + filter-branch: use require_clean_work_tree

* jn/gitweb-highlite-sanitise (2011-09-16) 1 commit
  (merged to 'next' on 2011-09-26 at c79390a)
 + gitweb: Strip non-printable characters from syntax highlighter output

* mh/check-ref-format-3 (2011-09-16) 22 commits
 - 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

* cn/eradicate-working-copy (2011-09-21) 1 commit
  (merged to 'next' on 2011-09-26 at 2683d36)
 + Remove 'working copy' from the documentation and C code

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

* mg/maint-doc-sparse-checkout (2011-09-21) 3 commits
  (merged to 'next' on 2011-09-21 at f316dec)
 + git-read-tree.txt: correct sparse-checkout and skip-worktree description
 + git-read-tree.txt: language and typography fixes
 + unpack-trees: print "Aborting" to stderr

* ms/patch-id-with-overlong-line (2011-09-22) 1 commit
  (merged to 'next' on 2011-09-26 at a33d0b2)
 + patch-id.c: use strbuf instead of a fixed buffer

* sn/doc-update-index-assume-unchanged (2011-09-21) 1 commit
  (merged to 'next' on 2011-09-21 at 325e796)
 + Documentation/git-update-index: refer to 'ls-files'

* jc/request-pull-show-head-4 (2011-09-21) 7 commits
 - 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
 (this branch uses bk/ancestry-path.)

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

* nd/maint-autofix-tag-in-head (2011-09-18) 4 commits
  (merged to 'next' on 2011-09-27 at dc8e2e3)
 + 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

* jk/maint-fetch-submodule-check-fix (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-12 at 3c73b8c)
 + fetch: avoid quadratic loop checking for updated submodules
 (this branch is used by jk/argv-array.)

This probably can wait, as long as the other half of the regression fix
is in the upcoming release.

* bc/attr-ignore-case (2011-09-14) 5 commits
  (merged to 'next' on 2011-09-26 at 1e0814c)
 + attr: read core.attributesfile from git_default_core_config
 + attr.c: respect core.ignorecase when matching attribute patterns
 + builtin/mv.c: plug miniscule memory leak
 + cleanup: use internal memory allocation wrapper functions everywhere
 + attr.c: avoid inappropriate access to strbuf "buf" member

* jc/maint-fsck-fwrite-size-check (2011-09-11) 1 commit
  (merged to 'next' on 2011-09-16 at 2258f11)
 + fsck: do not abort upon finding an empty blob

* jk/argv-array (2011-09-14) 7 commits
  (merged to 'next' on 2011-09-16 at 90feab4)
 + run_hook: use argv_array API
 + checkout: use argv_array API
 + bisect: use argv_array API
 + quote: provide sq_dequote_to_argv_array
 + refactor argv_array into generic code
 + quote.h: fix bogus comment
 + add sha1_array API docs
 (this branch uses jk/maint-fetch-submodule-check-fix.)

* js/cred-macos-x-keychain-2 (2011-09-14) 1 commit
  (merged to 'next' on 2011-09-26 at 4f289a4)
 + contrib: add a pair of credential helpers for Mac OS X's keychain
 (this branch uses jk/http-auth-keyring.)

Welcome addition to build our confidence in the jk/http-auth-keyring topic.

* rj/maint-t9159-svn-rev-notation (2011-09-21) 1 commit
  (merged to 'next' on 2011-09-26 at 525a567)
 + t9159-*.sh: skip for mergeinfo test for svn <= 1.4

* tr/doc-note-rewrite (2011-09-13) 1 commit
  (merged to 'next' on 2011-09-16 at 5fe813a)
 + Documentation: basic configuration of notes.rewriteRef

Updated to a safer wording.

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

Will be re-rolled after 1.7.7 final.

* hl/iso8601-more-zone-formats (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-12 at 270f5c7)
 + date.c: Support iso8601 timezone formats

* jc/run-receive-hook-cleanup (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-12 at 68dd431)
 + refactor run_receive_hook()
 (this branch is used by jc/signed-push and jc/signed-push-3.)

Just to make it easier to run a hook that reads from its standard input.

* jk/for-each-ref (2011-09-08) 5 commits
  (merged to 'next' on 2011-09-14 at 36ed515)
 + for-each-ref: add split message parts to %(contents:*).
 + for-each-ref: handle multiline subjects like --pretty
 + for-each-ref: refactor subject and body placeholder parsing
 + t6300: add more body-parsing tests
 + t7004: factor out gpg setup

* wh/normalize-alt-odb-path (2011-09-07) 1 commit
  (merged to 'next' on 2011-09-14 at 96f722b)
 + sha1_file: normalize alt_odb path before comparing and storing

* fk/use-kwset-pickaxe-grep-f (2011-09-11) 2 commits
  (merged to 'next' on 2011-09-14 at 436d858)
 + obstack.c: Fix some sparse warnings
 + sparse: Fix an "Using plain integer as NULL pointer" warning

In general we would prefer to see these fixed at the upstream first, but
we have essentially forked from them at their last GPLv2 versions...

* jc/make-static (2011-09-14) 4 commits
  (merged to 'next' on 2011-09-14 at c5943ff)
 + exec_cmd.c: prepare_git_cmd() is sometimes used
 + environment.c: have_git_dir() has users on Cygwin
  (merged to 'next' on 2011-09-11 at 2acb0af)
 + vcs-svn: remove unused functions and make some static
 + make-static: master

With a few fix-ups; probably needs to be ejected after 1.7.7 happens.

* rj/quietly-create-dep-dir (2011-09-11) 1 commit
  (merged to 'next' on 2011-09-12 at 93d1c6b)
 + Makefile: Make dependency directory creation less noisy

* mz/remote-rename (2011-09-11) 4 commits
  (merged to 'next' on 2011-09-26 at 5e64f68)
 + 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'

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

* cb/send-email-help (2011-09-12) 1 commit
  (merged to 'next' on 2011-09-14 at ae71999)
 + send-email: add option -h

A separate set of patches to remove the hidden fully-spelled "help" from
other commands would be nice to have as companion patches as well.

* jc/fetch-pack-fsck-objects (2011-09-04) 3 commits
  (merged to 'next' on 2011-09-12 at a031347)
 + test: fetch/receive with fsckobjects
 + transfer.fsckobjects: unify fetch/receive.fsckobjects
 + fetch.fsckobjects: verify downloaded objects

We had an option to verify the sent objects before accepting a push but
lacked the corresponding option when fetching. In the light of the recent
k.org incident, a change like this would be a good addition.

* jc/fetch-verify (2011-09-01) 3 commits
  (merged to 'next' on 2011-09-12 at 3f491ab)
 + 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; is tangled with jc/receive-verify.)

During a fetch, we verify that the pack stream is self consistent,
but did not verify that the refs that are updated are consistent with
objects contained in the packstream, and this adds such a check.

* jc/receive-verify (2011-09-09) 6 commits
  (merged to 'next' on 2011-09-12 at 856de78)
 + receive-pack: check connectivity before concluding "git push"
 + check_everything_connected(): libify
 + check_everything_connected(): refactor to use an iterator
 + 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; is tangled with jc/fetch-verify.)

While accepting a push, we verify that the pack stream is self consistent,
but did not verify that the refs the push updates are consistent with
objects contained in the packstream, and this adds such a check.

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

* bk/ancestry-path (2011-09-15) 4 commits
  (merged to 'next' on 2011-09-15 at aa64d04)
 + t6019: avoid refname collision on case-insensitive systems
  (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
 (this branch is used by jc/request-pull-show-head-4.)

* mg/branch-list (2011-09-13) 7 commits
  (merged to 'next' on 2011-09-14 at 6610a2e)
 + t3200: clean up checks for file existence
  (merged to 'next' on 2011-09-11 at 20a9cdb)
 + branch: -v does not automatically imply --list
  (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

* 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

* mm/mediawiki-as-a-remote (2011-09-28) 6 commits
  (merged to 'next' on 2011-09-28 at a1c9ae5)
 + git-remote-mediawiki: allow a domain to be set for authentication
  (merged to 'next' on 2011-09-27 at 7ce8254)
 + git-remote-mediawiki: obey advice.pushNonFastForward
 + git-remote-mediawiki: set 'basetimestamp' to let the wiki handle conflicts
 + git-remote-mediawiki: trivial fixes
  (merged to 'next' on 2011-09-12 at 163c6a5)
 + git-remote-mediawiki: allow push to set MediaWiki metadata
 + Add a remote helper to interact with mediawiki (fetch & push)

Fun.

* 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

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

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

* 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

* 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

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

* fg/submodule-auto-push (2011-09-11) 2 commits
  (merged to 'next' on 2011-09-11 at 3fc86f7)
 + submodule.c: make two functions static
  (merged to 'next' on 2011-08-24 at 398e764)
 + 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.

* 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 and jc/receive-verify.)

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

* mh/iterate-refs (2011-09-11) 7 commits
  (merged to 'next' on 2011-09-27 at c289699)
 + 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()

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.

* 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

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

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

* jk/http-auth-keyring (2011-09-28) 22 commits
  (merged to 'next' on 2011-09-28 at 65ce6c2)
 + credential-cache: don't cache items without context
  (merged to 'next' on 2011-09-16 at b4195eb)
 + check_expirations: don't copy over same element
 + t0300: add missing EOF terminator for <<
  (merged to 'next' on 2011-09-14 at 589c7c9)
 + 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
  (merged to 'next' on 2011-09-11 at 491ce6a)
 + credentials: make credential_fill_gently() static
  (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
 (this branch is tangled with js/cred-macos-x-keychain-2.)

* rr/revert-cherry-pick-continue (2011-09-11) 19 commits
  (merged to 'next' on 2011-09-11 at 7d78054)
 + builtin/revert.c: make commit_list_append() static
  (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

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

* js/cred-macos-x-keychain (2011-09-11) 15 commits
  (merged to 'next' on 2011-09-12 at 8d17f94)
 + contrib: add a credential helper for Mac OS X's keychain
  (merged to 'next' on 2011-09-11 at 491ce6a)
 + credentials: make credential_fill_gently() static
  (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
 (this branch is tangled with jk/http-auth-keyring and js/cred-macos-x-keychain-2.)

Reverted out of 'next'.

* jc/reflog-walk-use-only-nsha1 (2011-09-13) 4 commits
 . (baloon) teach reflog-walk to look at only new-sha1 field
 + environment.c: have_git_dir() has users on Cygwin
  (merged to 'next' on 2011-09-11 at 2acb0af)
 + vcs-svn: remove unused functions and make some static
 + make-static: master
 (this branch is tangled with jc/make-static.)

* hw/maint-abspath-cwd-limit (2011-09-21) 3 commits
  (merged to 'next' on 2011-09-21 at 210cf9a)
 + Revert 622fea4 (abspath.c: increase array size of cwd variable)
  (merged to 'next' on 2011-09-19 at 7d5e921)
 + abspath.c: increase array size of cwd variable to PATH_MAX
 + path.c: increase array size of cwd variable to PATH_MAX

Reverted out of 'next'.

* jc/request-pull-show-head (2011-09-13) 2 commits
  (merged to 'next' on 2011-09-13 at c82fb3a)
 + Revert "State what commit to expect in request-pull"
  (merged to 'next' on 2011-09-12 at c1c7b73)
 + State what commit to expect in request-pull

Reverted out of 'next'.

^ permalink raw reply

* [ANNOUNCE] Git 1.7.7
From: Junio C Hamano @ 2011-09-30 22:33 UTC (permalink / raw)
  To: git; +Cc: Linux Kernel

The latest feature release Git 1.7.7 is available.

The release tarballs are found at:

    http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

bbf85bd767ca6b7e9caa1489bb4ba7ec64e0ab35  git-1.7.7.tar.gz
33183db94fd25e001bd8a9fd6696b992f61e28d8  git-htmldocs-1.7.7.tar.gz
75d3cceb46f7a46eeb825033dff76af5eb5ea3d9  git-manpages-1.7.7.tar.gz

Also the following public repositories all have a copy of the v1.7.7
tag and the master branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git


The release tag and the tarballs can be verified with my GPG key if
anybody is so inclined. To get my public key:

  $ git fetch git://repo.or.cz/alt-git.git refs/tags/junio-gpg-pub
  $ git rev-parse FETCH_HEAD
  680865b90b18efbc9402ea979adf0302c6dfe72e
  $ git cat-file blob FETCH_HEAD | gpg --import
  
and then make sure that you got my key by checking the output from
"gpg --fingerprint", which should contain these lines:

  pub   1024D/F3119B9A 2004-01-28
        Key fingerprint = 3565 2A26 2040 E066 C9A7  4A7D C0C6 D9A4 F311 9B9A
  uid                  Junio C Hamano <gitster@pobox.com>


Git v1.7.7 Release Notes
========================

Updates since v1.7.6
--------------------

 * The scripting part of the codebase is getting prepared for i18n/l10n.

 * Interix, Cygwin and Minix ports got updated.

 * Various updates to git-p4 (in contrib/), fast-import, and git-svn.

 * Gitweb learned to read from /etc/gitweb-common.conf when it exists,
   before reading from gitweb_config.perl or from /etc/gitweb.conf
   (this last one is read only when per-repository gitweb_config.perl
   does not exist).

 * Various codepaths that invoked zlib deflate/inflate assumed that these
   functions can compress or uncompress more than 4GB data in one call on
   platforms with 64-bit long, which has been corrected.

 * Git now recognizes loose objects written by other implementations that
   use a non-standard window size for zlib deflation (e.g. Agit running on
   Android with 4kb window). We used to reject anything that was not
   deflated with 32kb window.

 * Interaction between the use of pager and coloring of the output has
   been improved, especially when a command that is not built-in was
   involved.

 * "git am" learned to pass the "--exclude=<path>" option through to underlying
   "git apply".

 * You can now feed many empty lines before feeding an mbox file to
   "git am".

 * "git archive" can be told to pass the output to gzip compression and
   produce "archive.tar.gz".

 * "git bisect" can be used in a bare repository (provided that the test
   you perform per each iteration does not need a working tree, of
   course).

 * The length of abbreviated object names in "git branch -v" output
   now honors the core.abbrev configuration variable.

 * "git check-attr" can take relative paths from the command line.

 * "git check-attr" learned an "--all" option to list the attributes for a
   given path.

 * "git checkout" (both the code to update the files upon checking out a
   different branch and the code to checkout a specific set of files) learned
   to stream the data from object store when possible, without having to
   read the entire contents of a file into memory first. An earlier round
   of this code that is not in any released version had a large leak but
   now it has been plugged.

 * "git clone" can now take a "--config key=value" option to set the
   repository configuration options that affect the initial checkout.

 * "git commit <paths>..." now lets you feed relative pathspecs that
   refer to outside your current subdirectory.

 * "git diff --stat" learned a --stat-count option to limit the output of
   a diffstat report.

 * "git diff" learned a "--histogram" option to use a different diff
   generation machinery stolen from jgit, which might give better
   performance.

 * "git diff" had a weird worst case behaviour that can be triggered
   when comparing files with potentially many places that could match.

 * "git fetch", "git push" and friends no longer show connection
   errors for addresses that couldn't be connected to when at least one
   address succeeds (this is arguably a regression but a deliberate
   one).

 * "git grep" learned "--break" and "--heading" options, to let users mimic
   the output format of "ack".

 * "git grep" learned a "-W" option that shows wider context using the same
   logic used by "git diff" to determine the hunk header.

 * Invoking the low-level "git http-fetch" without "-a" option (which
   git itself never did---normal users should not have to worry about
   this) is now deprecated.

 * The "--decorate" option to "git log" and its family learned to
   highlight grafted and replaced commits.

 * "git rebase master topci" no longer spews usage hints after giving
   the "fatal: no such branch: topci" error message.

 * The recursive merge strategy implementation got a fairly large
   fix for many corner cases that may rarely happen in real world
   projects (it has been verified that none of the 16000+ merges in
   the Linux kernel history back to v2.6.12 is affected with the
   corner case bugs this update fixes).

 * "git stash" learned an "--include-untracked option".

 * "git submodule update" used to stop at the first error updating a
   submodule; it now goes on to update other submodules that can be
   updated, and reports the ones with errors at the end.

 * "git push" can be told with the "--recurse-submodules=check" option to
   refuse pushing of the supermodule, if any of its submodules'
   commits hasn't been pushed out to their remotes.

 * "git upload-pack" and "git receive-pack" learned to pretend that only a
   subset of the refs exist in a repository. This may help a site to
   put many tiny repositories into one repository (this would not be
   useful for larger repositories as repacking would be problematic).

 * "git verify-pack" has been rewritten to use the "index-pack" machinery
   that is more efficient in reading objects in packfiles.

 * test scripts for gitweb tried to run even when CGI-related perl modules
   are not installed; they now exit early when the latter are unavailable.

Also contains various documentation updates and minor miscellaneous
changes.


Fixes since v1.7.6
------------------

Unless otherwise noted, all fixes in the 1.7.6.X maintenance track are
included in this release.

 * "git branch -m" and "git checkout -b" incorrectly allowed the tip
   of the branch that is currently checked out updated.

^ permalink raw reply

* Re: [PATCH 2/2] use new Git::config_path() for aliasesfile
From: Junio C Hamano @ 2011-09-30 22:18 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Cord Seele, Matthieu Moy, git, Eric Wong, Cord Seele
In-Reply-To: <201110010000.13328.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> On Fri, 30 Sep 2011, Junio C Hamano wrote:
>
>> I think the addition of "config --path" support is a good idea, but the
>> resulting code suffers from too many cut&paste cruft across the config*
>> family of methods.
>> 
>> How about doing a bit of refactoring, perhaps something like this, on top
>> as a separate patch?
>
> This is a good idea, in my opinion.

Thanks.

>> I tried to be careful to still forcing the "one value only" for config_bool
>> and config_int, but extra sets of eyeballs would be needed.
>
> We do have tests for that, have we?

Perhaps, but I consider Perl "other peoples' problem" ;-) so...

> BTW. why do you use hashref?  Do you plan for the future to pass more
> options that 'kind'?

I don't, but other people might, and I didn't want to place undue burden
on them.

^ permalink raw reply

* Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain
From: Jeff King @ 2011-09-30 22:13 UTC (permalink / raw)
  To: Jay Soffian; +Cc: John Szakmeister, git, Junio C Hamano
In-Reply-To: <CAG+J_DyhcA7RmHwgGJBw4r9JRij0_ONp3ZMD6oMTJ_f4dvYW8w@mail.gmail.com>

On Fri, Sep 30, 2011 at 03:33:37PM -0400, Jay Soffian wrote:

> Sorry, missed this part in my previous reply. I don't understand - how
> do you ever send a username to the credential helper if you don't get
> it from the config? But in any case, if you have a username (via
> config or some other way), yes, I think it should be given to the
> credential helper.

For example:

  git fetch https://user@host/repo.git
  git fetch https://user2@host/repo.git

-Peff

^ permalink raw reply

* Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain
From: Jeff King @ 2011-09-30 22:11 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Junio C Hamano, John Szakmeister
In-Reply-To: <CAG+J_DwntGc+j3duCVqsnoJGV18FqnwXJ99C1XqKope_zbGHAA@mail.gmail.com>

On Fri, Sep 30, 2011 at 03:16:58PM -0400, Jay Soffian wrote:

> This makes no sense to me at all. Ignore OS X for the moment. You use
> git on the command-line. Why would there be any expectation of it
> interacting with the user via anything other than the terminal.

Because security prompts sometimes are out of band. In particular,
you're already interacting with the user outside of the terminal;
opening the keychain will get you an "allow git to open the keychain"
dialog.

Another example: if you're running gpg-agent, and you run "git tag -s",
you'll be prompted for your key passphrase in an out-of-band dialog.

Maybe it doesn't make sense for the actual username/password, though.

> I don't understand where you're running ssh from/to in this scenario,
> but OS X has a notion of security contexts (this is a bit of a
> tangent):
> 
> http://developer.apple.com/library/mac/#technotes/tn2083/_index.html

I meant running sshd on OS X, and then ssh-ing in from some other box.
Your credential helper doesn't seem to work at all (I guess because it
has no access to the keychains). I don't think it's a big deal, though.
It's the minority case, and if somebody wants to figure out how to make
it work later, they can.

> I found it ugly that git's native getpass doesn't echo the username
> back, and it seems hackish to me for the credential helper to turn
> back around and invoke it in any case. :-(

Yes, but I think that's a bug that should be fixed in git. :)

As far as being hack-ish, the original design was that you could chain
these things if you wanted. But in practice, I don't know how useful
that is. In fact, after all of this discussion, I'm wondering how useful
it is that the helpers are allowed to prompt themselves at all.

The KDE one does it. But would people be happier if git simply did:

  1. ask the helper for a credential. if we get it, done

  2. prompt the user

  3. if the credential is valid, ask the helper to store

That's way less flexible. But it also makes the helpers really simple.

> > My test harness checks that this case just asks for the password without
> > bothering to do any lookup or storage. It probably doesn't really matter
> > in practice; I think git should always be providing _some_ context.
> 
> Okay, that wasn't clear from whatever documentation I read on how
> credential helpers should behave. But why invoke the credential helper
> just to ask for a password?

Because the helper might have an alternate way of asking for the
password (e.g., the KDE helper has its own dialog). Maybe it will never
be useful. I just wanted to future-proof helpers by giving them sane
behavior for this case, on the off chance that future versions of git do
actually do this.

> > Hrm. I was really hoping people wouldn't need to pick apart the "unique"
> > token, and it could remain an opaque blob. If helpers are going to do
> > this sort of parsing, then I'd just as soon have git break it down for
> > them, and do something like:
> >
> >  git credential-osxkeychain \
> >    --protocol=https \
> >    --host=github.com \
> >    --path=peff/git.git
> >    --username=peff
> >
> > to just hand over as much information as possible, and let the helper
> > throw it all together if it wants to.
> 
> Keychain entries have distinct fields. I broke apart the token and
> stored it the way other applications mostly do on OS X.

Don't get me wrong; I think you did the only sane thing. It was more
"Hmm, I was hoping that the right way to use the various security APIs
wouldn't need to...". And clearly my hope was wrong. :)

But this is exactly the sort of feedback I was hoping for; the interface
to the helpers could be better, and we are catching it now.

> > My series will also produce "cert:/path/to/certificate" when unlocking a
> > certificate. The other candidates for conversion are smtp-auth (for
> > send-email) and imap (for imap-send).  I guess for certs, you'd want to
> > use the "generic" keychain type.
> 
> Yep, I was punting on certificate for v1.

Not unreasonable. Again, my real concern is not perfect helpers, but
having helpers that exercise the helper interface enough so that we know
whether that interface is sufficient.

> Each keychain entry has an ACL of applications that are allowed to
> access it. When an application asks for an entry and the application
> isn't on that entry's ACL, OS X (not the application) presents the
> user the dialog you refer to. The application has no control over that
> dialog.
> 
> Now, I could have the credential helper ask the user "store this
> password?" after prompting for it, but why even use a credential
> helper if you don't want it to store your credentials?

That's not an unreasonable attitude. I mostly let the browser store
passwords, but sometimes override it for specific sites. But in this
case, I think it would be more per-repo. And you can turn off the helper
for a particular repo (actually, I'm not sure you can, but you probably
should be able to).

> It is for security reasons. 99% of users will probably just click
> "Okay" no matter what. For the 1% that bother to pay attention to the
> dialog, it provides the full path to the binary. I'd be suspicious if
> /tmp/iTunes wanted a password.

Not for me (on 10.7). Doing:

  cp git-credential-osxkeychain /tmp/iTunes
  /tmp/iTunes --unique=https:foo.tld

gives me the dialog "iTunes wants to access the 'login' keychain" with a
password prompt.

Anyway, that's neither here nor there. It would be nice if we could set
the text, but if we can't, then we'll have to live with it.

-Peff

^ permalink raw reply

* Dealing with rewritten upstream
From: Jay Soffian @ 2011-09-30 22:09 UTC (permalink / raw)
  To: git

I have a repo w/over two years of history whose upstream repo is a
git-svn mirror.

The upstream folks recently announced they need to retire the existing
repo and replace it with a new repo. The new repo is identical to the
old repo tree wise (commit for commit), but some of the commits in the
old repo had incorrect authorship which is corrected in the new repo,
so the new repo has different commit IDs than the old.

(i.e., it's as if they've run filter-branch --env-filter on the old repo.)

My repo has many merge points with the old history.

Pictorially:

---A---B---C---D---E... new-upstream/master

---a---b---c---d---e... old-upstream/master
    \       \       \
     1---2---3---4---5  master

The obvious way do deal with this situation is:

$ git merge -s ours -m "Splice in new-upstream/master" E

Are there any other/better options I'm missing?

(Eventually upstream plans to migrate entirely to git, so I can't just
run git-svn myself.)

j.

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-30 22:06 UTC (permalink / raw)
  To: git
  Cc: Christian Couder, Thomas Rast, René Scharfe, Julian Phillips,
	Michael Haggerty
In-Reply-To: <201109301502.30617.mfick@codeaurora.org>

On Friday, September 30, 2011 03:02:30 pm Martin Fick wrote:
> On Friday, September 30, 2011 10:41:13 am Martin Fick 
wrote:
> Since a full sync is now done to about 5mins, I broke
> down the output a bit.  It appears that the longest part
> (2:45m) is now the time spent scrolling though each
> change still. Each one of these takes about 2ms:
>  * [new branch]      refs/changes/99/71199/1 ->
> refs/changes/99/71199/1
> 
> Seems fast, but at about 80K... So, are there any obvious
> N loops over the refs happening inside each of of the
> [new branch] iterations?

OK, I narrowed it down I believe.  If I comment out the 
invalidate_cached_refs() line in write_ref_sha1(), it speeds 
through this section.  

I guess this makes sense, we invalidate the cache and have 
to rebuild it after every new ref is added?  Perhaps a 
simple fix would be to move the invalidation right after all 
the refs are updated?  Maybe write_ref_sha1 could take in a 
flag to tell it to not invalidate the cache so that during 
iterative updates it could be disabled and then run manually 
after the update?

-Martin

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

^ permalink raw reply

* Re: [PATCH 2/2] use new Git::config_path() for aliasesfile
From: Jakub Narebski @ 2011-09-30 22:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Cord Seele, Matthieu Moy, git, Eric Wong, Cord Seele
In-Reply-To: <7vsjndoldq.fsf@alter.siamese.dyndns.org>

On Fri, 30 Sep 2011, Junio C Hamano wrote:

> I think the addition of "config --path" support is a good idea, but the
> resulting code suffers from too many cut&paste cruft across the config*
> family of methods.
> 
> How about doing a bit of refactoring, perhaps something like this, on top
> as a separate patch?

This is a good idea, in my opinion.
 
> I tried to be careful to still forcing the "one value only" for config_bool
> and config_int, but extra sets of eyeballs would be needed.

We do have tests for that, have we?
 
[...]
> +Common subroutine to implement bulk of what the config* family of methods
> +do. This wraps command('config') so it is not so fast.

BTW. I wonder if it wouldn't be a good idea to restart work on Git::Config
with lazy/eager loading of the whole config, like git_get_project_config()
and git_parse_project_config() subroutines from gitweb do it - running 
"git config -z -l".  Though then --int/--bool/--path conversion would have
to be implemented in Perl...
  
>  =cut
>  
> -sub config {
> -	my ($self, $var) = _maybe_self(@_);
> -
> +sub _config_common {
> +	my ($self, $var, $opts) = _maybe_self(@_);
> +	
>  	try {
> -		my @cmd = ('config');
> +		my @cmd = ('config', $opts->{'kind'} ? @{$opts->{'kind'}} : ());

How it is supposed to work?

First, you probably want to check for "exists $opts->{'kind'}", or even
"defined $opts->{'kind'}".

Second, "@{$opts->{'kind'}}" assumes that $opts->{'kind'} is an array
reference (something we didn't check)... and it isn't.


BTW. why do you use hashref?  Do you plan for the future to pass more
options that 'kind'?

>  		unshift @cmd, $self if $self;
>  		if (wantarray) {
>  			return command(@cmd, '--get-all', $var);
> @@ -594,6 +590,21 @@ sub config {
>  			throw $E;
>  		}
>  	};
> +
> +}
> +
> +=item config ( VARIABLE )
> +
> +Retrieve the configuration C<VARIABLE> in the same manner as C<config>
> +does. In scalar context requires the variable to be set only one time
> +(exception is thrown otherwise), in array context returns allows the
> +variable to be set multiple times and returns all the values.
> +
> +=cut
> +
> +sub config {
> +	my ($self, $var) = _maybe_self(@_);
> +	return _config_common($self, $var, +{});

No need to pass an empty hash.  Perl has a feature called autovivification:
when an undefined variable is dereferenced, it gets silently upgraded to
an array or hash reference (depending of the type of the dereferencing).

It is un-Perlish to do so.

>  sub config_bool {
>  	my ($self, $var) = _maybe_self(@_);
[...]
> +	my $val = scalar _config_common($self, $var, {'kind' => '--bool'});
> +	return (defined $val && $val eq 'true');
>  }
[...]

>  sub config_path {
>  	my ($self, $var) = _maybe_self(@_);
[...]
> +	return _config_common($self, $var, +{'kind' => '--path'});
>  }

Why the difference between {'kind' => '--bool'} and +{'kind' => '--path'}?

> -This currently wraps command('config') so it is not so fast.
> -

Shouldn't this be mentioned somewhat, even indirectly?

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH] gitk: Show patch for initial commit
From: Marcus Karlsson @ 2011-09-30 21:50 UTC (permalink / raw)
  To: git; +Cc: gitster

Make gitk show the patch for the initial commit.

Signed-off-by: Marcus Karlsson <mk@acc.umu.se>
---
 gitk-git/gitk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..20aeae6 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -7436,7 +7436,7 @@ proc diffcmd {ids flags} {
 	    lappend cmd HEAD
 	}
     } else {
-	set cmd [concat | git diff-tree -r $flags $ids]
+	set cmd [concat | git diff-tree -r --root $flags $ids]
     }
     return $cmd
 }
-- 
1.7.7.rc3.4.g8d714

^ permalink raw reply related

* Re: Bug?: 'git log --find-copies' doesn't match 'git log --follow <rev> -- path/to/file'
From: Jeff King @ 2011-09-30 21:38 UTC (permalink / raw)
  To: Alexander Pepper; +Cc: git
In-Reply-To: <DBC73B3F-2703-4651-AADA-233A9CC38AFD@inf.fu-berlin.de>

On Fri, Sep 30, 2011 at 05:32:38PM +0200, Alexander Pepper wrote:

> So git log with copy and rename detection on (--find-copies) tells me,
> that the file StopClusterException.java is copied to
> ClusterOperation.java. But If I ask git log for that specific file
> with --follow git claims a copy from Immutable.java to
> ClusterOperation.java!

I think that --follow uses --find-copies-harder. Did you try:

  git log --numstat --find-copies-harder dd4e90f9

? Does it find Immutable.java as the source?

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] use new Git::config_path() for aliasesfile
From: Cord Seele @ 2011-09-30 21:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthieu Moy, git, Eric Wong, Jakub Narebski
In-Reply-To: <7vsjndoldq.fsf@alter.siamese.dyndns.org>

On Fri 30 Sep 2011 12:55:45 -0700, Junio C Hamano <gitster@pobox.com> wrote:

> I think the addition of "config --path" support is a good idea, but the
> resulting code suffers from too many cut&paste cruft across the config*
> family of methods.
> 
> How about doing a bit of refactoring, perhaps something like this, on top
> as a separate patch?

Sound very reasonable to me - unfortunately it's beyond my perl-scope to be of
much help here.

-- Cord

^ permalink raw reply

* Fwd: Release notes link problem
From: Eugene Sajine @ 2011-09-30 21:06 UTC (permalink / raw)
  To: git
In-Reply-To: <CAPZPVFbWtDM5T3ZPFMc_MH4aSsfoLyVoGrGm2FeO0TOdhLrz2g@mail.gmail.com>

hi,

The release notes link on http://git-scm.com/ is pointing to a page
which doesn't seem to exist. I'm getting error 404 when trying to
access it. Does anybody see the same issue?

the link is
https://raw.github.com/gitster/git/master/Documentation/RelNotes/1.7.6.4.txt

Thanks,
Eugene

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-30 21:02 UTC (permalink / raw)
  To: git
  Cc: Christian Couder, Thomas Rast, René Scharfe, Julian Phillips,
	Michael Haggerty
In-Reply-To: <201109301041.13848.mfick@codeaurora.org>

On Friday, September 30, 2011 10:41:13 am Martin Fick wrote:
> massive fix to bring it down to 7.5mins was awesome.
> 7-8mins sounded pretty good 2 weeks ago, especially when
> a checkout took 5+ mins!  but now that almost every
> other operation has been sped up, that is starting to
> feel a bit on the slow side still.  My spidey sense
> tells me something is still not quite right in the fetch
> path.

I guess I overlooked that there were 2 sides to this 
equation.  Even though I have been doing my fetches locally, 
I was using the file:// protocol and it appears that the 
remote was running git 1.7.6 which was in my path the whole 
time.  So eliminating that from my path and pointing to the 
the "best" binary with all the fixes for both remote and 
local, the full fetch does indeed speed up quite a bit, it 
goes from about 7.5mins down to ~5m!  Previously the remote 
seemed to primarily spend the extra time after:

 remote: Counting objects: 316961

yet before:

 remote: Compressing objects


> Here is some more data to backup my spidey sense: after
> all the improvements, a noop fetch of all the changes
> (noop meaning they are all already uptodate) takes
> around 3mins with a non gced (non packed refs) case. 
> That same noop only takes ~12s in the gced (packed ref
> case)!

I believe (it is hard to be go back and be sure) that this 
means that the timings above which gave me 3mins were 
because the remote was using git 1.7.6.  Now, with the good 
binary, in both repos (packed and unpacked), I get great 
warm cache times of about 11-13s for a noop fetch.  It is 
interesting to note that cold cache times are 20s for packed 
refs and 1m30s for unpacked refs.  I guess that makes some 
sense.  

But, this does leave me thinking that packed refs should 
become the default and that there should be a config option 
to disable it?  This still might help a fetch?

Since a full sync is now done to about 5mins, I broke down 
the output a bit.  It appears that the longest part (2:45m) 
is now the time spent scrolling though each change still.  
Each one of these takes about 2ms:
 * [new branch]      refs/changes/99/71199/1 -> 
refs/changes/99/71199/1

Seems fast, but at about 80K... So, are there any obvious N 
loops over the refs happening inside each of of the [new 
branch] iterations?


-Martin

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

^ permalink raw reply

* Re: [PATCH 2/2] use new Git::config_path() for aliasesfile
From: Junio C Hamano @ 2011-09-30 19:55 UTC (permalink / raw)
  To: Cord Seele; +Cc: Matthieu Moy, git, Eric Wong, Cord Seele, Jakub Narebski
In-Reply-To: <1317379945-9355-3-git-send-email-cowose@gmail.com>

I think the addition of "config --path" support is a good idea, but the
resulting code suffers from too many cut&paste cruft across the config*
family of methods.

How about doing a bit of refactoring, perhaps something like this, on top
as a separate patch?

I tried to be careful to still forcing the "one value only" for config_bool
and config_int, but extra sets of eyeballs would be needed.

 perl/Git.pm |   93 +++++++++++++++++-----------------------------------------
 1 files changed, 27 insertions(+), 66 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index c279bfb..f0a6e92 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -563,22 +563,18 @@ sub wc_chdir {
 }
 
 
-=item config ( VARIABLE )
+=item _config_common ( VARIABLE )
 
-Retrieve the configuration C<VARIABLE> in the same manner as C<config>
-does. In scalar context requires the variable to be set only one time
-(exception is thrown otherwise), in array context returns allows the
-variable to be set multiple times and returns all the values.
-
-This currently wraps command('config') so it is not so fast.
+Common subroutine to implement bulk of what the config* family of methods
+do. This wraps command('config') so it is not so fast.
 
 =cut
 
-sub config {
-	my ($self, $var) = _maybe_self(@_);
-
+sub _config_common {
+	my ($self, $var, $opts) = _maybe_self(@_);
+	
 	try {
-		my @cmd = ('config');
+		my @cmd = ('config', $opts->{'kind'} ? @{$opts->{'kind'}} : ());
 		unshift @cmd, $self if $self;
 		if (wantarray) {
 			return command(@cmd, '--get-all', $var);
@@ -594,6 +590,21 @@ sub config {
 			throw $E;
 		}
 	};
+
+}
+
+=item config ( VARIABLE )
+
+Retrieve the configuration C<VARIABLE> in the same manner as C<config>
+does. In scalar context requires the variable to be set only one time
+(exception is thrown otherwise), in array context returns allows the
+variable to be set multiple times and returns all the values.
+
+=cut
+
+sub config {
+	my ($self, $var) = _maybe_self(@_);
+	return _config_common($self, $var, +{});
 }
 
 
@@ -603,60 +614,24 @@ Retrieve the bool configuration C<VARIABLE>. The return value
 is usable as a boolean in perl (and C<undef> if it's not defined,
 of course).
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_bool {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--bool', '--get', $var);
-		unshift @cmd, $self if $self;
-		my $val = command_oneline(@cmd);
-		return undef unless defined $val;
-		return $val eq 'true';
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	my $val = scalar _config_common($self, $var, {'kind' => '--bool'});
+	return (defined $val && $val eq 'true');
 }
 
-
 =item config_path ( VARIABLE )
 
 Retrieve the path configuration C<VARIABLE>. The return value
 is an expanded path or C<undef> if it's not defined.
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_path {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--path');
-		unshift @cmd, $self if $self;
-		if (wantarray) {
-			return command(@cmd, '--get-all', $var);
-		} else {
-			return command_oneline(@cmd, '--get', $var);
-		}
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	return _config_common($self, $var, +{'kind' => '--path'});
 }
 
 =item config_int ( VARIABLE )
@@ -667,26 +642,12 @@ or 'g' in the config file will cause the value to be multiplied
 by 1024, 1048576 (1024^2), or 1073741824 (1024^3) prior to output.
 It would return C<undef> if configuration variable is not defined,
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_int {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--int', '--get', $var);
-		unshift @cmd, $self if $self;
-		return command_oneline(@cmd);
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	my $val = scalar _config_common($self, $var, +{'kind' => '--int'});
+	return $val;
 }
 
 =item get_colorbool ( NAME )

^ permalink raw reply related

* Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain
From: Jay Soffian @ 2011-09-30 19:33 UTC (permalink / raw)
  To: John Szakmeister; +Cc: Jeff King, git, Junio C Hamano
In-Reply-To: <CAEBDL5WhpVg17aPuRqrE5=2Q293kVD4fYtxGqRzx_K=87t-jgw@mail.gmail.com>

On Thu, Sep 29, 2011 at 6:03 AM, John Szakmeister <john@szakmeister.net> wrote:
>
> I've been working on a version of the keychain credential cache as
> well.  I did create a gui, although it's a bit painful.

I still don't understand why a CLI app should have a GUI credential prompt.

>>  2. The "get the username from the config" feature is triggered at the
>>     time of prompting the user (so instead of asking for the username,
>>     we check the config and pretend the user told us).
>>
>>     I did it this way originally so that helpers would have the first
>>     crack at setting a username, and we would fall back to the config.
>>     Thinking on it more, that may be backwards. If the user has told
>>     git "for github.com, I am user 'foo'", then that should probably
>>     take effect first, and --username=foo get passed to the helper.

Sorry, missed this part in my previous reply. I don't understand - how
do you ever send a username to the credential helper if you don't get
it from the config? But in any case, if you have a username (via
config or some other way), yes, I think it should be given to the
credential helper.

> I think that makes sense.  I think one thing we have to be careful
> about partial matches.  I wouldn't want the credential cache to send
> off the wrong password to a service.  This may be me being cautious,
> but if I don't have all the necessary bits, I'd rather we fail that to
> guess which entry is right.

The credential helper I wrote doesn't work that way. To do so would
mean using a rather more complicated form of the OS X Security API. It
asks for an entry using whatever fields it has, and OS X returns the
first match that satisfies. It's up to the user to yea/nay that match
if the credential helper isn't on the entry's ACL.

>>> +     /* "GitHub for Mac" compatibility */
>>> +     if (!strcmp(hostname, "github.com"))
>>> +             hostname = "github.com/mac";
>>
>> Nice touch. :)
>
> I honestly don't understand why this needs to be done.

Because GitHub for Mac stores its entries using "github.com/mac" as
the hostname.

> I don't use GitHub for Mac... does that mean this is busted for me?

No. It just means that the credential helper and GitHub for Mac store
their entry in a compatible fashion. (So that each can locate the
entry stored by the other.)

> [snip]
>> My series will also produce "cert:/path/to/certificate" when unlocking a
>> certificate. The other candidates for conversion are smtp-auth (for
>> send-email) and imap (for imap-send).  I guess for certs, you'd want to
>> use the "generic" keychain type.
>
> There is a method for adding a certificate to the keychain:
>   <http://developer.apple.com/library/mac/#documentation/Security/Reference/certifkeytrustservices/Reference/reference.html#//apple_ref/doc/uid/TP30000157>
>
> I'm not sure what that does exactly, but I do have a cert, and it
> shows up as "certificate" in the keychain.

That's for storing a certificate itself. In this case, I think we're
just talking about storing the passphrase which protects the
certificate's private key.

>> I wonder if some people would not want to cache cert passwords. Speaking
>> of which, I remember keychain asking me "do you want to let git see this
>> password?", but I don't ever remember it asking "do you want to save
>> this password?". Is that usually automatic? Again, I was kind of
>> expecting a dialog with a "remember this" checkbox.
>
> By the time you get Keychain involved, the decision has been made.
> Most applications offer that ability... and you're right, this should
> probably offer the same capability.  That also means stashing that
> data somewhere. :-(  OTOH, it does make for a better user experience.

What, no? If you don't want git to store usernames/passwords stored in
the OS X Keychain, don't use the git-osx-keychain credential helper.

j.

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-30 19:26 UTC (permalink / raw)
  To: git
  Cc: Christian Couder, Thomas Rast, René Scharfe, Julian Phillips,
	Michael Haggerty
In-Reply-To: <201109301041.13848.mfick@codeaurora.org>

On Friday, September 30, 2011 10:41:13 am Martin Fick wrote:
> I dug into this a bit further.  I took a non gced and non
> packed refs repo and this time instead of gcing it to get
> packedrefs, I only ran the above git pack-refs --all so
> that objects did not get gced.  With this, the noop
> fetch was also only around 12s.  This confirmed that the
> non gced objects are not interfering with the noop
> fetch, the problem really is just the unpacked refs. 
> Just to confirm that the FS is not horribly slow, I did
> a "find .git/refs" and it only takes about .4s for about
> 80Kresults!

Is there a way I can for refs to always be packed?  I didn't 
see a config option for this.  I would like to try a fetch 
this way even if I have to make a small code tweak.  

I tried simulating on the fly ref packing every now and then 
by running the pack from another repo during the fetch, it 
actually slowed things down (by more than the time it took 
to do the packs).


-Martin

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

^ permalink raw reply

* Re: [PATCH] contrib: add a pair of credential helpers for Mac OS X's keychain
From: Jay Soffian @ 2011-09-30 19:16 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, John Szakmeister
In-Reply-To: <20110929075627.GB14022@sigill.intra.peff.net>

On Thu, Sep 29, 2011 at 3:56 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Sep 14, 2011 at 10:51:53PM -0400, Jay Soffian wrote:
>
>> This credential helper adds, searches, and removes entries from
>> the Mac OS X keychain. The C version links against the Security
>> framework and is probably the best choice for daily use.
>>
>> A python version is also included primarily as a more readable
>> example and uses the /usr/bin/security CLI to access the keychain.
>>
>> Tested with 10.6.8.
>
> So I finally got a nice working OS X setup (10.7) to play around with
> these. Overall, works as advertised. :) I have a few comments, though.
>
>> Here's a C version that no longer links to git. I also kept the original
>> Python version as an example. I decided not to call out to
>> 'git credential-gitpass' as it was simple enough to manage /dev/tty
>> and there's no portability issues since this is OS X specific.
>
> This was my first one. I kind of expected there to be some kind of
> graphical password dialog. Especially because keychain will pop up a
> dialog and ask you "is it OK for git to access this password?". So I
> sort of assumed that people would assume that credentials happened
> outside of the regular terminal session (I see the same thing on Linux,
> for example, with gpg-agent, which will open a new window and grab
> focus).
>
> But I have no idea what's "normal" on OS X.

This makes no sense to me at all. Ignore OS X for the moment. You use
git on the command-line. Why would there be any expectation of it
interacting with the user via anything other than the terminal.

Anyway, I expect the username/password prompt on the command-line, in
the same terminal window where I just ran the git command that needs
credentials.

> I wondered if you were trying to be friendly to people who were
> connecting via ssh. But that doesn't seem to work at all. I couldn't get
> either version of your helper to actually do anything in an ssh session
> (even with the same user logged in on console).  I guess there is some
> magic to hook it into the keychain manager.

I don't understand where you're running ssh from/to in this scenario,
but OS X has a notion of security contexts (this is a bit of a
tangent):

http://developer.apple.com/library/mac/#technotes/tn2083/_index.html

> Also, regarding opening /dev/tty yourself versus using getpass. There
> are a few magic things getpass will do that your helper won't:
>
>  1. It respects core.askpass, GIT_ASKPASS, and SSH_ASKPASS if they are
>     set.
>
>  2. The "get the username from the config" feature is triggered at the
>     time of prompting the user (so instead of asking for the username,
>     we check the config and pretend the user told us).
>
>     I did it this way originally so that helpers would have the first
>     crack at setting a username, and we would fall back to the config.
>     Thinking on it more, that may be backwards. If the user has told
>     git "for github.com, I am user 'foo'", then that should probably
>     take effect first, and --username=foo get passed to the helper.
>
>     It doesn't make a big difference with long-term storage helpers,
>     because you tell them your username once and they remember it. But
>     for things like credential-cache, it lets you store the username
>     for a long time, but only cache the password (which means not
>     typing the username every time).
>
> So I think maybe reason (2) should go away. But (1) is definitely worth
> considering.

I found it ugly that git's native getpass doesn't echo the username
back, and it seems hackish to me for the credential helper to turn
back around and invoke it in any case. :-(

>> +       if (!unique)
>> +               die("Must specify --unique=TOKEN; try --help");
>
> My test harness checks that this case just asks for the password without
> bothering to do any lookup or storage. It probably doesn't really matter
> in practice; I think git should always be providing _some_ context.

Okay, that wasn't clear from whatever documentation I read on how
credential helpers should behave. But why invoke the credential helper
just to ask for a password?

>> +     hostname = strchr(unique, ':');
>> +     if (!hostname)
>> +             die("Invalid token `%s'", unique);
>> +     *hostname++ = '\0';
>
> Hrm. I was really hoping people wouldn't need to pick apart the "unique"
> token, and it could remain an opaque blob. If helpers are going to do
> this sort of parsing, then I'd just as soon have git break it down for
> them, and do something like:
>
>  git credential-osxkeychain \
>    --protocol=https \
>    --host=github.com \
>    --path=peff/git.git
>    --username=peff
>
> to just hand over as much information as possible, and let the helper
> throw it all together if it wants to.

Keychain entries have distinct fields. I broke apart the token and
stored it the way other applications mostly do on OS X.

>> +     /* "GitHub for Mac" compatibility */
>> +     if (!strcmp(hostname, "github.com"))
>> +             hostname = "github.com/mac";
>
> Nice touch. :)
>
>> +     if (!strcmp(unique, "https")) {
>> +             protocol = kSecProtocolTypeHTTPS;
>> +     } else if (!strcmp(unique, "http")) {
>> +             protocol = kSecProtocolTypeHTTP;
>> +     }
>> +     else
>> +             die("Unrecognized protocol `%s'", unique);
>
> My series will also produce "cert:/path/to/certificate" when unlocking a
> certificate. The other candidates for conversion are smtp-auth (for
> send-email) and imap (for imap-send).  I guess for certs, you'd want to
> use the "generic" keychain type.

Yep, I was punting on certificate for v1.

> I wonder if some people would not want to cache cert passwords. Speaking
> of which, I remember keychain asking me "do you want to let git see this
> password?", but I don't ever remember it asking "do you want to save
> this password?". Is that usually automatic? Again, I was kind of
> expecting a dialog with a "remember this" checkbox.

Each keychain entry has an ACL of applications that are allowed to
access it. When an application asks for an entry and the application
isn't on that entry's ACL, OS X (not the application) presents the
user the dialog you refer to. The application has no control over that
dialog.

Now, I could have the credential helper ask the user "store this
password?" after prompting for it, but why even use a credential
helper if you don't want it to store your credentials?

>> +def add_internet_password(protocol, hostname, username, password):
>> +    # We do this over a pipe so that we can provide the password more
>> +    # securely than as an argument which would show up in ps output.
>> +    # Unfortunately this is possibly less robust since the security man
>> +    # page does not document how to quote arguments. Emprically it seems
>> +    # that using the double-quote, escaping \ and " works properly.
>> +    username = username.replace('\\', '\\\\').replace('"', '\\"')
>> +    password = password.replace('\\', '\\\\').replace('"', '\\"')
>> +    command = ' '.join([
>> +        'add-internet-password', '-U',
>> +        '-r', protocol,
>> +        '-s', hostname,
>> +        '-a "%s"' % username,
>> +        '-w "%s"' % password,
>> +        '-j default',
>> +        '-l "%s (%s)"' % (hostname, username),
>> +    ]) + '\n'
>> +    args = ['/usr/bin/security', '-i']
>> +    p = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
>> +    p.communicate(command)
>
> I noticed that when using the python helper, the dialog asking something
> like: "security wants to know this password. Allow it?"
>
> Which was kind of lame. I would hope we could convince it to say "git".
> But I didn't see any option in the "security" tool for specifying the
> context[1]. The C helper says "git-credential-osxkeychain". Which isn't
> the end of the world, but it would be prettier if it just said "git".

That's partly why I wrote the C version.

> [1] I can kind of see why they might not want you to set this for
> security reasons (because it makes impersonating other programs easy).
> On the other hand, saying "security" conveys absolutely nothing. And as
> far as I can tell, I could just call my program /tmp/iTunes, and it
> would say "iTunes wants to know this password...".

It is for security reasons. 99% of users will probably just click
"Okay" no matter what. For the 1% that bother to pay attention to the
dialog, it provides the full path to the binary. I'd be suspicious if
/tmp/iTunes wanted a password.

j.

^ permalink raw reply

* Re: [PATCH v3 1/4] compat/win32/sys/poll.c: upgrade from upstream
From: René Scharfe @ 2011-09-30 19:00 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git, peff, j6t, gitster
In-Reply-To: <1317329963-6656-2-git-send-email-kusmabite@gmail.com>

Am 29.09.2011 22:59, schrieb Erik Faye-Lund:
> poll.c is updated from revision adc3a5b in
> git://git.savannah.gnu.org/gnulib.git
> 
> The changes are applied with --whitespace=fix to reduce noise.
> 
> poll.h is not upgraded, because the most recent version now
> contains template-stuff that breaks compilation for us.

> @@ -27,7 +27,10 @@
>  #include <malloc.h>
>  
>  #include <sys/types.h>
> -#include "poll.h"
> +
> +/* Specification.  */
> +#include <poll.h>
> +

In order to make bisecting easier, I think it's a good idea to squash in
the next patch that undoes this part, i.e. simply skip this hunk and add
a third non-verbatim note to the commit message.

René

^ permalink raw reply

* Re: [PATCH v3 3/4] enter_repo: do not modify input
From: René Scharfe @ 2011-09-30 19:00 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git, peff, j6t, gitster
In-Reply-To: <1317329963-6656-4-git-send-email-kusmabite@gmail.com>

Am 29.09.2011 22:59, schrieb Erik Faye-Lund:
> diff --git a/path.c b/path.c
> index 6f3f5d5..f7dfd0b 100644
> --- a/path.c
> +++ b/path.c
> @@ -283,7 +283,7 @@ return_null:
>   * links.  User relative paths are also returned as they are given,
>   * except DWIM suffixing.
>   */
> -char *enter_repo(char *path, int strict)
> +const char *enter_repo(const char *path, int strict)
>  {
>  	static char used_path[PATH_MAX];
>  	static char validated_path[PATH_MAX];
> @@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)
>  		};
>  		int len = strlen(path);
>  		int i;
> -		while ((1 < len) && (path[len-1] == '/')) {
> -			path[len-1] = 0;
> +		while ((1 < len) && (path[len-1] == '/'))
>  			len--;
> -		}
> +
>  		if (PATH_MAX <= len)
>  			return NULL;
> -		if (path[0] == '~') {
> -			char *newpath = expand_user_path(path);
> +		strncpy(used_path, path, len);
> +
> +		if (used_path[0] == '~') {
> +			char *newpath = expand_user_path(used_path);
>  			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
>  				free(newpath);
>  				return NULL;
> @@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)
>  			 * anyway.
>  			 */
>  			strcpy(used_path, newpath); free(newpath);
> -			strcpy(validated_path, path);
> -			path = used_path;
> +			strcpy(validated_path, used_path);
>  		}
>  		else if (PATH_MAX - 10 < len)
>  			return NULL;
> -		else {
> -			path = strcpy(used_path, path);
> -			strcpy(validated_path, path);
> -		}
> -		len = strlen(path);
> +		else
> +			strcpy(validated_path, used_path);
> +		len = strlen(used_path);
>  		for (i = 0; suffix[i]; i++) {
> -			strcpy(path + len, suffix[i]);
> -			if (!access(path, F_OK)) {
> +			strcpy(used_path + len, suffix[i]);
> +			if (!access(used_path, F_OK)) {
>  				strcat(validated_path, suffix[i]);
>  				break;
>  			}
>  		}
> -		if (!suffix[i] || chdir(path))
> +		if (!suffix[i] || chdir(used_path))
>  			return NULL;
>  		path = validated_path;
>  	}

The use of strcpy and strncpy makes me nervous, but I can't spot a bug
currently and strcpy and even strcat calls had been already in there
before your patch.

René

^ permalink raw reply

* Re: Updated tag 'junio-gpg-pub' ?
From: Andreas Schwab @ 2011-09-30 18:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stefan Näwe, Git List
In-Reply-To: <7voby2oy2s.fsf@alter.siamese.dyndns.org>

You might want to update the tag message the next time with
s/git-/git /.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH] Clarify that '--tags' fetches tags only
From: Junio C Hamano @ 2011-09-30 18:37 UTC (permalink / raw)
  To: Peter Shenkin; +Cc: git
In-Reply-To: <loom.20110930T041939-332@post.gmane.org>

Peter Shenkin <shenkin@gmail.com> writes:

> Perhaps it will be useful to say what would have been most
> helpful for me. In the current documentation for "fetch
> --tags", one sentence reads, "This flag lets all tags and
> their associated  objects be downloaded." The following small
> modification would, IMO, be sufficient: "This flag causes all
> tags and their associated objects (only) to be downloaded."

Hmm, from time to time we seem to see this kind of documentation
suggestion where:

 - We (try to) describe what xyzzy does by saying "This is what xyzzy
   does". We specifically do not say "In addition to what normally
   happens, xyzzy causes these additional things to happen."

 - The reader (somehow) assumes xyzzy does more than what we described in
   the documentation, even we did not say "In addition to..."; and then

 - A patch is proposed to add "these other things are _not_ done", after
   existing "This is what xyzzy does".

And it is not limited to the description of this particular option.

I think in general our documentation aims to spell out _all_ that happens,
and explicitly say "In addition to what normally happens", "This page
lists only the most common ways", etc., when such a clatification is
needed.

I am wondering if there is a systemic failure that gives an impression
that by default the documentation is incomplete and all other unspecified
thing also happens to the readers? If so are there things that we could
do better without going through individual description?

^ permalink raw reply

* Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions
From: Junio C Hamano @ 2011-09-30 18:19 UTC (permalink / raw)
  To: Pang Yan Han
  Cc: git, Sitaram Chamarty, Shawn O. Pearce, Jeff King,
	Johannes Schindelin
In-Reply-To: <20110930132903.GA1622@myhost>

Pang Yan Han <pangyanhan@gmail.com> writes:

> Sorry for asking, but do I need to reroll this with the fixup in
> origin/ph/push-to-delete-nothing ? Is the commit message fine especially
> in light of the changes in the fixup?

If you think that the result of squashing the fix-up commit into your
patch looks OK, I do not think there is a need to reroll. The patch title
may need to be shortened, but other than that I do not see anything
glaringly wrong in the commit log message that I cannot amend out.

Thanks.

^ 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