* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Larry D'Anna @ 2010-02-10 5:55 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20100210053937.GF28526@coredump.intra.peff.net>
* Jeff King (peff@peff.net) [100210 00:41]:
> I have not actually been running these patches, just reading them, but
> my impression was the goal _was_ to squelch all of the stderr cruft. But
> if we are not even close, then probably we should just give up and
> callers should "2>/dev/null".
Personally, I don't really care about squelching stderr cruft. All I really
want is for what goes to stdout be sane and the calling script to be able to
unambiguously figure out what happened, including
* which refs go to which remotes
* whether or not some mysterious error occurred (beyond those mentioned in the
ref status lines)
> I had initially endorsed it, but now I am having second thoughts.
> Especially if the "usual" calling convention is to redirect stderr as
> above, then we are probably missing out on any useful error messages
> that accompany a failure return, anyway. So maybe the sane thing to do
> is to leave the exit code alone, and include a --porcelain output line
> that either says "Everything was OK, see individual ref status" or "We
> couldn't even talk to the other side". Then the status code is
> irrelevant, and stdout contains all of the useful information (and if
> you don't get an error or OK message, you know there was some
> serious error like a broken git installation).
That serves my purposes as well as the exit code would. Is this the consensus?
--larry
^ permalink raw reply
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Jeff King @ 2010-02-10 5:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Larry D'Anna, git
In-Reply-To: <7vmxzj8hca.fsf@alter.siamese.dyndns.org>
On Mon, Feb 08, 2010 at 02:59:01PM -0800, Junio C Hamano wrote:
> >> + if (flags & TRANSPORT_PUSH_PORCELAIN) {
> >> + /* Do not give advice messages to Porcelain scripts */
> >> + advice_push_nonfastforward = 0;
> >> + }
> >
> > I think this is sane.
>
> I am tempted to suggest adding "clear_advice(void)" in advice.[ch], so
> that people adding new advices do not have to hunt for even the above
> hunk. It would be a good direction to go in general _if_ we will have
> more like this --porcelain thing in other parts of the system.
>
> I didn't do so because that "_if_" is still iffy.
Would it make sense to clear _all_ advice if we are just in porcelain
mode for git-push? That is, let's say I am in "push --porcelain" and I
try to write a reflog entry for a local tracking ref, but my identity is
implicitly picked up from the hostname[1]. Should it trigger
advice_implicit_identity and say "by the way, your identity is implicit"
on stderr or not?
I would say yes. The advice output should all be on stderr, and the
porcelain output should all be on stdout. So there is no parsing
conflict. And stderr either goes to /dev/null (because the porcelain is
not terminal-based, or doesn't want the terminal screwed up), in which
case the advice does nothing, or stderr goes to the terminal (because
the porcelain is some simple script), in which case the message is
probably something the user would want to see.
-Peff
[1] I am actually not sure if you can trigger the implicit identity
advice in this way. But I am arguing from the perspective of "assume you
have triggered some random advice through some sub-action", which may
include advice to be added in the future.
^ permalink raw reply
* Re: [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain
From: Jeff King @ 2010-02-10 5:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Larry D'Anna, git
In-Reply-To: <7viqa7cqs9.fsf@alter.siamese.dyndns.org>
On Mon, Feb 08, 2010 at 02:21:26PM -0800, Junio C Hamano wrote:
> I think the purpose of the patches that started this thread was to admit
> that 1965ff7 (add --porcelain option to git-push, 2009-06-22) was not well
> thought out, and to break compatibility to fix it.
I thought it was simply to remove stdout cruft that had crept in, and at
the same time to remove some stderr cruft that was simply noise on the
terminal. That being said, I am in favor of fixing it even if it means a
slight compatibility breakage.
> Having said that, I would say that what 1965ff7 specified was only these
> two:
>
> = TAB refs/heads/master:refs/heads/master TAB [up to date]
> - TAB :refs/heads/foobar TAB [deleted]
>
> so everything else that do not match this pattern is a fair game, most
> importantly, the line that begins with "To" would not be mistaken with
> this pattern, I think.
It depends on the parser. If the parser was something like:
switch (line[0]) {
case '=': ...; break;
case '-': ...; break;
default: die("wtf: %s", line);
}
then we are not introducing any ambiguity, but we are causing a
breakage. The problem is that we did not specify the format anywhere, so
it is hard to say whether we are breaking any promises we made.
> > This one, on the other hand, seems to me to be just noise. What does a
> > --porcelain caller learn by seeing "Everything up-to-date" that it did
> > not already know from seeing the list of refs?
>
> I do not care too much about this hunk either way. We could leave it as
> is, as we will be giving some other stuff to the standard error stream
> without squelching anyway, even with the three-patch series. We could
> squelch only this message, but it is dubious what it is buying us. If you
> forced me to decide, I would probably say "let's just drop this hunk and
> keep the code as-is".
I have not actually been running these patches, just reading them, but
my impression was the goal _was_ to squelch all of the stderr cruft. But
if we are not even close, then probably we should just give up and
callers should "2>/dev/null".
> As to the exit status, do you have any thoughts, by the way?
>
> I am not convinced that it would be necessary nor even a good idea to make
> it behave inconsistently between the normal case and Porcelain case, only
> to make it easier to special case the "remote side would reject due to
> non-fast-forward" failure mode (iow, even if the calling script knows that
> it would fail due to non-fast-forward but otherwise everything else would
> be fine, what good would it do?)
I had initially endorsed it, but now I am having second thoughts.
Especially if the "usual" calling convention is to redirect stderr as
above, then we are probably missing out on any useful error messages
that accompany a failure return, anyway. So maybe the sane thing to do
is to leave the exit code alone, and include a --porcelain output line
that either says "Everything was OK, see individual ref status" or "We
couldn't even talk to the other side". Then the status code is
irrelevant, and stdout contains all of the useful information (and if
you don't get an error or OK message, you know there was some
serious error like a broken git installation).
-Peff
^ permalink raw reply
* Re: A generalization of git notes from blobs to trees - git metadata?
From: Jeff King @ 2010-02-10 5:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jon Seymour, Johan Herland, git
In-Reply-To: <7vvde5irzz.fsf@alter.siamese.dyndns.org>
On Tue, Feb 09, 2010 at 09:23:12PM -0800, Junio C Hamano wrote:
> > Hmm. OK, I see the point of Jakub's message a bit more now. You want to
> > create a new view, inconsistent with that of either Alice or Bob (that
> > is, you have taken snippets of each's state, but you cannot in good
> > faith represent this as a history merge, because your state should not
> > supersede either of theirs).
>
> In the message you are quoting, I am not interested in creating a narrowed
> view. If I cannot resolve conflicts between Alice and Bob in a merge in
> the contents space, I would ask either of them (because they are more
> familiar with the area) to do the merge. I however was unsure if asking
> the same for merges in the notes space is a reasonable thing to do.
No, I don't see a problem with asking them to do it. If you are all
collaborating as a group, it is something they will need to do
eventually anyway. If they are not, and you are an intermediary, you are
eventually going to share Alice's history with Bob and vice versa. So
you pull from Alice, then say to Bob: "I have some history but I'm not
sure of the correct merge. Pull from me and merge please". The only real
problem is if you _never_ want to share the history between the two of
them. In that case, I think you should keep two parallel branches of
history (refs/notes/alice and refs/notes/bob), and then squash the trees
at run-time (either concatenating them, or favoring one over the other
in the case of conflicts).
-Peff
^ permalink raw reply
* Re: A generalization of git notes from blobs to trees - git metadata?
From: Junio C Hamano @ 2010-02-10 5:23 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Jon Seymour, Johan Herland, git
In-Reply-To: <20100210050902.GD28526@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Feb 07, 2010 at 12:25:13PM -0800, Junio C Hamano wrote:
>
>> Suppose Alice, Bob and I are involved in a project, and we annotate
>> commits for some shared purpose (say, tracking regressions). Alice and
>> Bob may independently annotate overlapping set of commits (and hopefully
>> they have shared root for their notes history as they are collaborating),
>> and they may even be working together on the same issue, but I may not be
>> involved in the area. What happens when I pull from Alice and Bob and get
>> conflicts in notes they produced, especially the only reason I was
>> interested was because they have new things to say about commits that I am
>> interested in?
>
> Hmm. OK, I see the point of Jakub's message a bit more now. You want to
> create a new view, inconsistent with that of either Alice or Bob (that
> is, you have taken snippets of each's state, but you cannot in good
> faith represent this as a history merge, because your state should not
> supersede either of theirs).
In the message you are quoting, I am not interested in creating a narrowed
view. If I cannot resolve conflicts between Alice and Bob in a merge in
the contents space, I would ask either of them (because they are more
familiar with the area) to do the merge. I however was unsure if asking
the same for merges in the notes space is a reasonable thing to do.
^ permalink raw reply
* Re: A Visual Git Reference
From: Jeff King @ 2010-02-10 5:20 UTC (permalink / raw)
To: Mark Lodato; +Cc: git list
In-Reply-To: <ca433831002081134m698f531bwa22f0474db0cdcb@mail.gmail.com>
On Mon, Feb 08, 2010 at 02:34:21PM -0500, Mark Lodato wrote:
> I put together a "Visual Git Reference" containing visualizations of
> the most common git commands, for people who prefer to see images over
> text. It is designed as a reference, not a tutorial, so readers need
> to have some amount of experience before the page will become useful.
>
> URL: http://marklodato.github.com/visual-git-guide/
> Git repo: http://github.com/marklodato/visual-git-guide/
>
> If you have any feedback or suggestions, please let me know!
This looks really awesome, thanks for doing it (though I'll admit I
threw up in my mouth a little when I saw you did all of the diagrams as
TeX. ;) ).
One of the projects I have wanted to do but never found time for is a
"Git Picture Glossary". I was intending to start with much simpler
concepts, like how the various object types relate, what a ref is, how a
symref differs, what a detached HEAD is, etc. And then move on to "here
is what happens when you branch", "here is what happens when you merge",
etc. Sort of like "Git for Computer Scientists", but organized as a
glossary, with each entry starting off with "to understand this, you
first need to understand entries X and Y", with links.
And it seems like what you have done is more or less the same thing,
except you skipped all of the really basic entries and went straight to
the action-oriented ones.
-Peff
^ permalink raw reply
* [PATCH] Documentation: reword --thin description
From: Stephen Boyd @ 2010-02-10 5:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
Don't know if git-push needs to say that --thin is passed to send-pack.
Documentation/git-fetch-pack.txt | 4 ++--
Documentation/git-push.txt | 6 +++---
Documentation/git-send-pack.txt | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
index e9952e8..2b4e4dd 100644
--- a/Documentation/git-fetch-pack.txt
+++ b/Documentation/git-fetch-pack.txt
@@ -44,8 +44,8 @@ OPTIONS
locked against repacking.
--thin::
- Spend extra cycles to minimize the number of objects to be sent.
- Use it on slower connection.
+ Spend extra cycles minimizing the number of sent objects.
+ Use it with a slow connection.
--include-tag::
If the remote side supports it, annotated tags objects will
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index bd79119..a14bc9c 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -141,9 +141,9 @@ useful if you write an alias or script around 'git push'.
--thin::
--no-thin::
- These options are passed to 'git send-pack'. Thin
- transfer spends extra cycles to minimize the number of
- objects to be sent and meant to be used on slower connection.
+ These options are passed to 'git send-pack'. A thin
+ transfer spends extra cycles minimizing the number of
+ sent objects and is meant to be used with a slow connection.
-v::
--verbose::
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 8178d92..0a78dac 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -48,8 +48,8 @@ OPTIONS
Run verbosely.
--thin::
- Spend extra cycles to minimize the number of objects to be sent.
- Use it on slower connection.
+ Spend extra cycles minimizing the number of sent objects.
+ Use it with a slow connection.
<host>::
A remote host to house the repository. When this
--
1.7.0.rc2
^ permalink raw reply related
* Re: A generalization of git notes from blobs to trees - git metadata?
From: Jeff King @ 2010-02-10 5:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jon Seymour, Johan Herland, git
In-Reply-To: <7v8wb4aj4m.fsf@alter.siamese.dyndns.org>
On Sun, Feb 07, 2010 at 12:25:13PM -0800, Junio C Hamano wrote:
> Suppose Alice, Bob and I are involved in a project, and we annotate
> commits for some shared purpose (say, tracking regressions). Alice and
> Bob may independently annotate overlapping set of commits (and hopefully
> they have shared root for their notes history as they are collaborating),
> and they may even be working together on the same issue, but I may not be
> involved in the area. What happens when I pull from Alice and Bob and get
> conflicts in notes they produced, especially the only reason I was
> interested was because they have new things to say about commits that I am
> interested in?
Hmm. OK, I see the point of Jakub's message a bit more now. You want to
create a new view, inconsistent with that of either Alice or Bob (that
is, you have taken snippets of each's state, but you cannot in good
faith represent this as a history merge, because your state should not
supersede either of theirs).
The standard way to do such a thing in git is to create a new, alternate
history through cherry-picking or rebasing. So I suspect we could do
something like:
1. git notes pull alice
We fast-forward (or do the trivial merge) with Alice's work.
2. git notes pull --ignore-conflicts bob
We try to merge Bob's work and see that there are conflicts. So we
iterate through refs/notes..bob/notes, cherry-picking each one that
applies cleanly and ignoring the rest.
And then you're at a state inconsistent with Bob, and a superset of what
Alice has. And that's what your history represents, too: you've branched
but done some of the same things as Bob. At that point you can examine
your inconsistent state, and then when you're done, you can either:
3a. Reset back to your pre-ignore-conflicts state.
3b. Leave it. When you pull from Bob later, your shared changes will
be ignored[1], and you will get the conflicts that you ignored
earlier.
It is perhaps a hacky band-aid to handle notes this way, but it is the
"most git" way of doing it. That is, it uses our standard tools and
practices. And when all you have is a hammer... :) And I really expect
the "I am collaborating with these people, but I want an inconsistent
view of their history" to be the exception. Most people would _want_ to
resolve the conflicts (especially if there is a --cat-conflicts
option to do it automatically) in a collaboration scenario.
-Peff
[1] Actually because history has diverged, you have the usual cherry
pick problems with merging later. If some note is at state A, then I
cherry-pick Bob's change to B, then Bob changes it to C and I try to
merge with him, from the 3-way merge's perspective we have a conflict,
because nothing in the history says that Bob's change to C meant to
supersede my cherry-picked version of his history.
^ permalink raw reply
* Re: Suggestion on git-push --porcelain
From: Jeff King @ 2010-02-10 4:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tay Ray Chuan, Git Mailing List, Larry D'Anna
In-Reply-To: <7vmxzhn6fp.fsf@alter.siamese.dyndns.org>
On Tue, Feb 09, 2010 at 06:57:46PM -0800, Junio C Hamano wrote:
> Tay Ray Chuan <rctay89@gmail.com> writes:
>
> > $ git push --porcelain
> > PORCELAIN To git://foo.com/git/myrepo.git
> > PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321
> > PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de
> >
> > This is an "positive" approach, in the sense that we don't remove
> > anything from the current output; we just add more printf("PORCELAIN")
> > lines to wherever is appropriate.
>
> Sorry, but I don't see what that would solve. For example, we used not to
> give the destination to the standard output stream, but that line carries
> a necessary information and Larry's series corrects that.
I think he is trying to future-proof any additional output that push (or
remote helpers) produce. I don't think it is really worth it, though.
All of that should be going to stderr, and thus would be, at worst,
noise on the terminal. I don't think it is that hard or error-prone a
rule to send such cruft to stderr.
-Peff
^ permalink raw reply
* Re: [RFD] Notes are independent: proposal for new notes implementation
From: Jeff King @ 2010-02-10 4:51 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Johan Herland, Junio C Hamano, Jon Seymour
In-Reply-To: <201002092105.25636.jnareb@gmail.com>
On Tue, Feb 09, 2010 at 09:05:23PM +0100, Jakub Narebski wrote:
> The proposed solution was to use custom merge strategy for notes. But
> what if the answer was to change implementation, decoupling history of
> notes from each other, and keeping history of each note separate.
If I am understanding you correctly, instead of keeping a commit history
of trees of many notes (one per sha1), we will have a tree of commit
histories, one history per sha1. What problem is this solving?
If I modify commit X and you modify commit Y, we avoid making a merge
commit. But so what? The merge would be trivial, since we did not modify
the same entries. The user never cares that there is a merge commit.
And if we both _did_ edit commit X, both cases result in a merge.
If we both modified X and Y, then you will presumably do the merge for X
and Y iteratively before you can create a new notes tree. Or you could
merge them separately. But why? Why would I want to pull some subset of
your notes (and keep in mind this is a subset of the commits you have
noted, not a semantically different notes namespace), and how would I
even specify which notes were of interest and which were not?
So what is the concrete use case where this helps?
-Peff
^ permalink raw reply
* [PATCH 4/4] git-push: make git push --dry-run --porcelain exit with status 0 even if updates will be rejected
From: Larry D'Anna @ 2010-02-10 4:51 UTC (permalink / raw)
To: Larry D'Anna; +Cc: Junio C Hamano, git, Larry D'Anna
In-Reply-To: <20100210041313.GA21516@cthulhu>
The script calling git push --dry-run --porcelain can see clearly from the
output that the updates will be rejected. However, it will probably need to
distinguish this condition from the push failing for other reasons, such as the
remote not being reachable.
Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
builtin-send-pack.c | 4 ++++
send-pack.h | 1 +
transport.c | 4 +++-
3 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 76c7206..358f5e1 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -476,6 +476,10 @@ int send_pack(struct send_pack_args *args,
if (ret < 0)
return ret;
+
+ if (args->porcelain && args->dry_run)
+ return 0;
+
for (ref = remote_refs; ref; ref = ref->next) {
switch (ref->status) {
case REF_STATUS_NONE:
diff --git a/send-pack.h b/send-pack.h
index 28141ac..60b4ba6 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -4,6 +4,7 @@
struct send_pack_args {
unsigned verbose:1,
quiet:1,
+ porcelain:1,
send_mirror:1,
force_update:1,
use_thin_pack:1,
diff --git a/transport.c b/transport.c
index fb653c6..31f2e84 100644
--- a/transport.c
+++ b/transport.c
@@ -791,6 +791,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
args.verbose = !!(flags & TRANSPORT_PUSH_VERBOSE);
args.quiet = !!(flags & TRANSPORT_PUSH_QUIET);
args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
+ args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
ret = send_pack(&args, data->fd, data->conn, remote_refs,
&data->extra_have);
@@ -1054,7 +1055,8 @@ int transport_push(struct transport *transport,
ret = transport->push_refs(transport, remote_refs, flags);
err = push_had_errors(remote_refs);
- ret |= err;
+ if ( !(pretend && porcelain) )
+ ret |= err;
if (!quiet || err)
print_push_status(transport->url, remote_refs,
--
1.7.0.rc1.33.g07cf0f.dirty
^ permalink raw reply related
* Re: [PATCH v3 3/3] git-push: make git push --dry-run --porcelain exit with status 0 even if updates will be rejected
From: Larry D'Anna @ 2010-02-10 4:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtytqyrlk.fsf@alter.siamese.dyndns.org>
* Junio C Hamano (gitster@pobox.com) [100209 17:25]:
> Larry D'Anna <larry@elder-gods.org> writes:
>
> > @@ -1052,7 +1053,7 @@ int transport_push(struct transport *transport,
> > flags & TRANSPORT_PUSH_FORCE);
> >
> > ret = transport->push_refs(transport, remote_refs, flags);
> > - err = push_had_errors(remote_refs);
> > + err = (pretend && porcelain) ? 0 : push_had_errors(remote_refs);
>
> Hmph, you are doing (rewritten in an easier to follow format)
>
> if (--dry-run && --porcelain)
> err = 0;
> else
> err = push_add_errors(remote_refs);
>
> here, which I think changes the semantics of what follows immediately
> after this hunk, namely:
>
> if (!quiet || err)
> print_push_status(transport->url, remote_refs,
> verbose | porcelain, porcelain,
> nonfastforward);
>
> Earlier, the logic said "even if you asked for --quiet, we would report if
> there is an error" but now you are changing the rule to "under --dry-run
> and --porcelain, --quiet means don't ever report the status, even when
> there are errors".
>
> I don't necessarily think it is a bad change, but in any case the semantic
> change is worth documenting.
Now that you point it out, I can't really think of any reason why this patch
ought to mess with the semantics of --quiet.
--larry
^ permalink raw reply
* Re: A Visual Git Reference
From: John J. Franey @ 2010-02-10 3:40 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.LNX.2.00.1002081513430.14365@iabervon.org>
On Mon, 08 Feb 2010 16:57:12 -0500, Daniel Barkalow wrote:
> On Mon, 8 Feb 2010, Mark Lodato wrote:
>
>> All,
>>
>> I put together a "Visual Git Reference" containing visualizations of
>> the most common git commands, for people who prefer to see images over
>> text. It is designed as a reference, not a tutorial, so readers need
>> to have some amount of experience before the page will become useful.
>>
>> URL: http://marklodato.github.com/visual-git-guide/ Git repo:
>> http://github.com/marklodato/visual-git-guide/
>>
>> If you have any feedback or suggestions, please let me know!
>
> The "3-way merge" node should graphically distinguish the base from the
> two sides, rather than having all three just go in. The "3-way merge"
> operation is tricky to understand visually without some sort of "split
> and rejoin, with specific points" thing.
>
> Also, it would probably be worth showing the use of the index in the
> process of a 3-way merge: all three versions go into the blue box, and a
> combination (with conflict markers) goes into the pink box; the user
> cleans up the pink box, and replaces the 3-part blue box content with
> the cleaned-up single result content; then the commit gives the diagram
> you have for "git merge other".
>
I often have difficulty trying to internally visualize a mapping from the
commits of a merge to the panes of a kdiff3 session. I have special
difficulty keeping straight which commit is the LOCAL and which is the
REMOTE. Looking at your visual guide, I can almost see a kdiff session
in this section on three way merge where each commit sits in one of the
panes. b325c in upper left, ed489 in upper middle, and 33104 in upper
right, "Working Directory" on the bottom pane. Not sure if that graphic
would be useful to anyone else. Thought I'd throw it in for grins.
I learned a few things from your guide. I've been using git for about a
year and a half. Thanks.
John
^ permalink raw reply
* Re: Suggestion on git-push --porcelain
From: Larry D'Anna @ 2010-02-10 3:35 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Git Mailing List, Jeff King, Junio C Hamano
In-Reply-To: <be6fef0d1002091834i1c4b202cp5afacc326bd1a4d6@mail.gmail.com>
* Tay Ray Chuan (rctay89@gmail.com) [100209 21:34]:
> Hi,
>
> this is regarding the recent patch series from Larry. (I thought
> replying to any of the patch messages was appropriate, I couldn't find
> a cover-letter, so I'm starting a new thread.)
>
> Around June last year, a patch from Larry was made to add the
> --porcelain option, so as to produce machine-readable output regarding
> ref status.
>
> The latest patch series goes a step further, and tries to change
> output - for example, suppressing user-friendly advice and giving "To:
> <destination>".
>
> I think this is an untenable path - adding/suppressing output of
> certain messages for porcelain writers, while trying to keep things
> fixed enough for porcelain writers to depend on. We will also have to
> keep and eye out for future patches from adding fprintfs to stdout and
> stderr that may break porcelain scripts.
While I agree with you in principle, I'm still advocating that we make these two
changes. *Especially* suppressing the advice. That advice did not exist last
June, so the output format has already been changed. Also, the advice never
should have gone to the standard output in the first place. All the other
instances of output like that go to standard error.
As for the "To: " lines, unfortunately the lack of them was a pretty serious
design flaw in the original patch :(
> I believe a better approach would be to prefix messages intended for
> porcelain writers. For example, a push session might look like this:
>
> $ git push --porcelain
> PORCELAIN To git://foo.com/git/myrepo.git
> PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321
> PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de
>
> This is an "positive" approach, in the sense that we don't remove
> anything from the current output; we just add more printf("PORCELAIN")
> lines to wherever is appropriate.
Actually, What I'm proposing is something very similar to this: I think that the
output for the porcelain writer and *only* the output for the porcelain writer
should go to standard output, and everything else should go to standard error.
--larry
^ permalink raw reply
* RFC: git sync
From: Larry D'Anna @ 2010-02-10 3:27 UTC (permalink / raw)
To: git
So say you have a project with a bunch of branches. You have two main computers
you work on: a laptop and a workstation, and you keep an authoritative copy on a
server somewhere. When you sit down at your laptop to work on your project, the
first thing you want to do is make sure that whatever you've got locally is
up-to-date with the repo on the server. So you run:
git push origin :
Then if it says anything isn't a fast-forward, you use some combination of git
pull, git checkout, or git fetch to get all you branches up to date, then
possibly you run the push again to push merges back to the server.
How about instead we add a new command called "git sync" that does all that for
you? So if you say
git sync origin :
It matches refs just like git-push would, but it will also automatically
fast-forward your local refs if possible. (and update the worktree+index if
HEAD is one of the local refs that gets fast forwarded). If any merges would
be required, it will print a warning and leave that ref alone.
And if you say
git sync --merge origin :
it will try to merge any refs that need it. If the merge succeeds, it will be
exactly as if you had said
git checkout foobranch
git pull origin foobranch
git push origin foobranch
What do you all think? If you like the idea, I'll do it as a builtin.
Otherwise I'll just hack up a perl script for myself.
--larry
^ permalink raw reply
* Re: [PATCH] git log -p -m: Document, honor --first-parent
From: Junio C Hamano @ 2010-02-10 3:23 UTC (permalink / raw)
To: Petr Baudis; +Cc: Christian MICHON, git list
In-Reply-To: <20100210021148.GT9553@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> * The branch was created long ago, but has merged latest changes of the
> top of HEAD in, and now the branch is merged back, that's a "bad
> fastforward" since that flips the perspective of main-vs-topic branch.
>
> I feel that it's important to point out this caveat.
Ah, I thought you were contrasting between ff and non-ff, but instead you
were giving caveat about trusting "fast-parent", which I didn't realize.
Yeah, but in general, unless it is the final merge to consolidate the work
on the topic to mainline that was delegated by the mainline maintainer to
the topic person, merging _from_ mainline _to_ topic should rarely happen.
And when it happens, relying on the first-parent ancestry obviously breaks
down.
> I really dislike the "first-parent ancestry" wording, I think it muds
> down the whole issue.
I am not particularly fond of the wording, either, but any other word you
would use, you would need to explain the background information, i.e. how
and why the concept embodied by that other word you choose to use relates
to the "--first-parent" option.
You can for example say "the changes introduced to the mainline by each
commit" (and by "commit" we mean both single parent ones directly made
while the mainline was the current branch, and merges made into that
branch); you need to define what you mean by "the mainline", and what your
assumptions are about the workflow employed (e.g. "rarely if ever merge
goes the wrong direction").
>> > + else if (opt->first_parent_only) {
>> > + /* Generate merge log entry only for the first
>> > + * parent, showing summary diff of the others
>> > + * we merged _in_. */
>>
>> Style?
>
> What's wrong?
/*
* We prefer to write multi-line comments
* like this.
*/
^ permalink raw reply
* [PATCH] git-archive documentation: .gitattributes must be committed
From: Francois Marier @ 2010-02-10 2:51 UTC (permalink / raw)
To: git; +Cc: Francois Marier
Add a note to the documentation stating that the .gitattributes
file must be present (i.e. committed) in the named tree that is
exported.
This can be a bit confusing because it's different from .gitignore
which takes effect as soon as the file is created.
Signed-off-by: Francois Marier <fmarier@gmail.com>
---
Documentation/git-archive.txt | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
index 799c8b6..fcd681d 100644
--- a/Documentation/git-archive.txt
+++ b/Documentation/git-archive.txt
@@ -112,6 +112,9 @@ export-subst::
expand several placeholders when adding this file to an archive.
See linkgit:gitattributes[5] for details.
+The .gitattributes file must be present in the named tree for it to take
+effect. Uncommitted attributes will not be considered in exports.
+
EXAMPLES
--------
git archive --format=tar --prefix=junk/ HEAD | (cd /var/tmp/ && tar xf -)::
--
1.6.6.1
^ permalink raw reply related
* Re: Suggestion on git-push --porcelain
From: Junio C Hamano @ 2010-02-10 2:57 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Git Mailing List, Larry D'Anna, Jeff King
In-Reply-To: <be6fef0d1002091834i1c4b202cp5afacc326bd1a4d6@mail.gmail.com>
Tay Ray Chuan <rctay89@gmail.com> writes:
> $ git push --porcelain
> PORCELAIN To git://foo.com/git/myrepo.git
> PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321
> PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de
>
> This is an "positive" approach, in the sense that we don't remove
> anything from the current output; we just add more printf("PORCELAIN")
> lines to wherever is appropriate.
Sorry, but I don't see what that would solve. For example, we used not to
give the destination to the standard output stream, but that line carries
a necessary information and Larry's series corrects that.
In your "prefix with PORCELAIN" scheme, such a change will start adding a
new line "PORCELAIN To ..." that older implementations may not be prepared
to see.
Other than making the output more noisy, I do not see an improvement here.
^ permalink raw reply
* Suggestion on git-push --porcelain
From: Tay Ray Chuan @ 2010-02-10 2:34 UTC (permalink / raw)
To: Git Mailing List; +Cc: Larry D'Anna, Jeff King, Junio C Hamano
Hi,
this is regarding the recent patch series from Larry. (I thought
replying to any of the patch messages was appropriate, I couldn't find
a cover-letter, so I'm starting a new thread.)
Around June last year, a patch from Larry was made to add the
--porcelain option, so as to produce machine-readable output regarding
ref status.
The latest patch series goes a step further, and tries to change
output - for example, suppressing user-friendly advice and giving "To:
<destination>".
I think this is an untenable path - adding/suppressing output of
certain messages for porcelain writers, while trying to keep things
fixed enough for porcelain writers to depend on. We will also have to
keep and eye out for future patches from adding fprintfs to stdout and
stderr that may break porcelain scripts.
I believe a better approach would be to prefix messages intended for
porcelain writers. For example, a push session might look like this:
$ git push --porcelain
PORCELAIN To git://foo.com/git/myrepo.git
PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321
PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de
This is an "positive" approach, in the sense that we don't remove
anything from the current output; we just add more printf("PORCELAIN")
lines to wherever is appropriate.
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [PATCH] git log -p -m: Document, honor --first-parent
From: Petr Baudis @ 2010-02-10 2:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian MICHON, git list
In-Reply-To: <7vpr4dop1m.fsf@alter.siamese.dyndns.org>
On Tue, Feb 09, 2010 at 05:30:29PM -0800, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
>
> > diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> > index 0e39bb6..a2a2d04 100644
> > --- a/Documentation/git-log.txt
> > +++ b/Documentation/git-log.txt
> > @@ -118,6 +118,15 @@ git log master --not --remotes=*/master::
> > Shows all commits that are in local master but not in any remote
> > repository master branches.
> >
> > +git log -p -m --first-parent::
> > +
> > + Shows the history including change diffs, but only from the
> > + "main branch" perspective, skipping commits that come only from
> > + merges, and showing full diffs of changes introduced by the merges.
> > + This makes sense only when following a strict policy of merging all
> > + topic branches when staying on a single integration branch and
> > + making sure the merges are not fast-forwards.
>
> I think the tone of the last three lines is too strong.
>
> Why is it necessary to make a merge with a single commit side branch when
> fast-forward would do? And if the side branch is actually two or more
> commits, it will show the broken-down changes in more detail, but the fact
> that it was made on the "primary" history would also have some
> significance (e.g. trivial and obvious fixes made directly on 'master',
> other branches merged from topic after cooking).
Ok, so what about "...the merges are not fast-forwards if the branch
histories are non-trivial"? Since there are two cases:
* The branch was created on top of HEAD, the commits were made and now
the branch is merged back, that's an "ok fastforward".
* The branch was created long ago, but has merged latest changes of the
top of HEAD in, and now the branch is merged back, that's a "bad
fastforward" since that flips the perspective of main-vs-topic branch.
I feel that it's important to point out this caveat.
> It is Ok to elaborate on the "policy" issues in the Discussion section,
> but otherwise, I would rather see you spend the same number of lines to
> clarify "showing full diffs of changes introduced by the merges" a bit
> better (e.g. it is unclear if you are showing diff from each parents or
> just from the first parent). Perhaps "s/introduced /& to the first-parent
> ancestry /" may suffice.
I really dislike the "first-parent ancestry" wording, I think it muds
down the whole issue. It would seem to me that the basic idea is clear
from the description (which might even now be excessively verbose) and
if anyone is still confused, they can quickly peek at -m description or
actually try the command out.
> > diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> > index 6e9baf8..d7d0dee 100644
> > --- a/Documentation/rev-list-options.txt
> > +++ b/Documentation/rev-list-options.txt
> > @@ -108,8 +108,8 @@ options may be given. See linkgit:git-diff-files[1] for more options.
> >
> > -c::
> >
> > - This flag changes the way a merge commit is displayed. It shows
> > - the differences from each of the parents to the merge result
> > + This flag forces the default way a merge commit is displayed. It
> > + shows the differences from each of the parents to the merge result
> > simultaneously instead of showing pairwise diff between a parent
>
> Sorry, I don't understand this change; "forces the default?" Any option
> "forces" the command to behave differently. At least the original is
> understandable "Ah, without it it shows one way but with this it shows in
> a different way", even though that does not carry much useful information
> (i.e. what are the two ways? ah, I need to read further down).
At some point when making this change, I was in the state of believing
that combined diffs are always the default. :-) That is not true, so
I will drop this change again.
> > diff --git a/log-tree.c b/log-tree.c
> > index 27afcf6..fb990a1 100644
> > --- a/log-tree.c
> > +++ b/log-tree.c
> > @@ -514,6 +514,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
> > return 0;
> > else if (opt->combine_merges)
> > return do_diff_combined(opt, commit);
> > + else if (opt->first_parent_only) {
> > + /* Generate merge log entry only for the first
> > + * parent, showing summary diff of the others
> > + * we merged _in_. */
>
> Style?
What's wrong? There should be an empty line at the comment beginning?
I have a faint memory of getting some-such undocumented comment ugliness
requirement wrong before. ;-)
> Don't we use --cc as default for "show" (and possibly "log"---I don't
> remember the details)?
Ah, that must be it! Turning that off in -m code makes it work for show
as well.
> > + diff_tree_sha1(parents->item->object.sha1, sha1, "", &opt->diffopt);
> > + log_tree_diff_flush(opt);
> > + return !opt->loginfo;
> > + }
>
> This needs some tests but I think it is a good first step in the right
> direction.
Thanks. Hrmh, testsuites... ;-)
--
Petr "Pasky" Baudis
If you can't see the value in jet powered ants you should turn in
your nerd card. -- Dunbal (464142)
^ permalink raw reply
* [PATCH 8/6] receive-pack: Send internal errors over side-band #2
From: Shawn O. Pearce @ 2010-02-10 2:01 UTC (permalink / raw)
To: git
In-Reply-To: <1265767290-25863-1-git-send-email-spearce@spearce.org>
If the client has requested side-band-64k capability, send any
of the internal error or warning messages in the muxed side-band
stream using the same band as our hook output, band #2. By putting
everything in one stream we ensure all messages are processed by
the side-band demuxer, avoiding interleaving between our own stderr
and the side-band demuxer's stderr buffers.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
builtin-receive-pack.c | 68 +++++++++++++++++++++++++++++++++++-----------
t/t5401-update-hooks.sh | 3 +-
2 files changed, 53 insertions(+), 18 deletions(-)
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index da1c26b..e98c2f1 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -139,6 +139,40 @@ static struct command *commands;
static const char pre_receive_hook[] = "hooks/pre-receive";
static const char post_receive_hook[] = "hooks/post-receive";
+static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+static void rp_warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
+
+static void report_message(const char *prefix, const char *err, va_list params)
+{
+ int sz = strlen(prefix);
+ char msg[4096];
+
+ strncpy(msg, prefix, sz);
+ sz += vsnprintf(msg + sz, sizeof(msg) - (sz + 1), err, params);
+ msg[sz++] = '\n';
+
+ if (use_sideband)
+ send_sideband(1, 2, msg, sz, use_sideband);
+ else
+ xwrite(2, msg, sz);
+}
+
+static void rp_warning(const char *err, ...)
+{
+ va_list params;
+ va_start(params, err);
+ report_message("warning: ", err, params);
+ va_end(params);
+}
+
+static void rp_error(const char *err, ...)
+{
+ va_list params;
+ va_start(params, err);
+ report_message("error: ", err, params);
+ va_end(params);
+}
+
static int copy_to_sideband(int in, int out, void *arg)
{
char data[128];
@@ -276,7 +310,7 @@ static void warn_unconfigured_deny(void)
{
int i;
for (i = 0; i < ARRAY_SIZE(warn_unconfigured_deny_msg); i++)
- warning("%s", warn_unconfigured_deny_msg[i]);
+ rp_warning("%s", warn_unconfigured_deny_msg[i]);
}
static char *warn_unconfigured_deny_delete_current_msg[] = {
@@ -302,7 +336,7 @@ static void warn_unconfigured_deny_delete_current(void)
for (i = 0;
i < ARRAY_SIZE(warn_unconfigured_deny_delete_current_msg);
i++)
- warning("%s", warn_unconfigured_deny_delete_current_msg[i]);
+ rp_warning("%s", warn_unconfigured_deny_delete_current_msg[i]);
}
static const char *update(struct command *cmd)
@@ -314,7 +348,7 @@ static const char *update(struct command *cmd)
/* only refs/... are allowed */
if (prefixcmp(name, "refs/") || check_ref_format(name + 5)) {
- error("refusing to create funny ref '%s' remotely", name);
+ rp_error("refusing to create funny ref '%s' remotely", name);
return "funny refname";
}
@@ -324,25 +358,25 @@ static const char *update(struct command *cmd)
break;
case DENY_UNCONFIGURED:
case DENY_WARN:
- warning("updating the current branch");
+ rp_warning("updating the current branch");
if (deny_current_branch == DENY_UNCONFIGURED)
warn_unconfigured_deny();
break;
case DENY_REFUSE:
- error("refusing to update checked out branch: %s", name);
+ rp_error("refusing to update checked out branch: %s", name);
return "branch is currently checked out";
}
}
if (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {
- error("unpack should have generated %s, "
- "but I can't find it!", sha1_to_hex(new_sha1));
+ rp_error("unpack should have generated %s, "
+ "but I can't find it!", sha1_to_hex(new_sha1));
return "bad pack";
}
if (!is_null_sha1(old_sha1) && is_null_sha1(new_sha1)) {
if (deny_deletes && !prefixcmp(name, "refs/heads/")) {
- error("denying ref deletion for %s", name);
+ rp_error("denying ref deletion for %s", name);
return "deletion prohibited";
}
@@ -354,10 +388,10 @@ static const char *update(struct command *cmd)
case DENY_UNCONFIGURED:
if (deny_delete_current == DENY_UNCONFIGURED)
warn_unconfigured_deny_delete_current();
- warning("deleting the current branch");
+ rp_warning("deleting the current branch");
break;
case DENY_REFUSE:
- error("refusing to delete the current branch: %s", name);
+ rp_error("refusing to delete the current branch: %s", name);
return "deletion of the current branch prohibited";
}
}
@@ -376,7 +410,7 @@ static const char *update(struct command *cmd)
if (!old_object || !new_object ||
old_object->type != OBJ_COMMIT ||
new_object->type != OBJ_COMMIT) {
- error("bad sha1 objects for %s", name);
+ rp_error("bad sha1 objects for %s", name);
return "bad ref";
}
old_commit = (struct commit *)old_object;
@@ -387,23 +421,23 @@ static const char *update(struct command *cmd)
break;
free_commit_list(bases);
if (!ent) {
- error("denying non-fast-forward %s"
- " (you should pull first)", name);
+ rp_error("denying non-fast-forward %s"
+ " (you should pull first)", name);
return "non-fast-forward";
}
}
if (run_update_hook(cmd)) {
- error("hook declined to update %s", name);
+ rp_error("hook declined to update %s", name);
return "hook declined";
}
if (is_null_sha1(new_sha1)) {
if (!parse_object(old_sha1)) {
- warning ("Allowing deletion of corrupt ref.");
+ rp_warning("Allowing deletion of corrupt ref.");
old_sha1 = NULL;
}
if (delete_ref(name, old_sha1, 0)) {
- error("failed to delete %s", name);
+ rp_error("failed to delete %s", name);
return "failed to delete";
}
return NULL; /* good */
@@ -411,7 +445,7 @@ static const char *update(struct command *cmd)
else {
lock = lock_any_ref_for_update(name, old_sha1, 0);
if (!lock) {
- error("failed to lock %s", name);
+ rp_error("failed to lock %s", name);
return "failed to lock";
}
if (write_ref_sha1(lock, new_sha1, "push")) {
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 7240fab..17bcb0b 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -124,6 +124,7 @@ remote: STDOUT update refs/heads/master
remote: STDERR update refs/heads/master
remote: STDOUT update refs/heads/tofail
remote: STDERR update refs/heads/tofail
+remote: error: hook declined to update refs/heads/tofail
remote: STDOUT post-receive
remote: STDERR post-receive
remote: STDOUT post-update
@@ -131,7 +132,7 @@ remote: STDERR post-update
EOF
test_expect_success 'send-pack stderr contains hook messages' '
grep ^remote: send.err | sed "s/ *\$//" >actual &&
- test_cmp - actual <expect
+ test_cmp expect actual
'
test_done
--
1.7.0.rc2.170.gbc565
^ permalink raw reply related
* [PATCH 7/6] t5401: Use a bare repository for the remote peer
From: Shawn O. Pearce @ 2010-02-10 2:01 UTC (permalink / raw)
To: git
We want to avoid the warnings (or later, test failures) about
updating the current branch. It was never my intention to have
this test deal with a repository with a working directory, and it
is a very old bug that the test even used a non-bare repository
for the remote side of the push operations.
This fixes the interleaved output error we were seeing as a test
failure by avoiding the giant warning message we were getting back
about updating the current branch being risky.
Its not a real fix, but is something we should do no matter what,
because the behavior will change in the future to reject, and the
test would break at that time.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
t/t5401-update-hooks.sh | 58 +++++++++++++++++++++++-----------------------
1 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index c3cf397..7240fab 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -17,22 +17,22 @@ test_expect_success setup '
commit1=$(echo modify | git commit-tree $tree1 -p $commit0) &&
git update-ref refs/heads/master $commit0 &&
git update-ref refs/heads/tofail $commit1 &&
- git clone ./. victim &&
- GIT_DIR=victim/.git git update-ref refs/heads/tofail $commit1 &&
+ git clone --bare ./. victim.git &&
+ GIT_DIR=victim.git git update-ref refs/heads/tofail $commit1 &&
git update-ref refs/heads/master $commit1 &&
git update-ref refs/heads/tofail $commit0
'
-cat >victim/.git/hooks/pre-receive <<'EOF'
+cat >victim.git/hooks/pre-receive <<'EOF'
#!/bin/sh
printf %s "$@" >>$GIT_DIR/pre-receive.args
cat - >$GIT_DIR/pre-receive.stdin
echo STDOUT pre-receive
echo STDERR pre-receive >&2
EOF
-chmod u+x victim/.git/hooks/pre-receive
+chmod u+x victim.git/hooks/pre-receive
-cat >victim/.git/hooks/update <<'EOF'
+cat >victim.git/hooks/update <<'EOF'
#!/bin/sh
echo "$@" >>$GIT_DIR/update.args
read x; printf %s "$x" >$GIT_DIR/update.stdin
@@ -40,77 +40,77 @@ echo STDOUT update $1
echo STDERR update $1 >&2
test "$1" = refs/heads/master || exit
EOF
-chmod u+x victim/.git/hooks/update
+chmod u+x victim.git/hooks/update
-cat >victim/.git/hooks/post-receive <<'EOF'
+cat >victim.git/hooks/post-receive <<'EOF'
#!/bin/sh
printf %s "$@" >>$GIT_DIR/post-receive.args
cat - >$GIT_DIR/post-receive.stdin
echo STDOUT post-receive
echo STDERR post-receive >&2
EOF
-chmod u+x victim/.git/hooks/post-receive
+chmod u+x victim.git/hooks/post-receive
-cat >victim/.git/hooks/post-update <<'EOF'
+cat >victim.git/hooks/post-update <<'EOF'
#!/bin/sh
echo "$@" >>$GIT_DIR/post-update.args
read x; printf %s "$x" >$GIT_DIR/post-update.stdin
echo STDOUT post-update
echo STDERR post-update >&2
EOF
-chmod u+x victim/.git/hooks/post-update
+chmod u+x victim.git/hooks/post-update
test_expect_success push '
- test_must_fail git send-pack --force ./victim/.git \
+ test_must_fail git send-pack --force ./victim.git \
master tofail >send.out 2>send.err
'
test_expect_success 'updated as expected' '
- test $(GIT_DIR=victim/.git git rev-parse master) = $commit1 &&
- test $(GIT_DIR=victim/.git git rev-parse tofail) = $commit1
+ test $(GIT_DIR=victim.git git rev-parse master) = $commit1 &&
+ test $(GIT_DIR=victim.git git rev-parse tofail) = $commit1
'
test_expect_success 'hooks ran' '
- test -f victim/.git/pre-receive.args &&
- test -f victim/.git/pre-receive.stdin &&
- test -f victim/.git/update.args &&
- test -f victim/.git/update.stdin &&
- test -f victim/.git/post-receive.args &&
- test -f victim/.git/post-receive.stdin &&
- test -f victim/.git/post-update.args &&
- test -f victim/.git/post-update.stdin
+ test -f victim.git/pre-receive.args &&
+ test -f victim.git/pre-receive.stdin &&
+ test -f victim.git/update.args &&
+ test -f victim.git/update.stdin &&
+ test -f victim.git/post-receive.args &&
+ test -f victim.git/post-receive.stdin &&
+ test -f victim.git/post-update.args &&
+ test -f victim.git/post-update.stdin
'
test_expect_success 'pre-receive hook input' '
(echo $commit0 $commit1 refs/heads/master;
echo $commit1 $commit0 refs/heads/tofail
- ) | test_cmp - victim/.git/pre-receive.stdin
+ ) | test_cmp - victim.git/pre-receive.stdin
'
test_expect_success 'update hook arguments' '
(echo refs/heads/master $commit0 $commit1;
echo refs/heads/tofail $commit1 $commit0
- ) | test_cmp - victim/.git/update.args
+ ) | test_cmp - victim.git/update.args
'
test_expect_success 'post-receive hook input' '
echo $commit0 $commit1 refs/heads/master |
- test_cmp - victim/.git/post-receive.stdin
+ test_cmp - victim.git/post-receive.stdin
'
test_expect_success 'post-update hook arguments' '
echo refs/heads/master |
- test_cmp - victim/.git/post-update.args
+ test_cmp - victim.git/post-update.args
'
test_expect_success 'all hook stdin is /dev/null' '
- ! test -s victim/.git/update.stdin &&
- ! test -s victim/.git/post-update.stdin
+ ! test -s victim.git/update.stdin &&
+ ! test -s victim.git/post-update.stdin
'
test_expect_success 'all *-receive hook args are empty' '
- ! test -s victim/.git/pre-receive.args &&
- ! test -s victim/.git/post-receive.args
+ ! test -s victim.git/pre-receive.args &&
+ ! test -s victim.git/post-receive.args
'
test_expect_success 'send-pack produced no output' '
--
1.7.0.rc2.170.gbc565
^ permalink raw reply related
* Re: [PATCH] git log -p -m: Document, honor --first-parent
From: Junio C Hamano @ 2010-02-10 1:30 UTC (permalink / raw)
To: Petr Baudis; +Cc: Christian MICHON, git list
In-Reply-To: <20100210011149.GR9553@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 0e39bb6..a2a2d04 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -118,6 +118,15 @@ git log master --not --remotes=*/master::
> Shows all commits that are in local master but not in any remote
> repository master branches.
>
> +git log -p -m --first-parent::
> +
> + Shows the history including change diffs, but only from the
> + "main branch" perspective, skipping commits that come only from
> + merges, and showing full diffs of changes introduced by the merges.
> + This makes sense only when following a strict policy of merging all
> + topic branches when staying on a single integration branch and
> + making sure the merges are not fast-forwards.
I think the tone of the last three lines is too strong.
Why is it necessary to make a merge with a single commit side branch when
fast-forward would do? And if the side branch is actually two or more
commits, it will show the broken-down changes in more detail, but the fact
that it was made on the "primary" history would also have some
significance (e.g. trivial and obvious fixes made directly on 'master',
other branches merged from topic after cooking).
It is Ok to elaborate on the "policy" issues in the Discussion section,
but otherwise, I would rather see you spend the same number of lines to
clarify "showing full diffs of changes introduced by the merges" a bit
better (e.g. it is unclear if you are showing diff from each parents or
just from the first parent). Perhaps "s/introduced /& to the first-parent
ancestry /" may suffice.
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 6e9baf8..d7d0dee 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -108,8 +108,8 @@ options may be given. See linkgit:git-diff-files[1] for more options.
>
> -c::
>
> - This flag changes the way a merge commit is displayed. It shows
> - the differences from each of the parents to the merge result
> + This flag forces the default way a merge commit is displayed. It
> + shows the differences from each of the parents to the merge result
> simultaneously instead of showing pairwise diff between a parent
Sorry, I don't understand this change; "forces the default?" Any option
"forces" the command to behave differently. At least the original is
understandable "Ah, without it it shows one way but with this it shows in
a different way", even though that does not carry much useful information
(i.e. what are the two ways? ah, I need to read further down).
> diff --git a/log-tree.c b/log-tree.c
> index 27afcf6..fb990a1 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -514,6 +514,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
> return 0;
> else if (opt->combine_merges)
> return do_diff_combined(opt, commit);
> + else if (opt->first_parent_only) {
> + /* Generate merge log entry only for the first
> + * parent, showing summary diff of the others
> + * we merged _in_. */
Style?
Don't we use --cc as default for "show" (and possibly "log"---I don't
remember the details)?
> + diff_tree_sha1(parents->item->object.sha1, sha1, "", &opt->diffopt);
> + log_tree_diff_flush(opt);
> + return !opt->loginfo;
> + }
This needs some tests but I think it is a good first step in the right
direction.
Thanks.
^ permalink raw reply
* Re: t5401-update-hooks test failure
From: Shawn O. Pearce @ 2010-02-10 1:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, Larry D'Anna, Jeff King, git
In-Reply-To: <7vpr4eyqok.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > The only way I can see this missing message happening is if the C
> > library isn't flushing the stdio buffer before the hook process
> > exits. Given that the hook process is a /bin/sh shell script,
> > and its using echo to print its messages... I'm at a loss for how
> > to fix that in Git.
> >
> > Unless its the recv_sideband() somehow skipping a line. But I
> > can't see it doing that.
>
> The detection method of test is fooled by intermixed message.
>
> This is what send.err has, and you grep for '^remote:' in it.
>
> -- >8 --
> warning: updating the current branch
> warning: Updating the currently cheremote: STDERR pre-receive
> ,
..
> But there indeed _is_ some skipping. "Updating the currently che"
> is interrupted by the output from the pre-receive hook, and I do not see
> the remainder "cked out branch may cause confusion,\n" anywhere.
Uh. I got the problem now, thanks.
What's going on is, other messages inside of builtin-receive-pack
are being sent to stderr, while hook output is going over the
multiplexed side-band through stdout, where its parsed and written
to stderr by send-pack.
What I missed in my patch was changing all of these other messages
inside of receive-pack to also go over the side-band #2 if we have
use_sideband enabled.
Patch coming in a few minutes.
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching
From: Petr Baudis @ 2010-02-10 1:23 UTC (permalink / raw)
To: Jakub Narebski
Cc: git, John 'Warthog9' Hawley,
John 'Warthog9' Hawley
In-Reply-To: <201002100212.26157.jnareb@gmail.com>
On Wed, Feb 10, 2010 at 02:12:24AM +0100, Jakub Narebski wrote:
> On Tue, 9 Feb 2010 at 11:30 +0100, Jakub Narebski wrote:
>
> > The cache_fetch subroutine captures output (from STDOUT only, as
> > STDERR is usually logged) using either ->push_layer()/->pop_layer()
> > from PerlIO::Util submodule (if it is available), or by setting and
> > restoring *STDOUT. Note that only the former could be tested reliably
> > to be reliable in t9503 test!
>
> Scratch that, I have just checked that (at least for Apache + mod_cgi,
> but I don't think that it matters) the latter solution, with setting
> and restoring *STDOUT doesn't work: I would get data in cache (so it
> can be restored later), but instead of output I would get Internal Server
> Error ("The server encountered an internal error or misconfiguration and
> was unable to complete your request.") without even a hint what the
> problem was. Sprinkling "die ...: $!" didn't help to catch this error:
> I suspect that the problem is with capturing.
>
> So we either would have to live with non-core PerlIO::Util or (pure Perl)
> Capture::Tiny, or do the 'print -> print $out' patch...
All the magic methods seem to be troublesome, but in that case I'd
really prefer a level of indirection instead of filehandle - as is,
'print (...) -> output (...)' ins. of 'print (...) -> print $out (...)'
(or whatever). That should be really flexible and completely
futureproof, and I don't think the level of indirection would incur any
measurable overhead, would it?
--
Petr "Pasky" Baudis
If you can't see the value in jet powered ants you should turn in
your nerd card. -- Dunbal (464142)
^ 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