* Re: am fails to apply patches for files with CRLF lineendings
From: Sverre Rabbelier @ 2009-12-15 6:33 UTC (permalink / raw)
To: Brandon Casey; +Cc: git
In-Reply-To: <ee63ef30912141809k27bc73edp20abddd5e9c7c063@mail.gmail.com>
Heya,
On Tue, Dec 15, 2009 at 03:09, Brandon Casey <drafnel@gmail.com> wrote:
> Forwarding to the list. The original was bounced since gmail sent a
> multipart mime version with html. Seems we can't disable html
> composing in the gmail settings anymore (I thought we used to be able
> to).
You can, it remembers when you click "« Plain Text", at least, it does
for me :P.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [PATCH] octopus: make merge process simpler to follow
From: Stephen Boyd @ 2009-12-15 6:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt, Jari Aalto
In-Reply-To: <7vk4wrrkce.fsf@alter.siamese.dyndns.org>
Its not very easy to understand what heads are being merged given
the current output of an octopus merge. Fix this by replacing the
sha1 with the (usually) better description in GITHEAD_<SHA1>.
Suggested-by: Jari Aalto <jari.aalto@cante.net>
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
Thanks both. Here's a replacement. Let me know if you want a reroll.
git-merge-octopus.sh | 11 +++++++----
t/t7602-merge-octopus-many.sh | 33 +++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/git-merge-octopus.sh b/git-merge-octopus.sh
index 825c52c..417e8fb 100755
--- a/git-merge-octopus.sh
+++ b/git-merge-octopus.sh
@@ -61,12 +61,15 @@ do
exit 2
esac
+ eval pretty_name="\$GITHEAD_$SHA1"
+ : ${pretty_name:=$SHA1}
+
common=$(git merge-base --all $SHA1 $MRC) ||
- die "Unable to find common commit with $SHA1"
+ die "Unable to find common commit with $pretty_name"
case "$LF$common$LF" in
*"$LF$SHA1$LF"*)
- echo "Already up-to-date with $SHA1"
+ echo "Already up-to-date with $pretty_name"
continue
;;
esac
@@ -81,7 +84,7 @@ do
# tree as the intermediate result of the merge.
# We still need to count this as part of the parent set.
- echo "Fast-forwarding to: $SHA1"
+ echo "Fast-forwarding to: $pretty_name"
git read-tree -u -m $head $SHA1 || exit
MRC=$SHA1 MRT=$(git write-tree)
continue
@@ -89,7 +92,7 @@ do
NON_FF_MERGE=1
- echo "Trying simple merge with $SHA1"
+ echo "Trying simple merge with $pretty_name"
git read-tree -u -m --aggressive $common $MRT $SHA1 || exit 2
next=$(git write-tree 2>/dev/null)
if test $? -ne 0
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 01e5415..7377033 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -49,4 +49,37 @@ test_expect_success 'merge c1 with c2, c3, c4, ... c29' '
done
'
+cat >expected <<\EOF
+Trying simple merge with c2
+Trying simple merge with c3
+Trying simple merge with c4
+Merge made by octopus.
+ c2.c | 1 +
+ c3.c | 1 +
+ c4.c | 1 +
+ 3 files changed, 3 insertions(+), 0 deletions(-)
+ create mode 100644 c2.c
+ create mode 100644 c3.c
+ create mode 100644 c4.c
+EOF
+
+test_expect_success 'merge output uses pretty names' '
+ git reset --hard c1 &&
+ git merge c2 c3 c4 >actual &&
+ test_cmp actual expected
+'
+
+cat >expected <<\EOF
+Already up-to-date with c4
+Trying simple merge with c5
+Merge made by octopus.
+ c5.c | 1 +
+ 1 files changed, 1 insertions(+), 0 deletions(-)
+ create mode 100644 c5.c
+EOF
+
+test_expect_success 'merge up-to-date output uses pretty names' '
+ git merge c4 c5 >actual &&
+ test_cmp actual expected
+'
test_done
--
1.6.6.rc2.5.g49666
^ permalink raw reply related
* Re: Generic filters for git archive?
From: Russ Dill @ 2009-12-15 7:08 UTC (permalink / raw)
To: René Scharfe; +Cc: git
In-Reply-To: <4B202945.50200@lsrfire.ath.cx>
On Wed, Dec 9, 2009 at 3:48 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Am 08.12.2009 02:06, schrieb Russ Dill:
>> I'm trying to add copyright headers to my source files as they are
>> exported via git archive. eg:
>>
>> * $Copyright$
>>
>> to
>>
>> * Copyright (c) 2003-2009 by Foo Bar
>> *
>> * This program is free software; you can redistribute it and/or modify it
>> * under the terms of the GNU General Public License as published by the
>> * Free Software Foundation; either version 2 of the License, or (at your
>> * option) any later version.
>> *
>> * This program 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 General Public License
>> * for more details.
>> *
>> * You should have received a copy of the GNU General Public License
>> * along with this program; if not, write to the Free Software Foundation,
>> * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>>
>> And properly handling things like '# $Copyright$', '// $Copyright$',
>> etc. I have a sed script that does this, but no way to apply it to the
>> output of git archive. I tried setting up a smudge filter that would
>> only smudge output on archive exports, but it doesn't appear that the
>> smudge filters get run on git archive.
>>
>> I am currently running 1.6.3.3
>
> Is the filter attribute contained in a .gitattribute file that's part of
> the tree you are trying to export? If it's only in the worktree copy,
> then you need to use the option --worktree-attributes to make git
> archive use it.
hmm..It does seem to be running. But I'd really like to use the
gitattributes from the tagged version I'm exporting and I don't want
the smudge filter to run on files I'm working on in my source tree,
just on the export.
^ permalink raw reply
* Re: [PATCH 03/23] Introduce "skip-worktree" bit in index, teach Git to get/set this bit
From: Johannes Sixt @ 2009-12-15 7:20 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Greg Price, git
In-Reply-To: <fcaeb9bf0912141951l5bbb4baanb991354aa3f11ae4@mail.gmail.com>
Nguyen Thai Ngoc Duy schrieb:
> 2009/12/15 Greg Price <price@ksplice.com>:
>> I confess I can't tell how the skip-worktree bit does differ from
>> assume-unchanged. Is its 'goal' different only in that you have a
>> different motivation for introducing it, or does it actually have a
>> different effect -- and what is that different effect?
>
> On the fun side, you could use both bits in the same worktree, to
> narrow your worktree and have some assume-unchanged files.
>
> Another difference is that with assume-unchanged bit, you make a
> promise to Git that those assume-unchanged files are "good", Git does
> not have to care for them. If somehow you violate the promise, Git can
> harm your files on worktree.
So, the difference is that skip-worktree will not overwrite a file that is
different from the version in the index, but assume-unchanged can? Right?
-- Hannes
^ permalink raw reply
* Re: [PATCH] octopus: make merge process simpler to follow
From: Junio C Hamano @ 2009-12-15 7:32 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Junio C Hamano, Johannes Sixt, Jari Aalto
In-Reply-To: <1260859755-3990-1-git-send-email-bebarino@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> Its not very easy to understand what heads are being merged given
> the current output of an octopus merge. Fix this by replacing the
> sha1 with the (usually) better description in GITHEAD_<SHA1>.
>
> Suggested-by: Jari Aalto <jari.aalto@cante.net>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
>
> Thanks both. Here's a replacement. Let me know if you want a reroll.
Ah, thanks and sorry for having you do an extra work. I amended the
assignment further like thi,s but haven't got a chance to push the result out...
eval pretty_name=\${GITHEAD_$SHA1:-$SHA1}
^ permalink raw reply
* Re: [PATCH] octopus: make merge process simpler to follow
From: Stephen Boyd @ 2009-12-15 7:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Sixt, Jari Aalto
In-Reply-To: <7v3a3c3d5p.fsf@alter.siamese.dyndns.org>
On Mon, 2009-12-14 at 23:32 -0800, Junio C Hamano wrote:
> Ah, thanks and sorry for having you do an extra work. I amended the
> assignment further like thi,s but haven't got a chance to push the result out...
>
> eval pretty_name=\${GITHEAD_$SHA1:-$SHA1}
>
Great, thanks.
^ permalink raw reply
* Re: [PATCH] help.autocorrect: do not run a command if the command given is junk
From: Johannes Sixt @ 2009-12-15 7:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List, Alex Riesen
In-Reply-To: <7vaaxlcehw.fsf@alter.siamese.dyndns.org>
Junio C Hamano schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>> On Montag, 14. Dezember 2009, Junio C Hamano wrote:
>>> In the meantime, I think squashing the following in would help us keep the
>>> two magic numbers in sync.
>> I do not think that keeping the numbers in sync is necessary. For example, the
>> similarity requirement for commands that run automatically could be stricter
>> than for the list of suggestions. Then it would be possible that a unique
>> best candidate is not good enough to be run automatically; there would only
>> be a list of suggestions.
>
> Well thought out. Would you want to reroll a patch with two symbolic
> constants then?
I briefly looked into it, but, no, I don't want to reroll the patch. Not
only would the change be larger than I first thought, but I would also
have to find a mis-typed command where a stricter limit makes a difference
*and* where it makes sense that the guessed command is not run
automatically. Moreover, I would also have to *find* a suitable new
similarity limit. Not something I want to do now.
Please take my original patch and squash in your suggested changes. Here
it is for your convenience with an updated commit message (only the
last paragraph changed).
-- Hannes
--- 8< ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] help.autocorrect: do not run a command if the command given is junk
If a given command is not found, then help.c tries to guess which one the
user could have meant. If help.autocorrect is 0 or unset, then a list of
suggestions is given as long as the dissimilarity between the given command
and the candidates is not excessively high. But if help.autocorrect was
non-zero (i.e., a delay after which the command is run automatically), the
latter restriction on dissimilarity was not obeyed.
In my case, this happened:
$ git ..daab02
WARNING: You called a Git command named '..daab02', which does not exist.
Continuing under the assumption that you meant 'read-tree'
in 4.0 seconds automatically...
The patch reuses the similarity limit that is also applied when the list of
suggested commands is printed.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
help.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/help.c b/help.c
index e8db31f..9da97d7 100644
--- a/help.c
+++ b/help.c
@@ -297,6 +297,9 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
old->names = NULL;
}
+/* An empirically derived magic number */
+#define SIMILAR_ENOUGH(x) ((x) < 6)
+
const char *help_unknown_cmd(const char *cmd)
{
int i, n, best_similarity = 0;
@@ -331,7 +334,7 @@ const char *help_unknown_cmd(const char *cmd)
n = 1;
while (n < main_cmds.cnt && best_similarity == main_cmds.names[n]->len)
++n;
- if (autocorrect && n == 1) {
+ if (autocorrect && n == 1 && SIMILAR_ENOUGH(best_similarity)) {
const char *assumed = main_cmds.names[0]->name;
main_cmds.names[0] = NULL;
clean_cmdnames(&main_cmds);
@@ -349,7 +352,7 @@ const char *help_unknown_cmd(const char *cmd)
fprintf(stderr, "git: '%s' is not a git-command. See 'git --help'.\n", cmd);
- if (best_similarity < 6) {
+ if (SIMILAR_ENOUGH(best_similarity)) {
fprintf(stderr, "\nDid you mean %s?\n",
n < 2 ? "this": "one of these");
--
1.6.6.rc1.46.g1635
^ permalink raw reply related
* Re: [PATCH 03/23] Introduce "skip-worktree" bit in index, teach Git to get/set this bit
From: Nguyen Thai Ngoc Duy @ 2009-12-15 8:05 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Greg Price, git
In-Reply-To: <4B2738D1.90002@viscovery.net>
On Tue, Dec 15, 2009 at 2:20 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Nguyen Thai Ngoc Duy schrieb:
>> 2009/12/15 Greg Price <price@ksplice.com>:
>>> I confess I can't tell how the skip-worktree bit does differ from
>>> assume-unchanged. Is its 'goal' different only in that you have a
>>> different motivation for introducing it, or does it actually have a
>>> different effect -- and what is that different effect?
>>
>> On the fun side, you could use both bits in the same worktree, to
>> narrow your worktree and have some assume-unchanged files.
>>
>> Another difference is that with assume-unchanged bit, you make a
>> promise to Git that those assume-unchanged files are "good", Git does
>> not have to care for them. If somehow you violate the promise, Git can
>> harm your files on worktree.
>
> So, the difference is that skip-worktree will not overwrite a file that is
> different from the version in the index, but assume-unchanged can? Right?
Yes.
--
Duy
^ permalink raw reply
* [PATCH 1/2] filter-branch: remove an unnecessary use of 'git read-tree'
From: Johannes Sixt @ 2009-12-15 8:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List
From: Johannes Sixt <j6t@kdbg.org>
The intent of this particular call to 'git read-tree' was to fill an
index. But in fact, it only allocated an empty index. Later in the
program, the index is filled anyway by calling read-tree with specific
commits, and considering that elsewhere the index is even removed (i.e.,
it is not relied upon that the index file exists), this first call of
read-tree is completely redundant.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Calling read-tree without arguments is not allowed according to the
documentation. The next patch will enforce this.
-- Hannes
git-filter-branch.sh | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index cb9d202..195b5ef 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -259,7 +259,6 @@ test -s "$tempdir"/heads ||
GIT_INDEX_FILE="$(pwd)/../index"
export GIT_INDEX_FILE
-git read-tree || die "Could not seed the index"
# map old->new commit ids for rewriting parents
mkdir ../map || die "Could not create map/ directory"
--
1.6.6.rc1.46.g1635
^ permalink raw reply related
* [PATCH 2/2] read-tree: at least one tree-ish argument is required
From: Johannes Sixt @ 2009-12-15 8:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <4B274BDE.8000504@viscovery.net>
From: Johannes Sixt <j6t@kdbg.org>
Previously, it was possible to run read-tree without any arguments,
whereupon it purged the index!
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Discovered by typing
git ..daab02
with help.autocorrect > 0 :-)
-- Hannes
builtin-read-tree.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 50413ca..31623b9 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -125,6 +125,9 @@ int cmd_read_tree(int argc, const char **argv,
stage = opts.merge = 1;
}
+ if (argc == 0)
+ usage_with_options(read_tree_usage, read_tree_options);
+
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
--
1.6.6.rc1.46.g1635
^ permalink raw reply related
* potential null dereference
From: Jiri Slaby @ 2009-12-15 12:41 UTC (permalink / raw)
To: git
Hi,
Stanse found the following error in unpack-trees.c:
dereferencing NULL pointer here.[. * o src_index]
int unpack_trees(unsigned len, struct tree_desc *t, struct
unpack_trees_options *o)
{
int ret;
static struct cache_entry *dfc;
...
if (o->src_index) { <-- loc0
o->result.timestamp.sec = o->src_index->timestamp.sec;
o->result.timestamp.nsec = o->src_index->timestamp.nsec;
}
o->merge_size = len;
if (!dfc)
dfc = xcalloc(1, ((1 + (0) + 8) & ~7));
o->df_conflict_entry = dfc;
if (len) {
...
}
if (o->merge) {
while (o->pos < o->src_index->cache_nr) { <-- here
It triggers, because there is a test for o->src_index being NULL at
loc0, but here, it is dereferenced without a check. Can this happen
(e.g. does o->merge != NULL imply o->src_index != NULL)?
Further, there is a warning in log-tree.c:
pointer always points to valid memory here, but checking for not
NULL.[parents]
static int log_tree_diff(struct rev_info *opt, struct commit *commit,
struct log_info *log)
{
int showed_log;
struct commit_list *parents;
unsigned const char *sha1 = commit->object.sha1;
if (!opt->diff && !((&opt->diffopt)->flags & (1 << 14)))
return 0;
parents = commit->parents;
if (!parents) { <-- loc0
if (opt->show_root_diff) {
diff_root_tree_sha1(sha1, "", &opt->diffopt);
log_tree_diff_flush(opt);
}
return !opt->loginfo; <-- loc1
}
if (parents && parents->next) { <-- here
I.e. if parents was NULL at loc0, we escaped at loc1. But we check
parents against NULL here again.
thanks,
--
js
^ permalink raw reply
* Re: Generic filters for git archive?
From: René Scharfe @ 2009-12-15 13:06 UTC (permalink / raw)
To: Russ Dill; +Cc: git
In-Reply-To: <f9d2a5e10912142308o50c8b9edy63bb485658c93a03@mail.gmail.com>
Am 15.12.2009 08:08, schrieb Russ Dill:
> hmm..It does seem to be running. But I'd really like to use the
> gitattributes from the tagged version I'm exporting and I don't want
> the smudge filter to run on files I'm working on in my source tree,
> just on the export.
This seems to work here:
$ git version
git version 1.6.3.3
$ mkdir /tmp/x
$ cd /tmp/x
$ git init
Initialized empty Git repository in /tmp/x/.git/
$ echo sc >a
$ git add a
$ echo '#!/bin/sh' >f
$ echo 'sed "s/sc/Santa Claus/"' >>f
$ chmod 755 f
$ git config filter.sc.smudge ./f
$ echo 'a filter=sc' >.gitattributes
$ git add .gitattributes
$ git commit -m.
[master (root-commit) 57f6bef] .
2 files changed, 2 insertions(+), 0 deletions(-)
create mode 100644 .gitattributes
create mode 100644 a
$ rm .gitattributes
$ git archive --prefix=x HEAD | tar xf - xa
$ cat xa
Santa Claus
$ git archive --prefix=y --worktree-attributes HEAD | tar xf - ya
$ cat ya
sc
The first archive call uses the committed .gitattributes file and runs
the file through the smudge filter. The second one uses the worktree
version, which doesn't exist, so no filtering occurs.
What commands do you use?
Thanks,
René
^ permalink raw reply
* TortoiseGIT
From: Laszlo Papp @ 2009-12-15 15:41 UTC (permalink / raw)
To: git
Hello,
I had got a problem in the past with cr + lf between windows and linux
client(maybe now too).
I heard that from more experienced users I need to set autocrlf +
safecrlf on windows, and core.autocrlf false on linux (which is the
default), but If I set core.autocrlf true on linux too, it worked
normally, after a git pull on windows I don't see any modified file
that should be committed, not the case in core.autocrlf false.
Could someone explain this habbit of git, please ?
Everybody said I need to set autocrlf for false, but it doesn't work
so, just with true on linux client, what do I do wrong ?
It's okay now for me, because it works without any problem, I just
don't understand the behaviour of it, and I don't know whether it will
cause further problems.
Thanks in advance!
Best Regards,
Laszlo Papp
^ permalink raw reply
* Re: am fails to apply patches for files with CRLF lineendings
From: Brandon Casey @ 2009-12-15 16:04 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Brandon Casey, git
In-Reply-To: <fabb9a1e0912142233u42fc4bb6k70ef7c539cc1ae76@mail.gmail.com>
Sverre Rabbelier wrote:
> Heya,
>
> On Tue, Dec 15, 2009 at 03:09, Brandon Casey <drafnel@gmail.com> wrote:
>> Forwarding to the list. The original was bounced since gmail sent a
>> multipart mime version with html. Seems we can't disable html
>> composing in the gmail settings anymore (I thought we used to be able
>> to).
>
> You can, it remembers when you click "« Plain Text", at least, it does
> for me :P.
Thanks, that seemed to be the case for the last few emails I wrote.
They defaulted to plain text.
-brandon
^ permalink raw reply
* Re: core.worktree bug
From: Nguyen Thai Ngoc Duy @ 2009-12-15 16:30 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200912071115.48085.robin.rosenberg.lists@dewire.com>
On Mon, Dec 07, 2009 at 11:15:47AM +0100, Robin Rosenberg wrote:
> $ git config core.worktree $(cd ../r1;pwd)
> $ git status
> # On branch master
> # Changed but not updated:
> # (use "git add/rm <file>..." to update what will be committed)
> # (use "git checkout -- <file>..." to discard changes in working directory)
> #
> # deleted: f
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> => Seems the config is actually honored even though GIT_DIR is not set.
>
> Bisect tells me 4f38f6b5bafb1f7f85c7b54d0bb0a0e977cd947c broke it. My main point is that I am
You should have CCed me.
> implementing this in JGit so I want the same behaviour. Question: Should we try to fix this
> in git so it matches the documentation or fix the documentation to match behaviour.
>
> The breakage appeared over a year ago and no one has complained.
This is, I think, due to the shared use of git_work_tree_cfg. When
setup_git_directory_gently() comes close to the end, work_tree has
been detected and set. Then check_repository_format_gently() is
called to make sure the repository is valid. Among the checks are
core.worktree check, which overrides the previously-set git_work_tree_cfg.
This might be the fix, or a start of new breakages. I'll need to look
at this again and make a proper patch message with tests if it's
really correct.
diff --git a/setup.c b/setup.c
index f67250b..1385edb 100644
--- a/setup.c
+++ b/setup.c
@@ -280,6 +280,18 @@ const char *read_gitfile_gently(const char *path)
return path;
}
+static int check_repository_work_tree(const char *var, const char *value, void *cb)
+{
+ if (strcmp(var, "core.worktree") == 0) {
+ if (!value)
+ return config_error_nonbool(var);
+ free(git_work_tree_cfg);
+ git_work_tree_cfg = xstrdup(value);
+ inside_work_tree = -1;
+ }
+ return 0;
+}
+
/*
* We cannot decide in this function whether we are in the work tree or
* not, since the config can only be read _after_ this function was called.
@@ -317,6 +329,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
if (!work_tree_env) {
retval = set_work_tree(gitdirenv);
/* config may override worktree */
+ git_config(check_repository_work_tree, NULL);
if (check_repository_format_gently(nongit_ok))
return NULL;
return retval;
@@ -394,6 +407,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
die_errno("Cannot change to '%s/..'", cwd);
}
+ git_config(check_repository_work_tree, NULL);
inside_git_dir = 0;
if (!work_tree_env)
inside_work_tree = 1;
@@ -471,12 +485,6 @@ int check_repository_format_version(const char *var, const char *value, void *cb
is_bare_repository_cfg = git_config_bool(var, value);
if (is_bare_repository_cfg == 1)
inside_work_tree = -1;
- } else if (strcmp(var, "core.worktree") == 0) {
- if (!value)
- return config_error_nonbool(var);
- free(git_work_tree_cfg);
- git_work_tree_cfg = xstrdup(value);
- inside_work_tree = -1;
}
return 0;
}
--
Duy
^ permalink raw reply related
* Re: Giving command line parameter to textconv command?
From: Jeff King @ 2009-12-15 16:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vfx7c3hmb.fsf@alter.siamese.dyndns.org>
On Mon, Dec 14, 2009 at 09:56:28PM -0800, Junio C Hamano wrote:
> Let's try to do a bit more work to make the coverage complete. After
> scanning "git grep -e start_async -e run_command" output, here is what I
> came up with:
>
> - editor.c::launch_editor() that allows a custom editor named via
> GIT_EDITOR does seem to honor your command line arguments.
>
> - pager.c::setup_pager() is used for GIT_PAGER and it does honor your
> command line arguments.
>
> - ll-merge.c::ll_ext_merge() that is used to handle custom merge drivers
> lets the user specify command line via templating to replace %O %A %B
> and naturally it needs to be aware of the command line arguments.
I think it is important to note in user-facing documentation (like
release notes describing the proposed textconv change) that these are
not just "honor your command line arguments" but "execute your command
with /bin/sh". Maybe that is obviously how it is implemented, but I
think it should be made clear that you have the power to use (and
responsibility to protect yourself from) arbitrary shell code.
> - diff.c::run_external_diff() that runs GIT_EXTERNAL_DIFF defines that
> the command has to take 7 parameters in a fixed order, and is not
> designed to permute its arguments like ll_ext_merge() does, but these
> days people don't use it directly (they use it indirectly via
> "difftool" wrapper), so it probably is not an issue.
There is also diff.*.command, which I think people _do_ set manually (I
used to, until I wrote textconv. :) ). It does not use the shell
currently. I agree that people almost certainly have to write a
shell-script wrapper anyway. But I wonder if we should pass it through
the shell, just for the sake of consistency with the other variables (in
particular, if textconv changes, I think diff.*.command is closely
related).
-Peff
^ permalink raw reply
* Re: Giving command line parameter to textconv command?
From: Jeff King @ 2009-12-15 17:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vvdg9ceud.fsf@alter.siamese.dyndns.org>
On Mon, Dec 14, 2009 at 03:31:38PM -0800, Junio C Hamano wrote:
> The change to do so looks like this; it has a few side effects:
>
> - If somebody else were relying on the fact that 'nkf -w' names the
> entire command, it now will run 'nkf' command with '-w' as an argument,
> and it will break such a set-up. IOW, command that has an IFS white
> space in its path will now need to be quoted from the shell.
>
> You can see the fallout from this in the damage made to t/ hierarchy in
> the attached patch.
>
> - You can now use $HOME and other environment variables your shell
> expands when defining your textconv command.
>
> Overall I think it is a good direction to go, but we need to be careful
> about how we transition the existing repositories that use the old
> semantics.
I agree that this is a good change. My only reservation would be that
spawning a shell would be slightly slower (think "git log -p"). However:
1. textconv is dog-slow already, as it has to dump each blob to a
tempfile[1].
2. There is an obvious optimization, which is that you can skip the
shell if there are no metacharacters (in fact, we seem to be doing
that already in launch_editor).
> We might need to introduce diff.*.xtextconv or something.
I am torn on this. I don't like introducing behavior changes that might
hurt people (and really, I think we are just talking about people with
textconv pointing to a program by its full path that has a space in it).
But I also hate carrying around baggage crap like xtextconv forever. It
makes git harder to explain why there are two variables that do almost
the same thing, and the one they want to use is probably the one with
the crappier, unlikely-to-be-guessed name.
So I am somewhat inclined to call it a bug fix, but I don't feel very
strongly.
-Peff
[1] The current textconv interface is really nice for things like just
using "antiword" out of the box. But I wrote a new interface which can
be much faster: it calls the textconv filter with the blob name, and
then the filter is responsible for using cat-file to get at the blob.
This means the filter can look at only part of a blob (e.g., if we are
interested in the metadata tags at the beginning of a large media file),
and it can cache answers as it sees fit, avoiding access to the blob
entirely.
I need to polish the code a bit and submit it. Obviously this is not
meant to replace the existing textconv, but rather to supplement it, for
when "fast and inconvenient" is better than "slow and simple". What is
the best way to configure this? I can imagine "diff.*.textconvType =
fast", or also "diff.*.fastTextconv".
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] filter-branch: remove an unnecessary use of 'git read-tree'
From: Johannes Schindelin @ 2009-12-15 17:19 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <4B274BDE.8000504@viscovery.net>
Hi,
On Tue, 15 Dec 2009, Johannes Sixt wrote:
> From: Johannes Sixt <j6t@kdbg.org>
>
> The intent of this particular call to 'git read-tree' was to fill an
> index. But in fact, it only allocated an empty index. Later in the
> program, the index is filled anyway by calling read-tree with specific
> commits, and considering that elsewhere the index is even removed (i.e.,
> it is not relied upon that the index file exists), this first call of
> read-tree is completely redundant.
Yes, indeed.
Ciao,
Dscho
^ permalink raw reply
* Re: Giving command line parameter to textconv command?
From: Junio C Hamano @ 2009-12-15 17:23 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <20091215170321.GB21322@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> [1] The current textconv interface is really nice for things like just
> using "antiword" out of the box. But I wrote a new interface which can
> be much faster: it calls the textconv filter with the blob name, and
> then the filter is responsible for using cat-file to get at the blob.
> This means the filter can look at only part of a blob (e.g., if we are
> interested in the metadata tags at the beginning of a large media file),
> and it can cache answers as it sees fit, avoiding access to the blob
> entirely.
>
> I need to polish the code a bit and submit it. Obviously this is not
> meant to replace the existing textconv, but rather to supplement it, for
> when "fast and inconvenient" is better than "slow and simple". What is
> the best way to configure this? I can imagine "diff.*.textconvType =
> fast", or also "diff.*.fastTextconv".
"diff.*.blobfilter"?
^ permalink raw reply
* the 100 mb push
From: Joey Hess @ 2009-12-15 19:23 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 3109 bytes --]
Is it normal for git push to sometimes transfer much more data
than seems necessary? Here is a case where that happens:
joey@gnu:~/src/p.t>git branch
* master
pristine-tar
testsuite
joey@gnu:~/src/p.t>git remote show origin
* remote origin
Fetch URL: ssh://joey@git.kitenet.net/srv/git/pristine-tar.test
Push URL: ssh://joey@git.kitenet.net/srv/git/pristine-tar.test
HEAD branch: master
Remote branches:
master tracked
pristine-tar tracked
testsuite tracked
Local branches configured for 'git pull':
master merges with remote master
pristine-tar merges with remote pristine-tar
testsuite merges with remote testsuite
Local refs configured for 'git push':
master pushes to master (fast forwardable)
pristine-tar pushes to pristine-tar (up to date)
testsuite pushes to testsuite (local out of date)
Here, master is a typical small project branch. It has a 1 line change
made locally.
Meanwhile, the testsuite branch is a 100+ mb monster, containing a lot
of big binaries. In it, a small change has been made in the origin
repo. In the local repo, a *lot* of *big* files have been deleted from
the same branch, about 20 mb of files were removed all told. But the diff
for this change should be quite small.
So, testsuite needs to be merged before it can be pushed, but git push
doesn't tell me that. Instead, it goes off and does this for 2+ hours:
joey@gnu:~/src/p.t>git push
Counting objects: 241, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (206/206), done.
Writing objects: 15% (36/237), 2.16 MiB | 15 KiB/s
^C
It seems to be uploading the entire repo over the wire, and this is a
typical asymmetric network connection, so that goes slow. (Took me a
while to realize it was not just auto-gcing the repo locally.)
Once I realized what was going on, it was easy to merge it as shown
below, and then the push transferred an appropriatly small amount of data.
So, my question is, assuming this is not a straight up bug in git, would
it make sense to avoid this gotcha in some way?
joey@gnu:~/src/p.t2>git checkout testsuite
Switched to branch 'testsuite'
Your branch is ahead of 'origin/testsuite' by 1 commit.
joey@gnu:~/src/p.t2>git pull
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (3/3), done.
remote: Total 3 (delta 2), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From ssh://git.kitenet.net/srv/git/pristine-tar.test
3c16948..fce7ec1 testsuite -> origin/testsuite
Merge made by recursive.
Makefile | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
joey@gnu:~/src/p.t2>git push
Counting objects: 13, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (8/8), done.
Writing objects: 100% (8/8), 889 bytes, done.
Total 8 (delta 5), reused 0 (delta 0)
To ssh://joey@git.kitenet.net/srv/git/pristine-tar.test
aab45a1..cc93945 master -> master
fce7ec1..d82f225 testsuite -> testsuite
git version 1.6.5.3
--
see shy jo
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/7] reset: do not accept a mixed reset in a .git dir
From: Christian Couder @ 2009-12-15 19:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <7vtyvvn9wx.fsf@alter.siamese.dyndns.org>
On samedi 12 décembre 2009, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > It is strange and fragile that a mixed reset is disallowed in a bare
> > repo but is allowed in a .git dir. So this patch simplifies things
> > by only allowing soft resets when not in a working tree.
>
> I would not mind what you said were "I find the difference strange", as
> it is a fairly subjective word. But if you say "fragile", you would need
> to defend the use of the word better. What kind of misuse does it
> invite, and what grave consequences such misuses would cause? I do not
> see any fragility and I would want to understand it better so that I can
> write about it in the Release Note to warn people and encourage them to
> upgrade.
>
> More importantly, I think the difference between the two actually makes
> sense. A bare repository by definition does _not_ have any work tree so
> there is no point in having the index file in there. On the other hand,
> even though it is not necessary to "cd .git && git reset HEAD^", I don't
> see a strong reason why it needs to be disallowed.
Ok. I have the following patch now:
---------- 8< ---------------
commit c20f969db6e565f2fe854b95202c3ef95ad0ff42
Author: Christian Couder <chriscool@tuxfamily.org>
Date: Thu Dec 10 22:10:07 2009 +0100
reset: improve mixed reset error message when in a bare repo
When running a "git reset --mixed" in a bare repository, the
message displayed is:
fatal: This operation must be run in a work tree
fatal: Could not reset index file to revision 'HEAD^'.
This message is a little bit misleading as a mixed reset is ok in
a git directory, so it is not absolutely needed to run it in a
work tree.
So this patch improves upon the above by changing the message to:
fatal: mixed reset is not allowed in a bare repository
This patch is also needed to speed up "git reset" by using
unpack_tree() directly (instead of execing "git read-tree"). A
following patch will do just that.
While at it, instead of disallowing "git reset --option" outside
a work tree only when option is "hard" or "merge", we now disallow
it except when option is "soft" or "mixed", as it is safer if we
ever add options to "git reset".
diff --git a/builtin-reset.c b/builtin-reset.c
index 11d1c6e..ac3505b 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -286,11 +286,15 @@ int cmd_reset(int argc, const char **argv, const char
*pre
if (reset_type == NONE)
reset_type = MIXED; /* by default */
- if ((reset_type == HARD || reset_type == MERGE)
- && !is_inside_work_tree())
+ if (reset_type != SOFT && reset_type != MIXED
+ && !is_inside_work_tree())
die("%s reset requires a work tree",
reset_type_names[reset_type]);
+ if (reset_type == MIXED && is_bare_repository())
+ die("%s reset is not allowed in a bare repository",
+ reset_type_names[reset_type]);
+
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
* the index file to the tree object we are switching to. */
---------- 8< ---------------
> On the other hand, I don't see a strong reason why such a use needs to be
> supported, other than "we've allowed it for a long time, and people may
> have hooks (they typically start in $GIT_DIR) that rely on it", which by
> itself is not something strong enough to veto the change. It is only a
> minor incompatibility and I can be persuaded to listen to a pros-and-cons
> argument.
>
> An honest justification might have been "This change to disallow a mixed
> reset in $GIT_DIR of a repository with a work tree will break existing
> scripts, but I think it is not widely used _for such and such reasons_,
> and can easily be worked around. On the other hand, this change vastly
> simplifies the reimplementation of 'reset' _because X and Y and Z_".
My opinion is that it works this way just by accident not by design (that's
why I said "fragile"). But if we don't want to take any risk of regression,
then let's use the new patch above.
> > This patch is also needed to speed up "git reset" by using
> > unpack_tree() directly (instead of execing "git read-tree").
>
> It is very unclear _why_ it is "needed" from this description.
The reason is that after the next patch, it will not fail in a bare
repository, so if we don't change anything, the test that checks that it
fails in a bare repo will fail. I will add this explanation to the new
patch.
Best regards,
Christian.
^ permalink raw reply related
* Re: the 100 mb push
From: Shawn O. Pearce @ 2009-12-15 19:42 UTC (permalink / raw)
To: Joey Hess; +Cc: git
In-Reply-To: <20091215192338.GA16654@gnu.kitenet.net>
Joey Hess <joey@kitenet.net> wrote:
> Is it normal for git push to sometimes transfer much more data
> than seems necessary? Here is a case where that happens:
Yes.
> Meanwhile, the testsuite branch is a 100+ mb monster, containing a lot
> of big binaries. In it, a small change has been made in the origin
> repo. In the local repo, a *lot* of *big* files have been deleted from
> the same branch, about 20 mb of files were removed all told. But the diff
> for this change should be quite small.
>
> So, testsuite needs to be merged before it can be pushed, but git push
> doesn't tell me that. Instead, it goes off and does this for 2+ hours:
The problem here is, unlike fetch, push does not do a common
ancestor negotiation. The sending side (your push client)
just assumes the remote side has *only* what the remote side
advertised.
Since the remote side advertised a commit on the testsuite branch
that your client doesn't have, your client was forced to assume
there was no common ancestor and sent the entire thing.
This usually doesn't show up that badly because the delta tends to
be smaller (no huge binary files), tends to be a strict fast forward
(so your client contains what the remote advertised), and tags may
help to limit the upload size by being at fixed points in the history
(so at worst you upload since the last tag).
Junio wrote a patch series for git push over a year ago to make
it do common ancestor negotiation like git fetch does, but it had
a deadlock problem and the patch series got dropped. Not enough
people were interested to help Junio carry it through to being
ready for inclusion.
--
Shawn.
^ permalink raw reply
* Re: the 100 mb push
From: Junio C Hamano @ 2009-12-15 20:00 UTC (permalink / raw)
To: Joey Hess; +Cc: git
In-Reply-To: <20091215192338.GA16654@gnu.kitenet.net>
Joey Hess <joey@kitenet.net> writes:
> So, my question is, assuming this is not a straight up bug in git, would
> it make sense to avoid this gotcha in some way?
The "push" support was originally written for people who push into their
own repositories for publishing (i.e. almost always fast-forwarding) and
lacked the elaborate common ancestor discovery negotiation the "fetch"
side had.
Suppose you have a rewound or forked history, like this:
(their side)
*---Y pu
/
*---X
/
*---*---Z master
(your side)
*---X---*---A pu
/
*---*---Z---*---B master
- You were in sync when the 'pu' was at X with them; somebody pushed a
few commits on top of it (forked case); or
- You were in sync when the 'pu' was at Y with them (you pushed it their
last time yourself), but you rebuilt 'pu' since then (rewound case).
If you run "git push there master +pu", it learns that the tips of
'master' and 'pu' are at Z and Y respectively at their end. Because the
protocol did not negotiate the common ancestor, it would try to send:
rev-list A B ^Z ^Y
but using only the information available at your end locally.
Because you either never have heard of (in a forked case) or no longer
know (in a rewound case) what 'Y' is, in order to update 'pu', you end up
sending commits 'Z..A', duplicating 'Z..X' part, because "^Y" cannot
participate in the ancestory computation. Commits 'Z..B' will be sent to
update 'master'.
And this aspect of "git push" protocol hasn't changed much since it was
written.
In contrast, the protocol used for "fetch" tries to discover 'X' in such a
case by a little exchange, like this:
downloader: Your tip is at 'Y'? I've never heard of it. Please
tell me about its parents.
uploader: It is this "Y^", and its parent is 'X', and ...
downloader: Ok, I know what 'X' is. I heard enough to proceed. Thank you.
^ permalink raw reply
* Re: [PATCH v5 1/7] reset: do not accept a mixed reset in a .git dir
From: Junio C Hamano @ 2009-12-15 20:17 UTC (permalink / raw)
To: Christian Couder
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <200912152041.36194.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> This patch is also needed to speed up "git reset" by using
> unpack_tree() directly (instead of execing "git read-tree"). A
> following patch will do just that.
This still doesn't seem to explain anything that the part you added after
your patch.
> While at it, instead of disallowing "git reset --option" outside
> a work tree only when option is "hard" or "merge", we now disallow
> it except when option is "soft" or "mixed", as it is safer if we
> ever add options to "git reset".
I fail to see any sane logic behind this reasoning; you cannot decide if
you need to allow or disallow the new --option with unspecified semantics
until you have that --option, and you are saying
Hmm, "reset --option" that does not work when it should work is a bug,
just like "reset --option" that does not refuse to work when it should
refuse is, and you cannot decide if you should allow a new --option until
you have it. Your "disallowing everything the code does not know about by
default" doesn't particularly sound safer to me. I'd suggest dropping it
from this patch.
It is perfectly fine to have a change like that, if it makes the logic
easier to follow with the updated repertoire when a new --option is added,
but not before.
>> An honest justification might have been "This change to disallow a mixed
>> reset in $GIT_DIR of a repository with a work tree will break existing
>> scripts, but I think it is not widely used _for such and such reasons_,
>> and can easily be worked around. On the other hand, this change vastly
>> simplifies the reimplementation of 'reset' _because X and Y and Z_".
>
> My opinion is that it works this way just by accident not by design (that's
> why I said "fragile").
And do you still think it is accident after I explained the difference
between the two for you (or perhaps you didn't read it)?
>> > This patch is also needed to speed up "git reset" by using
>> > unpack_tree() directly (instead of execing "git read-tree").
>>
>> It is very unclear _why_ it is "needed" from this description.
>
> The reason is that after the next patch, it will not fail in a bare
> repository,...
That sounds as if you want to change the definition of what the expected
behaviour is early, because you want to claim a regression you will later
introduce is not a regression. I hope that is not the case.
^ permalink raw reply
* Re: [PATCH v5 1/7] reset: do not accept a mixed reset in a .git dir
From: Junio C Hamano @ 2009-12-15 20:20 UTC (permalink / raw)
To: Christian Couder
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
Stephen Boyd
In-Reply-To: <200912152041.36194.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> diff --git a/builtin-reset.c b/builtin-reset.c
> index 11d1c6e..ac3505b 100644
> --- a/builtin-reset.c
> +++ b/builtin-reset.c
> @@ -286,11 +286,15 @@ int cmd_reset(int argc, const char **argv, const char
> *pre
> if (reset_type == NONE)
> reset_type = MIXED; /* by default */
>
> - if ((reset_type == HARD || reset_type == MERGE)
> - && !is_inside_work_tree())
> + if (reset_type != SOFT && reset_type != MIXED
> + && !is_inside_work_tree())
> die("%s reset requires a work tree",
> reset_type_names[reset_type]);
>
> + if (reset_type == MIXED && is_bare_repository())
> + die("%s reset is not allowed in a bare repository",
> + reset_type_names[reset_type]);
This patch text itself makes sense, I think, except the first part.
^ 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