What's cooking in git.git (Jun 2012, #05; Tue, 19)

What's cooking in git.git (Jun 2012, #05; Tue, 19)

[Junio C Hamano]

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

Now 1.7.11 is out, I'll start merging topics that have been cooking
in 'next' to 'master', but let's wait for a few days in case some
brown paper bag bugfixes are needed.

You can find the changes described here in the integration branches of the
repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

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

* cn/cherry-pick-range-docs (2012-06-15) 2 commits
 - git-cherry-pick.txt: clarify the use of revision range notation
 - Documentation: --no-walk is no-op if range is specified

Will merge to 'next' and soon to 'master'.

* jc/sha1-name-more (2012-06-18) 9 commits
 - sha1_name.c: get_describe_name() by definition groks only commits
 - sha1_name.c: teach get_short_sha1() a commit-only option
 - sha1_name.c: allow get_short_sha1() to take other flags
 - sha1_name.c: teach find_short_packed_object() a commit-only option
 - sha1_name.c: teach find_short_object_filename() a commit-only option
 - sha1_name.c: refactor find_short_packed_object()
 - sha1_name.c: rename "now" to "current"
 - sha1_name.c: clarify what "fake" is for in find_short_object_filename()
 - sha1_name.c: indentation fix

Teaches the object name parser that a "git describe" output is
always a commit object, to prolong the lifetime of abbreviated
object name in it.

* jk/version-string-dependency (2012-06-19) 3 commits
 - Makefile: split prefix flags from GIT-CFLAGS
 - Makefile: split GIT_USER_AGENT from GIT-CFLAGS
 - Makefile: apply dependencies consistently to sparse/asm targets
 (this branch uses jk/version-string.)

* jn/perl-makemaker-leading-paths (2012-06-15) 1 commit
 - perl/Makefile: move "mkdir -p" to module installation loop for maintainability

Will wait for a few days to see if somebody comes up with a more
concise and cleaner way to do this, but otherwise this looked good.

* mm/verify-filename-fix (2012-06-18) 2 commits
 - verify_filename(): ask the caller to chose the kind of diagnosis
 - sha1_name: do not trigger detailed diagnosis for file arguments

"git diff COPYING HEAD:COPYING" gave a nonsense error message that
claimed that the treeish HEAD did not have COPYING in it.

Will merge to 'next' and soon to 'master'.

* tr/maint-show-walk (2012-06-19) 2 commits
 - show: fix "range implies walking"
 - Demonstrate git-show is broken with ranges

Fixes "git show"'s auto-walking behaviour, and make it behave just
like "git log" does when it walks.

Note that this is different from Thomas's patch.

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

* db/vcs-svn (2012-06-01) 6 commits
 - vcs-svn: drop no-op reset methods
 - vcs-svn: fix signedness warnings
 - vcs-svn: prefer strstr over memmem
 - vcs-svn: prefer constcmp to prefixcmp
 - vcs-svn: simplify cleanup in apply_one_window()
 - vcs-svn: fix clang-analyzer error

Waiting for Jonathan's clean-up offered earlier.

* vr/use-our-perl-in-tests (2012-06-12) 3 commits
 - t/README: add a bit more Don'ts
 - tests: enclose $PERL_PATH in duoble quotes
 - t: Replace 'perl' by $PERL_PATH

Needs more work.

There are still unconverted use of bare 'perl' remaining in the test
scripts, the second patch needs its title typofixed, and PERL_PATH
needs to be exported to the environment from test-lib.sh.

* jc/apply-3way (2012-06-13) 19 commits
 - apply --3way: tests
 - apply: document --3way option
 - apply: allow rerere() upon --3way results
 - apply: register conflicted stages to the index
 - apply: --3way with add/add conflict
 - apply: move verify_index_match() higher
 - apply: plug the three-way merge logic in
 - apply: fall back on three-way merge
 - apply: accept -3/--3way command line option
 - apply: move "already exists" logic to check_to_create()
 - apply: move check_to_create_blob() closer to its sole caller
 - apply: further split load_preimage()
 - apply: refactor "previous patch" logic
 - apply: split load_preimage() helper function out
 - apply: factor out checkout_target() helper function
 - apply: refactor read_file_or_gitlink()
 - apply: clear_image() clears things a bit more
 - apply: a bit more comments on PATH_TO_BE_DELETED
 - apply: fix an incomplete comment in check_patch()

"git apply" learns to wiggle the base version and perform three-way merge
when a patch does not exactly apply to the version you have.

Waiting for comments.

* nl/http-proxy-more (2012-05-11) 2 commits
 - http: rename HTTP_REAUTH to HTTP_AUTH_RETRY
 - http: Avoid limit of retrying request only twice

I queued only the later two patches from this series, even though they do
not make much sense without the first one that seems to need a bit more
work, so that we won't forget.

Will discard without prejudice, unless rerolled.

* jk/no-op-push-message (2012-05-30) 1 commit
 - improve no-op push output

Rewords the status message of "git push" that pushed only one ref
differently from "Everything up-to-date", to give a bit more help to
people who get the message when their current branch is not pushed.

I had an impression after the discussion thread that a redesign is
coming, but it hasn't happened yet.

Will discard without prejudice, unless rerolled.

* sg/bash-prompt (2012-05-09) 4 commits
 . completion: respect $GIT_DIR
 . completion: use __gitdir() in _git_log()
 - tests: add tests for the bash prompt functions in the completion script
 - tests: move code to run tests under bash into a helper library
 (this branch is tangled with fc/git-prompt-script.)

This is only the "correction" bits taken from the beginning of a
larger series that is to be rerolled.  The tip commit has been
cherry-picked to fc/fc/git-prompt-script topic.

Will discard without prejudice.

* jc/maint-push-refs-all (2012-05-04) 2 commits
 - get_fetch_map(): tighten checks on dest refs
 - fetch/push: allow refs/*:refs/*

Allows pushing and fetching refs/stash.
There still seem to be other bugs hiding (e.g. try pushing twice).

Not ready.

* jc/run-hook-env-1 (2012-03-11) 1 commit
 . run_hook(): enhance the interface to pass arbitrary environment

Updates run_hook() API to be much less specific to "commit".  It would
only be useful if people start doing more interesting things with hooks.

Will discard.

* jc/split-blob (2012-04-03) 6 commits
 - chunked-object: streaming checkout
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - packfile: use varint functions

Not ready.

I finished the streaming checkout codepath, but as explained in
127b177 (bulk-checkin: support chunked-object encoding, 2011-11-30),
these are still early steps of a long and painful journey. At least
pack-objects and fsck need to learn the new encoding for the series
to be usable locally, and then index-pack/unpack-objects needs to
learn it to be used remotely.

Given that I heard a lot of noise that people want large files, and
that I was asked by somebody at GitTogether'11 privately for an
advice on how to pay developers (not me) to help adding necessary
support, I am somewhat dissapointed that the original patch series
that was sent almost two months ago still remains here without much
comments and updates from the developer community. I even made the
interface to the logic that decides where to split chunks easily
replaceable, and I deliberately made the logic in the original patch
extremely stupid to entice others, especially the "bup" fanboys, to
come up with a better logic, thinking that giving people an easy
target to shoot for, they may be encouraged to help out. The plan is
not working :-(.

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

* fc/git-prompt-script (2012-06-19) 7 commits
 - completion: respect $GIT_DIR
 - completion: warn people about duplicated function
 - completion: split __git_ps1 into a separate script
 - completion: remove executable mode
 - Merge branch 'fc/git-complete-helper' into fc/git-prompt-script
 - tests: add tests for the bash prompt functions in the completion script
 - tests: move code to run tests under bash into a helper library
 (this branch is tangled with sg/bash-prompt.)

Splits a rather heavy-ish "git completion" script out and creates a
separate "git prompting" script, to help lazy-autoloading of the
completion part while making prompting part always available.

Will merge to 'next'.

* lm/git-blame-el (2012-06-14) 3 commits
 - git-blame.el: Do not use bare 0 to mean (point-min)
 - git-blame.el: Use with-current-buffer where appropriate
 - git-blame.el: Do not use goto-line in lisp code

Will merge to 'next' and soon to 'master'.

* lp/no-cmd-http-fetch (2012-06-15) 1 commit
 - builtin.h: remove unused cmd_<foo> declarations

Will merge to 'next' and soon to 'master'.

* jk/diff-no-index-pager (2012-06-15) 2 commits
 - do not run pager with diff --no-index --quiet
 - fix pager.diff with diff --no-index

Will merge to 'next' and soon to 'master'.

* nd/i18n-branch-lego (2012-06-07) 1 commit
 - Remove i18n legos in notifying new branch tracking setup

Restructure the way message strings are created, in preparation for
marking them for i18n.

Will merge to 'next' and soon to 'master'.

* nd/i18n-misc (2012-06-07) 3 commits
 - rerere: remove i18n legos in result message
 - notes-merge: remove i18n legos in merge result message
 - reflog: remove i18n legos in pruning message

Restructure the way message strings are created, in preparation for
marking them for i18n.

Will merge to 'next' and soon to 'master'.

* rr/doc-commit (2012-06-08) 1 commit
 - commit: document a couple of options

Will merge to 'next' and soon to 'master'.

* hv/remote-end-hung-up (2012-06-19) 1 commit
 - remove the impression of unexpectedness when access is denied

Will merge to 'next'.

* hv/submodule-checkout-nuke-submodules (2012-06-11) 1 commit
 - update-index: allow overwriting existing submodule index entries

Will merge to 'next'.

* jc/rev-list-simplify-merges-first-parent (2012-06-13) 3 commits
 - revision: ignore side parents while running simplify-merges
 - revision: note the lack of free() in simplify_merges()
 - revision: "simplify" options imply topo-order sort

I need to send this out to the list for re-review.

* jc/ustar-checksum-is-unsigned (2012-06-13) 1 commit
 - archive: ustar header checksum is computed unsigned

Will merge to 'next' and soon to 'master'.

* rs/git-blame-mapcar-mapc (2012-06-10) 1 commit
 - git-blame.el: use mapc instead of mapcar

Will merge to 'next' and soon to 'master'.

* rs/ipv6-ssh-url (2012-06-13) 1 commit
 - git: Wrong parsing of ssh urls with IPv6 literals ignores port

Will merge to 'next' and soon to 'master'.

* nd/exclude-workaround-top-heavy (2012-06-07) 3 commits
 - exclude: do strcmp as much as possible before fnmatch
 - dir.c: get rid of the wildcard symbol set in no_wildcard()
 - Unindent excluded_from_list()

Attempts to optimize matching with an exclude pattern with a deep
directory hierarchy by taking the part that specifies leading path
without wildcard literally.

Will merge to 'next'.

* jc/bundle-complete-notice (2012-06-04) 1 commit
  (merged to 'next' on 2012-06-19 at b42227b)
 + tweak "bundle verify" of a complete history

Originally merged to 'next' on 2012-06-04.

Running "git bundle verify" on a bundle that records a complete
history said "it requires these 0 commits".

Will merge to 'master'.

* lk/more-helpful-status-hints (2012-06-14) 4 commits
 - status: better advices when splitting a commit (during rebase -i)
 - status: don't suggest "git rm" or "git add" if not appropriate
 - t7512-status-help.sh: better advices for git status
 - wt-status.*: better advices for git status added

Will merge to 'next'.

* jk/no-more-pre-exec-callback (2012-06-05) 1 commit
 - pager: drop "wait for output to run less" hack

On hold for 6 months until ancient "less" goes extinct.

* jk/maint-t1304-setfacl (2012-06-07) 1 commit
  (merged to 'next' on 2012-06-19 at 2449521)
 + t1304: improve setfacl prerequisite setup

Originally merged to 'next' on 2012-06-08.

Works around a false test failure caused by a bug in ecryptofs.

Will merge to 'master'.

* lk/rebase-i-x (2012-06-13) 1 commit
 - rebase -i: teach "--exec <cmd>"

Adds -x <cmd> to "rebase -i" to insert "exec <cmd>" after each
commit in the resulting history.

Will merge to 'next'.

* vr/help-per-platform (2012-06-06) 1 commit
  (merged to 'next' on 2012-06-19 at d9a08ba)
 + help: use HTML as the default help format on Windows

Originally merged to 'next' on 2012-06-08

We used to always default to "man" format even on platforms where
"man" viewer is not widely available.

Will merge to 'master'.

* jc/ls-files-i-dir (2012-06-05) 6 commits
  (merged to 'next' on 2012-06-19 at 4a7aa99)
 + dir.c: make excluded() file scope static
 + unpack-trees.c: use path_excluded() in check_ok_to_remove()
 + builtin/add.c: use path_excluded()
 + path_excluded(): update API to less cache-entry centric
 + ls-files -i: micro-optimize path_excluded()
 + ls-files -i: pay attention to exclusion of leading paths

Originally merged to 'next' on 2012-06-08.

"git ls-files --exclude=t -i" did not consider anything under t/
as excluded, as it did not pay attention to exclusion of leading
paths while walking the index.  Other two users of excluded() are
also updated.

Will merge to 'master'.

* jc/request-pull-match-tagname (2012-06-01) 1 commit
  (merged to 'next' on 2012-06-19 at bb96d6c)
 + request-pull: really favor a matching tag

Originally merged to 'next' on 2012-06-05.

"git request-pull $url dev" when the tip of "dev" branch was tagged
with "ext4-for-linus" used the contents from the tag in the output
but still asked the "dev" branch to be pulled, not the tag.

Will merge to 'master'.

* jk/version-string (2012-06-03) 3 commits
  (merged to 'next' on 2012-06-19 at 12f8e07)
 + http: get default user-agent from git_user_agent
 + version: add git_user_agent function
 + move git_version_string into version.c
 (this branch is used by jk/version-string-dependency.)

Originally merged to 'next' on 2012-06-05.

Teaches git native protocol agents to show software version over the
wire.

Will merge to 'master'.

* nd/stream-pack-objects (2012-05-29) 1 commit
 - pack-objects: use streaming interface for reading large loose blobs

Will merge to 'next'.

* jk/clone-local (2012-05-30) 2 commits
  (merged to 'next' on 2012-06-19 at a42bbcc)
 + clone: allow --no-local to turn off local optimizations
 + docs/clone: mention that --local may be ignored

Originally merged to 'next' on 2012-06-05.

"git clone --local $path" started its life as an experiment to
optionally use link/copy when cloning a repository on the disk, but
we didn't deprecate it after we made the option a no-op to always
use the optimization.

Will merge to 'master'.

* jk/no-more-asciidoc7 (2012-05-30) 2 commits
  (merged to 'next' on 2012-06-19 at a36b498)
 + docs: drop antique comment from Makefile
 + docs: drop asciidoc7compatible flag

Originally merged to 'next' on 2012-06-05.

We no longer use AsciiDoc7 syntax in our documentation and favor a
more modern style.

Will merge to 'master'.

* nd/stream-index-pack (2012-05-24) 4 commits
 - index-pack: use streaming interface for collision test on large blobs
 - index-pack: factor out unpack core from get_data_from_pack
 - index-pack: use streaming interface on large blobs (most of the time)
 - index-pack: hash non-delta objects while reading from stream

Use streaming API to read from the object store to avoid having to hold
a large blob object in-core while running index-pack.

Will merge to 'next'.

* js/submodule-relative (2012-06-14) 5 commits
 - t7400: avoid path mangling issues
 - submodule: fix handling of superproject origin URLs like foo, ./foo and ./foo/bar
 - submodule: fix sync handling of some relative superproject origin URLs
 - submodule: document failure to handle relative superproject origin URLs
 - submodule: additional regression tests for relative URLs

Makes "git submodule" deal with nested submodule structure where a
module is contained within a module whose origin is specified as a
relative URL to its superproject's origin.

Will merge to 'next'.

* mm/push-default-switch-warning (2012-06-06) 1 commit
 - push: start warning upcoming default change for push.default

Will merge to 'next'.

Hopwefully we can have a solidly tested series early in 1.7.12 or
1.7.13 at the latest.

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

* cb/daemon-test-race-fix (2012-04-27) 2 commits
  (merged to 'next' on 2012-04-27 at 84bbcf8)
 + Revert "git-daemon wrapper to wait until daemon is ready"
  (merged to 'next' on 2012-04-24 at d5c30be)
 + git-daemon wrapper to wait until daemon is ready

Reverted from 'next' to replace it with js/daemon-test-race-fix.

* jc/merge-annotated-tag (2012-06-05) 2 commits
 . merge: allow fast-forwarding to an annotated but unsigned tag
 . merge: separte the logic to check for a signed tag

"git merge anno" created a merge commit even when anno is an
unsigned annotated tag that points at a commit that can be fast
forwarded to; this came from a laziness of the implementation of
merging of signed tags in 1.7.9.  People may have different opinion
on making signed and unsigned annotated tag behave differently, but
I tend to agree that it is probably not a good idea.

[PATCH] push: start warning upcoming default change for push.default

[Matthieu Moy]

In preparation for flipping the default to the "simple" mode from
the "matching" mode that is the historical default, start warning
users when they rely on unconfigured "git push" to default to the
"matching" mode.

Also, advertise for 'simple' where 'current' and 'upstream' are advised.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
> * mm/push-default-switch-warning (2012-06-06) 1 commit
>  - push: start warning upcoming default change for push.default
> 
> Will merge to 'next'.
> 
> Hopwefully we can have a solidly tested series early in 1.7.12 or
> 1.7.13 at the latest.

I've made a slightly modified version of the patch which adds these
two lines:

+   "(the 'simple' mode was introduced in Git 1.7.11. Use 'current' instead if\n"
+   "you sometimes use older versions of Git)");

I actually had problems setting "push.default=upstream" on a
NFS-shared $HOME, where one machine was running a brand new Git, and
another the old version from Debian stable, who didn't know about the
upstream mode. The old machine started refusing pushes because of
this :-(.

'current' has existed for years (while upstream has been called
'tracking' before), so it makes more sense to advertise this one for
people occasionally using old versions of Git.

 builtin/push.c       | 29 +++++++++++++++++++++++++++--
 t/t5541-http-push.sh |  5 ++++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 08ccb89..2eba331 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -147,12 +147,37 @@ static void setup_push_upstream(struct remote *remote, int simple)
 	add_refspec(refspec.buf);
 }
 
+static char warn_unspecified_push_default_msg[] =
+N_("push.default is unset; its implicit value is changing in\n"
+   "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
+   "and maintain the current behavior after the default changes, use:\n"
+   "\n"
+   "  git config --global push.default matching\n"
+   "\n"
+   "To squelch this message and adopt the new behavior now, use:\n"
+   "\n"
+   "  git config --global push.default simple\n"
+   "\n"
+   "See 'git help config' and search for 'push.default' for further information.\n"
+   "(the 'simple' mode was introduced in Git 1.7.11. Use 'current' instead if\n"
+   "you sometimes use older versions of Git)");
+
+static void warn_unspecified_push_default_configuration(void)
+{
+	static int warn_once;
+
+	if (warn_once++)
+		return;
+	warning("%s\n", _(warn_unspecified_push_default_msg));
+}
+
 static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 		default_matching_used = 1;
+		warn_unspecified_push_default_configuration();
 		/* fallthru */
 	case PUSH_DEFAULT_MATCHING:
 		add_refspec(":");
@@ -186,8 +211,8 @@ static const char message_advice_pull_before_push[] =
 static const char message_advice_use_upstream[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. If you did not intend to push that branch, you may want to\n"
-	   "specify branches to push or set the 'push.default' configuration\n"
-	   "variable to 'current' or 'upstream' to push only the current branch.");
+	   "specify branches to push or set the 'push.default' configuration variable\n"
+	   "to 'simple', 'current' or 'upstream' to push only the current branch.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 5b170be..07fa199 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -64,7 +64,10 @@ test_expect_success 'no empty path components' '
 
 test_expect_success 'clone remote repository' '
 	rm -rf test_repo_clone &&
-	git clone $HTTPD_URL/smart/test_repo.git test_repo_clone
+	git clone $HTTPD_URL/smart/test_repo.git test_repo_clone &&
+	(
+		cd test_repo_clone && git config push.default matching
+	)
 '
 
 test_expect_success 'push to remote repository (standard)' '
-- 
1.7.11.rc3.235.gd0d1d08

Re: [PATCH] push: start warning upcoming default change for push.default

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> In preparation for flipping the default to the "simple" mode from
> the "matching" mode that is the historical default, start warning
> users when they rely on unconfigured "git push" to default to the
> "matching" mode.
>
> Also, advertise for 'simple' where 'current' and 'upstream' are advised.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>> * mm/push-default-switch-warning (2012-06-06) 1 commit
>>  - push: start warning upcoming default change for push.default
>> 
>> Will merge to 'next'.
>> 
>> Hopwefully we can have a solidly tested series early in 1.7.12 or
>> 1.7.13 at the latest.
>
> I've made a slightly modified version of the patch which adds these
> two lines:
>
> +   "(the 'simple' mode was introduced in Git 1.7.11. Use 'current' instead if\n"
> +   "you sometimes use older versions of Git)");

I am not sure who this wants to help.  For people who want to
anticipate the future, using anything but 'simple' is not living in
the future and waiting for an additional pain coming from the
differences between current and simple.  If current and simple are
similar enough that their differences do not matter, why wait for
simple to be available everywhere and switch the default to it,
instead of using current as the default in the first place?

So if anything, I'd prefer the above message without "Use 'current'
instead" bit.

Re: [PATCH] push: start warning upcoming default change for push.default

[Matthieu Moy]

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

> I am not sure who this wants to help.

The target is for people who may use different versions of Git (e.g.
several installed git on the same machine, several machine, or worse,
several machines with a shared $HOME/.gitconfig). This includes me.

For these people, setting push.default=simple is basically impossible
since Git errors out when encountering such setting (it's not ignored,
it really makes Git die).

> For people who want to anticipate the future, using anything but
> 'simple' is not living in the future and waiting for an additional
> pain coming from the differences between current and simple. If
> current and simple are similar enough that their differences do not
> matter, why wait for simple to be available everywhere and switch the
> default to it, instead of using current as the default in the first
> place?

'simple' is an improvement over 'current' as a default, especially for
beginners, but 'current' is already reasonably good.

So, the long term goal is really to switch to 'simple', but people who
use different versions of Git won't be able to use it before a few
years. These people have several options:

1) Keep push.default unset. This is not acceptable because they don't
   want the big fat warning each time they push.

2) Set push.default to 'matching', to keep the old behavior and squelsh
   the warning. If they go this way, they will never see the default
   change.

3) Set push.default to 'current', in which case they have the same
   behavior as 'simple', except for the safety feature of 'simple'
   (refuse to push when the name doesn't match the upstream). They can't
   expect anything better anyway since they are sometimes using a
   machine which doesn't support 'simple' anyway.

To me, option 3) clearly seems to be the best option. I don't understand
whether you would advise 1), 2), or maybe a 4) that I would have missed.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] push: start warning upcoming default change for push.default

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> So, the long term goal is really to switch to 'simple', but people who
> use different versions of Git won't be able to use it before a few
> years. These people have several options:
>
> 1) Keep push.default unset. This is not acceptable because they don't
>    want the big fat warning each time they push.

Yes, to them one of 'simple', 'current' or 'upstream' would be
sensible (but they need to read up on them to see which one they
want).

> 2) Set push.default to 'matching', to keep the old behavior and squelsh
>    the warning. If they go this way, they will never see the default
>    change.

This is not the audience of the quoted part of the message i.e. "If
you want to use it before default changes".  These people fall into
the "If you want to keep the current default, set push.default to
'matching'" category, so it is irrelevant to the discussion.

> 3) Set push.default to 'current', in which case they have the same
>    behavior as 'simple', except for the safety feature of 'simple'
>    (refuse to push when the name doesn't match the upstream). They can't
>    expect anything better anyway since they are sometimes using a
>    machine which doesn't support 'simple' anyway.

And they will be frozen to 'current' even after their sysadmins
update the version of git that support 'simple'.  

Telling somebody who would blindly follow what was suggested to use
'current' is what bothers me.

Re: [PATCH] push: start warning upcoming default change for push.default

[Matthieu Moy]

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

>> 3) Set push.default to 'current', in which case they have the same
>>    behavior as 'simple', except for the safety feature of 'simple'
>>    (refuse to push when the name doesn't match the upstream). They can't
>>    expect anything better anyway since they are sometimes using a
>>    machine which doesn't support 'simple' anyway.
>
> And they will be frozen to 'current' even after their sysadmins
> update the version of git that support 'simple'.  

And so what? They will miss the safety feature of 'simple', but that's
not really a big deal.

'simple' was not really designed to be _better_ than current or
upstream, but to be more adapted as a default value, because 1) it's
safer, so nice for beginners and 2) follows the least surprise
principle, as it does nothing in cases where there would be an
ambiguity. But once you're not really a beginner, the added value of
'simple' is really small compared to 'current' or 'upstream'.

> Telling somebody who would blindly follow what was suggested to use
> 'current' is what bothers me.

But what alternative do we have?

You suggest that I remove the mention of 'current', but then what should
users do? Removing it sounds like replacing it with "if you sometimes
use older versions of Git, you're doomed, sorry" to me.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] push: start warning upcoming default change for push.default

[Junio C Hamano]

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

> Telling somebody who would blindly follow what was suggested to use
> 'current' is what bothers me.

That is (quoting the difference between the previous round and this
one),

 static char warn_unspecified_push_default_msg[] =
 N_("push.default is unset; its implicit value is changing in\n"
    "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
    "and maintain the current behavior after the default changes, use:\n"
    "\n"
    "  git config --global push.default matching\n"
    "\n"
    "To squelch this message and adopt the new behavior now, use:\n"
    "\n"
    "  git config --global push.default simple\n"
    "\n"
-   "See 'git help config' and search for 'push.default' for further information.");
+   "See 'git help config' and search for 'push.default' for further information.\n"
+   "(the 'simple' mode was introduced in Git 1.7.11. Use 'current' instead if\n"
+   "you sometimes use older versions of Git)");

The latter half of the message is "To *adopt* the *new behaviour
now*" but setting it to 'current' is not adopting the new behaviour.

Perhaps we should say more to help people decide which one to choose
in this message.

    ... changing in Git 2.0 from 'matching' to 'simple'.
    'matching', which is the current default, pushes all branches
    that exist at the remote.  'simple', which will be the new
    default, pushes only the current branch to update the branch at
    the remote of the same name (the 'simple' mode is only available
    in Git 1.7.11 or newer; older versions of Git has a similar mode
    called 'current').

    You can squelch this message by picking your preferred default now,
    e.g. running one of these:

            git config push.default matching
            git config push.default simple
            git config push.default current

or something?

Re: [PATCH] push: start warning upcoming default change for push.default

[Matthieu Moy]

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

> Perhaps we should say more to help people decide which one to choose
> in this message.
[...]
>     You can squelch this message by picking your preferred default now,
>     e.g. running one of these:
>
>             git config push.default matching
>             git config push.default simple
>             git config push.default current

I don't see any added value. Seeing this, it's less clear to the user,
and he may chose 'simple', and then complain that it breaks his older
Git. Really, if 'simple' isn't an option for someone, why should we make
this someone think before chosing between 'current' an 'simple'?

Also, my version purposely made 'simple' more visible than 'current'
(cut-and-paste ready alone on its line Vs within parenthesis), because
this is the one we want to advertise. Putting the 3 options at the same
level is a regression to me.

We can rephrase the advice to make it clearer that 'simple' !=
'current', like

  (the 'simple' mode was introduced in Git 1.7.11. Use the similar mode
  'current' instead if you sometimes use older versions of Git)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] push: start warning upcoming default change for push.default

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Perhaps we should say more to help people decide which one to choose
>> in this message.
> [...]
>>     You can squelch this message by picking your preferred default now,
>>     e.g. running one of these:
>>
>>             git config push.default matching
>>             git config push.default simple
>>             git config push.default current
>
> I don't see any added value. Seeing this, it's less clear to the user,
> and he may chose 'simple', and then complain that it breaks his older
> Git. 

Re-read the part you omitted with [...].  Doesn't it say something
about "only available"?

> Really, if 'simple' isn't an option for someone, why should we make
> this someone think before chosing between 'current' an 'simple'?

This is not about choosing between simple and current when both are
available.  As [...] part tries to clearly state (and if it is
unclear, please clarify it further), the new default "simple" is
only available in certain versions, and 'current' is the closest
approximation for other versions.  So even though there are three
choices, it is really about choosing between 'matching' (push all
relevant and current branch does not matter) and 'simple/current'
(push current  branch only).  We could make it like this:

	e.g. running one of these:

		git config push.default matching
		git config push.default simple (or current)

and it may make it clearer what choices are suggested.  The only
reason I made it three lines is "(or current)" would make it harder
for people who want "current only" but runs older version of git in
the same repository to cut and paste.

Re: [PATCH] push: start warning upcoming default change for push.default

[Matthieu Moy]

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

> Re-read the part you omitted with [...].  Doesn't it say something
> about "only available"?

It does, but it seems you're trying hard to avoid telling the user "you
should use 'current'", where 'current' is the only reasonable option for
this user. I still don't understand what problem you're trying to solve.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] push: start warning upcoming default change for push.default

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Re-read the part you omitted with [...].  Doesn't it say something
>> about "only available"?
>
> It does, but it seems you're trying hard to avoid telling the user "you
> should use 'current'", where 'current' is the only reasonable option for
> this user. I still don't understand what problem you're trying to solve.

I am avoiding to say "you should use simple/current".  Choice
between matching and simple/current is for the user to make (mostly
dictated by the project's workflow) and we cannot make a suggestion
better than what user knows.

Choice between simple and current is mechanically derivable. If the
user also uses older version of git, simple is not an option.

Re: [PATCH] push: start warning upcoming default change for push.default

[Junio C Hamano]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Re-read the part you omitted with [...].  Doesn't it say something
>>> about "only available"?
>>
>> It does, but it seems you're trying hard to avoid telling the user "you
>> should use 'current'", where 'current' is the only reasonable option for
>> this user. I still don't understand what problem you're trying to solve.
>
> I am avoiding to say "you should use simple/current".  Choice
> between matching and simple/current is for the user to make (mostly
> dictated by the project's workflow) and we cannot make a suggestion
> better than what user knows.
>
> Choice between simple and current is mechanically derivable. If the
> user also uses older version of git, simple is not an option.

To put it another way, I am questioning your "where 'current is the
only reasonable option for this user".  If it were truly the case,
why would we be issuing a warning message?  Wouldn't we be instead
silently doing what 'simple' or 'current' would do?

The reason why we have this message is because either "push the
current one and not others" or "push all relevant ones, regardless
of the current" are reasonable choices depending on the user, and
because we had to choose one for the default, we previously chose
the latter but we are changing our mind and will default to the
other one, no?

Re: [PATCH] push: start warning upcoming default change for push.default

[Matthieu Moy]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Re-read the part you omitted with [...].  Doesn't it say something
>>> about "only available"?
>>
>> It does, but it seems you're trying hard to avoid telling the user "you
>> should use 'current'", where 'current' is the only reasonable option for
>> this user. I still don't understand what problem you're trying to solve.
>
> I am avoiding to say "you should use simple/current".  Choice
> between matching and simple/current is for the user to make (mostly
> dictated by the project's workflow) and we cannot make a suggestion
> better than what user knows.
>
> Choice between simple and current is mechanically derivable. If the
> user also uses older version of git, simple is not an option.

I do agree with that. My message tells the user to use 'current' instead
of 'simple' when not appropriate, not to use 'current' instead of
'matching', which would indeed be a nonsense. I thought it was clear
enough, but we can make that more explicit. How about:

  (the 'simple' mode was introduced in Git 1.7.11. Use the similar mode
  'current' instead of 'simple' if you sometimes use older versions of Git)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] push: start warning upcoming default change for push.default

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> I do agree with that. My message tells the user to use 'current' instead
> of 'simple' when not appropriate, not to use 'current' instead of
> 'matching', which would indeed be a nonsense. I thought it was clear
> enough, but we can make that more explicit. How about:
>
>   (the 'simple' mode was introduced in Git 1.7.11. Use the similar mode
>   'current' instead of 'simple' if you sometimes use older versions of Git)

Yeah, that would work (the minority that need to pick 'current'
cannot cut and paste, but that is not a big deal).

As I did not butcher the version you sent me when I queued it with
my rewrite, a follow-up patch to update it should be fairly minimal
;-)

Thanks.

[PATCH] push: start warning upcoming default change for push.default

[Matthieu Moy]

In preparation for flipping the default to the "simple" mode from
the "matching" mode that is the historical default, start warning
users when they rely on unconfigured "git push" to default to the
"matching" mode.

Also, advertise for 'simple' where 'current' and 'upstream' are advised.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:

>>   (the 'simple' mode was introduced in Git 1.7.11. Use the similar mode
>>   'current' instead of 'simple' if you sometimes use older versions of Git)
>
> Yeah, that would work (the minority that need to pick 'current'
> cannot cut and paste, but that is not a big deal).
>
> As I did not butcher the version you sent me when I queued it with
> my rewrite, a follow-up patch to update it should be fairly minimal
> ;-)

Here it is.

 builtin/push.c       | 29 +++++++++++++++++++++++++++--
 t/t5541-http-push.sh |  5 ++++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 08ccb89..9a0a035 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -147,12 +147,37 @@ static void setup_push_upstream(struct remote *remote, int simple)
 	add_refspec(refspec.buf);
 }
 
+static char warn_unspecified_push_default_msg[] =
+N_("push.default is unset; its implicit value is changing in\n"
+   "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
+   "and maintain the current behavior after the default changes, use:\n"
+   "\n"
+   "  git config --global push.default matching\n"
+   "\n"
+   "To squelch this message and adopt the new behavior now, use:\n"
+   "\n"
+   "  git config --global push.default simple\n"
+   "\n"
+   "See 'git help config' and search for 'push.default' for further information.\n"
+   "(the 'simple' mode was introduced in Git 1.7.11. Use the similar mode\n"
+   "'current' instead of 'simple' if you sometimes use older versions of Git)");
+
+static void warn_unspecified_push_default_configuration(void)
+{
+	static int warn_once;
+
+	if (warn_once++)
+		return;
+	warning("%s\n", _(warn_unspecified_push_default_msg));
+}
+
 static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 		default_matching_used = 1;
+		warn_unspecified_push_default_configuration();
 		/* fallthru */
 	case PUSH_DEFAULT_MATCHING:
 		add_refspec(":");
@@ -186,8 +211,8 @@ static const char message_advice_pull_before_push[] =
 static const char message_advice_use_upstream[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. If you did not intend to push that branch, you may want to\n"
-	   "specify branches to push or set the 'push.default' configuration\n"
-	   "variable to 'current' or 'upstream' to push only the current branch.");
+	   "specify branches to push or set the 'push.default' configuration variable\n"
+	   "to 'simple', 'current' or 'upstream' to push only the current branch.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 5b170be..07fa199 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -64,7 +64,10 @@ test_expect_success 'no empty path components' '
 
 test_expect_success 'clone remote repository' '
 	rm -rf test_repo_clone &&
-	git clone $HTTPD_URL/smart/test_repo.git test_repo_clone
+	git clone $HTTPD_URL/smart/test_repo.git test_repo_clone &&
+	(
+		cd test_repo_clone && git config push.default matching
+	)
 '
 
 test_expect_success 'push to remote repository (standard)' '
-- 
1.7.11.5.g0c7e058.dirty