* Re: [PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.
From: Christian Couder @ 2007-09-17 3:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk5qr575a.fsf@gitster.siamese.dyndns.org>
Le dimanche 16 septembre 2007, Junio C Hamano a écrit :
> > +static struct commit_list *do_find_bisection(struct commit_list *list,
> > + int nr, int *weights);
> > +
> > /*
> > * zero or positive weight is the number of interesting commits it can
> > * reach, including itself. Especially, weight = 0 means it does not
>
> The comment whose top part we can see here talks about the magic
> values -1 and -2 used while do_find_bisection() after the
> refactoring does its work, and these magic values are never
> visible to the calling function. You should move the comment to
> the top of do_find_bisection() as well.
>
> Also this forward declaration is unwarranted. A bottom-up
> sequence to define do_find_bisection() first, then to define its
> sole caller find_bisection() next is easier to read at least for
> me.
>
> The latter comment also applies to your other patch.
All right, I will send new patchs with these changes.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent
From: J. Bruce Fields @ 2007-09-17 2:45 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Junio C Hamano, git
In-Reply-To: <46a038f90709161624j6eb55de6m61aab9e585e22a05@mail.gmail.com>
On Mon, Sep 17, 2007 at 11:24:12AM +1200, Martin Langhoff wrote:
> On 9/17/07, J. Bruce Fields <bfields@citi.umich.edu> wrote:
> > Complain if we find 8 spaces or more in a row as part of the initial
> > whitespace on a line, and (with --whitespace=stripspace) replace such by
> > a tab.
>
> I do quite a bit of hacking on "spaces-for-indentation" projects and
> still use stripspace to cleanup my patches. So no, thanks.
OK, fair enough.
--b.
^ permalink raw reply
* Re: [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent
From: J. Bruce Fields @ 2007-09-17 2:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7f63zr4.fsf@gitster.siamese.dyndns.org>
On Sun, Sep 16, 2007 at 05:24:31PM -0700, Junio C Hamano wrote:
> "J. Bruce Fields" <bfields@citi.umich.edu> writes:
>
> > Complain if we find 8 spaces or more in a row as part of the initial
> > whitespace on a line, and (with --whitespace=stripspace) replace such by
> > a tab.
> >
> > Well, linux's checkpatch.pl complains about this sort of thing.
>
> Some people program in Python, so I am afraid that this needs to
> be a separate option.
OK.
> Maybe it is time to redo the --whitespace options as bitmasks so
> that we can say --whitespace-fix=tab,tail,lines to pick and
> choose which kinds of breakage to fix?
OK. Or maybe keep the current commandline options and have a
whitespace-style config option someplace?
I'm afraid I won't get to either anytime soon, though, so that project's
up for grabs....
--b.
^ permalink raw reply
* Re: metastore
From: david @ 2007-09-17 2:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <7vk5qq3y76.fsf@gitster.siamese.dyndns.org>
On Sun, 16 Sep 2007, Junio C Hamano wrote:
> david@lang.hm writes:
>
>> On Sun, 16 Sep 2007, Junio C Hamano wrote:
>>>
>>> I do not think there is nothing that writes file contents to
>>> stdout/pager other than "git cat-file" or "git show"; I do not
>>> think they are what you have in mind when talking about managing
>>> the files under /etc. So unfortunately I do not understand the
>>> rest of the discussion you made in your message.
>>
>> Ok, I thought that there was common code for these different
>> uses. could you re-read the rest of the logic based on the change
>> being done in checkout_entry?
>>
>> if you are unwilling to have any changes made to the checkout_entry
>> code then the only remaing question is what you think of Daniel's
>> suggestion to have a hook to replace check_updates()?
>>
>> if it's not acceptable either then we are down to doing a
>> post-checkout trigger.
>
> Post-checkout trigger is something I can say I can live with
> without looking at the actual patch, but that does not mean it
> would be a better approach at all.
we agree on this much at least :-)
> I would not be able to answer the first question right now; that
> needs a patch to prove that it can be done with a well contained
> set of changes that results in a maintainable code.
you cannot answer the question in the affirmitive, but you could say that
any changes in that area would be completely unacceptable to you (and for
a while it sounded like you were saying exactly that). in which case any
effort put into preparing patches would be a waste of time
> I haven't tried to assess the potential extent of damage needed
> to checkout_entry(), and I have never been interested in this
> "keeping track of /etc in place" topic myself. It is unlikely
> I'll try to come up with such a patch on my own to support it at
> such a low level near the core. Somebody who cares about that
> feature needs to take the initiative of doing that work before
> we can discuss and decide, although older-times including myself
> can help spot potential issues.
>
> So while I admit I am skeptical, consider me neither willing nor
> unwilling at this point.
this is reasonable. thanks for pointing me so clearly at the routine that
needs to be modified.
David Lang
^ permalink raw reply
* Re: metastore
From: Junio C Hamano @ 2007-09-17 0:58 UTC (permalink / raw)
To: david
Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709161541330.24221@asgard.lang.hm>
david@lang.hm writes:
> On Sun, 16 Sep 2007, Junio C Hamano wrote:
>>
>> I do not think there is nothing that writes file contents to
>> stdout/pager other than "git cat-file" or "git show"; I do not
>> think they are what you have in mind when talking about managing
>> the files under /etc. So unfortunately I do not understand the
>> rest of the discussion you made in your message.
>
> Ok, I thought that there was common code for these different
> uses. could you re-read the rest of the logic based on the change
> being done in checkout_entry?
>
> if you are unwilling to have any changes made to the checkout_entry
> code then the only remaing question is what you think of Daniel's
> suggestion to have a hook to replace check_updates()?
>
> if it's not acceptable either then we are down to doing a
> post-checkout trigger.
Post-checkout trigger is something I can say I can live with
without looking at the actual patch, but that does not mean it
would be a better approach at all.
I would not be able to answer the first question right now; that
needs a patch to prove that it can be done with a well contained
set of changes that results in a maintainable code.
I haven't tried to assess the potential extent of damage needed
to checkout_entry(), and I have never been interested in this
"keeping track of /etc in place" topic myself. It is unlikely
I'll try to come up with such a patch on my own to support it at
such a low level near the core. Somebody who cares about that
feature needs to take the initiative of doing that work before
we can discuss and decide, although older-times including myself
can help spot potential issues.
So while I admit I am skeptical, consider me neither willing nor
unwilling at this point.
^ permalink raw reply
* Re: [PATCH] apply --index-info: fall back to current index for mode changes
From: Chris Shoemaker @ 2007-09-17 0:46 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <Pine.LNX.4.64.0709170119270.28586@racer.site>
On Mon, Sep 17, 2007 at 01:24:57AM +0100, Johannes Schindelin wrote:
>
> "git diff" does not record index lines for pure mode changes (i.e. no
> lines changed). Therefore, apply --index-info would call out a bogus
> error.
>
> Instead, fall back to reading the info from the current index.
>
> Incidentally, this fixes an error where git-rebase would not rebase a
> commit including a pure mode change, and changes requiring a threeway
> merge.
>
> Noticed by Chris Shoemaker.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Works for me. Thanks.
Acked-by: Chris Shoemaker <chris.shoemaker@cox.net>
^ permalink raw reply
* Re: git-archive not working correctly ?
From: Junio C Hamano @ 2007-09-17 0:35 UTC (permalink / raw)
To: Niki Guldbrand; +Cc: git
In-Reply-To: <1189983026.22727.61.camel@niki2.guldbrand.net>
Niki Guldbrand <niki.guldbrand@gmail.com> writes:
> git-archive --format=<fmt> [--list] [--prefix=<prefix>/] [<extra>]
> [--remote=<repo>] <tree-ish> [path...]
>
> <extra>
> This can be any options that the archiver backend understand.
> See next section.
>
> I want to git tar the "--exclude=option", but i can't get it through.
> Is this option only valid for the zip format with the options "-0" and
> "-9" ?
The "next section" that sentence refers to is "BACKEND EXTRA
OPTIONS" section, which lists -0 and -9 for zip backend. There
is no --exclude=option in either tar or zip backend.
Note that we do not use GNU tar or zip as archiver backends.
^ permalink raw reply
* Re: git-archive not working correctly ?
From: Frank Lichtenheld @ 2007-09-17 0:31 UTC (permalink / raw)
To: Niki Guldbrand; +Cc: git
In-Reply-To: <1189983026.22727.61.camel@niki2.guldbrand.net>
On Mon, Sep 17, 2007 at 12:50:26AM +0200, Niki Guldbrand wrote:
> I have been playing with git-archive for and hour or so, but can't get
> it to pass "extra" options to tar as it's documented that it should, or
> am i reading the docs wrong ?
>
> git-archive --format=<fmt> [--list] [--prefix=<prefix>/] [<extra>]
> [--remote=<repo>] <tree-ish> [path…]
>
> <extra>
> This can be any options that the archiver backend understand.
> See next section.
>
> I want to git tar the "--exclude=option", but i can't get it through.
> Is this option only valid for the zip format with the options "-0" and
> "-9" ?
Yes. The tar backend currently doesn't support any extra options.
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* [PATCH] apply --index-info: fall back to current index for mode changes
From: Johannes Schindelin @ 2007-09-17 0:24 UTC (permalink / raw)
To: Chris Shoemaker, gitster, git
"git diff" does not record index lines for pure mode changes (i.e. no
lines changed). Therefore, apply --index-info would call out a bogus
error.
Instead, fall back to reading the info from the current index.
Incidentally, this fixes an error where git-rebase would not rebase a
commit including a pure mode change, and changes requiring a threeway
merge.
Noticed by Chris Shoemaker.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin-apply.c | 26 ++++++++++++++++++++++++--
t/t3400-rebase.sh | 15 +++++++++++++++
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 7057d0d..bae4413 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2215,6 +2215,20 @@ static int check_patch_list(struct patch *patch)
return err;
}
+/* This function tries to read the sha1 from the current index */
+static int get_current_sha1(const char *path, unsigned char *sha1)
+{
+ int pos;
+
+ if (read_cache() < 0)
+ return -1;
+ pos = cache_name_pos(path, strlen(path));
+ if (pos < 0)
+ return -1;
+ hashcpy(sha1, active_cache[pos]->sha1);
+ return 0;
+}
+
static void show_index_list(struct patch *list)
{
struct patch *patch;
@@ -2231,8 +2245,16 @@ static void show_index_list(struct patch *list)
if (0 < patch->is_new)
sha1_ptr = null_sha1;
else if (get_sha1(patch->old_sha1_prefix, sha1))
- die("sha1 information is lacking or useless (%s).",
- name);
+ /* git diff has no index line for mode/type changes */
+ if (!patch->lines_added && !patch->lines_deleted) {
+ if (get_current_sha1(patch->new_name, sha1) ||
+ get_current_sha1(patch->old_name, sha1))
+ die("mode change for %s, which is not "
+ "in current HEAD", name);
+ sha1_ptr = sha1;
+ } else
+ die("sha1 information is lacking or useless "
+ "(%s).", name);
else
sha1_ptr = sha1;
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 62205b2..95e33b5 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -68,4 +68,19 @@ test_expect_success \
test 3 = $(git rev-list master.. | wc -l)
'
+test_expect_success 'rebase a single mode change' '
+ git checkout master &&
+ echo 1 > X &&
+ git add X &&
+ test_tick &&
+ git commit -m prepare &&
+ git checkout -b modechange HEAD^ &&
+ echo 1 > X &&
+ git add X &&
+ chmod a+x A &&
+ test_tick &&
+ git commit -m modechange A X &&
+ GIT_TRACE=1 git rebase master
+'
+
test_done
--
1.5.3.1.949.g98c3
^ permalink raw reply related
* Re: [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent
From: Junio C Hamano @ 2007-09-17 0:24 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: git
In-Reply-To: <1189982942187-git-send-email-bfields@citi.umich.edu>
"J. Bruce Fields" <bfields@citi.umich.edu> writes:
> Complain if we find 8 spaces or more in a row as part of the initial
> whitespace on a line, and (with --whitespace=stripspace) replace such by
> a tab.
>
> Well, linux's checkpatch.pl complains about this sort of thing.
Some people program in Python, so I am afraid that this needs to
be a separate option.
Maybe it is time to redo the --whitespace options as bitmasks so
that we can say --whitespace-fix=tab,tail,lines to pick and
choose which kinds of breakage to fix?
^ permalink raw reply
* Re: [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent
From: Martin Langhoff @ 2007-09-16 23:24 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Junio C Hamano, git
In-Reply-To: <1189982942187-git-send-email-bfields@citi.umich.edu>
On 9/17/07, J. Bruce Fields <bfields@citi.umich.edu> wrote:
> Complain if we find 8 spaces or more in a row as part of the initial
> whitespace on a line, and (with --whitespace=stripspace) replace such by
> a tab.
I do quite a bit of hacking on "spaces-for-indentation" projects and
still use stripspace to cleanup my patches. So no, thanks.
Perhaps split it off to a separate option? I'm not opposed to the
functionality per-se, but don't put together with
trailing-space-trimming. It's a different beast. Everyone agrees
trimming trailing spaces as much as everyone disagrees on
tabs-vs-spaces.
cheers,
m
^ permalink raw reply
* Re: [RFC] strbuf's in builtin-apply
From: Junio C Hamano @ 2007-09-16 22:54 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20070916172833.GB26457@artemis.corp>
Pierre Habouzit <madcoder@debian.org> writes:
> The builtin-apply part:
> [1/6] New strbuf APIs: splice and attach.
> [2/6] Rewrite convert_to_{git,working_tree} to use strbuf's.
> [3/6] Now that cache.h needs strbuf.h, remove useless includes.
> [4/6] builtin-apply: use strbuf's instead of buffer_desc's.
>
> And the two somehow more independant patches (need 1/6 still):
> [5/6] Refactor replace_encoding_header.
> [6/6] Remove preemptive allocations.
Quite nice. Thanks, will queue.
^ permalink raw reply
* Re: metastore
From: david @ 2007-09-16 22:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <7v7imq5ki0.fsf@gitster.siamese.dyndns.org>
On Sun, 16 Sep 2007, Junio C Hamano wrote:
> david@lang.hm writes:
>
>>> I'd rather not implement it at such a low level where a true
>>> "checkout" happens. For one thing, I am afraid that the special
>>> casing will affect the normal codepath too much and would make
>>> it into a maintenance nightmare.
>>
>> as I understand it, at this point you already choose between three
>> options.
>>
>> 1. write to a file (and set the write bit if needed)
>> 2. write to stdout
>> 3. write to a pager program
>>
>> I am suggesting adding
>> ...
>> or am I missing something major here?
>
> I do not think we are choosing any option in the codepath at
> all.
>
> What I mean by the normal "checkout" is what checkout_entry in
> entry.c does. There is no other option than (1) above. I would
> want to see an extremely good justification if you need to touch
> that codepath to implement this fringe use case.
>
> I do not think there is nothing that writes file contents to
> stdout/pager other than "git cat-file" or "git show"; I do not
> think they are what you have in mind when talking about managing
> the files under /etc. So unfortunately I do not understand the
> rest of the discussion you made in your message.
Ok, I thought that there was common code for these different uses. could
you re-read the rest of the logic based on the change being done in
checkout_entry?
if you are unwilling to have any changes made to the checkout_entry code
then the only remaing question is what you think of Daniel's suggestion to
have a hook to replace check_updates()?
if it's not acceptable either then we are down to doing a post-checkout
trigger.
one concern I have with that approach is how to deal with partial
checkouts. if a user checks out one file how can the post-checkout trigger
know if it's looking at the correct permissions file as opposed to one
left over from something else? can/should it go and read the file from the
index instead of reading the file on the filesystem? (I don't like this
becouse it leads to non-obvious behavior), or can/should there be a config
option to say that whenever any file is checked out the permissions file
needs to be checked out as well.
a post checkout trigger is useful in enough different situations that the
answers to the above questions don't eliminate the usefulness of the
trigger, they just map out the pitfalls of useing it.
David Lang
^ permalink raw reply
* git-archive not working correctly ?
From: Niki Guldbrand @ 2007-09-16 22:50 UTC (permalink / raw)
To: git
Hi.
I have been playing with git-archive for and hour or so, but can't get
it to pass "extra" options to tar as it's documented that it should, or
am i reading the docs wrong ?
git-archive --format=<fmt> [--list] [--prefix=<prefix>/] [<extra>]
[--remote=<repo>] <tree-ish> [path…]
<extra>
This can be any options that the archiver backend understand.
See next section.
I want to git tar the "--exclude=option", but i can't get it through.
Is this option only valid for the zip format with the options "-0" and
"-9" ?
Kind regards
Niki Guldbrand
^ permalink raw reply
* [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent
From: J. Bruce Fields @ 2007-09-16 22:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <11899829424173-git-send-email-bfields@citi.umich.edu>
Complain if we find 8 spaces or more in a row as part of the initial
whitespace on a line, and (with --whitespace=stripspace) replace such by
a tab.
Well, linux's checkpatch.pl complains about this sort of thing.
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
builtin-apply.c | 34 +++++++++++++++++++++++++++-------
1 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 70359c1..fb63089 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -918,6 +918,7 @@ static void check_whitespace(const char *line, int len)
{
const char *err = "Adds trailing whitespace";
int seen_space = 0;
+ int consecutive_spaces = 0;
int i;
/*
@@ -944,6 +945,18 @@ static void check_whitespace(const char *line, int len)
else
break;
}
+
+ err = "Initial indent contains eight or more spaces in a row";
+ for (i = 1; i < len; i++) {
+ if (line[i] == ' ')
+ consecutive_spaces++;
+ else if (line[i] == '\t')
+ consecutive_spaces = 0;
+ else
+ break;
+ if (consecutive_spaces == 8)
+ goto error;
+ }
return;
error:
@@ -1607,9 +1620,10 @@ static int apply_line(char *output, const char *patch, int plen)
int i;
int add_nl_to_tail = 0;
int fixed = 0;
- int last_tab_in_indent = -1;
+ int after_indent = -1;
int last_space_in_indent = -1;
int need_fix_leading_space = 0;
+ int consecutive_spaces = 0;
char *buf;
if ((new_whitespace != strip_whitespace) || !whitespace_error ||
@@ -1630,23 +1644,27 @@ static int apply_line(char *output, const char *patch, int plen)
for (i = 1; i < plen; i++) {
char ch = patch[i];
if (ch == '\t') {
- last_tab_in_indent = i;
+ consecutive_spaces = 0;
if (0 <= last_space_in_indent)
need_fix_leading_space = 1;
}
- else if (ch == ' ')
+ else if (ch == ' ') {
+ consecutive_spaces++;
last_space_in_indent = i;
- else
+ } else
break;
+ if (consecutive_spaces == 8)
+ need_fix_leading_space = 1;
}
+ after_indent=i;
buf = output;
if (need_fix_leading_space) {
- int consecutive_spaces = 0;
+ consecutive_spaces = 0;
/* between patch[1..last_tab_in_indent] strip the
* funny spaces, updating them to tab as needed.
*/
- for (i = 1; i < last_tab_in_indent; i++, plen--) {
+ for (i = 1; i < after_indent; i++, plen--) {
char ch = patch[i];
if (ch != ' ') {
consecutive_spaces = 0;
@@ -1660,7 +1678,9 @@ static int apply_line(char *output, const char *patch, int plen)
}
}
fixed = 1;
- i = last_tab_in_indent;
+ i = after_indent;
+ i -= consecutive_spaces;
+ plen += consecutive_spaces;
}
else
i = 1;
--
1.5.3.1.42.gfe5df
^ permalink raw reply related
* [PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace
From: J. Bruce Fields @ 2007-09-16 22:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <1189982942187-git-send-email-bfields@citi.umich.edu>
Add tests to make sure we strip leading and trailing whitespace correctly.
Of the four tests, the first two should always have passed, the third
requires the "fix whitespace stripping" patch, and the fourth requires
the "complain about >= 8 consecutive spaces in initial indent" patch.
Note that this patch itself adds leading and trailing whitespace.
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
t/t4124-apply-whitespace-strip.sh | 43 +++++++++++++++++++++++++++++++++++++
t/t4124/1-after | 3 ++
t/t4124/1-before | 3 ++
t/t4124/2-after | 3 ++
t/t4124/2-before | 3 ++
t/t4124/3-after | 1 +
t/t4124/3-before | 1 +
t/t4124/4-after | 5 ++++
t/t4124/4-before | 5 ++++
9 files changed, 67 insertions(+), 0 deletions(-)
create mode 100644 t/t4124-apply-whitespace-strip.sh
create mode 100644 t/t4124/1-after
create mode 100644 t/t4124/1-before
create mode 100644 t/t4124/2-after
create mode 100644 t/t4124/2-before
create mode 100644 t/t4124/3-after
create mode 100644 t/t4124/3-before
create mode 100644 t/t4124/4-after
create mode 100644 t/t4124/4-before
diff --git a/t/t4124-apply-whitespace-strip.sh b/t/t4124-apply-whitespace-strip.sh
new file mode 100644
index 0000000..3b5f58b
--- /dev/null
+++ b/t/t4124-apply-whitespace-strip.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+
+test_description='handle space and tab combinations with --whitespace=strip'
+
+. ./test-lib.sh
+
+# The directory t4124/ contains pairs of files "n-before" and "n-after",
+# identicaly except leading and trailing whitespace are stripped from
+# the latter.
+#
+# Check that we strip whitespace correctly by checking that the diff
+# between the two files, applied to the first (with --whitespace=strip)
+# produces the second.
+
+mkpatch () {
+ cp "$1" foo
+ git diff /dev/null foo >patch
+ rm foo
+}
+
+checkstrip () {
+ mkpatch "../t4124/$1-before"
+ git apply --whitespace=strip patch
+ git diff foo "../t4124/$1-after"
+}
+
+test_expect_success \
+ 'trailing tabs and spaces' \
+ 'checkstrip 1'
+
+test_expect_success \
+ 'spaces before tabs' \
+ 'checkstrip 2'
+
+test_expect_success \
+ '8 or more non-consecutive initial spaces' \
+ 'checkstrip 3'
+
+test_expect_success \
+ '8 or more consecutive initial spaces' \
+ 'checkstrip 4'
+
+test_done
diff --git a/t/t4124/1-after b/t/t4124/1-after
new file mode 100644
index 0000000..cf5dfce
--- /dev/null
+++ b/t/t4124/1-after
@@ -0,0 +1,3 @@
+trailing space
+trailing tab
+trailing spaces and tabs
diff --git a/t/t4124/1-before b/t/t4124/1-before
new file mode 100644
index 0000000..1f2505b
--- /dev/null
+++ b/t/t4124/1-before
@@ -0,0 +1,3 @@
+trailing space
+trailing tab
+trailing spaces and tabs
diff --git a/t/t4124/2-after b/t/t4124/2-after
new file mode 100644
index 0000000..f198144
--- /dev/null
+++ b/t/t4124/2-after
@@ -0,0 +1,3 @@
+ space tab
+ space space tab
+ tab space tab
diff --git a/t/t4124/2-before b/t/t4124/2-before
new file mode 100644
index 0000000..8fc35bb
--- /dev/null
+++ b/t/t4124/2-before
@@ -0,0 +1,3 @@
+ space tab
+ space space tab
+ tab space tab
diff --git a/t/t4124/3-after b/t/t4124/3-after
new file mode 100644
index 0000000..4db0e80
--- /dev/null
+++ b/t/t4124/3-after
@@ -0,0 +1 @@
+ 4 spaces, tab, 4 spaces, tab
diff --git a/t/t4124/3-before b/t/t4124/3-before
new file mode 100644
index 0000000..f0e2b9c
--- /dev/null
+++ b/t/t4124/3-before
@@ -0,0 +1 @@
+ 4 spaces, tab, 4 spaces, tab
diff --git a/t/t4124/4-after b/t/t4124/4-after
new file mode 100644
index 0000000..a9b8cf6
--- /dev/null
+++ b/t/t4124/4-after
@@ -0,0 +1,5 @@
+ 7 spaces
+ 8 spaces
+ 9 spaces
+ tab 8 spaces
+ tab 9 spaces
diff --git a/t/t4124/4-before b/t/t4124/4-before
new file mode 100644
index 0000000..a35b624
--- /dev/null
+++ b/t/t4124/4-before
@@ -0,0 +1,5 @@
+ 7 spaces
+ 8 spaces
+ 9 spaces
+ tab 8 spaces
+ tab 9 spaces
--
1.5.3.1.42.gfe5df
^ permalink raw reply related
* [PATCH 1/3] git-apply: fix whitespace stripping
From: J. Bruce Fields @ 2007-09-16 22:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <11899829424040-git-send-email-bfields@citi.umich.edu>
The algorithm isn't right here: it accumulates any set of 8 spaces into
tabs even if they're separated by tabs, so
<four spaces><tab><four spaces><tab>
is converted to
<tab><tab><tab>
when it should be just
<tab><tab>
So teach git-apply that a tab hides any group of less than 8 previous
spaces in a row.
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
builtin-apply.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 976ec77..70359c1 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1642,15 +1642,22 @@ static int apply_line(char *output, const char *patch, int plen)
buf = output;
if (need_fix_leading_space) {
+ int consecutive_spaces = 0;
/* between patch[1..last_tab_in_indent] strip the
* funny spaces, updating them to tab as needed.
*/
for (i = 1; i < last_tab_in_indent; i++, plen--) {
char ch = patch[i];
- if (ch != ' ')
+ if (ch != ' ') {
+ consecutive_spaces = 0;
*output++ = ch;
- else if ((i % 8) == 0)
- *output++ = '\t';
+ } else {
+ consecutive_spaces++;
+ if (consecutive_spaces == 8) {
+ *output++ = '\t';
+ consecutive_spaces = 0;
+ }
+ }
}
fixed = 1;
i = last_tab_in_indent;
--
1.5.3.1.42.gfe5df
^ permalink raw reply related
* whitespace-stripping
From: J. Bruce Fields @ 2007-09-16 22:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The following patches fix one (probably rare) bug in
git apply --whitespace=strip
and then teach it to also complain about initial consecutive spaces that
could be tabs.
The latter is the standard for the kernel, but may not be appropriate
for other projects. I'd like to make the whitespace code handle the
kernel style completely first, then consider configuration to handle
other styles if people complain. But maybe the change of behavior would
be an unpleasant surprise for someone with apply.whitespace=strip and a
project that always uses spaces for indents.
--b.
^ permalink raw reply
* Re: metastore
From: david @ 2007-09-16 22:37 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Junio C Hamano, Johannes Schindelin, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709161715090.5298@iabervon.org>
On Sun, 16 Sep 2007, Daniel Barkalow wrote:
> On Sun, 16 Sep 2007, david@lang.hm wrote:
>
>> On Sun, 16 Sep 2007, Daniel Barkalow wrote:
>>
>>>> I however think your idea to have extra "permission information
>>>> file" is very interesting. What would be more palatable, than
>>>> mucking with the core level git, would be to have an external
>>>> command that takes two tree object names that tells it what the
>>>> old and new trees our work tree is switching between, and have
>>>> that command to:
>>>>
>>>> - inspect the diff-tree output to find out what were checked
>>>> out and might need their permission information tweaked;
>>>>
>>>> - inspect the differences between the "permission information
>>>> file" in these trees to find out what were _not_ checked out,
>>>> but still need their permission information tweaked.
>>>>
>>>> - tweak whatever external information you are interested in
>>>> expressing in your "permission information file" in the work
>>>> tree for the paths it discovered in the above two steps.
>>>> This step may involve actions specific to projects and call
>>>> hook scripts with <path, info from "permission information
>>>> file" for that path> tuples to carry out the actual tweaking.
>>>
>>> Why not have the command also responsible for creating the files that need
>>> to be created (calling back into git to read their contents)? That way,
>>> there's no window where they've been created without their metadata, and
>>> there's more that the core git doesn't have to worry about.
>>
>> my initial thoughts were to have git do all it's normal work and hook into git
>> at the point where it's writing the file out (where today it chooses between
>> writing the data to a file on disk, pipeing to stdout, or pipeing to a pager)
>> by adding the option to pipe into a different program that would deal with the
>> permission stuff. this program would only have to write the file and set the
>> permissions, it wouldn't have to know anything about git other then where to
>> find the permissions it needs to know.
>>
>> it sounds like you are suggesting that the hook be much earlier in the
>> process, and instead of one copy of git running and calling many copies of the
>> writing program, you would have one copy of the writing program that would
>> call many copies of git.
>
> A lot of the git commands are actually currently shell scripts that call
> back to git, so that's not too different. The reason to have a single copy
> of the writing program is that it would be able to get the whole set of
> differences that need to be handled, and first pick out the metadata file,
> process it to figure out the writing instructions once, figure out the
> changes in the writing instructions, and figure out the changes in the
> content, and decide what to do.
I'm still a little unclear on how much work this program would then have
to do. it's problably my lack of understanding that's makeing this sound
much scarier.
>>> I could see the program getting the index, the target tree, and the
>>> directory to put files in, and being told to do the whole 2-way merge
>>> (except, perhaps, updating the index to match the tree, which git could do
>>> afterwards). As far as git would be concerned, it would mostly be like a
>>> bare repository.
>>
>> if this functionality does shift to earlier in the process, how much of the
>> git logic needs to be duplicated in this program?
>>
>> if this program needs to do the merge, won't it have to duplicate the merge
>> logic, including the .gitattributes checking for custom merge calls?
>
> This is two-way merge, not three-way merge. The basic concept is that
> you're in state A, and you want to be in state B. Rather than writing out
> all of state B, you write out all of state B that's different from state
> A. Think of taking a diff of two big trees and then applying it as a
> patch, instead of copying the new tree onto the old tree; the benefit is
> that stuff that doesn't change doesn't get rewritten, and the diff is
> blazingly fast, given how we store our information.
so what would this program be given?
it sounds like it would be called once for the entire tree checkout
would it be handed just the start and end commits and query git for
everything else it needs?
it sounds like there is more then this, you refer to git fully crafting
the new index.
so would this program be accessing an old and new index and do the
comparison between the two?
or would git feed it a list of what's changed and then have it query git
to find the details of the changes.
> 3-way merge will be handled by git, and not in a live /etc directory
> anyway (that is, you'd want to fix up the metadata files as plain text
> files, not as metadata bits on a checked out directory; otherwise, you'll
> be trying to put conflict markers in mode bits, and that's clearly not
> what you want).
right, we don't want conflict markers on mode bits or other ACL type
things, that way lies madness ;)
>> I have been thinking primarily in terms of doing a complete checkout,
>> overwriting all files, and secondarily how do do a checkout of just a few
>> files, but again where all files selected overwrite the existing files.
>>
>> I wasn't thinking of the fact that git optimizes the checkout and avoids
>> writing a file that didn't change.
>>
>> this changes things slightly
>>
>> prior to this I was thinking that the permission file needed to be handled
>> differently becouse writing it out needed to avoid doing any circular
>> refrences where you would need to check the contents of it to write it out.
>>
>> it now appears as if what really needs to happen is that if the permission
>> file changes a different program needs to be called when it's written out then
>> when the other files are written out. by itself this isn't hard as
>> .gitattributes can have a special entry for this filename and that entry can
>> specify a different program, and that program fixes all the permissions
>> (and/or detects that they can't be fixed due to user/filesystem limits,
>> records the error, checks if the repository is set appropriately, and screams
>> to the user if it isn't)
>
> While we're at it, you probably don't even want to write the permission
> file to the live filesystem. It's just one more thing that could leak
> information, and changes to the permissions of files that you record by
> committing the live filesystem would presumably be done by changing the
> permissions of files in the filesystem, not by changing the text file.
the permissions and ACL's can be queried directly from the filesystem, so
I don't see any security problems with writing the permission file to the
filesystem.
changing the permissions would be done by changing the files themselves
(when you are running as root on a filesystem that supports the changes,
otherwise it would need to fall back to writing the file and getting the
changes there, but that should be able to be a local config option)
I don't like the idea of having a file that doesn't appear on the local
filesystem at any point, it just makes troubleshooting too hard.
> (Of course, you could check out the same commits as ordinary source, with
> developer-owned 644 files and a 644 "permissions" file, and there you'd
> have the permissions file appear in the work tree, and you could edit it
> and check it in in a totally mundane way.)
right, and the same thing if the filesystem doesn't support something in
the permission file.
>> it would be a nice optimization to this permission checkout for it to compare
>> the old and the new permissions so that it only tries to change the
>> permissions where it needs to, but is that really nessasary? the program can
>> look at the permissions of the existing files to see what they are and decide
>> if it needs to change them (this would tromp on local changes that aren't
>> checked in. how big of a problem is this?) my initial reaction is that having
>> to know the two commits and do the comparison between them is adding a lot of
>> logic and git interaction that I'd rather avoid if I could.
>
> You probably want to be able to keep local uncommitted changes. People
> like to be able to have things slightly different in their particular
> deployment from the way things are in the repository, for stuff that only
> applies to one system and isn't "how it should be".
if so this means that the permission changing program definantly needs to
operate on the diff of the permisison file, not on the absolute file. this
complicates things slightly, but it shouldn't be too bad.
changing topic slightly.
I know git has pre-commit hooks, but I've never needed to use them.
at what point can you hook in?
can you define a hook that runs when you do a git-add? or only when you do
a git-commit?
the reason I'm asking is to try and figure out when and how to create the
permissions file. when I was thinking in terms of dealing with the
permissions as a single bog block it wasn't that bad to say that at
git-commit time you have to scan every file and check it's permissions to
record them into the file, but with the push for the optimizations that
you're talking about this is no longer reasonable and it really should be
done when the file is added to the index.
on a related note, if this is implemented as a per-write hook then it
makes a lot of sense to have the permission file be per-directory, but if
we do a per-checkout hook like you are suggesting then the permission file
may make more sense as a single file in the top-level directory.
thoughts?
David Lang
^ permalink raw reply
* Re: metastore
From: Junio C Hamano @ 2007-09-16 22:11 UTC (permalink / raw)
To: david
Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709161245490.24221@asgard.lang.hm>
david@lang.hm writes:
>> I'd rather not implement it at such a low level where a true
>> "checkout" happens. For one thing, I am afraid that the special
>> casing will affect the normal codepath too much and would make
>> it into a maintenance nightmare.
>
> as I understand it, at this point you already choose between three
> options.
>
> 1. write to a file (and set the write bit if needed)
> 2. write to stdout
> 3. write to a pager program
>
> I am suggesting adding
> ...
> or am I missing something major here?
I do not think we are choosing any option in the codepath at
all.
What I mean by the normal "checkout" is what checkout_entry in
entry.c does. There is no other option than (1) above. I would
want to see an extremely good justification if you need to touch
that codepath to implement this fringe use case.
I do not think there is nothing that writes file contents to
stdout/pager other than "git cat-file" or "git show"; I do not
think they are what you have in mind when talking about managing
the files under /etc. So unfortunately I do not understand the
rest of the discussion you made in your message.
^ permalink raw reply
* Re: metastore
From: Daniel Barkalow @ 2007-09-16 22:02 UTC (permalink / raw)
To: david
Cc: Junio C Hamano, Johannes Schindelin, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709161346150.24221@asgard.lang.hm>
On Sun, 16 Sep 2007, david@lang.hm wrote:
> On Sun, 16 Sep 2007, Daniel Barkalow wrote:
>
> > > I however think your idea to have extra "permission information
> > > file" is very interesting. What would be more palatable, than
> > > mucking with the core level git, would be to have an external
> > > command that takes two tree object names that tells it what the
> > > old and new trees our work tree is switching between, and have
> > > that command to:
> > >
> > > - inspect the diff-tree output to find out what were checked
> > > out and might need their permission information tweaked;
> > >
> > > - inspect the differences between the "permission information
> > > file" in these trees to find out what were _not_ checked out,
> > > but still need their permission information tweaked.
> > >
> > > - tweak whatever external information you are interested in
> > > expressing in your "permission information file" in the work
> > > tree for the paths it discovered in the above two steps.
> > > This step may involve actions specific to projects and call
> > > hook scripts with <path, info from "permission information
> > > file" for that path> tuples to carry out the actual tweaking.
> >
> > Why not have the command also responsible for creating the files that need
> > to be created (calling back into git to read their contents)? That way,
> > there's no window where they've been created without their metadata, and
> > there's more that the core git doesn't have to worry about.
>
> my initial thoughts were to have git do all it's normal work and hook into git
> at the point where it's writing the file out (where today it chooses between
> writing the data to a file on disk, pipeing to stdout, or pipeing to a pager)
> by adding the option to pipe into a different program that would deal with the
> permission stuff. this program would only have to write the file and set the
> permissions, it wouldn't have to know anything about git other then where to
> find the permissions it needs to know.
>
> it sounds like you are suggesting that the hook be much earlier in the
> process, and instead of one copy of git running and calling many copies of the
> writing program, you would have one copy of the writing program that would
> call many copies of git.
A lot of the git commands are actually currently shell scripts that call
back to git, so that's not too different. The reason to have a single copy
of the writing program is that it would be able to get the whole set of
differences that need to be handled, and first pick out the metadata file,
process it to figure out the writing instructions once, figure out the
changes in the writing instructions, and figure out the changes in the
content, and decide what to do.
> > I could see the program getting the index, the target tree, and the
> > directory to put files in, and being told to do the whole 2-way merge
> > (except, perhaps, updating the index to match the tree, which git could do
> > afterwards). As far as git would be concerned, it would mostly be like a
> > bare repository.
>
> if this functionality does shift to earlier in the process, how much of the
> git logic needs to be duplicated in this program?
>
> if this program needs to do the merge, won't it have to duplicate the merge
> logic, including the .gitattributes checking for custom merge calls?
This is two-way merge, not three-way merge. The basic concept is that
you're in state A, and you want to be in state B. Rather than writing out
all of state B, you write out all of state B that's different from state
A. Think of taking a diff of two big trees and then applying it as a
patch, instead of copying the new tree onto the old tree; the benefit is
that stuff that doesn't change doesn't get rewritten, and the diff is
blazingly fast, given how we store our information.
3-way merge will be handled by git, and not in a live /etc directory
anyway (that is, you'd want to fix up the metadata files as plain text
files, not as metadata bits on a checked out directory; otherwise, you'll
be trying to put conflict markers in mode bits, and that's clearly not
what you want).
> I have been thinking primarily in terms of doing a complete checkout,
> overwriting all files, and secondarily how do do a checkout of just a few
> files, but again where all files selected overwrite the existing files.
>
> I wasn't thinking of the fact that git optimizes the checkout and avoids
> writing a file that didn't change.
>
> this changes things slightly
>
> prior to this I was thinking that the permission file needed to be handled
> differently becouse writing it out needed to avoid doing any circular
> refrences where you would need to check the contents of it to write it out.
>
> it now appears as if what really needs to happen is that if the permission
> file changes a different program needs to be called when it's written out then
> when the other files are written out. by itself this isn't hard as
> .gitattributes can have a special entry for this filename and that entry can
> specify a different program, and that program fixes all the permissions
> (and/or detects that they can't be fixed due to user/filesystem limits,
> records the error, checks if the repository is set appropriately, and screams
> to the user if it isn't)
While we're at it, you probably don't even want to write the permission
file to the live filesystem. It's just one more thing that could leak
information, and changes to the permissions of files that you record by
committing the live filesystem would presumably be done by changing the
permissions of files in the filesystem, not by changing the text file.
(Of course, you could check out the same commits as ordinary source, with
developer-owned 644 files and a 644 "permissions" file, and there you'd
have the permissions file appear in the work tree, and you could edit it
and check it in in a totally mundane way.)
> it would be a nice optimization to this permission checkout for it to compare
> the old and the new permissions so that it only tries to change the
> permissions where it needs to, but is that really nessasary? the program can
> look at the permissions of the existing files to see what they are and decide
> if it needs to change them (this would tromp on local changes that aren't
> checked in. how big of a problem is this?) my initial reaction is that having
> to know the two commits and do the comparison between them is adding a lot of
> logic and git interaction that I'd rather avoid if I could.
You probably want to be able to keep local uncommitted changes. People
like to be able to have things slightly different in their particular
deployment from the way things are in the repository, for stuff that only
applies to one system and isn't "how it should be".
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: metastore
From: david @ 2007-09-16 21:53 UTC (permalink / raw)
To: Junio C Hamano
Cc: Daniel Barkalow, Johannes Schindelin, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <7vbqc25mgi.fsf@gitster.siamese.dyndns.org>
On Sun, 16 Sep 2007, Junio C Hamano wrote:
> david@lang.hm writes:
>
>> my initial thoughts were to have git do all it's normal work and hook
>> into git at the point where it's writing the file out (where today it
>> chooses between writing the data to a file on disk, pipeing to stdout,
>> or pipeing to a pager) by adding the option to pipe into a different
>> program that would deal with the permission stuff. this program would
>> only have to write the file and set the permissions, it wouldn't have
>> to know anything about git other then where to find the permissions it
>> needs to know.
>>
>> it sounds like you are suggesting that the hook be much earlier in the
>> process,...
>
> Well, you misread me or what I said was confusing or both. I
> was suggesting totally opposite. Let git do all its normal
> work, and then call your hook to munge the work tree in any way
> you want.
so you are saying, have git write everything out as-is and then call a
program afterwords to do things? essentially a post-checkout hook?
such a hook is useful in many situations, and would allow for the workflow
where you have /etc, /etc.git, and write scripts to move things back and
forth between them.
so I do think that this is a capability that would be useful to git
overall.
however, for the specific use-case of maintaining /etc I don't think that
it's as good as having a hook at write time.
David Lang
^ permalink raw reply
* Re: metastore
From: david @ 2007-09-16 21:45 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Daniel Barkalow, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <7vwsur590q.fsf@gitster.siamese.dyndns.org>
some of this duplicates thoughts from other messages in this thread.
apologies for the duplication, but I want to be clear the response to
Junio's concerns here as well
On Sun, 16 Sep 2007, Junio C Hamano wrote:
> david@lang.hm writes:
>
>> git has pre-commit hooks that could be used to gather the permission
>> information and store it into a file.
>>
>> git now has the ability to define cusom merge strategies for specific
>> file types, which could be used to handle merges for the permission
>> files.
>> ...
>> There are some significant advantages of having the permission store
>> be just a text file.
>>
>> 1. it doesn't require a special API to a new datastore in git
>>
>> 2. when working in an environment that doesn't allow for implementing the
>> permissions (either a filesystem that can't store the permissions or
>> when not working as root so that you can't set the ownership) the file
>> can just be written and then edited with normal tools.
>>
>> 3. normal merge tools do a reasonable job of merging them.
>>
>> however to do this git would need to gain the ability to say 'this
>> filename is special, it must be checked out before any other file is
>> checked out' (either on a per-directory or per-repository level)
>
> I'd rather not implement it at such a low level where a true
> "checkout" happens. For one thing, I am afraid that the special
> casing will affect the normal codepath too much and would make
> it into a maintenance nightmare.
as I understand it, at this point you already choose between three
options.
1. write to a file (and set the write bit if needed)
2. write to stdout
3. write to a pager program
I am suggesting adding
4. write to a .gitattributes defined program and pass it some parameters.
(and only if the .gitattributes tell you to)
this should be a very small change to the codepath
or am I missing something major here?
if this program can get the contents of the permission file out of the
index, then the requirement I listed before to make sure the permission
file gets written before anything else goes away, and the only requirement
left is the ability to specify a different write method
> But more importantly, if you
> are switching between commits (this includes switching branches,
> checking out a different commit to a detached HEAD, or
> pulling/merging updates your HEAD and updates your work tree),
> and the contents of a path does not change between the original
> commit and the switched-to commit, you may still have to
> "checkout" the external information for that path if your
> "permission information file" are different between these two
> commits. To the underlying checkout aka "two tree merge"
> operation, that kind of change is invisible and it should stay
> so for performance reasons, not to harm the normal operation.
I had not thought of this condition.
however, I think this may be easier then you are thinking
we have two conditions.
1. the permission file hasn't changed.
Solution: do nothing
2. the permission file has changed
Solution: set all the permissions to match the new file
this could be done by useing .gitattributes to specify a different program
for checking out the permission file, and that program goes through the
file and sets the permssions on everything. yes this is a bit inefficiant
compared to diffing the two permission files and only touching the files
that have changed, but is the efficiancy at this point that critical? if
so then instead of feeding the program the contents of the new file you
could feed it the diff between the old and the new file.
in theory you could do this for any file, and it would be a win for some
files (a large file that has a few changes to it would possibly be more
efficiant to modify in place then to re-write), but I'm not sure the
results would be worth the complications. if .gitattributes gains the
ability to specify the program to be used to write the file, it could also
gain the ability to specify feeding that file the diff instead of the full
contents.
the one drawback to just setting all the permissions is that this will
overrule any local changes to files that weren't otherwise modified. how
big of a problem is this?
> IOW, I do not want the core level to even know about the
> existence of "permission information file", even the code that
> implements it is well isolated, ifdefed out or made conditional
> based on some config variable.
nobody is suggesting anything that wouldn't be at least conditional based
on some config variable.
> I however think your idea to have extra "permission information
> file" is very interesting. What would be more palatable, than
> mucking with the core level git, would be to have an external
> command that takes two tree object names that tells it what the
> old and new trees our work tree is switching between, and have
> that command to:
>
> - inspect the diff-tree output to find out what were checked
> out and might need their permission information tweaked;
>
> - inspect the differences between the "permission information
> file" in these trees to find out what were _not_ checked out,
> but still need their permission information tweaked.
>
> - tweak whatever external information you are interested in
> expressing in your "permission information file" in the work
> tree for the paths it discovered in the above two steps.
> This step may involve actions specific to projects and call
> hook scripts with <path, info from "permission information
> file" for that path> tuples to carry out the actual tweaking.
this is an area I wasn't aware of, but it doesn't seem that difficult to
do. the issue (as I address above) is if this needs to be done as a diff
or if it can be done simply by setting all the permissions according to
the new file.
> If we go that route, I am not deeply opposed to add code to
> Porcelains to call that new command after they "checkout" a new
> commit at the very end of their processing (namely, git-commit,
> git-merge, git-am, and git-rebase).
this is saying you want a wrapper around git instead of a hook in git.
> Yes, I am very well aware that somebody already mentioned "there
> is a window between the true checkout and permission tweaking".
> If you need to touch the core level in order to close that
> window, I am not interested.
no matter how small the change? (see the above comments) If so this
converstion isn't worth continuing, if you are just concerned about
maintainability and are willing to consider small changes that won't cause
big maintinance problems then we can continue to discuss if the changes I
am suggesting are small enough. the need to be able to close the
vunerability window is a showstopper to many uses.
>> The ability to handle /etc comes up every few months. it's got to be
>> the most common unimplemented request git has seen.
>
> Asking a pony for many times does not necessary make it the
> right for you to have the pony. The sane way to implement this
> is in your Makefile, as Randal and other people with more
> experience have already pointed out, and I happen to agree with
> them.
you don't always have a makefile. if other tools that you use make
modifications to the files in the locations where they reside, having to
pull those changes back before you can do a checking is a complication as
well
> My gut feeling is that the approach to use an external hook that
> reads your "permission information file" could be done with
> negligible impact to the normal operation of git. I suspect
> that the "new command" I suggested above that would run after
> "checkout" actions would perform what people need to do in their
> Makefiles' "install" rules (if they have the work tree vs target
> tree distinction), or "post-checkout" rules (if they want to use
> the work tree in-place), and not having to write/reinvent a
> Makefile target for this in every project would hopefully make
> it easier to use. That is the only reason I am writing this
> message on this topic.
but you are not willing to allow the hook to be created, you are saying
that there would need to be an external wrapper instead.
at this point it appears that having a hook to be able to specify external
programs at a point where you are already deciding between different
options would be sufficiant.
>> so would changes like this be acceptable?
>
> That is a different question. Is having an extention to help
> people who want to manage perm bits a worthy goal? Perhaps, but
> it depends. Is it worthy enough goal to complicate the really
> core parts of the code and add huge maintenance burden?
> Absolutely not. Can it be made in such a way that it does not
> have much impact to the core parts? We need to see how it is
> done.
this is why I was asking about this approach. do changes like this seem
small enough to be worth the effort of coding and submitting?
David Lang
^ permalink raw reply
* Re: metastore
From: Daniel Barkalow @ 2007-09-16 21:45 UTC (permalink / raw)
To: Junio C Hamano
Cc: david, Johannes Schindelin, martin f krafft, git,
Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <7vbqc25mgi.fsf@gitster.siamese.dyndns.org>
On Sun, 16 Sep 2007, Junio C Hamano wrote:
> david@lang.hm writes:
>
> > my initial thoughts were to have git do all it's normal work and hook
> > into git at the point where it's writing the file out (where today it
> > chooses between writing the data to a file on disk, pipeing to stdout,
> > or pipeing to a pager) by adding the option to pipe into a different
> > program that would deal with the permission stuff. this program would
> > only have to write the file and set the permissions, it wouldn't have
> > to know anything about git other then where to find the permissions it
> > needs to know.
> >
> > it sounds like you are suggesting that the hook be much earlier in the
> > process,...
>
> Well, you misread me or what I said was confusing or both. I
> was suggesting totally opposite. Let git do all its normal
> work, and then call your hook to munge the work tree in any way
> you want.
I think he was replying to me, not you. I was suggesting that git stop at
the index, and let him take care of deciding how the index relates to the
work tree. That is, he'd get called instead of check_updates() in
unpack-trees. (And we might have to funnel more code paths through this
function, so that checkout-index does what read-tree -m would do, wrt
changes to the filesystem).
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [StGit PATCH] It doesn't make sense to sink below an unapplied patch
From: Jon Smirl @ 2007-09-16 21:41 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Catalin Marinas, git
In-Reply-To: <20070915232252.GA25507@diana.vm.bytemark.co.uk>
On 9/15/07, Karl Hasselström <kha@treskal.com> wrote:
> On 2007-09-14 12:18:17 -0400, Jon Smirl wrote:
> > Another way to handle this would be to eliminate the ability of
> > pop/push to reorder and extend sink/float to handle unapplied
> > patches.
>
> I think that I'd like the latter without the former -- that is,
> teaching sink/float how to handle unapplied patches, and leaving
> push/pop as is. That'll let you do what you tried to do in the first
> place, and will leave us with a more redundant command set, but I
> don't think that's bad.
The stack model may be too simplistic. My current patches involve four
groups, there is ordering in the group but the groups can be applied
independently. Patches could be marked as depending on another
patch/group. You could then mark patches applied/unapplied and stg
would sort out the order. Allowing group names would make this easier.
The mm kernel has 1,500 patches. It must be a pain trying to keep
these in a linear order.
The git diff output format could be extended with
Depends-on/Group-name lines. That would let Andrew import the
dependencies.
> Another idea that's been kicked around is to have a general reorder
> command, that spawns an editor and lets you move around (and delete)
> patch names until you're satisfied. (This too would be implemented in
> terms of push and pop of single patches.)
>
> --
> Karl Hasselström, kha@treskal.com
> www.treskal.com/kalle
>
--
Jon Smirl
jonsmirl@gmail.com
^ 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