Git development
 help / color / mirror / Atom feed
* Re: What's cooking in git.git (Oct 2011, #03; Fri, 7)
From: Heiko Voigt @ 2011-10-10 20:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jens Lehmann, Brad King, Fredrik Gustafsson
In-Reply-To: <7vsjn4tukt.fsf@alter.siamese.dyndns.org>

Hi Junio,

sorry for the late reply I was taking some time off from email. Here now
some information on the two topics I am involved with that got stalled:

On Fri, Oct 07, 2011 at 01:28:34PM -0700, Junio C Hamano wrote:
> --------------------------------------------------
> [Stalled]
> 
> * hv/submodule-merge-search (2011-08-26) 5 commits
>  - submodule: Search for merges only at end of recursive merge
>  - allow multiple calls to submodule merge search for the same path
>  - submodule: Demonstrate known breakage during recursive merge

The three patches above belong to the merge-search fix topic. I think
they should be good to go.

>  - push: Don't push a repository with unpushed submodules
>  - push: teach --recurse-submodules the on-demand option
>  (this branch is tangled with fg/submodule-auto-push.)

These two belong into the fg/submodule-auto-push topic. It seems they
got mixed into this while dicussing the two topics.

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

I will look into that.

> * fg/submodule-auto-push (2011-09-11) 2 commits
>  - submodule.c: make two functions static
>  - push: teach --recurse-submodules the on-demand option
>  (this branch is tangled with hv/submodule-merge-search.)
> 
> What the topic aims to achieve may make sense, but the implementation
> looked somewhat suboptimal.

We will also have a look at the final cleanups we need here. (Fredrik?)

Cheers Heiko

^ permalink raw reply

* Re: Re: [PATCH 6/6] Retain caches of submodule refs
From: Heiko Voigt @ 2011-10-10 19:53 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Junio C Hamano, git, Jeff King, Drew Northup, Jakub Narebski
In-Reply-To: <4E918194.5060102@alum.mit.edu>

Hi Michael,

On Sun, Oct 09, 2011 at 01:12:20PM +0200, Michael Haggerty wrote:
> On 08/17/2011 12:45 AM, Junio C Hamano wrote:
> > All the changes except for this one made sense to me, but I am not sure
> > about this one. How often do we look into different submodule refs in the
> > same process over and over again?
> 
> I am having pangs of uncertainty about this patch.

Currently when doing a 3 way merge of submodule hashes and we find a
conflict. We then use this api to look into the submodule to search for
commit suggestions which contain both sides.

If there are multiple submodules that conflict in this way we look into
those as well.

Since the setup_revision() api can currently not be used to safely
iterate twice over the same submodule my patch

	allow multiple calls to submodule merge search for the same path

rewrites the search into using a child process. AFAIK the submodule ref
iteration api would then even be unused.

> Previous to this patch, the submodule reference cache was only used for
> the duration of one call to do_for_each_ref().  (It was not *discarded*
> until later, but the old cache was never reused.)  Therefore, the
> submodule reference cache was implicitly invalidated between successive
> uses.

The implicit discarding was just done because it was the quickest way to
get a handle on submodule refs from the main process. There was no need
that they get reloaded every time.

> After this change, submodule ref caches are invalidated whenever
> invalidate_cached_refs() is called.  But this function is static, and it
> is only called when main-module refs are changed.
> 
> AFAIK there is no way within refs.c to add, modify, or delete a
> submodule reference.  But if other code modifies submodule references
> directly, then the submodule ref cache in refs.c would become stale.
> Moreover, there is currently no API for invalidating the cache.
> 
> So I think I need help from a submodule guru (Heiko?) who can tell me
> what is done with submodule references and whether they might be
> modified while a git process is executing in the main module.  If so,
> then either this patch has to be withdrawn, or more work has to be put
> in to make such code invalidate the submodule reference cache.

At least in my code there is no place where a submodule ref is changed.
I only used it for merging submodule which only modifies the main
module. So I would say its currently safe to assume that submodule refs
do not get modified. If we do need that later on we can still add
invalidation for submodule refs.

Cheers Heiko

^ permalink raw reply

* Re: [PATCH 2/2] submodule::module_clone(): silence die() message from module_name()
From: Jens Lehmann @ 2011-10-10 19:34 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: git, Junio C Hamano
In-Reply-To: <1317978295-4796-2-git-send-email-rctay89@gmail.com>

BTW: this patch applies to next

Am 07.10.2011 11:04, schrieb Tay Ray Chuan:
> The die() message that may occur in module_name() is not really relevant
> to the user when called from module_clone(); the latter handles the
> "failure" (no submodule mapping) anyway.

Makes tons of sense, especially as adding a new submodule currently always
spews out the "No submodule mapping found in .gitmodules for path 'sub'"
message right before that mapping is added there. Thanks for noticing that
and ACK on that change from my side.

> Leave other callers of module_name() unchanged, as the die() message
> shown is either relevant for user consumption (such as those that exit()
> when the call fails), or will not occur at all (when called with paths
> returned by module_list()).

Hmm, while I agree on the first reasoning I'm not sure about the second.
module_list() asks the index for the submodule paths while module_name()
gets it's input from .gitmodules, so they can (and sometimes will)
disagree. When cmd_foreach() passes an empty "name" variable to the
spawned command that might still work (and even make sense), but using the
empty name in cmd_sync() to access the config is looking like an error to
me. It might make sense to add an "|| exit" at least to the callsite in
cmd_sync(). Or am I missing something here?

> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
>  git-submodule.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index ebea35b..3adab93 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -130,7 +130,7 @@ module_clone()
>  
>  	gitdir=
>  	gitdir_base=
> -	name=$(module_name "$path")
> +	name=$(module_name "$path" 2>/dev/null)
>  	base_path=$(dirname "$path")
>  
>  	gitdir=$(git rev-parse --git-dir)

^ permalink raw reply

* Re: [PATCH 1/2] submodule: whitespace fix
From: Jens Lehmann @ 2011-10-10 18:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Tay Ray Chuan, git
In-Reply-To: <1317978295-4796-1-git-send-email-rctay89@gmail.com>

Am 07.10.2011 11:04, schrieb Tay Ray Chuan:
> Replace SPs with TAB.

As these are the only lines where spaces are used for indentation in this
file this might be a worthwhile cleanup, but I really don't care that much.
Junio, what do you think?

> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
>  git-submodule.sh |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 928a62f..ebea35b 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -104,9 +104,9 @@ module_name()
>  	re=$(printf '%s\n' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
>  	name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
>  		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
> -       test -z "$name" &&
> -       die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$path'")"
> -       echo "$name"
> +	test -z "$name" &&
> +	die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$path'")"
> +	echo "$name"
>  }
>  
>  #

^ permalink raw reply

* Re: [PATCH v3 5/5] attr.c: respect core.ignorecase when matching attribute patterns
From: Brandon Casey @ 2011-10-10 18:01 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Brandon Casey, gitster, git, peff, j.sixt
In-Reply-To: <4E91BAC8.9060606@alum.mit.edu>

On Sun, Oct 9, 2011 at 10:16 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 10/06/2011 08:22 PM, Brandon Casey wrote:
>> The last set of tests is performed only on a case-insensitive filesystem.
>> Those tests make sure that git handles the case where the .gitignore file
>> resides in a subdirectory and the user supplies a path that does not match
>> the case in the filesystem.  In that case^H^H^H^Hsituation, part of the
>> path supplied by the user is effectively interpreted case-insensitively,
>> and part of it is dependent on the setting of core.ignorecase.  git should
>> only match the portion of the path below the directory holding the
>> .gitignore file according to the setting of core.ignorecase.
>
> Isn't this a rather perverse scenario?

No argument here. :)

> It is hard to imagine that
> anybody *wants* part of a single filename to be matched
> case-insensitively and another part to be matched case-sensitively.
> ISTM that a person who is using a case-insensitive filesystem and has
> core-ignorecase=false is (1) a glutton for punishment and (2) probably
> very careful to make sure that the case of all pathnames is correct.  So
> such a person is not likely to benefit from this schizophrenic behavior.
>
>> [...]  If git instead built the attr
>> stack by scanning the repository, then the paths in the origin field would
>> not necessarily match the paths supplied by the user.  If someone makes a
>> change like that in the future, these tests will notice.
>
> Your decision to treat path names as partly case-insensitive...

You are giving more credit to this patch than it deserves.  It really
doesn't do much.  It's not a design decision that I made to treat path
names as case-insensitive.  That is a consequence of using a
case-insensitive file system.  When you give git the path A/b/c and
there exists a/.gitattributes, then on a case insensitive file system
git will try to read A/.gitattributes and the filesystem will return a
handle for a/.gitattributes and git won't be able to tell the
difference.  git will then apply the rules in the file to the paths
below a/ even when the path is supplied like A/.  So, at the moment,
regardless of the setting of core.ignorecase, the path above a
.gitignore file is treated as case-insensitive on a case-insensitive
filesystem.

The purpose of the tests are primarily to ensure that nothing special
needs to be done concerning paths leading up to a directory containing
a .gitignore file in the core.ignorecase=1 case.  For example, it is
_not_ necessary to use strncmp_icase instead of strncmp on the leading
portion of the path.

What I meant when I said "If someone makes a change like that in the
future, these tests will notice", is that they will notice that they
must now use strncmp_icase when comparing the portion of the path
above the .gitattributes file.

> will make
> this kind of improvement considerably more complicated.
> Therefore, weighing the negligible benefit of declaring this
> schizophrenic behavior against the potential big pain of remaining
> backwards-compatible with this behavior, I suggest that we either (1)
> declare that when core.ignorecase=false then the *whole* filenames
> (including the path leading up to the .gitignore file) must match
> case-sensitively, or (2) declare the behavior in this situation to be
> undefined so that nobody thinks they should depend on it.

I do not plan to make git treat leading paths case-sensitively on a
case-insensitive file system when core.ignorecase=0 any more than I
plan to make git treat leading paths case-insensitively on a
case-sensitive file system when core.ignorecase=1.  For justification,
I refer back to your original comment about perversion. :)

So, #2 gets my vote.


Maybe my commit message is not clear that it is describing the current
behavior and not defining it.  Instead of

   git should only match the portion of the path below the directory
   holding the .gitignore file according to the setting of
   core.ignorecase.

maybe I should say

    git will currently only match the portion of the path...

I could also remove the following test from the CASE_INSENSITIVE_FS
tests since it is really a dontcare:

   attr_check A/b/h a/b/h "-c core.ignorecase=0"

We don't care what happens when the user supplies A/b/h and a/b/h
exists on disk when core.ignorecase=0, we only care that A/b/h is
interpreted correctly when core.ignorecase=1.

-Brandon

^ permalink raw reply

* Re: [PATCH 4/6] revert: Make commit descriptions in insn sheet optional
From: Junio C Hamano @ 2011-10-10 17:54 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Jonathan Nieder, Jeff King, Daniel Barkalow,
	Christian Couder
In-Reply-To: <1318095407-26429-5-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Change the instruction sheet format subtly so that a description of
> the commit after the object name is optional.  As a result, an
> instruction sheet like this is now perfectly valid:
>
>   pick 35b0426
>   pick fbd5bbcbc2e
>   pick 7362160f
>
> Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
> Acked-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  builtin/revert.c                |   19 ++++++++-----------
>  t/t3510-cherry-pick-sequence.sh |   14 ++++++++++++++
>  2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 6451089..aa6c34e 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -692,26 +692,23 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
>  	unsigned char commit_sha1[20];
>  	char sha1_abbrev[40];
>  	enum replay_action action;
> -	int insn_len = 0;
> -	char *p, *q;
> +	const char *p, *q;
>  
> +	p = start;
>  	if (!prefixcmp(start, "pick ")) {
>  		action = CHERRY_PICK;
> -		insn_len = strlen("pick");
> -		p = start + insn_len + 1;
> +		p += strlen("pick ");
>  	} else if (!prefixcmp(start, "revert ")) {
>  		action = REVERT;
> -		insn_len = strlen("revert");
> -		p = start + insn_len + 1;
> +		p += strlen("revert ");
>  	} else
>  		return NULL;
>  
> -	q = strchr(p, ' ');
> -	if (!q || q - p + 1 > sizeof(sha1_abbrev))
> +	q = p + strcspn(p, " \n");
> +	if (q - p + 1 > sizeof(sha1_abbrev))
>  		return NULL;
> -	q++;
> -
> -	strlcpy(sha1_abbrev, p, q - p);
> +	memcpy(sha1_abbrev, p, q - p);
> +	sha1_abbrev[q - p] = '\0';

Since this is a part of clean-up series...

Do you even need to have a sha1_abbrev[] local array that is limited to 40
bytes here? The incoming _line_ is not "const char *start", so you should
at least be able to temporarily terminate the commit object name with NUL
(while remembering what byte there was before), give it to get_sha1(), and
then restore the byte at the end before returning from this function.

A bonus point would be to introduce a variant of get_sha1() that can take
(a pointer + len) not (a pointer to NUL terminated string). While I think
that would be a right thing to do in the longer term, it is outside of the
scope of this series.

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-10 16:45 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <4E92919F.2030007@drmicha.warpmail.net>

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

> What norm? --amend keeps some header fields and discards others. In
> fact, signing a commit "without changing it" (i.e. keeping tree, parents
> etc., alias "--amend -C HEAD") should be the normal use case for signing
> the tip of an existing branch. I mean, I have no problems adding to this:
>
> git help fixup
> `git fixup' is aliased to `commit --amend -C HEAD'

You are *additionally* saying "-C HEAD" in an non-standard alias. Isn't
that enough indication that a vanila "--amend" is intended to record the
commit based on the updated context in which the new commit is made?
E.g. the authorship of the patch is still the same but committer
information is updated.

> But what is the best default for the workflows that we encourage (commit
> early, ...)? You answer a pull-request which happens to be a
> fast-forward, sign the tip and suddenly you've taken over ownership (and
> changed dates)??? Signing a commit should not do this.

I personally think a pull that is made in response to a pull-request,
i.e. the upstream merging from lieutenant, especially when the
authenticity of the puller matters, is perfectly fine with --no-ff.

Unlike the sign-less "we together made these history and nobody really
owns the result" (aka "Linus hates --no-ff merge because people do that to
leave a mark by peeing in the snow, without adding anything of value in
the history"), the whole purpose of signing a commit in the scenario you
mentioned is for the puller to leave his mark in the history.

^ permalink raw reply

* Re: [PATCH] commit: teach --gpg-sign option
From: Junio C Hamano @ 2011-10-10 16:35 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <4E9291B6.2090201@drmicha.warpmail.net>

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

> Junio C Hamano venit, vidit, dixit 09.10.2011 23:22:
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>> 
>>> I just noticed that this format differs from the one of signed
>>> tags. What special reason is there for the "sig " indentation?
>> 
>> Read the part of the message you are quoting.
>
> I certainly did, and certainly did not find any mention. Do you think I
> would have asked otherwise? I'm trying to be helpful by testing out a
> patch in flight. That is: *I* am trying to be helpful.

Surely, sorry and thanks. I should have pointed out where I _thought_ I
spelled out the reason but apparently in an ineffective way (as my wording
did not convey what I wanted to say at least to you) a bit more clearly.

> This
>> The lines of GPG detached signature are placed in new header lines, after
>> the standard tree/parent/author/committer headers, instead of tucking the
>> signature block at the end of the commit log message text (similar to how
>> signed tag is done), for multiple reasons:
> gave me the impression that commit signatures are done "similar to how
> signed tag is done".

Yeah, sorry if that sentence parses badly.

It was trying to say:

 - the sig is in header lines
   - which is different (instead of) from tucking it at the end of text
     - by the way that "tucking at the end" would have been similar to
       signed tag

The reasons why signatures in the header is better than tucking at the end
are enumerated in the part you did not quote in this message but in the
part you did quote in the previous message.

^ permalink raw reply

* Re: Git bug. gitattributes' pattern does not respect spaces in the filenames
From: Alexey Shumkin @ 2011-10-10 15:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vipnwooh1.fsf@alter.siamese.dyndns.org>

> Alexey Shumkin <Alex.Crezoff@gmail.com> writes:
> 
> > As far as cp1251 and UTF-8 files are in different folders,
> > it is logically enough to set pattern like
> >
> > <folder with a UTF-8-xmls>/*.xml diff=utf8-to-cp1251
> >
> > for the UTF-8 files.
> 
> ... IN the directory that needs conversion and not in the other one
> or at the toplevel. Problem solved, no?
Oh! yes! solved! thanks!
I did not take into account that each folder can have
its own .gitattributes-file

> 
> Another idea may be to use "?" in the directory part of the
> pattern. Unless the directory structure is sick enough to have these
> directory names:
> 
> 	dir-1/utf8-file.xml
>         dir 1/cp1251-file.xml
> 
> dir?1/*.xml would match the matter, so...

hmm... I like more the case above :)
but TMTOWTDI principle rulez

thanks again

^ permalink raw reply

* Re: Git bug. gitattributes' pattern does not respect spaces in the filenames
From: Junio C Hamano @ 2011-10-10 15:28 UTC (permalink / raw)
  To: Alexey Shumkin; +Cc: git
In-Reply-To: <20111010110221.38e9985a@ashu.dyn.rarus.ru>

Alexey Shumkin <Alex.Crezoff@gmail.com> writes:

> As far as cp1251 and UTF-8 files are in different folders,
> it is logically enough to set pattern like
>
> <folder with a UTF-8-xmls>/*.xml diff=utf8-to-cp1251
>
> for the UTF-8 files.

... IN the directory that needs conversion and not in the other one or at
the toplevel. Problem solved, no?

Another idea may be to use "?" in the directory part of the
pattern. Unless the directory structure is sick enough to have these
directory names:

	dir-1/utf8-file.xml
        dir 1/cp1251-file.xml

dir?1/*.xml would match the matter, so...

^ permalink raw reply

* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Junio C Hamano @ 2011-10-10 15:23 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Jay Soffian, Ramkumar Ramachandra
In-Reply-To: <m3ehyl1g5v.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Yet another issue is if we should blindly trust automatic merge resolution.
> It is considered a good practice by some to always check (e.g. by compiling
> and possibly also running tests) the result of merge, whether it required
> merge conflict resolution or not.
>
> IIRC Linus lately said that making "git merge" automatically commit
> was one of bad design decisions of git, for the above reason...

I think your recalling this discussion

    http://thread.gmane.org/gmane.linux.kernel/1191100/focus=181362

While I agree with what Linus said in the message, I think you are not
remembering the discussion correctly. It was about bad commit _message_,
and an improvement is not to let users tweak a cleanly automerged result,
but is to allow users or force them to always write their own message,
perhaps with "merge --[no-]edit", which is exactly the point of Jay's
patch in this thread.

^ permalink raw reply

* Git User's Survey 2011 very short summary
From: Jakub Narebski @ 2011-10-10 15:21 UTC (permalink / raw)
  To: git

Unfortunately Git Wiki is still down.  As soon as possible after it is
up I'll upload results of "Git User's Survey 2011" at
   
  https://git.wiki.kernel.org/index.php/GitSurvey2011


Currently you can download 'Survey results Oct 03, 11.csv.gz', a
compressed CSV file with raw survey data (individual responses), and
'GitSurvey2011_questions.yml' YAML file with question structure from
my repository with script used to analyse survey results:

  https://github.com/jnareb/GitSurvey-scripts


You can also view the summary of "Git User's Survey 2011" and
analyse this survey using provided filters at Survs.com:

  https://www.survs.com/results/Q5CA9SKQ/P7DE07F0PL
  http://tinyurl.com/GitSurvey2011Analysis
 

..................................................................
Below there is very short summary of some of the results of
"Git User's Survey 2011"

Total responders: 11498 (7177 complete responses i.e. all sections answered,
and 4321 incomplete responses, which gives completion rate of 62%).


About you
^^^^^^^^^
Most responders are from USA among other countries (35%), followed by
Germany, UK, Canada and France (12% to 4%).  Most responders are from Europe
among other continents (48%), with North America second (40%).

Most responders are between 26 and 30 years old; most common age (mode)
is 27 years old.  Youngest user who answered this question is 11 years 
old (next oldest is 12 years old).

820 of Git developers (out of around 910 in all) answered this survey.
This is 9.1% of responders who answered question 'Does Git include
code or documentation by you? (Are you a Git developer?)'.


Getting started with Git
^^^^^^^^^^^^^^^^^^^^^^^^
Responders say that Git is reasonably easy to learn, and easy to 
reasonably easy to use; it is easier to use than to learn.

More than 90% use git version 1.7.x at least at one place.  There are 5 
unfortunate stragglers who use pre 1.3.x version.

Responders fall somewhere between 'everyday use' (36%) and 'can offer 
advice' (34%) proficiency.


How you use Git
^^^^^^^^^^^^^^^
Most people (74%) obtain/install Git at least at one place using ready 
binary packages.

GNU/Linux dominates among operating systems used (82%), then there is
MacOS X (50%), then MS Windows (where msysGit is 3 to 1 preferred to
Cygwin), with 39%.

Almost 10% of responders use JGit.

62% of responders use some kind of graphical history viewer, and 41%
use Git support integrated with editor/IDE.  Graphical diff and
graphical commit tool are used by around 31% of responders.

Most popular feature, by quite a large margin, is *stash*, with two 
thirds (69.7%) of responses. "git stash" first appeared in git version 
1.5.3, from September 2007.

Second most common used feature is "shell completion of commands", with 
half of responses (49.9%).

Then there are, with percentage of responses from 45% to 42%, in
descending order of responders: interactive rebase, aliases (git
aliases, shell aliases, one's own git scripts), and comitting with
dirty tree (keeping some changes uncommitted).

Least used, with only 0.5% of responses, is "git cvsserver" and
"replace mechanism (git replace)".

Most requested features, both with more of 32% responders, are better
support for big files / large media, and support for tracking empty
directories.


Interacting with other repositories
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The leader among git hosting sites is (like in previous years) GitHub
with 87% of responders (up from 77% in 2010 and 62% in 2009); "self
hosted" is second with 38% (down from 44% in 2010 and 57% in 2009),
followed by "company internal" with 31%.

The high position of GitHub migh be at least partially caused by the
fact that GitHub has good announcement system for new features, which
was used to announce "Git User's Survey 2011" to GitHub users.

Most responders do not use paid git hosting (64%); those that pay for
git hosting do it mainly for private repositories (35%).

>From those that self-host git repositories (2932 / 11498) more use
unmaintained(?) Gitosis (36%) than Gitolite (25%).

git:// protocol and SSH have the same popularity for fetching; both
around 76%.  Only 46% responders use HTTP for fetching (somewhere).
Deprecated rsync protocol is used only by 20 responders (0.3%).


Other version control systems
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The question about other version control systems was lacking the
"N/A (I use only Git)" answer; in fact presence or absence of N/A
answer is quite inconsistent in this year survey.

Most popular other version contsol systems used alongside or as an
alternative to Git are Subversion (81%), followed after a large gap by
Mercurial (31%).


What do you think of Git
^^^^^^^^^^^^^^^^^^^^^^^^
Most responders are "very happy" with Git, and this answer seems to be a 
center of responses.

User interface, documentation and tools need improvement; performance 
rather doesn't need to be improved; people mostly don't care about 
improving communities or localization (but please remember that this 
survey was in English, and announced on English-speaking sites / 
lists).


Documentation. Getting and giving help.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Most used way to request help about Git (help 'channel') is asking on
StackOverflow or other StackExchange site, with 36% of responders (!),
slightly higher than previous year winner of asking somebody better
versed in Git personally, with 32%.

Note that this year responses will require processing to be able to
compare with results from previous surveys, as in this survey we have
"N/A (didn't request help)" answer with 40% of responses.

On the other hand 72% of responders *gave* help via talk / private
explanation, i.e. answered questions about Git from colleaugue.
Around the same number (about 20%) didn't give help about Git as gave
help via email or instant messaging.  Note that only 14% of responders
gave help via StackOverflow or similar site.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH] config: display key_delim for config --bool --get-regexp
From: Matthieu Moy @ 2011-10-10 12:54 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <201110101220.21890.brian.foster@maxim-ic.com>

The previous logic in show_config was to print the delimiter when the
value was set, but Boolean variables have an implicit value "true" when
they appear with no value in the config file. As a result, we got:

git_Config        --get-regexp '.*\.Boolean'	#1. Ok: example.boolean
git_Config --bool --get-regexp '.*\.Boolean'	#2. NO: example.booleantrue

Fix this by defering the display of the separator until after the value
to display has been computed.

Reported-by: Brian Foster <brian.foster@maxim-ic.com>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/config.c       |   20 +++++++++++++-------
 t/t1300-repo-config.sh |    6 ++++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 0b4ecac..0315ad7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -99,6 +99,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	const char *vptr = value;
 	int must_free_vptr = 0;
 	int dup_error = 0;
+	int must_print_delim = 0;
 
 	if (!use_key_regexp && strcmp(key_, key))
 		return 0;
@@ -109,10 +110,8 @@ static int show_config(const char *key_, const char *value_, void *cb)
 		return 0;
 
 	if (show_keys) {
-		if (value_)
-			printf("%s%c", key_, key_delim);
-		else
-			printf("%s", key_);
+		printf("%s", key_);
+		must_print_delim = 1;
 	}
 	if (seen && !do_all)
 		dup_error = 1;
@@ -130,16 +129,23 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	} else if (types == TYPE_PATH) {
 		git_config_pathname(&vptr, key_, value_);
 		must_free_vptr = 1;
+	} else if (value_) {
+		vptr = value_;
+	} else {
+		/* Just show the key name */
+		vptr = "";
+		must_print_delim = 0;
 	}
-	else
-		vptr = value_?value_:"";
 	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
 				key_, vptr);
 	}
-	else
+	else {
+		if (must_print_delim)
+			printf("%c", key_delim);
 		printf("%s%c", vptr, term);
+	}
 	if (must_free_vptr)
 		/* If vptr must be freed, it's a pointer to a
 		 * dynamically allocated buffer, it's safe to cast to
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3e140c1..dffccf8 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -333,6 +333,12 @@ test_expect_success 'get-regexp variable with no value' \
 	'git config --get-regexp novalue > output &&
 	 cmp output expect'
 
+echo 'novalue.variable true' > expect
+
+test_expect_success 'get-regexp --bool variable with no value' \
+	'git config --bool --get-regexp novalue > output &&
+	 cmp output expect'
+
 echo 'emptyvalue.variable ' > expect
 
 test_expect_success 'get-regexp variable with empty value' \
-- 
1.7.7.140.ge3099

^ permalink raw reply related

* Re: [BUG 1.7.6.1] `git config --bool --get-regexp' omits separating space... sometimes!
From: Matthieu Moy @ 2011-10-10 12:44 UTC (permalink / raw)
  To: Brian Foster; +Cc: git mailing list
In-Reply-To: <201110101220.21890.brian.foster@maxim-ic.com>

Brian Foster <brian.foster@maxim-ic.com> writes:

> example.boolean
> example.booleantrue

There's a problem in

static int show_config(const char *key_, const char *value_, void *cb)

in builtin/config.c. Patch follows.

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

^ permalink raw reply

* [BUG 1.7.6.1] `git config --bool --get-regexp' omits separating space... sometimes!
From: Brian Foster @ 2011-10-10 10:20 UTC (permalink / raw)
  To: git mailing list

Hello,

# Script to illustrate the problem:
rm -f Config
cat <<\EOF >Config
[Example]
	Boolean
	Other = yes
EOF
git_Config() { git config --file Config "$@"; }

git version
git_Config        --get-regexp '.*\.Boolean'	#1. ✓ Ok: example.boolean
git_Config --bool --get-regexp '.*\.Boolean'	#2. ✗ NO: example.booleantrue
git_Config        --get-regexp '.*\.Other'	#3. ✓ Ok: example.other yes
git_Config --bool --get-regexp '.*\.Other'	#4. ✓ Ok: example.other true
exit

# Output:
git version 1.7.6.1
example.boolean
example.booleantrue
example.other yes
example.other true

 Case 2 omits the space between the key-name and (generated) value,
 making the output difficult to parse/process.  Without checking,
 I assume --int (and friends) have a similar bug?

cheers!
	-blf-

-- 
Brian FOSTER
Principal MTS, Software
Maxim Integrated Products (Microcontroller BU), formerly Innova Card
Web    : http://www.maxim-ic.com/

^ permalink raw reply

* [PATCH v2 3/7] invalidate_ref_cache(): expose this function in refs API
From: Michael Haggerty @ 2011-10-10  8:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>

Make invalidate_ref_cache() an official part of the refs API.  It is
currently a fact of life that code outside of refs.c mucks about with
references.  This change gives such code a way of informing the refs
module that it should no longer trust its cache.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |    2 +-
 refs.h |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 89b2a0e..fb46cf5 100644
--- a/refs.c
+++ b/refs.c
@@ -223,7 +223,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 	return refs;
 }
 
-static void invalidate_ref_cache(const char *submodule)
+void invalidate_ref_cache(const char *submodule)
 {
 	clear_cached_refs(get_cached_refs(submodule));
 }
diff --git a/refs.h b/refs.h
index 5de06e5..3ddc4e4 100644
--- a/refs.h
+++ b/refs.h
@@ -77,6 +77,14 @@ extern void unlock_ref(struct ref_lock *lock);
 /** Writes sha1 into the ref specified by the lock. **/
 extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);
 
+/*
+ * Invalidate the reference cache for the specified submodule.  Use
+ * submodule=NULL to invalidate the cache for the main module.  This
+ * function must be called if references are changed via a mechanism
+ * other than the refs API.
+ */
+extern void invalidate_ref_cache(const char *submodule);
+
 /** Setup reflog before using. **/
 int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
 
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH v2 7/7] clear_cached_refs(): inline function
From: Michael Haggerty @ 2011-10-10  8:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>

clear_cached_refs() was only called from one place, so inline it
there.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index 01a5958..bf53189 100644
--- a/refs.c
+++ b/refs.c
@@ -191,12 +191,6 @@ static void clear_cached_loose_refs(struct cached_refs *refs)
 	refs->did_loose = 0;
 }
 
-static void clear_cached_refs(struct cached_refs *refs)
-{
-	clear_cached_packed_refs(refs);
-	clear_cached_loose_refs(refs);
-}
-
 static struct cached_refs *create_cached_refs(const char *submodule)
 {
 	int len;
@@ -237,7 +231,9 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 
 void invalidate_ref_cache(const char *submodule)
 {
-	clear_cached_refs(get_cached_refs(submodule));
+	struct cached_refs *refs = get_cached_refs(submodule);
+	clear_cached_packed_refs(refs);
+	clear_cached_loose_refs(refs);
 }
 
 static struct ref_list *read_packed_refs(FILE *f)
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH v2 6/7] write_ref_sha1(): only invalidate the loose ref cache
From: Michael Haggerty @ 2011-10-10  8:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>

Since write_ref_sha1() can only write loose refs and cannot write
symbolic refs, there is no need for it to invalidate the packed ref
cache.

Suggested by: Martin Fick <mfick@codeaurora.org>

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 6e480ad..01a5958 100644
--- a/refs.c
+++ b/refs.c
@@ -1519,7 +1519,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
-	invalidate_ref_cache(NULL);
+	clear_cached_loose_refs(get_cached_refs(NULL));
 	if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
 	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
 	     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH v2 5/7] clear_cached_refs(): extract two new functions
From: Michael Haggerty @ 2011-10-10  8:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>

Extract two new functions from clear_cached_refs():
clear_loose_ref_cache() and clear_packed_ref_cache().

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index 9f004ce..6e480ad 100644
--- a/refs.c
+++ b/refs.c
@@ -175,14 +175,26 @@ static void free_ref_list(struct ref_list *list)
 	}
 }
 
-static void clear_cached_refs(struct cached_refs *refs)
+static void clear_cached_packed_refs(struct cached_refs *refs)
 {
-	if (refs->did_loose && refs->loose)
-		free_ref_list(refs->loose);
 	if (refs->did_packed && refs->packed)
 		free_ref_list(refs->packed);
-	refs->loose = refs->packed = NULL;
-	refs->did_loose = refs->did_packed = 0;
+	refs->packed = NULL;
+	refs->did_packed = 0;
+}
+
+static void clear_cached_loose_refs(struct cached_refs *refs)
+{
+	if (refs->did_loose && refs->loose)
+		free_ref_list(refs->loose);
+	refs->loose = NULL;
+	refs->did_loose = 0;
+}
+
+static void clear_cached_refs(struct cached_refs *refs)
+{
+	clear_cached_packed_refs(refs);
+	clear_cached_loose_refs(refs);
 }
 
 static struct cached_refs *create_cached_refs(const char *submodule)
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH v2 2/7] invalidate_ref_cache(): take the submodule as parameter
From: Michael Haggerty @ 2011-10-10  8:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>

Instead of invalidating the ref cache on an all-or-nothing basis,
allow the cache for individual submodules to be invalidated.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 56e4254..89b2a0e 100644
--- a/refs.c
+++ b/refs.c
@@ -223,13 +223,9 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 	return refs;
 }
 
-static void invalidate_ref_cache(void)
+static void invalidate_ref_cache(const char *submodule)
 {
-	struct cached_refs *refs = cached_refs;
-	while (refs) {
-		clear_cached_refs(refs);
-		refs = refs->next;
-	}
+	clear_cached_refs(get_cached_refs(submodule));
 }
 
 static struct ref_list *read_packed_refs(FILE *f)
@@ -1212,7 +1208,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	ret |= repack_without_ref(refname);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
-	invalidate_ref_cache();
+	invalidate_ref_cache(NULL);
 	unlock_ref(lock);
 	return ret;
 }
@@ -1511,7 +1507,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
-	invalidate_ref_cache();
+	invalidate_ref_cache(NULL);
 	if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
 	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
 	     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH v2 4/7] clear_cached_refs(): rename parameter
From: Michael Haggerty @ 2011-10-10  8:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>

...for consistency with the rest of this module.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index fb46cf5..9f004ce 100644
--- a/refs.c
+++ b/refs.c
@@ -175,14 +175,14 @@ static void free_ref_list(struct ref_list *list)
 	}
 }
 
-static void clear_cached_refs(struct cached_refs *ca)
+static void clear_cached_refs(struct cached_refs *refs)
 {
-	if (ca->did_loose && ca->loose)
-		free_ref_list(ca->loose);
-	if (ca->did_packed && ca->packed)
-		free_ref_list(ca->packed);
-	ca->loose = ca->packed = NULL;
-	ca->did_loose = ca->did_packed = 0;
+	if (refs->did_loose && refs->loose)
+		free_ref_list(refs->loose);
+	if (refs->did_packed && refs->packed)
+		free_ref_list(refs->packed);
+	refs->loose = refs->packed = NULL;
+	refs->did_loose = refs->did_packed = 0;
 }
 
 static struct cached_refs *create_cached_refs(const char *submodule)
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH v2 1/7] invalidate_ref_cache(): rename function from invalidate_cached_refs()
From: Michael Haggerty @ 2011-10-10  8:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318235064-25915-1-git-send-email-mhagger@alum.mit.edu>

It is the cache that is being invalidated, not the references.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 2cb93e2..56e4254 100644
--- a/refs.c
+++ b/refs.c
@@ -223,7 +223,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 	return refs;
 }
 
-static void invalidate_cached_refs(void)
+static void invalidate_ref_cache(void)
 {
 	struct cached_refs *refs = cached_refs;
 	while (refs) {
@@ -1212,7 +1212,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	ret |= repack_without_ref(refname);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
-	invalidate_cached_refs();
+	invalidate_ref_cache();
 	unlock_ref(lock);
 	return ret;
 }
@@ -1511,7 +1511,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
-	invalidate_cached_refs();
+	invalidate_ref_cache();
 	if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
 	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
 	     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH v2 0/7] Provide API to invalidate refs cache
From: Michael Haggerty @ 2011-10-10  8:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Michael Haggerty
In-Reply-To: <1318225574-18785-1-git-send-email-mhagger@alum.mit.edu>

Sorry for superseding my own patch series, but I prefer this patch
series to the one that I submitted earlier today:

1. An API function deserves a more carefully-selected name:
   invalidate_ref_cache().

2. It gives me a chance to submit the first "bite" of my scalable-refs
   changes.

These patches apply on top of mh/iterate-refs, which is in next but
not in master.

This patch series provides an API for external code to invalidate the
ref cache that is used internally to refs.c.  It also allows code
*within* refs.c to invalidate only the packed or only the loose refs
for a module/submodule.

IMPORTANT:

I won't myself have time to figure out who, outside of refs.c, has to
*call* invalidate_ref_cache().  The candidates that I know off the top
of my head are git-clone, git-submodule, and git-pack-refs.  It would
be great if experts in those areas would insert calls to
invalidate_ref_cache() where needed.

Even better would be if the meddlesome code were changed to use the
refs API.  I'd be happy to help expanding the refs API if needed to
accommodate your needs.

This is why the API for invalidating only packed or loose refs is
private.  After code outside refs.c is changed to use the refs API, it
will get the optimal behavior for free (and at that time
invalidate_ref_cache() can be removed again).

Michael Haggerty (7):
  invalidate_ref_cache(): rename function from invalidate_cached_refs()
  invalidate_ref_cache(): take the submodule as parameter
  invalidate_ref_cache(): expose this function in refs API
  clear_cached_refs(): rename parameter
  clear_cached_refs(): extract two new functions
  write_ref_sha1(): only invalidate the loose ref cache
  clear_cached_refs(): inline function

 refs.c |   34 +++++++++++++++++++---------------
 refs.h |    8 ++++++++
 2 files changed, 27 insertions(+), 15 deletions(-)

-- 
1.7.7.rc2

^ permalink raw reply

* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Matthieu Moy @ 2011-10-10  7:50 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, git, Jay Soffian, Ramkumar Ramachandra
In-Reply-To: <m3ehyl1g5v.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Yet another issue is if we should blindly trust automatic merge resolution.
> It is considered a good practice by some to always check (e.g. by compiling
> and possibly also running tests) the result of merge, whether it required
> merge conflict resolution or not.

I agree that trusting merge blindly is bad, but still, if there are no
merge conflicts, and if the merge is broken, I'd prefer commiting a
fixup patch right after the merge than fixing it before committing.
Because if the merge needs a fix, it usually means something tricky that
deserves its own patch and commit message. At worse, one can still reset
--merge HEAD^.

One other issue with not committing automatically is for beginners. I
see that all the time when the merge has conflicts. newbies fix the
conflicts, and when they're done: "fine, conflicts solved, let's
continue hacking" without committing. The resulting history is totally
messy because it mixes merges and actual edits. For these users, not
committing automatically in the absence of conflict would make the
situation even worse.

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

^ permalink raw reply

* Re: [PATCH v2] Teach merge the '[-e|--edit]' option
From: Jakub Narebski @ 2011-10-10  7:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jay Soffian, Ramkumar Ramachandra
In-Reply-To: <7vr52lo1m3.fsf@alter.siamese.dyndns.org>

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

> By the way, on the other side of this same coin lies another use case
> (different from the one in the footnote in the previous message) for
> "merge --no-commit". When you know that a particular merge _will_ need
> semantic adjustments, even if it were to textually merge cleanly, you
> would want the command to ask you for help to come up with the final tree,
> instead of trusting the clean automerge result. This often happens when
> the topic branch you are about to merge has changed the semantics of an
> existing function (e.g. adding a new parameter) while the branch you are
> on has added new callsite to the function (or the other way around). In
> such a merge, you would need to adjust the new callsite that does not know
> about the additional parameter to the new function signature.  For exactly
> the same reason, it is not a kosher advice to give to users of modern Git
> to "interfere with the merge with 'merge --no-commit', and then conclude
> with 'commit'", as 'commit' has less information than 'merge' itself what
> 'merge' wants to do in addition to recording the result as a 'commit'.

Yet another issue is if we should blindly trust automatic merge resolution.
It is considered a good practice by some to always check (e.g. by compiling
and possibly also running tests) the result of merge, whether it required
merge conflict resolution or not.

IIRC Linus lately said that making "git merge" automatically commit
was one of bad design decisions of git, for the above reason...

-- 
Jakub Narębski

^ permalink raw reply


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