* Re: jc/shortstatus
From: Jeff King @ 2009-08-15 8:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8whltrqj.fsf@alter.siamese.dyndns.org>
On Sat, Aug 15, 2009 at 01:18:28AM -0700, Junio C Hamano wrote:
> It is handy to have both available while asking others help debugging the
> version in 'pu', and that is the only reason for the separate command. It
> could have been named 'git frotz' for all I care ;-)
>
> I do not intend to make it another "git merge-recur"; I would actually
> want to have it replace "status" before the series goes to 'next'.
Ah, OK. I thought we were going to live through 1.6.5 with the dual
commands. But if it is going to be "status", I don't care at all what it
is called in the interim. :)
> - What should its exit code be? Should it be consistent with the
> traditional "git status" at least when no paths are given, signallying
> failure when nothing is staged for committing, so that ancient out of
> tree scripts people may have written would not break too badly when we
> make the switch?
If I were designing it from scratch, I would say the exit code should be
the opposite. That is, it is really about doing several diffs, and if
there are no changes, then we should, like diff, exit zero.
If you want to know whether there is something to commit, you wouldn't
to use this tool. If you just want to know if there is something in the
index to commit, you would use diff-index. If you want to see if
some set of commit parameters would make a commit, you would use "commit
--dry-run".
So really there is only the historical argument. I am inclined not to
worry about it. We are already breaking compatibility in other ways
(like how command line parameters are treated), so you are really only
helping people whose scripts use a subset of the current "git status"
functionality. And since this is the time for breaking, I think it's
best to make all of the changes we want, and not have another
half-breakage later.
> - What should its default mode of output be? Do people prefer "svn st"
> style short-format output, or should we stay verbose and explanatory?
Personally I prefer the long format, but maybe just because I'm used to
it. I suspect others want the short format. This really should be a
porcelain command[1], so I don't see a problem with a
status.outputformat config option.
[1] One can, after all, call diff-index, diff-files, and "ls-files -o"
to get the same information from plumbing. If we really want to provide
plumbing that pulls them all together (e.g., because it is more
efficient or more convenient to do it all in one command), then I think
we should provide "git status --porcelain".
> - Is it handling corner cases correctly? e.g. Does it operate correctly
> when run from a subdirectory? How should it handle submodules? Before
> the initial commit? Use of colors?
I'll try out a few things, but I think largely we will need to put it in
"next" to shake out any bugs.
-Peff
^ permalink raw reply
* Re: [PATCH] git submodule summary: add --files option
From: Lars Hjemli @ 2009-08-15 8:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jens Lehmann
In-Reply-To: <7v8whmjhqh.fsf@alter.siamese.dyndns.org>
On Fri, Aug 14, 2009 at 21:52, Junio C Hamano<gitster@pobox.com> wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> git submodule summary is providing similar functionality for submodules as
>> git diff-index does for a git project (including the meaning of --cached).
>> But the analogon to git diff-files is missing, so add a --files option to
>> summarize the differences between the index of the super project and the
>> last commit checked out in the working tree of the submodule.
>>
>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>
> Makes sense to me. Comments?
Acked-by: Lars Hjemli <hjemli@gmail.com> with a tiny fixup:
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -129,7 +129,7 @@ summary::
in the submodule between the given super project commit and the
index or working tree (switched by --cached) are shown. If the option
--files is given, show the series of commits in the submodule between
- the index of super project the and the working tree of the submodule
+ the index of the super project and the working tree of the submodule
(this option doesn't allow to use the --cached option or to provide an
explicit commit).
^ permalink raw reply
* Re: jc/shortstatus
From: Junio C Hamano @ 2009-08-15 8:18 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20090815070904.GA23389@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> For the "git stat" portion still in pu, I have a few comments/questions:
>
> 1. Is "stat" a good name? I worry a little that it is _too_ similar to
> "status", and that will cause confusion while they both exist. So
> something like "git overview" would cause less confusion, and even
> though it sucks to type, it will eventually replace "status" (and
> in the meantime people can make aliases or whatever).
It is handy to have both available while asking others help debugging the
version in 'pu', and that is the only reason for the separate command. It
could have been named 'git frotz' for all I care ;-)
I do not intend to make it another "git merge-recur"; I would actually
want to have it replace "status" before the series goes to 'next'.
I hopefully will have some time to start doing that over the weekend; the
first step would be to rename the branch to jc/1.7.0-status and get rid of
cmd_status() from builtin-commit.c.
A few points I haven't managed to think about, decide, nor test, are:
- What should its exit code be? Should it be consistent with the
traditional "git status" at least when no paths are given, signallying
failure when nothing is staged for committing, so that ancient out of
tree scripts people may have written would not break too badly when we
make the switch?
- What should its default mode of output be? Do people prefer "svn st"
style short-format output, or should we stay verbose and explanatory?
- Is it handling corner cases correctly? e.g. Does it operate correctly
when run from a subdirectory? How should it handle submodules? Before
the initial commit? Use of colors?
> 2. Does it really belong in builtin-commit.c anymore? It seems like
> historical accident that "status" is so closely tied to commit. The
> whole point of the new command is to _not_ be tied; I think of the
> new command more as an extension of 'diff'. Admittedly, users don't
> care where the source is located, but it helps the developers
> understand the relationships between code.
It would make sense to create a separate builtin-status.c. I haven't
looked at the dependencies yet, but it shouldn't be too bad. We'll see.
> 3. Can you squash in the gitignore patch below? :)
Yes but hopefully it won't be necessary ;-)
^ permalink raw reply
* Re: [PATCH v5 0/6] {checkout,reset,stash} --patch
From: Junio C Hamano @ 2009-08-15 7:57 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, git, Sverre Rabbelier, Nanako Shiraishi,
Nicolas Sebrecht, Pierre Habouzit
In-Reply-To: <20090815065125.GA23068@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> reset -p [HEAD] Reset this hunk? (**)
>> reset -p other Apply this hunk to index? (**)
>
> This doesn't make sense to me.
Not to me, either.
Let's say you have modified $path and ran "git add $path" earlier.
"reset -p -- $path" and "reset -p HEAD -- $path" both show what your index
has relative to the commit you are resetting your index to and offer to
"Unstage" [*1*]. This is consistent and feels natural.
"reset -p HEAD^ -- $path" however shows the same forward diff (i.e. how
your index is different compared to the commit HEAD^ you are resetting
to), but offers to "Apply".
When a user is resetting to the current HEAD (with or without an explicit
HEAD parameter), it is likely that the user did "git add" earlier and is
trying to reverse the effect of that. And showing a forward diff makes
sense. But when a user is resetting to a different commit, it may make
sense to show a reverse diff, saying "Here is a hunk you _could_ choose to
use to bring the current index to a different state, do you want to apply
it to do so?" Perhaps you meant to show a reverse diff and use the word
"Apply".
However, that would break down rather badly when HEAD did not change $path
since HEAD^. Logically what the "reset -p" would do to $path is the same,
but the patch shown and the operation offered to the user are opposite.
You could compare HEAD and the commit you are resetting the index to and
see if the path in question is different between the two commits, and
switch the direction---if there is no change, you show forward diff and
offer to "Remove this change out of the index", if there is a change, you
show reverse diff and offer to "Apply this change to the index". But if
the difference between HEAD and the commit you are resetting to does not
overlap with the change you staged to the index earlier from your work
tree, it is unclear such heuristics would yield a natural feel.
So I actually think you may be better off if you consistently showed a
forward diff (i.e. what patch would have been applied to the commit in
question to bring the index into its current shape), and always offer
"Remove this change out of the index?"
The same comment applies to "checkout -p HEAD" vs "checkout -p HEAD^".
I think the latter shouldn't show a reverse diff and offer "Apply?";
instead both should consitently show a forward diff (i.e. what patch would
have been applied to the commit to bring your work tree into its current
shape), and offer "Remove this change out of the index and the work tree?".
[Footnote]
*1* I actually have a slight problem with the use of word "Unstage" in
this context; "to stage", at least to me, means "adding _from the work
tree_ to the index", not just "modifying the index" from a random source.
The command is resetting the index in this case from a tree-ish and there
is no work tree involved, and the word "stage/unstage" feels out of place.
^ permalink raw reply
* jc/shortstatus (was: What's cooking in git.git (Aug 2009, #02; Wed, 12))
From: Jeff King @ 2009-08-15 7:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtz0co3xe.fsf@alter.siamese.dyndns.org>
On Wed, Aug 12, 2009 at 07:14:53PM -0700, Junio C Hamano wrote:
> * jc/shortstatus (Tue Aug 4 23:55:22 2009 -0700) 12 commits
> - git stat -s: short status output
> - git stat: pathspec limits, unlike traditional "git status"
> - git stat: the beginning
> + wt-status: collect untracked files in a separate "collect" phase
> + Make git_status_config() file scope static to builtin-commit.c
> + wt-status: move wt_status_colors[] into wt_status structure
> + wt-status: move many global settings to wt_status structure
> + commit: --dry-run
> + status: show worktree status of conflicted paths separately
> + wt-status.c: rework the way changes to the index and work tree are
> summarized
> + diff-index: keep the original index intact
> + diff-index: report unmerged new entries
>
> Slowly progressing. I've addressed issues pointed out by Jeff in his
> review, and hopefully the whole thing would be ready to go.
I briefly looked over what's in next, and the fixes you made look good
to me. I've been running with this series for a little while, and I was
very pleased when "git status" showed me a more detailed description of
some unmerged paths the other day. :)
For the "git stat" portion still in pu, I have a few comments/questions:
1. Is "stat" a good name? I worry a little that it is _too_ similar to
"status", and that will cause confusion while they both exist. So
something like "git overview" would cause less confusion, and even
though it sucks to type, it will eventually replace "status" (and
in the meantime people can make aliases or whatever).
2. Does it really belong in builtin-commit.c anymore? It seems like
historical accident that "status" is so closely tied to commit. The
whole point of the new command is to _not_ be tied; I think of the
new command more as an extension of 'diff'. Admittedly, users don't
care where the source is located, but it helps the developers
understand the relationships between code.
3. Can you squash in the gitignore patch below? :)
---
diff --git a/.gitignore b/.gitignore
index c446290..14e0f51 100644
--- a/.gitignore
+++ b/.gitignore
@@ -128,6 +128,7 @@ git-show-index
git-show-ref
git-stage
git-stash
+git-stat
git-status
git-stripspace
git-submodule
^ permalink raw reply related
* Re: [PATCH v5 0/6] {checkout,reset,stash} --patch
From: Jeff King @ 2009-08-15 6:51 UTC (permalink / raw)
To: Thomas Rast
Cc: Junio C Hamano, git, Sverre Rabbelier, Nanako Shiraishi,
Nicolas Sebrecht, Pierre Habouzit
In-Reply-To: <cover.1250164190.git.trast@student.ethz.ch>
On Thu, Aug 13, 2009 at 02:29:38PM +0200, Thomas Rast wrote:
> Those marked (**) are the only ones that changed semantics compared to
> v4. However, I adjusted the messages to look different:
Thanks for following up on this. This is something I've wanted for a
while, and now my procrastination is paying off. ;)
> add -p Stage this hunk?
Makes sense.
> reset -p [HEAD] Reset this hunk? (**)
Actually, it says "Unstage this hunk", but I like that even better.
> reset -p other Apply this hunk to index? (**)
This doesn't make sense to me. For example:
$ git show HEAD^:file
content
$ git show :file
content
with some changes
$ git reset -p HEAD^
diff --git a/file b/file
index d95f3ad..60a1a4e 100644
--- a/file
+++ b/file
@@ -1 +1,2 @@
content
+with some changes
Apply this hunk to index [y,n,q,a,d,/,e,?]?
The hunk is _already_ in the index. You are really asking to remove it
from the index. So shouldn't it say something like "Unstage this hunk"
or "Remove this hunk from the index"?
Or did you intend to reverse the diff, as with "checkout -p" below?
> checkout -p Discard this hunk from worktree? (**)
Good, that addresses my earlier confusion.
> checkout -p HEAD Discard this hunk from index and worktree? (**)
Good. I like how it clarifies what is being touched.
> checkout -p other Apply this hunk to index and worktree? (**)
I really expected this to just be the same as the "HEAD" case. That is,
with "git checkout -p HEAD", you are saying "I'm not interested in these
bits, discard to return back to HEAD". So if I do "git checkout -p
HEAD^", that is conceptually the same thing, except going back further
in time.
But I guess you are thinking of it as "pull these changes out of
'other'", in which case showing the reverse diff makes sense.
I think this may be a situation where the user has one of two mental
models in issuing the command, and we don't necessarily know which. So I
guess what you have is fine, but I wanted to register my surprise.
> stash -p Stash this hunk?
Getting greedy, is there a reason not to have "stash apply -p" as well?
-Peff
^ permalink raw reply
* Re: rebase-with-history -- a technique for rebasing without trashing your repo history
From: Björn Steinbrink @ 2009-08-15 3:36 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Git Mailing List
In-Reply-To: <4A85D53D.9050805@alum.mit.edu>
On 2009.08.14 23:21:01 +0200, Michael Haggerty wrote:
> Björn Steinbrink wrote:
> > On 2009.08.14 00:39:48 +0200, Michael Haggerty wrote:
> >> Björn Steinbrink wrote:
> >>> On 2009.08.13 14:46:07 +0200, Michael Haggerty wrote:
> > [...]
> > Doing a plain "git rebase subsystem topic" would of course also try to
> > rebase the "o" commits, so that problematic. Instead, you do:
> >
> > git rebase --onto subsystem O topic
> >
> > That turns O..topic (the * commits) into patches, and applies them on
> > top of O'. So the "o" commits aren't to be rebased.
> >
> > And that's exactly what your rebase-with-history would do as well. Just
> > that O is naturally a common ancestor of subsystem and topic, and so
> > just using "git rebase-w-h subsystem topic" would be enough. Conflicts
> > etc. should be 100% the same.
> >
> > If you know that your upstream is going to rebase/rewrite history, you
> > can tag (or otherwise mark) the current branching point of your branch,
> > so you can easily specify it for the --onto rebase. IOW: This is
> > primarily a social problem (tell your downstream that you rebase this or
> > that branch), but having built-in support to store the branching point
> > for rebasing _might_ be worth a thought.
>
> Recording branch points manually, coordinating merges via email -- OMG
> you are giving me flashbacks of CVS ;-)
Not merging, but rewriting history. One of the primary purposes of
rebasing is to forget the old history, the new version overrides it. And
telling someone to forget something is a social problem. You can help
the user to forget the history by tracking the branching points and I
said that git could maybe learn to do that, so the user doesn't have
to do so. Quick idea:
On branch creation, create refs/bases/<branchname> (let's call that
<base>) referencing the commit the branch initially references.
On rebase, check if <branchname>..<onto> is not empty. If so, update
refs/bases/<branchname> to reference <base>.
On reset, check if the commit the branch head is being reset to is
reachable through the commit the branch head currently references. If
not, update <base> to reference the commit we're resetting to.
Find some sane syntax for rebase that implicitly uses <base> as the
<upstream> argument, e.g. just "git rebase --onto <whatever>" could work
as "git rebase --onto <whatever> <base>".
Most likely, I missed a bunch of corner cases though...
> *Of course* you can get around all of these problems if you put the
> burden of bookkeeping on the user. The whole point of
> rebase-with-history is to have the VCS handle it automatically!
What your approach does, is simply moving the "just forget the
history" part. Instead of forgetting it at rebase time, you have to
forget it when you want to submit patches. It's obviously a bit easier
though, as you can just say "--first-parent <upstream>", assuming that
you teach format-patch to use a special first-parent diff mode for the
merge commits (see below).
> >> and merging in a topic branch makes it more difficult to create an
> >> easily-reviewable patch series. rebase-with-history has neither of
> >> these problems.
> >
> > Sure, merging is a no-go if you submit patches by email (or other,
> > similar means). But you compared that to an "enhanced" rebase approach,
> > instead of comparing your rebase approach to the currently available
> > one.
>
> In [1] I compared rebase-with-history with both of the
> currently-available options (rebase and merge). Rebase and merge can
> each deal with some of the issues that come up, but each one falls flat
> on others. I believe that rebase-with-history has the advantages of both.
And some disadvantages.
1) Cluttered history, which needs to be rewritten again when the emailed
patches are just for review, but the maintainer will actually merge from
you later.
Taking the old master, subsystem, topic example, you get (for example):
o2--o2 (subsystem)
/ \
m---m---m---m---m (master)
\ \
\ o'--o'
\ / / \
o---o *'--*' (topic)
\ / /
*---*
Now the user that maintains "topic" is back at the hard case. He now
needs to rebase onto master, using the last o' as <upstream>. The
DAG doesn't help here, the base-tracking would handle that.
2) Merge commits, which are usually displayed in a special format. So
for "git show" or "git log -p" to give useful output for those special
merges, you'd have to introduce a new "diff only against first-parent"
mode, and mark those merge in a special way, so that diff mode is used
for them, but not for real merges. And users of old git versions would
have to deal with the basic -m merge diff mode, ignoring the useless
diff for the second parent and the fact that the real merges also get
shown in that format. The base tracking doesn't have this problem
either.
> The example in [2] was taken straight from the git-rebase man page [3];
> I did not want to claim that current practice would use merging in this
> situation, but rather just to show that rebase-with-history removes the
> pain from this well-known example.
Well, the man pages says: Don't merge, rebase needs to trickle down, but
you'll likely need to use "git rebase --onto subsystem subsystem@{1}".
So the rebase-with-history really just saves that "use --onto and the
right <upstream>" from the hard case. The plain base-tracking does the
same.
Another way to reach the same goal would be just to explictly override
the old history.
m---m---m (master)
\
o---o (subsystem)
\
*---* (topic)
(Hypothetical): git rebase --override master subsystem
Leads to:
m---m---m---- (master)
\ \
o---o---O---o'--o' (subsystem)
\
*---* (topic)
Where O is an --ours merge, that just marks the old o commits as merged,
but has the same tree as the last m commit.
Now topic can be rebase using: git rebase --override subsystem topic
m---m---m---- (master)
\ \
o---o---O---o'--o' (subsystem)
\ \
*---*-------X---*'--*' (topic)
Again, X being an ours merge.
As the O and X commits have the last o and * commits as their second
parents, this even doesn't break things like "git show" and "git log
-p", as the interesting commits aren't merge commits. So "git
log -p --first-parent subsystem..topic" would do the right thing
(optionally with --no-merges to avoid the merge commit, but seeing that
doesn't hurt that much I guess).
This also trivially supports the reorder, squash, edit whatever stuff,
as it doesn't rely on 1:1 commit counterparts to exist. But it also
falls flat on its face as soon as subsystem gets "really" rewritting, so
that the old history is no longer reachable from the new history.
Björn
^ permalink raw reply
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Junio C Hamano @ 2009-08-15 3:16 UTC (permalink / raw)
To: Brandon Casey
Cc: Junio C Hamano, Linus Torvalds, Johannes Schindelin, nico, git,
Brandon Casey
In-Reply-To: <MecVLMUB15cEJQiZpwSBg9ysnmFSB1QYZfIQuSOStnFx2KoeScNXGQ@cipher.nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> I was just pointing out that the SUNWspro compiler does not produce an
> identical binary on x86 for each of the three versions like the GNU
> compiler does.
>
> Sorry for the confusion. Linus's suggestion of casting to "const char*"
> should be adopted.
Ah, I see. Thanks.
Let's queue this, then.
-- >8 --
From: Brandon Casey <drafnel@gmail.com>
Date: Fri, 14 Aug 2009 17:52:15 -0500
Subject: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void * to char *
Some compilers produce errors when arithmetic is attempted on pointers to
void. We want computations done on byte addresses, so cast them to char *
to work them around.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
block-sha1/sha1.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index e5a1007..464cb25 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -246,14 +246,14 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
memcpy(lenW + (char *)ctx->W, data, left);
lenW = (lenW + left) & 63;
len -= left;
- data += left;
+ data = ((const char *)data + left);
if (lenW)
return;
blk_SHA1_Block(ctx, ctx->W);
}
while (len >= 64) {
blk_SHA1_Block(ctx, data);
- data += 64;
+ data = ((const char *)data + 64);
len -= 64;
}
if (len)
^ permalink raw reply related
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Brandon Casey @ 2009-08-15 2:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Johannes Schindelin, nico, git, Brandon Casey
In-Reply-To: <7v8whl7ss9.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
>
>> Linus Torvalds wrote:
>>> I suspect it might as well be cast to "const char *", and then
>>> hopefully you only need one cast.
>>>
>>> So maybe it could be written as
>>>
>>> data = (const char *) data + len;
>>>
>>> instead, and avoid the second cast (because the assignment should be ok,
>>> since it's assigning back to a "const void *").
>> Yep, that's enough. It produces an identical binary on all platforms
>> except Solaris x86 using SUNWspro compiler.
>
> Casting to "const char *" to tell compilers that we are interested in byte
> address differences/increments makes sense to me,
Yes, I agree.
> but I do not know how to interpret your last comment.
> Do you mean that SUNWspro compiler misbehave with the "cast to char *" and
> do you meed your "casting to uintptr_t to explicitly compute byte
> addresses as int" to make it behave?
No, sorry, the SUNWspro compiler produces a working binary with any of the
three versions: the original, cast to uintptr_t, or cast to const char*.
Though it emits a warning with the original.
I was just pointing out that the SUNWspro compiler does not produce an
identical binary on x86 for each of the three versions like the GNU
compiler does.
Sorry for the confusion. Linus's suggestion of casting to "const char*"
should be adopted.
-brandon
^ permalink raw reply
* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Junio C Hamano @ 2009-08-15 2:01 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Nguyen Thai Ngoc Duy, git, Johannes Schindelin
In-Reply-To: <200908142223.07994.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
>>> Hmmm... this looks like either argument for introducing --full option
>>> to git-checkout (ignore CE_VALID bit, checkout everything, and clean
>>> CE_VALID (?))...
>>>
>>> ...or for going with _separate_ bit for partial checkout, like in the
>>> very first version of this series, which otherwise functions like
>>> CE_VALID, or is just used to mark that CE_VALID was set using sparse.
How would a separate bit help? Just like you need to clear CE_VALID bit
to revert the index into a normal (or "non sparse") state somehow, you
would need to have a way to clear that separate bit anyway.
A separate bit would help only if you want to handle assume-unchanged and
sparse checkout independently. But my impression was that the recent lstat
reduction effort addressed the issue assume-unchanged were invented to
work around in the first place.
Cf. http://thread.gmane.org/gmane.comp.version-control.git/123218/focus=123252
There is no reason to use assume-unchanged to tell git not to lstat to see
if a path is up-to-date by promising that you are not going to touch it
after you checked it out.
So I do not understand why you would want a separate bit, nor why you
think a separate bit would help when changing the index state from sparse
to non-sparse (or vice versa).
^ permalink raw reply
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Junio C Hamano @ 2009-08-15 2:01 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Jakub Narebski, Git List
In-Reply-To: <7v7hx6suiy.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> One more thing. I suspect that adding --delete and nothing else probably
> makes things worse than not doing anything.
>
> In a mental model where "push there --delete $branch" is natural, there
> are branches on the other side, and when you run 'push' command, you name
> the special operation, 'delete', that you would want to inflict on them.
>
> In such a world, there probably are other (perhaps not so special)
> operations you can inflict on the branches on the other side as well.
> They are probably called something like:
>
> push there --create $branch $commit
> push there --update $branch $commit
>
> If you give them only --delete without completing the vocabulary by giving
> these operations as well, you would force people to mix "git native" world
> model (i.e. there is no "mode" nor "opration"; there is only "list of
> instructions, each of which encodes the equivalent of 'mode'") with this
> Hg-inspired world model.
One final note on this topic. A more problematic is --rename.
In the "there are branches on the other side, and a push command can be
told to operate on them in different ways" world model, you naturally
would want to:
push there --delete $branch
push there --rename $old $new
push there --copy $old $new
The first one you can implement as a syntax sugar by turning it into the
native way of pushing ":$branch". You however cannot shoehorn rename and
copy, because the way in which git push works does not have direct access
to their "original" values of remote branches. If we do not have the
current object $old points at, you cannot emulate "rename $old $new" nor
"copy $old $new" only with simple syntax sugarcoating. We would likely
need a protocol extension to fit them in, and that is of dubious value.
Among the ones Jakub listed in another message:
1. "<scm> init <directory>" (done)
2. "hg commit --close-branch" vs slightly cryptic
"git push origin :refs/heads/feature-tweak", which can be written
simply as "git push origin :feature-tweak" I think.
(that is what this patch series is about)
3. Numeric local references, e.g. 18:a432bc and "hg checkout 18"...
but more realistic example would be "hg checkout 6324" :-P
4. sensible defaults: meaning of revert, staging area (i.e. commit -a)
5. "hg serve" (gitweb and a kind of git-daemon equivalent)
I think only #1 and #5 make sense (and I wonder if it would be enough to
mention "instaweb" and "daemon" to cover #5).
As we discussed in this thread at length, #2 is on the borderline. It
makes sense if you take only --delete out of possible vocabulary, but when
you think about it as a part of a coherent whole, it becomes somewhat
iffy---it does not fit particularly well with other parts of the system.
I think #3 and #4 comes from fundamental difference of the world views.
Regarding #3, giving monotonically increasing numbers to local commits
would not be very hard without breaking the underlying data model. You
would automatically tag the original commit whenever a new branch is
created, and count commits relative to that origin-point tag for the
current branch, perhaps following only the first parent chain, or
something like that.
For such a incrementing number be any useful, the user's history should
rarely rewind, and this also introduces a notion/illusion that a branch
somehow has its own identity, which we deliberately have rejected. I doubt
this is worth it. Most of the time you are interested in the recent past,
and HEAD~14 would be far easier to use than r19354 anyway ;-)
About #4, in general, I do not think it is worth discussing a topic that
begins with the word "sensible" when the person who uses that word does
not realize that what is sensible is in the eyes of beholder. Different
SCMs use "revert" to mean different things, because there are a lot of
combinations of _things_ you would want to revert, _states_ you would want
to revert to, and _ways_ you would want the revert to be expressed. You
may be familiar with the way BK used the word revert, or you may be
familiar with the way SVN used the word revert. There is no single
"right" use of the word.
It also is not worth repeating the discussion on fear of index, either.
This is a good example of why you should _think_ before blindly parrotting
whatever random people say on the net. They have not necessarily thought
things through before saying "git can learn from X". You need to do the
thinking for them to decide if they are making any sense when you read
these things.
^ permalink raw reply
* Re: What's cooking in git.git (Aug 2009, #02; Wed, 12)
From: Junio C Hamano @ 2009-08-15 1:51 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m3k516nic0.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
>> * jn/gitweb-blame (Thu Aug 6 19:11:52 2009 +0200) 14 commits
>> - gitweb: Create links leading to 'blame_incremental' using
>> JavaScript
>> - gitweb: Incremental blame (proof of concept)
>> - gitweb: Add optional "time to generate page" info in footer
>> + Revert the four topmost commits from jn/gitweb-blame topic
>> + gitweb: Create links leading to 'blame_incremental' using
>> JavaScript
>> + gitweb: Incremental blame (proof of concept)
>> + gitweb: Add optional "time to generate page" info in footer
>> + gitweb: Add -partial_query option to href() subroutine
>> + gitweb: Use light/dark for class names also in 'blame' view
>> + gitweb: Add author initials in 'blame' view, a la "git gui blame"
>> + gitweb: Mark commits with no "previous" in 'blame' view
>> + gitweb: Use "previous" header of git-blame -p in 'blame' view
>> + gitweb: Mark boundary commits in 'blame' view
>> + gitweb: Make .error style generic
>>
>> I haven't picked up Jakub's replacement to the second one from the tip.
>> We probably should merge up to "Use light/dark" (aef3768) to 'master' and
>> start the rest over.
>
> That would be good idea.
Thanks, will do.
> What about 'gitweb: fix variable scoping issue in git_get_projects_list'
> latest version of patch, adding (re)declaring $project_maxdepth,
> $projectroot as global variables using our? Or are you waiting for
> response of our resident Perl expert: Randal L. Schwartz (merlyn)?
Not in my queue, but no need to resend --- I can find it.
^ permalink raw reply
* Re: [PATCH] git stash: Give friendlier errors when there is nothing to apply
From: Junio C Hamano @ 2009-08-15 1:50 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git, Ori Avtalion
In-Reply-To: <20090815063956.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Quoting Junio C Hamano <gitster@pobox.com>
> ...
>> A tangent; we might want an analogue to "shortlog -s -n" but based on
>> "blame".
>
> I'm sorry, I don't understand what you mean.
I often use shortlog to find whom to CC to, but sometimes blame output
gives us a much better picture.
$ git shortlog -s -n git-stash.sh | head -n 6
22 Junio C Hamano
4 Brandon Casey
2 Jeff King
2 Johannes Schindelin
2 Nanako Shiraishi
2 SZEDER Gábor
$ git blame -w git-stash.sh |
sed -e 's/^[0-9a-f]* (\(.*\) *200[7-9]-..-.. .*/\1/' |
sort | uniq -c | sort -n -r | head -n 6
110 Nanako Shiraishi
86 Junio C Hamano
55 Stephen Boyd
33 Brandon Casey
20 Abhijit Menon-Sen
16 Johannes Schindelin
^ permalink raw reply
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Junio C Hamano @ 2009-08-15 1:46 UTC (permalink / raw)
To: Brandon Casey
Cc: Linus Torvalds, Johannes Schindelin, nico, git, Brandon Casey
In-Reply-To: <5HEgWEs64y4Ty7FnekstRP8Q71pNi_FTweLbMhYWc6zoZBA4jkJqpQ@cipher.nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> Linus Torvalds wrote:
>> I suspect it might as well be cast to "const char *", and then
>> hopefully you only need one cast.
>>
>> So maybe it could be written as
>>
>> data = (const char *) data + len;
>>
>> instead, and avoid the second cast (because the assignment should be ok,
>> since it's assigning back to a "const void *").
>
> Yep, that's enough. It produces an identical binary on all platforms
> except Solaris x86 using SUNWspro compiler.
Casting to "const char *" to tell compilers that we are interested in byte
address differences/increments makes sense to me, but I do not know how to
interpret your last comment.
Do you mean that SUNWspro compiler misbehave with the "cast to char *" and
do you meed your "casting to uintptr_t to explicitly compute byte
addresses as int" to make it behave?
^ permalink raw reply
* Re: http-push sends MKCOL command to create remote directory MKCOL http://user@server:80/url/.git/refs/ that already exists
From: Tay Ray Chuan @ 2009-08-15 1:28 UTC (permalink / raw)
To: willievu; +Cc: git
In-Reply-To: <1250264521299-3445796.post@n2.nabble.com>
Hi,
On Fri, Aug 14, 2009 at 11:42 PM, willievu<willievu@dev.java.net> wrote:
> Is MKCOL expected even for existing directory in the remote repository? Is
> this a WebDAV server problem that 500 is returned?
Yes, you should expect MKCOLs to be done on directories that already
exist. On my Apache server with WebDAV, when a MKCOL is done on a
directory that already exists, it returns 405. Reading the WebDAV
specs (http://www.webdav.org/specs/rfc4918.html#rfc.section.9.3.1):
405 (Method Not Allowed) - MKCOL can only be executed on an unmapped URL.
So this behaviour is accounted for in the spec itself, and git handles this too.
What I think is that your server isn't implementing WebDAV right, or
that you have some scripts/mods over the git repository you're serving
that are failing leading to the 500 error.
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Brandon Casey @ 2009-08-15 0:52 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: nico, gitster, torvalds, git, Brandon Casey
In-Reply-To: <alpine.DEB.1.00.0908150207270.8306@pacific.mpi-cbg.de>
Johannes Schindelin wrote:
> Hi,
>
> On Fri, 14 Aug 2009, Brandon Casey wrote:
>
>> Some compilers produce errors when arithmetic is attempted on pointers to
>> void. So cast to uintptr_t when performing arithmetic on void pointers.
>
> I am confused. Is sizeof(*(uintptr_t)NULL) not larger than 1, and as a
> consequence ((uintptr_t)p)+1 not different from ((void *)p)+1?
If you try this:
printf("NULL + 1: %u\n", (void*)NULL + 1);
the MIPSpro compiler complains like:
The expression must be a pointer to a complete object type.
printf("NULL + 1: %u\n", (void*)NULL + 1);
^
1 error detected in the compilation of "test.c".
Compilers other than gcc at least issue a warning, if they do
not also fail.
-brandon
^ permalink raw reply
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Brandon Casey @ 2009-08-15 0:44 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Johannes Schindelin, nico, gitster, git, Brandon Casey
In-Reply-To: <alpine.LFD.2.01.0908141714040.3162@localhost.localdomain>
Linus Torvalds wrote:
> I suspect it might as well be cast to "const char *", and then
> hopefully you only need one cast.
>
> So maybe it could be written as
>
> data = (const char *) data + len;
>
> instead, and avoid the second cast (because the assignment should be ok,
> since it's assigning back to a "const void *").
Yep, that's enough. It produces an identical binary on all platforms
except Solaris x86 using SUNWspro compiler.
-brandon
^ permalink raw reply
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Linus Torvalds @ 2009-08-15 0:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Brandon Casey, nico, gitster, git, Brandon Casey
In-Reply-To: <alpine.DEB.1.00.0908150207270.8306@pacific.mpi-cbg.de>
On Sat, 15 Aug 2009, Johannes Schindelin wrote:
>
> I am confused. Is sizeof(*(uintptr_t)NULL) not larger than 1
You can't do that. "uintptr_t" isn't actually a pointer. It's just a
unsigned integer value large enough to contain a pointer.
So it's _not_ a "pointer to uint". It's "uint that can contain all the
bits of a poitner".
> and as a consequence ((uintptr_t)p)+1 not different from ((void *)p)+1?
No, they're the same.
That said, I suspect it might as well be cast to "const char *", and then
hopefully you only need one cast.
So maybe it could be written as
data = (const char *) data + len;
instead, and avoid the second cast (because the assignment should be ok,
since it's assigning back to a "const void *").
Linus
^ permalink raw reply
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Johannes Schindelin @ 2009-08-15 0:08 UTC (permalink / raw)
To: Brandon Casey; +Cc: nico, gitster, torvalds, git, Brandon Casey
In-Reply-To: <_-0l9qI_nKm-VVtKlL5hzen4bhcyRmiZ1z4jSmKK1LwECXueziRDzSTyacEehOkoqsfYIaEUlds@cipher.nrlssc.navy.mil>
Hi,
On Fri, 14 Aug 2009, Brandon Casey wrote:
> Some compilers produce errors when arithmetic is attempted on pointers to
> void. So cast to uintptr_t when performing arithmetic on void pointers.
I am confused. Is sizeof(*(uintptr_t)NULL) not larger than 1, and as a
consequence ((uintptr_t)p)+1 not different from ((void *)p)+1?
Ciao,
Dscho
^ permalink raw reply
* Linus' sha1 is much faster!
From: Pádraig Brady @ 2009-08-14 23:25 UTC (permalink / raw)
To: Bug-coreutils, Linus Torvalds; +Cc: Git Mailing List
[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]
I've noticed before that coreutils hashing utils
were a little behind in performance, but was prompted
to look at it again when I noticed the recently
updated sha1 implementation in git:
http://git.kernel.org/?p=git/git.git;a=history;f=block-sha1;h=d3121f7;hb=pu
Testing that with the attached program which I wrote
in a couple of mins to try and match sha1sum's system calls
shows that it's around 33% faster, as shown below:
$ gcc $(rpm -q --qf="%{OPTFLAGS}\n" coreutils) linus-sha1.c sha1.c -o linus-sha1
$ time ./linus-sha1 300MB_file
df1e19e245fee4f53087b50ef953ca2c8d1644d7 300MB_file
real 0m2.742s
user 0m2.516s
sys 0m0.206s
$ time ~/git/coreutils/src/sha1sum 300MB_file
df1e19e245fee4f53087b50ef953ca2c8d1644d7 300MB_file
real 0m4.166s
user 0m3.846s
sys 0m0.298s
So, could we use that code in coreutils?
Think of all the dead fish it would save.
I've also attached a trivial block-sha1 patch which doesn't
affect performance, but does suppress a signed unsigned
comparison warning which occurs with -Wextra for example.
cheers,
Pádraig.
[-- Attachment #2: linus-sha1.c --]
[-- Type: text/x-csrc, Size: 681 bytes --]
/* gcc -O2 -Wall linus-sha1.c sha1.c -o linus-sha1 */
#include <stdio.h>
#include <stdlib.h>
#include "sha1.h"
int main(int argc, char** argv)
{
if (argc != 2) return 1;
const char* filename = argv[1];
FILE *fp = fopen (filename, "r");
if (!fp) return 1;
#define BS 4096 /* match coreutils */
blk_SHA_CTX ctx;
blk_SHA1_Init(&ctx);
size_t nr;
char buf[BS];
while ((nr=fread_unlocked(buf, 1, sizeof(buf), fp)))
blk_SHA1_Update(&ctx, buf, nr);
unsigned char hash[20];
blk_SHA1_Final(hash, &ctx);
int i;
for (i=0; i<sizeof(hash); i++)
printf("%02x",*(hash+i));
printf(" %s\n", filename);
return 0;
}
[-- Attachment #3: block-sha1-signed-warning.diff --]
[-- Type: text/x-patch, Size: 998 bytes --]
>From fa75e818836f763357ff9b7bbde3327e1aabbe47 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <P@draigBrady.com>
Date: Sat, 15 Aug 2009 00:17:30 +0100
Subject: [PATCH] block-sha1: suppress signed unsigned comparison warning
* block-sha1/sha1.c: Use unsigned ints as the values
will never go negative.
---
block-sha1/sha1.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index d3121f7..be763d8 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -231,13 +231,13 @@ void blk_SHA1_Init(blk_SHA_CTX *ctx)
void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
{
- int lenW = ctx->size & 63;
+ unsigned int lenW = ctx->size & 63;
ctx->size += len;
/* Read the data into W and process blocks as they get full */
if (lenW) {
- int left = 64 - lenW;
+ unsigned int left = 64 - lenW;
if (len < left)
left = len;
memcpy(lenW + (char *)ctx->W, data, left);
--
1.6.2.5
^ permalink raw reply related
* Re: [PATCH 1/5] port --ignore-unmatch to "git add"
From: Luke-Jr @ 2009-08-14 22:54 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Junio C Hamano, Thomas Rast, Luke Dashjr, git
In-Reply-To: <20090815063958.6117@nanako3.lavabit.com>
On Friday 14 August 2009 04:39:58 pm Nanako Shiraishi wrote:
> Second of all, if a race condition makes an "add" fail, isn't it a good
> thing? If your "add" ignored such a failure, you'd be recording an
> inconsistent or incomplete state.
If the file doesn't exist, it is no longer in the current state.
> IMHO, fixing your racy script is a much cleaner solution to your problem
> than forcing "git add" to ignore errors.
In this situation, the race condition is not a bug, it is normal operation.
^ permalink raw reply
* [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Brandon Casey @ 2009-08-14 22:52 UTC (permalink / raw)
To: nico; +Cc: gitster, torvalds, git, Brandon Casey
In-Reply-To: <alpine.LFD.2.00.0908130017080.10633@xanadu.home>
From: Brandon Casey <drafnel@gmail.com>
Some compilers produce errors when arithmetic is attempted on pointers to
void. So cast to uintptr_t when performing arithmetic on void pointers.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
This, on top of Nico's last patch "block-sha1: more good unaligned memory
access candidates", allows this series to compile and run successfully on
Sun Sparc and IRIX MIPS for me.
It produces no differences on Linux using gcc. gcc 3.4.6 and 4.1.2 produce
an identical binary for me on x86 and x86-64 with and without this change.
Any conceivable negative effects on other arch's?
-brandon
block-sha1/sha1.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index e5a1007..ccaba9e 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -246,14 +246,14 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
memcpy(lenW + (char *)ctx->W, data, left);
lenW = (lenW + left) & 63;
len -= left;
- data += left;
+ data = (const void*) ((uintptr_t) data + left);
if (lenW)
return;
blk_SHA1_Block(ctx, ctx->W);
}
while (len >= 64) {
blk_SHA1_Block(ctx, data);
- data += 64;
+ data = (const void*) ((uintptr_t) data + 64);
len -= 64;
}
if (len)
--
1.6.4
^ permalink raw reply related
* Re: What's cooking in git.git (Aug 2009, #02; Wed, 12)
From: Jakub Narebski @ 2009-08-14 22:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtz0co3xe.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> * mr/gitweb-xz (Thu Aug 6 10:28:25 2009 -0400) 3 commits
> - gitweb: add support for XZ compressed snapshots
> - gitweb: update INSTALL regarding specific snapshot settings
> - gitweb: support to globally disable a snapshot format
>
> This should be safe as long as it does not silently enable itself for
> folks who merely updated gitweb to this version without explicitly asking
> for the new format (but I do not remember if that is what the patch does).
The latest version (v5, from 6.08) has XZ compressed snapshot ('txz')
disabled by default.
> * zf/maint-gitweb-acname (Sun Aug 2 09:42:24 2009 +0200) 1 commit
> - gitweb: parse_commit_text encoding fix
Good.
> * jn/gitweb-blame (Thu Aug 6 19:11:52 2009 +0200) 14 commits
> - gitweb: Create links leading to 'blame_incremental' using
> JavaScript
> - gitweb: Incremental blame (proof of concept)
> - gitweb: Add optional "time to generate page" info in footer
> + Revert the four topmost commits from jn/gitweb-blame topic
> + gitweb: Create links leading to 'blame_incremental' using
> JavaScript
> + gitweb: Incremental blame (proof of concept)
> + gitweb: Add optional "time to generate page" info in footer
> + gitweb: Add -partial_query option to href() subroutine
> + gitweb: Use light/dark for class names also in 'blame' view
> + gitweb: Add author initials in 'blame' view, a la "git gui blame"
> + gitweb: Mark commits with no "previous" in 'blame' view
> + gitweb: Use "previous" header of git-blame -p in 'blame' view
> + gitweb: Mark boundary commits in 'blame' view
> + gitweb: Make .error style generic
>
> I haven't picked up Jakub's replacement to the second one from the tip.
> We probably should merge up to "Use light/dark" (aef3768) to 'master' and
> start the rest over.
That would be good idea.
[...]
What about 'gitweb: fix variable scoping issue in git_get_projects_list'
latest version of patch, adding (re)declaring $project_maxdepth,
$projectroot as global variables using our? Or are you waiting for
response of our resident Perl expert: Randal L. Schwartz (merlyn)?
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* RE : trailling whitespace with git am ?
From: PICCA Frédéric-Emmanuel @ 2009-08-14 22:21 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, git
In-Reply-To: <81b0412b0908141516t757d2cb1tc6d0ff3805691418@mail.gmail.com>
> As it all is history (in the new code) already, there is nothing to talk about.
> Git-am in "next" branch just accepts such a patch without a warning.
ok no problem, I will waite for the "next" version :)
thanks
Frederic
^ permalink raw reply
* Re: RE : trailling whitespace with git am ?
From: Alex Riesen @ 2009-08-14 22:16 UTC (permalink / raw)
To: PICCA Frédéric-Emmanuel; +Cc: Junio C Hamano, git
On Fri, Aug 14, 2009 at 23:50, PICCA
Frédéric-Emmanuel<frederic-emmanuel.picca@synchrotron-soleil.fr>
wrote:
> Maybe git apply should warn the user a different way if their is that sort of
> dos/unix imcompatibilities.
As it all is history (in the new code) already, there is nothing to talk about.
Git-am in "next" branch just accepts such a patch without a warning.
^ 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