Git development
 help / color / mirror / Atom feed
* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Jakub Narebski @ 2007-12-05 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Ramsay Jones, Arjen Laarhoven, Brian Gernhardt
In-Reply-To: <7vtzmxortn.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> +AC_MSG_CHECKING([for old iconv()])
>> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
>> +	[AC_MSG_RESULT([no])],
>> +	[AC_MSG_RESULT([yes])
>> +	OLD_ICONV=YesPlease])
>> +AC_SUBST(OLD_ICONV)
>>  
> 
> Which result does COMPILE_IFELSE give for non error warnings?  Ok, or
> Bad?

 - Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND],
          [ACTION-IF-NOT-FOUND])
     Run the compiler and compilation flags of the current language
     (*note Language Choice::) on the INPUT, run the shell commands
     ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise.  The INPUT
     can be made by `AC_LANG_PROGRAM' and friends.

And if I have checked correctly code which causes only warnings
returns Ok (in this case print 'no' after 'checking for old iconv()... '
and do not set OLD_ICONV, which means it will be unset).
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: git-svn: .git/svn disk usage
From: Steven Grimm @ 2007-12-05 21:30 UTC (permalink / raw)
  To: Eric Wong; +Cc: David Voit, git
In-Reply-To: <20071205085451.GA347@soma>

How about using git itself to keep some of this information? I'll just  
throw this idea out there; might or might not make any actual sense.

Create a new "git-svn metadata" branch. This branch contains a fake  
directory (never intended for checkout, though you could do it) that  
has a "file" for each svn revision. The filename is just the svn  
revision number, maybe divided into subdirectories in case you want to  
check the branch out for debugging purposes or whatever. The contents  
are the git commit SHA1 and whatever other metadata you want to keep  
in the future.

The advantage of doing it this way? You can pass around svn metadata  
using the normal git fetch/push tools, query the metadata using "git  
show", etc. In terms of data integrity, it's as secure as anything  
else in a git repository, much more so than a separately maintained db  
file under .git.

Along similar lines, a separate branch where the filenames are commit  
SHA1s and the file contents are the stuff that currently gets written  
into the git-svn-id: lines would mean no more need to rewrite history  
when doing dcommit, and thus easier mixing of native git workflows and  
interactions with an svn repository.

It would be great if you could clone a git-svn repository and then do  
"git svn dcommit" from the clone, secure in the knowledge that things  
will stay consistent even if the origin gets your changes via "git svn  
fetch" rather than from you.

-Steve

^ permalink raw reply

* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Brian Gernhardt @ 2007-12-05 21:38 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Wincent Colaiuta, git, Junio C Hamano, Ramsay Jones,
	Arjen Laarhoven
In-Reply-To: <200712052219.00930.jnareb@gmail.com>


On Dec 5, 2007, at 4:19 PM, Jakub Narebski wrote:

> Ahhh... now I understand. You have installed new iconv() on your
> computer, and generic 'uname -s' (OS name) based guessing in Makefile
> guesses wrongly that you need OLD_ICONV, while ./configure script
> actually tests it and correctly decides to unset OLD_ICONV !

As far as the "installed new iconv()" goes, you may be wrong there.   
The latest major release of OS X includes a version of iconv that does  
not need OLD_ICONV as part of the base system.  I've had to unset it  
in my config.mak in order to avoid the warning.

~~ Brian Gernhardt

^ permalink raw reply

* [PATCH 7/6] builtin-remote: guard remote->url accesses by remote->url_nr check
From: Johannes Schindelin @ 2007-12-05 21:40 UTC (permalink / raw)
  To: git, gitster; +Cc: Johannes Sixt
In-Reply-To: <Pine.LNX.4.64.0712051902080.27959@racer.site>


struct remote's url member is not a NULL terminated list.  Instead, we
have to check url_nr before accessing it.

Noticed by Johannes Sixt.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	For your viewing pleasure, this is not a resend, but an amend.  
	Feel free to squash with 4/6, or to bug me to do it.

 builtin-remote.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 142eb97..ea323e2 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -356,7 +356,8 @@ static int show_or_prune(int argc, const char **argv, int prune)
 		states.remote = remote_get(*argv);
 		if (!states.remote)
 			return error("No such remote: %s", *argv);
-		transport = transport_get(NULL, states.remote->url[0]);
+		transport = transport_get(NULL, states.remote->url_nr > 0 ?
+			states.remote->url[0] : NULL);
 		ref = transport_get_remote_refs(transport);
 
 		read_branches();
@@ -391,7 +392,7 @@ static int show_or_prune(int argc, const char **argv, int prune)
 		}
 
 		printf("* remote %s\n  URL: %s\n", *argv,
-			states.remote->url[0] ?
+			states.remote->url_nr > 0 ?
 				states.remote->url[0] : "(no URL)");
 
 		for (i = 0; i < branch_list.nr; i++) {
@@ -468,8 +469,9 @@ static int get_one_entry(struct remote *remote, void *priv)
 {
 	struct path_list *list = priv;
 
-	path_list_append(remote->name, list)->util = (void *)remote->url[0];
-	if (remote->url[0] && remote->url[1])
+	path_list_append(remote->name, list)->util = remote->url_nr ?
+		(void *)remote->url[0] : NULL;
+	if (remote->url_nr > 1)
 		warning ("Remote %s has more than one URL", remote->name);
 
 	return 0;
-- 
1.5.3.7.2157.g9598e

^ permalink raw reply related

* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Pascal Obry @ 2007-12-05 21:46 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: git, Junio C Hamano, Ramsay Jones, Arjen Laarhoven,
	Brian Gernhardt
In-Reply-To: <1196869526-2197-1-git-send-email-jnareb@gmail.com>

Jakub Narebski a écrit :
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This patch needs checking if it correctly sets OLD_ICONV
> when needed.  I have checked only that it is not set when
> with new iconv() declaration.  Could people using Cygwin
> (and other with OLD_ICONV: Darwin) test it?

Not working on Cygwin:

   $ autoconf
   $ ./configure --prefix=/usr/local --build=i686-pc-cygwin
   ...
   configure: CHECKS for header files
   checking for old iconv()... no

It should be yes above. And in config.mak.autogen we have:

   OLD_ICONV=

Note also that you should remove all the hard-coded settings in Makefile
anyway.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Miles Bader @ 2007-12-05 21:58 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: David Kastrup, Jakub Narebski, git
In-Reply-To: <alpine.LFD.0.99999.0711261601240.9605@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:
> Yet Junio just replied to my mail, apparently using his news reader, and 
> I was directly addressed.

If you're using Gnus as a MUA, and reading via gmane, you can bypass
gmane for followups by setting the group "To List" parameter to the list
address ("git@vger.kernel.org"); be careful _not_ to set the adjacent
"To Address" parameter, which does something else.

After doing that, followups are sent via email with To and CCs correctly
set up, exactly as if you were reading an ordinary mailing list.

I presume other newsreaders having something similar.

-Miles

[You can hit C-M-a in the group summary buffer to modify group parameters]

-- 
`Life is a boundless sea of bitterness'

^ permalink raw reply

* Re: [PATCH 3/6] Test "git remote show" and "git remote prune"
From: René Scharfe @ 2007-12-05 21:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <Pine.LNX.4.64.0712051901550.27959@racer.site>

Johannes Schindelin schrieb:
> While at it, also fix a few instances where a cd was done outside of a
> subshell.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t5505-remote.sh |   34 ++++++++++++++++++++++++++++++++++
>  1 files changed, 34 insertions(+), 0 deletions(-)

It seems to me the patch only adds tests, but doesn't fix existing ones.
 And looking at t5505-remote.sh, every call of cd is already done inside
of a subshell, so there doesn't seem to be anything to fix either. :-?

^ permalink raw reply

* Re: builtin command's prefix question
From: Junio C Hamano @ 2007-12-05 22:12 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <fcaeb9bf0712050856t5d730779q82783fdb9876f41@mail.gmail.com>

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

> I have been looking at setup_git_directory_gently() lately. From my
> understanding, setup_git_directory* will return a string called
> "prefix" that is passed to builtin commands. What is the exact meaning
> of this prefix?

Some historical background is in order.  For a long time, we only worked
from the very top of the work tree and nowhere else.  The path you get
from the user on the command line was supposed to be relative to the top
of the work tree, the user was supposed to be at the top of the work
tree, no work from subdirectories.

This was limiting, so "setup" was introduced.  The commands originally
worked only from the top would chdir up to the top of the work tree if
it was run from a subdirectory.  This however needs adjustments to the
paths we get from the user from the command line (or stdin for commands
that take such).  If we claim we work from a subdirectory, we should
interpret path given by the user who is in a subdirectory as relative to
that subdirectory.  The way to do this adjustment is to prefix the
subdirectory path to the path given by the user.

So, if you start a command from Documentation/ subdirectory, like this:

	$ cd Documentation
	$ git ls-files howto

internally, ls-files would:

 * Notice it was run from Documentation/ subdirectory;

 * cd up to the top level;

 * prefix "Documentation/" to the pathspec given by the user
   (i.e. "howto"), to form "Documentation/howto";

 * Act as before, except that it strips "Documentation/" from its usual
   output, to retain the illusion of working from that subdirectory.

And prefix is "Documentation/" (note the trailing slash) in such a
case.  If you run from the top, it is NULL to signal that there is no
need to do any of these tricks.

> ... Correct me if I'm wrong. In early (read: no worktree)
> days, cwd was moved to working root directory and prefix contained
> relative path from working root directory to the original cwd. So it
> had a few implications:

>  1. A non-empty prefix indicates cwd has been moved

Correct (I do not know if we care, though)

>  2. If cwd is moved, it is moved to working root directory

Correct ("we always work from top of the tree internally" matters)

>  3. cwd+prefix should point to the current directory at the time the
> command was issued (let's call it "user cwd")

Correct.

> Things change a bit since the rise of worktree:
>  - If GIT_DIR is set and GIT_WORK_TREE is not, prefix is relative to
> the to-be-set-up worktree, but cwd is not changed, so point 3 is gone.
>  - If GIT_DIR is not set and GiT_WORK_TREE is,
>   - and it is found that user cwd is inside a gitdir (bare repo), cwd
> has been moved and prefix is empty, cwd+prefix no longer point to user
> cwd
>   - for other cases, cwd may not be worktree (the real worktree will
> be setup in setup_work_tree or setup_git_directory)

The intention is:

 * If GIT_DIR is set but not GIT_WORK_TREE (nor core.worktree in
   config), you are either inside project.git/ directory of bare
   repository or at the toplevel of worktree-full directory.  This has
   been the traditional behaviour before GIT_WORK_TREE and we shouldn't
   break the existing setups that assume this behaviour.  So in that
   sense, with this combination:

   - If the repository is bare, the value of the prefix should not
     matter; the command that wants to look at prefix by definition
     wants to run from a subdirectory but there is no notion of
     "the user directory being a subdirectory of the top of the work
     tree" in a bare repository;

   - If the repository is not bare, the user directory _MUST_ be at the
     top of the work tree, as that is what the traditional behaviour is.
     Anything else would break existing setups.

     IOW, if you use GIT_DIR and still want to run from a subdirectory
     of the worktree, you must have either GIT_WORK_TREE or
     core.worktree to tell where the top of the worktree is, and if you
     don't, then you must be at the top.

   So the right thing to do in this case is not going anywhere and using
   prefix=NULL.

 * I would say it is a misconfiguration if GIT_DIR is not set and
   GIT_WORK_TREE is, as the sole purpose of GIT_WORK_TREE is so that you
   can work from a subdirectory when you set GIT_DIR.  I may be missing
   an obvious use case that this is useful, but I do not think of any.
   Dscho may be able to correct me on this, as he fixed up the original
   work tree series that was even messier quite a bit during the last
   round.

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Steven Grimm @ 2007-12-05 22:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vlk8csetl.fsf@gitster.siamese.dyndns.org>

On Dec 2, 2007, at 6:16 PM, Junio C Hamano wrote:
>> ..., but an
>> "ok, but btw I changed your commit" status from receive-pack seems  
>> like
>> it would be useful, for two reasons:
> Sensible argument.  I stand corrected.

If we want that status in principle, I'd argue that sending down the  
updated commit SHA1 is actually the right way to indicate it, because  
it gives the client all the information it needs to make an  
intelligent choice about what to do next. If you don't transmit the  
modified SHA1, the client will have to do another fetch to find out  
what rewriting was done by the server, and if another push happened in  
the meantime, the client will have to basically guess about which  
commits correspond to the ones it pushed.

I'm going to have to modify the "ok" line for this either way, and  
it's not like the extra 39 bytes (for sending a hex SHA1 instead of a  
one-character status indicator) is going to hurt in any but the  
narrowest corner cases.

But if people really object to that, I will add a simple flag to the  
"ok" line.

-Steve

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Junio C Hamano @ 2007-12-05 22:19 UTC (permalink / raw)
  To: Steven Grimm; +Cc: Jeff King, git
In-Reply-To: <5920F34B-A94B-4C24-A95B-D35F35A4F0C0@midwinter.com>

Steven Grimm <koreth@midwinter.com> writes:

> On Dec 2, 2007, at 6:16 PM, Junio C Hamano wrote:
>>> ..., but an
>>> "ok, but btw I changed your commit" status from receive-pack seems
>>> like
>>> it would be useful, for two reasons:
>> Sensible argument.  I stand corrected.
>
> If we want that status in principle, I'd argue that sending down the
> updated commit SHA1 is actually the right way to indicate it, because
> it gives the client all the information it needs to make an
> intelligent choice about what to do next. If you don't transmit the
> modified SHA1, the client will have to do another fetch to find out
> what rewriting was done by the server, and if another push happened in
> the meantime, the client will have to basically guess about which
> commits correspond to the ones it pushed.

Ok, but the output from fetch is meant to be human readable and we do
not promise parsability, so if we go this route (which I think you made
a sensible argument for) we would need a hook on the pushing end to act
on this (perhaps record the correspondence of pushed and rewritten sha1
somewhere for the hook's own use).

^ permalink raw reply

* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Junio C Hamano @ 2007-12-05 22:22 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Ramsay Jones, Arjen Laarhoven, Brian Gernhardt
In-Reply-To: <200712052226.53543.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>> +AC_MSG_CHECKING([for old iconv()])
>>> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
>>> +	[AC_MSG_RESULT([no])],
>>> +	[AC_MSG_RESULT([yes])
>>> +	OLD_ICONV=YesPlease])
>>> +AC_SUBST(OLD_ICONV)
>>>  
>> 
>> Which result does COMPILE_IFELSE give for non error warnings?  Ok, or
>> Bad?
>
>  - Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND],
>           [ACTION-IF-NOT-FOUND])
>      Run the compiler and compilation flags of the current language
>      (*note Language Choice::) on the INPUT, run the shell commands
>      ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise.  The INPUT
>      can be made by `AC_LANG_PROGRAM' and friends.
>
> And if I have checked correctly code which causes only warnings
> returns Ok (in this case print 'no' after 'checking for old iconv()... '
> and do not set OLD_ICONV, which means it will be unset).

Which means the real-life compilation will get the warning on type
mismatch.  Wasn't OLD_ICONV about squelching that?

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Junio C Hamano @ 2007-12-05 22:29 UTC (permalink / raw)
  To: Steven Grimm; +Cc: Jeff King, git
In-Reply-To: <7vhciwn5rl.fsf@gitster.siamese.dyndns.org>

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

> Ok, but the output from fetch is meant to be human readable and we do
> not promise parsability, so if we go this route (which I think you made

s/parsability/machine &/;

> a sensible argument for) we would need a hook on the pushing end to act
> on this (perhaps record the correspondence of pushed and rewritten sha1
> somewhere for the hook's own use).

s/on this/& information/;
s/own use/& in machine readable way/;

^ permalink raw reply

* Re: [PATCH 3/6] Test "git remote show" and "git remote prune"
From: Johannes Schindelin @ 2007-12-05 22:38 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, gitster
In-Reply-To: <47571F3E.1060903@lsrfire.ath.cx>

Hi,

On Wed, 5 Dec 2007, Ren? Scharfe wrote:

> Johannes Schindelin schrieb:
> > While at it, also fix a few instances where a cd was done outside of a 
> > subshell.
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  t/t5505-remote.sh |   34 ++++++++++++++++++++++++++++++++++
> >  1 files changed, 34 insertions(+), 0 deletions(-)
> 
> It seems to me the patch only adds tests, but doesn't fix existing ones. 
> And looking at t5505-remote.sh, every call of cd is already done inside 
> of a subshell, so there doesn't seem to be anything to fix either. :-?

Right.  This comment is from a long time ago, back when I had my own 
(incomplete) t5505-remote.sh.  It was funny to me that Junio chose exactly 
the same name... but his implementation was different ;-)

I agree that this comment is obsolete.

Thanks,
Dscho

^ permalink raw reply

* [PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning)
From: Jakub Narebski @ 2007-12-05 23:05 UTC (permalink / raw)
  To: git, Junio C Hamano, Pascal Obry
  Cc: Ramsay Jones, Arjen Laarhoven, Brian Gernhardt, Jakub Narebski
In-Reply-To: <7vd4tkn5mk.fsf@gitster.siamese.dyndns.org>

On Wed, 5 Dec 2007, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> Junio C Hamano wrote:
>>> Jakub Narebski <jnareb@gmail.com> writes:
>>>
>>>> +AC_MSG_CHECKING([for old iconv()])
>>>> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
>>>> +	[AC_MSG_RESULT([no])],
>>>> +	[AC_MSG_RESULT([yes])
>>>> +	OLD_ICONV=YesPlease])
>>>> +AC_SUBST(OLD_ICONV)
>>>>  
>>> 
>>> Which result does COMPILE_IFELSE give for non error warnings?
>>> Ok, or Bad?
>>
>>  - Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND],
>>           [ACTION-IF-NOT-FOUND])
>>      Run the compiler and compilation flags of the current language
>>      (*note Language Choice::) on the INPUT, run the shell commands
>>      ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise.  The INPUT
>>      can be made by `AC_LANG_PROGRAM' and friends.
>>
>> And if I have checked correctly code which causes only warnings
>> returns Ok (in this case print 'no' after 'checking for old iconv()... '
>> and do not set OLD_ICONV, which means it will be unset).
> 
> Which means the real-life compilation will get the warning on type
> mismatch.  Wasn't OLD_ICONV about squelching that?

Gah, I don't know why I though OLD_ICONV was about compile errors, and
not about compile warnings. This version uses -Werror to check for
warnings; I hope it doesn't give false positives...


On Wed, 5 Dec 2007, Pascal Obry wrote:
> Jakub Narebski a écrit :
>> ---
>> This patch needs checking if it correctly sets OLD_ICONV
>> when needed.  I have checked only that it is not set when
>> with new iconv() declaration.  Could people using Cygwin
>> (and other with OLD_ICONV: Darwin) test it?
> 
> Not working on Cygwin:
> 
>    $ autoconf
>    $ ./configure --prefix=/usr/local --build=i686-pc-cygwin
>    ...
>    configure: CHECKS for header files
>    checking for old iconv()... no
> 
> It should be yes above. And in config.mak.autogen we have:
> 
>    OLD_ICONV=

Check out current version of patch. It should work correctly now
(I thought OLD_ICONV was about compile errors, and it is about
squelching compile warnings). It should give now:

  $ make configure
  $ ./configure --prefix=/usr/local --build=i686-pc-cygwin

  configure: CHECKS for header files
  checking for old iconv()... yes
 
  $ cat config.mak.autogen
 
  OLD_ICONV=UnfortunatelyYes

> Note also that you should remove all the hard-coded settings
> in Makefile anyway.

No, I should not. ./configure script is purely optional in git,
and compiling should work with reasonable defaults even if you
don't have autoconf installed and/or you don't want to run
./configure script (because e.g. it is too slow).

-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH] autoconf: Add test for OLD_ICONV (squelching compiler warning)

Update configure.ac (and config.mak.in) to keep up with git
development by adding [compile] test whether your library has an old
iconv(), where the second (input buffer pointer) parameter is declared
with type (const char **) (OLD_ICONV).

Test-proposed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 config.mak.in |    1 +
 configure.ac  |   24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/config.mak.in b/config.mak.in
index 11d256e..7d5df9b 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -41,4 +41,5 @@ NO_STRTOUMAX=@NO_STRTOUMAX@
 NO_SETENV=@NO_SETENV@
 NO_MKDTEMP=@NO_MKDTEMP@
 NO_ICONV=@NO_ICONV@
+OLD_ICONV=@OLD_ICONV@
 NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
diff --git a/configure.ac b/configure.ac
index 5f8a15b..196ab3e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -212,6 +212,30 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 
 
 ## Checks for header files.
+AC_MSG_NOTICE([CHECKS for header files])
+#
+# Define OLD_ICONV if your library has an old iconv(), where the second
+# (input buffer pointer) parameter is declared with type (const char **).
+AC_DEFUN([OLDICONVTEST_SRC], [[
+#include <iconv.h>
+
+int main(void)
+{
+	iconv_t cd;
+	char *ibp, *obp;
+	size_t insz, outsz;
+	iconv(cd, &ibp, &insz, &obp, &outsz);
+}
+]])
+AC_MSG_CHECKING([for old iconv()])
+CFLAGS_ORIG=$CFLAGS
+CFLAGS="$CFLAGS -Werror"
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+	[AC_MSG_RESULT([no])],
+	[AC_MSG_RESULT([yes])
+	OLD_ICONV=UnfortunatelyYes])
+CFLAGS=$CFLAGS_ORIG
+AC_SUBST(OLD_ICONV)
 
 
 ## Checks for typedefs, structures, and compiler characteristics.
-- 
1.5.3.7

^ permalink raw reply related

* Re: builtin command's prefix question
From: Junio C Hamano @ 2007-12-05 23:22 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <7vlk88n648.fsf@gitster.siamese.dyndns.org>

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

>  * I would say it is a misconfiguration if GIT_DIR is not set and
>    GIT_WORK_TREE is, as the sole purpose of GIT_WORK_TREE is so that you
>    can work from a subdirectory when you set GIT_DIR.  I may be missing
>    an obvious use case that this is useful, but I do not think of any.
>    Dscho may be able to correct me on this, as he fixed up the original
>    work tree series that was even messier quite a bit during the last
>    round.

I had a short discussion with Dscho on this.  One scenario that was
brought up was this.

You have a work tree of mixed contents that logically belong to
separate repository.  Think $HOME/.?*, and tracking .vimrc and
.pinerc as separate "projects".  You have $HOME/gits/vim.git and
$HOME/gits/pine.git bare-looking repositories.

The "kosher" way of doing this might be:

        $ cd $HOME
        $ GIT_WORK_TREE=$HOME; export GIT_WORK_TREE
        $ edit .vimrc
        $ GIT_DIR=gits/vim.git git commit .vimrc
        $ edit .pinerc
        $ GIT_DIR=gits/pine.git git commit .pinerc

However, if we define setup() to behave this way when GIT_DIR is not
defined and GIT_WORK_TREE is:

 (1) internally pretend as if GIT_DIR was specified to be the
     directory where the command was started from (iow, do getcwd()
     once upon startup);

 (2) chdir to GIT_WORK_TREE (which means "callers of setup() always
     run from the top of the work tree");

 (3) set prefix to NULL;

Then this workflow becomes possible:

	$ cd $HOME
        $ GIT_WORK_TREE=$HOME; export GIT_WORK_TREE
	$ edit .vimrc .pinerc
        $ cd $HOME/gits/vimrc.git && git commit .vimrc
        $ cd $HOME/gits/pinerc.git && git commit .pinerc

I am not convinced this is giving any natural user experience, nor an
alternative:

	$ cd $HOME
        $ GIT_WORK_TREE=$HOME; export GIT_WORK_TREE
        $ cd $HOME/gits/vimrc.git
	$ edit $HOME/.vimrc
        $ git commit .vimrc
        $ cd $HOME/gits/pinerc.git
	$ edit $HOME/.pinerc
        $ git commit .pinerc

While I still think the combination is simply crazy and does not make
any sense, if enough users on the list agrees that it makes sense, I
wouldn't mind setup() did (1) to (3) mentioned above.  The alternative
is simply to declare GIT_WORK_TREE without GIT_DIR is a nonsense and
either error error out or ignore GIT_WORK_TREE, which might be easier to
explain to people.

Opinions?

^ permalink raw reply

* Re: [PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning)
From: Junio C Hamano @ 2007-12-05 23:27 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: git, Pascal Obry, Ramsay Jones, Arjen Laarhoven, Brian Gernhardt
In-Reply-To: <1196895948-25115-1-git-send-email-jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

>> Which means the real-life compilation will get the warning on type
>> mismatch.  Wasn't OLD_ICONV about squelching that?
>
> Gah, I don't know why I though OLD_ICONV was about compile errors, and
> not about compile warnings. This version uses -Werror to check for
> warnings; I hope it doesn't give false positives...

But use of -Werror means you are married to gcc, doesn't it?

How important is it to detect OLD_ICONV anyway, I have to wonder?

> On Wed, 5 Dec 2007, Pascal Obry wrote:
> ...
>> Note also that you should remove all the hard-coded settings
>> in Makefile anyway.
>
> No, I should not. ./configure script is purely optional in git,

Correct.  Thanks.

^ permalink raw reply

* Re: builtin command's prefix question
From: Junio C Hamano @ 2007-12-05 23:43 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <7vk5nsloa8.fsf@gitster.siamese.dyndns.org>

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

> I am not convinced this is giving any natural user experience, nor an
> alternative:
>
>       $ cd $HOME
>       $ GIT_WORK_TREE=$HOME; export GIT_WORK_TREE
>       $ cd $HOME/gits/vimrc.git
>       $ edit $HOME/.vimrc
>       $ git commit .vimrc
>       $ cd $HOME/gits/pinerc.git
>       $ edit $HOME/.pinerc
>       $ git commit .pinerc
>
> While I still think the combination is simply crazy and does not make
> any sense, if enough users on the list agrees that it makes sense, I
> wouldn't mind setup() did (1) to (3) mentioned above.  The alternative
> is simply to declare GIT_WORK_TREE without GIT_DIR is a nonsense and
> either error error out or ignore GIT_WORK_TREE, which might be easier to
> explain to people.
>
> Opinions?

Side note.

By saying the above, I do not mean it is nonsense to try supporting a
work tree that is an overlay of disjoint set of work tree files from
multiple repositories/projects.  I do think it is a worthwhile goal to
support such a layout.

What I do not like is the way the ugly workaround does it, by
encouraging (rather, requiring) to issue git commands from a location
that is completely separate from the actual editing of the content
happens.

An independent issue of supporting such a overlayed work tree layout is
what to do with .gitignore files.  I think, especially with the recent
addition of --exclude-standard to ls-files and setup_standard_excludes()
in dir.c, we could have a per repository configuration that names the
per repository exclude files, so that .gitignore-vim and .gitignore-pine
can co-exist in $HOME, each excluding everything other than the
project's own files.

^ permalink raw reply

* Re: Cosmetic git-am interactive bug
From: Jeff Garzik @ 2007-12-05 23:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v8x491v79.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Jeff Garzik <jeff@garzik.org> writes:
> 
>> The use of the older one-line summary led me to believe that it had
>> not committed my changelog edits.  Looking at the result, however,
>> proved that the commit changelog was my new, corrected version.
> 
> I knew about this for quite some time but it was a very low priority for
> me.  This should fix it.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thanks!  :)

^ permalink raw reply

* [BUG/PATCH] git grep shows the same hit repeatedly for unmerged paths
From: Junio C Hamano @ 2007-12-06  0:13 UTC (permalink / raw)
  To: git

When the index is unmerged, e.g.

	$ git ls-files -u
        100644 faf413748eb6ccb15161a212156c5e348302b1b6 1	setup.c
        100644 145eca50f41d811c4c8fcb21ed2604e6b2971aba 2	setup.c
        100644 cb9558c49b6027bf225ba2a6154c4d2a52bcdbe2 3	setup.c

running "git grep" for work tree files repeats hits for each unmerged
stage.

	$ git grep -n -e setup_work_tree -- '*.[ch]'
        setup.c:209:void setup_work_tree(void)
        setup.c:209:void setup_work_tree(void)
        setup.c:209:void setup_work_tree(void)

This should fix it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 builtin-grep.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index bbf747f..f1ff8dc 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -343,12 +343,12 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 			memcpy(name + 2, ce->name, len + 1);
 		}
 		argv[argc++] = name;
-		if (argc < MAXARGS)
-			continue;
-		status = flush_grep(opt, argc, nr, argv, &kept);
-		if (0 < status)
-			hit = 1;
-		argc = nr + kept;
+		if (MAXARGS <= argc) {
+			status = flush_grep(opt, argc, nr, argv, &kept);
+			if (0 < status)
+				hit = 1;
+			argc = nr + kept;
+		}
 		if (ce_stage(ce)) {
 			do {
 				i++;

^ permalink raw reply related

* Re: fetch_refs_via_pack() discards status?
From: André Goddard Rosa @ 2007-12-06  0:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Daniel Barkalow, Shawn O. Pearce, Git Mailing List
In-Reply-To: <7vwsrsonqm.fsf@gitster.siamese.dyndns.org>

On Dec 5, 2007 7:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > On Tue, 4 Dec 2007, Junio C Hamano wrote:
> >
> >> The code calls fetch_pack() to get the list of refs it fetched, and
> >> discards refs and always returns 0 to signal success.
> >>
> >> But builtin-fetch-pack.c::fetch_pack() has error cases.  The function
> >> returns NULL if error is detected (shallow-support side seems to choose
> >> to die but I suspect that is easily fixable to error out as well).
> >>
> >> Shouldn't fetch_refs_via_pack() propagate that error to the caller?
> >
> > I think that's right. I think I got as far as having the error status from
> > fetch_pack() actually returned correctly, and then failed to look at it.
> > I'd personally avoid testing a pointer to freed memory, but that's
> > obviously not actually wrong.
> >
> >       -Daniel
>
> Hmph, is that an Ack that the patchlet is actually a bugfix?
>

Hi, Mr. Junio!

     My 2 cents: I think he means that we should not test a freed pointer:

       free_refs(refs);
       free(dest);                     <===
-       return 0;
+       return (refs ? 0 : -1);     <===

Best regards,
-- 
[]s,
André Goddard

^ permalink raw reply

* Re: [PATCH] Do check_repository_format() early
From: Junio C Hamano @ 2007-12-06  1:18 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Johannes Schindelin
In-Reply-To: <20071205132514.GA5580@laptop>

Thanks, this looks very sensible to me.

^ permalink raw reply

* [PATCH] Revert "git-am: catch missing author date early."
From: Junio C Hamano @ 2007-12-06  1:20 UTC (permalink / raw)
  To: git; +Cc: Jens Axboe
In-Reply-To: <7vir3conab.fsf@gitster.siamese.dyndns.org>

This reverts commit 6e9e0327b7d7f384d8a223b4bc40330ef3e7fb61.  People
can prepare a text file with Subject: and From: headers and feed it to
"am" (pretending the file is a piece of e-mail), and have actually been
doing so.  Strict checking for Date: breaks this established workflow,
which wants to record the time of the commit as the author time.

Thanks go to Jens Axboe for injection of sanity.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-am.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 2e40708..76c1c84 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -307,9 +307,9 @@ do
 	GIT_AUTHOR_EMAIL="$(sed -n '/^Email/ s/Email: //p' "$dotest/info")"
 	GIT_AUTHOR_DATE="$(sed -n '/^Date/ s/Date: //p' "$dotest/info")"
 
-	if test -z "$GIT_AUTHOR_EMAIL" || test -z "$GIT_AUTHOR_DATE"
+	if test -z "$GIT_AUTHOR_EMAIL"
 	then
-		echo "Patch does not have valid authorship information."
+		echo "Patch does not have a valid e-mail address."
 		stop_here $this
 	fi
 
-- 
1.5.3.7-2132-gbd1cf

^ permalink raw reply related

* [PATCH 1/3] Documentation: color.* = true means "auto"
From: Junio C Hamano @ 2007-12-06  2:05 UTC (permalink / raw)
  To: git
In-Reply-To: <475697BC.2090701@viscovery.net>

We forgot to document the earlier sanity-fix.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 72a33e9..0e45ec5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -359,8 +359,8 @@ clean.requireForce::
 
 color.branch::
 	A boolean to enable/disable color in the output of
-	gitlink:git-branch[1]. May be set to `true` (or `always`),
-	`false` (or `never`) or `auto`, in which case colors are used
+	gitlink:git-branch[1]. May be set to `always`,
+	`false` (or `never`) or `auto` (or `true`), in which case colors are used
 	only when the output is to a terminal. Defaults to false.
 
 color.branch.<slot>::
@@ -378,9 +378,9 @@ second is the background.  The position of the attribute, if any,
 doesn't matter.
 
 color.diff::
-	When true (or `always`), always use colors in patch.
-	When false (or `never`), never.  When set to `auto`, use
-	colors only when the output is to the terminal.
+	When set to `always`, always use colors in patch.
+	When false (or `never`), never.  When set to `true` or `auto`, use
+	colors only when the output is to the terminal. Defaults to false.
 
 color.diff.<slot>::
 	Use customized color for diff colorization.  `<slot>` specifies
@@ -397,8 +397,8 @@ color.pager::
 
 color.status::
 	A boolean to enable/disable color in the output of
-	gitlink:git-status[1]. May be set to `true` (or `always`),
-	`false` (or `never`) or `auto`, in which case colors are used
+	gitlink:git-status[1]. May be set to `always`,
+	`false` (or `never`) or `auto` (or `true`), in which case colors are used
 	only when the output is to a terminal. Defaults to false.
 
 color.status.<slot>::
-- 
1.5.3.7-2132-gbd1cf

^ permalink raw reply related

* [PATCH 0/3] Reroll colorized "git add -i"
From: Junio C Hamano @ 2007-12-06  2:05 UTC (permalink / raw)
  To: git
In-Reply-To: <1196906706-11170-3-git-send-email-gitster@pobox.com>

Another try of the colorized "git add -i" series.

[PATCH 1/3] Documentation: color.* = true means "auto"

This is a documentation fix that has already been applied to 'master'
(not pushed out yet as of this writing).

[PATCH 2/3] git config --get-colorbool

This allows scripts to figure out if they should use colors.

[PATCH 3/3] Color support for "git-add -i"

This is Dan's colorized git-add -i, but uses --get-color and
--get-colorbool options to git-config.

^ permalink raw reply

* Re: * [BUG] "git clean" does not pay attention to its parameters
From: Junio C Hamano @ 2007-12-06  2:14 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: Nanako Shiraishi, git
In-Reply-To: <20071205152816.GA21347@mediacenter.austin.rr.com>

Shawn Bohrer <shawn.bohrer@gmail.com> writes:

> Before the rewrite in C git clean would refuse to remove a directory if
> you said:
>
>    git clean dir
>
> without using the -d parameter.  Per your suggestion this check causes
> git clean to remove the directory anyway since you explicitly asked it
> to.

Thanks for reminding me of that issue.

^ 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