* [PATCH] Fix errno usage in connect.c
From: Petr Baudis @ 2006-07-01 21:56 UTC (permalink / raw)
To: Morten Welinder; +Cc: GIT Mailing List
In-Reply-To: <118833cc0606280956s4081029ci5b3cd1fdf4b10c97@mail.gmail.com>
Dear diary, on Wed, Jun 28, 2006 at 06:56:12PM CEST, I got a letter
where Morten Welinder <mwelinder@gmail.com> said that...
> It looks like connect.c waits too long before it uses errno in both copies
> of git_tcp_connect_sock. Both close and freeaddrinfo can poke any
> non-zero value in there.
Nice catch.
->8-
errno was used after it could've been modified by a subsequent library call.
Spotted by Morten Welinder.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
connect.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/connect.c b/connect.c
index cb4656d..9a87bd9 100644
--- a/connect.c
+++ b/connect.c
@@ -328,7 +328,7 @@ #ifndef NO_IPV6
*/
static int git_tcp_connect_sock(char *host)
{
- int sockfd = -1;
+ int sockfd = -1, saved_errno = 0;
char *colon, *end;
const char *port = STR(DEFAULT_GIT_PORT);
struct addrinfo hints, *ai0, *ai;
@@ -362,9 +362,12 @@ static int git_tcp_connect_sock(char *ho
for (ai0 = ai; ai; ai = ai->ai_next) {
sockfd = socket(ai->ai_family,
ai->ai_socktype, ai->ai_protocol);
- if (sockfd < 0)
+ if (sockfd < 0) {
+ saved_errno = errno;
continue;
+ }
if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
+ saved_errno = errno;
close(sockfd);
sockfd = -1;
continue;
@@ -375,7 +378,7 @@ static int git_tcp_connect_sock(char *ho
freeaddrinfo(ai0);
if (sockfd < 0)
- die("unable to connect a socket (%s)", strerror(errno));
+ die("unable to connect a socket (%s)", strerror(saved_errno));
return sockfd;
}
@@ -387,7 +390,7 @@ #else /* NO_IPV6 */
*/
static int git_tcp_connect_sock(char *host)
{
- int sockfd = -1;
+ int sockfd = -1, saved_errno = 0;
char *colon, *end;
char *port = STR(DEFAULT_GIT_PORT), *ep;
struct hostent *he;
@@ -426,8 +429,10 @@ static int git_tcp_connect_sock(char *ho
for (ap = he->h_addr_list; *ap; ap++) {
sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
- if (sockfd < 0)
+ if (sockfd < 0) {
+ saved_errno = errno;
continue;
+ }
memset(&sa, 0, sizeof sa);
sa.sin_family = he->h_addrtype;
@@ -435,6 +440,7 @@ static int git_tcp_connect_sock(char *ho
memcpy(&sa.sin_addr, *ap, he->h_length);
if (connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
+ saved_errno = errno;
close(sockfd);
sockfd = -1;
continue;
@@ -443,7 +449,7 @@ static int git_tcp_connect_sock(char *ho
}
if (sockfd < 0)
- die("unable to connect a socket (%s)", strerror(errno));
+ die("unable to connect a socket (%s)", strerror(saved_errno));
return sockfd;
}
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam
^ permalink raw reply related
* Re: A note on merging conflicts..
From: Junio C Hamano @ 2006-07-01 20:14 UTC (permalink / raw)
To: git
In-Reply-To: <7vveqhccnk.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Linus Torvalds <torvalds@osdl.org> writes:
>
>> On Sat, 1 Jul 2006, Rene Scharfe wrote:
>>>
>>> I wonder why the two clear_commit_marks() calls at the end of
>>> get_merge_bases() are not sufficient, though.
>>
>> Why does that thing check for "parent->object.parsed"?
>
> Oh, I thought I fixed that up when I merged. Sorry.
>
>> Also, it only clears commit marks if they are contiguous, but some commit
>> marking may not be dense (eg, the "UNINTERESTING" mark may have been set
>> by (PARENT1 && PARENT2) triggering, but is not set in the commits that
>> reach it.
>
> That is true.
I'll be offline for a few hours, but if anybody is inclined to
fix this up, I'd appreciate it to be based on top of 7c6f8aa
commit (which is the tip of js/merge-base topic branch).
Perhaps as a starter (not even compile tested)...
diff --git a/commit.c b/commit.c
index 0431027..23ac210 100644
--- a/commit.c
+++ b/commit.c
@@ -397,13 +397,13 @@ void clear_commit_marks(struct commit *c
{
struct commit_list *parents;
- parents = commit->parents;
+ if (!commit)
+ return;
commit->object.flags &= ~mark;
+ parents = commit->parents;
while (parents) {
struct commit *parent = parents->item;
- if (parent && parent->object.parsed &&
- (parent->object.flags & mark))
- clear_commit_marks(parent, mark);
+ clear_commit_marks(parent, mark);
parents = parents->next;
}
}
@@ -1012,7 +1012,8 @@ static void mark_reachable_commits(struc
}
}
-struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2)
+struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2,
+ int cleanup_needed)
{
struct commit_list *list = NULL;
struct commit_list *result = NULL;
@@ -1081,8 +1082,10 @@ struct commit_list *get_merge_bases(stru
}
/* reset flags */
- clear_commit_marks(rev1, PARENT1 | PARENT2 | UNINTERESTING);
- clear_commit_marks(rev2, PARENT1 | PARENT2 | UNINTERESTING);
+ if (cleanup_needed) {
+ clear_commit_marks(rev1, PARENT1 | PARENT2 | UNINTERESTING);
+ clear_commit_marks(rev2, PARENT1 | PARENT2 | UNINTERESTING);
+ }
return result;
}
diff --git a/commit.h b/commit.h
index 89b9dad..53bc697 100644
--- a/commit.h
+++ b/commit.h
@@ -105,6 +105,6 @@ struct commit_graft *read_graft_line(cha
int register_commit_graft(struct commit_graft *, int);
int read_graft_file(const char *graft_file);
-extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2);
+extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2, int cleanup_needed);
#endif /* COMMIT_H */
diff --git a/merge-base.c b/merge-base.c
index b41f76c..59f723f 100644
--- a/merge-base.c
+++ b/merge-base.c
@@ -6,7 +6,7 @@ static int show_all = 0;
static int merge_base(struct commit *rev1, struct commit *rev2)
{
- struct commit_list *result = get_merge_bases(rev1, rev2);
+ struct commit_list *result = get_merge_bases(rev1, rev2, 0);
if (!result)
return 1;
^ permalink raw reply related
* Re: A note on merging conflicts..
From: Junio C Hamano @ 2006-07-01 20:07 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0607011301480.12404@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> On Sat, 1 Jul 2006, Rene Scharfe wrote:
>>
>> I wonder why the two clear_commit_marks() calls at the end of
>> get_merge_bases() are not sufficient, though.
>
> Why does that thing check for "parent->object.parsed"?
Oh, I thought I fixed that up when I merged. Sorry.
> Also, it only clears commit marks if they are contiguous, but some commit
> marking may not be dense (eg, the "UNINTERESTING" mark may have been set
> by (PARENT1 && PARENT2) triggering, but is not set in the commits that
> reach it.
That is true.
^ permalink raw reply
* Re: A note on merging conflicts..
From: Linus Torvalds @ 2006-07-01 20:04 UTC (permalink / raw)
To: Rene Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <44A6CD1D.2000600@lsrfire.ath.cx>
On Sat, 1 Jul 2006, Rene Scharfe wrote:
>
> I wonder why the two clear_commit_marks() calls at the end of
> get_merge_bases() are not sufficient, though.
Why does that thing check for "parent->object.parsed"?
Also, it only clears commit marks if they are contiguous, but some commit
marking may not be dense (eg, the "UNINTERESTING" mark may have been set
by (PARENT1 && PARENT2) triggering, but is not set in the commits that
reach it.
Linus
^ permalink raw reply
* Re: A note on merging conflicts..
From: Junio C Hamano @ 2006-07-01 19:56 UTC (permalink / raw)
To: Rene Scharfe; +Cc: git
In-Reply-To: <44A6CD1D.2000600@lsrfire.ath.cx>
Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> I wonder why the two clear_commit_marks() calls at the end of
> get_merge_bases() are not sufficient, though.
I missed to notice that Johannes had added those calls there; we
should remove them from get_merge_bases().
The normal case of git-merge-base calling get_merge_bases() once
and exiting should NOT have to pay for the clean-up cost at all.
^ permalink raw reply
* Re: [PATCH] Enable tree (directory) history display
From: Luben Tuikov @ 2006-07-01 19:38 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0606301954140.12404@g5.osdl.org>
--- Linus Torvalds <torvalds@osdl.org> wrote:
> Well, with history simplification, we still show merges that are required
> to make the history _complete_, ie say that you had
>
> a
> |
> b
> / \
> c d
> | |
>
> and neither "a" nor "b" actually changed the file, but both "c" and "d"
> did: in this case we have to leave "b" around just because otherwise there
> would be no way to show the _relationship_, even if "b" itself doesn't
> actually change the tree in any way what-so-ever.
I agree. If "c" and/or "d" changed the file but neither "a" nor "b" did,
then by (merge/diff/etc) "inheritance" we do need to leave "b" around.
(This is similar to git-blame/git-annotate which should show "b", so that
we can track down the change to "c" and/or "d".)
> > Can you consider the default case to be simplify_history=1,
> > which is currently the default behaviour of git-rev-list.
>
> Actually, for your case, you don't want _any_ merges, unless those merges
> literally changed the tree from all of the parents.
Yes, that's true. s/all/one or more:
Don't want to show a merge, unless one or more of the parents,
changed the file. If no parent changed the tree, then do not
show the commit.
> I think it would make sense to make that further simplification if the
> "--parents" flag wasn't present.
>
> Hmm. Maybe something like this..
>
> BTW! Junio, I think this patch actually fixes a real bug.
>
> Without this patch, the "--parents --full-history" combination (which
> you'd get if you do something like
>
> gitk --full-history Makefile
>
> or similar) will actually _drop_ merges where all children are identical.
> That's wrong in the --full-history case, because it measn that the graph
> ends up missing lots of entries.
>
> In the process, this also should make
>
> git-rev-list --full-history Makefile
>
> give just the _true_ list of all commits that changed Makefile (and
> properly ignore merges that were identical in one parent), because now
> we're not asking for "--parent", so we don't need the unnecessary merge
> commits to keep the history together.
>
> Luben, does this fix the problem for you?
Given Junio's analysis, and briefly looking at the logic, it does seem
correct. Let me apply it and see what I get, but I think it is a good thing.
Thanks for the patch!
Luben
^ permalink raw reply
* Re: A note on merging conflicts..
From: Rene Scharfe @ 2006-07-01 19:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <7vfyhldvd2.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano schrieb:
> I suspect this has the same problem I pointed out to Kristian's
> attempt to make git-branch a built-in.
>
> Subject: Re: [PATCH] Implement git-branch and git-merge-base as
> built-ins. Date: Thu, 08 Jun 2006 11:53:48 -0700 Message-ID:
> <7vverbsclf.fsf@assigned-by-dhcp.cox.net>
>
> Namely, merge-base code is not set up to be called more than once
> without cleaning things up.
Eek! This is not a nice interface. Your example IDs from the your mail
to Kristian:
$ ./git-rev-list 89719209...262a6ef7 66ae0c77...ced9456a | wc
75 75 3075
$ git-rev-list 89719209 262a6ef7 \
--not $(git-merge-base --all 89719209 262a6ef7) \
--not 66ae0c77 ced9456a \
--not $(git-merge-base --all 66ae0c77 ced9456a) | wc
75 75 3075
$ ./git-rev-list 66ae0c77...ced9456a 89719209...262a6ef7 | wc
76 76 3116
$ git-rev-list 66ae0c77 ced9456a \
--not $(git-merge-base --all 66ae0c77 ced9456a) \
--not 89719209 262a6ef7 \
--not $(git-merge-base --all 89719209 262a6ef7) | wc
75 75 3075
Yep, that doesn't seem right. The additional line is 262a6ef (which is
the merge base for 66ae0c77 and ced9456a btw.). The other 4x 75 lines
match.
I wonder why the two clear_commit_marks() calls at the end of
get_merge_bases() are not sufficient, though.
René
^ permalink raw reply
* Re: A note on merging conflicts..
From: Linus Torvalds @ 2006-07-01 18:52 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <e86ega$gnc$1@sea.gmane.org>
On Sat, 1 Jul 2006, Jakub Narebski wrote:
>
> Caret is used twice, with different meaning. As prefix operator "^" means
> "exclude lineage of commit" (while commit without "^" in front means:
> "include lineage of commit and commit itself"). BTW. why we don't use '!'
> for that?
Using '!' is really nasty with most shells. Avoid, avoid, avoid.
It would be more sensible to use ~ (mathematical negation), but that also
has magic meaning for shell at the beginning of a word..
Linus
^ permalink raw reply
* Re: [PATCH 1/3] Add read_cache_from() and discard_cache()
From: Junio C Hamano @ 2006-07-01 18:51 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0607011657460.29667@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> True, I missed that one. But it is just a call to
> cache_tree_free(active_cache_tree); in discard_cache(), right?
On the codepath to write out the new index file, calling
cache_free_tree(&active_cache_tree) before write_cache() is all
that should be needed. When "active_cache_tree == NULL",
write_cache() would write out an index file without the cached
tree information.
Currently not many things take advantage of cached tree
information to optimize its operation. But I'd like to change
that. For example, tree merges by read-tree should be able to
take advantage of the fact that a cached tree read from the
index and three trees being read all match for a subdirectory
and do the merge of the directory without descending into it.
>> - index_timestamp is left as the old value in this patch when
>> you switch cache using read_cache_from() directly. I have a
>> suspicion you may be bitten by "Racy Git" problem, especially
>> because the operations are supposed to happen quickly thanks
>> to the effort of you two ;-) increasing the risks that the
>> file timestamp of the working tree file and the cached entry
>> match.
>
> Yes. Again, just one line to discard_cache(), right?
>
> index_file_timestamp = 0;
This one I am not sure. Read the comment in ce_match_stat() and
see the problematic sequence of events that this variable tries
to help resolve applies to your use.
^ permalink raw reply
* Re: A note on merging conflicts..
From: Junio C Hamano @ 2006-07-01 18:37 UTC (permalink / raw)
To: Rene Scharfe; +Cc: git, Linus Torvalds
In-Reply-To: <20060701150926.GA25800@lsrfire.ath.cx>
I suspect this has the same problem I pointed out to Kristian's
attempt to make git-branch a built-in.
Subject: Re: [PATCH] Implement git-branch and git-merge-base as built-ins.
Date: Thu, 08 Jun 2006 11:53:48 -0700
Message-ID: <7vverbsclf.fsf@assigned-by-dhcp.cox.net>
Namely, merge-base code is not set up to be called more than
once without cleaning things up.
^ permalink raw reply
* Re: A note on merging conflicts..
From: Jakub Narebski @ 2006-07-01 18:22 UTC (permalink / raw)
To: git
In-Reply-To: <20060701180125.GA27550@fieldses.org>
J. Bruce Fields wrote:
> On Sat, Jul 01, 2006 at 05:09:26PM +0200, Rene Scharfe wrote:
>> +Another special notation is <commit1>...<commit2> which is useful for
>> +merges. The resulting set of commits is the symmetric difference
>> +between the two operands. The following two commands are equivalent:
>
> What's the logic behind naming the operator "..."?
>
> Seems like asking for trouble to have two visually similar operators (".." and
> "...") with different meanings, and "..." seems like kind of an arbitrary
> choice anyway.
Because A...B is extension of A..B for merges.
> A symmetric difference is basically equivalent to an xor--would a carat ("^")
> work? Or could we just stick a word there instead of using some tricky
> notation?
Caret is used twice, with different meaning. As prefix operator "^" means
"exclude lineage of commit" (while commit without "^" in front means:
"include lineage of commit and commit itself"). BTW. why we don't use '!'
for that?
As postfix operator "^" means "dereference", i.e. parent in the case
of commit; allows choosing a parent (commit^n) and listing all parents
(commit^@). Using it as binary infix operator that would be I think
too much.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: A note on merging conflicts..
From: Linus Torvalds @ 2006-07-01 18:20 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Rene Scharfe, Junio C Hamano, git
In-Reply-To: <20060701180125.GA27550@fieldses.org>
On Sat, 1 Jul 2006, J. Bruce Fields wrote:
>
> What's the logic behind naming the operator "..."?
Well, if ".." is set difference, why not "..." for symmetric set
difference.
The operations really _are_ related.
Also, the parse syntax and logic really is the same, even if Rene's patch
didn't take advantage of that.
That said, it does have a real downside, and that's simply that it can
take a long time to compute.
Somebody should also verify that there are no interesting interaction with
the fact that we end up traversing the commit lists twice (no object flag
interactions etc) with the new "get_merge_bases()"
Linus
^ permalink raw reply
* Re: A note on merging conflicts..
From: Rene Scharfe @ 2006-07-01 18:13 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0607010921450.12404@g5.osdl.org>
On Sat, Jul 01, 2006 at 09:25:43AM -0700, Linus Torvalds wrote:
>
>
> On Sat, 1 Jul 2006, Rene Scharfe wrote:
> >
> > You mean something like the following patch on top of the 'next' branch?
> > It also documents the --not switch because I needed it for the example.
>
> Yes.
>
> However, I think that 90% of the code for the ".." and "..." case are the
> same, as is largely the finding of it.
>
> So why not just do this all inside the already existing
>
> dotdot = strstr(arg, "..");
> if (dotdot) {
> unsigned char other_sha1[20];
> const char *one = arg;
> const char *two = arg + 2;
> int symmetric = *two == '.';
>
> *dotdot = '\0';
> two += symmetric;
>
> if (one == arg)
> one = "HEAD";
> if (!*two)
> two = "HEAD";
> ...
>
> because the only difference is really at the very end.
Hrm, I'm not sure this is really cleaner. The two operators consist
of all dots only coincidentally, this is not functionally inherent.
So I think it's better to keep them apart.
Let's see.. [Time passes. A patch materializes at six o'clock.]
With a little helper factored out this doesn't look as bad as I
imagined. Maybe we can take it. What do you think?
> Did you test that it looks correct too?
Sort of; I checked that the two forms (with ... and $(git-merge-base))
gave the same results for 7b8cf0cf and 51d1e83f, that's all. For a
proper test script we'd need to create a repo for which git-merge-base
can report multiple results. I wasn't able to come up with the needed
commands without thinking and gave up for now. Am working on it..
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index ad6d14c..6c370e1 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[ \--sparse ]
[ \--no-merges ]
[ \--remove-empty ]
+ [ \--not ]
[ \--all ]
[ \--topo-order ]
[ \--parents ]
@@ -37,6 +38,14 @@ not in 'baz'".
A special notation <commit1>..<commit2> can be used as a
short-hand for {caret}<commit1> <commit2>.
+Another special notation is <commit1>...<commit2> which is useful for
+merges. The resulting set of commits is the symmetric difference
+between the two operands. The following two commands are equivalent:
+
+------------
+$ git-rev-list A B --not $(git-merge-base --all A B)
+$ git-rev-list A...B
+------------
OPTIONS
-------
@@ -93,6 +102,11 @@ OPTIONS
--remove-empty::
Stop when a given path disappears from the tree.
+--not::
+ Reverses the meaning of the '{caret}' prefix (or lack
+ thereof) for all following revision specifiers, up to
+ the next `--not`.
+
--all::
Pretend as if all the refs in `$GIT_DIR/refs/` are
listed on the command line as <commit>.
diff --git a/revision.c b/revision.c
index ae4ca82..bcedf66 100644
--- a/revision.c
+++ b/revision.c
@@ -536,6 +536,18 @@ void init_revisions(struct rev_info *rev
diff_setup(&revs->diffopt);
}
+static void add_pending_commit_list(struct rev_info *revs,
+ struct commit_list *commit_list,
+ unsigned int flags)
+{
+ while (commit_list) {
+ struct object *object = &commit_list->item->object;
+ object->flags |= flags;
+ add_pending_object(revs, object, sha1_to_hex(object->sha1));
+ commit_list = commit_list->next;
+ }
+}
+
/*
* Parse revision information, filling in the "rev_info" structure,
* and removing the used arguments from the argument list.
@@ -771,27 +783,45 @@ int setup_revisions(int argc, const char
unsigned char from_sha1[20];
const char *next = dotdot + 2;
const char *this = arg;
+ int symmetric = *next == '.';
+ unsigned int flags_exclude = flags ^ UNINTERESTING;
+
*dotdot = 0;
+ next += symmetric;
+
if (!*next)
next = "HEAD";
if (dotdot == arg)
this = "HEAD";
if (!get_sha1(this, from_sha1) &&
!get_sha1(next, sha1)) {
- struct object *exclude;
- struct object *include;
-
- exclude = get_reference(revs, this, from_sha1, flags ^ UNINTERESTING);
- include = get_reference(revs, next, sha1, flags);
- if (!exclude || !include)
- die("Invalid revision range %s..%s", arg, next);
+ struct commit *a, *b;
+ struct commit_list *exclude;
+
+ a = lookup_commit_reference(from_sha1);
+ b = lookup_commit_reference(sha1);
+ if (!a || !b) {
+ die(symmetric ?
+ "Invalid symmetric difference expression %s...%s" :
+ "Invalid revision range %s..%s",
+ arg, next);
+ }
if (!seen_dashdash) {
*dotdot = '.';
verify_non_filename(revs->prefix, arg);
}
- add_pending_object(revs, exclude, this);
- add_pending_object(revs, include, next);
+
+ if (symmetric) {
+ exclude = get_merge_bases(a, b);
+ add_pending_commit_list(revs, exclude,
+ flags_exclude);
+ a->object.flags |= flags;
+ } else
+ a->object.flags |= flags_exclude;
+ b->object.flags |= flags;
+ add_pending_object(revs, &a->object, this);
+ add_pending_object(revs, &b->object, next);
continue;
}
*dotdot = '.';
^ permalink raw reply related
* Re: A note on merging conflicts..
From: J. Bruce Fields @ 2006-07-01 18:01 UTC (permalink / raw)
To: Rene Scharfe; +Cc: Linus Torvalds, Junio C Hamano, git
In-Reply-To: <20060701150926.GA25800@lsrfire.ath.cx>
On Sat, Jul 01, 2006 at 05:09:26PM +0200, Rene Scharfe wrote:
> +Another special notation is <commit1>...<commit2> which is useful for
> +merges. The resulting set of commits is the symmetric difference
> +between the two operands. The following two commands are equivalent:
What's the logic behind naming the operator "..."?
Seems like asking for trouble to have two visually similar operators (".." and
"...") with different meanings, and "..." seems like kind of an arbitrary
choice anyway.
A symmetric difference is basically equivalent to an xor--would a carat ("^")
work? Or could we just stick a word there instead of using some tricky
notation?
--b.
^ permalink raw reply
* Re: [RFC/PATCH 14] autoconf: Added --with/--without for openssl, curl, expat to ./configure
From: Jakub Narebski @ 2006-07-01 17:55 UTC (permalink / raw)
To: Pavel Roskin, git
In-Reply-To: <20060630233004.7xckw444g4g0gcs8@webmail.spamcop.net>
Pavel Roskin wrote:
> Hi Jakub,
>
> you lost cc: but please feel free to return to the list.
Sending reply to GMane newsgroup tied to mailing list, and via email
to author, but _not_ via mail to mailing list would do that...
especially if the author I'm replying to doesn't use newsgroup
interface.
> Quoting Jakub Narebski <jnareb@gmail.com>:
>> I suspect that AS_HELP_WITH does some strange quoting, or stripping. Both
>> [=PATH] and [[=PATH]] produces =PATH in ./configure --help output.
>> When using @<:@=PATH@:>@ I get [=PATH], but the description of option begins
>> line below.
>
> If you are not following quoting rules, every macro can do strange things!
Well, [=PATH] or [[=PATH]] doesn't work even if GIT_APPEND_LINE is without
double quotes. Besides, that doesn't matter because this is inside of
AS_HELP_STRING macro. No combination of quoting (I think I tried all)
works... I guess I would just not use AS_HELP_STRING, and format help
message "by hand".
>> Any ideas for name of MY_APPEND_LINE(LINE)/GIT_APPEND_LINE(LINE) macro,
>> which as a result adds line to output (e.g. LINE = "NO_CURL=YesPlease")?
>
> GIT_LIB_CURL
>
> Generally, please try to avoid negations. They are confising to the end users.
> Lack of curl may be an anomaly to git developers, but it is not an anomaly for a
> user who has never heard of curl. If you can, try to use positive logic, like
> CURL=1, and translate it to negative logic only as a temporary solution and far
> away from the user's eyes.
I'm following example set by main Makefile. That, and I tried to make configure.ac
as simple as possible...
By the way, if you know autoconf well, perhaps you could tell me how to write
tests for the following programs: ar, tar, rpmbuild, how to write test for
Python version (or rather for WITH_OWN_SUBPROCESS_PY) and other test autoconf.ac
lacks now (NEEDS_SSL_WITH_CRYPTO, NEEDS_LIBICONV, NEEDS_SOCKET, NO_MMAP,
NO_IPV6, NO_ICONV, NO_ACCURATE_DIFF unless that was removed or changed name).
--
Jakub Narebski
ShadeHawk on #git
^ permalink raw reply
* Re: A note on merging conflicts..
From: Linus Torvalds @ 2006-07-01 16:25 UTC (permalink / raw)
To: Rene Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <20060701150926.GA25800@lsrfire.ath.cx>
On Sat, 1 Jul 2006, Rene Scharfe wrote:
>
> You mean something like the following patch on top of the 'next' branch?
> It also documents the --not switch because I needed it for the example.
Yes.
However, I think that 90% of the code for the ".." and "..." case are the
same, as is largely the finding of it.
So why not just do this all inside the already existing
dotdot = strstr(arg, "..");
if (dotdot) {
unsigned char other_sha1[20];
const char *one = arg;
const char *two = arg + 2;
int symmetric = *two == '.';
*dotdot = '\0';
two += symmetric;
if (one == arg)
one = "HEAD";
if (!*two)
two = "HEAD";
...
because the only difference is really at the very end.
Did you test that it looks correct too?
Linus
^ permalink raw reply
* Re: A note on merging conflicts..
From: Johannes Schindelin @ 2006-07-01 15:23 UTC (permalink / raw)
To: Rene Scharfe; +Cc: Linus Torvalds, Junio C Hamano, git
In-Reply-To: <20060701150926.GA25800@lsrfire.ath.cx>
Hi,
On Sat, 1 Jul 2006, Rene Scharfe wrote:
> + exclude = get_merge_bases(a, b);
Aaah! Junio, Linus, I see the light now.
Ciao,
Dscho
^ permalink raw reply
* Re: A note on merging conflicts..
From: Rene Scharfe @ 2006-07-01 15:09 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0606302046230.12404@g5.osdl.org>
On Fri, Jun 30, 2006 at 08:54:33PM -0700, Linus Torvalds wrote:
> Now, the expression
>
> A...B == B...A == A B --not $(git-merge-base --all A B)
>
> is meaningful (and the one I want for merges), but it's largely useless
> for anything else. It just means "the set of all commits that aren't
> trivially in both" (it's not strictly a valid set operation, but it
> approaches being an "xor" instead of a union or an intersection or a
> difference).
You mean something like the following patch on top of the 'next' branch?
It also documents the --not switch because I needed it for the example.
TODO: There are still a few undocumented options left and setup_revisions()
is fat and ugly. Any volunteers? I'd clean it up if I only could
write comprehensible documentation and wasn't that lazy..
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index ad6d14c..ffbf0c9 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[ \--sparse ]
[ \--no-merges ]
[ \--remove-empty ]
+ [ \--not ]
[ \--all ]
[ \--topo-order ]
[ \--parents ]
@@ -37,6 +38,14 @@ not in 'baz'".
A special notation <commit1>..<commit2> can be used as a
short-hand for {caret}<commit1> <commit2>.
+Another special notation is <commit1>...<commit2> which is useful for
+merges. The resulting set of commits is the symmetric difference
+between the two operands. The following two commands are equivalent:
+
+------------
+$ git-rev-list A B --not $(git-merge-base --all A B)
+$ git-rev-list A...B
+------------
OPTIONS
-------
@@ -93,6 +102,11 @@ OPTIONS
--remove-empty::
Stop when a given path disappears from the tree.
+--not::
+ Reverses the meaning of the '{caret}' prefix (or lack
+ thereof) for all following revision specifiers, up to
+ the next `--not`.
+
--all::
Pretend as if all the refs in `$GIT_DIR/refs/` are
listed on the command line as <commit>.
diff --git a/revision.c b/revision.c
index ae4ca82..d4224a1 100644
--- a/revision.c
+++ b/revision.c
@@ -766,6 +766,47 @@ int setup_revisions(int argc, const char
left++;
continue;
}
+ dotdot = strstr(arg, "...");
+ if (dotdot) {
+ unsigned char other_sha1[20];
+ const char *one = arg;
+ const char *two = dotdot + 3;
+ *dotdot = '\0';
+ if (dotdot == arg)
+ one = "HEAD";
+ if (!*two)
+ two = "HEAD";
+ if (!get_sha1(one, sha1) &&
+ !get_sha1(two, other_sha1)) {
+ struct commit *a, *b;
+ struct commit_list *exclude;
+
+ a = lookup_commit_reference(sha1);
+ b = lookup_commit_reference(other_sha1);
+ if (!a || !b)
+ die("Invalid symmetric difference expression %s...%s", arg, two);
+
+ if (!seen_dashdash) {
+ *dotdot = '.';
+ verify_non_filename(revs->prefix, arg);
+
+ }
+ exclude = get_merge_bases(a, b);
+ while (exclude) {
+ struct object *object =
+ &exclude->item->object;
+ object->flags |= flags ^ UNINTERESTING;
+ add_pending_object(revs, object, sha1_to_hex(object->sha1));
+ exclude = exclude->next;
+ }
+ a->object.flags |= flags;
+ add_pending_object(revs, &a->object, one);
+ b->object.flags |= flags;
+ add_pending_object(revs, &b->object, two);
+ continue;
+ }
+ *dotdot = '.';
+ }
dotdot = strstr(arg, "..");
if (dotdot) {
unsigned char from_sha1[20];
^ permalink raw reply related
* Re: [PATCH 1/3] Add read_cache_from() and discard_cache()
From: Johannes Schindelin @ 2006-07-01 15:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Alex Riesen
In-Reply-To: <7v3bdmk2zj.fsf@assigned-by-dhcp.cox.net>
Hi,
On Fri, 30 Jun 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > +int discard_cache()
> > +{
> > + int ret;
> > +
> > + if (cache_mmap == NULL)
> > + return 0;
> > + ret = munmap(cache_mmap, cache_mmap_size);
> > + cache_mmap = NULL;
> > + cache_mmap_size = 0;
> > + active_nr = active_cache_changed = 0;
> > + /* no need to throw away allocated active_cache */
> > + return ret;
> > +}
> > +
>
> I haven't been following the details of the patches in this
> thread while they are being cooked actively, but two things to
> look out for are:
>
> - I am guessing you run discard_cache() because you want to
> read in a new cache (or start from a clean slate). I am not
> sure what you are doing with the old cache tree data
> structure. If you are starting from a clean slate
> (i.e. there is no read_cache_from() after you call
> discard_cache), you would probably need to discard the old
> cache tree; otherwise your next write-tree may produce an
> incorrect index file. If you keep the old one and later
> swap it in, the problem might be even more severe.
True, I missed that one. But it is just a call to
cache_tree_free(active_cache_tree); in discard_cache(), right?
> - index_timestamp is left as the old value in this patch when
> you switch cache using read_cache_from() directly. I have a
> suspicion you may be bitten by "Racy Git" problem, especially
> because the operations are supposed to happen quickly thanks
> to the effort of you two ;-) increasing the risks that the
> file timestamp of the working tree file and the cached entry
> match.
Yes. Again, just one line to discard_cache(), right?
index_file_timestamp = 0;
If there is more to it, please don't let me die dumb.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] autoconf: Use autoconf to write installation directories to config.mak
From: Jakub Narebski @ 2006-07-01 13:58 UTC (permalink / raw)
To: git
In-Reply-To: <44A51693.5020501@op5.se>
Andreas Ericsson wrote:
> This is bad, since it forces users to do one thing first and then do
> what they're used to. Better to have the script add
>
> -include config.mak.autogen
>
> LAST in config.mak, unless it's already in the file and generate
> config.mak.autogen with configure.
>
> Since Make does things bottoms-up (much like swedish students and
> midsummer celebrators), the previous hand-edited defaults in config.mak
> will beat the ones in config.mak.autogen (a good thing).
That's not true, unless we use '?=' assignment in Makefile. And we _want_ to
override defaults provided in main git Makefile, so in config.mak.autoconf,
and probably in user's own config.mak we use '=' overriding assignment. So
it would be better to just add "-include config.mak.autogen" to makefile
before "-include config.mak", as in patch below.
diff --git a/Makefile b/Makefile
index ccd7c62..a37d400 100644
--- a/Makefile
+++ b/Makefile
@@ -333,6 +333,7 @@ ifneq (,$(findstring arm,$(uname_M)))
ARM_SHA1 = YesPlease
endif
+-include config.mak.autogen
-include config.mak
ifdef WITH_OWN_SUBPROCESS_PY
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply related
* git-send-email: mail send at 1st december 2005
From: Bertrand Jacquin @ 2006-07-01 13:41 UTC (permalink / raw)
To: git
Hi,
Since git 1.4.0 all mail send with git-send-email have a date of
01/12/2005. Is it a known problem ?
Thanks
--
# Beber : beber@gna.org
# IM : beber@jabber.fr
# http://guybrush.ath.cx, irc://irc.freenode.net/#{e.fr,gentoofr}
^ permalink raw reply
* Re: [RFC] gitweb wishlist and TODO list
From: Paul Mackerras @ 2006-07-01 10:35 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Jakub Narebski, git
In-Reply-To: <46a038f90606201417k71c4c43ak59204774bcfe8246@mail.gmail.com>
Martin Langhoff writes:
> > If I remember correctly, it was done in the background, and it was done
> > at least partially _in_ gitk (Tcl/Tk).
>
> I suspect it is doing a whole lot of git-merge-base invocations, which
> are rather costly. I don't know of any cheaper way to ask that
> question.
There's no git-merge-base involved. Gitk does a
git-rev-list --all --topo-order --parents
and reads the output of that, and then traverses the entire graph
forwards and backwards (in Tcl). (This is after gitk has read the
output of git ls-remote $GIT_DIR, so it knows which commits have
tags.)
Paul.
^ permalink raw reply
* Re: Problem with GITK
From: Paul Mackerras @ 2006-07-01 10:49 UTC (permalink / raw)
To: Paolo Ciarrocchi; +Cc: Git Mailing List
In-Reply-To: <4d8e3fd30606281340n147821e2hcbd4ccaf9551173f@mail.gmail.com>
Paolo Ciarrocchi writes:
> there is a strange orange line in the following screenshot from gitk:
> http://paolo.ciarrocchi.googlepages.com/Screenshotgit.png
That's an X server bug, and just today I found it. The bug
description and proposed patch are at:
https://bugs.freedesktop.org/show_bug.cgi?id=7381
Paul.
^ permalink raw reply
* [PATCH] git-svn: allow a local target directory to be specified for init
From: Eric Wong @ 2006-07-01 4:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Luca Barbato, Eric Wong
Original patch by Luca Barbato, cleaned up and made to work for
the current version of git-svn by me (Eric Wong).
Luca Barbato <lu_zero@gentoo.org> wrote:
> Since I'm lazy I just hacked a bit git-svn in order to create a target
> dir and init it if is passed as second parameter.
>
> git-svn init url://to/the/repo local-repo
>
> will create the local-repo dir if doesn't exist yet and populate it as
> expected.
>
> Maybe someone else could find it useful
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
contrib/git-svn/git-svn.perl | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/contrib/git-svn/git-svn.perl b/contrib/git-svn/git-svn.perl
index b3d3f47..1e19aa1 100755
--- a/contrib/git-svn/git-svn.perl
+++ b/contrib/git-svn/git-svn.perl
@@ -264,9 +264,19 @@ when you have upgraded your tools and ha
}
sub init {
- $SVN_URL = shift or die "SVN repository location required " .
+ my $url = shift or die "SVN repository location required " .
"as a command-line argument\n";
- $SVN_URL =~ s!/+$!!; # strip trailing slash
+ $url =~ s!/+$!!; # strip trailing slash
+
+ if (my $repo_path = shift) {
+ unless (-d $repo_path) {
+ mkpath([$repo_path]);
+ }
+ $GIT_DIR = $ENV{GIT_DIR} = $repo_path . "/.git";
+ init_vars();
+ }
+
+ $SVN_URL = $url;
unless (-d $GIT_DIR) {
my @init_db = ('git-init-db');
push @init_db, "--template=$_template" if defined $_template;
--
1.4.1.rc1.g7fe3
^ permalink raw reply related
* Re: A note on merging conflicts..
From: Linus Torvalds @ 2006-07-01 3:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0606302046230.12404@g5.osdl.org>
On Fri, 30 Jun 2006, Linus Torvalds wrote:
>
> (it's not strictly a valid set operation, but it approaches being an
> "xor" instead of a union or an intersection or a difference).
Oh, I guess it _is_ perfectly valid. It's called a "symmetric difference"
in set theory.
So from a set standpoint:
Git op: Set theory:
git-rev-list a..b // difference: B - A
git-rev-list b..a // difference: A - B
git-rev-list a b // union of A B (order doesn't matter)
git-rev-list a...b // symmetric difference A B (order doesn't matter)
git-rev-list $(git-merge-base --all a b)
// intersection of A and B
I think.
Linus
^ 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