Git development
 help / color / mirror / Atom feed
* Re: [RFC/PATCH 0/2] Commits with ancient timestamps
From: Thomas Rast @ 2012-02-03 10:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1328218903-5681-1-git-send-email-gitster@pobox.com>

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

> avoid misinterpreting human-written timestamp in other formats, and
> timestamps before 1975 do not have enough number of digits in them.
>
> Here is a two-patch series that may improve the situation.

Doing this just makes me wonder how important exactly the 1970-1975
range is.  Is there a notable software history from that era that can be
recovered?

(Your [1/2] does not seem to parse negative offsets from the unix epoch,
so anything before 1970 is still out.)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH v3 1/4] completion: be nicer with zsh
From: Felipe Contreras @ 2012-02-03 10:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7vhaz8vkhd.fsf@alter.siamese.dyndns.org>

On Fri, Feb 3, 2012 at 2:17 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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?

Fair enough. Just wondering.

> In any case, "be nicer with zsh" conveys no more meaningful information
> than "this is some patch about zsh".

And that already tells you a lot more than other alternatives.

> 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.

True. Which why I listened to the suggestion from Thomas Rast and
didn't use that, but "completion: work around zsh option propagation
bug" instead.

> 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.

This is getting ridiculous, now I sent the patches directly to you, is
your pobox.com server also silently dropping them for no reason? I
think this is totally counter-productive. I haven't received any reply
from the vger postmaster, but I guess you should be able to find out
why your host is dropping mails. Am I the only one that has such
issues?

Anyway. I have uploaded all the mails to here:

http://people.freedesktop.org/~felipec/git-patches/

As for $gmane/189683, the changes seem to be correct, but I still
prefer my commit message[1]--which I have written and rewritten many
times now to improve it.

Regarding $gmane/189606, I still prefer my commit message[2], because
it starts with the *purpose* of the patch. As for the changes, they
are correct, and I don't mind squashing them, but they are *two*
logically independent changes; imagine in the future somebody
wants/need to re-enable __git_shopt, well, all they have to do is
revert the second patch. But that's up to you.

Cheers.

[1] http://people.freedesktop.org/~felipec/git-patches/4
[2] http://people.freedesktop.org/~felipec/git-patches/2

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH] Convert isatty() calls to git_isatty()
From: Nguyen Thai Ngoc Duy @ 2012-02-03  9:59 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4F2BAD8B.1080403@viscovery.net>

2012/2/3 Johannes Sixt <j.sixt@viscovery.net>:
> Am 2/3/2012 9:35, schrieb Nguyễn Thái Ngọc Duy:
>> isatty() is used to check for interactive use cases. However if pager is
>> set up, standard file handles may be redirected and istty() calls later
>> on no longer reflect the original state.
>
> So what? What's wrong with this behavior?
>
> You converted many cases involving progress indicators. Wouldn't the new
> code pipe progress output to the pager where earlier it was not shown if a
> pager was present? That is plainly wrong: Progress output is destined only
> for the terminal, not for the pager.

Yeah right. My mistake.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] Convert isatty() calls to git_isatty()
From: Johannes Sixt @ 2012-02-03  9:48 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1328258101-10636-1-git-send-email-pclouds@gmail.com>

Am 2/3/2012 9:35, schrieb Nguyễn Thái Ngọc Duy:
> isatty() is used to check for interactive use cases. However if pager is
> set up, standard file handles may be redirected and istty() calls later
> on no longer reflect the original state.

So what? What's wrong with this behavior?

You converted many cases involving progress indicators. Wouldn't the new
code pipe progress output to the pager where earlier it was not shown if a
pager was present? That is plainly wrong: Progress output is destined only
for the terminal, not for the pager.

-- Hannes

^ permalink raw reply

* Re: [RFC/PATCH git-remote-bzr] Adapt to new semantics of remote-helper "import" command
From: Gabriel Filion @ 2012-02-03  9:43 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: git, Simon Poirier, Sverre Rabbelier, Jeff King, David Barr,
	Dmitry Ivankov, Jelmer Vernooij
In-Reply-To: <20120122054657.GA25103@burratino>

Hello,

On 12-01-22 12:46 AM, Jonathan Nieder wrote:
> Hi Simon and Gabriel,
> 
> Here's a rough patch against git://github.com/lelutin/git-remote-bzr.git
> master.

great! thanks for your help.

I must admit that this project never got to completion and is now
getting quite the low priority for my part. I'm no longer working for
the company that was using mainly Bazaar as their VCS, and I've been a
happy git-only user for some time now.
So, I don't have the same incentive to complete the project as I had before.

But I'd be happy to see this get to a point where it's working fine.

> Without this patch, whenever I try to use "git clone bzr::<something>",
> after doing all the work it removes the resulting repo and exits with
> status 141 (SIGPIPE).  Maybe the transport-helper should mask SIGPIPE
> when writing the final newline to avoid that.
> 
> I'd have prefered to write a patch for remote-bzr that works with
> older versions of git fast-import, too, but it wasn't obvious how.
> Hints welcome.

hmm.. I can wait some time to see if some ideas come out around this,
and commit your patch as-is if there are no comments/reworks.

> BTW, would you mind if I sent a patch to include git-remote-bzr in
> git.git under contrib/?

absolutely not, that'd be great actually :)

I didn't do that up to now, though, since I bumped into so much bugs
that I couldn't work out -- some very bad performance issues, and
problems with handling mark files with bzr-fastimport.

> Thanks for git remote-bzr!  I'd be happy for any thoughts you have.

The idea behind git-remote-bzr was to be able to interact with Bazaar
from within your git repository, i.e. to expose remote branches that you
can pull from and push to using the default git commands, without having
to learn to use yet another tool since the remote-helper would be
interfacing with the tool for you.

I have dived for a short period of time into bzrlib, the python library
behind Bazaar, to see how much work it would take to put together a
simplified fast-import client.. and .... wechrk!
It was a huge maze of version-dependant code (the API undergoes big
changes frequently, and backwards compatibility is maintained pretty
far) mixed with a 40-foot-deep class hierarchy. So I quickly gave up on
that idea..

IIRC, at the time I started work on this, Simon and I used
bzr-fastimport[1] because we were able to get farther with this tool. I
haven't used bzr-git[2] that much, though, so I can't comment too
extensively on it. But I would guess that it is maintained more
frequently than bzr-fastimport is, so it could be a better choice for
the backend fast-import client.

I would guess that Jelmer would be happy to help out with interfacing
with bzr-git.

-- 
Gabriel Filion

^ permalink raw reply

* [PATCH] Convert isatty() calls to git_isatty()
From: Nguyễn Thái Ngọc Duy @ 2012-02-03  8:35 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

isatty() is used to check for interactive use cases. However if pager is
set up, standard file handles may be redirected and istty() calls later
on no longer reflect the original state.

Convert isatty() calls to git_isatty() and allow git_isatty() to cache
tty info before pager is set up.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/commit.c         |    2 +-
 builtin/fsck.c           |    2 +-
 builtin/merge.c          |    4 ++--
 builtin/pack-objects.c   |    2 +-
 builtin/pack-redundant.c |    2 +-
 builtin/prune-packed.c   |    2 +-
 builtin/prune.c          |    2 +-
 builtin/revert.c         |    2 +-
 builtin/shortlog.c       |    4 ++--
 builtin/unpack-objects.c |    2 +-
 cache.h                  |    1 +
 color.c                  |    2 +-
 pager.c                  |   17 +++++++++++++++++
 transport.c              |    4 ++--
 14 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index eba1377..cc72f13 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -672,7 +672,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		strbuf_addbuf(&sb, &message);
 		hook_arg1 = "message";
 	} else if (logfile && !strcmp(logfile, "-")) {
-		if (isatty(0))
+		if (git_isatty(0))
 			fprintf(stderr, _("(reading log message from standard input)\n"));
 		if (strbuf_read(&sb, 0, 0) < 0)
 			die_errno(_("could not read log from standard input"));
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 8c479a7..0b4e8cf 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -637,7 +637,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
 
 	if (show_progress == -1)
-		show_progress = isatty(2);
+		show_progress = git_isatty(2);
 	if (verbose)
 		show_progress = 0;
 
diff --git a/builtin/merge.c b/builtin/merge.c
index 62c7b68..49b9176 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -714,7 +714,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		o.renormalize = option_renormalize;
 		o.show_rename_progress =
-			show_progress == -1 ? isatty(2) : show_progress;
+			show_progress == -1 ? git_isatty(2) : show_progress;
 
 		for (x = 0; x < xopts_nr; x++)
 			if (parse_merge_opt(&o, xopts[x]))
@@ -1129,7 +1129,7 @@ static int default_edit_option(void)
 	/* Use editor if stdin and stdout are the same and is a tty */
 	return (!fstat(0, &st_stdin) &&
 		!fstat(1, &st_stdout) &&
-		isatty(0) &&
+		git_isatty(0) &&
 		st_stdin.st_dev == st_stdout.st_dev &&
 		st_stdin.st_ino == st_stdout.st_ino &&
 		st_stdin.st_mode == st_stdout.st_mode);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0f2e7b8..4468c84 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2328,7 +2328,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (!pack_compression_seen && core_compression_seen)
 		pack_compression_level = core_compression_level;
 
-	progress = isatty(2);
+	progress = git_isatty(2);
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index f5c6afc..8197c9e 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -646,7 +646,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
 
 	/* ignore objects given on stdin */
 	llist_init(&ignore);
-	if (!isatty(0)) {
+	if (!git_isatty(0)) {
 		while (fgets(buf, sizeof(buf), stdin)) {
 			sha1 = xmalloc(20);
 			if (get_sha1_hex(buf, sha1))
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index f9463de..498c6e9 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -71,7 +71,7 @@ void prune_packed_objects(int opts)
 
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)
 {
-	int opts = isatty(2) ? VERBOSE : 0;
+	int opts = git_isatty(2) ? VERBOSE : 0;
 	const struct option prune_packed_options[] = {
 		OPT_BIT('n', "dry-run", &opts, "dry run", DRY_RUN),
 		OPT_NEGBIT('q', "quiet", &opts, "be quiet", VERBOSE),
diff --git a/builtin/prune.c b/builtin/prune.c
index 58d7cb8..821772e 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -158,7 +158,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix)
 	}
 
 	if (show_progress == -1)
-		show_progress = isatty(2);
+		show_progress = git_isatty(2);
 	if (show_progress)
 		progress = start_progress_delay("Checking connectivity", 0, 0, 2);
 
diff --git a/builtin/revert.c b/builtin/revert.c
index e6840f2..6f87c9b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -199,7 +199,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	int res;
 
 	memset(&opts, 0, sizeof(opts));
-	if (isatty(0))
+	if (git_isatty(0))
 		opts.edit = 1;
 	opts.action = REPLAY_REVERT;
 	git_config(git_default_config, NULL);
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 37f3193..c77b472 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -289,10 +289,10 @@ parse_done:
 	log.abbrev = rev.abbrev;
 
 	/* assume HEAD if from a tty */
-	if (!nongit && !rev.pending.nr && isatty(0))
+	if (!nongit && !rev.pending.nr && git_isatty(0))
 		add_head_to_pending(&rev);
 	if (rev.pending.nr == 0) {
-		if (isatty(0))
+		if (git_isatty(0))
 			fprintf(stderr, _("(reading log message from standard input)\n"));
 		read_from_stdin(&log);
 	}
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 14e04e6..4aaba69 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -501,7 +501,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 
-	quiet = !isatty(2);
+	quiet = !git_isatty(2);
 
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/cache.h b/cache.h
index 9bd8c2d..4073fc9 100644
--- a/cache.h
+++ b/cache.h
@@ -1176,6 +1176,7 @@ extern void setup_pager(void);
 extern const char *pager_program;
 extern int pager_in_use(void);
 extern int pager_use_color;
+extern int git_isatty(int);
 
 extern const char *editor_program;
 extern const char *askpass_program;
diff --git a/color.c b/color.c
index e8e2681..7151b48 100644
--- a/color.c
+++ b/color.c
@@ -183,7 +183,7 @@ int git_config_colorbool(const char *var, const char *value)
 static int check_auto_color(void)
 {
 	if (color_stdout_is_tty < 0)
-		color_stdout_is_tty = isatty(1);
+		color_stdout_is_tty = git_isatty(1);
 	if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
 		char *term = getenv("TERM");
 		if (term && strcmp(term, "dumb"))
diff --git a/pager.c b/pager.c
index 975955b..a9380ab 100644
--- a/pager.c
+++ b/pager.c
@@ -72,12 +72,17 @@ const char *git_pager(int stdout_is_tty)
 void setup_pager(void)
 {
 	const char *pager = git_pager(isatty(1));
+	int i;
 
 	if (!pager)
 		return;
 
 	setenv("GIT_PAGER_IN_USE", "true", 1);
 
+	/* cache tty info */
+	for (i = 0; i <= 2; i++)
+		git_isatty(i);
+
 	/* spawn the pager */
 	pager_argv[0] = pager;
 	pager_process.use_shell = 1;
@@ -110,3 +115,15 @@ int pager_in_use(void)
 	env = getenv("GIT_PAGER_IN_USE");
 	return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0;
 }
+
+int git_isatty(int fd)
+{
+	static int tty[3] = { -1, -1, -1 };
+
+	if (fd < 0 || fd > 2)
+		return isatty(fd);
+
+	if (tty[fd] == -1)
+		tty[fd] = isatty(fd);
+	return tty[fd];
+}
diff --git a/transport.c b/transport.c
index cac0c06..af48f7c 100644
--- a/transport.c
+++ b/transport.c
@@ -880,7 +880,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	const char *helper;
 	struct transport *ret = xcalloc(1, sizeof(*ret));
 
-	ret->progress = isatty(2);
+	ret->progress = git_isatty(2);
 
 	if (!remote)
 		die("No remote provided to transport_get()");
@@ -997,7 +997,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 	 *   2. Don't report progress, if verbosity < 0 (ie. -q/--quiet ).
 	 *   3. Report progress if isatty(2) is 1.
 	 **/
-	transport->progress = force_progress || (verbosity >= 0 && isatty(2));
+	transport->progress = force_progress || (verbosity >= 0 && git_isatty(2));
 }
 
 int transport_push(struct transport *transport,
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

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

Jakub Narebski <jnareb@gmail.com> wrote:

> 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?

I mean "garbage in search field". Search works even without the patch
(at least on Debian with git-1.7.7.3, perl-5.10.1 and CGI-3.43; I
don't have my notebook nearby at the moment to check).

>  
> > Will investigate.

Thanks for your time spending on this. I wouldn't call this problem
"production critial" but it seems wrong to support UTF-8 everywhere
properly except for one place.

> 
> Damn.  If we use $cgi->textfield(-name => "s", -value => $searchtext)
> like in gitweb, CGI.pm would read $cgi->param("s") by itself -
> without decoding. 

Makes sense. When I tried calling to_utf8() in the line that defines
textfield (this was my first approach to this problem), it haven't
changed anything.

> 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.
> 

^ permalink raw reply

* Re: General support for ! in git-config values
From: Kyle Moffett @ 2012-02-03  7:35 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: demerphq, Jeff King, Ævar Arnfjörð,
	Git Mailing List
In-Reply-To: <7vmx90say8.fsf@alter.siamese.dyndns.org>

On Thu, Feb 2, 2012 at 22:11, Junio C Hamano <gitster@pobox.com> wrote:
> 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.

Well yes, but running commands from .git/config is exactly what the OP
requested, and if it applies to .git/config it should also be
applicable to other trusted include sources too, no?

Perhaps allow config files to perform a "trusted" include, EG:
[include]
    trusted_exec = /usr/local/bin/site-specific-config-program
    blob = v1.0:src/gitconfig

By default, the only files which would be trusted are /etc/gitconfig,
~/.gitconfig, and .git/config (but ONLY if it has the same owner and
mode go-w), and they would only pass trust on to other files if they
use "trusted_*" include lines.

Also, since "include" is intended to introduce a
non-backwards-compatible change in behavior, perhaps a totally
different format should be used, EG:

$include exec_trusted /usr/local/bin/site-specific-config-program
$include blob v1.0:src/gitconfig

Something that would cause noticeable warnings in older versions of
git instead of silently ignoring the desired config includes.

Just a few thoughts.

Cheers,
Kyle Moffett

^ permalink raw reply

* 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


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