* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Mark Levedahl @ 2008-01-15 23:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7arhas9.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
>> Nope, git submodule *still* requires origin (e.g., execute git
>> submodule init or update on a detached head).
>>
>
> Now I am even more confused.
>
> The approach I suggested in a few paragraphs above, to which you
> just said "I like this change", is about making "git submodule
> update" to use the url configured in the upper level repository
> when it runs "git fetch". I am looking at around l.238 of
> git-submodule.sh. In the current code, it runs "git-fetch"
> without any parameter, which would allow it default to origin or
> whatever, which may or may not be desirable depending on where
> the 'origin' points at. If you make that particular git-fetch
> explicitly say where the fetch should be done from, wouldn't it
> fix the issue for that codepath? Why does it still require
> origin?
1) If top-level is on a detached head, then the remotes machinery will
find current remote is "origin". This is what would be passed down the
chain.
2) Absent the other changes in the thread, git-submodule-init still
invokes git clone *without* -o in the submodules, and thus still
defines and points to remote "origin".
Mark
^ permalink raw reply
* Re: git-commit fatal: Out of memory? mmap failed: Bad file descriptor
From: Junio C Hamano @ 2008-01-15 23:10 UTC (permalink / raw)
To: Linus Torvalds
Cc: Brandon Casey, Git Mailing List, drafnel, Alex Riesen,
Kristian Høgsberg
In-Reply-To: <alpine.LFD.1.00.0801151036110.2806@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, 15 Jan 2008, Brandon Casey wrote:
>>
>> Linus Torvalds wrote:
>> > It would obviously be interesting to see the base repository and the
>> > commit you are trying to do - is that possibly publicly available?
>>
>> I wish it was.
>
> It's ok, I found the bug in your full strace.
>
> The bug really is pretty stupid:
>
> - prepare_index() does a
>
> fd = hold_lock_file_for_update(&false_lock, ...
> ...
> if (write_cache(fd, active_cache, active_nr) || close(fd))
> die("unable to write temporary index file");
>
> and the magic here is that *it*closes*the*fd*.
While I think the ones that are immediately followed by
commit_locked_index() can drop the close(fd) safely, I am not
sure about Kristian's changes to the other ones that we
currently close(fd) but do not commit nor rollback immediately.
These indices are now shown to the hook with open fd to it if
you choose not to close them. Is that okay for Windows guys? I
somehow had an impression that the other process may have
trouble accessing a file that is still open elsewhere for
writing.
So I think the approach along the lines of your "hack" to close
and tell lockfile API not to double-close is more appropriate.
We would perhaps want "close_lock_file(struct lock_file *)" that
calls close(lk->fd) and does lk->fd = -1 without rename/unlink,
and replace these close() with that.
I am sick today, feeling feverish, and not thinking straight,
so I may be talking total nonsense...
^ permalink raw reply
* Re: [PATCH] treat any file with NUL as binary
From: Junio C Hamano @ 2008-01-15 23:11 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: git, Steffen Prohaska
In-Reply-To: <1200407309-10992-1-git-send-email-dpotapov@gmail.com>
Dmitry Potapov <dpotapov@gmail.com> writes:
> So, please, consider it for inclusion as a bug fix.
Somebody has to go back to the "git log" output and the list
archive to see if you two did not forget other ramifications,
because I vaguely recall this 1% thing was done for a reason and
Linus had a very good argument (at least back then the argument
sounded very good to me) supporting the deliberate difference
between the two "binary" heuristics.
If I did not have that vague recollection and all I had to judge
the proposed change were the issue in this thread, I'd probably
agree this is a good change, though.
^ permalink raw reply
* Re: [FEATURE REQUEST] git-svn format-patch
From: Daniel Barkalow @ 2008-01-15 23:11 UTC (permalink / raw)
To: Chris Ortman; +Cc: Johannes Schindelin, git
In-Reply-To: <c0f2d4110801150758t68714570y83e1e74acbb67325@mail.gmail.com>
On Tue, 15 Jan 2008, Chris Ortman wrote:
> The format that TortoiseSVN expects is the same as the format of svn diff.
> The most apparent differences are
>
> diff --git a/Facilities/EventWiring/Castle.Facilities.EventWiring.Tests/Castle.Facilities.EventWiring.Tests-vs2005.csproj
> b/Facilities/EventWiring/Castle.Facilities.EventWiring.Tests/Castle.Facilities.EventWiring.Tests-vs2005.csproj
>
> becomes
>
> Index: Facilities/EventWiring/Castle.Facilities.EventWiring.Tests/Castle.Facilities.EventWiring.Tests-vs2005.csproj
When --no-prefix is used, we should probably do:
Index: <filename>
instead of
diff --git <filename> <filename>
If nothing else, --no-prefix generates patches that git-apply can't apply
but thinks that it should be able to because of the "diff --git" line.
> and
>
> index a0a0d38..9676e16 100644
>
> becomes
>
> ===================================================================
Can't tell if this matters, or if this is meant to underline the Index
line, and if we can leave some extra info after it. The source link you
sent requires a login; is this line actually important to recognition, or
is it just different in the generated patches?
> and
>
> --- a/Facilities/EventWiring/Castle.Facilities.EventWiring.Tests/Castle.Facilities.EventWiring.Tests-vs2005.csproj
> +++ b/Facilities/EventWiring/Castle.Facilities.EventWiring.Tests/Castle.Facilities.EventWiring.Tests-vs2005.csproj
>
> becomes
>
> --- Facilities/EventWiring/Castle.Facilities.EventWiring.Tests/Castle.Facilities.EventWiring.Tests-vs2005.csproj (revision 4715)
> +++ Facilities/EventWiring/Castle.Facilities.EventWiring.Tests/Castle.Facilities.EventWiring.Tests-vs2005.csproj (working copy)
This (putting a description of the revision at the end) would be nice in
general for those of us who can't remember what arguments we gave to git
diff and can't get back to them without quitting less and no longer having
the diff.
Of course, it would take a lot of magic to get git to describe things with
the svn revision info in a non-svn-specific command, but that may not be
necessary if tortoise is willing to apply patches where the base revision
is unknown. Or git-svn could just make a lot of tags like "revision 4715".
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] safecrlf: Add flag to convert_to_git() to disable safecrlf check
From: Junio C Hamano @ 2008-01-15 23:23 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Dmitry Potapov, Linus Torvalds, Git Mailing List
In-Reply-To: <FF86F119-5BF4-4ED3-B6AD-BADFDC91301D@zib.de>
Steffen Prohaska <prohaska@zib.de> writes:
> What is the right way to iterate over the changed files?
I think something like
read_cache();
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
struct stat st;
if (!lstat(ce->name, &st) &&
ce_match_stat(ce, &st, 0)) {
/* do your thing */
}
}
would do.
^ permalink raw reply
* Be more careful about updating refs
From: Linus Torvalds @ 2008-01-15 23:50 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano
This makes write_ref_sha1() more careful: it actually checks the SHA1 of
the ref it is updating, and refuses to update a ref with an object that it
cannot find.
Perhaps more importantly, it also refuses to update a branch head with a
non-commit object. I don't quite know *how* the stable series maintainers
were able to corrupt their repository to have a HEAD that pointed to a tag
rather than a commit object, but they did. Which results in a totally
broken repository that cannot be cloned or committed on.
So make it harder for people to shoot themselves in the foot like that.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
I'm signing off on this, but I hope people will double-check this: I
didn't actually test it very much.
refs.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index 58f6d17..c3ffe03 100644
--- a/refs.c
+++ b/refs.c
@@ -1119,10 +1119,16 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
return 0;
}
+static int is_branch(const char *refname)
+{
+ return !strcmp(refname, "HEAD") || !prefixcmp(refname, "refs/heads/");
+}
+
int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
{
static char term = '\n';
+ struct object *o;
if (!lock)
return -1;
@@ -1130,6 +1136,19 @@ int write_ref_sha1(struct ref_lock *lock,
unlock_ref(lock);
return 0;
}
+ o = parse_object(sha1);
+ if (!o) {
+ error("Trying to write ref %s with nonexistant object %s",
+ lock->ref_name, sha1_to_hex(sha1));
+ unlock_ref(lock);
+ return -1;
+ }
+ if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) {
+ error("Trying to write non-commit object %s to branch %s",
+ sha1_to_hex(sha1), lock->ref_name);
+ unlock_ref(lock);
+ return -1;
+ }
if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
write_in_full(lock->lock_fd, &term, 1) != 1
|| close(lock->lock_fd) < 0) {
^ permalink raw reply related
* Re: Be more careful about updating refs
From: Linus Torvalds @ 2008-01-16 0:02 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.LFD.1.00.0801151546560.2806@woody.linux-foundation.org>
On Tue, 15 Jan 2008, Linus Torvalds wrote:
>
> This makes write_ref_sha1() more careful: it actually checks the SHA1 of
> the ref it is updating, and refuses to update a ref with an object that it
> cannot find.
Side note: this breaks some tests, because those tests do things like
git update-ref refs/heads/master 1111111111111111111111111111111111111111 &&
test 1111111111111111111111111111111111111111 = $(cat .git/refs/heads/master)
which obviously won't work. I didn't update the tests, because I'm an evil
person who just finds it very onerous to touch tests, and I particularly
hate tests that turn out to be wrong.
(Pet peeve on mine: people fixing assert()'s by changing the source-code,
without ever asking themselves whether maybe the assert itself was the
bug).
Linus
^ permalink raw reply
* Re: [PATCH] ls-remote: add -t and -h options.
From: Johannes Schindelin @ 2008-01-16 0:12 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, git
In-Reply-To: <20080115203454.GU29972@genesis.frugalware.org>
Hi,
On Tue, 15 Jan 2008, Miklos Vajna wrote:
> These options are listed in the manpage (aliases for --tags/--heads) but
> they were not handled.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> Alternatively, if it's too late to introduce new options, I think we
> should remove these options from the manpage, but I prefer the current
> fix.
It's not like you added functionality, but just short option handling.
IMHO this is perfectly okay, even this late in the 1.5.4 cycle.
Ciao,
Dscho
^ permalink raw reply
* Make builtin-commit.c more careful about parenthood
From: Linus Torvalds @ 2008-01-16 0:12 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano
When creating the commit object, be a whole lot more careful about making
sure that the parent lines really are valid parent lines. Check things
like MERGE_HEAD having proper SHA1 lines in it, and double-check that all
the parents exist and are actually commits.
---
This is related to the previous patch, in that it is meant to get us
better error messages and behaviour with a corrupt repository where the
HEAD is not a proper commit.
builtin-commit.c | 24 +++++++++++++++++++-----
1 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 6d2ca80..30586a1 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -756,6 +756,17 @@ static const char commit_utf8_warn[] =
"You may want to amend it after fixing the message, or set the config\n"
"variable i18n.commitencoding to the encoding your project uses.\n";
+static void add_parent(struct strbuf *sb, const unsigned char *sha1)
+{
+ struct object *obj = parse_object(sha1);
+ const char *parent = sha1_to_hex(sha1);
+ if (!obj)
+ die("Unable to find commit parent %s", parent);
+ if (obj->type != OBJ_COMMIT)
+ die("Parent %s isn't a proper commit", parent);
+ strbuf_addf(sb, "parent %s\n", parent);
+}
+
int cmd_commit(int argc, const char **argv, const char *prefix)
{
int header_len;
@@ -818,21 +829,24 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
die("could not parse HEAD commit");
for (c = commit->parents; c; c = c->next)
- strbuf_addf(&sb, "parent %s\n",
- sha1_to_hex(c->item->object.sha1));
+ add_parent(&sb, c->item->object.sha1);
} else if (in_merge) {
struct strbuf m;
FILE *fp;
reflog_msg = "commit (merge)";
- strbuf_addf(&sb, "parent %s\n", sha1_to_hex(head_sha1));
+ add_parent(&sb, head_sha1);
strbuf_init(&m, 0);
fp = fopen(git_path("MERGE_HEAD"), "r");
if (fp == NULL)
die("could not open %s for reading: %s",
git_path("MERGE_HEAD"), strerror(errno));
- while (strbuf_getline(&m, fp, '\n') != EOF)
- strbuf_addf(&sb, "parent %s\n", m.buf);
+ while (strbuf_getline(&m, fp, '\n') != EOF) {
+ unsigned char sha1[20];
+ if (get_sha1_hex(m.buf, sha1) < 0)
+ die("Corrupt MERGE_HEAD file (%s)", m.buf);
+ add_parent(&sb, sha1);
+ }
fclose(fp);
strbuf_release(&m);
} else {
^ permalink raw reply related
* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Johannes Schindelin @ 2008-01-16 0:17 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Junio C Hamano, git
In-Reply-To: <478D3CD8.3040805@gmail.com>
Hi,
On Tue, 15 Jan 2008, Mark Levedahl wrote:
> Junio C Hamano wrote:
> > > Nope, git submodule *still* requires origin (e.g., execute git
> > > submodule init or update on a detached head).
> > >
> >
> > Now I am even more confused.
> >
> > The approach I suggested in a few paragraphs above, to which you
> > just said "I like this change", is about making "git submodule
> > update" to use the url configured in the upper level repository
> > when it runs "git fetch". I am looking at around l.238 of
> > git-submodule.sh. In the current code, it runs "git-fetch"
> > without any parameter, which would allow it default to origin or
> > whatever, which may or may not be desirable depending on where
> > the 'origin' points at. If you make that particular git-fetch
> > explicitly say where the fetch should be done from, wouldn't it
> > fix the issue for that codepath? Why does it still require
> > origin?
> 1) If top-level is on a detached head, then the remotes machinery will
> find current remote is "origin". This is what would be passed down the
> chain.
>
> 2) Absent the other changes in the thread, git-submodule-init still
> invokes git clone *without* -o in the submodules, and thus still defines
> and points to remote "origin".
There's got to be a way to fix this _without_ affecting other users.
Ciao,
Dscho
^ permalink raw reply
* Re: [FEATURE REQUEST] git-svn format-patch
From: Junio C Hamano @ 2008-01-16 0:19 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Chris Ortman, Johannes Schindelin, git
In-Reply-To: <alpine.LNX.1.00.0801151728120.13593@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> When --no-prefix is used, we should probably do:
>
> Index: <filename>
>
> instead of
>
> diff --git <filename> <filename>
>
> If nothing else, --no-prefix generates patches that git-apply can't apply
> but thinks that it should be able to because of the "diff --git" line.
While I do not necessarily agree with that "Index: <filename>"
thing, I think dropping "--git" from there is probably a good
idea, as that is clearly not "--git" patch meant to be fed to
git-apply.
Actually I vaguely recall somebody suggested that we drop
"--git" if any nonstandard --src-prefix and --dst-prefix, but
sorry I do not recall the details (I am a bit sick today). I
guess somehow we did not heed that wise advise and accepted the
change as-is, and that new feature ended up with a half-baked
hack that did not consider its consequences X-<.
^ permalink raw reply
* Re: Be more careful about updating refs
From: Junio C Hamano @ 2008-01-16 0:29 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <alpine.LFD.1.00.0801151546560.2806@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>
> I'm signing off on this, but I hope people will double-check this: I
> didn't actually test it very much.
Does "Signed-off-by:" line mean something different for you than
for other people these days?
I thought that the rule that applies to you (and only you) on
this list was that all patches from you are DCO certified and I
am free to forge your signature on them, and the other rule that
applies to everybody including you is that Signed-off-by: is
about DCO certification and not about anything else
(e.g. author's confidence level about the patch).
I am asking because you did not have S-o-b and did not say
anything about the other patch about commit object creation.
Although both patches looked sane to me.
^ permalink raw reply
* Make 'git fsck' complain about non-commit branches
From: Linus Torvalds @ 2008-01-16 0:34 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano
Since having non-commits in branches is a no-no, and just means you cannot
commit on them, let's make fsck tell you when a branch is bad.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
builtin-fsck.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/builtin-fsck.c b/builtin-fsck.c
index 8876d34..6fc9525 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -555,20 +555,23 @@ static int fsck_handle_reflog(const char *logname, const unsigned char *sha1, in
return 0;
}
+static int is_branch(const char *refname)
+{
+ return !strcmp(refname, "HEAD") || !prefixcmp(refname, "refs/heads/");
+}
+
static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
{
struct object *obj;
- obj = lookup_object(sha1);
+ obj = parse_object(sha1);
if (!obj) {
- if (has_sha1_file(sha1)) {
- default_refs++;
- return 0; /* it is in a pack */
- }
error("%s: invalid sha1 pointer %s", refname, sha1_to_hex(sha1));
/* We'll continue with the rest despite the error.. */
return 0;
}
+ if (obj->type != OBJ_COMMIT && is_branch(refname))
+ error("%s: not a commit", refname);
default_refs++;
obj->used = 1;
mark_reachable(obj, REACHABLE);
^ permalink raw reply related
* Re: Be more careful about updating refs
From: Linus Torvalds @ 2008-01-16 0:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4s2d33x.fsf@gitster.siamese.dyndns.org>
On Tue, 15 Jan 2008, Junio C Hamano wrote:
>
> Does "Signed-off-by:" line mean something different for you than
> for other people these days?
I intentionally don't tend to sign off stuff that I just send out as trial
balloons, not meant to "really" be applied.
Of course, then I *also* forget to sign off emails that I do intend to be
applied, so that rule-of-thumb is pretty weak ;)
Linus
^ permalink raw reply
* Re: Make 'git fsck' complain about non-commit branches
From: Junio C Hamano @ 2008-01-16 0:43 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.1.00.0801151618300.2806@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Since having non-commits in branches is a no-no, and just means you cannot
> commit on them, let's make fsck tell you when a branch is bad.
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
I think all of the three patches (so far -- do you have more?)
look sane from the Porcelain-plumbing combination point of view,
but I wonder if we should make it more explicit that we are now
moving in the direction that makes plumbing much more aware of
the Porcelain convention.
So far, the plumbing level did not care much about the Porcelain
convention, such as refs/heads and refs/remotes (you seem to
have forgot) are about "branches" and must point at commit
objects.
> ---
>
> builtin-fsck.c | 13 ++++++++-----
> 1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/builtin-fsck.c b/builtin-fsck.c
> index 8876d34..6fc9525 100644
> --- a/builtin-fsck.c
> +++ b/builtin-fsck.c
> @@ -555,20 +555,23 @@ static int fsck_handle_reflog(const char *logname, const unsigned char *sha1, in
> return 0;
> }
>
> +static int is_branch(const char *refname)
> +{
> + return !strcmp(refname, "HEAD") || !prefixcmp(refname, "refs/heads/");
> +}
> +
> static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
> {
> struct object *obj;
>
> - obj = lookup_object(sha1);
> + obj = parse_object(sha1);
> if (!obj) {
> - if (has_sha1_file(sha1)) {
> - default_refs++;
> - return 0; /* it is in a pack */
> - }
> error("%s: invalid sha1 pointer %s", refname, sha1_to_hex(sha1));
> /* We'll continue with the rest despite the error.. */
> return 0;
> }
> + if (obj->type != OBJ_COMMIT && is_branch(refname))
> + error("%s: not a commit", refname);
> default_refs++;
> obj->used = 1;
> mark_reachable(obj, REACHABLE);
^ permalink raw reply
* [PATCH/RFC] Do not show "diff --git" metainfo with --no-prefix
From: Junio C Hamano @ 2008-01-16 0:58 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Chris Ortman, Johannes Schindelin, git
In-Reply-To: <7vhched3kw.fsf@gitster.siamese.dyndns.org>
If a non-standard prefix is used by --no-prefix, --src-prefix,
or --dst-prefix options, the resulting diff becomes something
git-apply would not grok. In such a case, we should not trigger
the more strict check git-apply does for "diff --git" format.
This checks the prefix specified when generating diff and if src
and dst prefix are not one-level of directory name followed by a
slash (i.e. the standard "diff --git a/foo b/foo" is fine, a
custom "diff --git l/foo k/foo" is Ok, but "diff --git foo foo"
is NOT Ok).
---
diff.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/diff.c b/diff.c
index b18c140..8126a74 100644
--- a/diff.c
+++ b/diff.c
@@ -1233,6 +1233,18 @@ static const char *diff_funcname_pattern(struct diff_filespec *one)
return NULL;
}
+static int with_standard_prefix(struct diff_options *o)
+{
+ const char *slash;
+ slash = strchr(o->a_prefix, '/');
+ if (!slash || slash[1])
+ return 0;
+ slash = strchr(o->b_prefix, '/');
+ if (!slash || slash[1])
+ return 0;
+ return 1;
+}
+
static void builtin_diff(const char *name_a,
const char *name_b,
struct diff_filespec *one,
@@ -1246,30 +1258,46 @@ static void builtin_diff(const char *name_a,
char *a_one, *b_two;
const char *set = diff_get_color_opt(o, DIFF_METAINFO);
const char *reset = diff_get_color_opt(o, DIFF_RESET);
+ int is_git_diff = with_standard_prefix(o);
a_one = quote_two(o->a_prefix, name_a + (*name_a == '/'));
b_two = quote_two(o->b_prefix, name_b + (*name_b == '/'));
lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
- printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
+
+ if (!is_git_diff)
+ printf("%sIndex: %s%s\n", set, b_two, reset);
+ else
+ printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
+
if (lbl[0][0] == '/') {
/* /dev/null */
- printf("%snew file mode %06o%s\n", set, two->mode, reset);
- if (xfrm_msg && xfrm_msg[0])
- printf("%s%s%s\n", set, xfrm_msg, reset);
+ if (is_git_diff) {
+ printf("%snew file mode %06o%s\n",
+ set, two->mode, reset);
+ if (xfrm_msg && xfrm_msg[0])
+ printf("%s%s%s\n", set, xfrm_msg, reset);
+ }
}
else if (lbl[1][0] == '/') {
- printf("%sdeleted file mode %06o%s\n", set, one->mode, reset);
- if (xfrm_msg && xfrm_msg[0])
- printf("%s%s%s\n", set, xfrm_msg, reset);
+ if (is_git_diff) {
+ printf("%sdeleted file mode %06o%s\n",
+ set, one->mode, reset);
+ if (xfrm_msg && xfrm_msg[0])
+ printf("%s%s%s\n", set, xfrm_msg, reset);
+ }
}
else {
- if (one->mode != two->mode) {
- printf("%sold mode %06o%s\n", set, one->mode, reset);
- printf("%snew mode %06o%s\n", set, two->mode, reset);
+ if (is_git_diff) {
+ if (one->mode != two->mode) {
+ printf("%sold mode %06o%s\n",
+ set, one->mode, reset);
+ printf("%snew mode %06o%s\n",
+ set, two->mode, reset);
+ }
+ if (xfrm_msg && xfrm_msg[0])
+ printf("%s%s%s\n", set, xfrm_msg, reset);
}
- if (xfrm_msg && xfrm_msg[0])
- printf("%s%s%s\n", set, xfrm_msg, reset);
/*
* we do not run diff between different kind
* of objects.
^ permalink raw reply related
* Re: Make 'git fsck' complain about non-commit branches
From: Linus Torvalds @ 2008-01-16 1:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v8x2qd2hu.fsf@gitster.siamese.dyndns.org>
On Tue, 15 Jan 2008, Junio C Hamano wrote:
>
> So far, the plumbing level did not care much about the Porcelain
> convention, such as refs/heads and refs/remotes (you seem to
> have forgot) are about "branches" and must point at commit
> objects.
Yeah. I'm not sure this is all a great idea, but I think they are correct
(and no, "refs/remotes/" would *not* have been correct).
The "be more careful about parents" patch (builtin-commit.c) is
unquestionably a good idea - those things really *have* to be commits from
a plumbing standpoint.
The other ones follow the same rules: "HEAD" really does need to be a
commit, since that will otherwise cause breakage (not just for "git
commit", but for "git clone" too). The same is true of "git checkout" and
"refs/heads/" - if a "refs/heads/" ref isn't a commit, switching branches
will get confused!
So now git-fsck verifies things that would confuse git itself. But that's
also why refs/remotes/* aren't checked for being commits, and really a
remote branch is very *different* from a local branch - because those
things would never be used for a commit chain by the native git commands,
so git itself shouldn't care.
We've clearly moved a lot of the porcelain layer into git internals, and
maybe this went too far, but I suspect not. You can still do whatever the
heck you want from a porcelain angle, it just has to follow the (fairly
lax) rules that core git itself does expect.
Linus
^ permalink raw reply
* Re: Be more careful about updating refs
From: Junio C Hamano @ 2008-01-16 1:11 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.1.00.0801151546560.2806@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> This makes write_ref_sha1() more careful: it actually checks the SHA1 of
> the ref it is updating, and refuses to update a ref with an object that it
> cannot find.
>
> Perhaps more importantly, it also refuses to update a branch head with a
> non-commit object. I don't quite know *how* the stable series maintainers
> were able to corrupt their repository to have a HEAD that pointed to a tag
> rather than a commit object, but they did. Which results in a totally
> broken repository that cannot be cloned or committed on.
Two questions and a comment:
- Do we want to impose the same restriction on refs/remotes/?
I think that is a logical thing to do.
- What should the receive-pack and git-fetch do if they trigger
the check in this codepath while updating the refs under the
affected hierarchies? Fail the push and fetch?
- I think !strcmp(refname, "HEAD") is not quite a right check
to do here. In order to catch the detached head case, it
must be checked, but at the same time if the head is not
detached, it should look at where the symref points at
(i.e. a symref HEAD that points outside refs/heads is an
error, and we need to catch that).
> +static int is_branch(const char *refname)
> +{
> + return !strcmp(refname, "HEAD") || !prefixcmp(refname, "refs/heads/");
> +}
^ permalink raw reply
* Re: [PATCH] treat any file with NUL as binary
From: Dmitry Potapov @ 2008-01-16 1:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Steffen Prohaska
In-Reply-To: <7vsl0yd6r8.fsf@gitster.siamese.dyndns.org>
On Tue, Jan 15, 2008 at 03:11:07PM -0800, Junio C Hamano wrote:
> Dmitry Potapov <dpotapov@gmail.com> writes:
>
> > So, please, consider it for inclusion as a bug fix.
>
> Somebody has to go back to the "git log" output and the list
> archive to see if you two did not forget other ramifications,
> because I vaguely recall this 1% thing was done for a reason and
> Linus had a very good argument (at least back then the argument
> sounded very good to me) supporting the deliberate difference
> between the two "binary" heuristics.
First of all, my patch does not make them being the same, it just
makes one being stricter than the other, and I explained why it
is the tight thing to do.
Second, it is difficult for me to find to what particular words
of Linus *you* refer to. However, if it is something like this
post:
http://article.gmane.org/gmane.comp.version-control.git/39618
Then it seems to me, Linus sounded more in favor of that change than
against it. His main argument was against 'diff' heuristic, which he
felt was not strict enough for CRLF translation: "It's *much* better
to miss some CRLF translation than to do too much of it."
Dmitry
^ permalink raw reply
* Re: [PATCH] treat any file with NUL as binary
From: Junio C Hamano @ 2008-01-16 1:16 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: git, Steffen Prohaska
In-Reply-To: <20080116011321.GD13984@dpotapov.dyndns.org>
Dmitry Potapov <dpotapov@gmail.com> writes:
> Then it seems to me, Linus sounded more in favor of that change than
> against it. His main argument was against 'diff' heuristic, which he
> felt was not strict enough for CRLF translation: "It's *much* better
> to miss some CRLF translation than to do too much of it."
Yeah, you found the right one, and I agreed with the argument
back then, and I agree with it now. I think NUL heuristics is a
good one and obviously it makes it tighter than what diff uses,
which should be what we are aiming for in this thread.
^ permalink raw reply
* Re: [PATCH] treat any file with NUL as binary
From: Junio C Hamano @ 2008-01-16 1:21 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: git, Steffen Prohaska
In-Reply-To: <1200407309-10992-1-git-send-email-dpotapov@gmail.com>
Dmitry Potapov <dpotapov@gmail.com> writes:
> There are two heuristics in Git to detect whether a file is binary
> or text. One in xdiff-interface.c relied on existing NUL byte at
"relies on" (not past tense); we may want to say that it is
stolen from GNU diff to be compatible.
> the beginning. However, convert.c used a different heuristic, which
> relied that the number of non-printable symbols is less than 1%.
>
> Due to difference in approaches whether a file is binary or not,
> it was possible that a file that diff treats as binary will not be
> treated as text by CRLF conversation. This is very confusing for
"conversion".
> a user who seeing that 'git diff' shows file as binary expects it
"sees".
> to be added as binary.
>
> This patch makes is_binary to consider any file that contains at
> least one NUL character as binary.
> ---
>
> So, please, consider it for inclusion as a bug fix.
Please typofix and apply "s/.$/, to ensure that the heuristics
used for CRLF conversion is tighter than what is used by diff./"
or something like that at the end.
Also please add sign-off. The patch looks correct.
^ permalink raw reply
* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Mark Levedahl @ 2008-01-16 1:25 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LSU.1.00.0801160016320.17650@racer.site>
Johannes Schindelin wrote:
>
> There's got to be a way to fix this _without_ affecting other users.
>
> Ciao,
> Dscho
>
>
>
I believe the only normally visible change from my proposal is that
git-submodule update will follow top-level's branch from
branch.<name>.remote, which is a good thing. The other changes are only
visible if using clone -o or otherwise explicitly asking that "origin"
not be used or defined, which again is actually following the user's
request.
If you know of other effects, please explain them.
Mark
^ permalink raw reply
* Re: git-commit fatal: Out of memory? mmap failed: Bad file descriptor
From: Junio C Hamano @ 2008-01-16 1:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: Brandon Casey, Git Mailing List, drafnel, Alex Riesen,
Kristian Høgsberg
In-Reply-To: <7vzlv6d6sa.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> On Tue, 15 Jan 2008, Brandon Casey wrote:
>>>
>>> Linus Torvalds wrote:
>>> > It would obviously be interesting to see the base repository and the
>>> > commit you are trying to do - is that possibly publicly available?
>>>
>>> I wish it was.
>>
>> It's ok, I found the bug in your full strace.
>>
>> The bug really is pretty stupid:
>>
>> - prepare_index() does a
>>
>> fd = hold_lock_file_for_update(&false_lock, ...
>> ...
>> if (write_cache(fd, active_cache, active_nr) || close(fd))
>> die("unable to write temporary index file");
>>
>> and the magic here is that *it*closes*the*fd*.
>
> While I think the ones that are immediately followed by
> commit_locked_index() can drop the close(fd) safely, I am not
> sure about Kristian's changes to the other ones that we
> currently close(fd) but do not commit nor rollback immediately.
> These indices are now shown to the hook with open fd to it if
> you choose not to close them. Is that okay for Windows guys? I
> somehow had an impression that the other process may have
> trouble accessing a file that is still open elsewhere for
> writing.
>
> So I think the approach along the lines of your "hack" to close
> and tell lockfile API not to double-close is more appropriate.
> We would perhaps want "close_lock_file(struct lock_file *)" that
> calls close(lk->fd) and does lk->fd = -1 without rename/unlink,
> and replace these close() with that.
>
> I am sick today, feeling feverish, and not thinking straight,
> so I may be talking total nonsense...
I'll aplly and push out Kristian's one that apparently got
Tested-by from Brandon for tonight.
^ permalink raw reply
* Re: [PATCH/RFC] Do not show "diff --git" metainfo with --no-prefix
From: Johannes Schindelin @ 2008-01-16 1:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, Chris Ortman, git
In-Reply-To: <7v4pded1rk.fsf_-_@gitster.siamese.dyndns.org>
Hi,
On Tue, 15 Jan 2008, Junio C Hamano wrote:
> diff --git a/diff.c b/diff.c
> index b18c140..8126a74 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1246,30 +1258,46 @@ static void builtin_diff(const char *name_a,
> char *a_one, *b_two;
> const char *set = diff_get_color_opt(o, DIFF_METAINFO);
> const char *reset = diff_get_color_opt(o, DIFF_RESET);
> + int is_git_diff = with_standard_prefix(o);
>
> a_one = quote_two(o->a_prefix, name_a + (*name_a == '/'));
> b_two = quote_two(o->b_prefix, name_b + (*name_b == '/'));
> lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
> lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
> - printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
> +
> + if (!is_git_diff)
> + printf("%sIndex: %s%s\n", set, b_two, reset);
> + else
> + printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
> +
Hmm. AFAICT plain diff outputs "diff ...", not "Index: ...". IMHO doing
half of what SVN does, and half what GNU diff does, but not completely
what something else does, does not help anybody.
So I'm mildly negative on this hunk.
Also, I am not quite sure what to do about the "rename from" and "copy
from" headers... The "--git" was always an indication that this patch may
contain something like these headers.
All in all, I think this would be too much. Let's just keep our patch
format, and if anybody else is not able to grok unified diffs as we output
them, have a transformer. Let's not have git core affected.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Johannes Schindelin @ 2008-01-16 1:40 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Junio C Hamano, git
In-Reply-To: <478D5D07.3030202@gmail.com>
Hi,
On Tue, 15 Jan 2008, Mark Levedahl wrote:
> Johannes Schindelin wrote:
>
> > There's got to be a way to fix this _without_ affecting other users.
> I believe the only normally visible change from my proposal is that
> git-submodule update will follow top-level's branch from
> branch.<name>.remote, which is a good thing. The other changes are only
> visible if using clone -o or otherwise explicitly asking that "origin"
> not be used or defined, which again is actually following the user's
> request.
>
> If you know of other effects, please explain them.
Do I really have to explain this late in the game for 1.5.4 how such
intrusive changes can affect stability of code paths which would be
otherwise unaffected by submodules? I think not.
Ciao,
Dscho
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox