* Re: Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-01 9:57 UTC (permalink / raw)
To: Jongman Heo; +Cc: Junio C Hamano, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <17285958.791361359710080706.JavaMail.weblogic@epml01>
On Fri, Feb 01, 2013 at 09:14:41AM +0000, Jongman Heo wrote:
> I can reproduce the issue in my machine (RedHat Enterprise 5, x86 PAE) as follows.
Great, thanks for taking the time to reproduce.
> But in my different machine (Fedora 16 x86) I can't reproduce.
That makes me wonder if it is related to the gcc or make version. I
couldn't reproduce the problem on my gcc-4.1 system, though. My make is:
$ make --version
GNU Make 3.81
[...]
> $ git reset --hard v1.8.1 # back to v1.8.1
> $ make clean
> $ make all install # this git works fine
After this step, what does builtin/.depend/fetch.o.d contain? It should
show a dependency of builtin/fetch.o on string-list (among other
things).
> $ git pull # top commit 9a6c84e6, "Merge git://ozlabs.org/~paulus/gitk"
> $ make all install
Can you try running "make -d builtin/fetch.o" here instead of "make all
install"? Can you confirm that it reads builtin/.depend/fetch.o, and
that fetch.o gets rebuilt (you should even be able to see the list of
"newer than" dependencies in the debug output)?
Another thing to double-check: does it work if you instead run
make all install COMPUTE_HEADER_DEPENDENCIES=no
?
-Peff
^ permalink raw reply
* Re: [PATCH 4/6] introduce a commit metapack
From: Jeff King @ 2013-02-01 10:00 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130130135607.GA23154@lanh>
On Wed, Jan 30, 2013 at 08:56:07PM +0700, Nguyen Thai Ngoc Duy wrote:
> Another point, but not really important at this stage, I think we have
> memory leak somewhere (lookup_commit??). It used up to 800 MB RES on
> linux-2.6.git while generating the cache.
We generate (and then leak!) the linked list in commit_metapack_write.
That may be the culprit.
-Peff
^ permalink raw reply
* Re: [PATCH 4/6] introduce a commit metapack
From: Jeff King @ 2013-02-01 10:15 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130131110656.GA28093@lanh>
On Thu, Jan 31, 2013 at 06:06:56PM +0700, Nguyen Thai Ngoc Duy wrote:
> On Wed, Jan 30, 2013 at 09:16:29PM +0700, Duy Nguyen wrote:
> > Perhaps we could store abbrev sha-1 instead of full sha-1. Nice
> > space/time trade-off.
>
> Following the on-disk format experiment yesterday, I changed the
> format to:
>
> - a list a _short_ SHA-1 of cached commits
> - a list of cache entries, each (5 uint32_t) consists of:
> - uint32_t for the index in .idx sha-1 table to get full SHA-1 of
> the commit
> - uint32_t for timestamp
> - uint32_t for tree, 1st and 2nd parents for the index in .idx
> table
Thanks for working on this, as it was the next step I was going to take. :)
The short-sha1 is a clever idea. Looks like it saves us on the order of
4MB for linux-2.6 (versus the full 20-byte sha1). Not as big as the
savings we get from dropping the other 3 sha1's to uint32_t, but still
not bad.
I guess the next steps in iterating on this would be:
1. splitting out the refactoring here into separate patches
2. squashing the cleaned-up bits into my patch 4/6
3. deciding whether this should go into a separate file or as part of
index v3. Your offsets depend on the .idx file having a sorted sha1
list. That is not likely to change, but it would still be nice to
make sure they cannot get out of sync. I'm still curious what the
performance impact is for mmap-ing N versus N+8MB.
> The length of SHA-1 is chosen to be able to unambiguously identify any
> cached commits. Full SHA-1 check is done after to catch false
> positives.
Just to be clear, these false positives come because the abbreviation is
unambiguous within the packfile, but we might be looking for a commit
that is not even in our pack, right?
-Peff
^ permalink raw reply
* [PATH/RFC] Revert "compat: add strtok_r()"
From: Erik Faye-Lund @ 2013-02-01 10:33 UTC (permalink / raw)
To: git; +Cc: Erik Faye-Lund
This reverts commit 78457bc0ccc1af8b9eb776a0b17986ebd50442bc.
commit 28c5d9e ("vcs-svn: drop string_pool") previously removed
the only call-site for strtok_r. So let's get rid of the compat
implementation as well.
Conflicts:
Makefile
config.mak.in
configure.ac
---
I just noticed that we have an unused compat-implementation of strtok_r,
so here's a patch to get rid of it.
Makefile | 6 ------
compat/strtok_r.c | 61 -------------------------------------------------------
configure.ac | 6 ------
git-compat-util.h | 5 -----
4 files changed, 78 deletions(-)
delete mode 100644 compat/strtok_r.c
diff --git a/Makefile b/Makefile
index 1b30d7b..6d16a52 100644
--- a/Makefile
+++ b/Makefile
@@ -98,8 +98,6 @@ all::
#
# Define NO_MKSTEMPS if you don't have mkstemps in the C library.
#
-# Define NO_STRTOK_R if you don't have strtok_r in the C library.
-#
# Define NO_FNMATCH if you don't have fnmatch in the C library.
#
# Define NO_FNMATCH_CASEFOLD if your fnmatch function doesn't have the
@@ -1202,10 +1200,6 @@ endif
ifdef NO_STRTOULL
COMPAT_CFLAGS += -DNO_STRTOULL
endif
-ifdef NO_STRTOK_R
- COMPAT_CFLAGS += -DNO_STRTOK_R
- COMPAT_OBJS += compat/strtok_r.o
-endif
ifdef NO_FNMATCH
COMPAT_CFLAGS += -Icompat/fnmatch
COMPAT_CFLAGS += -DNO_FNMATCH
diff --git a/compat/strtok_r.c b/compat/strtok_r.c
deleted file mode 100644
index 7b5d568..0000000
--- a/compat/strtok_r.c
+++ /dev/null
@@ -1,61 +0,0 @@
-/* Reentrant string tokenizer. Generic version.
- Copyright (C) 1991,1996-1999,2001,2004 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, write to the Free
- Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
- 02111-1307 USA. */
-
-#include "../git-compat-util.h"
-
-/* Parse S into tokens separated by characters in DELIM.
- If S is NULL, the saved pointer in SAVE_PTR is used as
- the next starting point. For example:
- char s[] = "-abc-=-def";
- char *sp;
- x = strtok_r(s, "-", &sp); // x = "abc", sp = "=-def"
- x = strtok_r(NULL, "-=", &sp); // x = "def", sp = NULL
- x = strtok_r(NULL, "=", &sp); // x = NULL
- // s = "abc\0-def\0"
-*/
-char *
-gitstrtok_r (char *s, const char *delim, char **save_ptr)
-{
- char *token;
-
- if (s == NULL)
- s = *save_ptr;
-
- /* Scan leading delimiters. */
- s += strspn (s, delim);
- if (*s == '\0')
- {
- *save_ptr = s;
- return NULL;
- }
-
- /* Find the end of the token. */
- token = s;
- s = strpbrk (token, delim);
- if (s == NULL)
- /* This token finishes the string. */
- *save_ptr = token + strlen (token);
- else
- {
- /* Terminate the token and make *SAVE_PTR point past it. */
- *s = '\0';
- *save_ptr = s + 1;
- }
- return token;
-}
diff --git a/configure.ac b/configure.ac
index 1991258..f3462d9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -901,12 +901,6 @@ GIT_CHECK_FUNC(strcasestr,
[NO_STRCASESTR=YesPlease])
GIT_CONF_SUBST([NO_STRCASESTR])
#
-# Define NO_STRTOK_R if you don't have strtok_r
-GIT_CHECK_FUNC(strtok_r,
-[NO_STRTOK_R=],
-[NO_STRTOK_R=YesPlease])
-GIT_CONF_SUBST([NO_STRTOK_R])
-#
# Define NO_FNMATCH if you don't have fnmatch
GIT_CHECK_FUNC(fnmatch,
[NO_FNMATCH=],
diff --git a/git-compat-util.h b/git-compat-util.h
index e5a4b74..89a44ab 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -408,11 +408,6 @@ extern uintmax_t gitstrtoumax(const char *, char **, int);
extern intmax_t gitstrtoimax(const char *, char **, int);
#endif
-#ifdef NO_STRTOK_R
-#define strtok_r gitstrtok_r
-extern char *gitstrtok_r(char *s, const char *delim, char **save_ptr);
-#endif
-
#ifdef NO_HSTRERROR
#define hstrerror githstrerror
extern const char *githstrerror(int herror);
--
1.8.0.msysgit.0.3.gd0186ec
^ permalink raw reply related
* Re: [PATH/RFC] Revert "compat: add strtok_r()"
From: Erik Faye-Lund @ 2013-02-01 10:37 UTC (permalink / raw)
To: git; +Cc: Erik Faye-Lund
In-Reply-To: <1359714786-1912-1-git-send-email-kusmabite@gmail.com>
On Fri, Feb 1, 2013 at 11:33 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> This reverts commit 78457bc0ccc1af8b9eb776a0b17986ebd50442bc.
>
> commit 28c5d9e ("vcs-svn: drop string_pool") previously removed
> the only call-site for strtok_r. So let's get rid of the compat
> implementation as well.
>
> Conflicts:
> Makefile
> config.mak.in
> configure.ac
> ---
>
> I just noticed that we have an unused compat-implementation of strtok_r,
> so here's a patch to get rid of it.
>
Ugh, I forgot this part:
diff --git a/config.mak.uname b/config.mak.uname
index bea34f0..7f3018f 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -321,7 +321,6 @@ ifeq ($(uname_S),Windows)
NO_UNSETENV = YesPlease
NO_STRCASESTR = YesPlease
NO_STRLCPY = YesPlease
- NO_STRTOK_R = YesPlease
NO_FNMATCH = YesPlease
NO_MEMMEM = YesPlease
# NEEDS_LIBICONV = YesPlease
@@ -476,7 +475,6 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_UNSETENV = YesPlease
NO_STRCASESTR = YesPlease
NO_STRLCPY = YesPlease
- NO_STRTOK_R = YesPlease
NO_FNMATCH = YesPlease
NO_MEMMEM = YesPlease
NEEDS_LIBICONV = YesPlease
^ permalink raw reply related
* Re: [PATCH 4/6] introduce a commit metapack
From: Jeff King @ 2013-02-01 10:40 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130131110656.GA28093@lanh>
On Thu, Jan 31, 2013 at 06:06:56PM +0700, Nguyen Thai Ngoc Duy wrote:
> On Wed, Jan 30, 2013 at 09:16:29PM +0700, Duy Nguyen wrote:
> > Perhaps we could store abbrev sha-1 instead of full sha-1. Nice
> > space/time trade-off.
>
> Following the on-disk format experiment yesterday, I changed the
> format to:
>
> - a list a _short_ SHA-1 of cached commits
> - a list of cache entries, each (5 uint32_t) consists of:
> - uint32_t for the index in .idx sha-1 table to get full SHA-1 of
> the commit
> - uint32_t for timestamp
> - uint32_t for tree, 1st and 2nd parents for the index in .idx
> table
BTW, I needed the minor fixups below to silence some warnings from your
patch. Here are the cold and warm cache timings I got, as compared to
stock git and my implementation:
Pack | Cold Revs | Warm Revs
-------+--------------+------------
stock | 12.54 | 4.14
me | 4.76 (-62%) | 0.66 (-84%)
duy | 4.36 (-65%) | 0.55 (-86%)
Not surprising; yours is just a bit faster in terms of CPU, and even
gains a little more in the cold cache case. Nice. Of course that is just
gravy on top of the smaller disk usage, too. :)
---
diff --git a/commit-metapack.c b/commit-metapack.c
index c984b8e..78fd961 100644
--- a/commit-metapack.c
+++ b/commit-metapack.c
@@ -106,7 +106,7 @@ int commit_metapack(unsigned char *sha1,
for (p = commit_metapacks; p; p = p->next) {
struct commit_entry *data;
uint32_t p1, p2;
- unsigned lo, hi, mi;
+ unsigned lo, hi;
int pos;
/* sha1_entry_pos does not work with abbreviated sha-1 */
@@ -161,7 +161,7 @@ static void get_commits(struct metapack_writer *mw,
struct write_cb *write_cb = (struct write_cb *)data;
enum object_type type = sha1_object_info(sha1, NULL);
struct commit *c;
- int p1, p2;
+ int p1 = -1, p2 = -1;
if (type != OBJ_COMMIT)
return;
diff --git a/commit.c b/commit.c
index b326201..5b776f8 100644
--- a/commit.c
+++ b/commit.c
@@ -309,7 +309,7 @@ static int parse_commit_metapack(struct commit *item)
static int parse_commit_metapack(struct commit *item)
{
- unsigned char *tree, *p1, *p2;
+ const unsigned char *tree, *p1, *p2;
uint32_t ts;
if (commit_metapack(item->object.sha1, &ts, &tree, &p1, &p2) < 0)
^ permalink raw reply related
* Re: Re: Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-01 10:44 UTC (permalink / raw)
To: Jongman Heo; +Cc: Junio C Hamano, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <32006438.796571359714624362.JavaMail.weblogic@epml01>
On Fri, Feb 01, 2013 at 10:30:24AM +0000, Jongman Heo wrote:
> Short answer;
>
> * Version of make is 3.81 on both machines
> * builtin/fetch.o is not rebuilt (see entire log below)
> * git works fine with "make all install COMPUTE_HEADER_DEPENDENCIES=no"
OK, that gets us closer. It's definitely a problem with the automatic
header dependencies, then.
> [...make debug output...]
> Reading makefile `builtin/.depend/fetch.o.d' (search path) (no ~ expansion)...
So we definitely have the dep file...
> [...]
> Finished prerequisites of target file `builtin/fetch.o'.
> Prerequisite `builtin/fetch.c' is older than target `builtin/fetch.o'.
> Prerequisite `GIT-CFLAGS' is older than target `builtin/fetch.o'.
> No need to remake target `builtin/fetch.o'.
But it doesn't stimulate any prerequisites in make, which is weird.
What's in builtin/.depend/fetch.o.d?
-Peff
^ permalink raw reply
* Re: Re: Re: Re: Segmentation fault with latest git (070c57df)
From: Jongman Heo @ 2013-02-01 10:59 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Thomas Rast, git, Antoine Pelisse
>> [...]
>> Finished prerequisites of target file `builtin/fetch.o'.
>> Prerequisite `builtin/fetch.c' is older than target `builtin/fetch.o'.
>> Prerequisite `GIT-CFLAGS' is older than target `builtin/fetch.o'.
>> No need to remake target `builtin/fetch.o'.
>
>But it doesn't stimulate any prerequisites in make, which is weird.
>What's in builtin/.depend/fetch.o.d?
>
>-Peff
Hi,
please see below~.
$ cat builtin/.depend/fetch.o.d
fetch.o: builtin/fetch.c cache.h git-compat-util.h compat/bswap.h \
strbuf.h hash.h advice.h gettext.h convert.h refs.h commit.h object.h \
tree.h decorate.h builtin.h cache.h commit.h notes.h string-list.h \
string-list.h remote.h transport.h remote.h run-command.h \
parse-options.h sigchain.h submodule.h connected.h argv-array.h
cache.h:
git-compat-util.h:
compat/bswap.h:
strbuf.h:
hash.h:
advice.h:
gettext.h:
convert.h:
refs.h:
commit.h:
object.h:
tree.h:
decorate.h:
builtin.h:
cache.h:
commit.h:
notes.h:
string-list.h:
string-list.h:
remote.h:
transport.h:
remote.h:
run-command.h:
parse-options.h:
sigchain.h:
submodule.h:
connected.h:
argv-array.h:
^ permalink raw reply
* Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines
From: John Keeping @ 2013-02-01 11:16 UTC (permalink / raw)
To: Michael Haggerty; +Cc: John Keeping, git
In-Reply-To: <510B7F4B.7040902@alum.mit.edu>
On Fri, Feb 01, 2013 at 09:39:39AM +0100, Michael Haggerty wrote:
> On 01/30/2013 09:31 PM, John Keeping wrote:
> > On Wed, Jan 30, 2013 at 11:05:10AM +0100, Michael Haggerty wrote:
> >> [...] maybe we should establish a small Python library of
> >> compatibility utilities (like a small "six"). [...]
> >> But I haven't had time to think of where to put such a library, how to
> >> install it, etc.
> >
> > If we want to go that route, I think restructuring the
> > "git_remote_helpers" directory and re-using its infrastructure for
> > installing the "Git Python modules" would be the way to go. The
> > directory structure would become something like this:
> >
> > git/
> > `-- python/
> > |-- Makefile # existing file pulled out of git_remote_helpers
> > |-- < some new utility library >
> > |-- git_remote_helpers
> > | |-- __init__.py
> > | |-- git
> > | | |-- __init__.py
> > | | |-- exporter.py
> > | | |-- git.py
> > | | |-- importer.py
> > | | |-- non_local.py
> > | | `-- repo.py
> > | `-- util.py
> > |-- setup.cfg # existing file pulled out of git_remote_helpers
> > `-- setup.py # existing file pulled out of git_remote_helpers
> >
> >
> > It looks like the GitPython project[1] as already taken the "git" module
> > name, so perhaps we should use "git_core" if we do introduce a new
> > module.
> >
> > [1] http://pypi.python.org/pypi/GitPython
>
> This sounds reasonable. But not all Python code will go under the
> "python" subdirectory, right? For example, I am working on a Python
> script that fits thematically under contrib/hooks.
I was thinking of it as analagous with the "perl" directory that
currently exists. So the "python" directory will contain library code
but scripts can live wherever is most appropriate.
One way of looking at it is: could the user want to have this installed
for all available versions of Python? For a script, the answer is "no"
because they will call it and it will just run. For libraries, you want
them to be available with whatever Python interpreter you happen to be
running (assuming that it is a version supported by the library).
> OTOH (I'm thinking aloud here) it is probably a bad idea for a hook
> script to depend on a Python module that is part of git itself. Doing
> so would make the hook script depend on a particular version of git (or
> at least a version with a compatible Python module). But users might be
> reluctant to upgrade git just to install a hook script.
I don't think such a dependency is a bad idea in the longer term. If a
"Git Python library" is developed, then at some point most people who
have Git installed will have some version of that library - it becomes a
case of perhaps wanting to limit yourself to some subset of the library
rather than just not using it.
In fact, git_remote_helpers has been available since Git 1.7.0 and
contains a lot of functionality that is more generic than its name
suggests.
John
^ permalink raw reply
* Aw: Re: Aw: Re: [PATCH v3 3/6] Change 'git' to 'Git' whenever the whole system is referred to #2
From: Thomas Ackermann @ 2013-02-01 18:04 UTC (permalink / raw)
To: gitster, th.acker; +Cc: git, davvid
In-Reply-To: <7vzjzodfp1.fsf@alter.siamese.dyndns.org>
>
> I think we have waited long enough and as far as I recall we didn't
> see any reports of misconversion or forgotten conversion, so I'll
> squash the fixes parked on the topic branch, whose tip is at
> bfb8e1eb6375afb (fixup! Change 'git' to 'Git' whenever the whole
> system is referred to #4, 2013-01-22), and merge the result to
> 'next' sometime tomorrow.
>
Here are my final fixups on top of bfb8e1eb6375afb:
I morphed the remarks from Junio and David into patch form and
also did the reverse check for wrong usage of 'Git' instead of 'git'.
While doing the later I also noticed some small glitches and fixed them.
Squash at your will.
[PATCH 1/6] fixup! fixup! Change 'git' to 'Git' whenever the whole system is referred to #2
[PATCH 2/6] fixup! fixup! fixup! Change 'git' to 'Git' whenever the whole system is referred to #1
[PATCH 3/6] fixup! Documentation: avoid poor-man's small caps
[PATCH 4/6] Fix places where 'Git' should be 'git
[PATCH 5/6] Add a description for 'gitfile' to glossary
[PATCH 6/6] Use consistent links for User Manual and Everyday Git; Fix a quoting error
---
Thomas
^ permalink raw reply
* [PATCH 1/6] fixup! fixup! Change 'git' to 'Git' whenever the whole system is referred to #2
From: Thomas Ackermann @ 2013-02-01 18:06 UTC (permalink / raw)
To: gitster, th.acker; +Cc: git, davvid
In-Reply-To: <210945773.1090030.1359741840305.JavaMail.ngmail@webmail18.arcor-online.net>
Found by David and Junio:
Inconsistent quoting within config.txt should be fixed in a separate patch.
Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
---
Documentation/config.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3652ee0..1bfbc7a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1773,7 +1773,7 @@ pull.twohead::
The default merge strategy to use when pulling a single branch.
push.default::
- Defines the action Git push should take if no refspec is given
+ Defines the action `git push` should take if no refspec is given
on the command line, no refspec is configured in the remote, and
no refspec is implied by any of the options given on the command
line. Possible values are:
--
1.8.0.msysgit.0
---
Thomas
^ permalink raw reply related
* [PATCH 2/6] fixup! fixup! fixup! Change 'git' to 'Git' whenever the whole system is referred to #1
From: Thomas Ackermann @ 2013-02-01 18:07 UTC (permalink / raw)
To: gitster, th.acker; +Cc: git, davvid
In-Reply-To: <210945773.1090030.1359741840305.JavaMail.ngmail@webmail18.arcor-online.net>
Found by Junio:
Change git-dir to $GIT_DIR and git-file to gitfile.
Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
---
Documentation/git-rev-parse.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index c743469..14386ed 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -187,9 +187,9 @@ print a message to stderr and exit with nonzero status.
Flags and parameters to be parsed.
--resolve-git-dir <path>::
- Check if <path> is a valid git-dir or a git-file pointing to a valid
- git-dir. If <path> is a valid git-dir the resolved path to git-dir will
- be printed.
+ Check if <path> is a valid `$GIT_DIR` or a gitfile pointing to a valid
+ `$GIT_DIR`. If <path> is a valid `$GIT_DIR` the resolved path to `$GIT_DIR`
+ will be printed.
include::revisions.txt[]
--
1.8.0.msysgit.0
---
Thomas
^ permalink raw reply related
* [PATCH 3/6] fixup! Documentation: avoid poor-man's small caps
From: Thomas Ackermann @ 2013-02-01 18:08 UTC (permalink / raw)
To: gitster, th.acker; +Cc: git, davvid
In-Reply-To: <210945773.1090030.1359741840305.JavaMail.ngmail@webmail18.arcor-online.net>
Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
---
Documentation/git-tools.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-tools.txt b/Documentation/git-tools.txt
index 338986a..ad8b823 100644
--- a/Documentation/git-tools.txt
+++ b/Documentation/git-tools.txt
@@ -29,7 +29,7 @@ Alternative/Augmentative Porcelains
- *pg* (http://www.spearce.org/category/projects/scm/pg/)
pg is a shell script wrapper around Git to help the user manage a set of
- patches to files. pg is somewhat like quilt or StGIT, but it does have a
+ patches to files. pg is somewhat like quilt or StGit, but it does have a
slightly different feature set.
@@ -60,7 +60,7 @@ History Viewers
- *qgit* (http://digilander.libero.it/mcostalba/)
- QGit is a git/StGIT GUI viewer built on Qt/C++. QGit could be used
+ QGit is a git/StGit GUI viewer built on Qt/C++. QGit could be used
to browse history and directory tree, view annotated files, commit
changes cherry picking single files or applying patches.
Currently it is the fastest and most feature rich among the Git
--
1.8.0.msysgit.0
---
Thomas
^ permalink raw reply related
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Junio C Hamano @ 2013-02-01 18:09 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, git
In-Reply-To: <20130201085248.GA30644@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Should this be "From:" Shawn? The tone of the message and the S-O-B
> order makes it look like it.
Yes. I should have left that line when edited the format-patch
output in my MUA to say I was resending something that vger rejected
and people did not see after tweaking the patch to slip their taboo
list.
>> @@ -133,16 +135,19 @@ static struct discovery* discover_refs(const char *service)
>> last->buf = last->buf_alloc;
>>
>> if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
>> - /* smart HTTP response; validate that the service
>> + /*
>> + * smart HTTP response; validate that the service
>> * pkt-line matches our request.
>> */
>> - struct strbuf exp = STRBUF_INIT;
>> -
>> + strbuf_addf(&exp, "application/x-%s-advertisement", service);
>> + if (strbuf_cmp(&exp, &type))
>> + die("invalid content-type %s", type.buf);
>
> Hmm. I wondered if it is possible for a non-smart server to send us down
> this code path, which would now complain of the bogus content-type.
> Something like an info/refs file with:
>
> # 1
> # the comment above is meaningless, but puts a "#" at position 4.
>
> But I note that we would already die in the next line:
>
>> if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
>> die("%s has invalid packet header", refs_url);
>
> so I do not think the patch makes anything worse. However, should we
> take this opportunity to make the "did we get a smart response" test
> more robust? That is, should we actually be checking the content-type
> in the outer conditional, and going down the smart code-path if it is
> application/x-%s-advertisement, and otherwise treating the result as
> dumb?
Does the outer caller that switches between dumb and smart actually
know what service type it is requesting (I am not familiar with the
callchain involved)? Even if it doesn't, it may still make sense.
> As a side note, should we (can we) care about the content-type for dumb
> http? It should probably be text/plain or application/octet-stream, but
> I would not be surprised if we get a variety of random junk in the real
> world, though.
The design objective of dumb http protocol was to allow working with
any dumb bit transfer thing, so I'd prefer to keep it lenient and
allow application/x-git-loose-object-file and somesuch.
Thanks.
^ permalink raw reply
* [PATCH 4/6] Fix places where 'Git' should be 'git
From: Thomas Ackermann @ 2013-02-01 18:10 UTC (permalink / raw)
To: gitster, th.acker; +Cc: git, davvid
In-Reply-To: <210945773.1090030.1359741840305.JavaMail.ngmail@webmail18.arcor-online.net>
Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
---
Documentation/git-p4.txt | 4 ++--
Documentation/git-remote-fd.txt | 2 +-
Documentation/gittutorial.txt | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 1d1b020..3417cb3 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -334,11 +334,11 @@ configuration file. This allows future 'git p4 submit' commands to
work properly; the submit command looks only at the variable and does
not have a command-line option.
-The full syntax for a p4 view is documented in 'p4 help views'. 'Git p4'
+The full syntax for a p4 view is documented in 'p4 help views'. 'git p4'
knows only a subset of the view syntax. It understands multi-line
mappings, overlays with '+', exclusions with '-' and double-quotes
around whitespace. Of the possible wildcards, 'git p4' only handles
-'...', and only when it is at the end of the path. 'Git p4' will complain
+'...', and only when it is at the end of the path. 'git p4' will complain
if it encounters an unhandled wildcard.
Bugs in the implementation of overlap mappings exist. If multiple depot
diff --git a/Documentation/git-remote-fd.txt b/Documentation/git-remote-fd.txt
index 9bfe12c..933c2ad 100644
--- a/Documentation/git-remote-fd.txt
+++ b/Documentation/git-remote-fd.txt
@@ -12,7 +12,7 @@ SYNOPSIS
DESCRIPTION
-----------
This helper uses specified file descriptors to connect to a remote Git server.
-This is not meant for end users but for programs and scripts calling Git
+This is not meant for end users but for programs and scripts calling git
fetch, push or archive.
If only <infd> is given, it is assumed to be a bidirectional socket connected
diff --git a/Documentation/gittutorial.txt b/Documentation/gittutorial.txt
index 6091988..8262196 100644
--- a/Documentation/gittutorial.txt
+++ b/Documentation/gittutorial.txt
@@ -462,7 +462,7 @@ Exploring history
Git history is represented as a series of interrelated commits. We
have already seen that the 'git log' command can list those commits.
-Note that first line of each Git log entry also gives a name for the
+Note that first line of each git log entry also gives a name for the
commit:
-------------------------------------
--
1.8.0.msysgit.0
---
Thomas
^ permalink raw reply related
* [PATCH 5/6] Add a description for 'gitfile' to glossary
From: Thomas Ackermann @ 2013-02-01 18:11 UTC (permalink / raw)
To: gitster, th.acker; +Cc: git, davvid
In-Reply-To: <210945773.1090030.1359741840305.JavaMail.ngmail@webmail18.arcor-online.net>
Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
---
Documentation/glossary-content.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 7c15bc0..ddf2f66 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -149,6 +149,9 @@ to point at the new commit.
[[def_git_archive]]Git archive::
Synonym for <<def_repository,repository>> (for arch people).
+[[def_gitfile]]gitfile::
+ A `.git` file which points to a `$GIT_DIR` (used for Git submodules).
+
[[def_grafts]]grafts::
Grafts enables two otherwise different lines of development to be joined
together by recording fake ancestry information for commits. This way
--
1.8.0.msysgit.0
---
Thomas
^ permalink raw reply related
* [PATCH 6/6] Use consistent links for User Manual and Everyday Git; Fix a quoting error
From: Thomas Ackermann @ 2013-02-01 18:12 UTC (permalink / raw)
To: gitster, th.acker; +Cc: git, davvid
In-Reply-To: <210945773.1090030.1359741840305.JavaMail.ngmail@webmail18.arcor-online.net>
Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
---
Documentation/git.txt | 2 +-
Documentation/gitcore-tutorial.txt | 6 +++---
Documentation/gittutorial-2.txt | 2 +-
Documentation/gittutorial.txt | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index c431ba2..c5f6845 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -23,7 +23,7 @@ and full access to internals.
See linkgit:gittutorial[7] to get started, then see
link:everyday.html[Everyday Git] for a useful minimum set of
-commands. The link:user-manual.html[Git User's Manual] has a more
+commands. The link:user-manual.html[The Git User's Manual] has a more
in-depth introduction.
After you mastered the basic concepts, you can come back to this
diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
index 59c1c17..88e9839 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -17,7 +17,7 @@ work with a Git repository.
If you just need to use Git as a revision control system you may prefer
to start with "A Tutorial Introduction to Git" (linkgit:gittutorial[7]) or
-link:user-manual.html[the Git User Manual].
+link:user-manual.html[The Git User's Manual].
However, an understanding of these low-level tools can be helpful if
you want to understand Git's internals.
@@ -568,8 +568,8 @@ can explore on your own.
[NOTE]
Most likely, you are not directly using the core
-Git Plumbing commands, but using Porcelain such as 'git add', `git-rm'
-and `git-commit'.
+Git Plumbing commands, but using Porcelain such as 'git add', 'git rm'
+and 'git commit'.
Tagging a version
diff --git a/Documentation/gittutorial-2.txt b/Documentation/gittutorial-2.txt
index 94c906e..2db4ea3 100644
--- a/Documentation/gittutorial-2.txt
+++ b/Documentation/gittutorial-2.txt
@@ -406,7 +406,7 @@ pages for any of the git commands; one good place to start would be
with the commands mentioned in link:everyday.html[Everyday Git]. You
should be able to find any unknown jargon in linkgit:gitglossary[7].
-The link:user-manual.html[Git User's Manual] provides a more
+The link:user-manual.html[The Git User's Manual] provides a more
comprehensive introduction to Git.
linkgit:gitcvs-migration[7] explains how to
diff --git a/Documentation/gittutorial.txt b/Documentation/gittutorial.txt
index 8262196..e63fd9f 100644
--- a/Documentation/gittutorial.txt
+++ b/Documentation/gittutorial.txt
@@ -656,7 +656,7 @@ digressions that may be interesting at this point are:
* linkgit:gitworkflows[7]: Gives an overview of recommended
workflows.
- * link:everyday.html[Everyday Git with 20 Commands Or So]
+ * link:everyday.html[Everyday Git]
* linkgit:gitcvs-migration[7]: Git for CVS users.
--
1.8.0.msysgit.0
---
Thomas
^ permalink raw reply related
* Re: Aw: Re: Aw: Re: [PATCH v3 3/6] Change 'git' to 'Git' whenever the whole system is referred to #2
From: Junio C Hamano @ 2013-02-01 18:37 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: git, davvid
In-Reply-To: <210945773.1090030.1359741840305.JavaMail.ngmail@webmail18.arcor-online.net>
Thomas Ackermann <th.acker@arcor.de> writes:
> >
>> I think we have waited long enough and as far as I recall we didn't
>> see any reports of misconversion or forgotten conversion, so I'll
>> squash the fixes parked on the topic branch, whose tip is at
>> bfb8e1eb6375afb (fixup! Change 'git' to 'Git' whenever the whole
>> system is referred to #4, 2013-01-22), and merge the result to
>> 'next' sometime tomorrow.
>
> Here are my final fixups on top of bfb8e1eb6375afb:
> I morphed the remarks from Junio and David into patch form and
> also did the reverse check for wrong usage of 'Git' instead of 'git'.
> While doing the later I also noticed some small glitches and fixed them.
> Squash at your will.
Thanks.
> [PATCH 1/6] fixup! fixup! Change 'git' to 'Git' whenever the whole system is referred to #2
> [PATCH 2/6] fixup! fixup! fixup! Change 'git' to 'Git' whenever the whole system is referred to #1
> [PATCH 3/6] fixup! Documentation: avoid poor-man's small caps
> [PATCH 4/6] Fix places where 'Git' should be 'git
> [PATCH 5/6] Add a description for 'gitfile' to glossary
> [PATCH 6/6] Use consistent links for User Manual and Everyday Git; Fix a quoting error
^ permalink raw reply
* Re: [PATCH 5/6] Add a description for 'gitfile' to glossary
From: Junio C Hamano @ 2013-02-01 18:41 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: git, davvid
In-Reply-To: <274664435.1090127.1359742284591.JavaMail.ngmail@webmail18.arcor-online.net>
Thomas Ackermann <th.acker@arcor.de> writes:
> Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
> ---
> Documentation/glossary-content.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
> index 7c15bc0..ddf2f66 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -149,6 +149,9 @@ to point at the new commit.
> [[def_git_archive]]Git archive::
> Synonym for <<def_repository,repository>> (for arch people).
>
> +[[def_gitfile]]gitfile::
> + A `.git` file which points to a `$GIT_DIR` (used for Git submodules).
> +
It is more like 'used by' ;-) People are free to write their
Porcelain system that uses this mechanism.
> [[def_grafts]]grafts::
> Grafts enables two otherwise different lines of development to be joined
> together by recording fake ancestry information for commits. This way
^ permalink raw reply
* [PATCH] l10n: de.po: translate "revision" consistently as "Revision"
From: Ralf Thielow @ 2013-02-01 18:42 UTC (permalink / raw)
To: trast, jojo; +Cc: git, jk, stimming, Ralf Thielow
In-Reply-To: <1359398014-3325-2-git-send-email-ralf.thielow@gmail.com>
In the current German translation, the word "revision" was
translated as both "Version" (translation of "commit") and
"Revision". Since a revision in Git is not necessarily a
commit, we should not translate it with the same word in
order to give the user an idea that it's not necessarily
the same. After this commit, "revision" is consistently
translated as "Revision".
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
This replaces a patch [1] earlier sent to the list.
I've also updated the glossary [2] of the German translation.
[1]
http://article.gmane.org/gmane.comp.version-control.git/214852
[2]
https://github.com/ralfth/git-po-de/wiki/Glossary
po/de.po | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/po/de.po b/po/de.po
index ed8330a..0a94b4b 100644
--- a/po/de.po
+++ b/po/de.po
@@ -4438,7 +4438,7 @@ msgid ""
msgstr ""
"Kein externes Projektarchiv angegeben. Bitte geben Sie entweder eine URL\n"
"oder den Namen des externen Archivs an, von welchem neue\n"
-"Versionen angefordert werden sollen."
+"Revisionen angefordert werden sollen."
#: builtin/fetch.c:932
msgid "You need to specify a tag name."
@@ -4623,7 +4623,7 @@ msgstr ""
#: builtin/grep.c:22
msgid "git grep [options] [-e] <pattern> [<rev>...] [[--] <path>...]"
-msgstr "git grep [Optionen] [-e] <Muster> [<Version>...] [[--] <Pfad>...]"
+msgstr "git grep [Optionen] [-e] <Muster> [<Revision>...] [[--] <Pfad>...]"
#: builtin/grep.c:217
#, c-format
@@ -5501,7 +5501,7 @@ msgstr "ungültiges in-reply-to: %s"
#: builtin/log.c:913
msgid "git format-patch [options] [<since> | <revision range>]"
-msgstr "git format-patch [Optionen] [<seit> | <Versionsbereich>]"
+msgstr "git format-patch [Optionen] [<seit> | <Revisionsbereich>]"
#: builtin/log.c:958
msgid "Two output directories?"
@@ -6193,7 +6193,7 @@ msgstr "findet Vorfahren für eine einzelne n-Wege-Zusammenführung"
#: builtin/merge-base.c:100
msgid "list revs not reachable from others"
-msgstr "listet Revisionen auf, die nicht durch Andere erreichbar sind"
+msgstr "listet Versionen auf, die nicht durch Andere erreichbar sind"
#: builtin/merge-base.c:102
msgid "is the first one ancestor of the other?"
@@ -7952,7 +7952,7 @@ msgstr "'%s' Zurücksetzung ist in einem bloßen Projektarchiv nicht erlaubt"
#: builtin/reset.c:333
#, c-format
msgid "Could not reset index file to revision '%s'."
-msgstr "Konnte Bereitstellungsdatei nicht zu Version '%s' zurücksetzen."
+msgstr "Konnte Bereitstellungsdatei nicht zu Revision '%s' zurücksetzen."
#: builtin/reset.c:339
msgid "Unstaged changes after reset:"
@@ -9053,7 +9053,7 @@ msgstr "nicht erkannte Option: '$arg'"
#: git-bisect.sh:99
#, sh-format
msgid "'$arg' does not appear to be a valid revision"
-msgstr "'$arg' scheint keine gültige Version zu sein"
+msgstr "'$arg' scheint keine gültige Revision zu sein"
#: git-bisect.sh:117
msgid "Bad HEAD - I need a HEAD"
@@ -9116,7 +9116,7 @@ msgid ""
"You need to give me at least one good and one bad revisions.\n"
"(You can use \"git bisect bad\" and \"git bisect good\" for that.)"
msgstr ""
-"Sie müssen mindestens eine korrekte und eine fehlerhafte Version angeben.\n"
+"Sie müssen mindestens eine korrekte und eine fehlerhafte Revision angeben.\n"
"(Sie können dafür \"git bisect bad\" und \"git bisect good\" benutzen.)"
#: git-bisect.sh:292
@@ -9126,7 +9126,7 @@ msgid ""
"(You can use \"git bisect bad\" and \"git bisect good\" for that.)"
msgstr ""
"Sie müssen mit \"git bisect start\" beginnen.\n"
-"Danach müssen Sie mindestens eine korrekte und eine fehlerhafte Version "
+"Danach müssen Sie mindestens eine korrekte und eine fehlerhafte Revision "
"angeben.\n"
"(Sie können dafür \"git bisect bad\" und \"git bisect good\" benutzen.)"
@@ -9650,7 +9650,7 @@ msgstr ""
#: git-submodule.sh:662
#, sh-format
msgid "Unable to find current revision in submodule path '$sm_path'"
-msgstr "Konnte aktuelle Version in Unterprojekt-Pfad '$sm_path' nicht finden"
+msgstr "Konnte aktuelle Revision in Unterprojekt-Pfad '$sm_path' nicht finden"
#: git-submodule.sh:671 git-submodule.sh:695
#, sh-format
--
1.8.1.2.623.g7d3a2ef
^ permalink raw reply related
* Re: [PATCH 6/6] Use consistent links for User Manual and Everyday Git; Fix a quoting error
From: Junio C Hamano @ 2013-02-01 18:43 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: git, davvid
In-Reply-To: <549641245.1090170.1359742373184.JavaMail.ngmail@webmail18.arcor-online.net>
Thomas Ackermann <th.acker@arcor.de> writes:
> Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
> ---
> Documentation/git.txt | 2 +-
> Documentation/gitcore-tutorial.txt | 6 +++---
> Documentation/gittutorial-2.txt | 2 +-
> Documentation/gittutorial.txt | 2 +-
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index c431ba2..c5f6845 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -23,7 +23,7 @@ and full access to internals.
>
> See linkgit:gittutorial[7] to get started, then see
> link:everyday.html[Everyday Git] for a useful minimum set of
> -commands. The link:user-manual.html[Git User's Manual] has a more
> +commands. The link:user-manual.html[The Git User's Manual] has a more
> in-depth introduction.
Thanks.
^ permalink raw reply
* Re: [PATCH 3/6] fixup! Documentation: avoid poor-man's small caps
From: Junio C Hamano @ 2013-02-01 18:44 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: git, davvid
In-Reply-To: <544568068.1090092.1359742136040.JavaMail.ngmail@webmail18.arcor-online.net>
Thomas Ackermann <th.acker@arcor.de> writes:
> Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
> ---
I think this deserves to be a separate patch in the series, that
says "StGit is the right spelling of the system, not StGIT",
referring to https://gna.org/projects/stgit/ or somewhere, in the
log message.
Will massage the above in; no need to resend.
Thanks.
> Documentation/git-tools.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-tools.txt b/Documentation/git-tools.txt
> index 338986a..ad8b823 100644
> --- a/Documentation/git-tools.txt
> +++ b/Documentation/git-tools.txt
> @@ -29,7 +29,7 @@ Alternative/Augmentative Porcelains
> - *pg* (http://www.spearce.org/category/projects/scm/pg/)
>
> pg is a shell script wrapper around Git to help the user manage a set of
> - patches to files. pg is somewhat like quilt or StGIT, but it does have a
> + patches to files. pg is somewhat like quilt or StGit, but it does have a
> slightly different feature set.
>
>
> @@ -60,7 +60,7 @@ History Viewers
>
> - *qgit* (http://digilander.libero.it/mcostalba/)
>
> - QGit is a git/StGIT GUI viewer built on Qt/C++. QGit could be used
> + QGit is a git/StGit GUI viewer built on Qt/C++. QGit could be used
> to browse history and directory tree, view annotated files, commit
> changes cherry picking single files or applying patches.
> Currently it is the fastest and most feature rich among the Git
^ permalink raw reply
* Re: [PATCH] Rename {git- => git}remote-helpers.txt
From: Junio C Hamano @ 2013-02-01 18:52 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, John Keeping, git, Matthieu Moy, Max Horn
In-Reply-To: <20130201073352.GB970@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Right. So we have some that must be "gitfoo", and others that do not
> care. If we turned "githooks" into "git-hooks" and removed the "is it a
> command?" magic from "git help", then "git help hooks" would still find
> hooks. And likewise, "git help gitignore" would still find "gitignore".
> But you could no longer say "git help ignore" to find "gitignore".
Which I think is a good thing ;-)
> Which personally I am OK with. It is not a sensible thing to ask for;
> the concept is not "ignore", but rather "the gitignore file".
Yes, exactly.
> 4. Replace the rename "gitfoo" above with a "see git-foo..." pointer.
> Users of "git help foo" would not ever see this, but people who
> have trained their fingers to type "man gitfoo" would, along with
> anybody following an outdated HTML link.
>
> 5. Update internal references to "linkgit:gitfoo" to point to
> "git-foo".
>
> Hmm. That really does not seem so bad. The biggest downside is the
> people who have to see the redirect made in step 4.
Yeah, I see that a show-stopper in the whole sequence.
This is one of the "if we had perfect knowledge we would have
designed it this way, and we could still migrate our current system
to that ideal, but it is dubious the difference between the current
system and the ideal will outweigh the cost of migration" moment,
isn't it?
^ permalink raw reply
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Jeff King @ 2013-02-01 18:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Pearce, git
In-Reply-To: <7vip6bc3e1.fsf@alter.siamese.dyndns.org>
On Fri, Feb 01, 2013 at 10:09:26AM -0800, Junio C Hamano wrote:
> > so I do not think the patch makes anything worse. However, should we
> > take this opportunity to make the "did we get a smart response" test
> > more robust? That is, should we actually be checking the content-type
> > in the outer conditional, and going down the smart code-path if it is
> > application/x-%s-advertisement, and otherwise treating the result as
> > dumb?
>
> Does the outer caller that switches between dumb and smart actually
> know what service type it is requesting (I am not familiar with the
> callchain involved)? Even if it doesn't, it may still make sense.
I was specifically thinking of this (on top of your patch):
diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..63680a8 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,12 @@ static struct discovery* discover_refs(const char *service)
last->buf_alloc = strbuf_detach(&buffer, &last->len);
last->buf = last->buf_alloc;
- if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
+ strbuf_addf(&exp, "application/x-%s-advertisement", service);
+ if (maybe_smart && !strbuf_cmp(&exp, &type)) {
/*
* smart HTTP response; validate that the service
* pkt-line matches our request.
*/
- strbuf_addf(&exp, "application/x-%s-advertisement", service);
- if (strbuf_cmp(&exp, &type))
- die("invalid content-type %s", type.buf);
if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
die("%s has invalid packet header", refs_url);
if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
To just follow the dumb path if we don't get the content-type we expect.
We may want to keep the '#' format check in addition (packet_get_line
will check it and die, anyway, but we may want to drop back to
considering it dumb, just to protect against a badly configured dumb
server which uses our mime type, but I do not think it likely).
> > As a side note, should we (can we) care about the content-type for dumb
> > http? It should probably be text/plain or application/octet-stream, but
> > I would not be surprised if we get a variety of random junk in the real
> > world, though.
>
> The design objective of dumb http protocol was to allow working with
> any dumb bit transfer thing, so I'd prefer to keep it lenient and
> allow application/x-git-loose-object-file and somesuch.
Yeah, I do not think it really buys us anything in practice, and we have
no way of knowing what kind of crap is in the wild. Not worth it.
-Peff
^ permalink raw reply related
* Re: [PATCH] Rename {git- => git}remote-helpers.txt
From: Jeff King @ 2013-02-01 19:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, John Keeping, git, Matthieu Moy, Max Horn
In-Reply-To: <7vr4kzamt7.fsf@alter.siamese.dyndns.org>
On Fri, Feb 01, 2013 at 10:52:52AM -0800, Junio C Hamano wrote:
> > 4. Replace the rename "gitfoo" above with a "see git-foo..." pointer.
> > Users of "git help foo" would not ever see this, but people who
> > have trained their fingers to type "man gitfoo" would, along with
> > anybody following an outdated HTML link.
> >
> > 5. Update internal references to "linkgit:gitfoo" to point to
> > "git-foo".
> >
> > Hmm. That really does not seem so bad. The biggest downside is the
> > people who have to see the redirect made in step 4.
>
> Yeah, I see that a show-stopper in the whole sequence.
>
> This is one of the "if we had perfect knowledge we would have
> designed it this way, and we could still migrate our current system
> to that ideal, but it is dubious the difference between the current
> system and the ideal will outweigh the cost of migration" moment,
> isn't it?
Yeah, perhaps. I did the patch series just to see what the effort would
be like. But at this point I am fine if we drop it (it sounded like
Jonathan was in favor of this direction, so maybe he wants to make a
final argument).
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox