Git development
 help / color / mirror / Atom feed
* Re: [PATCH v4] Teach git var about GIT_EDITOR
From: Junio C Hamano @ 2009-11-01  4:29 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091031075627.GB635@progeny.tock>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Expose the command used by launch_editor() for scripts to use.
> This should allow one to avoid searching for a proper editor
> separately in each command.

Ok, so the idea is...

 * git_editor(void) uses the logic to decide which editor to use that used
   to live in launch_editor().  The function returns NULL if there is no
   suitable editor; the caller is expected to issue an error message when
   appropriate.

 * launch_editor() uses git_editor() and gives the error message the same
   way as before when EDITOR is not set.

 * "git var GIT_EDITOR" gives the editor name, or an error message when
   there is no appropriate one.

 * "git var -l" gives GIT_EDITOR=name only if there is an appropriate
   editor.

The above all look sensible, but IIRC, the true "vi" fell back on "ex"
mode on dumb terminals and was usable as a line editor, so we should be
able to run it even on dumb terminals.  I do not know about vi-clones
though.

^ permalink raw reply

* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Junio C Hamano @ 2009-11-01  4:27 UTC (permalink / raw)
  To: Samium Gromoff; +Cc: git, Daniel Barkalow, Tay Ray Chuan, Mike Hommey
In-Reply-To: <20091101.010702.527849118592864646._deepfire@feelingofgreen.ru>

Samium Gromoff <_deepfire@feelingofgreen.ru> writes:

> Attached is the SEGV bugreport I sent to debian.

Thanks for a report.

There are two issues.

 * transport-helper.c::get_helper() assumes that the transport structure
   always has its remote member filled and it can get name out of it.
   This is the segv in the report.

 * Even if we work around the above issue, the helper subprocess (in this
   case, remote-curl helper) insists on being inside a git repository,  To
   satisfy ls-remote request, you do not have to be in one.

Attached is a minimum fix/work around, but this is done without being very
familiar with the current assumptions in the codepaths involved.

Issues I want area experts to consider before coming up with the final fix
are:

 - Should we fix get_helper() in transport-helper.c, instead of touching
   ls-remote.c like this patch does?

   This issue really boils down to this question: is it valid for a
   transport to have NULL in its remote field, and should all the code
   that touch transport be prepared to deal with such a transport
   structure?

 - When helping to handle ls-remote request, there is no need for the
   helper to know anything about the local state.  We probably shouldn't
   even run setup_git_directory_gently() at all in this case.  But when
   helping other kinds of request, the helper does need to know where our
   repository is.

   In general, what should the initial environment for helpers be?  Should
   they assume that they have to figure out where the git repository is
   themselves (in other words, should they assume they cannot rely on
   anything the caller does before they are called?  Would the caller
   generally have done the usual repo discovery (including chdir() to the
   toplevel), and there are some set of assumptions they can make?  If so
   what are they?


 builtin-ls-remote.c |    2 +-
 remote-curl.c       |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..a8d5613 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -86,7 +86,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			pattern[j - i] = p;
 		}
 	}
-	remote = nongit ? NULL : remote_get(dest);
+	remote = remote_get(dest);
 	if (remote && !remote->url_nr)
 		die("remote %s has no configured URL", dest);
 	transport = transport_get(remote, remote ? remote->url[0] : dest);
diff --git a/remote-curl.c b/remote-curl.c
index ad6a163..7c83f77 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -81,8 +81,9 @@ int main(int argc, const char **argv)
 	struct strbuf buf = STRBUF_INIT;
 	const char *url;
 	struct walker *walker = NULL;
+	int nongit = 0;
 
-	setup_git_directory();
+	setup_git_directory_gently(&nongit);
 	if (argc < 2) {
 		fprintf(stderr, "Remote needed\n");
 		return 1;

^ permalink raw reply related

* Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend
From: Paolo Bonzini @ 2009-10-31 23:34 UTC (permalink / raw)
  To: git
In-Reply-To: <20091030202628.GA26513@coredump.intra.peff.net>


> So my suspicion is that there are some people who almost always want
> --new-timestamp, and some people who almost always want --old-timestamp,
> depending on their usual workflow. In which case a config option
> probably makes the most sense (but keeping the command-line to override
> the config, of course).

I'd say the config option should be per-branch (so that you can set the 
old-timestamp option only in integration branches, and not in topic 
branches), and that with such an option you could make the default be 
--new-timestamp in all three cases.

Paolo

^ permalink raw reply

* Re: [PATCH 1/2] gitk: Initialize msgcat before first use
From: Pat Thoyts @ 2009-10-31 21:34 UTC (permalink / raw)
  To: Bernt Hansen; +Cc: git, Paul Mackerras
In-Reply-To: <1256415640-10328-2-git-send-email-bernt@norang.ca>

Bernt Hansen <bernt@norang.ca> writes:

>The error text generated when your version of Tcl is too old is
>translated with msgcat (mc) before msgcat is initialized.  This
>causes Tcl to abort with:
>
>    Error in startup script: invalid command name "mc"
>
>We now initialize msgcat first before we check the Tcl version.  Msgcat
>is available since Tcl 8.1.
>

This doesn't quite work. [file normalize] was introduced with Tcl 8.4
and when I test this by starting it using Tcl 8.3 I get an error:
 "bad option "normalize": must be atime, attributes, channels..."
from line 11014. It is probably sufficient to just drop the [file
normalize] here. On Windows $argv0 is fully qualified and 
[file dirname] works ok on it. By removing the [file normalize] I get
the expected error dialog when testing with 8.3.

However, on Windows we actually get a better looking result by not
catching the [package require Tcl 8.4] and just letting Tk bring up a
standard message box with the version conflict error message.

Well, actually if show_error just used tk_messageBox it would look
better on Windows.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

^ permalink raw reply

* Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Samium Gromoff @ 2009-10-31 22:07 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: Text/Plain, Size: 377 bytes --]

Good day folks,

Attached is the SEGV bugreport I sent to debian.

I tried to convince ld to use /usr/lib/debug, via LD_LIBRARY_PATH,
and run the thing under gdb 7.0, but it won't use debug libraries
for some reason.


regards,
  Samium Gromoff
--
                                 _deepfire-at-feelingofgreen.ru
O< ascii ribbon campaign - stop html mail - www.asciiribbon.org


[-- Attachment #2: Type: Message/Rfc822, Size: 4691 bytes --]

From: Samium Gromoff <deepfire@feelingofgreen.ru>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
Date: Sun, 01 Nov 2009 00:38:31 +0300
Message-ID: <20091031213831.3786.15715.reportbug@auriga.deep> (sfid-20091101_003832_561599_A7C90C6C)

Package: git-core
Version: 1:1.6.5-1
Severity: important


Basically,

deepfire@auriga:/$ git peek-remote http://common-lisp.net/project/qitab/git/poiu.git 
Segmentation fault
deepfire@auriga:/$ git peek-remote http://www.lichteblau.com/git/atdoc.git 
Segmentation fault

Strace shows following:
[snip]
open("/dev/urandom", O_RDONLY)          = 3
read(3, "\27\n\7\210!\373\210\265", 8)  = 8
close(3)                                = 0
mprotect(0x7fe055d69000, 16384, PROT_READ) = 0
mprotect(0x7fe055f89000, 4096, PROT_READ) = 0
mprotect(0x7fe0563c2000, 4096, PROT_READ) = 0
munmap(0x7fe0563ae000, 67595)           = 0
set_tid_address(0x7fe0563ac7c0)         = 4047
set_robust_list(0x7fe0563ac7d0, 0x18)   = 0
futex(0x7fff5e3c057c, FUTEX_WAKE_PRIVATE, 1) = 0
futex(0x7fff5e3c057c, FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, 1, NULL, 7fe0563ac6f0) = -1 ENOSYS (Function not implemented)
rt_sigaction(SIGRTMIN, {0x7fe055d78750, [], SA_RESTORER|SA_SIGINFO, 0x7fe055d819a0}, NULL, 8) = 0
rt_sigaction(SIGRT_1, {0x7fe055d787e0, [], SA_RESTORER|SA_RESTART|SA_SIGINFO, 0x7fe055d819a0}, NULL, 8) = 0
rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
getrlimit(RLIMIT_STACK, {rlim_cur=8192*1024, rlim_max=RLIM_INFINITY}) = 0
brk(0)                                  = 0x26d6000
brk(0x26f7000)                          = 0x26f7000
getcwd("/", 4096)                       = 2
stat(".git", 0x7fff5e3c02d0)            = -1 ENOENT (No such file or directory)
access(".git/objects", X_OK)            = -1 ENOENT (No such file or directory)
access("./objects", X_OK)               = -1 ENOENT (No such file or directory)
chdir("..")                             = 0
stat(".git", 0x7fff5e3c02d0)            = -1 ENOENT (No such file or directory)
access(".git/objects", X_OK)            = -1 ENOENT (No such file or directory)
access("./objects", X_OK)               = -1 ENOENT (No such file or directory)
chdir("/")                              = 0
--- SIGSEGV (Segmentation fault) @ 0 (0) ---
+++ killed by SIGSEGV +++
Segmentation fault


-- System Information:
Debian Release: squeeze/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.26-2-amd64 (SMP w/2 CPU cores)
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)
Shell: /bin/sh linked to /bin/dash

Versions of packages git-core depends on:
ii  libc6                  2.10.1-3          GNU C Library: Shared libraries
ii  libcurl3-gnutls        7.19.5-1.1        Multi-protocol file transfer libra
ii  libdigest-sha1-perl    2.12-1            NIST SHA-1 message digest algorith
ii  liberror-perl          0.17-1            Perl module for error/exception ha
ii  libexpat1              2.0.1-4           XML parsing C library - runtime li
ii  perl-modules           5.10.1-6          Core Perl modules
ii  zlib1g                 1:1.2.3.3.dfsg-15 compression library - runtime

Versions of packages git-core recommends:
ii  less                          436-1      pager program similar to more
ii  openssh-client [ssh-client]   1:5.1p1-8  secure shell client, an rlogin/rsh
ii  patch                         2.5.9-5    Apply a diff file to an original
ii  rsync                         3.0.6-1    fast remote file copy program (lik

Versions of packages git-core suggests:
pn  git-arch                      <none>     (no description available)
ii  git-cvs                       1:1.6.5-1  fast, scalable, distributed revisi
ii  git-daemon-run                1:1.6.5-1  fast, scalable, distributed revisi
pn  git-doc                       <none>     (no description available)
pn  git-email                     <none>     (no description available)
ii  git-gui                       1:1.6.5-1  fast, scalable, distributed revisi
ii  git-svn                       1:1.6.5-1  fast, scalable, distributed revisi
ii  gitk                          1:1.6.5-1  fast, scalable, distributed revisi
pn  gitweb                        <none>     (no description available)

-- no debconf information

^ permalink raw reply

* Re: [PATCH v4] imap-send.c: fix compiler warnings for OpenSSL 1.0
From: Junio C Hamano @ 2009-10-31 21:34 UTC (permalink / raw)
  To: Vietor Liu; +Cc: Junio C Hamano, git
In-Reply-To: <1256970963-6345-1-git-send-email-vietor@vxwo.org>

Thanks.

^ permalink raw reply

* Re: [PATCH] commit -c/-C/--amend: take over authorship and restamp time with --claim
From: Junio C Hamano @ 2009-10-31 21:24 UTC (permalink / raw)
  To: Erick Mattos; +Cc: git
In-Reply-To: <1256958480-6775-1-git-send-email-erick.mattos@gmail.com>

Erick Mattos <erick.mattos@gmail.com> writes:

> The new --claim option is meant to solve this need by regenerating the
> timestamp and setting as new author the committer or the one specified
> on --author option.

I'll leave discussion on the option name to others.

> +--claim::
> +	When used with -C/-c/--amend options the committer takes over
> +	the cloned commit authorship and renew the timestamp thus using
> +	only the commit message from the source.

"The cloned commit" is a bit misleading; in the mind of users --amend does
not clone but rewrite, as the old commit usually only belongs to a reflog
and not any other branch.  I'd rewrite it this way, perhaps.

	When used with -C/-c/--amend options, declare that the authorship
	of the resulting commit now belongs of the committer.  This also
	renews the author timestamp.

We also would need a test to protect this new feature from getting broken
by future updates.

Thanks.

^ permalink raw reply

* Re: [PATCH 7/8] Provide a build time default-editor setting
From: Jonathan Nieder @ 2009-10-31 21:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <7vzl775ol5.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> Honestly speaking, my preference is to see if the built-in editor is
> exactly spelled as 'v' 'i', and skip this test altogether if it isn't.

That does make sense.  But let me try one last time, before doing
that.  (I should have sat down and thought this through carefully
before sending the first version --- sorry.)

Though the first two iterations of the patch were pretty ugly, the
third was just 's/vi/"$editor"/g' after setting editor and bailing out
if it does not consist of lowercase letters.  As you mentioned, it
makes more sense to skip only the "vi" part of the test.

Tested with DEFAULT_EDITOR=vi, vim, /usr/bin/nonexistent.

 t/t7005-editor.sh |   37 +++++++++++++++++++++++++------------
 1 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index a95fe19..5257f4d 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -4,7 +4,21 @@ test_description='GIT_EDITOR, core.editor, and stuff'
 
 . ./test-lib.sh
 
-for i in GIT_EDITOR core_editor EDITOR VISUAL vi
+unset EDITOR VISUAL GIT_EDITOR
+
+test_expect_success 'determine default editor' '
+
+	vi=$(TERM=vt100 git var GIT_EDITOR) &&
+	test -n "$vi"
+
+'
+
+if ! expr "$vi" : '^[a-z]*$' >/dev/null
+then
+	vi=
+fi
+
+for i in GIT_EDITOR core_editor EDITOR VISUAL $vi
 do
 	cat >e-$i.sh <<-EOF
 	#!$SHELL_PATH
@@ -12,19 +26,18 @@ do
 	EOF
 	chmod +x e-$i.sh
 done
-unset vi
-mv e-vi.sh vi
-unset EDITOR VISUAL GIT_EDITOR
+
+if ! test -z "$vi"
+then
+	mv e-$vi.sh $vi
+fi
 
 test_expect_success setup '
 
-	msg="Hand edited" &&
+	msg="Hand-edited" &&
+	test_commit "$msg" &&
 	echo "$msg" >expect &&
-	git add vi &&
-	test_tick &&
-	git commit -m "$msg" &&
-	git show -s --pretty=oneline |
-	sed -e "s/^[0-9a-f]* //" >actual &&
+	git show -s --format=%s > actual &&
 	diff actual expect
 
 '
@@ -54,7 +67,7 @@ test_expect_success 'dumb should prefer EDITOR to VISUAL' '
 
 TERM=vt100
 export TERM
-for i in vi EDITOR VISUAL core_editor GIT_EDITOR
+for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
 do
 	echo "Edited by $i" >expect
 	unset EDITOR VISUAL GIT_EDITOR
@@ -78,7 +91,7 @@ done
 
 unset EDITOR VISUAL GIT_EDITOR
 git config --unset-all core.editor
-for i in vi EDITOR VISUAL core_editor GIT_EDITOR
+for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
 do
 	echo "Edited by $i" >expect
 	case "$i" in

^ permalink raw reply related

* Re: Add '--bisect' revision machinery argument
From: Christian Couder @ 2009-10-31 20:58 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0910281631400.31845@localhost.localdomain>

On Thursday 29 October 2009, Linus Torvalds wrote:
> On Wed, 28 Oct 2009, Junio C Hamano wrote:
> > This shows a very nice direction to evolve, but your patch as-is breaks
> > "rev-list --bisect", I think.
>
> I think you're right. I tested git rev-parse, and the 'git log'
> machinery, but I didn't think about the fact that we already had a
> meaning
> for '--bisect' in rev-list.
>
> > Also, the helper of "git bisect" can and probably should be taught to
> > just ask this new behaviour from the revision machinery, instead of
> > collecting good and bad refs itself using bisect.c::read_bisect_refs().
>
> Yeah. And git-bisect.sh can be simplified too.

I will have a look at that.

Sorry for not responding earlier but I just came back today from the Linux 
Kongress 2009 in Dresden (http://www.linux-kongress.org/2009/).

Best regards,
Christian.

^ permalink raw reply

* [PATCH 2/2] configure: allow user to set gitconfig, pager and editor
From: Ben Walton @ 2009-10-31 20:41 UTC (permalink / raw)
  To: gitster, jrnieder, git; +Cc: Ben Walton
In-Reply-To: <1257021695-21260-2-git-send-email-bwalton@artsci.utoronto.ca>

Use the new GIT_WITH_MAKE_VAR function to allow user specified
values for ETC_GITCONFIG, DEFAULT_PAGER and DEFAULT_EDITOR.

Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
 configure.ac |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index 2829dbb..d50d492 100644
--- a/configure.ac
+++ b/configure.ac
@@ -241,6 +241,16 @@ GIT_PARSE_WITH(iconv))
 # change being considered an inode change from the update-index perspective.
 
 #
+# Allow user to set ETC_GITCONFIG variable
+GIT_WITH_MAKE_VAR(gitconfig, ETC_GITCONFIG)
+#
+# Allow user to set the default pager
+GIT_WITH_MAKE_VAR(pager, DEFAULT_PAGER)
+#
+# Allow user to set the default editor
+GIT_WITH_MAKE_VAR(editor, DEFAULT_EDITOR)
+
+#
 # Define SHELL_PATH to provide path to shell.
 GIT_ARG_SET_PATH(shell)
 #
-- 
1.6.5

^ permalink raw reply related

* [PATCH 0/2] Set Makefile variables from configure
From: Ben Walton @ 2009-10-31 20:41 UTC (permalink / raw)
  To: gitster, jrnieder, git; +Cc: Ben Walton

These patches add support for setting the newly created DEFAULT_EDITOR
and DEFAULT_PAGER from the configure script.  I also tacked in
ETC_GITCONFIG, since I can't currently toggle this without setting a
command line value when building, but have need to alter it.

The function added is generic, and will allow for setting new
variables as needed in the future.  No validation is done on the
values.  It is less specific than the *_PATH setting functions that
already exist.

Ben Walton (2):
  configure: add function to directly set Makefile variables
  configure: allow user to set gitconfig, pager and editor

 configure.ac |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

^ permalink raw reply

* [PATCH 1/2] configure: add function to directly set Makefile variables
From: Ben Walton @ 2009-10-31 20:41 UTC (permalink / raw)
  To: gitster, jrnieder, git; +Cc: Ben Walton
In-Reply-To: <1257021695-21260-1-git-send-email-bwalton@artsci.utoronto.ca>

Add function GIT_WITH_MAKE_VAR to provide an easy way to allow user
input to directly specify values for variables in the Makefile.

An example use is:
GIT_WITH_MAKE_VAR(gitconfig, ETC_GITCONFIG)

This would allow the user to add --with-gitconfig=/etc/mysiteconf
to their ./configure command line to add
ETC_GITCONFIG=/etc/mysiteconf to the config.mak.autogen file.

Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
 configure.ac |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index 84b6cf4..2829dbb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,6 +68,20 @@ else \
 	GIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval); \
 fi \
 ])# GIT_PARSE_WITH
+dnl
+dnl GIT_WITH_MAKE_VAR(withname, VAR)
+dnl ---------------------
+dnl Set VAR to the value specied by --with-$withname if --with-$withname
+dnl is specified.  This is a direct way to allow setting variables in the
+dnl Makefile.
+AC_DEFUN([GIT_WITH_MAKE_VAR],
+[AC_ARG_WITH([$1],
+ [AS_HELP_STRING([--with-$1=VALUE],
+		 [provide value for $2])],
+ if test -n "$withval"; then \
+    AC_MSG_NOTICE([Setting $2 to $withval]); \
+    GIT_CONF_APPEND_LINE($2=$withval); \
+ fi)])# GIT_WITH_MAKE_VAR
 
 dnl
 dnl GIT_CHECK_FUNC(FUNCTION, IFTRUE, IFFALSE)
-- 
1.6.5

^ permalink raw reply related

* Re: [PATCH] Update packfile transfer protocol documentation
From: Johannes Sixt @ 2009-10-31 20:12 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Scott Chacon, git list
In-Reply-To: <20091031020634.GL10505@spearce.org>

Shawn O. Pearce schrieb:
> Scott Chacon <schacon@gmail.com> wrote:
>> +Currently only 'host' is allowed in the extra information.  It's
> 
> No.  We should make this a MUST.  As in:
> 
> 	Only host-parameter is allowed in the git-proto-request.
> 	Clients MUST NOT attempt to send additional parameters.
> 
> Sending another header can cause older git-daemons to lock up.

I think you are making a wrong case here: Older git-daemons that lock up 
are security holes because they allow DoS attacks, and any decent admin 
will want to upgrade to a fixed git-daemon anyway.

Fixed git-daemons can allow extra information in addition to 'host'. I 
know you argued otherwise when you submitted the fix to git-daemon, but I 
think you were wrong already back then.

-- Hannes

^ permalink raw reply

* Re: [PATCH 7/8] Provide a build time default-editor setting
From: Junio C Hamano @ 2009-10-31 19:51 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091031032647.GA5583@progeny.tock>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>  
>>> +test_expect_success 'does editor have a simple name (no slashes, etc)?' '
>>> +
>>> +	editor=$(TERM=vt100 git var GIT_EDITOR) &&
>>> +	test -n "$editor" &&
>>> +	simple=t &&
>>> +	case "$editor" in
>>> +	*/* | core_editor | [A-Z]*)
>> 
>> Hmm, what are the latter two cases designed to catch?
>
> Both are meant to allow the test to work without too many changes.

Honestly speaking, my preference is to see if the built-in editor is
exactly spelled as 'v' 'i', and skip this test altogether if it isn't.
Then the patch only needs to insert these lines (and reword "default
editor name too complicated" to "using customized default editor") without
touching the rest.  It simply does not look worth the complication.

You _might_ be able to skip only the "vi" part of the test when you see
that the built-in default is customized, though.  I didn't look closely
enough.

> diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
> ...
> +unset EDITOR VISUAL GIT_EDITOR
> +
> +test_expect_success 'determine default editor' '
> +
> +	editor=$(TERM=vt100 git var GIT_EDITOR) &&
> +	test -n "$editor"
> +
> +'
> +
> +if ! test -z "$(printf '%s\n' "$editor" | sed '/^[a-z]*$/d')"
> +then
> +	say 'skipping editor tests, default editor name too complicated'
> +	test_done
> +fi
> +

^ permalink raw reply

* Re: [PATCH v2 3/8] Teach git var about GIT_EDITOR
From: Johannes Sixt @ 2009-10-31 19:40 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, Ben Walton, David Roundy, GIT List
In-Reply-To: <20091031013934.GD5160@progeny.tock>

Jonathan Nieder schrieb:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> Expose the command used by launch_editor() for scripts to use...
 >
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

This patch has grown far beyond my original submission. I can't validly 
sign it off anymore. Please take authorship ;)

-- Hannes

^ permalink raw reply

* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Junio C Hamano @ 2009-10-31 19:32 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Thomas Rast, Robin Rosenberg, git, sasa.zivkov
In-Reply-To: <20091031182416.GO10505@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

>> This patch alone breaks tests in the t55?? series quite a lot,
>
> Drop the patch.

Ok, I missed that the unstated goal was "we will eventually drop support
for reading from branches and remotes".  I think it is a worthy goal, but
also agree that should be done at 1.7.0 or 1.8.0 boundary.

If this were Andrew alone, I personally do not think it is such a big
deal.  My understanding is that an eventual goal over there in the kernel
land is to grow 'linux-next' tree even more so that akpm tree will shrink
in the longer term, anyway.  I consulted Andrew in early days while he was
fighting with git to get his scripts to do what he needs them to do, and I
can do that again to bring them up-to-date if necessary.

In order to better support "massive integrator" workflow
that involves interacting with dozens of remote branches, we need to admit
that some of the things you can do if you are set up like Andrew are less
convenient to do via "git remotes" managing .git/config file.  E.g.

    : add new
    $ echo "$korg:/home/rmk/linux-2.6-arm.git#master" >.git/branches/arm-current
    : remove stale
    $ rm -f .git/branches/powerpc
    : find source
    $ grep -r $something .git/branches
    : make random small changes, e.g. change branch name only
    $ vi .git/branches/sparc

and we _might_ need to improve "git remote" interface before dropping
support for reading branches and remotes files.

Admittedly, managing integration trees like -mm and linux-next needs not
just nickname-to-repo-branch mapping but also involves the correct merge
order anyway, and people like Andrew and Stephan Rothwell (linux-next)
maintain a text file to describe them that git does not know about.

E.g. http://linux.f-seidel.de/linux-next/pmwiki/pmwiki.php?n=Linux-next.IncludedTrees

We do not have any infrastructure support for that kind of thing.

To manage 'pu' and 'next', I use specialized scripts (in my 'todo' branch,
look for Reintegrate script) myself, even though the number of topics I
manage is far smaller than what we are discussing here[*1*].  In that
sense, the difference between the remotes sections in .git/config file and
.git/branches/* files is not such a big issue in the larger picture.

As long as we keep the UI to deal with bare URL and branch names from the
command line properly working, the "massive intergrator" workflow might be
better done without _any_ remote definition, either in the config nor
branches/remotes files.  Two integrator scripts might read like these:

	#!/bin/sh
	# fetch-all script
        # fetch from repositories
        failed=
	while read nickname url branch
        do
        	git fetch -q "$url" "+$branch:refs/remotes/$nickname" ||
		failed="$failed$nickname "
	done <merge-order
        test -z "$failed" ||
        echo "Failed to fetch from $failed"

        #!/bin/sh
	# merge-all script
	# git reset --hard remotes/linus-tip
        while read nickname url branch
        do
        	git merge -m "Merge from $url#$branch" "remotes/$nickname" ||
		accept_rerere ||
                break
	done <merge-order

where accept_rerere is something like what my Reintegrate script (in
'todo' branch) has in it.  Then the workflow for the integrator would
become:

  1. to run "fetch-all" once;
  2. reset to Linus's tip of the day;
  3. run "merge-all";
     3.a fix up conflicts;
	 edit && git commit
     3.b decide to drop the day's tree and use previous day's:
	 git reset --hard &&
         git update-ref refs/remotes/$nick refs/remotes/$nick@{1.day}
     3.c decide to drop the tree:
	 git reset --hard &&
         edit merge-order
     and go back to step 3.


[Footnote]

*1* I do not keep a "merge order" file, but existing merges on 'pu' for
that purpose.  The Reintegrate script figures it out by looking at what
was in 'pu'.  One cycle of my git day looks like this, in this order:

    : record what topics are in 'next' and 'pu'
    : 'jch' is a shadow of 'next' that merges all the topics in 'next'
    : on top of 'master'.
    $ Meta/Reintegrate master..jch >/var/tmp/redo-jch.sh
    $ Meta/Reintegrate jch..pu >/var/tmp/redo-pu.sh

    : queue a new topic
    $ git checkout -b xx/topic master
    $ git am -s $patch

    : update a topic
    $ git checkout xx/topic
    $ git am -s $patch

    : fix a topic (that is not in 'next' yet)
    $ git checkout xx/topic
    $ git rebase -i $(git merge-base master HEAD)

    : decide to graduate a topic to 'master'
    $ git checkout master
    $ git merge xx/topic

    : apply directly to master
    $ git checkout master
    $ git am -s $patch

    : update 'next' with what's new in 'master'
    $ git checkout next && git merge master
    : rebuild 'jch' (shadow of 'next')
    $ git branch -f jch master && git checkout jch
    $ sh /var/tmp/redo-jch.sh
    : at this point, 'jch' and 'next' must exactly match

    : add topics that are next-ready to 'jch' and test
    $ git merge xx/topic

    : merge them to 'next' as well
    $ Meta/Reintegrate master..jch >/var/tmp/redo-jch.sh
    $ git checkout next && sh /var/tmp/redo-jch.sh
    : at this point, 'jch' and 'next' must exactly match

    : rebuild 'pu'
    $ git branch -f pu jch && git checkout pu
    $ sh /var/tmp/redo-pu.sh
    : merge new topics
    $ git merge xx/topic

^ permalink raw reply

* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Shawn O. Pearce @ 2009-10-31 18:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, Robin Rosenberg, git, sasa.zivkov
In-Reply-To: <7vr5sj8m5f.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Modern git Porcelains write remote definitions solely to .git/config, but
> still reads from .git/{branches,remotes}.
...
> Andrew Morton explicitly asked for this to be kept a few years
> ago and I do not see a reason to deprecate this.
...
>     Shawn and other wants to stop JGit from creating this directory on

I probably said something like this.  I won't bother denying it,
because list archives are more accurate than my own fallible memory.

But I didn't know the Andrew Morton part above.  After hearing it
from you, I'm reversing my (apparent) direction here.  We should
continue to create the branches directory within a new repository.

Sorry Robin, but Andrew Morton matters.  Its one stupid unused
directory in a repository that will chew through thousands of inodes
as loose objects.  Its a drop in the bucket in terms of resource
cost used by Git.  And Andrew is someone whose workflow we don't
want to break if we can avoid it.  He's a long time Git user who is
also high up in the kernel food chain.  Interrupting him disrupts
a fair chunk of kernel work while he grumbles about the Goddamn
Idiotic Truckload of s**t that Linus begat.

> This patch alone breaks tests in the t55?? series quite a lot,

Drop the patch.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Junio C Hamano @ 2009-10-31 18:15 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Robin Rosenberg, Junio C Hamano, git, spearce, sasa.zivkov
In-Reply-To: <200910311011.31189.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> Robin Rosenberg wrote:
>> Git itself does not even look at this directory.

Modern git Porcelains write remote definitions solely to .git/config, but
still reads from .git/{branches,remotes}.  What we do not do is to update
these locations, and we do not need to have these locations to operate.

So "not even look at" is too strong; it just "not touch".

I do not think there is reason to change that part of the equation.  For
people who need to fetch and merge hundreds of random places, it is a lot
handier to be able to do

	echo "$url#$branch" >.git/branch/$nickname
        rm .git/branch/$nickname

to manage the set of locations added to and deleted from the daily
compose.  Andrew Morton explicitly asked for this to be kept a few years
ago and I do not see a reason to deprecate this.

Now, not installing an empty .git/branch directory does break the above
workflow.  You would need to mkdir _once_ yourself, but I do not think
that is such a big deal.

On the other hand, I do not think it is such a big deal to have otherwise
unused .git/branches/ directory, either.  Robin wrote:

    Shawn and other wants to stop JGit from creating this directory on
    init with the motivation that newer Git version doesn't create it
    anymore. This patch would make that assertion true.

and after re-reading it, I realize "the motivation" is not a motivation at
all---it is merely an excuse ("after this patch is applied, git wouldn't
create it anymore"---so JGit will have an excuse not to do so).  It does
not say _why_ it shouldn't be there in the first place.  IOW, we need to
fill in the blank in: "JGit is merely following suit; the reason git
stopped creating the directory is ________").

This patch alone breaks tests in the t55?? series quite a lot, and I am
tempted to revert it.  My time is more valuable than fixing the fallouts
from this change, when the real purpose of the change is not yet stated.

^ permalink raw reply

* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Shawn O. Pearce @ 2009-10-31 18:09 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Thomas Rast, Junio C Hamano, git, sasa.zivkov
In-Reply-To: <200910311902.48317.robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> >   * a remote in the git configuration file: `$GIT_DIR/config`,
> >   * a file in the `$GIT_DIR/remotes` directory, or
> >   * a file in the `$GIT_DIR/branches` directory.
> 
> I, and a few other people, it seems. Seems the purpose of these
> files is a bit different. Git does look in these directories (both)
> when fetch is run. Seems remotes is not created by init though.

Since remotes isn't created by init, branches shouldn't be either.
Cogito is dead, and that was the main customer who wanted branches
to be present in a repository.

I think its safe to remove branches from the template repository
and stop creating it, but continue to read from branches and
remotes if they exist.

We might want to consider dropping support for them in 1.7.0
or 1.8.0, because any new tools largely focus on config.
E.g. git-remote probably can't edit branches or remotes, git-gui
probably doesn't use them, JGit doesn't use them.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Don't create the $GIT_DIR/branches directory on init
From: Robin Rosenberg @ 2009-10-31 18:02 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, git, spearce, sasa.zivkov
In-Reply-To: <200910311011.31189.trast@student.ethz.ch>

lördag 31 oktober 2009 10:11:29 skrev  Thomas Rast:
> Robin Rosenberg wrote:
> > Git itself does not even look at this directory.
>
> This contradicts the git-fetch manpage though: from urls-remotes.txt,
> it includes
>
>   The name of one of the following can be used instead
>   of a URL as `<repository>` argument:
>
>   * a remote in the git configuration file: `$GIT_DIR/config`,
>   * a file in the `$GIT_DIR/remotes` directory, or
>   * a file in the `$GIT_DIR/branches` directory.
>
> (and a longer explanation of what they need to look like).
>
> So which one is wrong?

I, and a few other people, it seems. Seems the purpose of these
files is a bit different. Git does look in these directories (both)
when fetch is run. Seems remotes is not created by init though.

-- robin

^ permalink raw reply

* Re: [PATCH 10/19] Allow helpers to request marks for fast-import
From: Sverre Rabbelier @ 2009-10-31 16:19 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <200910311304.05408.johan@herland.net>

Heya,

On Sat, Oct 31, 2009 at 13:04, Johan Herland <johan@herland.net> wrote:
> On Friday 30 October 2009, Sverre Rabbelier wrote:
> This conglomeration of patch series is becoming fairly complicated, and it's
> becoming hard to stay in sync. I suggest that you drop the CVS-specific
> parts from this series, and work on stabilizing the common infrastructure.
> Once that has settled, you can send a git-remote-hg series, and I can send a
> rebased and updated git-remote-cvs series.

That sounds like a good idea; I want to use the marks in git-remote-hg as well.

> Feel free to reorganize the patches so that the git_remote_helpers
> infrastructure is created in the correct location (instead of reorganizing
> git_remote_cvs).

Ok, will do.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH nd/sparse] Support directory exclusion from index
From: Nguyễn Thái Ngọc Duy @ 2009-10-31 14:09 UTC (permalink / raw)
  To: git, Junio C Hamano, skillzero; +Cc: Nguyễn Thái Ngọc Duy

The function excluded_from_list (or its public API excluded) is
currently used to mark what entry is included in sparse checkout.
Because index does not have directories, the pattern "foo", while
would match directory "foo" on working directory, would not match
against index.

To overcome this, if a pattern does not match, check if it matches
parent directories too before moving on to the next pattern. This
behavior only applies to index matching.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 There is subtle difference, which might break things. When working
 with file system, it checks top-down, parent directories first.
 I do bottom-up.

 If/you/have/deep/directories sparse checkout may become slow.

 And my test broke :(

 dir.c                                |   24 ++++++++++++++++++++++--
 t/t1009-read-tree-sparse-checkout.sh |    4 ++--
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/dir.c b/dir.c
index 3a8d3e6..82227e5 100644
--- a/dir.c
+++ b/dir.c
@@ -334,13 +334,15 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
 }
 
 /* Scan the list and let the last match determine the fate.
+ * dtype == NULL means matching against index, not working directory.
  * Return 1 for exclude, 0 for include and -1 for undecided.
  */
-int excluded_from_list(const char *pathname,
-		       int pathlen, const char *basename, int *dtype,
+int excluded_from_list(const char *pathname_,
+		       int pathlen_, const char *basename_, int *dtype,
 		       struct exclude_list *el)
 {
 	int i;
+	char buf[PATH_MAX];
 
 	if (el->nr) {
 		for (i = el->nr - 1; 0 <= i; i--) {
@@ -348,6 +350,11 @@ int excluded_from_list(const char *pathname,
 			const char *exclude = x->pattern;
 			int to_exclude = x->to_exclude;
 
+			const char *pathname = pathname_;
+			const char *basename = basename_;
+			int pathlen = pathlen_;
+
+recheck:
 			if (x->flags & EXC_FLAG_MUSTBEDIR) {
 				if (!dtype) {
 					if (!prefixcmp(pathname, exclude))
@@ -398,6 +405,19 @@ int excluded_from_list(const char *pathname,
 					    return to_exclude;
 				}
 			}
+
+			if (!dtype) { /* matching against index */
+				basename = strrchr(pathname, '/');
+				if (basename) {
+					pathlen = basename-pathname;
+					memcpy(buf, pathname, pathlen);
+					buf[pathlen] = '\0';
+					pathname = buf;
+					basename = strrchr(pathname, '/');
+					basename = (basename) ? basename+1 : pathname;
+					goto recheck;
+				}
+			}
 		}
 	}
 	return -1; /* undecided */
diff --git a/t/t1009-read-tree-sparse-checkout.sh b/t/t1009-read-tree-sparse-checkout.sh
index 62246db..b57d237 100755
--- a/t/t1009-read-tree-sparse-checkout.sh
+++ b/t/t1009-read-tree-sparse-checkout.sh
@@ -84,13 +84,13 @@ cat >expected.swt <<EOF
 H init.t
 H sub/added
 EOF
-test_expect_failure 'match directories without trailing slash' '
+test_expect_success 'match directories without trailing slash' '
 	echo init.t > .git/info/sparse-checkout &&
 	echo sub >> .git/info/sparse-checkout &&
 	git read-tree -m -u HEAD &&
 	git ls-files -t > result &&
 	test_cmp expected.swt result &&
-	test ! -f init.t &&
+	test -f init.t &&
 	test -f sub/added
 '
 
-- 
1.6.5.2.216.g9c1ec

^ permalink raw reply related

* [PATCH resend] gitk: Fix "git gui blame" invocation when called from topdir
From: Markus Heidelberg @ 2009-10-31 12:09 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Markus Heidelberg

In this case "git rev-parse --git-dir" doesn't return an absolute path,
but merely ".git", so the selected file has a relative path.
The function make_relative then tries to make the already relative path
relative, which results in a path like "../../../../Makefile" with as
much ".." as the number of parts [pwd] consists of.

This regression was introduced by commit 9712b81 (gitk: Fix bugs in
blaming code, 2008-12-06), which fixed "git gui blame" when called from
subdirs.

This also fixes it for bare repositories.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 gitk |   30 +++++++++++++++++-------------
 1 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/gitk b/gitk
index a0214b7..07a9440 100755
--- a/gitk
+++ b/gitk
@@ -3377,21 +3377,25 @@ proc index_sha1 {fname} {
 
 # Turn an absolute path into one relative to the current directory
 proc make_relative {f} {
-    set elts [file split $f]
-    set here [file split [pwd]]
-    set ei 0
-    set hi 0
-    set res {}
-    foreach d $here {
-	if {$ei < $hi || $ei >= [llength $elts] || [lindex $elts $ei] ne $d} {
-	    lappend res ".."
-	} else {
-	    incr ei
+    if {[file pathtype $f] ne "relative"} {
+	set elts [file split $f]
+	set here [file split [pwd]]
+	set ei 0
+	set hi 0
+	set res {}
+	foreach d $here {
+	    if {$ei < $hi || $ei >= [llength $elts] || [lindex $elts $ei] ne $d} {
+		lappend res ".."
+	    } else {
+		incr ei
+	    }
+	    incr hi
 	}
-	incr hi
+	set elts [concat $res [lrange $elts $ei end]]
+	return [eval file join $elts]
+    } else {
+	return $f
     }
-    set elts [concat $res [lrange $elts $ei end]]
-    return [eval file join $elts]
 }
 
 proc external_blame {parent_idx {line {}}} {
-- 
1.6.5.2.155.gaa0e5

^ permalink raw reply related

* ef/msys-imap, was Re: What's cooking in git.git (Oct 2009, #06; Fri, 30)
From: Johannes Schindelin @ 2009-10-31 12:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vljis9pks.fsf@alter.siamese.dyndns.org>

Hi,

On Fri, 30 Oct 2009, Junio C Hamano wrote:

> * ef/msys-imap (2009-10-22) 9 commits.
>  - Windows: use BLK_SHA1 again
>  - MSVC: Enable OpenSSL, and translate -lcrypto
>  - mingw: enable OpenSSL
>  - mingw: wrap SSL_set_(w|r)fd to call _get_osfhandle
>  - imap-send: build imap-send on Windows
>  - imap-send: fix compilation-error on Windows
>  - imap-send: use run-command API for tunneling
>  - imap-send: use separate read and write fds
>  - imap-send: remove useless uid code
> 
> This is pulled from J6t; I'll merge it to 'next' if Dscho is Ok with it.

Dscho is Ok with it.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 10/19] Allow helpers to request marks for fast-import
From: Johan Herland @ 2009-10-31 12:04 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <fabb9a1e0910300526v5cbcf685l69f60c58b7e3732@mail.gmail.com>

On Friday 30 October 2009, Sverre Rabbelier wrote:
> Heya,
> 
> On Fri, Oct 30, 2009 at 01:21, Johan Herland <johan@herland.net> wrote:
> > Please drop this patch from the series. The functionality is not
> > needed, since we'll use the fast-import "option" command from the
> > sr/gfi-options series instead.
> 
> In that case I will rebase the series on top of sr/gfi-options then as
> soon as I reroll that one.

Good.

> Also, do you need to change anything else in git-remote-cvs to do that?

Yes, the sr/gfi-options series does cause some changes, both in git-remote-
cvs, and in the support libraries (adding a couple of methods to the 
GitFastImport class in git.py).

This conglomeration of patch series is becoming fairly complicated, and it's 
becoming hard to stay in sync. I suggest that you drop the CVS-specific 
parts from this series, and work on stabilizing the common infrastructure. 
Once that has settled, you can send a git-remote-hg series, and I can send a 
rebased and updated git-remote-cvs series.

Feel free to reorganize the patches so that the git_remote_helpers 
infrastructure is created in the correct location (instead of reorganizing 
git_remote_cvs).


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ 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