Git development
 help / color / mirror / Atom feed
* Re: General support for ! in git-config values
From: Junio C Hamano @ 2012-02-03  6:11 UTC (permalink / raw)
  To: Kyle Moffett
  Cc: demerphq, Jeff King, Ævar Arnfjörð,
	Git Mailing List
In-Reply-To: <CAGZ=bq++R+X+2r2_zQ4UZ6JvDC9W9_4nF23MQ6+612_Qe2RS4Q@mail.gmail.com>

Kyle Moffett <kyle@moffetthome.net> writes:

> Alternatively, you could extend the recent proposal for GIT config
> "include" statements so that something like this works:
>
> [include]
>     exec = echo "deploy.prefix = `cat /etc/SERVER_ROLE`"
>     exec = /usr/local/bin/git-config-for-ldap-user

Erh...

Running known stuff from your own .git/config may be justifiable as "at
your own risk", but if we consider sources that are not under your direct
control, such as /etc/gitconfig and whatever your project encourages you
to include from your .git/config,... eek.

^ permalink raw reply

* Re: [PATCH, RFC] Fix build problems related to profile-directed optimization
From: Junio C Hamano @ 2012-02-03  6:00 UTC (permalink / raw)
  To: Ted Ts'o; +Cc: git, Andi Kleen, Clemens Buchacher
In-Reply-To: <20120203020743.GE1032@thunk.org>

Ted Ts'o <tytso@mit.edu> writes:

> What do you think of this?  I'm still running a test build --- "make
> PROFILE=BUILD all" takes quite a long time, so this is still an RFC; I
> figure there will still be some places where people will point out
> more nits to be polished.  :-)
>
> (In particular, I just noticed I left the V=1 for debugging purposes
> in this version....)

Thanks.

Three comments:

 * I am happy that this version handles this well:

   $ make PROFILE=BUILD install

   even though you did not advertise as such in INSTALL ;-).

 * However, I think "clean" target should remove *.gcda unconditionally.

   $ make PROFILE=BUILD install ; make clean ; git clean -n -x | grep gcda

 * Running "make PROFILE=BUILD install" immediately after another one,
   without "make clean" in between, resulted in full rebuild and test
   before the second "install", which somewhat surprised me.  I however do
   not think this is a big show-stopper problem.

^ permalink raw reply

* Re: How best to handle multiple-authorship commits in GIT?
From: Junio C Hamano @ 2012-02-03  5:11 UTC (permalink / raw)
  To: Valerie Aurora; +Cc: David Howells, git@vger.kernel.org, Rusty Russell
In-Reply-To: <CAD-XujkVK=tOtmVS90U0KAutFZ55jxsHMKuuMppXOi-H6ZY=RQ@mail.gmail.com>

Valerie Aurora <valerie.aurora@gmail.com> writes:

> And Rusty's practice as of a year or two ago is for "minor mods" to a
> patch, to leave the authorship the same, and add a Signed-off-by:
>
> Signed-off-by: Some Upstream Author
> Signed-off-by: Maintainer or Merger (rewrote error handling)
>
> And for a complete (meaningful) rewrite such as David has done, he
> changes the commit authorship and adds a Signed-off-by for the
> original author.
>
> That's existing guidelines and practice.

All sounds very sensible.  Thanks for a summary.

^ permalink raw reply

* Re: General support for ! in git-config values
From: Kyle Moffett @ 2012-02-03  5:08 UTC (permalink / raw)
  To: demerphq
  Cc: Jeff King, Junio C Hamano, Ævar Arnfjörð,
	Git Mailing List
In-Reply-To: <CANgJU+XoZd6x6jdSHszigZaPgi+6H3Nbf4OG7p0y1_=7m+qntA@mail.gmail.com>

On Thu, Feb 2, 2012 at 02:21, demerphq <demerphq@gmail.com> wrote:
>> So far I haven't seen an actual patch to comment on
>> (or even a proposed syntax beyond starting a string with "!", which I
>> think is a non-starter due to conflicting with existing uses),
>
> I understand. I think we will probably use backtick quoting in git-deploy. So
>
> deploy.prefix=`cat /etc/SERVER_ROLE`
>
> will execute cat /etc/SERVER_ROLE and use the results as the value of
> the config option.

Alternatively, you could extend the recent proposal for GIT config
"include" statements so that something like this works:

[include]
    exec = echo "deploy.prefix = `cat /etc/SERVER_ROLE`"
    exec = /usr/local/bin/git-config-for-ldap-user

Thoughts?

Cheers,
Kyle Moffett

^ permalink raw reply

* Re: how to determine oldest supported version of git
From: Junio C Hamano @ 2012-02-03  4:52 UTC (permalink / raw)
  To: Neal Kreitzinger; +Cc: git
In-Reply-To: <jgeekn$of2$1@dough.gmane.org>

"Neal Kreitzinger" <neal@rsss.com> writes:

> What is the best way for me (a git user) to determine what is currently
> the oldest supported version of git (the oldest version still getting
> bugfixes)?  IOW, when can I tell that my version of git is no longer
> supported?

"A note from the maintainer" only promises that the latest major release
(as of this writing, 1.7.9) gets regular maintenance releases until the
next major release happens.

When queuing a fix to an old bug, however, I try to build a topic branch
for that fix from as old an release as practical, in order to make sure
that older maintenance tracks could benefit, and I do give updates for
older maintenance tracks when able (but no promises).

For example, during the last cycle leading to 1.7.9, in other words, back
when 1.7.8 was the latest major release, in addition to the maintenance
releases 1.7.8.1, 1.7.8.2, 1.7.8.3 and 1.7.8.4, maintenance releases for
older version of Git were tagged (1.7.6.5, 1.7.7.5, and 1.7.7.6).  Note
that 1.7.6 was originally released on June 26th, 2011.

One cycle of major release development is expected to last between 8 to 10
weeks, so keeping two stale maintenance tracks in addition to the latest
maintenance track alive would roughly translate to 6 months shelf life for
an ancient release.

As other people mentioned, if you are on a (probably paid) support plan
from a(n enterprise) distro, asking them would be the best way, and if you
are running Git supplied as part of a distro, the distro would dictate the
version it supplies to you, so asking here would not help very much.

^ permalink raw reply

* Re: How best to handle multiple-authorship commits in GIT?
From: Valerie Aurora @ 2012-02-03  2:18 UTC (permalink / raw)
  To: David Howells; +Cc: git@vger.kernel.org, Rusty Russell
In-Reply-To: <15281.1328207789@redhat.com>

On Thu, Feb 2, 2012 at 10:36 AM, David Howells <dhowells@redhat.com> wrote:
> Valerie Aurora <valerie.aurora@gmail.com> wrote:
>
>> There is a line in the kernel doc saying how this should be handled,
>> suggested by Rusty, but it's not being followed.
>
> Do you know where?

A fault in my memory - I asked Rusty for advice on a related problem
and he gave me additional advice, he didn't suggest (so far as I know)
the changes to the kernel docs.  Here's what SubmittingPatches has to
say:

"If you are a subsystem or branch maintainer, sometimes you need to slightly
modify patches you receive in order to merge them, because the code is not
exactly the same in your tree and the submitters'. If you stick strictly to
rule (c), you should ask the submitter to rediff, but this is a totally
counter-productive waste of time and energy. Rule (b) allows you to adjust
the code, but then it is very impolite to change one submitter's code and
make him endorse your bugs. To solve this problem, it is recommended that
you add a line between the last Signed-off-by header and yours, indicating
the nature of your changes. While there is nothing mandatory about this, it
seems like prepending the description with your mail and/or name, all
enclosed in square brackets, is noticeable enough to make it obvious that
you are responsible for last-minute changes. Example :

	Signed-off-by: Random J Developer <random@developer.example.org>
	[lucky@maintainer.example.org: struct foo moved from foo.c to foo.h]
	Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org>

This practise is particularly helpful if you maintain a stable branch and
want at the same time to credit the author, track changes, merge the fix,
and protect the submitter from complaints. Note that under no circumstances
can you change the author's identity (the From header), as it is the one
which appears in the changelog."

And Rusty's practice as of a year or two ago is for "minor mods" to a
patch, to leave the authorship the same, and add a Signed-off-by:

Signed-off-by: Some Upstream Author
Signed-off-by: Maintainer or Merger (rewrote error handling)

And for a complete (meaningful) rewrite such as David has done, he
changes the commit authorship and adds a Signed-off-by for the
original author.

That's existing guidelines and practice.

-VAL

^ permalink raw reply

* Re: [PATCH, RFC] Fix build problems related to profile-directed optimization
From: Ted Ts'o @ 2012-02-03  2:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Andi Kleen, Clemens Buchacher
In-Reply-To: <7vvcnou40u.fsf@alter.siamese.dyndns.org>

What do you think of this?  I'm still running a test build --- "make
PROFILE=BUILD all" takes quite a long time, so this is still an RFC; I
figure there will still be some places where people will point out
more nits to be polished.  :-)

(In particular, I just noticed I left the V=1 for debugging purposes
in this version....)

	      	   	       	      - Ted

>From 4bf14e732216fd1327da2e3c8c6dfc0a3f689e1b Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Thu, 2 Feb 2012 13:56:22 -0500
Subject: [PATCH] Fix build problems related to profile-directed optimization

There was a number of problems I ran into when trying the
profile-directed optimizations added by Andi Kleen in git commit
7ddc2710b9.  (This was using gcc 4.4 found on many enterprise
distros.)

1) The -fprofile-generate and -fprofile-use commands are incompatible
with ccache; the code ends up looking in the wrong place for the gcda
files based on the ccache object names.

2) If the makefile notices that CFLAGS are different, it will rebuild
all of the binaries.  Hence the recipe originally specified by the
INSTALL file ("make profile-all" followed by "make install") doesn't
work.  It will appear to work, but the binaries will end up getting
built with no optimization.

This patch fixes this by using an explicit set of options passed via
the PROFILE variable then using this to directly manipulate CFLAGS and
EXTLIBS.

The developer can run "make PROFILE=BUILD all ; make PROFILE=BUILD
install" to do an automatic two-pass build using the test suite as the
sample workload for the purpose of profiling.

Alternatively, the profiling version of binaries can be built using:

	make PROFILE=GEN PROFILE_DIR=/var/cache/profile all
	make PROFILE=GEN install

and then after git has been used a number of times, the optimized
version of the binary can be built as follows:

	make PROFILE=USE PROFILE_DIR=/var/cache/profile all
	make PROFILE=USE install

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andi Kleen <ak@linux.intel.com>
---
 INSTALL  |    4 ++--
 Makefile |   41 ++++++++++++++++++++++++++++++-----------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/INSTALL b/INSTALL
index 6fa83fe..73b654b 100644
--- a/INSTALL
+++ b/INSTALL
@@ -28,8 +28,8 @@ set up install paths (via config.mak.autogen), so you can write instead
 If you're willing to trade off (much) longer build time for a later
 faster git you can also do a profile feedback build with
 
-	$ make profile-all
-	# make prefix=... install
+	$ make --prefix=/usr PROFILE=BUILD all
+	# make --prefix=/usr PROFILE=BUILD install
 
 This will run the complete test suite as training workload and then
 rebuild git with the generated profile feedback. This results in a git
diff --git a/Makefile b/Makefile
index c457c34..7d66d5c 100644
--- a/Makefile
+++ b/Makefile
@@ -1772,6 +1772,24 @@ ifdef ASCIIDOC7
 	export ASCIIDOC7
 endif
 
+### profile feedback build
+#
+
+# Can adjust this to be a global directory if you want to do extended
+# data gathering
+PROFILE_DIR := $(CURDIR)
+
+ifeq "$(PROFILE)" "GEN"
+	CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1
+	EXTLIBS += -lgcov
+	export CCACHE_DISABLE=t
+	V=1
+else ifneq "$PROFILE" ""
+	CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1
+	export CCACHE_DISABLE=t
+	V=1
+endif
+
 # Shell quote (do not use $(call) to accommodate ancient setups);
 
 SHA1_HEADER_SQ = $(subst ','\'',$(SHA1_HEADER))
@@ -1828,7 +1846,15 @@ export DIFF TAR INSTALL DESTDIR SHELL_PATH
 
 SHELL = $(SHELL_PATH)
 
-all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
+all:: shell_compatibility_test
+
+ifeq "$(PROFILE)" "BUILD"
+all:: profile-clean
+	$(MAKE) PROFILE=GEN all
+	$(MAKE) PROFILE=GEN -j1 test
+endif
+
+all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
 ifneq (,$X)
 	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
 endif
@@ -2699,16 +2725,9 @@ cover_db_html: cover_db
 
 ### profile feedback build
 #
-.PHONY: profile-all profile-clean
-
-PROFILE_GEN_CFLAGS := $(CFLAGS) -fprofile-generate -DNO_NORETURN=1
-PROFILE_USE_CFLAGS := $(CFLAGS) -fprofile-use -fprofile-correction -DNO_NORETURN=1
+.PHONY: profile-clean
 
 profile-clean:
-	$(RM) $(addsuffix *.gcda,$(object_dirs))
-	$(RM) $(addsuffix *.gcno,$(object_dirs))
+	$(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
+	$(RM) $(addsuffix *.gcno,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
 
-profile-all: profile-clean
-	$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" all
-	$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" -j1 test
-	$(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" all
-- 
1.7.8.11.gefc1f.dirty

^ permalink raw reply related

* Re: [PATCH] i18n: po for zh_cn
From: Jiang Xin @ 2012-02-03  1:35 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git List, Nguyen Thai Ngoc Duy
In-Reply-To: <CACBZZX5AceH+pc-teFm=PE9u14PM88GLtLnNg5ENOpNni0Tsig@mail.gmail.com>

2012/2/3 Ævar Arnfjörð Bjarmason <avarab@gmail.com>:
> For now we can also avoid this whole issue and just declare that
> anything that's used for interchange or permanently stored
> (git-format-patch output, reflogs, merge commit messages etc.) will be
> in English.

An extreme case is: Hg translated 'hg --versoin' and broke many other software.
For example, redmine had to grep hg version properly using 'LC_ALL=C
hg --version'.

I think Duy‘s patch maybe better for such cases.
http://thread.gmane.org/gmane.comp.version-control.git/189453/focus=189509
The call of interactive_use makes translations only available for human
but not for interchange and permanently stored.

-- 
Jiang Xin

^ permalink raw reply

* Re: [PATCH] i18n: diff/apply statistics
From: Nguyen Thai Ngoc Duy @ 2012-02-03  1:28 UTC (permalink / raw)
  To: Jiang Xin; +Cc: Git List
In-Reply-To: <CANYiYbHtJMw25ZDbYRbe8S_GM-gR6okkDwwNXo_yxPhPrHQp+w@mail.gmail.com>

On Fri, Feb 3, 2012 at 8:23 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
> 2012/2/3 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
>> On Fri, Feb 3, 2012 at 8:12 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
>>> 2012/2/2 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
>>>>
>>>> There's another patch with similar goal:
>>>>
>>>> http://thread.gmane.org/gmane.comp.version-control.git/189453/focus=189509
>>>
>>> It's cool, especially the call of interactive_use, so that
>>> translations for the statistics line is only for human, not for scripts/pipe...
>>
>> That's the idea but they decided to allow translations unconditionally
>> in the end.
>
> Ohh, sad, not so cool. Are there any reasons?
>

Inconsistency mainly. I don't see anything sad about that though. You
should read that thread.
-- 
Duy

^ permalink raw reply

* Re: Problem pushing to a Novell share
From: Junio C Hamano @ 2012-02-03  1:28 UTC (permalink / raw)
  To: Rüdiger Kessel; +Cc: git
In-Reply-To: <CAJ4nRM1K=sCy8_0PG-NADVn4T0XG1ELC7AvtQyW-Dc1cUMzEXQ@mail.gmail.com>

Rüdiger Kessel <ruediger.kessel@gmail.com> writes:

> Trying git_mkstemp_mode() first means trying more 16000 times to create a
> random file before realizing that something might be wrong.

Well, git_mkstemps_mode() does have such a 16k loop, and it tries to
create a unique, unused file this way:

		fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
		if (fd > 0)
			return fd;
		/*
		 * Fatal error (EPERM, ENOSPC etc).
		 * It doesn't make sense to loop.
		 */
		if (errno != EEXIST)
			break;

If you do not have a directory D and try to create D/tmp_random here,
shouldn't you get an error that is _NOT_ EEXIST and trigger this break?

^ permalink raw reply

* Re: [PATCH] i18n: diff/apply statistics
From: Jiang Xin @ 2012-02-03  1:23 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git List
In-Reply-To: <CACsJy8DfdTPfwZJq1_=RzO_f67FoLGJ1Q__Qfqtg+HxKZEMp4g@mail.gmail.com>

2012/2/3 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
> On Fri, Feb 3, 2012 at 8:12 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
>> 2012/2/2 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
>>>
>>> There's another patch with similar goal:
>>>
>>> http://thread.gmane.org/gmane.comp.version-control.git/189453/focus=189509
>>
>> It's cool, especially the call of interactive_use, so that
>> translations for the statistics line is only for human, not for scripts/pipe...
>
> That's the idea but they decided to allow translations unconditionally
> in the end.

Ohh, sad, not so cool. Are there any reasons?

-- 
Jiang Xin

^ permalink raw reply

* Re: [PATCH v2] Use correct grammar in diffstat summary line
From: Junio C Hamano @ 2012-02-03  1:18 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: git, Jonathan Nieder, Ævar Arnfjörð,
	Frederik Schwarzer, Brandon Casey
In-Reply-To: <CACsJy8CcBB9OF=8a1hQEpDFzqsrkbpFKnVAcU65h_5Cnym90SQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Fri, Feb 3, 2012 at 1:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>>
>>> I take it --summary is un-i18n-able,...
>>
>> ... because?
>
> .. of scripts? We have --numstat for scripts, but I see no alternative
> to --summary.

Ahh, you meant that --summary.  I somehow mixed that up with the "N files
added, M insersion(s),...".

I've always thought that "--summary" lines belong to the same category as
"new file mode M", "rename from A" and "similarity X%" that come at the
beginning of individual patches, and it probably would not make sense to
localize them.

^ permalink raw reply

* Re: [PATCH] i18n: diff/apply statistics
From: Jiang Xin @ 2012-02-03  1:15 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git List, avarab
In-Reply-To: <CACsJy8DrNHwLGJjj4nJHDm-NEsr6c=QW-kgxmTbsRxq057keWQ@mail.gmail.com>

2012/2/2 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
> On Thu, Feb 2, 2012 at 12:22 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
>> translate oneline statistics of diff/apply.
>
> There's another patch with similar goal:
>
> http://thread.gmane.org/gmane.comp.version-control.git/189453/focus=189509

It's cool, especially the call of interactive_use, so that
translations for the statistics line is only for human, not for scripts/pipe...

--
Jiang Xin

^ permalink raw reply

* Re: [PATCH v2] Use correct grammar in diffstat summary line
From: Nguyen Thai Ngoc Duy @ 2012-02-03  1:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jonathan Nieder, Ævar Arnfjörð,
	Frederik Schwarzer, Brandon Casey
In-Reply-To: <7v8vklxfe4.fsf@alter.siamese.dyndns.org>

On Fri, Feb 3, 2012 at 1:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Wed, Feb 01, 2012 at 01:26:43PM -0800, Junio C Hamano wrote:
>>> Nice.  Will queue
>>
>> Please also squash this in (resend looks ugly and it's hard to point
>> out changes). It makes the code look less ugly, use Q_() for gettext
>> poisoning and revert am input text back as Jonathan suggested.
>>
>> I take it --summary is un-i18n-able,...
>
> ... because?

.. of scripts? We have --numstat for scripts, but I see no alternative
to --summary. Does anybody parse --summary output?
-- 
Duy

^ permalink raw reply

* Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug
From: Junio C Hamano @ 2012-02-03  1:02 UTC (permalink / raw)
  To: Jeff King; +Cc: Ben Walton, git
In-Reply-To: <20120202200240.GC9246@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I wonder if a better solution is to use a known-good shell instead of
> trying to work around problems in a bogus shell.

Yeah, I think that is a better approach.

What prevents us from doing 's|^#! */bin/sh|$#$SHELL_PATH|' on everything
in t/ directory (I am not suggesting to do this. I just want to know if
there is a reason we want hardcoded "#!/bin/sh" for some instances).

^ permalink raw reply

* Re: [PATCH, RFC] Fix build problems related to profile-directed optimization
From: Junio C Hamano @ 2012-02-03  0:58 UTC (permalink / raw)
  To: Ted Ts'o; +Cc: git, Andi Kleen, Clemens Buchacher
In-Reply-To: <20120202201226.GA1032@thunk.org>

Ted Ts'o <tytso@mit.edu> writes:

> ...
> At least in theory, it should be possible to have something which
> supports both PROFILE_GEN/PROFILE_USE as well as a combined
> PROFILE_BUILD.
>
> The hard part is that PROFILE_BUILD requires a multi-pass process; you
> need to build with one set of CFLAGS, then run the sample workload to
> get the data for your feedback directed optimizations, and then re-run
> the build with another set of CFLAGS.

Yeah, I can see how that forces us to some kludgy solution, but I tend to
agree that the separation between GEN/USE is a good thing.

> I think what we could to check
> for PROFILE_BUILD, and if it is set, do the first PROFILE_GEN / make
> test commands as part of the top-level Makefile's all: rule, and then
> do the normal build after that.

Yeah, something like that would emulate the "make profile-all" well enough
for people not to notice the change while giving us the flexibility of
GEN/USE separation. I kinda like it.

Thanks.

^ permalink raw reply

* Re: Alternates corruption issue
From: Junio C Hamano @ 2012-02-03  0:47 UTC (permalink / raw)
  To: Jeff King
  Cc: Jonathan Nieder, Richard Purdie, GIT Mailing-list, Hart, Darren,
	Ashfield, Bruce
In-Reply-To: <20120202215913.GA26727@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> @@ -324,8 +324,11 @@ const char *enter_repo(const char *path, int strict)
>  			return NULL;
>  		len = strlen(used_path);
>  		for (i = 0; suffix[i]; i++) {
> +			struct stat st;
>  			strcpy(used_path + len, suffix[i]);
> -			if (!access(used_path, F_OK)) {
> +			if (!stat(used_path, &st) &&
> +			    (S_ISREG(st.st_mode) ||
> +			    (S_ISDIR(st.st_mode) && is_git_directory(used_path)))) {

Hmm, how would this change interact with

>  				strcat(validated_path, suffix[i]);
>  				break;
>  			}

	gitfile = read_gitfile(used_path);

that appear after the context in the patch?

^ permalink raw reply

* Re: [PATCH v2 1/3] pack-objects: do not accept "--index-version=version,"
From: Junio C Hamano @ 2012-02-03  0:31 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1328109440-9510-1-git-send-email-pclouds@gmail.com>

Thanks; will queue all three in the series.

^ permalink raw reply

* Re: [PATCH v3 1/4] completion: be nicer with zsh
From: Junio C Hamano @ 2012-02-03  0:17 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <CAMP44s1gWNG+jJ6M7OnLS-1VA5YPa07LJfnrvdbsQb0MOJB7PA@mail.gmail.com>

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

> On Thu, Feb 2, 2012 at 9:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>> However, clearly I did not say it clearly enough. :) I guess it's
>>> better to take a cue from storytellers and show rather than tell.
>>
>> Very big thanks for this ;-)
>
> Not a single comment regarding what I said?

What entitles you to force me to refraining from commenting at all until I
read everything in my mailbox and after waiting for a while to make sure
there is no more to come to the thread?

In any case, "be nicer with zsh" conveys no more meaningful information
than "this is some patch about zsh".  Let's try to avoid warm and fuzzy
words that imply "goodness", e.g. "improve" and "be nicer with" because
nobody sends a patch to purposefully make Git worse and expects it to be
applied.

I found Jonathan's alternative "avoid default value assignment on : true
command" at least a bit better for the purpose of jogging the short-term
memory in the "'git shortlog v1.7.9.. contrib/completion/' tells us that
we have applied several patches, and I remember that : ${var=word} one!"
sense.  It is not super-useful for the longer term, though.

Here is what I ended up in preparation for queuing the series.  I still
haven't seen any version of 4/4, but please check $gmane/189683 and see if
that matches what you intended.  Also I am assuming $gmane/189606 relayed
by Jonathan is a squash between your 2 and 3 (which didn't reach me), so
please advise if that does not match what you want to have.

Thanks.

-- >8 --
From: Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH] completion: work around zsh option propagation bug

zsh versions from 4.3.0 to present (4.3.15) do not correctly propagate the
SH_WORD_SPLIT option into the subshell in ${foo:=$(bar)} expressions.  For
example, after running

	emulate sh
	fn () {
		var='one two'
		printf '%s\n' $var
	}
	x=$(fn)
	: ${y=$(fn)}

printing "$x" results in two lines as expected, but printing "$y" results
in a single line because $var is expanded as a single word when evaluating
fn to compute y.

So avoid the construct, and use an explicit 'test -n "$foo" || foo=$(bar)'
instead.  This fixes a bug tht caused all commands to be treated as
porcelain and show up in "git <TAB><TAB>" completion, because the list of
all commands was treated as a single word in __git_list_porcelain_commands
and did not match any of the patterns that would usually cause plumbing to
be excluded.

[jn: clarified commit message, indentation style fix]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/completion/git-completion.bash |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1496c6d..c636166 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -676,7 +676,8 @@ __git_merge_strategies=
 # is needed.
 __git_compute_merge_strategies ()
 {
-	: ${__git_merge_strategies:=$(__git_list_merge_strategies)}
+	test -n "$__git_merge_strategies" ||
+	__git_merge_strategies=$(__git_list_merge_strategies)
 }
 
 __git_complete_revlist_file ()
@@ -854,7 +855,8 @@ __git_list_all_commands ()
 __git_all_commands=
 __git_compute_all_commands ()
 {
-	: ${__git_all_commands:=$(__git_list_all_commands)}
+	test -n "$__git_all_commands" ||
+	__git_all_commands=$(__git_list_all_commands)
 }
 
 __git_list_porcelain_commands ()
@@ -947,7 +949,8 @@ __git_porcelain_commands=
 __git_compute_porcelain_commands ()
 {
 	__git_compute_all_commands
-	: ${__git_porcelain_commands:=$(__git_list_porcelain_commands)}
+	test -n "$__git_porcelain_commands" ||
+	__git_porcelain_commands=$(__git_list_porcelain_commands)
 }
 
 __git_pretty_aliases ()
-- 
1.7.9.172.ge26ae

^ permalink raw reply related

* Re: Problem pushing to a Novell share
From: Junio C Hamano @ 2012-02-02 23:45 UTC (permalink / raw)
  To: Rüdiger Kessel; +Cc: git
In-Reply-To: <CAJ4nRM0-3GWN=OYOXDdGtktU0mc8mhcoKqbo=U4Jm5AmSbPAyQ@mail.gmail.com>

Rüdiger Kessel <ruediger.kessel@gmail.com> writes:

> A quick fix is to improve the create_tmpfile() function in sha1_file.c:
>
> static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
> {
>     static struct stat sb;
>     int fd, dirlen = directory_size(filename);
>
>     if (dirlen + 20 > bufsiz) {
>         errno = ENAMETOOLONG;
>         return -1;
>     }
>
>     memcpy(buffer, filename, dirlen);
>     buffer[dirlen-1] = 0;
>     if (stat(buffer, &sb) != 0 && errno == ENOENT) {
>         if (mkdir(buffer, 0777) || adjust_shared_perm(buffer))
>             return -1;
>     }
>
>     memcpy(buffer, filename, dirlen);
>     strcpy(buffer + dirlen, "tmp_obj_XXXXXX");
>     fd = git_mkstemp_mode(buffer, 0444);
>     return fd;
> }
>
> The function will create one missing directory level if it does not
> exist prior to creating the tmp-file.

Please remove your modification above, and follow the code we have.  We
try git_mkstemp_mode() first, because it is more common than not having
the leading directory and we do not want to waste an extra stat() every
tie we come here.  Only when the first attempt fails by returning error
(i.e. fd < 0 and errno == ENOENT), we try mkdir() and then try again.

If you see "missing directory", it would be because on your system the
code somehow does not notice the failure from the first attempt.  Why?

The breakage you are seeing could come from either:

 - git_mkstemp_mode(), more specifically, the underlying mkstemp(), not
   returning negative when it fails for some reason; or

 - when the above fails, it fails to set errno to ENOENT.

and *that* is what needs to be fixed.

I think pessimizing create_tmpfile() like you did is not a fix; it is
sweeping the real problem under the rug.

^ permalink raw reply

* Re: [PATCH/RFC (version B)] gitweb: Allow UTF-8 encoded CGI query parameters and  path_info
From: Jakub Narebski @ 2012-02-02 22:57 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: git
In-Reply-To: <201202022207.52220.jnareb@gmail.com>

On Thu, 2 Feb 2012, Jakub Narebski wrote:
> On Thu, 2 Feb 2012, Michał Kiedrowicz wrote:
> > Jakub Narebski <jnareb@gmail.com> wrote:
> > 
> > > Gitweb tries hard to properly process UTF-8 data, by marking output
> > > from git commands and contents of files as UTF-8 with to_utf8()
> > > subroutine.  This ensures that gitweb would print correctly UTF-8
> > > e.g. in 'log' and 'commit' views.
> > > 
> > > Unfortunately it misses another source of potentially Unicode input,
> > > namely query parameters.  The result is that one cannot search for a
> > > string containing characters outside US-ASCII.  For example searching
> > > for "Michał Kiedrowicz" (containing letter 'ł' - LATIN SMALL LETTER L
> > > WITH STROKE, with Unicode codepoint U+0142, represented with 0xc5 0x82
> > > bytes in UTF-8 and percent-encoded as %C5%81) result in the following
> > > incorrect data in search field
> > > 
> > > 	Michał Kiedrowicz
> > > 
> > > This is caused by CGI by default treating '0xc5 0x82' bytes as two
> > > characters in Perl legacy encoding latin-1 (iso-8859-1), because 's'
> > > query parameter is not processed explicitly as UTF-8 encoded string.
> > > 
> > > The solution used here follows "Using Unicode in a Perl CGI script"
> > > article on http://www.lemoda.net/cgi/perl-unicode/index.html:
> > > 
> > > 	use CGI;
> > > 	use Encode 'decode_utf8;
> > > 	my $value = params('input');
> > > 	$value = decode_utf8($value);
> > > 
> > > This is done when filling %input_params hash; this required to move
> > > from explicit $cgi->param(<label>) to $input_params{<name>} in a few
> > > places.
> > 
> > I'm sorry but this doesn't work for me. I would be happy to help if you
> > have some questions about it.
> 
> Strange.  http://www.lemoda.net/cgi/perl-unicode/index.html says that
> those two approaches should be equivalent.  The -utf8 pragma version
> doesn't work for me at all, while this one works in that if finds what
> it is supposed to, but shows garbage in search form.

Is it what you mean by "this doesn't work for me", i.e. working search,
garbage in search field?
 
> Will investigate.

Damn.  If we use $cgi->textfield(-name => "s", -value => $searchtext) like
in gitweb, CGI.pm would read $cgi->param("s") by itself - without decoding.
To skip this we need to pass -force=>1  or  -override=>1 (i.e. further
changes to gitweb).

-utf8 pragma works with more modern CGI.pm, but does not with 3.10.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Problem pushing to a Novell share
From: Rüdiger Kessel @ 2012-02-02 22:54 UTC (permalink / raw)
  To: git

Hi

a push to an .git-repository on a Novell network-share using msysgit
fails with the following error message:

error: unable to create temporary sha1 filename : File exists

The problem has been found in git version 1.7.9

The problem is caused by a non-existing sub-directory in the file path
which is not automatically created on Novell shares.

A quick fix is to improve the create_tmpfile() function in sha1_file.c:

static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
{
    static struct stat sb;
    int fd, dirlen = directory_size(filename);

    if (dirlen + 20 > bufsiz) {
        errno = ENAMETOOLONG;
        return -1;
    }

    memcpy(buffer, filename, dirlen);
    buffer[dirlen-1] = 0;
    if (stat(buffer, &sb) != 0 && errno == ENOENT) {
        if (mkdir(buffer, 0777) || adjust_shared_perm(buffer))
            return -1;
    }

    memcpy(buffer, filename, dirlen);
    strcpy(buffer + dirlen, "tmp_obj_XXXXXX");
    fd = git_mkstemp_mode(buffer, 0444);
    return fd;
}

The function will create one missing directory level if it does not
exist prior to creating the tmp-file.

This method seems to work, but it works only if one directory level is
missing which might not be the case in general. A better solution
would be to create the whole directory tree if needed.

The best solution would be if MINGW would handle this issue.

I recommend including this patch into GIT since it does no harm on
posix systems but it improves compatibility on others.

Rüdiger

^ permalink raw reply

* Re: BUG 1.7.9: git-update-ref strange behavior with ref with trailing newline
From: Jeff King @ 2012-02-02 22:32 UTC (permalink / raw)
  To: Mark Jason Dominus; +Cc: git
In-Reply-To: <15093.1328220568@plover.com>

On Thu, Feb 02, 2012 at 05:09:28PM -0500, Mark Jason Dominus wrote:

> Here I use git symbolic-ref to update HEAD with a ref whose name
> contains trailing newlines:
> 
>         $ git symbolic-ref -m "this message does not appear" HEAD 'refs/heads/master
>         >
>         >
>         > '
> 
> The newlines are inserted into .git/HEAD, but are innocuous, because
> other git commands ignore them.  The bug is that the -m option is
> completely ignored:
> 
>         $ git reflog HEAD | grep 'message does not appear'

Is it trailing newlines, or is simply pointing to a ref that does not
exist? Because I believe we do not create a HEAD reflog entry in that
case, as we would have nothing to write in the "new sha1" field. I guess
we could write an entry that it went to "0{40}", though I'm not sure how
things like "git reflog show" would handle that. The logic is at the
very end of refs.c:create_symref if you want to experiment.

As far as the newlines go, I'm surprised we don't reject that. We should
probably run check_refname_format on the proposed contents of the
symbolic-ref.

-Peff

^ permalink raw reply

* Re: [PATCH 3/3] vcs-svn: suppress a -Wtype-limits warning
From: Ramsay Jones @ 2012-02-02 22:18 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Junio C Hamano, David Barr, GIT Mailing-list, Dmitry Ivankov
In-Reply-To: <20120202110601.GL3823@burratino>

Jonathan Nieder wrote:
> ---
> That's the end of the series.  I hope it was entertaining.
> 
> Thoughts of all kinds welcome, as usual.

Yes, this is a much better approach! Thanks!

I've only compile tested (on cygwin and mingw) so far, but
I don't expect any problems ...

So, please disregard my earlier v2 patch. [If it's not already
obvious, I often don't read the list every day and I missed
all of the discussion which resulted in this series (while, at
the same time, writing testing and sending the v2 patch!).]

ATB,
Ramsay Jones

^ permalink raw reply

* Git-gui: crashes on OS X when entering combining ("dead") keys
From: Beat Bolli @ 2012-02-02 22:04 UTC (permalink / raw)
  To: git

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

Hi

I've just had git-gui crash on me when I tried to enter the ~ (tilde)
character on my Mac mini under OS X 10.6.8:

2012-02-02 22:26:25.992 Wish[49140:60f] An uncaught exception was raised
2012-02-02 22:26:25.995 Wish[49140:60f] *** -[NSCFString
characterAtIndex:]: Range or index out of bounds
2012-02-02 22:26:25.996 Wish[49140:60f] *** Terminating app due to
uncaught exception 'NSRangeException', reason: '*** -[NSCFString
characterAtIndex:]: Range or index out of bounds'
*** Call stack at first throw:
(
        0   CoreFoundation                      0x00007fff8273d784
__exceptionPreprocess + 180
        1   libobjc.A.dylib                     0x00007fff87c10f03
objc_exception_throw + 45
        2   CoreFoundation                      0x00007fff8273d5a7
+[NSException raise:format:arguments:] + 103
        3   CoreFoundation                      0x00007fff8273d534
+[NSException raise:format:] + 148
        4   Foundation                          0x00007fff843ec6ad
- -[NSCFString characterAtIndex:] + 97
        5   Tk                                  0x00000001000bdbcf
Tk_SetCaretPos + 663
        6   Tk                                  0x00000001000c3d94
Tk_MacOSXSetupTkNotifier + 699
        7   Tcl                                 0x00000001001c629e
Tcl_DoOneEvent + 297
        8   Tk                                  0x000000010001c080
Tk_MainLoop + 24
        9   Tk                                  0x0000000100028ab4
Tk_MainEx + 1555
        10  Wish                                0x0000000100004545 0x0
+ 4294985029
        11  Wish                                0x00000001000044a4 0x0
+ 4294984868
)
terminate called after throwing an instance of 'NSException'
error: git-gui died of signal 6

[sorry for wrapping]

I use the Swiss German keyboard layout where the tilde is a dead key
on Fn+n (for entering the spanish ñ, I assume). Other dead keys like
¨, ´ and ` also cause git-gui to crash. It does not matter whether the
dead key is the last one in the commit message or not.

Interestingly, the commit text was saved to .git/COMMIT_MSG in spite
of the crash, so I didn't lose any of it.

I use the current git 1.7.9 installed via homebrew.

Thanks,
Beat Bolli

PS: Please CC me on replies, I'm not on the list. Thanks.
- -- 
pgp: 0x506A903A; 49D5 794A EA77 F907 764F  D89E 304B 93CF 506A 903A
gsm: 4.7.7.6.0.7.7.9.7.1.4.e164.arpa
icbm: 47.0452 N, 7.2715 E

"It takes love over gold, and mind over matter" -- Dire Straits
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk8rCFwACgkQMEuTz1BqkDoMhwCfZr+/FAsl7LsVxxHmelirDh+w
RZsAoOzusVol5b8zZEGq9NLanHlfi4bt
=hFrL
-----END PGP SIGNATURE-----

^ 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