* [PATCH 2/2] editor.c: Libify launch_editor()
From: Stephan Beyer @ 2008-07-25 16:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin, Stephan Beyer
In-Reply-To: <1217003322-10291-1-git-send-email-s-beyer@gmx.net>
This patch removes exit()/die() calls and builtin-specific messages
from launch_editor(), so that it can be used as a general libgit.a
function to launch an editor.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
builtin-commit.c | 6 +++++-
builtin-tag.c | 6 +++++-
editor.c | 24 ++++++++++++------------
foo | 1 +
strbuf.h | 2 +-
5 files changed, 24 insertions(+), 15 deletions(-)
create mode 100644 foo
diff --git a/builtin-commit.c b/builtin-commit.c
index 6a9dc0e..9a11ca0 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -646,7 +646,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
char index[PATH_MAX];
const char *env[2] = { index, NULL };
snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
- launch_editor(git_path(commit_editmsg), NULL, env);
+ if (launch_editor(git_path(commit_editmsg), NULL, env)) {
+ fprintf(stderr,
+ "Please supply the message using either -m or -F option.\n");
+ exit(1);
+ }
}
if (!no_verify &&
diff --git a/builtin-tag.c b/builtin-tag.c
index 219f51d..325b1b2 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -295,7 +295,11 @@ static void create_tag(const unsigned char *object, const char *tag,
write_or_die(fd, tag_template, strlen(tag_template));
close(fd);
- launch_editor(path, buf, NULL);
+ if (launch_editor(path, buf, NULL)) {
+ fprintf(stderr,
+ "Please supply the message using either -m or -F option.\n");
+ exit(1);
+ }
unlink(path);
free(path);
diff --git a/editor.c b/editor.c
index 483b62d..eebc3e9 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
{
const char *editor, *terminal;
@@ -15,12 +15,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
editor = getenv("EDITOR");
terminal = getenv("TERM");
- if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
- fprintf(stderr,
- "Terminal is dumb but no VISUAL nor EDITOR defined.\n"
- "Please supply the message using either -m or -F option.\n");
- exit(1);
- }
+ if (!editor && (!terminal || !strcmp(terminal, "dumb")))
+ return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
if (!editor)
editor = "vi";
@@ -28,6 +24,7 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
+ int failed;
const char *args[6];
struct strbuf arg0;
@@ -43,14 +40,17 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
args[i++] = path;
args[i] = NULL;
- if (run_command_v_opt_cd_env(args, 0, NULL, env))
- die("There was a problem with the editor %s.", editor);
+ failed = run_command_v_opt_cd_env(args, 0, NULL, env);
strbuf_release(&arg0);
+ if (failed)
+ return error("There was a problem with the editor '%s'.",
+ editor);
}
if (!buffer)
- return;
+ return 0;
if (strbuf_read_file(buffer, path, 0) < 0)
- die("could not read message file '%s': %s",
- path, strerror(errno));
+ return error("could not read file '%s': %s",
+ path, strerror(errno));
+ return 0;
}
diff --git a/foo b/foo
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/foo
@@ -0,0 +1 @@
+
diff --git a/strbuf.h b/strbuf.h
index 0c6ffad..eba7ba4 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -123,6 +123,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
extern int strbuf_getline(struct strbuf *, FILE *, int);
extern void stripspace(struct strbuf *buf, int skip_comments);
-extern void launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
+extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
#endif /* STRBUF_H */
--
1.6.0.rc0.102.ga1791
^ permalink raw reply related
* [PATCH 1/2] Move launch_editor() from builtin-tag.c to editor.c
From: Stephan Beyer @ 2008-07-25 16:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin, Stephan Beyer
In-Reply-To: <alpine.DEB.1.00.0807251636140.11976@eeepc-johanness>
launch_editor() is declared in strbuf.h but defined in builtin-tag.c.
This patch moves launch_editor() into a new source file editor.c,
but keeps the declaration in strbuf.h.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
Johannes Schindelin wrote:
> > This means I should send them again?
>
> If you want to be nice to the maintainer, yes. If not, no.
I understand ;-)
No need to play the bad guy, so here's the patchset again.
To be kind to the maintainer, I've also run the test suite again,
all tests passed except t4116*.sh, but this is not my fault.
It's the fault of 9a885fac.
Regards,
Stephan
Makefile | 1 +
builtin-tag.c | 53 -----------------------------------------------------
editor.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+), 53 deletions(-)
create mode 100644 editor.c
diff --git a/Makefile b/Makefile
index b01cf1c..df5b0d0 100644
--- a/Makefile
+++ b/Makefile
@@ -410,6 +410,7 @@ LIB_OBJS += diff-no-index.o
LIB_OBJS += diff-lib.o
LIB_OBJS += diff.o
LIB_OBJS += dir.o
+LIB_OBJS += editor.o
LIB_OBJS += entry.o
LIB_OBJS += environment.o
LIB_OBJS += exec_cmd.o
diff --git a/builtin-tag.c b/builtin-tag.c
index c2cca6c..219f51d 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -23,59 +23,6 @@ static const char * const git_tag_usage[] = {
static char signingkey[1000];
-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
-{
- const char *editor, *terminal;
-
- editor = getenv("GIT_EDITOR");
- if (!editor && editor_program)
- editor = editor_program;
- if (!editor)
- editor = getenv("VISUAL");
- if (!editor)
- editor = getenv("EDITOR");
-
- terminal = getenv("TERM");
- if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
- fprintf(stderr,
- "Terminal is dumb but no VISUAL nor EDITOR defined.\n"
- "Please supply the message using either -m or -F option.\n");
- exit(1);
- }
-
- if (!editor)
- editor = "vi";
-
- if (strcmp(editor, ":")) {
- size_t len = strlen(editor);
- int i = 0;
- const char *args[6];
- struct strbuf arg0;
-
- strbuf_init(&arg0, 0);
- if (strcspn(editor, "$ \t'") != len) {
- /* there are specials */
- strbuf_addf(&arg0, "%s \"$@\"", editor);
- args[i++] = "sh";
- args[i++] = "-c";
- args[i++] = arg0.buf;
- }
- args[i++] = editor;
- args[i++] = path;
- args[i] = NULL;
-
- if (run_command_v_opt_cd_env(args, 0, NULL, env))
- die("There was a problem with the editor %s.", editor);
- strbuf_release(&arg0);
- }
-
- if (!buffer)
- return;
- if (strbuf_read_file(buffer, path, 0) < 0)
- die("could not read message file '%s': %s",
- path, strerror(errno));
-}
-
struct tag_filter {
const char *pattern;
int lines;
diff --git a/editor.c b/editor.c
new file mode 100644
index 0000000..483b62d
--- /dev/null
+++ b/editor.c
@@ -0,0 +1,56 @@
+#include "cache.h"
+#include "strbuf.h"
+#include "run-command.h"
+
+void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+{
+ const char *editor, *terminal;
+
+ editor = getenv("GIT_EDITOR");
+ if (!editor && editor_program)
+ editor = editor_program;
+ if (!editor)
+ editor = getenv("VISUAL");
+ if (!editor)
+ editor = getenv("EDITOR");
+
+ terminal = getenv("TERM");
+ if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
+ fprintf(stderr,
+ "Terminal is dumb but no VISUAL nor EDITOR defined.\n"
+ "Please supply the message using either -m or -F option.\n");
+ exit(1);
+ }
+
+ if (!editor)
+ editor = "vi";
+
+ if (strcmp(editor, ":")) {
+ size_t len = strlen(editor);
+ int i = 0;
+ const char *args[6];
+ struct strbuf arg0;
+
+ strbuf_init(&arg0, 0);
+ if (strcspn(editor, "$ \t'") != len) {
+ /* there are specials */
+ strbuf_addf(&arg0, "%s \"$@\"", editor);
+ args[i++] = "sh";
+ args[i++] = "-c";
+ args[i++] = arg0.buf;
+ }
+ args[i++] = editor;
+ args[i++] = path;
+ args[i] = NULL;
+
+ if (run_command_v_opt_cd_env(args, 0, NULL, env))
+ die("There was a problem with the editor %s.", editor);
+ strbuf_release(&arg0);
+ }
+
+ if (!buffer)
+ return;
+ if (strbuf_read_file(buffer, path, 0) < 0)
+ die("could not read message file '%s': %s",
+ path, strerror(errno));
+}
--
1.6.0.rc0.102.ga1791
^ permalink raw reply related
* Re: [PATCH] Avoid warning when From: is encoded
From: Junio C Hamano @ 2008-07-25 16:33 UTC (permalink / raw)
To: Peter Valdemar Mørch; +Cc: git
In-Reply-To: <1216991208-18782-1-git-send-email-4ux6as402@sneakemail.com>
Peter Valdemar Mørch <4ux6as402@sneakemail.com> writes:
> From: Peter Valdemar Mørch <peter@morch.com>
>
> In commit 0706bd19ef9b41e7519df2c73796ef93484272fd $1 is used from a regexp
> without using () to set up $1. Later, when that value was used, it caused a
> warning about a variable being undefined.
>
> Signed-off-by: Peter Valdemar Mørch <peter@morch.com>
Thanks. The patch is obviously correct; I think you are fixing 8291db6
(git-send-email: add charset header if we add encoded 'From', 2007-11-16),
not 0706bd1 (send-email: specify content-type of --compose body,
2008-03-28).
Will apply to maint and merge upwards.
^ permalink raw reply
* [PATCH] Set TAR in t/Makefile and in t4116-apply-reverse.sh
From: Stephan Beyer @ 2008-07-25 16:37 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, Junio C Hamano, Stephan Beyer
In-Reply-To: <TE3N1FoSy-vEEv0qsAyvBwBMMq2RuJCWYw7DNLBC7mEh6PxM1LCsOw@cipher.nrlssc.navy.mil>
Hence, the test passes also when you run "make" in t/
or when you invoke t4116-apply-reverse.sh directly,
without $TAR being set.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
t/Makefile | 2 +-
t/t4116-apply-reverse.sh | 1 +
2 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/t/Makefile b/t/Makefile
index 0d65ced..b720943 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -5,7 +5,7 @@
#GIT_TEST_OPTS=--verbose --debug
SHELL_PATH ?= $(SHELL)
-TAR ?= $(TAR)
+TAR ?= tar
RM ?= rm -f
# Shell quote;
diff --git a/t/t4116-apply-reverse.sh b/t/t4116-apply-reverse.sh
index 2298ece..3ff5d9e 100755
--- a/t/t4116-apply-reverse.sh
+++ b/t/t4116-apply-reverse.sh
@@ -8,6 +8,7 @@ test_description='git apply in reverse
'
. ./test-lib.sh
+TAR=${TAR:-tar}
test_expect_success setup '
--
1.6.0.rc0.102.ga1791
^ permalink raw reply related
* Re: [PATCH] Avoid warning when From: is encoded
From: Jon Loeliger @ 2008-07-25 16:39 UTC (permalink / raw)
To: sverre; +Cc: Abhijit Menon-Sen, Peter Valdemar Mørch, git
In-Reply-To: <bd6139dc0807250901n7408a8e6w5dead238e897fc03@mail.gmail.com>
Sverre Rabbelier wrote:
> Acked-by is reserved for people who are "owners" of the area the patch
> touches.
I love pronouncements like this. While that may be exactly true
for the Git project, it is not, in general, always true. Within
parts of the Kernel development process, anyone who wants to may
ACK a patch if they have done some level of work to confirm that
it "is good", for some measure of "good", even if that is just
applying the patch and testing it. It is re-assurance that other
people consider the patch acceptable.
Of course, if there are, say, multiple functional areas with
different maintainers, and the patch should go in via one repository
but crosses into a second or third functional area, getting the
ACK from the other maintainers may be considered essential for
its ultimate acceptance. In that regard, yes, the maintainer's
ACK carries more weight.
> So for example, a patch to git-gui could be Acked-by Shawn O.
> Pierce, or one related to pack format by Nico (I think?). So you
> should Ack it if you have done (a lot of) work in the same area as the
> patch before and if the patch looks good.
Agreed.
jdl
^ permalink raw reply
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Shawn O. Pearce @ 2008-07-25 16:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Bjjjrn Steinbrink, Junio C Hamano, Nicolas Pitre, git
In-Reply-To: <alpine.DEB.1.00.0807251513240.11976@eeepc-johanness>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Fri, 25 Jul 2008, Björn Steinbrink wrote:
> > On 2008.07.24 22:21:14 -0700, Junio C Hamano wrote:
> > > Reading get_data_from_pack(), it does rely on hdr_size, idx.offset and
> > > idx.offset of the next entry to be set correctly. The function does
> > > not seem to use type (which the patch is also setting) nor real_type
> > > (which the patch does not set).
> >
> > type is used in get_base_data().
> >
> > > However, the code checks objects[nth].real_type all over the place in
> > > the code. Doesn't the lack of real_type assignment in
> > > append_obj_to_pack() affect them in any way?
> >
> > I had thought that resolve_delta() would set that, but it seems that we
> > never call that function like that. Hm...
>
> So, let's add the comment as Nico suggested, and set real_type, too? (And
> it would be smashing if you could verify that the type is indeed correctly
> set to non-delta...)
>
> I think that setting real_type is necessary to have less surprises when
> the code is extended in the future.
The patch looks correct, but it should set real_type too because
I'm pretty sure we use that when we unpack the delta base again if
it was pruned out of memory.
--
Shawn.
^ permalink raw reply
* [PATCH] Add "install-html" rule to top level Makefile
From: Jay Soffian @ 2008-07-25 16:55 UTC (permalink / raw)
To: git; +Cc: Jay Soffian
Add "install-html" rule to top level Makefile
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index d029be3..74b6106 100644
--- a/Makefile
+++ b/Makefile
@@ -1355,6 +1355,8 @@ install-info:
quick-install-doc:
$(MAKE) -C Documentation quick-install
+install-html:
+ $(MAKE) -C Documentation install-html
### Maintainer's dist rules
--
1.6.0.rc0.155.g4fcc6
^ permalink raw reply related
* Re: git reset musings
From: Stephan Beyer @ 2008-07-25 17:04 UTC (permalink / raw)
To: sverre; +Cc: Git Mailinglist
In-Reply-To: <bd6139dc0807241151p177bb8eey6ff0fbd0a5d9008@mail.gmail.com>
Hi,
Sverre Rabbelier wrote:
> Heya,
>
> After doing "git reset" you always get a whole bunch of lines saying
> "foo: locally modified". Now I have a "OMG?!" reaction to that every
> so often, where for a brief moment I think something went wrong. A bit
> silly surely, but I suspect that some other users (especially those
> new to git) have had similar reactions. Maybe it would be worth
> letting the user know what's going on? E.g., before suddenly spitting
> out an un-asked-for status report, let the user know that a status
> report is following? Why not just do a 'git status' instead of this
> we-hacked-up-a-quick-status-listing thing?
This is no "quick status listing hack", it is a
"We've just refreshed the index regarding this entry, so we
should output that this file in the working tree is different
from the index now."
So you would use the REFRESH_QUIET flag and invoke git-status at the
end of git-reset?
Hmmm, I don't know if this is good.
Isn't typing "git status" the standard reaction to the "OMG?! What's
going here?" feeling?
And after you've first experienced this, it's something you know. There
will not be a second "OMG?!" :)
And "locally modified" seems to be less ambiguous than "needs update".
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: [PATCH] Set TAR in t/Makefile and in t4116-apply-reverse.sh
From: Miklos Vajna @ 2008-07-25 17:05 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Brandon Casey, git, Junio C Hamano
In-Reply-To: <1217003860-10609-1-git-send-email-s-beyer@gmx.net>
[-- Attachment #1: Type: text/plain, Size: 259 bytes --]
On Fri, Jul 25, 2008 at 06:37:40PM +0200, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hence, the test passes also when you run "make" in t/
> or when you invoke t4116-apply-reverse.sh directly,
> without $TAR being set.
Thanks, I just hit this issue today. ;-)
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Set TAR in t/Makefile and in t4116-apply-reverse.sh
From: Stephan Beyer @ 2008-07-25 17:12 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Brandon Casey, git, Junio C Hamano
In-Reply-To: <20080725170522.GG32057@genesis.frugalware.org>
[-- Attachment #1: Type: text/plain, Size: 507 bytes --]
Hi,
Miklos Vajna wrote:
> On Fri, Jul 25, 2008 at 06:37:40PM +0200, Stephan Beyer <s-beyer@gmx.net> wrote:
> > Hence, the test passes also when you run "make" in t/
> > or when you invoke t4116-apply-reverse.sh directly,
> > without $TAR being set.
>
> Thanks, I just hit this issue today. ;-)
Puh, I'm glad that you're not writing
"I've hit this issue yesterday and already sent a patch to this list" :)
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] index-pack: correctly initialize appended objects
From: Björn Steinbrink @ 2008-07-25 17:13 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Nicolas Pitre, spearce, git
In-Reply-To: <alpine.DEB.1.00.0807251513240.11976@eeepc-johanness>
When index-pack completes a thin pack it appends objects to the pack.
Since the commit 92392b4(index-pack: Honor core.deltaBaseCacheLimit when
resolving deltas) such an object can be pruned in case of memory pressure.
To be able to re-read the object later, a few more fields have to be set.
Noticed by Pierre Habouzit.
Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Acked-by: Nicolas Pitre <nico@cam.org>
---
On 2008.07.25 15:15:48 +0200, Johannes Schindelin wrote:
> So, let's add the comment as Nico suggested, and set real_type,
> too?
OK, I hope the comment is what was expected. My lack of knowledge
made we wonder what to write... :-/
> (And it would be smashing if you could verify that the type is
> indeed correctly set to non-delta...)
Hm, we get the object via read_sha1_file, can that return a delta? I
would not expect it to. Sorry, never looked at those code paths
(and don't have the time to investigate at the moment).
index-pack.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/index-pack.c b/index-pack.c
index ac20a46..d757b07 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -699,6 +699,12 @@ static struct object_entry *append_obj_to_pack(
write_or_die(output_fd, header, n);
obj[0].idx.crc32 = crc32(0, Z_NULL, 0);
obj[0].idx.crc32 = crc32(obj[0].idx.crc32, header, n);
+ // This object comes from outside the thin pack, so we need to
+ // initialize the size and type fields
+ obj[0].hdr_size = n;
+ obj[0].size = size;
+ obj[0].type = type;
+ obj[0].real_type = type;
obj[1].idx.offset = obj[0].idx.offset + n;
obj[1].idx.offset += write_compressed(output_fd, buf, size, &obj[0].idx.crc32);
hashcpy(obj->idx.sha1, sha1);
--
1.6.0.rc0.14.g95f8.dirty
^ permalink raw reply related
* Re: [PATCH 2/2] editor.c: Libify launch_editor()
From: Stephan Beyer @ 2008-07-25 17:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <1217003322-10291-2-git-send-email-s-beyer@gmx.net>
Hi,
Stephan Beyer wrote:
> This patch removes exit()/die() calls and builtin-specific messages
> from launch_editor(), so that it can be used as a general libgit.a
> function to launch an editor.
>
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
> ---
> builtin-commit.c | 6 +++++-
> builtin-tag.c | 6 +++++-
> editor.c | 24 ++++++++++++------------
> foo | 1 +
Ouch! Please exclude "foo"...
Sorry, for this maintainer-unfriendly patch again. *embarrassed*
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Shawn O. Pearce @ 2008-07-25 17:20 UTC (permalink / raw)
To: Bjjjrn Steinbrink; +Cc: Johannes Schindelin, Junio C Hamano, Nicolas Pitre, git
In-Reply-To: <20080725171315.GA27285@atjola.homenet>
Bjjjrn Steinbrink <B.Steinbrink@gmx.de> wrote:
> When index-pack completes a thin pack it appends objects to the pack.
> Since the commit 92392b4(index-pack: Honor core.deltaBaseCacheLimit when
> resolving deltas) such an object can be pruned in case of memory pressure.
>
> To be able to re-read the object later, a few more fields have to be set.
>
> Noticed by Pierre Habouzit.
>
> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
> Acked-by: Nicolas Pitre <nico@cam.org>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
> On 2008.07.25 15:15:48 +0200, Johannes Schindelin wrote:
> > So, let's add the comment as Nico suggested, and set real_type,
> > too?
>
> OK, I hope the comment is what was expected. My lack of knowledge
> made we wonder what to write... :-/
The commit message makes sense to me. :)
> > (And it would be smashing if you could verify that the type is
> > indeed correctly set to non-delta...)
>
> Hm, we get the object via read_sha1_file, can that return a delta? I
> would not expect it to. Sorry, never looked at those code paths
> (and don't have the time to investigate at the moment).
read_sha1_file() _never_ returns a delta. It always reutrns the
whole object, even if the object was stored as a delta in a pack
somewhere. The function is widely used within git to read an object
for processing, without the caller needing to worry about the types
of compression used to store the object.
> diff --git a/index-pack.c b/index-pack.c
> index ac20a46..d757b07 100644
> --- a/index-pack.c
> +++ b/index-pack.c
> @@ -699,6 +699,12 @@ static struct object_entry *append_obj_to_pack(
> write_or_die(output_fd, header, n);
> obj[0].idx.crc32 = crc32(0, Z_NULL, 0);
> obj[0].idx.crc32 = crc32(obj[0].idx.crc32, header, n);
> + // This object comes from outside the thin pack, so we need to
> + // initialize the size and type fields
> + obj[0].hdr_size = n;
> + obj[0].size = size;
> + obj[0].type = type;
> + obj[0].real_type = type;
> obj[1].idx.offset = obj[0].idx.offset + n;
> obj[1].idx.offset += write_compressed(output_fd, buf, size, &obj[0].idx.crc32);
> hashcpy(obj->idx.sha1, sha1);
--
Shawn.
^ permalink raw reply
* Re: Mailing lists, was Re: [RFC] Git User's Survey 2008
From: Shawn O. Pearce @ 2008-07-25 17:23 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Marek Zawirski, Junio C Hamano, Jakub Narebski, git,
Stephan Beyer
In-Reply-To: <alpine.DEB.1.00.0807241308260.8986@racer>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Thu, 24 Jul 2008, Marek Zawirski wrote:
>
> > Junio C Hamano wrote:
>
> > > I am not sure how and where, but I think j/egit should also be
> > > mentioned and/or asked about.
> >
> > There is no separate mailing list for j/egit, we just used private mails
> > for some discussions/less important notifications.
>
> I hope not for too much, because this is one of the lessons of last year's
> GSoC (and to a large degree this year's Gitorrent project): if you keep
> the project too secret, nobody will know, and as a consequence nobody will
> care.
We've done most patch review discussions right here on git@vger,
but yea, some stuff happens in private. I think what happens in
private this year on egit is really just the standard mentor-student
working relationship prior to posting patches for discussion.
--
Shawn.
^ permalink raw reply
* git-scm.com
From: Scott Chacon @ 2008-07-25 17:35 UTC (permalink / raw)
To: git
Hey all,
A followup on the post I did a few days ago about Git documentation.
I forked Petr's git.or.cz site and put up a version that I think is a
bit more accessible and newbie-friendly at git-scm.com. I had meant
to discuss this with Petr before posting it to you all, but I
published a blog post that got a bit more attention than I expected,
and I didn't want you all to think I didn't care about your opinion,
as some have already accused me of.
Anyhow, I'm discussing with Petr about where we want to go from here -
what changes he'd like to make, etc, but I obviously value your
opinion as well, so please let me know what you think. The content
has barely changed, it's really just a usability overhaul. I want to
make sure that whatever someone is looking for (especially someone
new), they can find in a few clicks and a few seconds.
Next, I will be working on the larger end-user documentation project,
which will linked to from the documentation page of this site, and
probably the main page too. I'll keep this list updated as I go,
since people tend to think I don't care about the community when I try
not to waste your time. :)
Scott
^ permalink raw reply
* [PATCH] Propagate -u/--upload-pack option of "git clone" to transport.
From: Steve Haslam @ 2008-07-25 17:51 UTC (permalink / raw)
To: git; +Cc: Steve Haslam
The -u option to override the remote system's path to git-upload-pack was
being ignored by "git clone"; caused by a missing call to
transport_set_option to set TRANS_OPT_UPLOADPACK. Presumably this crept in
when git-clone was converted from shell to C.
Signed-off-by: Steve Haslam <shaslam@lastminute.com>
---
builtin-clone.c | 4 ++++
t/t5602-clone-remote-exec.sh | 26 ++++++++++++++++++++++++++
2 files changed, 30 insertions(+), 0 deletions(-)
create mode 100755 t/t5602-clone-remote-exec.sh
diff --git a/builtin-clone.c b/builtin-clone.c
index 3522245..e086a40 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -480,6 +480,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (option_quiet)
transport->verbose = -1;
+ if (option_upload_pack)
+ transport_set_option(transport, TRANS_OPT_UPLOADPACK,
+ option_upload_pack);
+
refs = transport_get_remote_refs(transport);
transport_fetch_refs(transport, refs);
}
diff --git a/t/t5602-clone-remote-exec.sh b/t/t5602-clone-remote-exec.sh
new file mode 100755
index 0000000..8367a68
--- /dev/null
+++ b/t/t5602-clone-remote-exec.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description=clone
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ echo "#!/bin/sh" > not_ssh
+ echo "echo \"\$*\" > not_ssh_output" >> not_ssh
+ echo "exit 1" >> not_ssh
+ chmod +x not_ssh
+'
+
+test_expect_success 'clone calls git-upload-pack unqualified with no -u option' '
+ GIT_SSH=./not_ssh git clone localhost:/path/to/repo junk
+ echo "localhost git-upload-pack '\''/path/to/repo'\''" >expected
+ test_cmp expected not_ssh_output
+'
+
+test_expect_success 'clone calls specified git-upload-pack with -u option' '
+ GIT_SSH=./not_ssh git clone -u /something/bin/git-upload-pack localhost:/path/to/repo junk
+ echo "localhost /something/bin/git-upload-pack '\''/path/to/repo'\''" >expected
+ test_cmp expected not_ssh_output
+'
+
+test_done
--
1.6.0.rc0.43.g1cd6
^ permalink raw reply related
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Junio C Hamano @ 2008-07-25 18:15 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Johannes Schindelin, spearce, git, Björn Steinbrink
In-Reply-To: <alpine.LFD.1.10.0807250751220.9968@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> On Fri, 25 Jul 2008, Johannes Schindelin wrote:
>
>> Hi,
>>
>> On Thu, 24 Jul 2008, Junio C Hamano wrote:
>>
>> > The function does not seem to use type (which the patch is also setting)
>> > nor real_type (which the patch does not set).
>> >
>> > However, the code checks objects[nth].real_type all over the place in
>> > the code. Doesn't the lack of real_type assignment in
>> > append_obj_to_pack() affect them in any way?
>>
>> >From staring at the code, I thought that real_type was set in
>> resolve_delta(), but I may be wrong.
>>
>> The safer thing would be to set it, but I am not quite sure if we can use
>> "type" directly, or if type can be "delta" for an object that is used to
>> complete the pack, and therefore stored as a non-delta.
>
> Objects to complete the pack are always non delta, so the type and
> real_type should be the same. However that shouldn't matter since at
> that point the object array is not walked anymore, at least not for
> appended objects, and therefore initializing the type at that point is
> redundant.
Thanks. Here is what I committed.
commit 72de2883bd7d4ceda05f107826c7607c594de965
Author: Björn Steinbrink <B.Steinbrink@gmx.de>
Date: Thu Jul 24 18:32:00 2008 +0100
index-pack.c: correctly initialize appended objects
When index-pack completes a thin pack it appends objects to the pack.
Since the commit 92392b4(index-pack: Honor core.deltaBaseCacheLimit when
resolving deltas) such an object can be pruned in case of memory
pressure, and will be read back again by get_data_from_pack(). For this
to work, the fields in object_entry structure need to be initialized
properly.
Noticed by Pierre Habouzit.
Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Acked-by: Nicolas Pitre <nico@cam.org>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/index-pack.c b/index-pack.c
index c359f8c..7d5344a 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -698,6 +698,10 @@ static struct object_entry *append_obj_to_pack(
write_or_die(output_fd, header, n);
obj[0].idx.crc32 = crc32(0, Z_NULL, 0);
obj[0].idx.crc32 = crc32(obj[0].idx.crc32, header, n);
+ obj[0].size = size;
+ obj[0].hdr_size = n;
+ obj[0].type = type;
+ obj[0].real_type = type;
obj[1].idx.offset = obj[0].idx.offset + n;
obj[1].idx.offset += write_compressed(output_fd, buf, size, &obj[0].idx.crc32);
hashcpy(obj->idx.sha1, sha1);
^ permalink raw reply related
* Re: [PATCH] Set TAR in t/Makefile and in t4116-apply-reverse.sh
From: Junio C Hamano @ 2008-07-25 18:18 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Brandon Casey, git
In-Reply-To: <1217003860-10609-1-git-send-email-s-beyer@gmx.net>
Actually, 455a7f3 (More portability., 2005-09-30) introduced $TAR and it
is also used in t5000. cb34882 (fix t5000-tar-tree.sh when $TAR isn't
set, 2005-11-08) did the same fix you are adding, but I think both of
these fixes are in a wrong place.
I think we should do this instead. That's how SHELL_PATH is passed around
from build to all the test scripts already.
---
Makefile | 1 +
t/t5000-tar-tree.sh | 1 -
2 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index b003e3e..1d14209 100644
--- a/Makefile
+++ b/Makefile
@@ -1212,6 +1212,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
@echo SHELL_PATH=\''$(SHELL_PATH_SQ)'\' >$@
+ @echo TAR=\''$(subst ','\'',$(TAR))'\' >>$@
### Detect Tck/Tk interpreter path changes
ifndef NO_TCLTK
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 9b0baac..5eb119e 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -25,7 +25,6 @@ commit id embedding:
'
. ./test-lib.sh
-TAR=${TAR:-tar}
UNZIP=${UNZIP:-unzip}
SUBSTFORMAT=%H%n
^ permalink raw reply related
* [PATCH] git-am: Mention --abort in usage string part of OPTIONS_SPEC
From: Stephan Beyer @ 2008-07-25 18:22 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stephan Beyer
The three separate lines for --skip, --resolved and --abort
are merged into one so that it is easy to see that they're
alternative and related options.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
I could've sworn this had already been in fcab40a38 (git am --abort)
but it wasn't. So here it is.
Regards.
git-am.sh | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index f4abd9d..6aa8192 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -6,8 +6,7 @@ SUBDIRECTORY_OK=Yes
OPTIONS_KEEPDASHDASH=
OPTIONS_SPEC="\
git am [options] [<mbox>|<Maildir>...]
-git am [options] --resolved
-git am [options] --skip
+git am [options] (--resolved | --skip | --abort)
--
d,dotest= (removed -- do not use)
i,interactive run interactively
--
1.6.0.rc0.106.g0f5bf
^ permalink raw reply related
* Re: [PATCH] Set TAR in t/Makefile and in t4116-apply-reverse.sh
From: Stephan Beyer @ 2008-07-25 18:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Casey, git
In-Reply-To: <7vvdytsu7n.fsf@gitster.siamese.dyndns.org>
Hi,
> diff --git a/Makefile b/Makefile
> index b003e3e..1d14209 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1212,6 +1212,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
>
> GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
> @echo SHELL_PATH=\''$(SHELL_PATH_SQ)'\' >$@
> + @echo TAR=\''$(subst ','\'',$(TAR))'\' >>$@
>
> ### Detect Tck/Tk interpreter path changes
> ifndef NO_TCLTK
But then TAR has to be set in test-lib.sh also, to be able to
invoke t5000 and t4116 directly, hasn't it?
Regards.
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* [PATCH] Remove references to git-fetch-pack from "git clone" documentation.
From: Steve Haslam @ 2008-07-25 18:37 UTC (permalink / raw)
To: git; +Cc: Steve Haslam
"git clone" no longer calls "git-fetch-pack", so the documentation is a bit
stale. Instead, state that the -u option is to be used when accessing a
repository over ssh.
Signed-off-by: Steve Haslam <shaslam@lastminute.com>
---
This is somewhat related to a patch I just sent; typically I forgot
about the documentation until after sending it.
Documentation/git-clone.txt | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 91efac9..26fd1b1 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -87,8 +87,8 @@ then the cloned repository will become corrupt.
--quiet::
-q::
- Operate quietly. This flag is passed to "rsync" and
- 'git-fetch-pack' commands when given.
+ Operate quietly. This flag is also passed to the `rsync'
+ command when given.
--no-checkout::
-n::
@@ -113,9 +113,8 @@ then the cloned repository will become corrupt.
--upload-pack <upload-pack>::
-u <upload-pack>::
- When given, and the repository to clone from is handled
- by 'git-fetch-pack', `--exec=<upload-pack>` is passed to
- the command to specify non-default path for the command
+ When given, and the repository to clone from is accessed
+ via ssh, this specifies a non-default path for the command
run on the other end.
--template=<template_directory>::
--
1.6.0.rc0.43.g1cd6
^ permalink raw reply related
* Re: [PATCH] Avoid warning when From: is encoded
From: Jeff King @ 2008-07-25 18:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Peter Valdemar Mørch, git
In-Reply-To: <7vhcadudmz.fsf@gitster.siamese.dyndns.org>
On Fri, Jul 25, 2008 at 09:33:56AM -0700, Junio C Hamano wrote:
> > In commit 0706bd19ef9b41e7519df2c73796ef93484272fd $1 is used from a regexp
> > without using () to set up $1. Later, when that value was used, it caused a
> > warning about a variable being undefined.
> >
> > Signed-off-by: Peter Valdemar Mørch <peter@morch.com>
>
> Thanks. The patch is obviously correct; I think you are fixing 8291db6
> (git-send-email: add charset header if we add encoded 'From', 2007-11-16),
> not 0706bd1 (send-email: specify content-type of --compose body,
> 2008-03-28).
>
> Will apply to maint and merge upwards.
Discussions about who can ACK this code aside, the original bogosity was
totally mine, so
Acked-by: Jeff King <peff@peff.net>
-Peff
^ permalink raw reply
* Re: Mailing lists, was Re: [RFC] Git User's Survey 2008
From: Junio C Hamano @ 2008-07-25 18:49 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Johannes Schindelin, Marek Zawirski, Jakub Narebski, git,
Stephan Beyer
In-Reply-To: <20080725172313.GE21117@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> On Thu, 24 Jul 2008, Marek Zawirski wrote:
>>
>> > Junio C Hamano wrote:
>>
>> > > I am not sure how and where, but I think j/egit should also be
>> > > mentioned and/or asked about.
>> >
>> > There is no separate mailing list for j/egit, we just used private mails
>> > for some discussions/less important notifications.
>>
>> I hope not for too much, because this is one of the lessons of last year's
>> GSoC (and to a large degree this year's Gitorrent project): if you keep
>> the project too secret, nobody will know, and as a consequence nobody will
>> care.
>
> We've done most patch review discussions right here on git@vger,
> but yea, some stuff happens in private. I think what happens in
> private this year on egit is really just the standard mentor-student
> working relationship prior to posting patches for discussion.
Sorry for causing confusion in the discussion, but I was not talking about
"mailing list on e/jgit".
What I meant was that we might want to have a few more questions about
e/jgit as an independent entity in the survey, as it is a completely
different re-implementation of the same git theme.
^ permalink raw reply
* Re: [PATCH] Set TAR in t/Makefile and in t4116-apply-reverse.sh
From: Junio C Hamano @ 2008-07-25 18:54 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Brandon Casey, git
In-Reply-To: <20080725182416.GG27172@leksak.fem-net>
Stephan Beyer <s-beyer@gmx.net> writes:
> Hi,
>
>> diff --git a/Makefile b/Makefile
>> index b003e3e..1d14209 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1212,6 +1212,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
>>
>> GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
>> @echo SHELL_PATH=\''$(SHELL_PATH_SQ)'\' >$@
>> + @echo TAR=\''$(subst ','\'',$(TAR))'\' >>$@
>>
>> ### Detect Tck/Tk interpreter path changes
>> ifndef NO_TCLTK
>
> But then TAR has to be set in test-lib.sh also, to be able to
> invoke t5000 and t4116 directly, hasn't it?
Dosen't test-lib source GIT-BUILD-OPTIONS?
^ permalink raw reply
* Re: [PATCH] Set TAR in t/Makefile and in t4116-apply-reverse.sh
From: Stephan Beyer @ 2008-07-25 18:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Casey, git
In-Reply-To: <7vmyk5sska.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Stephan Beyer <s-beyer@gmx.net> writes:
> >> diff --git a/Makefile b/Makefile
> >> index b003e3e..1d14209 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -1212,6 +1212,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
> >>
> >> GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
> >> @echo SHELL_PATH=\''$(SHELL_PATH_SQ)'\' >$@
> >> + @echo TAR=\''$(subst ','\'',$(TAR))'\' >>$@
> >>
> >> ### Detect Tck/Tk interpreter path changes
> >> ifndef NO_TCLTK
> >
> > But then TAR has to be set in test-lib.sh also, to be able to
> > invoke t5000 and t4116 directly, hasn't it?
>
> Dosen't test-lib source GIT-BUILD-OPTIONS?
It does. Great, then.
Regards.
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ 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