* Re: What's cooking in git.git (topics)
From: Sverre Rabbelier @ 2008-07-20 22:58 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, Johannes Schindelin, Nanako Shiraishi, git
In-Reply-To: <20080720203306.GO10347@genesis.frugalware.org>
On Sun, Jul 20, 2008 at 10:33 PM, Miklos Vajna <vmiklos@frugalware.org> wrote:
> On Sun, Jul 20, 2008 at 10:03:24PM +0200, Sverre Rabbelier <alturin@gmail.com> wrote:
>> Whatever happened to quotes?
>>
>> $ git merge -s subtree -Xpath="git-gui git-gui/master"
>
> Read again what did you wrote. ;-)
>
> The current form is
>
> git merge -s subtree git-gui/master, so at most it could be
>
> $ git merge -s subtree -Xpath="git-gui" git-gui/master
Meh, what I ofcourse mean was:
$ git merge -s subtree -X"path=git-gui" git-gui/master
But that looks rather awkward, which is probably why I typed it the
way I did? Maybe something like....
$ git merge -s subtree -X(--path=git-gui --foo=bar) git-gui/master
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Junio C Hamano @ 2008-07-20 23:04 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20080720224040.GG32184@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> Do you think this would create serious problems?
>
> One thing this patch series depends on now is changing the git mv
> semantics in a rather non-trivial way, which is something we might want
> to do in a major release instead of within the 1.6 series.
The change to git-mv perhaps is necessary to happen between a major
release boundary. I do not know if the current round of patch to do so
will become ready in time for 1.6.0. The rename-ce-at patch I looked at
did not look like it was.
^ permalink raw reply
* Re: [PATCH] git-shell needs $(COMPAT_OBJS)
From: Miklos Vajna @ 2008-07-20 23:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <7vprp82nb6.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 412 bytes --]
On Sun, Jul 20, 2008 at 03:38:53PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> That's a rather misguided approach, isn't it?
>
> After all, the work requested by the end user will be handled by code in
> the main git executable by spawning a subprocess, and you are auditing the
> codepath that leads to the spawning anyway.
Hm, but then what's the purpose of having git-shell as a not-builtin?
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Petr Baudis @ 2008-07-20 23:08 UTC (permalink / raw)
To: Steve Frécinaux; +Cc: git
In-Reply-To: <1216592735-23789-1-git-send-email-code@istique.net>
On Mon, Jul 21, 2008 at 12:25:35AM +0200, Steve Frécinaux wrote:
> The notice covers this behaviour:
> if you are in the git/ subdirectory of your repository, it will pick
> the tree corresponding to that directory instead of the root one if you
> specify the root tree object id.
>
> Compare the output of both of those commands:
> git-ls-tree cb44e6571708aa2792c73a289d87586fe3c0c362
> git-cat-file -p cb44e6571708aa2792c73a289d87586fe3c0c362
> ---
> Documentation/git-ls-tree.txt | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt
> index 1cdec22..7cba394 100644
> --- a/Documentation/git-ls-tree.txt
> +++ b/Documentation/git-ls-tree.txt
> @@ -21,6 +21,10 @@ though - 'paths' denote just a list of patterns to match, e.g. so specifying
> directory name (without '-r') will behave differently, and order of the
> arguments does not matter.
>
> +Note that if you give ls-tree the sha1 id of a parent of the tree
> +corresponding to the directory you're in, it will resolve that tree and list
> +its contents instead of listing the contents of the tree you gave.
> +
> OPTIONS
> -------
> <tree-ish>::
It's hard to make out what do you mean, the patch description is much
clearer, paradoxically. Also, this in fact holds for the root tree
instead of the parent tree, and the behaviour changes from "weird" to
"simply broken" when you try to list a tree object that is _not_ the
root project tree from within a subdirectory:
git$ git ls-tree HEAD Documentation
040000 tree 066c25e86a44d4c7bde2d3e9b91e6891d752efa1 Documentation
git/Documentation$ git ls-tree 066c25e86a44d4c7bde2d3e9b91e6891d752efa1
git/Documentation$
I think that ls-tree simply shouldn't auto-fill its pathspec based on
current prefix in case no pathspec was supplied. Patch to follow.
--
Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name. -- Ken Thompson and Dennis M. Ritchie
^ permalink raw reply
* Re: [PATCH] git-shell needs $(COMPAT_OBJS)
From: Junio C Hamano @ 2008-07-20 23:12 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <7vhcak2mip.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Perhaps we should apply a variant of your patch to allow linking from
> compat/ routines to git-shell, so that people affected by the compat layer
> functions that call outside compat layer have incentive to fix them.
So the previous one will be queued in 'pu' as...
Link shell with compat layer functions
This in the short term will break on platforms that use compat implemenations
that call outside compat layer, but that is exactly what we want. To give
incentive to fix things for people who are affected and more importantly have
environment to test their fixes.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
and here is an example to fix pread for you, even I do not need it,
though.
-- >8 --
Move read_in_full() and write_in_full() to wrapper.c
A few compat/* layer functions call these functions, but we would really
want to keep them thin, without depending too much on the libgit proper.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
wrapper.c | 38 ++++++++++++++++++++++++++++++++++++++
write_or_die.c | 38 --------------------------------------
2 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/wrapper.c b/wrapper.c
index 4e04f76..93562f0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -133,6 +133,44 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
}
}
+ssize_t read_in_full(int fd, void *buf, size_t count)
+{
+ char *p = buf;
+ ssize_t total = 0;
+
+ while (count > 0) {
+ ssize_t loaded = xread(fd, p, count);
+ if (loaded <= 0)
+ return total ? total : loaded;
+ count -= loaded;
+ p += loaded;
+ total += loaded;
+ }
+
+ return total;
+}
+
+ssize_t write_in_full(int fd, const void *buf, size_t count)
+{
+ const char *p = buf;
+ ssize_t total = 0;
+
+ while (count > 0) {
+ ssize_t written = xwrite(fd, p, count);
+ if (written < 0)
+ return -1;
+ if (!written) {
+ errno = ENOSPC;
+ return -1;
+ }
+ count -= written;
+ p += written;
+ total += written;
+ }
+
+ return total;
+}
+
int xdup(int fd)
{
int ret = dup(fd);
diff --git a/write_or_die.c b/write_or_die.c
index e4c8e22..4c29255 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -45,44 +45,6 @@ void maybe_flush_or_die(FILE *f, const char *desc)
}
}
-ssize_t read_in_full(int fd, void *buf, size_t count)
-{
- char *p = buf;
- ssize_t total = 0;
-
- while (count > 0) {
- ssize_t loaded = xread(fd, p, count);
- if (loaded <= 0)
- return total ? total : loaded;
- count -= loaded;
- p += loaded;
- total += loaded;
- }
-
- return total;
-}
-
-ssize_t write_in_full(int fd, const void *buf, size_t count)
-{
- const char *p = buf;
- ssize_t total = 0;
-
- while (count > 0) {
- ssize_t written = xwrite(fd, p, count);
- if (written < 0)
- return -1;
- if (!written) {
- errno = ENOSPC;
- return -1;
- }
- count -= written;
- p += written;
- total += written;
- }
-
- return total;
-}
-
void fsync_or_die(int fd, const char *msg)
{
if (fsync(fd) < 0) {
^ permalink raw reply related
* Re: [RFC] Stopping those fat "What's cooking in git.git" threads
From: Junio C Hamano @ 2008-07-20 23:16 UTC (permalink / raw)
To: sverre; +Cc: Junio C Hamano, Miklos Vajna, git
In-Reply-To: <bd6139dc0807201550v27d6db3epd0d0b4bc663e0351@mail.gmail.com>
"Sverre Rabbelier" <alturin@gmail.com> writes:
> On Sun, Jul 20, 2008 at 11:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> I could make "What's cooking" not a follow-up to the previous issue, or
>> perhaps add "(volume 1.6.0, issue 28)" at the end of the Subject.
>
> The downside of this is that it'll be less easy to see the difference
> with the previous version.
My vague recollection is that it was Pasky who complained long time ago
when "What's in" was not a follow-up to its previous round, which led me
to switch my workflow to send them in the current form. You cannot
satisfy certain people no matter what you do.
^ permalink raw reply
* Re: [RFC] Stopping those fat "What's cooking in git.git" threads
From: Sverre Rabbelier @ 2008-07-20 23:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, git
In-Reply-To: <7v3am42lk2.fsf@gitster.siamese.dyndns.org>
On Mon, Jul 21, 2008 at 1:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> On Sun, Jul 20, 2008 at 11:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> I could make "What's cooking" not a follow-up to the previous issue, or
>>> perhaps add "(volume 1.6.0, issue 28)" at the end of the Subject.
>>
>> The downside of this is that it'll be less easy to see the difference
>> with the previous version.
>
> My vague recollection is that it was Pasky who complained long time ago
> when "What's in" was not a follow-up to its previous round, which led me
> to switch my workflow to send them in the current form. You cannot
> satisfy certain people no matter what you do.
Add an interdiff at the bottom of the mail? You can't satisfy
everybody no matter what you do, but you can come quite far, it
usually means you have to do a lot of work to do so though.
^ permalink raw reply
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Steve Frécinaux @ 2008-07-20 23:22 UTC (permalink / raw)
To: Petr Baudis; +Cc: Steve Frécinaux, git
In-Reply-To: <20080720230846.GH32184@machine.or.cz>
> It's hard to make out what do you mean, the patch description is much
> clearer, paradoxically.
It is hard to explain such a strange behaviour with a description that
is both short and generic enough... But I agree with you. I just got
bitten by this and I thought it was important enough to be specified.
> Also, this in fact holds for the root tree
> instead of the parent tree, and the behaviour changes from "weird" to
> "simply broken" when you try to list a tree object that is _not_ the
> root project tree from within a subdirectory:
>
> git$ git ls-tree HEAD Documentation
> 040000 tree 066c25e86a44d4c7bde2d3e9b91e6891d752efa1 Documentation
> git/Documentation$ git ls-tree 066c25e86a44d4c7bde2d3e9b91e6891d752efa1
> git/Documentation$
>
> I think that ls-tree simply shouldn't auto-fill its pathspec based on
> current prefix in case no pathspec was supplied. Patch to follow.
I also thought this behaviour was broken. But I didn't want to patch
it because I was afraid of breaking things that would rely on it,
despite it seems unexpected enough not to be used...
^ permalink raw reply
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Junio C Hamano @ 2008-07-20 23:24 UTC (permalink / raw)
To: Petr Baudis; +Cc: Steve Frécinaux, git
In-Reply-To: <20080720230846.GH32184@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> I think that ls-tree simply shouldn't auto-fill its pathspec based on
> current prefix in case no pathspec was supplied. Patch to follow.
Have you dug the list archive from mid-to-late December 2005 that prompted
the current behaviour (and introduction of --full-name)? I haven't. A
change to always do the --full-name can only be justified by doing so and
rehashing the issues.
On the other hand, "fix" is welcome.
^ permalink raw reply
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Petr Baudis @ 2008-07-20 23:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steve Frécinaux, git
In-Reply-To: <7vy73w16nj.fsf@gitster.siamese.dyndns.org>
On Sun, Jul 20, 2008 at 04:24:00PM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
>
> > I think that ls-tree simply shouldn't auto-fill its pathspec based on
> > current prefix in case no pathspec was supplied. Patch to follow.
>
> Have you dug the list archive from mid-to-late December 2005 that prompted
> the current behaviour (and introduction of --full-name)? I haven't. A
> change to always do the --full-name can only be justified by doing so and
> rehashing the issues.
>
> On the other hand, "fix" is welcome.
You are right, now that I understand the issue better, there's no good
fix for this except perhaps introducing --no-prefix, which is not my
itch to scratch. Here's my original wording improvement:
Note that if you are within a subdirectory of your working copy,
'git ls-tree' will automatically prepend the subdirectory prefix
to the specified paths, and assume the prefix specified in case
no paths were given - no matter what the tree object is! Thus,
within a subdirectory, 'git ls-tree' behaves as expected only
when run on a root tree object.
--
Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name. -- Ken Thompson and Dennis M. Ritchie
^ permalink raw reply
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Junio C Hamano @ 2008-07-20 23:53 UTC (permalink / raw)
To: Petr Baudis; +Cc: Steve Frécinaux, git
In-Reply-To: <7vy73w16nj.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Petr Baudis <pasky@suse.cz> writes:
>
>> I think that ls-tree simply shouldn't auto-fill its pathspec based on
>> current prefix in case no pathspec was supplied. Patch to follow.
>
> Have you dug the list archive from mid-to-late December 2005 that prompted
> the current behaviour (and introduction of --full-name)? I haven't.
Now, I did:
http://thread.gmane.org/gmane.comp.version-control.git/13028/focus=13135
I think the answer is --full-name (cf. a69dd58 (ls-tree: chomp leading
directories when run from a subdirectory, 2005-12-23)).
^ permalink raw reply
* Re: [PATCH 2/2] git-add -a: add all files
From: Jeff King @ 2008-07-20 23:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jay Soffian, Johannes Schindelin, Tarmigan, git, Michael J Gruber
In-Reply-To: <7v4p6k8l36.fsf@gitster.siamese.dyndns.org>
On Sun, Jul 20, 2008 at 11:30:21AM -0700, Junio C Hamano wrote:
> So for such people who would find "add -A" useful, "commit -a" will not be
> "unrelated changes in the same commit". And for such people, I would even
> say "commit -A" would be even more useful, too.
>
> I'll never be in that camp of perfect people myself, though..
I don't claim to be perfect, but I do use "commit -a" and I haven't ever
had a problem committing unrelated changes. My secret is to keep a good
.gitignore, and to peek at "git status" and "git diff" before
committing. So it's just a shorthand because after seeing that
everything is ready for commit, I'm too lazy to type each filename. I
also use "git add ." for the same purpose if files are to be added.
But note that avoiding "-a" doesn't save you from unrelated changes
anyway; it only saves you from changes in unrelated files. You still
have to look below the file granularity with "git diff" to avoid (for
example) a debugging printf. I often will use "git add -i" if I have a
lot of complex changes, even if I end up staging _everything_. But it
lets me say "yes, this should go in" for each hunk.
So maybe I will use "git add -A", but I have to admit to not really
having felt its lack to this point. However, something Lars said makes
me wonder: do people _really_ want this as an option to add? I seem to
recall the primary request for this being the "undisciplined" approach
you gave ("I have a project that periodically gets data dumped by an
external program, and I want to commit it all"). In that case, the next
step is always commit, so what would be most convenient is "commit -A".
But again, I haven ever felt the lack of this feature; such usage for me
always goes in scripts, where I am more than happy to write out "add .
&& add -u && commit".
-Peff
^ permalink raw reply
* [PATCH v2] Ensure that SSH runs in non-interactive mode
From: Fredrik Tolf @ 2008-07-21 0:00 UTC (permalink / raw)
To: git; +Cc: Fredrik Tolf
OpenSSH has the nice feature that it sets the IP TOS value of its
connection depending on usage. When used in interactive mode, it
is set to Minimize-Delay, and other wise to Maximize-Throughput. Its
usage by Git is best served by Maximize-Throughput, for obvious
reasons.
However, it seems to use a DWIM heuristic for detecting interactive
mode. The current implementation enters interactive mode if either
a PTY is allocated or X11 forwarding is enabled, and even though Git
SSH:ing does not allocate a PTY, X11 forwarding is often turned on
by default.
This patch allows the Git config file to specify the SSH command to
use and its parameters in a rather flexible manner. It should also be
enough to configure Git to use other SSH implementations than OpenSSH.
Signed-off-by: Fredrik Tolf <fredrik@dolda2000.com>
---
I'm following my previous SSH patch up with this one, which should at
least solve the problems discussed, and probably some more. If anything,
it might be considered a bit overkill for the problem at hand.
I assume it might have to be documented as well, if people approve of it.
connect.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 117 insertions(+), 13 deletions(-)
diff --git a/connect.c b/connect.c
index 574f42f..46379fa 100644
--- a/connect.c
+++ b/connect.c
@@ -474,6 +474,114 @@ char *get_port(char *host)
return NULL;
}
+static char *git_ssh_command;
+static struct {
+ char *name;
+ char *exp;
+} ssh_templates[] = {
+ {"openssh", "ssh -xT %P-p %p %h"},
+ {"plink", "plink %P-P %p %h"},
+ {NULL, NULL}
+};
+
+static int git_ssh_command_options(const char *var, const char *value, void *cb)
+{
+ int i;
+
+ if(!strcmp(var, "core.sshcommand")) {
+ if(git_ssh_command)
+ return 0;
+ if(!value)
+ return config_error_nonbool(var);
+ if(strchr(value, ' ')) {
+ git_ssh_command = xstrdup(value);
+ } else {
+ for(i = 0; ssh_templates[i].name; i++) {
+ if(!strcmp(ssh_templates[i].name, value)) {
+ git_ssh_command = xstrdup(ssh_templates[i].exp);
+ break;
+ }
+ }
+ if(git_ssh_command == NULL)
+ git_ssh_command = xstrdup(value);
+ }
+ }
+
+ return git_default_config(var, value, cb);
+}
+
+static char *ssh_arg_subst(char *arg, const char *host, const char *port)
+{
+ if(!strncmp(arg, "%P", 2)) {
+ if(!port)
+ return NULL;
+ return xstrdup(arg + 2);
+ } else if(!strcmp(arg, "%p")) {
+ if(!port)
+ return NULL;
+ return xstrdup(port);
+ } else if(!strcmp(arg, "%h")) {
+ return xstrdup(host);
+ }
+ return arg;
+}
+
+static const char **setup_ssh_command(const char *host, const char *port, int extra_args)
+{
+ char **argv;
+ char *tok, *buf;
+ int i, sz;
+
+ if((buf = getenv("GIT_SSH")) != NULL) {
+ if(port) {
+ argv = xcalloc(5 + extra_args, sizeof(*argv));
+ argv[0] = xstrdup(buf);
+ argv[1] = xstrdup("-p");
+ argv[2] = xstrdup(port);
+ argv[3] = xstrdup(host);
+ } else {
+ argv = xcalloc(3 + extra_args, sizeof(*argv));
+ argv[0] = xstrdup(buf);
+ argv[1] = xstrdup(host);
+ }
+ return (const char **)argv;
+ }
+
+ git_config(git_ssh_command_options, NULL);
+ if(git_ssh_command == NULL)
+ buf = xstrdup(ssh_templates[0].exp);
+ else
+ buf = xstrdup(git_ssh_command);
+
+ argv = xmalloc((sz = 5) * sizeof(*argv));
+ for(i = 0, tok = strtok(buf, " "); tok != NULL; tok = strtok(NULL, " ")) {
+ argv[i++] = xstrdup(tok);
+ if(sz - i < 1)
+ argv = xrealloc(argv, (sz += 5) * sizeof(*argv));
+ }
+ argv[i] = NULL;
+ argv = xrealloc(argv, ((sz = i) + extra_args + 1) * sizeof(*argv));
+ free(buf);
+
+ for(i = 0; i < sz;) {
+ buf = ssh_arg_subst(argv[i], host, port);
+ if(buf == argv[i]) {
+ i++;
+ continue;
+ } else if(buf == NULL) {
+ free(argv[i]);
+ memmove(argv + i, argv + i + 1, sizeof(*argv) * (sz-- - i));
+ continue;
+ } else {
+ free(argv[i]);
+ argv[i] = buf;
+ i++;
+ }
+ }
+
+ return (const char **)argv;
+}
+
static struct child_process no_fork;
/*
@@ -596,19 +704,12 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
die("command line too long");
conn->in = conn->out = -1;
- conn->argv = arg = xcalloc(6, sizeof(*arg));
if (protocol == PROTO_SSH) {
- const char *ssh = getenv("GIT_SSH");
- if (!ssh) ssh = "ssh";
-
- *arg++ = ssh;
- if (port) {
- *arg++ = "-p";
- *arg++ = port;
- }
- *arg++ = host;
+ conn->argv = setup_ssh_command(host, port, 1);
+ for(arg = conn->argv; *arg; arg++);
}
else {
+ conn->argv = arg = xcalloc(6, sizeof(*arg));
/* remove these from the environment */
const char *env[] = {
ALTERNATE_DB_ENVIRONMENT,
@@ -620,10 +721,10 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
NULL
};
conn->env = env;
- *arg++ = "sh";
- *arg++ = "-c";
+ *arg++ = xstrdup("sh");
+ *arg++ = xstrdup("-c");
}
- *arg++ = cmd.buf;
+ *arg++ = xstrdup(cmd.buf);
*arg = NULL;
if (start_command(conn))
@@ -640,11 +741,14 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
int finish_connect(struct child_process *conn)
{
+ const char **argp;
int code;
if (!conn || conn == &no_fork)
return 0;
code = finish_command(conn);
+ for(argp = conn->argv; *argp; argp++)
+ free((void *)*argp);
free(conn->argv);
free(conn);
return code;
--
1.5.6.2
^ permalink raw reply related
* Re: [PATCH 2/2] git-add -a: add all files
From: Junio C Hamano @ 2008-07-21 0:06 UTC (permalink / raw)
To: Jeff King
Cc: Jay Soffian, Johannes Schindelin, Tarmigan, git, Michael J Gruber
In-Reply-To: <20080720235933.GA12454@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> But again, I haven ever felt the lack of this feature; such usage for me
> always goes in scripts, where I am more than happy to write out "add .
> && add -u && commit".
The reason we did not have such "feature" so far was not because somebody
high in the git foodchain was opposed to the idea, but simply because
nobody came up with a usable patch to do so.
I do not have anything fundamentally against "add -A" nor "commit -A". To
me, this is in "perhaps nice to have for some people, but I would not use
it myself and I wouldn't bother" category, not in "I'm opposed -- it would
promote bad workflow" cateogry.
^ permalink raw reply
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Petr Baudis @ 2008-07-21 0:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steve Frécinaux, git
In-Reply-To: <7vtzek15b5.fsf@gitster.siamese.dyndns.org>
On Sun, Jul 20, 2008 at 04:53:02PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Petr Baudis <pasky@suse.cz> writes:
> >
> >> I think that ls-tree simply shouldn't auto-fill its pathspec based on
> >> current prefix in case no pathspec was supplied. Patch to follow.
> >
> > Have you dug the list archive from mid-to-late December 2005 that prompted
> > the current behaviour (and introduction of --full-name)? I haven't.
>
> Now, I did:
>
> http://thread.gmane.org/gmane.comp.version-control.git/13028/focus=13135
>
> I think the answer is --full-name (cf. a69dd58 (ls-tree: chomp leading
> directories when run from a subdirectory, 2005-12-23)).
I don't understand your point now. --full-name cares only about the
displaying part; do you suggest that it should be extended to also turn
off prepending the prefix during the filtering phase? That would make a
lot of sense, if you are not worried about compatibility trouble.
--
Petr "Pasky, missing something" Baudis
As in certain cults it is possible to kill a process if you know
its true name. -- Ken Thompson and Dennis M. Ritchie
^ permalink raw reply
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Junio C Hamano @ 2008-07-21 0:14 UTC (permalink / raw)
To: Petr Baudis; +Cc: Steve Frécinaux, git
In-Reply-To: <20080721000824.GI10151@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> I don't understand your point now. --full-name cares only about the
> displaying part; do you suggest that it should be extended to also turn
> off prepending the prefix...
Ah, sorry, I thought you were talking about the display part.
I never thought you would think "showing relative to tree-root" is even an
option. That would make it inconsistent with not just the established
semantics of what the plumbing did, but also with what ls-files does.
^ permalink raw reply
* Re: [PATCH] Ensure that SSH runs in non-interactive mode
From: Jeff King @ 2008-07-21 0:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Fredrik Tolf, Keith Packard, git,
Edward Z. Yang, Steffen Prohaska
In-Reply-To: <7v63r0bejy.fsf@gitster.siamese.dyndns.org>
On Sun, Jul 20, 2008 at 11:23:13AM -0700, Junio C Hamano wrote:
> I think that is a very sensible approach, but just like we have a few
> "built-in" function-header regexps with customization possibilities for
> the user, we might want to:
>
> * Have that "-x", "-T" in the command line we generate for OpenSSH;
I am slightly negative on this, because we are setting OpenSSH
preferences behind the user's back that they would not normally expect
git to be tampering with.
I think the expectation for this is that it impacts only the ssh session
used by git. But because OpenSSH supports the concept of "master" and
"slave" sessions (i.e., it can multiplex many sessions over a single ssh
session, avoiding authentication and thus reducing latency until the
start of the session), what you do in one session can impact other
sessions. In particular, if the 'master' does not have x11 forwarding
(because it happens to be started by git), then slave connections do not
get it. So a user with X11Forwarding and ControlMaster set in his config
would usually have everything work, but bad timing with the
git-initiated session as the master would unexpectedly break his
X11Forwarding for other sessions.
I don't know how commonly the ControlMaster option for openssh is used.
I also don't know if this should simply be considered a bug in openssh,
since it silently ignores the request for X forwarding. Personally, I
will not be affected because I don't do X forwarding by default, anyway.
But I thought I would raise the point.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] git-add -a: add all files
From: Jeff King @ 2008-07-21 0:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jay Soffian, Johannes Schindelin, Tarmigan, git, Michael J Gruber
In-Reply-To: <7vprp814oe.fsf@gitster.siamese.dyndns.org>
On Sun, Jul 20, 2008 at 05:06:41PM -0700, Junio C Hamano wrote:
> > But again, I haven ever felt the lack of this feature; such usage for me
> > always goes in scripts, where I am more than happy to write out "add .
> > && add -u && commit".
>
> The reason we did not have such "feature" so far was not because somebody
> high in the git foodchain was opposed to the idea, but simply because
> nobody came up with a usable patch to do so.
>
> I do not have anything fundamentally against "add -A" nor "commit -A". To
> me, this is in "perhaps nice to have for some people, but I would not use
> it myself and I wouldn't bother" category, not in "I'm opposed -- it would
> promote bad workflow" cateogry.
I think I didn't make my point well; I am also not that I am opposed to
this feature. The paragraph you quoted meant to say "What I described
above with commit -A is what I think people who are asking for this
feature want. But _I_ don't actually want it, even as somebody who does
this workflow, so I might be wrong."
-Peff
^ permalink raw reply
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Petr Baudis @ 2008-07-21 0:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steve Frécinaux, git
In-Reply-To: <7vljzw14br.fsf@gitster.siamese.dyndns.org>
On Sun, Jul 20, 2008 at 05:14:16PM -0700, Junio C Hamano wrote:
> I never thought you would think "showing relative to tree-root" is even an
> option.
I assume you mean "not filtering relative to tree-root"?
> That would make it inconsistent with not just the established
> semantics of what the plumbing did, but also with what ls-files does.
But ls-files always works on the index; ls-tree can work on trees,
and when you're inspecting a non-root tree object from within
a subdirectory, this behaviour can be rather unexpected. But as I said,
I'm fine with just documenting it.
--
Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name. -- Ken Thompson and Dennis M. Ritchie
^ permalink raw reply
* Re: [PATCH 2/2] git-add -a: add all files
From: Jeff King @ 2008-07-21 0:22 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jay Soffian, Johannes Schindelin, Tarmigan, git, Michael J Gruber
In-Reply-To: <20080721001731.GC12454@sigill.intra.peff.net>
On Sun, Jul 20, 2008 at 08:17:32PM -0400, Jeff King wrote:
> I think I didn't make my point well; I am also not that I am opposed to
Urgh, bad editing. "I am also not opposed" in case it was not clear.
-Peff
^ permalink raw reply
* Re: [PATCH] git-mv: Keep moved index entries inact
From: Petr Baudis @ 2008-07-21 0:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vej5pwhub.fsf@gitster.siamese.dyndns.org>
On Sat, Jul 19, 2008 at 04:54:20PM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
>
> > diff --git a/read-cache.c b/read-cache.c
> > index 1648428..70e5f57 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -38,6 +38,21 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
> > istate->cache_changed = 1;
> > }
> >
> > +void rename_index_entry_at(struct index_state *istate, int nr, const char *new_name)
> > +{
> > + struct cache_entry *old = istate->cache[nr], *new;
> > + int namelen = strlen(new_name);
> > +
> > + new = xmalloc(cache_entry_size(namelen));
> > + copy_cache_entry(new, old);
> > + new->ce_flags = (new->ce_flags & ~CE_NAMEMASK) | namelen;
> > + memcpy(new->name, new_name, namelen);
> > +
> > + cache_tree_invalidate_path(istate->cache_tree, old->name);
> > + remove_index_entry_at(istate, nr);
> > + add_index_entry(istate, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
> > +}
>
> Hmm, would this use of copy_cache_entry() kosher, I have to wonder. This
> new copy of cache entry begins its life unhashed, doesn't it? Shouldn't
> we be not copying its hashed/unhashed bits from the old one?
>
> Also setting of that ce_flags looks wrong when namelen does not fit within
> the width of CE_NAMEMASK. Shouldn't it be doing the same thing as
> create_ce_flags()?
I have to admit that I don't clearly understand all the index entry
intricacies. It is good you didn't see my earlier misguided attempts.
:-) I have patched the two mistakes you pointed out. It is too bad I
cannot simply use existing functions for this, but I want to keep a
different set of traits that either copy_cache_entry() or
create_ce_flags() assumes.
> > diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> > index 336cfaa..6b615f8 100755
> > --- a/t/t7001-mv.sh
> > +++ b/t/t7001-mv.sh
> > @@ -156,4 +156,61 @@ test_expect_success 'absolute pathname outside should fail' '(
> >
> > )'
> >
> > +# git mv meets angry Git maintainer
>
> What's this comment about?
Oh. Well, you sounded agitated in your original mail, but this actually
just slipped through. :-)
> > +test_expect_success 'git mv should not change sha1 of moved cache entry' '
> > +
> > + rm -fr .git &&
> > + git init &&
> > + echo 1 >dirty &&
> > + git add dirty &&
> > + entry="$(git ls-files --stage dirty | cut -f 1)"
>
> "rev-parse :dirty"?
I want to make sure the whole index entry is intact, not just the sha1.
--
Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name. -- Ken Thompson and Dennis M. Ritchie
^ permalink raw reply
* [PATCHv2] git-mv: Keep moved index entries inact
From: Petr Baudis @ 2008-07-21 0:25 UTC (permalink / raw)
To: gitster; +Cc: git
In-Reply-To: <20080721002354.GK10151@machine.or.cz>
The rewrite of git-mv from a shell script to a builtin was perhaps
a little too straightforward: the git add and git rm queues were
emulated directly, which resulted in a rather complicated code and
caused an inconsistent behaviour when moving dirty index entries;
git mv would update the entry based on working tree state,
except in case of overwrites, where the new entry would still have
sha1 of the old file.
This patch introduces rename_index_entry_at() into the index toolkit,
which will rename an entry while removing any entries the new entry
might render duplicate. This is then used in git mv instead
of all the file queues, resulting in a major simplification
of the code and an inevitable change in git mv -n output format.
A new test has been added to the testsuite to reflect this change.
Also, based on suggestion by Junio about desired symlink behaviour
of git mv, I have added two tests for that; however, I do not have
need or desire to spend time fixing this, so they are expected
to fail for now until someone gets around to fixing that.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
builtin-mv.c | 62 ++++++++-------------------------------------------------
cache.h | 2 ++
read-cache.c | 17 ++++++++++++++++
t/t7001-mv.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 83 insertions(+), 53 deletions(-)
diff --git a/builtin-mv.c b/builtin-mv.c
index 33ad082..28ebc9c 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -36,17 +36,6 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
return get_pathspec(prefix, result);
}
-static void show_list(const char *label, struct path_list *list)
-{
- if (list->nr > 0) {
- int i;
- printf("%s", label);
- for (i = 0; i < list->nr; i++)
- printf("%s%s", i > 0 ? ", " : "", list->items[i].path);
- putchar('\n');
- }
-}
-
static const char *add_slash(const char *path)
{
int len = strlen(path);
@@ -75,11 +64,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
const char **source, **destination, **dest_path;
enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
struct stat st;
- struct path_list overwritten = {NULL, 0, 0, 0};
struct path_list src_for_dst = {NULL, 0, 0, 0};
- struct path_list added = {NULL, 0, 0, 0};
- struct path_list deleted = {NULL, 0, 0, 0};
- struct path_list changed = {NULL, 0, 0, 0};
git_config(git_default_config, NULL);
@@ -189,7 +174,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
" will overwrite!\n",
bad);
bad = NULL;
- path_list_insert(dst, &overwritten);
} else
bad = "Cannot overwrite";
}
@@ -218,6 +202,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
for (i = 0; i < argc; i++) {
const char *src = source[i], *dst = destination[i];
enum update_mode mode = modes[i];
+ int pos;
if (show_only || verbose)
printf("Renaming %s to %s\n", src, dst);
if (!show_only && mode != INDEX &&
@@ -227,45 +212,16 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
if (mode == WORKING_DIRECTORY)
continue;
- assert(cache_name_pos(src, strlen(src)) >= 0);
-
- path_list_insert(src, &deleted);
- /* destination can be a directory with 1 file inside */
- if (path_list_has_path(&overwritten, dst))
- path_list_insert(dst, &changed);
- else
- path_list_insert(dst, &added);
+ pos = cache_name_pos(src, strlen(src));
+ assert(pos >= 0);
+ if (!show_only)
+ rename_cache_entry_at(pos, dst);
}
- if (show_only) {
- show_list("Changed : ", &changed);
- show_list("Adding : ", &added);
- show_list("Deleting : ", &deleted);
- } else {
- for (i = 0; i < changed.nr; i++) {
- const char *path = changed.items[i].path;
- int j = cache_name_pos(path, strlen(path));
- struct cache_entry *ce = active_cache[j];
-
- if (j < 0)
- die ("Huh? Cache entry for %s unknown?", path);
- refresh_cache_entry(ce, 0);
- }
-
- for (i = 0; i < added.nr; i++) {
- const char *path = added.items[i].path;
- if (add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0))
- die("updating index entries failed");
- }
-
- for (i = 0; i < deleted.nr; i++)
- remove_file_from_cache(deleted.items[i].path);
-
- if (active_cache_changed) {
- if (write_cache(newfd, active_cache, active_nr) ||
- commit_locked_index(&lock_file))
- die("Unable to write new index file");
- }
+ if (active_cache_changed) {
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(&lock_file))
+ die("Unable to write new index file");
}
return 0;
diff --git a/cache.h b/cache.h
index a779d92..6f1d003 100644
--- a/cache.h
+++ b/cache.h
@@ -260,6 +260,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
#define unmerged_cache() unmerged_index(&the_index)
#define cache_name_pos(name, namelen) index_name_pos(&the_index,(name),(namelen))
#define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
+#define rename_cache_entry_at(pos, new_name) rename_index_entry_at(&the_index, (pos), (new_name))
#define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
#define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
#define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))
@@ -370,6 +371,7 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
#define ADD_CACHE_JUST_APPEND 8 /* Append only; tree.c::read_tree() */
extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
+extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
extern int remove_index_entry_at(struct index_state *, int pos);
extern int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_VERBOSE 1
diff --git a/read-cache.c b/read-cache.c
index 1648428..e93ee3c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -38,6 +38,23 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
istate->cache_changed = 1;
}
+void rename_index_entry_at(struct index_state *istate, int nr, const char *new_name)
+{
+ struct cache_entry *old = istate->cache[nr], *new;
+ int namelen = strlen(new_name);
+
+ new = xmalloc(cache_entry_size(namelen));
+ copy_cache_entry(new, old);
+ new->ce_flags = (new->ce_flags & ~CE_HASHED) | CE_UNHASHED;
+ new->ce_flags = (new->ce_flags & ~CE_NAMEMASK)
+ | (namelen >= CE_NAMEMASK ? CE_NAMEMASK : namelen);
+ memcpy(new->name, new_name, namelen);
+
+ cache_tree_invalidate_path(istate->cache_tree, old->name);
+ remove_index_entry_at(istate, nr);
+ add_index_entry(istate, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
+}
+
/*
* This only updates the "non-critical" parts of the directory
* cache, ie the parts that aren't tracked by GIT, and only used
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 336cfaa..7e47931 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -156,4 +156,59 @@ test_expect_success 'absolute pathname outside should fail' '(
)'
+test_expect_success 'git mv should not change sha1 of moved cache entry' '
+
+ rm -fr .git &&
+ git init &&
+ echo 1 >dirty &&
+ git add dirty &&
+ entry="$(git ls-files --stage dirty | cut -f 1)"
+ git mv dirty dirty2 &&
+ [ "$entry" = "$(git ls-files --stage dirty2 | cut -f 1)" ] &&
+ echo 2 >dirty2 &&
+ git mv dirty2 dirty &&
+ [ "$entry" = "$(git ls-files --stage dirty | cut -f 1)" ]
+
+'
+
+rm -f dirty dirty2
+
+test_expect_failure 'git mv should overwrite symlink to a file' '
+
+ rm -fr .git &&
+ git init &&
+ echo 1 >moved &&
+ ln -s moved symlink &&
+ git add moved symlink &&
+ ! git mv moved symlink &&
+ git mv -f moved symlink &&
+ [ ! -e moved ] &&
+ [ -f symlink ] &&
+ [ $(cat symlink) = 1 ] &&
+ git diff-files --quiet
+
+'
+
+rm -f moved symlink
+
+test_expect_failure 'git mv should follow symlink to a directory' '
+
+ rm -fr .git &&
+ git init &&
+ echo 1 >moved &&
+ mkdir -p dir &&
+ touch dir/.keep &&
+ ln -s dir symlink &&
+ git add moved dir/.keep symlink &&
+ git mv moved symlink &&
+ [ ! -e moved ] &&
+ [ -f symlink/moved ] &&
+ [ $(cat symlink/moved) = 1 ] &&
+ [ "$(ls dir)" = "$(ls symlink)" ] &&
+ git diff-files --quiet
+
+'
+
+rm -rf moved symlink dir
+
test_done
^ permalink raw reply related
* git-shell build error
From: SungHyun Nam @ 2008-07-21 0:29 UTC (permalink / raw)
To: git
Hello,
If NO_SETENV is defined, git-shell cannot be built.
Simply adding the 'compat/setenv.o' to the make rule fixes the problem.
Regards,
[master] ~/srcs/git[30]$ LANG= make
LINK git-shell
Undefined first referenced
symbol in file
gitsetenv exec_cmd.o
ld: fatal: Symbol referencing errors. No output written to git-shell
collect2: ld returned 1 exit status
make: *** [git-shell] Error 1
^ permalink raw reply
* [PATCH] Fix git-shell build error when NO_SETENV is defined
From: Stephan Beyer @ 2008-07-21 0:43 UTC (permalink / raw)
To: SungHyun Nam; +Cc: git, gitster, Stephan Beyer
In-Reply-To: <g60la4$diu$1@ger.gmane.org>
If NO_SETENV is defined, git-shell could not be built.
Thanks to SungHyun Nam for the hint.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
This was my mistake. I haven't tested with several build options.
Now I've tested with
NO_SETENV=1 NO_EXPAT=1 NO_MEMMEM=1 NO_STRTOUMAX=1 NO_MKDTEMP=1
NO_SYS_SELECT_H=1 NO_SYMLINK_HEAD=1
and compat/setenv.o seems to be the only one that was missing.
Regards.
Makefile | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 2b670d7..b650ee6 100644
--- a/Makefile
+++ b/Makefile
@@ -1203,7 +1203,8 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
-git-shell$X: compat/strlcpy.o abspath.o ctype.o exec_cmd.o quote.o strbuf.o usage.o wrapper.o shell.o
+git-shell$X: compat/strlcpy.o compat/setenv.o abspath.o ctype.o exec_cmd.o \
+ quote.o strbuf.o usage.o wrapper.o shell.o
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^)
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
--
1.5.6.3.390.g7b30
^ permalink raw reply related
* Re: [PATCH v2 1/4] builtin-add.c: restructure the code for maintainability
From: Johannes Schindelin @ 2008-07-21 0:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1216534144-23826-1-git-send-email-gitster@pobox.com>
Hi,
On Sat, 19 Jul 2008, Junio C Hamano wrote:
> diff --git a/builtin-add.c b/builtin-add.c
> index bf13aa3..9b2ee8c 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> [...]
> + /*
> + * If we are adding new files, we need to scan the working
> + * tree to find the ones that match pathspecs; this needs
> + * to be done before we read the index.
> + */
This comment left me scratching my head. While I do see a breakage when
reading the index first, I had the impression that it should not.
I can only imagine that the other users of read_directory() depend on some
funny interaction between the index and treat_directory().
Ciao,
Dscho
^ 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