Git development
 help / color / mirror / Atom feed
* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Felipe Contreras @ 2012-12-13 22:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwxhycii.fsf@alter.siamese.dyndns.org>

On Thu, Dec 13, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>>>>  New remote helper for bzr (v3).  With minor fixes, this may be ready
>>>>>  for 'next'.
>>>>
>>>> What minor fixes?
>>>
>>> Lookng at the above (fixup), $gmane/210744 comes to mind
>>
>> That doesn't matter. The code and the tests would work just fine.
>
> One of us must be very confused.  Perhaps you were looking at a
> wrong message (or I quoted a wrong one?).
>
>   ... goes and double checks ...
>
> One of the review points were about this piece in the test:
>
>     > +cmd=<<EOF
>     > +import bzrlib
>     > +bzrlib.initialize()
>     > +import bzrlib.plugin
>     > +bzrlib.plugin.load_plugins()
>     > +import bzrlib.plugins.fastimport
>     > +EOF
>     > +if ! "$PYTHON_PATH" -c "$cmd"; then
>
>     I cannot see how this could have ever worked.
>
> And I still don't see how your "would work just fine" can be true.

As I have explained, all this code is the equivalent of python -c '',
or rather, it's a no-op. It works in the sense that it doesn't break
anything.

The purpose of the code is to check for the fastimport plug-in, but
that plug-in is not used any more, it's vestigial code, it doesn't
matter if the check works or not, as long as it doesn't fail.

>>> but there may be others.  It is the responsibility of a contributor to keep
>>> track of review comments others give to his or her patches and
>>> reroll them, so I do not recall every minor details, sorry.
>
> There may be others, but $gmane/210745 is also relevant, I think.

I'll comment on the patch, but I don't think it really prevents the merge.

>> There is nothing that prevents remote-bzr from being merged.
>
> What we merge may not be perfect.  Bugs and misdesigns are often
> identified long after a topic is merged and it is perfectly normal
> we fix things up in-tree.  There are even times where we say "OK, it
> is known to break if the user does something that pokes this and
> that obscure corners of this code, but the benefit of merging this
> 99% working code to help users that do not exercise the rare cases
> is greater than having them wait for getting the remaining 1% right,
> so let's merge it with known breakage documentation".
>
> But it is totally a different matter to merge a crap with known
> breakage that is one easy fix away from the get-go.  Allowing that
> means that all the times we spend on reviewing patches here go
> wasted, discouraging reviewers.

There is no breakage.

> If you want others to take your patches with respect, please take
> reviewers' comments with the same respect you expect to be paid by
> others.

I don't need others to take my patches with respect, my patches are
not people, they don't have feelings.

That being said, I don't think I have disrespected any of your
comments. Yes, you are right that the above code is wrong and doesn't
do anything, what part of agreeing is disrespectful? But I don't agree
that it is a merge blocker. Disagreeing is not disrespecting.

This code was ready for 1.8.1, but it's not going to be there, so, I
don't see any hurry. As I said, I think the code is ready, and these
minor details can wait.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* asd
From: Philipp Bartsch @ 2012-12-13 21:25 UTC (permalink / raw)
  To: git

subscribe git

^ permalink raw reply

* [PATCH] For git-subtree, when installing docs (make install-doc), create man1 folder first.
From: Jesper L. Nielsen @ 2012-12-13 20:09 UTC (permalink / raw)
  To: git, gitster; +Cc: Jesper L. Nielsen

From: "Jesper L. Nielsen" <lyager@gmail.com>

Hi..

I installed Git subtree and discovered that the if the man1dir doesn't exist the man-page for Git Subtree is just called man1.

So, small patch to create the folder first in the Makefile. Hope everything is right with the patch and submitting of the patch.

Best Regards
Jesper

Signed-off-by: Jesper L. Nielsen <lyager@gmail.com>
---
 contrib/subtree/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 05cdd5c..a341cf4 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -35,6 +35,7 @@ install: $(GIT_SUBTREE)
 install-doc: install-man
 
 install-man: $(GIT_SUBTREE_DOC)
+	mkdir -p $(man1dir)
 	$(INSTALL) -m 644 $^ $(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
-- 
1.8.0.2

^ permalink raw reply related

* Re: [PATCHv2] Add directory pattern matching to attributes
From: Junio C Hamano @ 2012-12-13 20:08 UTC (permalink / raw)
  To: Jean-Noël AVILA; +Cc: git
In-Reply-To: <201212082104.39411.avila.jn@gmail.com>

"Jean-Noël AVILA" <avila.jn@gmail.com> writes:

> The manpage of gitattributes says: "The rules how the pattern
> matches paths are the same as in .gitignore files" and the gitignore
> pattern matching has a pattern ending with / for directory matching.
>
> This rule is specifically relevant for the 'export-ignore' rule used
> for git archive.
>
> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> ---
>  archive.c                       |    3 ++-
>  attr.c                          |   32 ++++++++++++++++------
>  t/t5002-archive-attr-pattern.sh |   57 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+), 9 deletions(-)
>  create mode 100644 t/t5002-archive-attr-pattern.sh

Looks nicely done.

> diff --git a/attr.c b/attr.c
> index 097ae87..cdba88a 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -564,17 +564,31 @@ static void bootstrap_attr_stack(void)
>  	attr_stack = elem;
>  }
>  
> +static const char *find_basename(const char *path)
> +{
> +	char pathbuf[PATH_MAX];
> +	int pathlen;
> +	const char *cp;
> +
> +	pathlen =strlen(path);
> +	if (path[pathlen-1] != '/') {
> +		cp =strrchr(path, '/');
> +		return cp ? cp + 1: path;
> +	} else {
> +		strncpy(pathbuf, path, pathlen);
> +		pathbuf[pathlen-1] = '\0';
> +		cp =strrchr(pathbuf, '/');
> +		return cp ? path + (cp - pathbuf) + 1 : path;
> +	}
> +}

Let's do this function like this instead; shorter and equally easy
to understand.

        static const char *find_basename(const char *path)
        {
                const char *cp, *last_slash = NULL;

                for (cp = path; *cp; cp++) {
                        if (*cp == '/' && cp[1])
                                last_slash = cp;
                }
                return last_slash ? last_slash + 1 : path;
        }

Thanks; will queue.

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Junio C Hamano @ 2012-12-13 19:31 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s3uyC0V6ycTv78mG36_i7ugMLwwNk2cqNZatEJuL7Ee1w@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>>>  New remote helper for bzr (v3).  With minor fixes, this may be ready
>>>>  for 'next'.
>>>
>>> What minor fixes?
>>
>> Lookng at the above (fixup), $gmane/210744 comes to mind
>
> That doesn't matter. The code and the tests would work just fine.

One of us must be very confused.  Perhaps you were looking at a
wrong message (or I quoted a wrong one?).

  ... goes and double checks ...

One of the review points were about this piece in the test:

    > +cmd=<<EOF
    > +import bzrlib
    > +bzrlib.initialize()
    > +import bzrlib.plugin
    > +bzrlib.plugin.load_plugins()
    > +import bzrlib.plugins.fastimport
    > +EOF
    > +if ! "$PYTHON_PATH" -c "$cmd"; then

    I cannot see how this could have ever worked.

And I still don't see how your "would work just fine" can be true.

>> but there may be others.  It is the responsibility of a contributor to keep
>> track of review comments others give to his or her patches and
>> reroll them, so I do not recall every minor details, sorry.

There may be others, but $gmane/210745 is also relevant, I think.

> There is nothing that prevents remote-bzr from being merged.

What we merge may not be perfect.  Bugs and misdesigns are often
identified long after a topic is merged and it is perfectly normal
we fix things up in-tree.  There are even times where we say "OK, it
is known to break if the user does something that pokes this and
that obscure corners of this code, but the benefit of merging this
99% working code to help users that do not exercise the rare cases
is greater than having them wait for getting the remaining 1% right,
so let's merge it with known breakage documentation".

But it is totally a different matter to merge a crap with known
breakage that is one easy fix away from the get-go.  Allowing that
means that all the times we spend on reviewing patches here go
wasted, discouraging reviewers.

If you want others to take your patches with respect, please take
reviewers' comments with the same respect you expect to be paid by
others.

^ permalink raw reply

* Re: [PATCH] Fix sizeof usage in get_permutations
From: Junio C Hamano @ 2012-12-13 19:11 UTC (permalink / raw)
  To: Matthew Daley; +Cc: git
In-Reply-To: <1355405790-20302-1-git-send-email-mattjd@gmail.com>

Thanks; it shows how rarely this obscure tool is used these days.

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
From: Felipe Contreras @ 2012-12-13 19:06 UTC (permalink / raw)
  To: Max Horn; +Cc: Junio C Hamano, git
In-Reply-To: <BF9B1394-0321-4F1C-AD1B-F40D02DBE71A@quendi.de>

On Thu, Dec 13, 2012 at 6:04 AM, Max Horn <postbox@quendi.de> wrote:
>
> On 13.12.2012, at 11:08, Felipe Contreras wrote:
>
>> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>>>> New remote helper for bzr (v3).  With minor fixes, this may be ready
>>>>> for 'next'.
>>>>
>>>> What minor fixes?
>>>
>>> Lookng at the above (fixup), $gmane/210744 comes to mind
>>
>> That doesn't matter. The code and the tests would work just fine.
>
>
> It doesn't matter? I find that statement hard to align with what the maintainer of git, and thus the person who decides whether your patch series gets merged or not, wrote just above? In fact, it seems to me that what Junio said matters a great deal...

So you think Junio knows more about remote-bzr than I do? I repeat; it
doesn't affect the tests, it doesn't affect the code, it doesn't cause
any problem. remote-bzr could be merged today, in fact, it could have
been merged a month ago.

You don't trust me? Here, look:

	cmd=<<EOF
	import bzrlib
	bzrlib.initialize()
	import bzrlib.plugin
	bzrlib.plugin.load_plugins()
	import bzrlib.plugins.fastimport
	EOF

	if ! "$PYTHON_PATH" -c "$cmd"; then
		echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
		skip_all='skipping remote-bzr tests; bzr-fastimport not available'
		test_done
	fi

All this code is a no-op, because, as Junio pointed out, cmd is null.
How is that a problem? It's not. The first version of remote-bzr
relied on the bazaar fastimport plug-in, so this check was needed, in
case you had bazaar, but not this particular plug-in, but today
remote-bzr doesn't need this plug-in, so this chunk of code should be
removed. The fact that this code does nothing (because python -c ''
does nothing) is *not a problem*.

In fact, even if that code failed 100% of the time, it wouldn't hurt
anybody, because 'make -C t' would work, everything would work, the
only thing that would fail is 'make -C contrib/remote-helpers/
test-bzr', which very very few people would consider a problem. But it
doesn't fail, it works.

Who benefits by delaying the merging of this code? Nobody. Who gets
hurt? The users, of course.

> This is a very strange attitude...
>
> In another email, you complained about nobody reviewing your patches respectively nobody voicing any constructive criticism. Yet Junio did just that, and again in $gmane/210745 -- and you replied to neither, and acted on neither (not even by refuting the points brought up), and now summarily dismiss them as irrelevant. I find that quite disturbing :-(.

I didn't say it was irrelevant, it should be fixed, but Junio said
"With minor fixes, this may be ready for 'next'." which is no true
IMO, it's ready *now*, it was ready one month ago. For 'next', this
problem doesn't matter.

The feedback is appreciated, but delaying the merging of this code for
no reason makes little sense to me. Junio, of course, can do whatever
he wants. The removal of this no-op code can wait, or it can be done
on top of v3, there's no need for re-roll, and Junio already
complained about the v3 re-roll.

And I didn't act because I was on vacations, git development is not my
only priority. And even if I had time, I don't see why I should
prioritize this fix, it's not important, the code is ready.

>>> but there may be others.  It is the responsibility of a contributor to keep
>>> track of review comments others give to his or her patches and
>>> reroll them, so I do not recall every minor details, sorry.
>>
>> There is nothing that prevents remote-bzr from being merged.
>
> Well, I think that is up to Junio to decide in the end, though :-). He wrote

No. He can decide if the code gets merged, but he is not the voice of
truth. Nothing prevents him from merging the code, except himself.
There is no known issue with the code, that is a true fact.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v2] git.txt: add missing info about --git-dir command-line option
From: Junio C Hamano @ 2012-12-13 18:54 UTC (permalink / raw)
  To: Manlio Perillo; +Cc: git
In-Reply-To: <1355421439-14024-1-git-send-email-manlio.perillo@gmail.com>

Thanks.

^ permalink raw reply

* Re: Build fixes for another obscure Unix
From: Junio C Hamano @ 2012-12-13 18:53 UTC (permalink / raw)
  To: David Michael; +Cc: git
In-Reply-To: <CAEvUa7nn9M5np3wD=Z1152K4pwNGhSKkC=rS9U=yc=UcaOxMCw@mail.gmail.com>

David Michael <fedora.dm0@gmail.com> writes:

> I've been experimenting with git running on z/OS USS.  It is not yet
> stable, but I have had to make a few fixes and generalizations in the
> build system to get it to compile.
>
> Would there be any interest in applying such individual compatibility
> fixes for this system, even if a full port doesn't reach completion?

If you post patches here, it may help you find like-minded people
who may want to join forces and help you complete the port, so by
all means.

If I pick up a partially completed series and carry it in my tree is
a separate matter.  The patch has to be cleanly done (i.e. without
too many #ifdefs) for longer-term maintainability, and it has to be
clear that the changes do not affect other platforms negatively.

^ permalink raw reply

* Re: unclear documentation of git fetch --tags option and tagopt config
From: Junio C Hamano @ 2012-12-13 18:44 UTC (permalink / raw)
  To: 乙酸鋰; +Cc: git
In-Reply-To: <CAHtLG6Ti7yPFfhTb2qfSKE1+5n4Ftey4DQeqpm3SSL-bOfspUg@mail.gmail.com>

乙酸鋰 <ch3cooli@gmail.com> writes:

> With git fetch --tags
> or remote.origin.tagopt = --tags
> git fetch only fetches tags, but not branches.
> Current documentation does not mention that no branches are fetched /
> pulled when --tags option or remote.origin.tagopt = --tags is
> specified.

In the canonical form you spell out what you want to fetch from
where, and a lazy "git fetch" or "git fetch origin" that does not
specify what are to be fetched are the special cases.  Because they
do not say what to fetch, they would become a no-op, which would not
be very useful, if there is no special casing for them.  Instead,
they use sensible default, taking refspec from the configuration
variable remote.$name.fetch.

Giving refspecs or the "--tags" option from the command line is a
way to explicitly override this default, hence:

    $ git fetch origin HEAD

only fetches the history leading to the commit at HEAD at the
remote, ignoring the configured refspecs.  As "--tags" is a synonym
to "refs/tags/*:refs/tags/*", "git fetch --tags origin" tells us to
ignore refspecs and grab only the tags, i.e.:

    $ git fetch origin "refs/tags/*:refs/tags/*"

which does not grab any branches.

You can of course do:

    $ git fetch --tags origin refs/heads/master:refs/remotes/origin/master

^ permalink raw reply

* Re: [PATCH v3 2/2] cache-tree: remove dead i-t-a code in verify_cache()
From: Junio C Hamano @ 2012-12-13 18:34 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King, Jonathon Mah
In-Reply-To: <1355362747-13474-2-git-send-email-pclouds@gmail.com>

Will replace the one in 'pu' with these two.  Thanks.

^ permalink raw reply

* Re: [PATCH 0/2] mailmap from blobs
From: Junio C Hamano @ 2012-12-13 18:23 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20121213130447.GA4353@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Subject: [PATCH] mailmap: default mailmap.blob in bare repositories
>
> The motivation for mailmap.blob is to let users of bare
> repositories use the mailmap feature, as they would not have
> a checkout containing the .mailmap file. We can make it even
> easier for them by just looking in HEAD:.mailmap by default.
>
> We can't know for sure that this is where they would keep a
> mailmap, of course, but it is the best guess (and it matches
> the non-bare behavior, which reads from HEAD:.mailmap in the
> working tree). If it's missing, git will silently ignore the
> setting.
>
> We do not do the same magic in the non-bare case, because:
>
>   1. In the common case, HEAD:.mailmap will be the same as
>      the .mailmap in the working tree, which is a no-op.
>
>   2. In the uncommon case, the user has modified .mailmap
>      but not yet committed it, and would expect the working
>      tree version to take precedence.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I went with defaulting mailmap.blob, because it provides an easy path
> for turning off the feature (you just override mailmap.blob).

Very sensibly explained.  I like it when people clearly explain the
thinking behind the change in the log message.

Thanks, will queue.


>  Documentation/config.txt |  8 +++++---
>  mailmap.c                |  5 +++++
>  t/t4203-mailmap.sh       | 25 +++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 3760077..1a3c554 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1519,9 +1519,11 @@ mailmap.blob::
>  
>  mailmap.blob::
>  	Like `mailmap.file`, but consider the value as a reference to a
> -	blob in the repository (e.g., `HEAD:.mailmap`). If both
> -	`mailmap.file` and `mailmap.blob` are given, both are parsed,
> -	with entries from `mailmap.file` taking precedence.
> +	blob in the repository. If both `mailmap.file` and
> +	`mailmap.blob` are given, both are parsed, with entries from
> +	`mailmap.file` taking precedence. In a bare repository, this
> +	defaults to `HEAD:.mailmap`. In a non-bare repository, it
> +	defaults to empty.
>  
>  man.viewer::
>  	Specify the programs that may be used to display help in the
> diff --git a/mailmap.c b/mailmap.c
> index 5ffe48a..b16542f 100644
> --- a/mailmap.c
> +++ b/mailmap.c
> @@ -233,7 +233,12 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)
>  int read_mailmap(struct string_list *map, char **repo_abbrev)
>  {
>  	int err = 0;
> +
>  	map->strdup_strings = 1;
> +
> +	if (!git_mailmap_blob && is_bare_repository())
> +		git_mailmap_blob = "HEAD:.mailmap";
> +
>  	err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
>  	err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
>  	err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index e7ea40c..aae30d9 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -218,6 +218,31 @@ test_expect_success 'mailmap.blob can be missing' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'mailmap.blob defaults to off in non-bare repo' '
> +	git init non-bare &&
> +	(
> +		cd non-bare &&
> +		test_commit one .mailmap "Fake Name <author@example.com>" &&
> +		echo "     1	Fake Name" >expect &&
> +		git shortlog -ns HEAD >actual &&
> +		test_cmp expect actual &&
> +		rm .mailmap &&
> +		echo "     1	A U Thor" >expect &&
> +		git shortlog -ns HEAD >actual &&
> +		test_cmp expect actual
> +	)
> +'
> +
> +test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '
> +	git clone --bare non-bare bare &&
> +	(
> +		cd bare &&
> +		echo "     1	Fake Name" >expect &&
> +		git shortlog -ns HEAD >actual &&
> +		test_cmp expect actual
> +	)
> +'
> +
>  test_expect_success 'cleanup after mailmap.blob tests' '
>  	rm -f .mailmap
>  '

^ permalink raw reply

* Re: [PATCH v2] index-format.txt: be more liberal on what can represent invalid cache tree
From: Junio C Hamano @ 2012-12-13 18:11 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <7v8v921zt6.fsf@alter.siamese.dyndns.org>

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

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>>  How would that work with existing versions? If you write -2 in
>>  cache-tree, the next time 1.8.0 updates cache tree it writes -1 back.
>>  That loses whatever information you attach to -2. A new cache-tree
>>  extension is probably better.
>
> You can easily imagine a definition like this:
> ...

As we clarified that we do not allow implementations to write
anything but -1 for invalidated entries until we decide what we will
use other values for, the whole log message needs to be updated, I
think.

-- >8 --
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date: Thu, 13 Dec 2012 08:14:47 +0700
Subject: [PATCH] index-format.txt: clarify what is "invalid"

A cache-tree entry with a negative entry count is considered "invalid"
in the current Git; it records that we do not know the object name
of a tree that would result by writing the directory covered by the
cache-tree as a tree object.

Clarify that any entry with a negative entry count is invalid, but
the implementations must write -1 there. This way, we can later
decide to allow writers to use negative values other than -1 to
encode optional information without harming interoperability; we do
not know what is encoded how, so keep these other negative values as
reserved for now.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/technical/index-format.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/index-format.txt b/Documentation/technical/index-format.txt
index 9d25b30..ce28a7a 100644
--- a/Documentation/technical/index-format.txt
+++ b/Documentation/technical/index-format.txt
@@ -161,8 +161,9 @@ GIT index format
     this span of index as a tree.
 
   An entry can be in an invalidated state and is represented by having
-  -1 in the entry_count field. In this case, there is no object name
-  and the next entry starts immediately after the newline.
+  a negative number in the entry_count field. In this case, there is no
+  object name and the next entry starts immediately after the newline.
+  When writing an invalid entry, -1 should always be used as entry_count.
 
   The entries are written out in the top-down, depth-first order.  The
   first entry represents the root level of the repository, followed by the
-- 
1.8.1.rc1.141.g0ffea5d

^ permalink raw reply related

* Re: [PATCH] Documentation/git: add missing info about --git-dir command-line option
From: Manlio Perillo @ 2012-12-13 18:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwxj3vxx.fsf@alter.siamese.dyndns.org>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 12/12/2012 20:35, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
> 
>> The Documentation/git.txt file, in the GIT_DIR environment variable
>> section, did not mentioned that this value can also be set using the
>> --git-dir command line option.
>> ---
> 
> s/mentioned/mention/; Also it may help to say
> 
> 	Unlike other environment variables (e.g. GIT_WORK_TREE,
> 	GIT_NAMESPACE),
> 
> somewhere in the description.
> 
> Please sign-off your patch (see Documentation/SubmittingPatches).
> 
> Thanks.
> 

Thanks to you.

I have sent the updated patch, let me know if is ok.



Manlio Perillo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDKF9YACgkQscQJ24LbaUQyHwCcDiaJjFZ5vwHzxjHyhEBCyFdd
GnIAn34MjoWmQOcLKJEl/EpE0ImeQBLG
=yrux
-----END PGP SIGNATURE-----

^ permalink raw reply

* [PATCH v2] git.txt: add missing info about --git-dir command-line option
From: Manlio Perillo @ 2012-12-13 17:57 UTC (permalink / raw)
  To: git; +Cc: Manlio Perillo

Unlike other environment variables (e.g. GIT_WORK_TREE,	GIT_NAMESPACE),
the Documentation/git.txt file did not mention that the GIT_DIR
environment variable can also be set using the --git-dir command line
option.

Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---
 Documentation/git.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index e643683..60db292 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -650,6 +650,7 @@ git so take care if using Cogito etc.
 	If the 'GIT_DIR' environment variable is set then it
 	specifies a path to use instead of the default `.git`
 	for the base of the repository.
+	The '--git-dir' command-line option also sets this value.
 
 'GIT_WORK_TREE'::
 	Set the path to the working tree.  The value will not be
-- 
1.8.1.rc1.19.g2021cc5.dirty

^ permalink raw reply related

* RE: Build fixes for another obscure Unix
From: Pyeron, Jason J CTR (US) @ 2012-12-13 17:18 UTC (permalink / raw)
  To: git@vger.kernel.org
In-Reply-To: <CAEvUa7nn9M5np3wD=Z1152K4pwNGhSKkC=rS9U=yc=UcaOxMCw@mail.gmail.com>

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

> -----Original Message-----
> From: David Michael
> Sent: Thursday, December 13, 2012 10:23 AM
> 
> Hi,
> 
> I've been experimenting with git running on z/OS USS.  It is not yet
> stable, but I have had to make a few fixes and generalizations in the
> build system to get it to compile.

Maybe it would help address other build issues on other platforms.

> 
> Would there be any interest in applying such individual compatibility
> fixes for this system, even if a full port doesn't reach completion?

What are the down sides? Can your changes be shown to not impact builds on other systems?

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5615 bytes --]

^ permalink raw reply

* Re: [PATCH] Fix sizeof usage in get_permutations
From: Joachim Schmitz @ 2012-12-13 16:13 UTC (permalink / raw)
  To: git
In-Reply-To: <1355405790-20302-1-git-send-email-mattjd@gmail.com>

Matthew Daley wrote:
> Currently it gets the size of an otherwise unrelated, unused variable
> instead of the expected struct size.
> 
> Signed-off-by: Matthew Daley <mattjd@gmail.com>
> ---
> builtin/pack-redundant.c |    6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> index f5c6afc..7544687 100644
> --- a/builtin/pack-redundant.c
> +++ b/builtin/pack-redundant.c
> @@ -301,14 +301,14 @@ static void pll_free(struct pll *l)
>  */
> static struct pll * get_permutations(struct pack_list *list, int n)
> {
> - struct pll *subset, *ret = NULL, *new_pll = NULL, *pll;
> + struct pll *subset, *ret = NULL, *new_pll = NULL;
> 
>  if (list == NULL || pack_list_size(list) < n || n == 0)
>  return NULL;
> 
>  if (n == 1) {
>  while (list) {
> - new_pll = xmalloc(sizeof(pll));
> + new_pll = xmalloc(sizeof(struct pll));
>  new_pll->pl = NULL;
>  pack_list_insert(&new_pll->pl, list);
>  new_pll->next = ret;
> @@ -321,7 +321,7 @@ static struct pll * get_permutations(struct
>  pack_list *list, int n) while (list->next) {
>  subset = get_permutations(list->next, n - 1);
>  while (subset) {
> - new_pll = xmalloc(sizeof(pll));
> + new_pll = xmalloc(sizeof(struct pll));

Why not just
new_pll = xmalloc(sizeof(*new_pll));

>  new_pll->pl = subset->pl;
>  pack_list_insert(&new_pll->pl, list);
>  new_pll->next = ret;

^ permalink raw reply

* Re: [PATCH v2] submodule: add 'deinit' command
From: Marc Branchaud @ 2012-12-13 15:47 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Junio C Hamano, Michael J Gruber, Phil Hord, W. Trevor King, Git,
	Heiko Voigt, Jeff King, Shawn Pearce, Nahor
In-Reply-To: <50C90469.8080303@web.de>

On 12-12-12 05:25 PM, Jens Lehmann wrote:
> 
> So unless people agree that deinit should also remove the work
> tree I'll prepare some patches teaching all git commands to
> consistently ignore deinitialized submodules. Opinions?

I agree with Trevor's suggestion that deinit should restore the user to the
state he would be in if he were never interested in the submodule.  So clean
up .git/config and remove the work tree.  (Maybe just issue a warning instead
if the submodule's work tree is dirty.)

Also, given that semantic, I agree with Michael that a bare "git submodule
deinit" should *not* deinitialize all the submodules.  It should require a
"--all" for that.  The bare command should just print a usage summary.

		M.

^ permalink raw reply

* RE: FW: Git log --graph doesn't output color when redirected
From: Srb, Michal @ 2012-12-13 15:34 UTC (permalink / raw)
  To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20121213131329.GA5042@sigill.intra.peff.net>

From: Jeff King [peff@peff.net]
Sent: Thursday, December 13, 2012 1:13 PM

>> Is there a setting somewhere in config to change this?

> Yes. If you use "--color" on the command line, that means
> "unconditionally use color". If you set color.ui (or any other
> color config option) to "always", then you will always get color (and
> you can disable it for a particular run with "--no-color"). Setting a color
> config option to "true" is the same as "auto", which gets you 
> the auto mode.
 
Setting color in gitconfig didn't work for me on msys, but --color 
works like magic, thanks!

^ permalink raw reply

* Build fixes for another obscure Unix
From: David Michael @ 2012-12-13 15:22 UTC (permalink / raw)
  To: git

Hi,

I've been experimenting with git running on z/OS USS.  It is not yet
stable, but I have had to make a few fixes and generalizations in the
build system to get it to compile.

Would there be any interest in applying such individual compatibility
fixes for this system, even if a full port doesn't reach completion?

Thanks.

David

^ permalink raw reply

* [PATCH] Fix sizeof usage in get_permutations
From: Matthew Daley @ 2012-12-13 13:36 UTC (permalink / raw)
  To: git; +Cc: Matthew Daley

Currently it gets the size of an otherwise unrelated, unused variable
instead of the expected struct size.

Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 builtin/pack-redundant.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index f5c6afc..7544687 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -301,14 +301,14 @@ static void pll_free(struct pll *l)
  */
 static struct pll * get_permutations(struct pack_list *list, int n)
 {
-	struct pll *subset, *ret = NULL, *new_pll = NULL, *pll;
+	struct pll *subset, *ret = NULL, *new_pll = NULL;
 
 	if (list == NULL || pack_list_size(list) < n || n == 0)
 		return NULL;
 
 	if (n == 1) {
 		while (list) {
-			new_pll = xmalloc(sizeof(pll));
+			new_pll = xmalloc(sizeof(struct pll));
 			new_pll->pl = NULL;
 			pack_list_insert(&new_pll->pl, list);
 			new_pll->next = ret;
@@ -321,7 +321,7 @@ static struct pll * get_permutations(struct pack_list *list, int n)
 	while (list->next) {
 		subset = get_permutations(list->next, n - 1);
 		while (subset) {
-			new_pll = xmalloc(sizeof(pll));
+			new_pll = xmalloc(sizeof(struct pll));
 			new_pll->pl = subset->pl;
 			pack_list_insert(&new_pll->pl, list);
 			new_pll->next = ret;
-- 
1.7.10.4

^ permalink raw reply related

* Re: FW: Git log --graph doesn't output color when redirected
From: Jeff King @ 2012-12-13 13:13 UTC (permalink / raw)
  To: Srb, Michal; +Cc: git@vger.kernel.org
In-Reply-To: <72BB37CB88C48F4B925365539F1EE46C18261403@icexch-m3.ic.ac.uk>

On Wed, Dec 12, 2012 at 05:35:17PM +0000, Srb, Michal wrote:

> Unlike --pretty-format, --graph doesn’t output colors when the git log output
> is redirected.

I do not think it has anything to do with --graph in particular, but
rather that when colorization is set to the "auto" mode, it is enabled
only when stdout is a tty. Diff coloring, for example, follows the same
rules.  If you are using --format="%C(red)" or similar placeholders,
they are the odd duck by not respecting the auto-color mode.

> Is there a setting somewhere in config to change this?

Yes. If you use "--color" on the command line, that means
"unconditionally use color". If you set color.ui (or any other color
config option) to "always", then you will always get color (and you can
disable it for a particular run with "--no-color"). Setting a color
config option to "true" is the same as "auto", which gets you the auto
mode.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] mailmap: support reading mailmap from blobs
From: Jeff King @ 2012-12-13 13:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20121212110404.GB19653@sigill.intra.peff.net>

On Wed, Dec 12, 2012 at 06:04:04AM -0500, Jeff King wrote:

> In a bare repository, there isn't a simple way to respect an
> in-tree mailmap without extracting it to a temporary file.
> This patch provides a config variable, similar to
> mailmap.file, which reads the mailmap from a blob in the
> repository.

While preparing the "default mailmap.blob" patch, I noticed a few loose
ends in the documentation. They can be squashed into this 2/2
(jk/mailmap-from-blob~1), or can go on top of the series:

-- >8 --
Subject: [PATCH] fix some documentation loose-ends for mailmap.blob

Anywhere we mention mailmap.file, it is probably worth
mentioning mailmap.blob, as well.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-log.txt | 2 +-
 Documentation/mailmap.txt | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 585dac4..08a185d 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -167,7 +167,7 @@ log.showroot::
 	`git log -p` output would be shown without a diff attached.
 	The default is `true`.
 
-mailmap.file::
+mailmap.*::
 	See linkgit:git-shortlog[1].
 
 notes.displayRef::
diff --git a/Documentation/mailmap.txt b/Documentation/mailmap.txt
index 288f04e..bb349c2 100644
--- a/Documentation/mailmap.txt
+++ b/Documentation/mailmap.txt
@@ -1,5 +1,6 @@ If the file `.mailmap` exists at the toplevel of the repository, or at
 If the file `.mailmap` exists at the toplevel of the repository, or at
-the location pointed to by the mailmap.file configuration option, it
+the location pointed to by the mailmap.file or mailmap.blob
+configuration options, it
 is used to map author and committer names and email addresses to
 canonical real names and email addresses.
 
-- 
1.8.0.2.4.g59402aa

^ permalink raw reply related

* Re: [PATCH 0/2] mailmap from blobs
From: Jeff King @ 2012-12-13 13:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20121212175900.GA32767@sigill.intra.peff.net>

On Wed, Dec 12, 2012 at 12:59:00PM -0500, Jeff King wrote:

> > Have you considered defaulting to read from HEAD:.mailmap even when
> > this new configuration is not there if core.bare is set?  I would
> > imagine that it would be the most convenient and match people's
> > expectations.
> 
> Yeah, I almost suggested that, but I figured it could wait for the
> feature to prove itself in the real world before turning it on by
> default. It _should_ be pretty harmless, though, so I don't mind turning
> it on by default.

That patch would look like this:

-- >8 --
Subject: [PATCH] mailmap: default mailmap.blob in bare repositories

The motivation for mailmap.blob is to let users of bare
repositories use the mailmap feature, as they would not have
a checkout containing the .mailmap file. We can make it even
easier for them by just looking in HEAD:.mailmap by default.

We can't know for sure that this is where they would keep a
mailmap, of course, but it is the best guess (and it matches
the non-bare behavior, which reads from HEAD:.mailmap in the
working tree). If it's missing, git will silently ignore the
setting.

We do not do the same magic in the non-bare case, because:

  1. In the common case, HEAD:.mailmap will be the same as
     the .mailmap in the working tree, which is a no-op.

  2. In the uncommon case, the user has modified .mailmap
     but not yet committed it, and would expect the working
     tree version to take precedence.

Signed-off-by: Jeff King <peff@peff.net>
---
I went with defaulting mailmap.blob, because it provides an easy path
for turning off the feature (you just override mailmap.blob).

Another way of thinking about this would be that it is the bare analog
to "read .mailmap from the working tree", and the logic should be:

  if (is_bare_repository())
          read_mailmap_blob(map, "HEAD:.mailmap");
  else
          read_mailmap_file(map, ".mailmap");
  read_mailmap_blob(map, git_mailmap_blob);
  read_mailmap_file(map, git_mailmap_file);

However, the current is not exactly "read from the root of the working
tree". It is "read from the current directory", and it works because all
of the callers run from the toplevel of the working tree (when one
exists). That means that bare repositories have always read from
$GIT_DIR/.mailmap. I doubt that was intentional, but people with bare
repositories may be depending on it.  So I think that falls into the
"how I might do it from scratch" bin.

We could still do:

  if (is_bare_repository())
          read_mailmap_blob(map, "HEAD:.mailmap");
  read_mailmap_file(map, ".mailmap");
  read_mailmap_blob(map, git_mailmap_blob);
  read_mailmap_file(map, git_mailmap_file);

but IMHO that is just making the rules more complicated to explain for
little benefit (and in fact, you lose the ability to suppress the HEAD
mailmap, which you might care about if you are hosting multiple bits of
unrelated history in the same repo).

 Documentation/config.txt |  8 +++++---
 mailmap.c                |  5 +++++
 t/t4203-mailmap.sh       | 25 +++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3760077..1a3c554 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1519,9 +1519,11 @@ mailmap.blob::
 
 mailmap.blob::
 	Like `mailmap.file`, but consider the value as a reference to a
-	blob in the repository (e.g., `HEAD:.mailmap`). If both
-	`mailmap.file` and `mailmap.blob` are given, both are parsed,
-	with entries from `mailmap.file` taking precedence.
+	blob in the repository. If both `mailmap.file` and
+	`mailmap.blob` are given, both are parsed, with entries from
+	`mailmap.file` taking precedence. In a bare repository, this
+	defaults to `HEAD:.mailmap`. In a non-bare repository, it
+	defaults to empty.
 
 man.viewer::
 	Specify the programs that may be used to display help in the
diff --git a/mailmap.c b/mailmap.c
index 5ffe48a..b16542f 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -233,7 +233,12 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)
 int read_mailmap(struct string_list *map, char **repo_abbrev)
 {
 	int err = 0;
+
 	map->strdup_strings = 1;
+
+	if (!git_mailmap_blob && is_bare_repository())
+		git_mailmap_blob = "HEAD:.mailmap";
+
 	err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
 	err |= read_mailmap_blob(map, git_mailmap_blob, repo_abbrev);
 	err |= read_mailmap_file(map, git_mailmap_file, repo_abbrev);
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index e7ea40c..aae30d9 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -218,6 +218,31 @@ test_expect_success 'mailmap.blob can be missing' '
 	test_cmp expect actual
 '
 
+test_expect_success 'mailmap.blob defaults to off in non-bare repo' '
+	git init non-bare &&
+	(
+		cd non-bare &&
+		test_commit one .mailmap "Fake Name <author@example.com>" &&
+		echo "     1	Fake Name" >expect &&
+		git shortlog -ns HEAD >actual &&
+		test_cmp expect actual &&
+		rm .mailmap &&
+		echo "     1	A U Thor" >expect &&
+		git shortlog -ns HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '
+	git clone --bare non-bare bare &&
+	(
+		cd bare &&
+		echo "     1	Fake Name" >expect &&
+		git shortlog -ns HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'cleanup after mailmap.blob tests' '
 	rm -f .mailmap
 '
-- 
1.8.0.2.4.g59402aa

^ permalink raw reply related

* Re: [PATCH] git(1): remove a defunct link to "list of authors"
From: Jeff King @ 2012-12-13 12:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7v8v935en3.fsf@alter.siamese.dyndns.org>

On Wed, Dec 12, 2012 at 10:06:24AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I find the ohloh one a little more informative than the GitHub graph. I
> > couldn't find any others (Google Code does not seem to have one,
> > kernel.org and other gitweb sites do not, and I can't think of anywhere
> > else that hosts a mirror).
> 
> Then let's do this.
> 
> -- >8 --
> Subject: git(1): show link to contributor summary page
> 
> We earlier removed a link to list of contributors that pointed to a
> defunct page; let's use a working one from Ohloh.net to replace it
> instead.

Looks good to me. Thanks.

-Peff

^ permalink raw reply


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