git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [ANNOUNCE] Git 1.7.8.rc0
@ 2011-10-31  5:00 Junio C Hamano
  2011-10-31 14:17 ` Stefan Näwe
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2011-10-31  5:00 UTC (permalink / raw)
  To: git; +Cc: Linux Kernel

A release candidate Git 1.7.8.rc0 is available for testing.

The release tarballs are found at:

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

and their SHA-1 checksums are:

4c437ecb17ba7d1b69cecd06eae9543ad35be7a6  git-1.7.8.rc0.tar.gz
5fc490a7ab29bf020a8f46eecfdb421d970b6235  git-htmldocs-1.7.8.rc0.tar.gz
856259f71c10b21620caa27dbc74c3794f0c6854  git-manpages-1.7.8.rc0.tar.gz

Also the following public repositories all have a copy of the v1.7.8.rc0
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

There are a few topics that I would further merge down before -rc1 but
this should be pretty much it for the upcoming release, as far as "new
features" are concerned. Please test thoroughly to hunt for regressions.

Hopefully we can have something reasonable by late November before many in
the US will stop working and start stuffing themselves.


Git v1.7.8 Release Notes (draft)
================================

Updates since v1.7.7
--------------------

 * Some git-svn, git-gui, git-p4 (in contrib) and msysgit updates.

 * Updates to bash completion scripts.

 * The build procedure has been taught to take advantage of computed
   dependency automatically when the complier supports it.

 * The date parser now accepts timezone designators that lack minutes
   part and also has a colon between "hh:mm".

 * The contents of the /etc/mailname file, if exists, is used as the
   default value of the hostname part of the committer/author e-mail.

 * "git am" learned how to read from patches generated by Hg.

 * "git archive" talking with a remote repository can report errors
   from the remote side in a more informative way.

 * "git branch" learned an explicit --list option to ask for branches
   listed, optionally with a glob matching pattern to limit its output.

 * "git check-attr" learned "--cached" option to look at .gitattributes
   files from the index, not from the working tree.

 * Variants of "git cherry-pick" and "git revert" that take multiple
   commits learned to "--continue".

 * "git daemon" gives more human readble error messages to clients
   using ERR packets when appropriate.

 * Errors at the network layer is logged by "git daemon".

 * "git diff" learned "--minimal" option to spend extra cycles to come
   up with a minimal patch output.

 * "git diff" learned "--function-context" option to show the whole
   function as context that was affected by a change.

 * "git difftool" can be told to skip launching the tool for a path by
   answering 'n' to its prompt.

 * "git fetch" learned to honor transfer.fsckobjects configuration to
   validate the objects that were received from the other end, just like
   "git receive-pack" (the receiving end of "git push") does.

 * "git fetch" makes sure that the set of objects it received from the
   other end actually completes the history before updating the refs.
   "git receive-pack" (the receiving end of "git push") learned to do the
   same.

 * "git fetch" learned that fetching/cloning from a regular file on the
   filesystem is not necessarily a request to unpack a bundle file; the
   file could be ".git" with "gitdir: <path>" in it.

 * "git for-each-ref" learned "%(contents:subject)", "%(contents:body)"
   and "%(contents:signature)". The last one is useful for signed tags.

 * "git grep" used to incorrectly pay attention to .gitignore files
   scattered in the directory it was working in even when "--no-index"
   option was used. It no longer does this. The "--exclude-standard"
   option needs to be given to explicitly activate the ignore
   mechanism.

 * "git grep" learned "--untracked" option, where given patterns are
    searched in untracked (but not ignored) files as well as tracked
    files in the working tree, so that matches in new but not yet
    added files do not get missed.

 * The recursive merge backend no longer looks for meaningless
   existing merges in submodules unless in the outermost merge.

 * "git log" and friends learned "--children" option.

 * "git ls-remote" learned to respond to "-h"(elp) requests.

 * "git merge" learned the "--edit" option to allow users to edit the
   merge commit log message.

 * "git rebase -i" can be told to use special purpose editor suitable
   only for its insn sheet via sequence.editor configuration variable.

 * "git send-email" learned to respond to "-h"(elp) requests.

 * "git send-email" allows the value given to sendemail.aliasfile to begin
   with "~/" to refer to the $HOME directory.

 * "git send-email" forces use of Authen::SASL::Perl to work around
   issues between Authen::SASL::Cyrus and AUTH PLAIN/LOGIN.

 * "git stash" learned "--include-untracked" option to stash away
   untracked/ignored cruft from the working tree.

 * "git submodule clone" does not leak an error message to the UI
   level unnecessarily anymore.

 * "git submodule update" learned to honor "none" as the value for
   submodule.<name>.update to specify that the named submodule should
   not be checked out by default.

 * When populating a new submodule directory with "git submodule init",
   the $GIT_DIR metainformation directory for submodules is created inside
   $GIT_DIR/modules/<name>/ directory of the superproject and referenced
   via the gitfile mechanism. This is to make it possible to switch
   between commits in the superproject that has and does not have the
   submodule in the tree without re-cloning.

 * "mediawiki" remote helper can interact with (surprise!) MediaWiki
   with "git fetch" & "git push".

 * "gitweb" leaked unescaped control characters from syntax hiliter
   outputs.

 * "gitweb" can be told to give custom string at the end of the HTML
   HEAD element.

 * "gitweb" now has its own manual pages.


Also contains other documentation updates and minor code cleanups.


Fixes since v1.7.7
------------------

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

 * We used to drop error messages from libcurl on certain kinds of
   errors.
   (merge be22d92eac8 jn/maint-http-error-message later to maint).

 * Error report from smart HTTP transport, when the connection was
   broken in the middle of a transfer, showed a useless message on
   a corrupt packet.
   (merge 6cdf022 sp/smart-http-failure later to maint).

 * HTTP transport did not use pushurl correctly, and also did not tell
   what host it is trying to authenticate with when asking for
   credentials.
   (merge deba493 jk/http-auth later to maint).

 * "git branch -m/-M" advertised to update RENAME_REF ref in the
   commit log message that introduced the feature but not anywhere in
   the documentation, and never did update such a ref anyway. This
   undocumented misfeature that did not exist has been excised.
   (merge b0eab01 jc/maint-remove-renamed-ref later to maint).

 * Adding many refs to the local repository in one go (e.g. "git fetch"
   that fetches many tags) and looking up a ref by name in a repository
   with too many refs were unnecessarily slow.
   (merge 17d68a54d jp/get-ref-dir-unsorted later to maint).

 * "git fetch --prune" was unsafe when used with refspecs from the
   command line.
   (merge e8c1e6c cn/fetch-prune later to maint).

 * Report from "git commit" on untracked files was confused under
   core.ignorecase option.
   (merge 2548183b jk/name-hash-dirent later to maint).

 * The attribute mechanism did not use case insensitive match when
   core.ignorecase was set.
   (merge 6eba621 bc/attr-ignore-case later to maint).

 * "git bisect" did not notice when it failed to update the working tree
   to the next commit to be tested.
   (merge 1acf11717 js/bisect-no-checkout later to maint).

 * "git config --bool --get-regexp" failed to separate the variable name
   and its value "true" when the variable is defined without "= true".
   (merge 880e3cc mm/maint-config-explicit-bool-display later to maint).

 * "git remote rename $a $b" were not careful to match the remote name
   against $a (i.e. source side of the remote nickname).
   (merge b52d00aed mz/remote-rename later to maint).

 * "git diff --[num]stat" used to use the number of lines of context
   different from the default, potentially giving different results from
   "git diff | diffstat" and confusing the users.
   (merge f01cae918 jc/maint-diffstat-numstat-context later to maint).

 * "git merge" did not understand ":/<pattern>" as a way to name a commit.

 * "git mergetool" learned to use its arguments as pathspec, not a path to
   the file that may not even have any conflict.
   (merge 6d9990a jm/mergetool-pathspec later to maint).

 * "git pull" and "git rebase" did not work well even when GIT_WORK_TREE is
   set correctly with GIT_DIR if the current directory is outside the working
   tree.
   (merge 035b5bf jk/pull-rebase-with-work-tree later to maint).

 " "git push" on the receiving end used to call post-receive and post-update
   hooks for attempted removal of non-existing refs.
   (merge 160b81ed ph/push-to-delete-nothing later to maint).

 * "git send-email" did not honor the configured hostname when restarting
   the HELO/EHLO exchange after switching TLS on.
   (merge 155b940 md/smtp-tls-hello-again later to maint).

 * "gitweb" used to produce a non-working link while showing the contents
   of a blob, when JavaScript actions are enabled.
   (merge 2b07ff3ff ps/gitweb-js-with-lineno later to maint).

 * The logic to filter out forked projects in the project list in
   "gitweb" was broken for some time.
   (merge 53c632f jm/maint-gitweb-filter-forks-fix later to maint).

----------------------------------------------------------------

Changes since v1.7.7 are as follows:

Bert Wesarg (7):
      grep: do not use --index in the short usage output
      grep --no-index: don't use git standard exclusions
      git-gui: search and linenumber input are mutual exclusive in the blame view
      git-gui: only accept numbers in the goto-line input
      git-gui: clear the goto line input when hiding
      git-gui: incremental goto line in blame view
      grep: fix the error message that mentions --exclude

Brad King (3):
      rev-list: Demonstrate breakage with --ancestry-path --all
      submodule: Demonstrate known breakage during recursive merge
      submodule: Search for merges only at end of recursive merge

Brandon Casey (13):
      t/t3905: use the name 'actual' for test output, swap arguments to test_cmp
      git-stash.sh: fix typo in error message
      t/t3905: add missing '&&' linkage
      git-stash: remove untracked/ignored directories when stashed
      attr.c: avoid inappropriate access to strbuf "buf" member
      cleanup: use internal memory allocation wrapper functions everywhere
      builtin/mv.c: plug miniscule memory leak
      refs.c: ensure struct whose member may be passed to realloc is initialized
      refs.c: abort ref search if ref array is empty
      refs.c: free duplicate entries in the ref array instead of leaking them
      attr.c: respect core.ignorecase when matching attribute patterns
      strbuf.c: remove unnecessary strbuf_grow() from strbuf_getwholeline()
      t/t3000-ls-files-others.sh: use $SHELL_PATH to run git-new-workdir script

Carlos Martín Nieto (7):
      Remove 'working copy' from the documentation and C code
      fetch: free all the additional refspecs
      t5510: add tests for fetch --prune
      remote: separate out the remote_find_tracking logic into query_refspecs
      fetch: honor the user-provided refspecs when pruning refs
      fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
      Documentation: update [section.subsection] to reflect what git does

Chris Packham (1):
      git-web--browse: avoid the use of eval

Christian Couder (1):
      bisect: fix exiting when checkout failed in bisect_start()

Christoffer Pettersson (1):
      git-gui: Corrected a typo in the Swedish translation of 'Continue'

Clemens Buchacher (5):
      remove prefix argument from pathspec_prefix
      rename pathspec_prefix() to common_prefix() and move to dir.[ch]
      send-email: add option -h
      use -h for synopsis and --help for manpage consistently
      use test number as port number

Cord Seele (3):
      Add Git::config_path()
      use new Git::config_path() for aliasesfile
      send-email: Fix %config_path_settings handling

Dan McGee (2):
      tree-walk: drop unused parameter from match_dir_prefix
      tree-walk: micro-optimization in tree_entry_interesting

David Aguilar (1):
      Makefile: Improve compiler header dependency check

David Fries (2):
      git-gui: Enable jumping to a specific line number in blame view.
      git-gui: Add keyboard shortcuts for search and goto commands in blame view.

Dmitry Ivankov (3):
      Fix typo: existant->existent
      fast-import: don't allow to tag empty branch
      fast-import: don't allow to note on empty branch

Drew Northup (1):
      gitweb: Add gitweb.conf(5) manpage for gitweb configuration files

Erik Faye-Lund (2):
      enter_repo: do not modify input
      mingw: avoid using strbuf in syslog

Fredrik Gustafsson (2):
      rev-parse: add option --resolve-git-dir <path>
      Move git-dir for submodules

Fredrik Kuivinen (1):
      Makefile: Use computed header dependencies if the compiler supports it

Frédéric Heitzmann (1):
      git svn dcommit: new option --interactive.

Giuseppe Bilotta (1):
      am: preliminary support for hg patches

Haitao Li (1):
      date.c: Support iso8601 timezone formats

Heiko Voigt (4):
      git-gui: warn when trying to commit on a detached head
      submodule: move update configuration variable further up
      add update 'none' flag to disable update of submodule by default
      git-gui: deal with unknown files when pressing the "Stage Changed" button

Hui Wang (1):
      sha1_file: normalize alt_odb path before comparing and storing

Ilari Liusvaara (1):
      Support ERR in remote archive like in fetch/push

Jakub Narebski (6):
      gitweb: Strip non-printable characters from syntax highlighter output
      gitweb: Add gitweb(1) manpage for gitweb itself
      Documentation: Link to gitweb(1) and gitweb.conf(5) in other manpages
      Documentation: Add gitweb config variables to git-config(1)
      gitweb: Add gitweb manpages to 'gitweb' package in git.spec
      Add simple test for Git::config_path() in t/t9700-perl-git.sh

Jay Soffian (6):
      Teach '--cached' option to check-attr
      log --children
      merge-one-file: fix "expr: non-numeric argument"
      revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
      cherry-pick: do not give irrelevant advice when cherry-pick punted
      Teach merge the '[-e|--edit]' option

Jeff King (30):
      url: decode buffers that are not NUL-terminated
      improve httpd auth tests
      remote-curl: don't retry auth failures with dumb protocol
      http: retry authentication failures for all http requests
      t7004: factor out gpg setup
      t6300: add more body-parsing tests
      for-each-ref: refactor subject and body placeholder parsing
      for-each-ref: handle multiline subjects like --pretty
      fetch: avoid quadratic loop checking for updated submodules
      t3200: clean up checks for file existence
      add sha1_array API docs
      quote.h: fix bogus comment
      refactor argv_array into generic code
      quote: provide sq_dequote_to_argv_array
      bisect: use argv_array API
      checkout: use argv_array API
      run_hook: use argv_array API
      filter-branch: use require_clean_work_tree
      fix phantom untracked files when core.ignorecase is set
      t1300: put git invocations inside test function
      t1300: test mixed-case variable retrieval
      pull,rebase: handle GIT_WORK_TREE better
      pack-objects: protect against disappearing packs
      downgrade "packfile cannot be accessed" errors to warnings
      daemon: give friendlier error messages to clients
      http_init: accept separate URL parameter
      contrib: add diff highlight script
      tests: add missing executable bits
      contrib: add git-jump script
      completion: match ctags symbol names in grep patterns

Jeremie Nikaes (1):
      Add a remote helper to interact with mediawiki (fetch & push)

Jim Meyering (2):
      fix "git apply --index ..." not to deref NULL
      make the sample pre-commit hook script reject names with newlines, too

Johannes Schindelin (5):
      Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
      t1020: disable the pwd test on MinGW
      t9001: do not fail only due to CR/LF issues
      t9300: do not run --cat-blob-fd related tests on MinGW
      git grep: be careful to use mutexes only when they are initialized

Johannes Sixt (2):
      t1402-check-ref-format: skip tests of refs beginning with slash on Windows
      t1300: attempting to remove a non-existent .git/config is not an error

Jonathan Nieder (7):
      http: remove extra newline in error message
      http: avoid empty error messages for some curl errors
      ident: check /etc/mailname if email is unknown
      Makefile: do not set setgid bit on directories on GNU/kFreeBSD
      ident: do not retrieve default ident when unnecessary
      Makefile: fix permissions of mergetools/ checked out with permissive umask
      RelNotes/1.7.7.1: setgid bit patch is about fixing "git init" via Makefile setting

Jonathon Mah (1):
      mergetool: Use args as pathspec to unmerged files

Julian Phillips (2):
      Don't sort ref_list too early
      refs: Use binary search to lookup refs faster

Julien Muchembled (1):
      gitweb: fix regression when filtering out forks

Junio C Hamano (66):
      rev-list: fix finish_object() call
      revision.c: add show_object_with_name() helper function
      revision.c: update show_object_with_name() without using malloc()
      revision: keep track of the end-user input from the command line
      revision: do not include sibling history in --ancestry-path output
      rebase -i: notice and warn if "exec $cmd" modifies the index or the working tree
      traverse_trees(): allow pruning with pathspec
      unpack-trees: allow pruning with pathspec
      diff-index: pass pathspec down to unpack-trees machinery
      list-objects: pass callback data to show_objects()
      rev-list --verify-object
      fetch: verify we have everything we need before updating our ref
      fetch.fsckobjects: verify downloaded objects
      transfer.fsckobjects: unify fetch/receive.fsckobjects
      test: fetch/receive with fsckobjects
      consolidate pathspec_prefix and common_prefix
      fetch: verify we have everything we need before updating our ref
      check_everything_connected(): refactor to use an iterator
      check_everything_connected(): libify
      receive-pack: check connectivity before concluding "git push"
      builtin/revert.c: make commit_list_append() static
      refs.c: make create_cached_refs() static
      fsck: do not abort upon finding an empty blob
      send-pack: typofix error message
      refactor run_receive_hook()
      rename "match_refs()" to "match_push_refs()"
      Allow git merge ":/<pattern>"
      ls-remote: a lone "-h" is asking for help
      Teach progress eye-candy to fetch_refs_from_bundle()
      diff: teach --stat/--numstat to honor -U$num
      t0003: remove extra whitespaces
      mergetool: no longer need to save standard input
      apply --whitespace=error: correctly report new blank lines at end
      parse-options: deprecate OPT_BOOLEAN
      archive.c: use OPT_BOOL()
      checkout $tree $path: do not clobber local changes in $path not in $tree
      url.c: simplify is_url()
      diff: resurrect XDF_NEED_MINIMAL with --minimal
      grep: teach --untracked and --exclude-standard options
      Post 1.7.7 first wave
      attr: read core.attributesfile from git_default_core_config
      Update draft release notes to 1.7.8
      branch -m/-M: remove undocumented RENAMED-REF
      refs.c: move dwim_ref()/dwim_log() from sha1_name.c
      Update draft release notes to 1.7.8
      bundle: allowing to read from an unseekable fd
      bundle: add parse_bundle_header() helper function
      Update draft release notes to 1.7.8
      t7800: avoid arithmetic expansion notation
      Prepare for 1.7.7.1
      Update draft release notes to 1.7.8
      resolve_gitlink_packed_ref(): fix mismerge
      Update draft release notes to 1.7.8
      Makefile: ask "ls-files" to list source files if available
      libperl-git: refactor Git::config_*
      Update draft release notes to 1.7.8
      resolve_ref(): expose REF_ISBROKEN flag
      resolve_ref(): report breakage to the caller without warning
      Almost ready for 1.7.7.1
      Update draft release notes to 1.7.8
      Git 1.7.7.1
      builtin/grep: make lock/unlock into static inline functions
      builtin/grep: simplify lock_and_read_sha1_file()
      Update draft release notes to 1.7.8
      Update draft release notes to 1.7.8
      Git 1.7.8-rc0

Luke Diamand (1):
      git-p4: handle files with shell metacharacters

Lénaïc Huard (1):
      gitweb: provide a way to customize html headers

Martin von Zweigbergk (4):
      remote: write correct fetch spec when renaming remote 'remote'
      remote: "rename o foo" should not rename ref "origin/bar"
      remote rename: warn when refspec was not updated
      remote: only update remote-tracking branch if updating refspec

Matthew Daley (1):
      send-email: Honour SMTP domain when using TLS

Matthieu Moy (8):
      rebase -i: clean error message for --continue after failed exec
      git-remote-mediawiki: allow push to set MediaWiki metadata
      git-remote-mediawiki: trivial fixes
      git-remote-mediawiki: set 'basetimestamp' to let the wiki handle conflicts
      git-remote-mediawiki: obey advice.pushNonFastForward
      git-remote-mediawiki: allow a domain to be set for authentication
      config: display key_delim for config --bool --get-regexp
      git-remote-mediawiki: don't include HTTP login/password in author

Michael Haggerty (37):
      Extract a function clear_cached_refs()
      Access reference caches only through new function get_cached_refs()
      Change the signature of read_packed_refs()
      Allocate cached_refs objects dynamically
      Store the submodule name in struct cached_refs
      Retain caches of submodule refs
      notes_merge_commit(): do not pass temporary buffer to other function
      get_sha1_hex(): do not read past a NUL character
      t1402: add some more tests
      git check-ref-format: add options --allow-onelevel and --refspec-pattern
      Change bad_ref_char() to return a boolean value
      Change check_ref_format() to take a flags argument
      Refactor check_refname_format()
      Do not allow ".lock" at the end of any refname component
      Make collapse_slashes() allocate memory for its result
      Inline function refname_format_print()
      Change check_refname_format() to reject unnormalized refnames
      resolve_ref(): explicitly fail if a symlink is not readable
      resolve_ref(): use prefixcmp()
      resolve_ref(): only follow a symlink that contains a valid, normalized refname
      resolve_ref(): turn buffer into a proper string as soon as possible
      resolve_ref(): extract a function get_packed_ref()
      resolve_ref(): do not follow incorrectly-formatted symbolic refs
      remote: use xstrdup() instead of strdup()
      remote: avoid passing NULL to read_ref()
      resolve_ref(): verify that the input refname has the right format
      resolve_ref(): emit warnings for improperly-formatted references
      resolve_ref(): also treat a too-long SHA1 as invalid
      resolve_ref(): expand documentation
      add_ref(): verify that the refname is formatted correctly
      invalidate_ref_cache(): rename function from invalidate_cached_refs()
      invalidate_ref_cache(): take the submodule as parameter
      invalidate_ref_cache(): expose this function in the refs API
      clear_ref_cache(): rename parameter
      clear_ref_cache(): extract two new functions
      write_ref_sha1(): only invalidate the loose ref cache
      clear_ref_cache(): inline function

Michael J Gruber (10):
      t6040: test branch -vv
      git-tag: introduce long forms for the options
      git-branch: introduce missing long forms for the options
      branch: introduce --list option
      branch: allow pattern arguments
      branch: -v does not automatically imply --list
      unpack-trees: print "Aborting" to stderr
      git-read-tree.txt: language and typography fixes
      git-read-tree.txt: correct sparse-checkout and skip-worktree description
      http: use hostname in credential description

Michael Schubert (1):
      patch-id.c: use strbuf instead of a fixed buffer

Michael W. Olson (1):
      git-svn: Allow certain refs to be ignored

Michał Górny (1):
      for-each-ref: add split message parts to %(contents:*).

Nguyễn Thái Ngọc Duy (12):
      merge: keep stash[] a local variable
      merge: use return value of resolve_ref() to determine if HEAD is invalid
      merge: remove global variable head[]
      Accept tags in HEAD or MERGE_HEAD
      sparse checkout: show error messages when worktree shaping fails
      Add explanation why we do not allow to sparse checkout to empty working tree
      git-read-tree.txt: update sparse checkout examples
      pack-protocol: document "ERR" line
      daemon: return "access denied" if a service is not allowed
      daemon: log errors if we could not use some sockets
      t5403: convert leading spaces to tabs
      Reindent closing bracket using tab instead of spaces

Nicolas Morey-Chaisemartin (1):
      grep: Fix race condition in delta_base_cache

Pang Yan Han (1):
      receive-pack: don't pass non-existent refs to post-{receive,update} hooks

Pat Thoyts (6):
      git-gui: updated translator README for current procedures.
      Fix tooltip display with multiple monitors on windows.
      git-gui: drop the 'n' and 'Shift-n' bindings from the last patch.
      mergetools: use the correct tool for Beyond Compare 3 on Windows
      mingw: ensure sockets are initialized before calling gethostname
      t9901: fix line-ending dependency on windows

Pete Wyckoff (5):
      git-p4 tests: refactor and cleanup
      git-p4: handle utf16 filetype properly
      git-p4: recognize all p4 filetypes
      git-p4: stop ignoring apple filetype
      git-p4: keyword flattening fixes

Peter Oberndorfer (1):
      "rebase -i": support special-purpose editor to edit insn sheet

Peter Stuge (1):
      gitweb: Fix links to lines in blobs when javascript-actions are enabled

Phil Hord (3):
      Learn to handle gitfiles in enter_repo
      Teach transport about the gitfile mechanism
      Add test showing git-fetch groks gitfiles

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

Ramsay Allan Jones (6):
      Makefile: Make dependency directory creation less noisy
      sparse: Fix an "Using plain integer as NULL pointer" warning
      obstack.c: Fix some sparse warnings
      t9159-*.sh: skip for mergeinfo test for svn <= 1.4
      Fix some "variable might be used uninitialized" warnings
      gitweb/Makefile: Remove static/gitweb.js in the clean target

René Scharfe (26):
      Revert removal of multi-match discard heuristic in 27af01
      parseopt: add OPT_NOOP_NOARG
      revert: use OPT_NOOP_NOARG
      apply: use OPT_NOOP_NOARG
      checkout: check for "Previous HEAD" notice in t2020
      revision: factor out add_pending_sha1
      checkout: use add_pending_{object,sha1} in orphan check
      revision: add leak_pending flag
      bisect: use leak_pending flag
      bundle: use leak_pending flag
      checkout: use leak_pending flag
      commit: factor out clear_commit_marks_for_object_array
      test-ctype: macrofy
      test-ctype: add test for is_pathspec_magic
      name-rev: split usage string
      pickaxe: plug diff filespec leak with empty needle
      pickaxe: plug regex leak
      pickaxe: plug regex/kws leak
      pickaxe: factor out has_changes
      pickaxe: pass diff_options to contains and has_changes
      pickaxe: give diff_grep the same signature as has_changes
      pickaxe: factor out pickaxe
      xdiff: factor out get_func_line()
      diff: add option to show whole functions as context
      t1304: fall back to $USER if $LOGNAME is not defined
      read-cache.c: fix index memory allocation

Richard Hartmann (1):
      clone: Quote user supplied path in a single quote pair

SZEDER Gábor (2):
      completion: unite --reuse-message and --reedit-message for 'notes'
      completion: unite --format and --pretty for 'log' and 'show'

Sebastian Schuberth (2):
      git-svn: On MSYS, escape and quote SVN_SSH also if set by the user
      inet_ntop.c: Work around GCC 4.6's detection of uninitialized variables

Shawn O. Pearce (1):
      remote-curl: Fix warning after HTTP failure

Sitaram Chamarty (1):
      git-difftool: allow skipping file by typing 'n' at prompt

Stefan Naewe (2):
      Documentation/git-update-index: refer to 'ls-files'
      completion: fix issue with process substitution not working on Git for Windows

Tay Ray Chuan (3):
      fetch: plug two leaks on error exit in store_updated_refs
      submodule: whitespace fix
      submodule::module_clone(): silence die() message from module_name()

Teemu Matilainen (3):
      completion: unite --reuse-message and --reedit-message handling
      completion: commit --fixup and --squash
      completion: push --set-upstream

Thomas Rast (3):
      Symlink mergetools scriptlets into valgrind wrappers
      Documentation: basic configuration of notes.rewriteRef
      t6019: avoid refname collision on case-insensitive systems

Zbigniew Jędrzejewski-Szmek (1):
      send-email: auth plain/login fix

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-10-31  5:00 [ANNOUNCE] Git 1.7.8.rc0 Junio C Hamano
@ 2011-10-31 14:17 ` Stefan Näwe
  2011-10-31 17:19   ` Junio C Hamano
  2011-11-01  9:53   ` Stefan Näwe
  0 siblings, 2 replies; 26+ messages in thread
From: Stefan Näwe @ 2011-10-31 14:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, Linux Kernel

Am 31.10.2011 06:00, schrieb Junio C Hamano:
> A release candidate Git 1.7.8.rc0 is available for testing.
> 
> [...]
> 
> 
> Git v1.7.8 Release Notes (draft)
> ================================
> 
> Updates since v1.7.7
> --------------------
> 
> [...]
> 
>  * HTTP transport did not use pushurl correctly, and also did not tell
>    what host it is trying to authenticate with when asking for
>    credentials.
>    (merge deba493 jk/http-auth later to maint).

This seems to break pushing with https for me.
It never uses values from my '~/.netrc'.
I'll come up with a detailed scenario later.

 
Stefan
-- 
----------------------------------------------------------------
/dev/random says: Justice is incidental to law and order.
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-10-31 14:17 ` Stefan Näwe
@ 2011-10-31 17:19   ` Junio C Hamano
  2011-11-01  9:53   ` Stefan Näwe
  1 sibling, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2011-10-31 17:19 UTC (permalink / raw)
  To: Stefan Näwe; +Cc: git@vger.kernel.org, Linux Kernel

Stefan Näwe <stefan.naewe@atlas-elektronik.com> writes:

>>  * HTTP transport did not use pushurl correctly, and also did not tell
>>    what host it is trying to authenticate with when asking for
>>    credentials.
>>    (merge deba493 jk/http-auth later to maint).
>
> This seems to break pushing with https for me.
> It never uses values from my '~/.netrc'.
> I'll come up with a detailed scenario later.

Thanks.

I have been pushing my updates out to code.google.com via authentication
token stored in ~/.netrc over https, so it would be nice to see what
breaks for you that works for me. There probably is something subtly
different.

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-10-31 14:17 ` Stefan Näwe
  2011-10-31 17:19   ` Junio C Hamano
@ 2011-11-01  9:53   ` Stefan Näwe
  2011-11-01 18:12     ` Junio C Hamano
  1 sibling, 1 reply; 26+ messages in thread
From: Stefan Näwe @ 2011-11-01  9:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, Linux Kernel

Am 31.10.2011 15:17, schrieb Stefan Näwe:
> Am 31.10.2011 06:00, schrieb Junio C Hamano:
>> A release candidate Git 1.7.8.rc0 is available for testing.
>>
>> [...]
>>
>>
>> Git v1.7.8 Release Notes (draft)
>> ================================
>>
>> Updates since v1.7.7
>> --------------------
>>
>> [...]
>>
>>  * HTTP transport did not use pushurl correctly, and also did not tell
>>    what host it is trying to authenticate with when asking for
>>    credentials.
>>    (merge deba493 jk/http-auth later to maint).
> 
> This seems to break pushing with https for me.
> It never uses values from my '~/.netrc'.
> I'll come up with a detailed scenario later.

Update:

git push prompts for the password but just pressing return succeeds.

Weird...


Stefan
-- 
----------------------------------------------------------------
/dev/random says: Budget: A method for going broke methodically.
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-01  9:53   ` Stefan Näwe
@ 2011-11-01 18:12     ` Junio C Hamano
  2011-11-01 18:19       ` Jeff King
                         ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Junio C Hamano @ 2011-11-01 18:12 UTC (permalink / raw)
  To: Stefan Näwe; +Cc: git@vger.kernel.org, Jeff King

[administrivia: dropped the kernel mailing list from and added Peff to Cc]

Stefan Näwe <stefan.naewe@atlas-elektronik.com> writes:

>>>  * HTTP transport did not use pushurl correctly, and also did not tell
>>>    what host it is trying to authenticate with when asking for
>>>    credentials.
>>>    (merge deba493 jk/http-auth later to maint).
>> 
>> This seems to break pushing with https for me.
>> It never uses values from my '~/.netrc'.
>> I'll come up with a detailed scenario later.
>
> Update:
>
> git push prompts for the password but just pressing return succeeds.
>
> Weird...

There are only handful of commits that even remotely touch http related
codepath between v1.7.7 and v1.7.8-rc0:

  * deba493 http_init: accept separate URL parameter

  This could change the URL string given to http_auth_init().

  * 070b4dd http: use hostname in credential description

  This only changes the prompt string; as far as I understand it, the
  condition the password is prompted in the callsites of git_getpass()
  has not changed.

  * 6cdf022 remote-curl: Fix warning after HTTP failure
  * be22d92 http: avoid empty error messages for some curl errors
  * 8abc508 http: remove extra newline in error message
  * 8d677ed http: retry authentication failures for all http requests
  * 28d0c10 remote-curl: don't retry auth failures with dumb protocol

  These shouldn't affect anything wrt prompting, unless you are somehow
  internally reauthenticating.

Could you try reverting deba493 and retest, and then if the behaviour is
the same "need ENTER", further revert 070b4dd and retest?

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-01 18:12     ` Junio C Hamano
@ 2011-11-01 18:19       ` Jeff King
  2011-11-01 20:06       ` Stefan Naewe
  2011-11-02  8:52       ` [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0) Stefan Näwe
  2 siblings, 0 replies; 26+ messages in thread
From: Jeff King @ 2011-11-01 18:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stefan Näwe, git@vger.kernel.org

On Tue, Nov 01, 2011 at 11:12:49AM -0700, Junio C Hamano wrote:

> Stefan Näwe <stefan.naewe@atlas-elektronik.com> writes:
> 
> >>>  * HTTP transport did not use pushurl correctly, and also did not tell
> >>>    what host it is trying to authenticate with when asking for
> >>>    credentials.
> >>>    (merge deba493 jk/http-auth later to maint).
> >> 
> >> This seems to break pushing with https for me.
> >> It never uses values from my '~/.netrc'.
> >> I'll come up with a detailed scenario later.
> >
> > Update:
> >
> > git push prompts for the password but just pressing return succeeds.
> >
> > Weird...
> 
> There are only handful of commits that even remotely touch http related
> codepath between v1.7.7 and v1.7.8-rc0:
> 
>   * deba493 http_init: accept separate URL parameter
> 
>   This could change the URL string given to http_auth_init().
> 
>   * 070b4dd http: use hostname in credential description
> 
>   This only changes the prompt string; as far as I understand it, the
>   condition the password is prompted in the callsites of git_getpass()
>   has not changed.
> 
>   * 6cdf022 remote-curl: Fix warning after HTTP failure
>   * be22d92 http: avoid empty error messages for some curl errors
>   * 8abc508 http: remove extra newline in error message
>   * 8d677ed http: retry authentication failures for all http requests
>   * 28d0c10 remote-curl: don't retry auth failures with dumb protocol
> 
>   These shouldn't affect anything wrt prompting, unless you are somehow
>   internally reauthenticating.
> 
> Could you try reverting deba493 and retest, and then if the behaviour is
> the same "need ENTER", further revert 070b4dd and retest?

I don't use .netrc, but with all of my patches (most of which aren't
even in what you are running), I tried not to affect the netrc case. I
just checked a few things, and it seems to be working as I expect. Do we
have a repeatable test?

-Peff

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-01 18:12     ` Junio C Hamano
  2011-11-01 18:19       ` Jeff King
@ 2011-11-01 20:06       ` Stefan Naewe
  2011-11-01 20:18         ` Stefan Naewe
  2011-11-01 21:53         ` Stefan Naewe
  2011-11-02  8:52       ` [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0) Stefan Näwe
  2 siblings, 2 replies; 26+ messages in thread
From: Stefan Naewe @ 2011-11-01 20:06 UTC (permalink / raw)
  To: git

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

> 
> [administrivia: dropped the kernel mailing list from and added Peff to Cc]
> 
> Stefan Näwe <stefan.naewe <at> atlas-elektronik.com> writes:
> 
> >>>  * HTTP transport did not use pushurl correctly, and also did not tell
> >>>    what host it is trying to authenticate with when asking for
> >>>    credentials.
> >>>    (merge deba493 jk/http-auth later to maint).
> >> 
> >> This seems to break pushing with https for me.
> >> It never uses values from my '~/.netrc'.
> >> I'll come up with a detailed scenario later.
> >
> > Update:
> >
> > git push prompts for the password but just pressing return succeeds.
> >
> > Weird...
> 
> There are only handful of commits that even remotely touch http related
> codepath between v1.7.7 and v1.7.8-rc0:
> 
> [...]
> 
> Could you try reverting deba493 and retest, and then if the behaviour is
> the same "need ENTER", further revert 070b4dd and retest?

Did some tests again at my home machine with v1.7.8-rc0.

Push with https works, if the URL looks e.g. like this:

  https://github.com/user/repo.git

rather than this

  https://user@github.com/user/repo.git

and having a ~/.netrc like this

  machine github.com login user password YouDontWantToKnow

If the URL contains 'user@' I get the 'need ENTER' behaviour.

I'll recheck everything at work, where I live behind a very restrictive
firewall (Don't know if that makes any difference).

Regards,
  Stefan

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-01 20:06       ` Stefan Naewe
@ 2011-11-01 20:18         ` Stefan Naewe
  2011-11-02 10:27           ` Michael J Gruber
  2011-11-02 18:03           ` Jeff King
  2011-11-01 21:53         ` Stefan Naewe
  1 sibling, 2 replies; 26+ messages in thread
From: Stefan Naewe @ 2011-11-01 20:18 UTC (permalink / raw)
  To: git

Stefan Naewe <stefan.naewe <at> gmail.com> writes:

> Push with https works, if the URL looks e.g. like this:
> 
>   https://github.com/user/repo.git
> 
> rather than this
> 
>   https://user <at> github.com/user/repo.git
> 
> and having a ~/.netrc like this
> 
>   machine github.com login user password YouDontWantToKnow
> 
> If the URL contains 'user@' I get the 'need ENTER' behaviour.
> 

Another update:

If I revert deba493 the 'need ENTER' is gone and everything works as above.


Stefan

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-01 20:06       ` Stefan Naewe
  2011-11-01 20:18         ` Stefan Naewe
@ 2011-11-01 21:53         ` Stefan Naewe
  1 sibling, 0 replies; 26+ messages in thread
From: Stefan Naewe @ 2011-11-01 21:53 UTC (permalink / raw)
  To: git

Stefan Naewe <stefan.naewe <at> gmail.com> writes:
> I'll recheck everything at work, where I live behind a very restrictive
> firewall (Don't know if that makes any difference).

s/firewall/proxy/

Stefan

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

* [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0)
  2011-11-01 18:12     ` Junio C Hamano
  2011-11-01 18:19       ` Jeff King
  2011-11-01 20:06       ` Stefan Naewe
@ 2011-11-02  8:52       ` Stefan Näwe
  2011-11-02 14:08         ` Michael J Gruber
  2011-11-02 17:13         ` [RFC/PATCH] http-push: don't always prompt for password Junio C Hamano
  2 siblings, 2 replies; 26+ messages in thread
From: Stefan Näwe @ 2011-11-02  8:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, Jeff King

Am 01.11.2011 19:12, schrieb Junio C Hamano:
> 
> There are only handful of commits that even remotely touch http related
> codepath between v1.7.7 and v1.7.8-rc0:
> 
>   * deba493 http_init: accept separate URL parameter
> 
>   This could change the URL string given to http_auth_init().
> 
>   * 070b4dd http: use hostname in credential description
> 
>   This only changes the prompt string; as far as I understand it, the
>   condition the password is prompted in the callsites of git_getpass()
>   has not changed.
> 
>   * 6cdf022 remote-curl: Fix warning after HTTP failure
>   * be22d92 http: avoid empty error messages for some curl errors
>   * 8abc508 http: remove extra newline in error message
>   * 8d677ed http: retry authentication failures for all http requests
>   * 28d0c10 remote-curl: don't retry auth failures with dumb protocol
> 
>   These shouldn't affect anything wrt prompting, unless you are somehow
>   internally reauthenticating.
> 
> Could you try reverting deba493 and retest, and then if the behaviour is
> the same "need ENTER", further revert 070b4dd and retest?

I did a little more testing.
This WIP makes it work for me (i.e. "need ENTER" is gone, works with
and without .netrc, with 'https://host/repo.git' and 
'https://user@host...' URL). Needs testing, of course.

---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
diff --git a/http.c b/http.c
index a4bc770..008ad72 100644
--- a/http.c
+++ b/http.c
@@ -279,8 +279,6 @@ static CURL *get_curl_handle(void)
        curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif

-       init_curl_http_auth(result);
-
        if (ssl_cert != NULL)
                curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
        if (has_cert_password())
@@ -846,7 +844,7 @@ static int http_request(const char *url, void *result, int target, int options)
                else if (missing_target(&results))
                        ret = HTTP_MISSING_TARGET;
                else if (results.http_code == 401) {
-                       if (user_name) {
+                       if (user_name && user_pass) {
                                ret = HTTP_NOAUTH;
                        } else {
                                /*
@@ -855,7 +853,8 @@ static int http_request(const char *url, void *result, int target, int options)
                                 * but that is non-portable.  Using git_getpass() can at least be stubbed
                                 * on other platforms with a different implementation if/when necessary.
                                 */
-                               user_name = xstrdup(git_getpass_with_description("Username", description));
+                               if (!user_name)
+                                       user_name = xstrdup(git_getpass_with_description("Username", description));
                                init_curl_http_auth(slot->curl);
                                ret = HTTP_REAUTH;
                        }
---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---


Regards,
  Stefan
-- 
----------------------------------------------------------------
/dev/random says: Efficiency takes time! Frugality: who can afford it?
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-01 20:18         ` Stefan Naewe
@ 2011-11-02 10:27           ` Michael J Gruber
  2011-11-02 18:03           ` Jeff King
  1 sibling, 0 replies; 26+ messages in thread
From: Michael J Gruber @ 2011-11-02 10:27 UTC (permalink / raw)
  To: Stefan Naewe; +Cc: git, Junio C Hamano, Jeff King

[Re-adding cc's]

Stefan Naewe venit, vidit, dixit 01.11.2011 21:18:
> Stefan Naewe <stefan.naewe <at> gmail.com> writes:
> 
>> Push with https works, if the URL looks e.g. like this:
>>
>>   https://github.com/user/repo.git
>>
>> rather than this
>>
>>   https://user <at> github.com/user/repo.git
>>
>> and having a ~/.netrc like this
>>
>>   machine github.com login user password YouDontWantToKnow
>>
>> If the URL contains 'user@' I get the 'need ENTER' behaviour.
>>
> 
> Another update:
> 
> If I revert deba493 the 'need ENTER' is gone and everything works as above.
> 
> 
> Stefan
> 
I can confirm that (and feel partly responsible given the history of of
deba493). For the record: A simple test looks like

SSH_ASKPASS='' git push -n bitbucket
Password for 'bitbucket.org':

which succeeds with a simple ENTER when you have the (log and) PW in
.netrc for that host, and your config says https://user@host.

The workaround is to remove 'user@' from the url in gitconfig, it is not
needed nor used, probably: I haven't checked yet, but that would mean we
can't have two different logins on the same server in .netrc. Can we?

I'll try to have a look later, too.

Michael

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

* Re: [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0)
  2011-11-02  8:52       ` [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0) Stefan Näwe
@ 2011-11-02 14:08         ` Michael J Gruber
  2011-11-02 17:13         ` [RFC/PATCH] http-push: don't always prompt for password Junio C Hamano
  1 sibling, 0 replies; 26+ messages in thread
From: Michael J Gruber @ 2011-11-02 14:08 UTC (permalink / raw)
  To: Stefan Näwe; +Cc: Junio C Hamano, git@vger.kernel.org, Jeff King

Stefan Näwe venit, vidit, dixit 02.11.2011 09:52:
> Am 01.11.2011 19:12, schrieb Junio C Hamano:
>>
>> There are only handful of commits that even remotely touch http related
>> codepath between v1.7.7 and v1.7.8-rc0:
>>
>>   * deba493 http_init: accept separate URL parameter
>>
>>   This could change the URL string given to http_auth_init().
>>
>>   * 070b4dd http: use hostname in credential description
>>
>>   This only changes the prompt string; as far as I understand it, the
>>   condition the password is prompted in the callsites of git_getpass()
>>   has not changed.
>>
>>   * 6cdf022 remote-curl: Fix warning after HTTP failure
>>   * be22d92 http: avoid empty error messages for some curl errors
>>   * 8abc508 http: remove extra newline in error message
>>   * 8d677ed http: retry authentication failures for all http requests
>>   * 28d0c10 remote-curl: don't retry auth failures with dumb protocol
>>
>>   These shouldn't affect anything wrt prompting, unless you are somehow
>>   internally reauthenticating.
>>
>> Could you try reverting deba493 and retest, and then if the behaviour is
>> the same "need ENTER", further revert 070b4dd and retest?
> 
> I did a little more testing.
> This WIP makes it work for me (i.e. "need ENTER" is gone, works with
> and without .netrc, with 'https://host/repo.git' and 
> 'https://user@host...' URL). Needs testing, of course.
> 
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
> diff --git a/http.c b/http.c
> index a4bc770..008ad72 100644
> --- a/http.c
> +++ b/http.c
> @@ -279,8 +279,6 @@ static CURL *get_curl_handle(void)
>         curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
>  #endif
> 
> -       init_curl_http_auth(result);
> -
>         if (ssl_cert != NULL)
>                 curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
>         if (has_cert_password())
> @@ -846,7 +844,7 @@ static int http_request(const char *url, void *result, int target, int options)
>                 else if (missing_target(&results))
>                         ret = HTTP_MISSING_TARGET;
>                 else if (results.http_code == 401) {
> -                       if (user_name) {
> +                       if (user_name && user_pass) {
>                                 ret = HTTP_NOAUTH;
>                         } else {
>                                 /*
> @@ -855,7 +853,8 @@ static int http_request(const char *url, void *result, int target, int options)
>                                  * but that is non-portable.  Using git_getpass() can at least be stubbed
>                                  * on other platforms with a different implementation if/when necessary.
>                                  */
> -                               user_name = xstrdup(git_getpass_with_description("Username", description));
> +                               if (!user_name)
> +                                       user_name = xstrdup(git_getpass_with_description("Username", description));
>                                 init_curl_http_auth(slot->curl);
>                                 ret = HTTP_REAUTH;
>                         }
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
> 
> 
> Regards,
>   Stefan
Thanks!

Tested-by: Michael J Gruber <git@drmicha.warpmail.net>

More specifically, I ran our test suite (next plus Stefan's patch), and
tested

https://user@host with .netrc and with askpass
https://host with .netrc

The latter fails with askpass because we ask
Password for 'host'
and not
Password for 'user@host'
but that is true both with and without the patch. (I thought we had
changed that, but I guess it's cooking.)

Michael

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

* Re: [RFC/PATCH] http-push: don't always prompt for password
  2011-11-02  8:52       ` [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0) Stefan Näwe
  2011-11-02 14:08         ` Michael J Gruber
@ 2011-11-02 17:13         ` Junio C Hamano
  2011-11-02 17:23           ` Jeff King
  1 sibling, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2011-11-02 17:13 UTC (permalink / raw)
  To: Stefan Näwe; +Cc: git@vger.kernel.org, Jeff King

Stefan Näwe <stefan.naewe@atlas-elektronik.com> writes:

> Am 01.11.2011 19:12, schrieb Junio C Hamano:
>> 
>> There are only handful of commits that even remotely touch http related
>> codepath between v1.7.7 and v1.7.8-rc0:
>> 
>>   * deba493 http_init: accept separate URL parameter
>> 
>>   This could change the URL string given to http_auth_init().
>> ... 
>> Could you try reverting deba493 and retest, and then if the behaviour is
>> the same "need ENTER", further revert 070b4dd and retest?
>
> I did a little more testing.
> This WIP makes it work for me (i.e. "need ENTER" is gone, works with
> and without .netrc, with 'https://host/repo.git' and 
> 'https://user@host...' URL). Needs testing, of course.
>
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
> diff --git a/http.c b/http.c
> index a4bc770..008ad72 100644
> --- a/http.c
> +++ b/http.c
> @@ -279,8 +279,6 @@ static CURL *get_curl_handle(void)
>         curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
>  #endif
>
> -       init_curl_http_auth(result);
> -
>         if (ssl_cert != NULL)
>                 curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
>         if (has_cert_password())
> @@ -846,7 +844,7 @@ static int http_request(const char *url, void *result, int target, int options)
>                 else if (missing_target(&results))
>                         ret = HTTP_MISSING_TARGET;
>                 else if (results.http_code == 401) {
> -                       if (user_name) {
> +                       if (user_name && user_pass) {
>                                 ret = HTTP_NOAUTH;
>                         } else {
>                                 /*
> @@ -855,7 +853,8 @@ static int http_request(const char *url, void *result, int target, int options)
>                                  * but that is non-portable.  Using git_getpass() can at least be stubbed
>                                  * on other platforms with a different implementation if/when necessary.
>                                  */
> -                               user_name = xstrdup(git_getpass_with_description("Username", description));
> +                               if (!user_name)
> +                                       user_name = xstrdup(git_getpass_with_description("Username", description));
>                                 init_curl_http_auth(slot->curl);
>                                 ret = HTTP_REAUTH;
>                         }
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---

This defers the calls to git_getpass* until we get 401 from the server
side.

I am guessing the reason why in the current code get_curl_handle() has a
call to init_curl_http_auth() very early, but only when user_name is set,
is because it is likely for a site to require authentication when the user
already has "username@" in its URL, and doing it this way will avoid the
extra round-trip because by the time we make an HTTP request, we have both
name and pass. If we apply this patch, the check in init_curl_http_auth()
that asks for the password only when user_name is set becomes unnecessary.

I think the second hunk at l.846 sort of makes sense, but not quite.

"We got 401 even though we know we have supplied name and pass" is a valid
criterion to decide that the name/pass is an invalid combination. But it
makes me wonder if this code in its early days guaranteed whenever we have
user_name we always have made sure we have user_pass (otherwise by asking
for it with git_getpass) and that is the reason why it had to check only
for user_name, and if that is the case perhaps the real breakage is we are
not keeping that guarantee in the current code?

IOW, when in the current code do we make a new HTTP request while
user_name is set but no user_pass?  Perhaps if we fix that codepath we do
not have to have this extra 401 roundtrip this patch is introducing when
the username (but not password) is given?

Peff, what do you think?

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

* Re: [RFC/PATCH] http-push: don't always prompt for password
  2011-11-02 17:13         ` [RFC/PATCH] http-push: don't always prompt for password Junio C Hamano
@ 2011-11-02 17:23           ` Jeff King
  2011-11-02 17:40             ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Jeff King @ 2011-11-02 17:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stefan Näwe, git@vger.kernel.org

On Wed, Nov 02, 2011 at 10:13:32AM -0700, Junio C Hamano wrote:

> This defers the calls to git_getpass* until we get 401 from the server
> side.
> 
> I am guessing the reason why in the current code get_curl_handle() has a
> call to init_curl_http_auth() very early, but only when user_name is set,
> is because it is likely for a site to require authentication when the user
> already has "username@" in its URL, and doing it this way will avoid the
> extra round-trip because by the time we make an HTTP request, we have both
> name and pass. If we apply this patch, the check in init_curl_http_auth()
> that asks for the password only when user_name is set becomes unnecessary.

Yeah, that was my reading, as well. I was tempted to do away with it, as
it makes the code much simpler, at the cost of doing that extra
round-trip. However, most browsers do that round-trip, and I don't think
it's that big a deal.

In the end I decided not to switch it just because I wanted to be as
minimally invasive as possible, just in case somebody did care about the
round trip.

> I think the second hunk at l.846 sort of makes sense, but not quite.
> 
> "We got 401 even though we know we have supplied name and pass" is a valid
> criterion to decide that the name/pass is an invalid combination. But it
> makes me wonder if this code in its early days guaranteed whenever we have
> user_name we always have made sure we have user_pass (otherwise by asking
> for it with git_getpass) and that is the reason why it had to check only
> for user_name, and if that is the case perhaps the real breakage is we are
> not keeping that guarantee in the current code?

All of my patches attempted to keep that condition, as well (because
credential_fill tries to do so). But I didn't think about it during the
re-roll of the patches that are in master now, so maybe that wasn't
kept.

It seems to me that Stefan's patch actually causes that (because he
removes the early "set password if we have a username" logic). But I'll
take another look at what's in master.

-Peff

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

* Re: [RFC/PATCH] http-push: don't always prompt for password
  2011-11-02 17:23           ` Jeff King
@ 2011-11-02 17:40             ` Junio C Hamano
  2011-11-04  7:03               ` [PATCH] " Stefan Naewe
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2011-11-02 17:40 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Stefan Näwe, git@vger.kernel.org

Jeff King <peff@peff.net> writes:

> It seems to me that Stefan's patch actually causes that (because he
> removes the early "set password if we have a username" logic). But I'll
> take another look at what's in master.

Thanks.

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-01 20:18         ` Stefan Naewe
  2011-11-02 10:27           ` Michael J Gruber
@ 2011-11-02 18:03           ` Jeff King
  2011-11-02 18:10             ` Jeff King
  1 sibling, 1 reply; 26+ messages in thread
From: Jeff King @ 2011-11-02 18:03 UTC (permalink / raw)
  To: Stefan Naewe; +Cc: Junio C Hamano, Michael J Gruber, git

On Tue, Nov 01, 2011 at 08:18:47PM +0000, Stefan Naewe wrote:

> Stefan Naewe <stefan.naewe <at> gmail.com> writes:
> 
> > Push with https works, if the URL looks e.g. like this:
> > 
> >   https://github.com/user/repo.git
> > 
> > rather than this
> > 
> >   https://user <at> github.com/user/repo.git
> > 
> > and having a ~/.netrc like this
> > 
> >   machine github.com login user password YouDontWantToKnow
> > 
> > If the URL contains 'user@' I get the 'need ENTER' behaviour.
> > 
> 
> Another update:
> 
> If I revert deba493 the 'need ENTER' is gone and everything works as above.

I think this is a false positive. The problem that deba493 fixes is that
"git push" was not properly looking at the push URL, but rather pulling
the initial auth information from the fetch URL. So I suspect your
finding the bug there or not may have to do with it actually respecting
your config properly.

I think the bug is much older than this, and we are just exposing it by
finally using the correct URL.

Without using a configured remote, try this (with .netrc configured):

  git push https://github.com/user/repo.git :refs/heads/nothing

which should work, and then this:

  git push https://user@github.com/user/repo.git :refs/heads/nothing

which will do the "must hit enter to accept password" thing.

That fails even with v1.7.7. I didn't bisect, but it has been there
quite a while (v1.6.6 has it, but v1.6.5 has a weird error, so I didn't
bisect further).

-Peff

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-02 18:03           ` Jeff King
@ 2011-11-02 18:10             ` Jeff King
  2011-11-02 19:13               ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Jeff King @ 2011-11-02 18:10 UTC (permalink / raw)
  To: Stefan Naewe; +Cc: Junio C Hamano, Michael J Gruber, git

On Wed, Nov 02, 2011 at 02:03:27PM -0400, Jeff King wrote:

> Without using a configured remote, try this (with .netrc configured):
> 
>   git push https://github.com/user/repo.git :refs/heads/nothing
> 
> which should work, and then this:
> 
>   git push https://user@github.com/user/repo.git :refs/heads/nothing
> 
> which will do the "must hit enter to accept password" thing.
> 
> That fails even with v1.7.7. I didn't bisect, but it has been there
> quite a while (v1.6.6 has it, but v1.6.5 has a weird error, so I didn't
> bisect further).

OK, I see the issue.

The logic is "if we have a username, but not a password, then ask for
the password before trying any http" (this is what avoids the extra
round trip).

But if you are using netrc, we don't parse it ourselves. We just tell
curl "when you are making the request, check netrc, too".

So the ideal logic is:

  1. look in netrc

  2. If we have a username and no password, ask for password

  3. Otherwise, try it and see if we get a 401.

But we can't do that, because (1) and (3) happen atomically inside of
curl.

The simplest thing is to just drop the behavior in (2), and let it drop
to a 401. The extra round trip probably isn't that big a deal.

The other option is to start parsing netrc ourselves, or do the extra
round trip if we detect ~/.netrc or something. But that last one is
getting pretty hackish.

-Peff

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-02 18:10             ` Jeff King
@ 2011-11-02 19:13               ` Junio C Hamano
  2011-11-02 20:09                 ` Jeff King
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2011-11-02 19:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Stefan Naewe, Michael J Gruber, git

Jeff King <peff@peff.net> writes:

> So the ideal logic is:
>
>   1. look in netrc
>
>   2. If we have a username and no password, ask for password
>
>   3. Otherwise, try it and see if we get a 401.
>
> But we can't do that, because (1) and (3) happen atomically inside of
> curl.
>
> The simplest thing is to just drop the behavior in (2), and let it drop
> to a 401. The extra round trip probably isn't that big a deal.

That is essentially what Stefan's fix is about.

The cases we have "extra" roundtrip are:

 - when you have username@ in URL but no password is stored in .netrc;
 - when you have username@ in URL and no $HOME/.netrc file.

and in such a case using URL without username@ in it as a workaround would
save the roundtrip but forces you to type your username@ over and over
again, which is _not_ a real workaround.

A workaround for people who want ultimate convenience is to use .netrc to
have both username:password, but that is at the cost of potentially
reduced security. Having username@ in URL and typing password
interactively, if it worked properly, would have been the best of both
worlds.

> The other option is to start parsing netrc ourselves, or do the extra
> round trip if we detect ~/.netrc or something. But that last one is
> getting pretty hackish.

I tend to agree that we wouldn't want to parse netrc ourselves (that is
what library support e.g. CURLOPT_NETRC is for). The latter is hackish but
on the other hand it is a cheap, simple and useful hack.

How would the upcoming keystore support fit in this picture, by the way?

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-02 19:13               ` Junio C Hamano
@ 2011-11-02 20:09                 ` Jeff King
  2011-11-03 23:02                   ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Jeff King @ 2011-11-02 20:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stefan Naewe, Michael J Gruber, git

On Wed, Nov 02, 2011 at 12:13:48PM -0700, Junio C Hamano wrote:

> > The simplest thing is to just drop the behavior in (2), and let it drop
> > to a 401. The extra round trip probably isn't that big a deal.
> 
> That is essentially what Stefan's fix is about.

Right. I think it may be the sanest thing to do.

> The cases we have "extra" roundtrip are:
> 
>  - when you have username@ in URL but no password is stored in .netrc;
>  - when you have username@ in URL and no $HOME/.netrc file.
> 
> and in such a case using URL without username@ in it as a workaround would
> save the roundtrip but forces you to type your username@ over and over
> again, which is _not_ a real workaround.

Yeah. There's no way for us to know before we hand off to curl what you
have in netrc. So these netrc cases will always be at odds with the
no-netrc case.

Normally I would say to implement in favor of the no-netrc case, as it
is probably more common (and will hopefully be more so after the auth
helpers are finished). But the problem is that the penalties are
different. On the one hand, we have the extra http round-trip. Which is
annoying, but mostly invisible to the user. But on the other, we have
git prompting the user unnecessarily, which is just awful.

> > The other option is to start parsing netrc ourselves, or do the extra
> > round trip if we detect ~/.netrc or something. But that last one is
> > getting pretty hackish.
> 
> I tend to agree that we wouldn't want to parse netrc ourselves (that is
> what library support e.g. CURLOPT_NETRC is for). The latter is hackish but
> on the other hand it is a cheap, simple and useful hack.

Note that it's not always right, of course. You might have a .netrc but
no entry for that host. But at least it lets the common case people
(i.e., people who never heard of or touched netrc) to avoid the round
trip.

> How would the upcoming keystore support fit in this picture, by the way?

Any time we would call getpass(), we ask the helper for the credential.
So for user@host, we would call out to the helper for the password
proactively, and otherwise wait for a 401.

We _could_ be proactive and actually ask the helpers for a username and
password even for "https://host/repo", which would save a round-trip to
get the 401 in some cases. But that assumes that asking the helper is
cheap. It might actually require the user inputting a password to unlock
the keystore, which would be annoying if the remote doesn't require
auth at all.

We could try to be clever and use a heuristic that fetch probably
doesn't need auth, but push does. Then fetch gets the extra round-trip
but push doesn't. But that just seems needlessly complex to save one
http round-trip on push.

-Peff

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

* Re: [ANNOUNCE] Git 1.7.8.rc0
  2011-11-02 20:09                 ` Jeff King
@ 2011-11-03 23:02                   ` Junio C Hamano
  0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2011-11-03 23:02 UTC (permalink / raw)
  To: Stefan Naewe, Jeff King; +Cc: Michael J Gruber, git

Jeff King <peff@peff.net> writes:

> Normally I would say to implement in favor of the no-netrc case, as it
> is probably more common (and will hopefully be more so after the auth
> helpers are finished). But the problem is that the penalties are
> different. On the one hand, we have the extra http round-trip. Which is
> annoying, but mostly invisible to the user. But on the other, we have
> git prompting the user unnecessarily, which is just awful.

Ok, so are we in agreement that Stefan's patch $gmane/184617 is the right
fix at least for the time being?

This will be a minor regression if left unfixed, so I'd like to have a
minimum fix in before I tag -rc1 over the weekend.

Could any one of you guys please care to package it up with a readable
commit log message with a sign-off?

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

* [PATCH] http-push: don't always prompt for password
  2011-11-02 17:40             ` Junio C Hamano
@ 2011-11-04  7:03               ` Stefan Naewe
  2011-11-04 16:48                 ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Stefan Naewe @ 2011-11-04  7:03 UTC (permalink / raw)
  To: gitster, git, peff; +Cc: Stefan Naewe

http-push prompts for a password when the URL is set as
'https://user@host/repo' even though there is one set
in ~/.netrc. Pressing ENTER at the password prompt succeeds
then, but is a annoying and makes it almost useless
in a shell script, e.g.

Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
---
 http.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/http.c b/http.c
index a4bc770..008ad72 100644
--- a/http.c
+++ b/http.c
@@ -279,8 +279,6 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
-	init_curl_http_auth(result);
-
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
@@ -846,7 +844,7 @@ static int http_request(const char *url, void *result, int target, int options)
 		else if (missing_target(&results))
 			ret = HTTP_MISSING_TARGET;
 		else if (results.http_code == 401) {
-			if (user_name) {
+			if (user_name && user_pass) {
 				ret = HTTP_NOAUTH;
 			} else {
 				/*
@@ -855,7 +853,8 @@ static int http_request(const char *url, void *result, int target, int options)
 				 * but that is non-portable.  Using git_getpass() can at least be stubbed
 				 * on other platforms with a different implementation if/when necessary.
 				 */
-				user_name = xstrdup(git_getpass_with_description("Username", description));
+				if (!user_name)
+					user_name = xstrdup(git_getpass_with_description("Username", description));
 				init_curl_http_auth(slot->curl);
 				ret = HTTP_REAUTH;
 			}
-- 
1.7.8.rc0.1.gb345ae

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

* Re: [PATCH] http-push: don't always prompt for password
  2011-11-04  7:03               ` [PATCH] " Stefan Naewe
@ 2011-11-04 16:48                 ` Junio C Hamano
  2011-11-04 17:43                   ` Jeff King
  2011-11-04 18:34                   ` Stefan Naewe
  0 siblings, 2 replies; 26+ messages in thread
From: Junio C Hamano @ 2011-11-04 16:48 UTC (permalink / raw)
  To: Stefan Naewe; +Cc: git, peff

Stefan Naewe <stefan.naewe@gmail.com> writes:

> http-push prompts for a password when the URL is set as
> 'https://user@host/repo' even though there is one set
> in ~/.netrc. Pressing ENTER at the password prompt succeeds
> then, but is a annoying and makes it almost useless
> in a shell script, e.g.
>
> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
> ---

Thanks.

With this the only callsite of init_curl_http_auth() becomes the one after
we get the 401 response, and this caller makes sure that user_name is not
NULL.

Do we still want "if (user_name)" inside init_curl_http_auth()?

I tried to rewrite the proposed commit log message to describe the real
issue, and here is what I came up with:

Author: Stefan Naewe <stefan.naewe@gmail.com>
Date:   Fri Nov 4 08:03:08 2011 +0100

    http: don't always prompt for password
    
    When a username is already specified at the beginning of any HTTP
    transaction (e.g. "git push https://user@hosting.example.com/project.git"
    or "git ls-remote https://user@hosting.example.com/project.git"), the code
    interactively asks for a password before calling into the libcurl library.
    It is very likely that the reason why user included the username in the
    URL is because the user knows that it would require authentication to
    access the resource. Asking for the password upfront would save one
    roundtrip to get a 401 response, getting the password and then retrying
    the request. This is a reasonable optimization.
    
    HOWEVER.
    
    This is done even when $HOME/.netrc might have a corresponding entry to
    access the site, or the site does not require authentication to access the
    resource after all. But neither condition can be determined until we call
    into libcurl library (we do not read and parse $HOME/.netrc ourselves). In
    these cases, the user is forced to respond to the password prompt, only to
    give a password that is not used in the HTTP transaction. If the password
    is in $HOME/.netrc, an empty input would later let the libcurl layer to
    pick up the password from there, and if the resource does not require
    authentication, any input would be taken and then discarded without
    getting used. It is wasteful to ask this unused information to the end
    user.
    
    Reduce the confusion by not trying to optimize for this case and always
    incur roundtrip penalty. An alternative might be to document this and keep
    this round-trip optimization as-is.
    
    Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
    Helped-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

What is somewhat troubling is that after analyzing the root cause of the
issue, I am wondering if a more correct fix is to remove the user@ part
from the URL (in other words, document that a URL with an embedded
username will ask for password upfront, and tell the users that if they
have netrc entries or if they are accessing a resource that does not
require authentication, they should omit the username from the URL).

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

* Re: [PATCH] http-push: don't always prompt for password
  2011-11-04 16:48                 ` Junio C Hamano
@ 2011-11-04 17:43                   ` Jeff King
  2011-11-04 19:06                     ` Junio C Hamano
  2011-11-04 18:34                   ` Stefan Naewe
  1 sibling, 1 reply; 26+ messages in thread
From: Jeff King @ 2011-11-04 17:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stefan Naewe, git

On Fri, Nov 04, 2011 at 09:48:17AM -0700, Junio C Hamano wrote:

> Stefan Naewe <stefan.naewe@gmail.com> writes:
> 
> > http-push prompts for a password when the URL is set as
> > 'https://user@host/repo' even though there is one set
> > in ~/.netrc. Pressing ENTER at the password prompt succeeds
> > then, but is a annoying and makes it almost useless
> > in a shell script, e.g.
> >
> > Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
> > ---
> 
> Thanks.
> 
> With this the only callsite of init_curl_http_auth() becomes the one after
> we get the 401 response, and this caller makes sure that user_name is not
> NULL.
> 
> Do we still want "if (user_name)" inside init_curl_http_auth()?

Since we now only call init_curl_http_auth when we know we need auth, I
think it would make more sense to just move the user_name asking there,
too, like:

  static void init_curl_http_auth(CURL *result)
  {
          struct strbuf up = STRBUF_INIT;

          if (!user_name)
                  user_name = xstrdup(git_getpass_with_description("Username", description);
          if (!user_pass)
                  user_pass = xstrdup(git_getpass_with_description("Password", description);

          strbuf_addf(&up, "%s:%s", user_name, user_pass);
          curl_easy_setopt(result, CURLOPT_USERPWD, strbuf_detach(&up, NULL));
  }

And then it's easy to swap out the asking for credential_fill() when it
becomes available. But I admit I don't care that much now, as I'll just
end up doing that refactoring later with my credential patches anyway.

> I tried to rewrite the proposed commit log message to describe the real
> issue, and here is what I came up with:

Your description looks accurate to me.

> What is somewhat troubling is that after analyzing the root cause of the
> issue, I am wondering if a more correct fix is to remove the user@ part
> from the URL (in other words, document that a URL with an embedded
> username will ask for password upfront, and tell the users that if they
> have netrc entries or if they are accessing a resource that does not
> require authentication, they should omit the username from the URL).

It's tempting, because the non-netrc case is the common one, and we are
dropping the round-trip avoidance for those people. I'm just not sure
that it's going to be obvious to users that they need to drop the user@
portion from their URL when using netrc. That seems like a bizarre
requirement from the user's POV, even if we do document it.

-Peff

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

* Re: [PATCH] http-push: don't always prompt for password
  2011-11-04 16:48                 ` Junio C Hamano
  2011-11-04 17:43                   ` Jeff King
@ 2011-11-04 18:34                   ` Stefan Naewe
  2011-11-05  6:45                     ` Junio C Hamano
  1 sibling, 1 reply; 26+ messages in thread
From: Stefan Naewe @ 2011-11-04 18:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, peff

On Fri, Nov 4, 2011 at 5:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Naewe <stefan.naewe@gmail.com> writes:
>
>> http-push prompts for a password when the URL is set as
>> 'https://user@host/repo' even though there is one set
>> in ~/.netrc. Pressing ENTER at the password prompt succeeds
>> then, but is a annoying and makes it almost useless
>> in a shell script, e.g.
>>
>> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
>> ---
>
> Thanks.
>
> With this the only callsite of init_curl_http_auth() becomes the one after
> we get the 401 response, and this caller makes sure that user_name is not
> NULL.
>
> Do we still want "if (user_name)" inside init_curl_http_auth()?

Dunno...
I think what Peff says makes sense.

> I tried to rewrite the proposed commit log message to describe the real
> issue, and here is what I came up with:
>
> [...]

Looks good to me.

> What is somewhat troubling is that after analyzing the root cause of the
> issue, I am wondering if a more correct fix is to remove the user@ part
> from the URL (in other words, document that a URL with an embedded
> username will ask for password upfront, and tell the users that if they
> have netrc entries or if they are accessing a resource that does not
> require authentication, they should omit the username from the URL).

Don't get me wrong, but I really don't care.
I just wanted to have that issue fixed, to get my scripted 'multi
repository pull' working
again.

Regards,
  Stefan
-- 
----------------------------------------------------------------
python -c "print '73746566616e2e6e6165776540676d61696c2e636f6d'.decode('hex')"

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

* Re: [PATCH] http-push: don't always prompt for password
  2011-11-04 17:43                   ` Jeff King
@ 2011-11-04 19:06                     ` Junio C Hamano
  0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2011-11-04 19:06 UTC (permalink / raw)
  To: Jeff King; +Cc: Stefan Naewe, git

Jeff King <peff@peff.net> writes:

> Since we now only call init_curl_http_auth when we know we need auth, I
> think it would make more sense to just move the user_name asking there,
> too, like:
>
>   static void init_curl_http_auth(CURL *result)
>   {
>           struct strbuf up = STRBUF_INIT;
>
>           if (!user_name)
>                   user_name = xstrdup(git_getpass_with_description("Username", description);
>           if (!user_pass)
>                   user_pass = xstrdup(git_getpass_with_description("Password", description);
>
>           strbuf_addf(&up, "%s:%s", user_name, user_pass);
>           curl_easy_setopt(result, CURLOPT_USERPWD, strbuf_detach(&up, NULL));
>   }
>
> And then it's easy to swap out the asking for credential_fill() when it
> becomes available. But I admit I don't care that much now, as I'll just
> end up doing that refactoring later with my credential patches anyway.

Yeah, let's not over-churn this part for now as we know it will change
anyway.

Thanks both!

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

* Re: [PATCH] http-push: don't always prompt for password
  2011-11-04 18:34                   ` Stefan Naewe
@ 2011-11-05  6:45                     ` Junio C Hamano
  0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2011-11-05  6:45 UTC (permalink / raw)
  To: Stefan Naewe; +Cc: git, peff, Shawn O. Pearce

Stefan Naewe <stefan.naewe@gmail.com> writes:

> I just wanted to have that issue fixed, to get my scripted 'multi
> repository pull' working
> again.

For the latter, dropping username@ would also be a valid solution; saying
"want to have that issue fixed" is not helping the discussion very much.

Because the change is fairly straightforward, I am tempted to merge this
soon and see if anybody screams (in which case we would revert it by 1.7.8
final). The only people who may possibly scream are people who care about
latencies additional HTTP roundtrip incurs, which might not be a big deal.
We'll find it out and hopefully soon enough ;-)

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

end of thread, other threads:[~2011-11-05  6:45 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-31  5:00 [ANNOUNCE] Git 1.7.8.rc0 Junio C Hamano
2011-10-31 14:17 ` Stefan Näwe
2011-10-31 17:19   ` Junio C Hamano
2011-11-01  9:53   ` Stefan Näwe
2011-11-01 18:12     ` Junio C Hamano
2011-11-01 18:19       ` Jeff King
2011-11-01 20:06       ` Stefan Naewe
2011-11-01 20:18         ` Stefan Naewe
2011-11-02 10:27           ` Michael J Gruber
2011-11-02 18:03           ` Jeff King
2011-11-02 18:10             ` Jeff King
2011-11-02 19:13               ` Junio C Hamano
2011-11-02 20:09                 ` Jeff King
2011-11-03 23:02                   ` Junio C Hamano
2011-11-01 21:53         ` Stefan Naewe
2011-11-02  8:52       ` [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0) Stefan Näwe
2011-11-02 14:08         ` Michael J Gruber
2011-11-02 17:13         ` [RFC/PATCH] http-push: don't always prompt for password Junio C Hamano
2011-11-02 17:23           ` Jeff King
2011-11-02 17:40             ` Junio C Hamano
2011-11-04  7:03               ` [PATCH] " Stefan Naewe
2011-11-04 16:48                 ` Junio C Hamano
2011-11-04 17:43                   ` Jeff King
2011-11-04 19:06                     ` Junio C Hamano
2011-11-04 18:34                   ` Stefan Naewe
2011-11-05  6:45                     ` Junio C Hamano

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