* Re: Patterns work unexpectedly with "git log" commit limiting
From: Junio C Hamano @ 2008-07-17 8:17 UTC (permalink / raw)
To: Teemu Likonen; +Cc: git
In-Reply-To: <20080717074706.GA5392@mithlond.arda.local>
Teemu Likonen <tlikonen@iki.fi> writes:
> 1. Option order changes the behaviour. "git log" with
> ...
> 2. Internally --author= and --committer= fields contain more stuff than
> ...
> 3. What is the supposed behaviour of -F (--fixed-strings) when combined
I won't have time right now to comment on the previous three, because they
are not my code (meaning, I need to dig around before I can intelligently
answer -- maybe when I have free time). What you expected does sound very
sensible, though.
> 4. Logical AND/OR operation.
The --grep and --author search of "log" family shares git-grep logical
expression engine, so if you somehow can come up with a way to pass --and
(or even better, --all-match) from the command line just like we can give
them to git-grep, searching with combinations of and/or/not should be doable.
^ permalink raw reply
* What's cooking in git.git (topics)
From: Junio C Hamano @ 2008-07-17 8:08 UTC (permalink / raw)
To: git
In-Reply-To: <7vfxqawlja.fsf@gitster.siamese.dyndns.org>
Here are the topics that have been cooking. Commits prefixed
with '-' are only in 'pu' while commits prefixed with '+' are
in 'next'.
The topics list the commits in reverse chronological order. The topics
meant to be merged to the maintenance series have "maint-" in their names.
Right now 'next' is very thin. After today's new topics, perhaps except
for the submodule stuff by Pasky, are merged to 'master', we will have the
1.6.0-rc0, and from there the usual pre-release freeze begins.
Due to increased activity level from people including GSoC students, I
expect 'next' to stay somewhat more active than previous rounds during the
1.6.0-rc cycle. The request for people who usually follow 'next' is the
same as usual, though. After -rc1 is tagged, please run 'master' for your
daily git use instead, in order to make sure 'master' does what it claims
to do without regression.
Tentative schedule, my wishful thinking:
- 1.6.0-rc0 (Jul 20)
- 1.6.0-rc1 (Jul 23)
- 1.6.0-rc2 (Jul 30)
- 1.6.0-rc3 (Aug 6)
- 1.6.0 (Aug 10)
----------------------------------------------------------------
[New Topics]
* jc/rerere-auto-more (Wed Jul 16 20:25:18 2008 -0700) 1 commit
- rerere.autoupdate: change the message when autoupdate is in effect
This one is for Ingo.
This changes the message rerere issues after reusing previous conflict
resolution from "Resolved" to "Staged" when autoupdate option is in
effect.
It is envisioned that in practice, some auto resolutions are trickier and
iffier than others, and we would want to add a feature to mark individual
resolutions as "this is ok to autoupdate" or "do not autoupdate the result
using this resolution even when rerere.autoupdate is in effect" in the
future. When that happens, these messages will make the distinction
clearer.
* ap/trackinfo (Wed Jul 16 15:19:27 2008 -0400) 1 commit
- Reword "your branch has diverged..." lines to reduce line length
You saw the exchange on the list. Queued is my "make it shorter and make
sure variable parts are closer to left edge of the screen" version but
better alternatives are welcome. I suspect not many people would care too
much about details, as long as the message fits and does not waste screen
real estate.
* ns/am-abort (Wed Jul 16 19:39:10 2008 +0900) 1 commit
- git am --abort
This one is for Ted; builds on top of the recent "am and rebase leaves
ORIG_HEAD just like reset, merge and pull does" rather nicely.
* pb/submodule (Wed Jul 16 21:11:40 2008 +0200) 7 commits
- t7403: Submodule git mv, git rm testsuite
- git rm: Support for removing submodules
- git mv: Support moving submodules
- submodule.*: Introduce simple C interface for submodule lookup by
path
- git submodule add: Fix naming clash handling
- t7400: Add short "git submodule add" testsuite
- git-mv: Remove dead code branch
Long overdue usability improvement series for submodule. Very much
welcomed. It would be nice to have some submodule improvements in 1.6.0.
Realistically speaking, however, I predict that it would take us a few
more rounds to hit 'next' with this, and it will not be in 'master' when
1.6.0 ships.
----------------------------------------------------------------
[Graduated to "master"]
* sp/maint-index-pack (Tue Jul 15 04:45:34 2008 +0000) 4 commits
+ index-pack: Honor core.deltaBaseCacheLimit when resolving deltas
+ index-pack: Track the object_entry that creates each base_data
+ index-pack: Chain the struct base_data on the stack for traversal
+ index-pack: Refactor base arguments of resolve_delta into a struct
* rs/rebase-checkout-not-so-quiet (Mon Jul 14 14:05:35 2008 -0700) 1 commit
+ git-rebase: report checkout failure
* ag/blame (Wed Jul 16 02:00:58 2008 +0400) 2 commits
+ Do not try to detect move/copy for entries below threshold.
+ Avoid rescanning unchanged entries in search for copies.
This gives a drastic performance improvement to "git-blame -C -C" with
quite straightforward and obvious code change.
* rs/archive (Mon Jul 14 21:22:05 2008 +0200) 6 commits
+ archive: remove extra arguments parsing code
+ archive: unify file attribute handling
+ archive: centralize archive entry writing
+ archive: add baselen member to struct archiver_args
+ add context pointer to read_tree_recursive()
+ archive: remove args member from struct archiver
* sb/dashless (Sun Jul 13 15:36:15 2008 +0200) 3 commits
+ Make usage strings dash-less
+ t/: Use "test_must_fail git" instead of "! git"
+ t/test-lib.sh: exit with small negagive int is ok with
test_must_fail
* mv/dashless (Fri Jul 11 02:12:06 2008 +0200) 4 commits
+ make remove-dashes: apply to scripts and programs as well, not
just to builtins
+ git-bisect: use dash-less form on git bisect log
+ t1007-hash-object.sh: use quotes for the test description
+ t0001-init.sh: change confusing directory name
* ls/mailinfo (Sun Jul 13 20:30:12 2008 +0200) 3 commits
+ git-mailinfo: use strbuf's instead of fixed buffers
+ Add some useful functions for strbuf manipulation.
+ Make some strbuf_*() struct strbuf arguments const.
This actually had a tiny regression I did not discover until I merged it
to 'master', where a fixup has already been applied.
----------------------------------------------------------------
[On Hold]
* rs/imap (Wed Jul 9 22:29:02 2008 +0100) 5 commits
- Documentation: Improve documentation for git-imap-send(1)
- imap-send.c: more style fixes
- imap-send.c: style fixes
- git-imap-send: Support SSL
- git-imap-send: Allow the program to be run from subdirectories of
a git tree
I said: "Some people seem to prefer having this feature available also
with gnutls. If such a patch materializes soon, that would be good, but
otherwise I'll merge this as-is to 'next'. Such an enhancement can be
done in-tree on top of this series." Anybody?
* xx/merge-in-c-into-next (Wed Jul 9 13:51:46 2008 -0700) 4 commits
+ Teach git-merge -X<option> again.
+ Merge branch 'jc/merge-theirs' into xx/merge-in-c-into-next
+ builtin-merge.c: use parse_options_step() "incremental parsing"
machinery
+ Merge branch 'ph/parseopt-step-blame' into xx/merge-in-c-into-next
This needs to be merged to master iff/when merge-theirs gets merged,
but I do not think this series is widely supported, so both are on hold.
* jc/merge-theirs (Mon Jun 30 22:18:57 2008 -0700) 5 commits
+ Make "subtree" part more orthogonal to the rest of merge-
recursive.
+ Teach git-pull to pass -X<option> to git-merge
+ Teach git-merge to pass -X<option> to the backend strategy module
+ git-merge-recursive-{ours,theirs}
+ git-merge-file --ours, --theirs
Punting a merge by discarding your own work in conflicting parts but still
salvaging the parts that are cleanly automerged. It is likely that this
will result in nonsense mishmash, but somehow often people want this, so
here they are. The interface to the backends is updated so that you can
say "git merge -Xours -Xsubtree=foo/bar/baz -s recursive other" now.
* sg/merge-options (Sun Apr 6 03:23:47 2008 +0200) 1 commit
+ merge: remove deprecated summary and diffstat options and config
variables
This was previously in "will be in master soon" category, but it turns out
that the synonyms to the ones this one deletes are fairly new invention
that happend in 1.5.6 timeframe, and we cannot do this just yet. Perhaps
in 1.7.0.
* jc/dashless (Thu Jun 26 16:43:34 2008 -0700) 2 commits
+ Revert "Make clients ask for "git program" over ssh and local
transport"
+ Make clients ask for "git program" over ssh and local transport
This is the "botched" one. Will be resurrected during 1.7.0 or 1.8.0
timeframe.
* jk/renamelimit (Sat May 3 13:58:42 2008 -0700) 1 commit
- diff: enable "too large a rename" warning when -M/-C is explicitly
asked for
This would be the right thing to do for command line use, but gitk will be
hit due to tcl/tk's limitation, so I am holding this back for now.
----------------------------------------------------------------
[Stalled/Needs more work]
* gi/cherry-cache (Sat Jul 12 20:14:51 2008 -0700) 1 commit
. cherry: cache patch-ids to avoid repeating work
The discussion suggested that the value of having the cache itself is
iffy, but I should pick up the updated one and look at it.
* lw/gitweb (Fri Jul 11 03:11:48 2008 +0200) 3 commits
. gitweb: use new Git::Repo API, and add optional caching
. Add new Git::Repo API
. gitweb: add test suite with Test::WWW::Mechanize::CGI
* sb/sequencer (Tue Jul 1 04:38:34 2008 +0200) 4 commits
. Migrate git-am to use git-sequencer
. Add git-sequencer test suite (t3350)
. Add git-sequencer prototype documentation
. Add git-sequencer shell prototype
I haven't looked at the updated series yet. I should, but nobody else
seems to be looking at these patches, which is somewhat depressing but
understandable. Summer is slower ;-)
* jc/grafts (Wed Jul 2 17:14:12 2008 -0700) 1 commit
- [BROKEN wrt shallow clones] Ignore graft during object transfer
Cloning or fetching from a repository from grafts did not send objects
that are hidden by grafts, but the commits in the resulting repository do
need these to pass fsck. This fixes object transfer to ignore grafts.
Another fix is needed to git-prune so that it ignores grafts but treats
commits that are mentioned in grafts as reachable.
* jc/blame (Wed Jun 4 22:58:40 2008 -0700) 2 commits
- blame: show "previous" information in --porcelain/--incremental
format
- git-blame: refactor code to emit "porcelain format" output
This is for peeling the line from the blamed version to see what's behind
it, which may or may not help applications like gitweb.
^ permalink raw reply
* (Remaining) problems with the http backend ?
From: Mike Hommey @ 2008-07-17 8:04 UTC (permalink / raw)
To: git
Hi list,
Except for the typical known stuff such as bad error reporting, people
forgetting to run git update-server-info and lack of support for
packed-refs, are there any other known issues with the http backend ?
Mike
PS: Answers to this post will be used to fill my GIT-HTTP-TODO list.
^ permalink raw reply
* Re: [PATCH 0/5] add pack index v2 reading capability to git v1.4.4.4
From: Junio C Hamano @ 2008-07-17 8:01 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nicolas Pitre, git
In-Reply-To: <alpine.LFD.1.10.0807161033170.2835@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, 16 Jul 2008, Junio C Hamano wrote:
>>
>> I do not think it should SEGV. The pack-idx signature was chosen rather
>> carefully to allow older ones to die gracefully.
>
> Well, Pasky reported differently.
>
>> error: non-monotonic index
>> error: Could not read 4a588075c54cd5902e5f4d43b9d6b0c31d0f9769
>
> Pasky's report was
>
> error: non-monotonic index
> /usr/bin/git-fetch: line 297: 30402 Segmentation fault git-http-fetch -v -a "$head" "$remote/"
>
> but maybe that was something specific to his case.
It is caused by the http walker not being careful. In v1.4.4.5
http-fetch.c, this code appears unmodified since v1.4.4.4, and an
equivalent code is still in http-walker.c in more recent versions:
static int setup_index(struct alt_base *repo, unsigned char *sha1)
{
struct packed_git *new_pack;
if (has_pack_file(sha1))
return 0; /* don't list this as something we can get */
if (fetch_index(repo, sha1))
return -1;
new_pack = parse_pack_index(sha1);
new_pack->next = repo->packs;
repo->packs = new_pack;
return 0;
}
Nico taught parse_pack_index() what v2 pack idx file looks like, but when
the code hits unknown idx file (or a corrupt one), the function signals
error by returning NULL; assigning to new_pack->next without checking
would segfault.
We would need this fix to futureproof ourselves for pack idx v3 and later,
and also for protecting from a corrupt idx file coming over the wire.
---
http-walker.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/http-walker.c b/http-walker.c
index 51c18f2..9dc6b27 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -442,6 +442,8 @@ static int setup_index(struct walker *walker, struct alt_base *repo, unsigned ch
return -1;
new_pack = parse_pack_index(sha1);
+ if (!new_pack)
+ return -1; /* parse_pack_index() already issued an error message */
new_pack->next = repo->packs;
repo->packs = new_pack;
return 0;
^ permalink raw reply related
* Patterns work unexpectedly with "git log" commit limiting
From: Teemu Likonen @ 2008-07-17 7:47 UTC (permalink / raw)
To: git
It looks to me that some commit limiting options used with "git log"
work a bit buggyish or unexpectedly. Please don't take this as a rant;
I only mean to express some unexpected behaviour in the user interface.
You can be sure that I don't understand the plumbing stuff and
implementation details.
1. Option order changes the behaviour. "git log" with
-E --author=pattern
interprets "pattern" as _basic_ regexp. To have it interpreted as
extended regexp the order must be
--author=pattern -E
BUT the "pattern" in
--author=non-matching-nonsense -E --author=pattern
is interpreted as extended regexp. So -E's behaviour depens on the
preceding options.
2. Internally --author= and --committer= fields contain more stuff than
just person's name and email address. I mean user might expect
--author='@iki.fi>$'
to match all authors with this email host/domain. It took quite some
time for me to realise that the (usually hidden) raw author field
contains also date information, such as "1216023662 +0300". Well,
not a big deal but certainly unexpected in the context of commit
limiting by author.
3. What is the supposed behaviour of -F (--fixed-strings) when combined
with --author= ?
--author=pattern -F
doesn't seem to match anything. I also tried putting the entire raw
author field (i.e. including the raw date) but no match. With -F
before the --author= it behaves like no -F at all.
"--grep=fixedstring -F" seems to work, though.
4. Logical AND/OR operation. I realised that several commit limiting
options combined together do not limit commits more but the
opposite. So it seems like a logical OR operation between the
limiting options. It's fine. It's just that I'd have expected that
more limiting options means limiting more (i.e. the AND operation,
just like in the "find" command normally).
Well, unexpected but I'll survive. Shouldn't this be mentioned under
the title "Commit Limiting" in the man page? (My English is not
really manual-writing quality but I could try to come up with
a patch.)
^ permalink raw reply
* Re: gitk: Author/Committer name with special characters
From: Torsten Paul @ 2008-07-17 7:34 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git
In-Reply-To: <18558.35423.933860.915622@cargo.ozlabs.ibm.com>
Paul Mackerras wrote:
> Something like that, I think, but to be sure I'd like to see the
> actual author and committer lines that are causing the problem. Could
> you send me the output of "git cat-file commit" on one of the
> problematic commits?
>
The header of the commits look like this:
tree 2eaf317290917660c03dc977d5eae180b39420e0
parent e99bf1c38540300c501dbe776de28eed6a2250cd
author DOM\paul <DOM\paul@28e39a90-19ad-e645-8baa-5c9ab2323746> 1216216768 +0000
committer DOM\paul <DOM\paul@28e39a90-19ad-e645-8baa-5c9ab2323746> 1216216768 +0000
With my name it's just not displaying the backslash, but with names
starting with a 't' or even better 'n' it get's more interresting
as they are shown as tab or newline. And that looks very funny in
the commit list view.
ciao,
Torsten.
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: "Peter Valdemar Mørch (Lists)" @ 2008-07-17 7:30 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.DEB.1.00.0807161804400.8950@racer>
Johannes Schindelin Johannes.Schindelin-at-gmx.de |Lists| wrote:
> there have been a number of occasions where I came across people trying to
> be helpful and teaching Git newbies a few tricks.
>
> However, in quite a number of cases, which seem to surge over the last
> weeks, I see people suggesting the use of rev-parse, ls-tree, rev-list
> etc.
As a total git newbie (5 days) coming from svn, I *am* bewildered. Even
sticking to porcelain, it is a feature-rich new tool I have in my hands!
I'm missing clarity about what is porcelain and what is plumbing. `git
help` shows
"The most commonly used git commands are:" add .. tag.
Is this list exactly the list of porcelain commands? Then say so there.
Neither `git help diff` nor `git help ls-tree` say whether they are
porcelain or plumbing commands. `git help diff` mentions git-diff-index,
which i suspect is plumbing. When I read a man page, it would be nice to
know whether a command (either the topic of the page or another
mentioned command) is intended as porcelain or not.
Also, I'm guessing that some switches for some porcelain commands have
plumbing purposes and vice versa. I hope not, but if so that would be
nice to have documented in 'git help *'
All of this of course assumes that there is consensus and a clear
distinction between what is porcelain and what is plumbing which I'm
don't even know if there is.
Peter
--
Peter Valdemar Mørch
http://www.morch.com
^ permalink raw reply
* Re: [PATCH 1/2] t/test-lib.sh: Let test_must_fail fail on signals only
From: Junio C Hamano @ 2008-07-17 7:25 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Stephan Beyer, git
In-Reply-To: <487EF31D.8090007@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> Jeff King schrieb:
>> On Wed, Jul 16, 2008 at 11:31:41PM -0700, Junio C Hamano wrote:
>>> Is it that somebody do not want 255 exit value, or anything that has 7th
>>> bit set? 2488df8 (builtin run_command: do not exit with -1., 2007-11-13)
>>> suggests otherwise at least for Windows runtime, so what we currently have
>>> that does extra truncation ourselves might be sufficient.
>>
>> Johannes will have to answer that; however, the truncation there does
>> leave the extra 7th bit. Maybe & 0x7f would be more appropriate?
>
> I never found out the real reason why -1 would not be recognized as
> "failure"; the conclusion of my debugging session was that MSYS bash has
> an issue, and I chose to append '& 0xff' because the documentation of
> WEXITSTATUS() says that it can receive only 8 bits of the exit() code. The
> intention of 2488df8 was to keep as much information as possible. But if
> that extra information hurts, we should better truncate to 7 bits.
>
> The source code of Windows's C runtime suggests that any value that fits
> in 4 bytes can be supplied to exit() and can be received by cwait()
> (Windows's version of waitpid()); but I haven't looked at how MSYS
> implements waitpit() and whether it can receive that much information.
Well, POSIX cannot do that much anyway, but does allow 8-bit, so I'd say
the current code is fine.
^ permalink raw reply
* Re: [PATCH 1/2] t/test-lib.sh: Let test_must_fail fail on signals only
From: Johannes Sixt @ 2008-07-17 7:22 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Stephan Beyer, git
In-Reply-To: <20080717063856.GA10450@sigill.intra.peff.net>
Jeff King schrieb:
> On Wed, Jul 16, 2008 at 11:31:41PM -0700, Junio C Hamano wrote:
>> Is it that somebody do not want 255 exit value, or anything that has 7th
>> bit set? 2488df8 (builtin run_command: do not exit with -1., 2007-11-13)
>> suggests otherwise at least for Windows runtime, so what we currently have
>> that does extra truncation ourselves might be sufficient.
>
> Johannes will have to answer that; however, the truncation there does
> leave the extra 7th bit. Maybe & 0x7f would be more appropriate?
I never found out the real reason why -1 would not be recognized as
"failure"; the conclusion of my debugging session was that MSYS bash has
an issue, and I chose to append '& 0xff' because the documentation of
WEXITSTATUS() says that it can receive only 8 bits of the exit() code. The
intention of 2488df8 was to keep as much information as possible. But if
that extra information hurts, we should better truncate to 7 bits.
The source code of Windows's C runtime suggests that any value that fits
in 4 bytes can be supplied to exit() and can be received by cwait()
(Windows's version of waitpid()); but I haven't looked at how MSYS
implements waitpit() and whether it can receive that much information.
-- Hannes
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Junio C Hamano @ 2008-07-17 6:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807170152190.4318@eeepc-johanness>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > However, my point was about telling users, especially new ones.
>>
>> Perhaps you did not read my first paragraph?
>
> Well, I did.
Then perhaps I wasn't being clear, and I think we are saying the same
thing.
> Sure, advanced usage is nice, and often involves plumbing, especially for
> scripting.
That is not what I am saying. What I am saying actually is that these
usage that _need_ to involve plumbing is not "advanced", but merely
showing weakness of the current Porcelain. Fixing that would allow us
move further away from having to resort to plumbing in our daily
workflow.
Perhaps our Porcelains already passed that point, in which case you do not
have to touch the plumbing commands 99% of time during the day and you do
not have to teach new people plumbing at all. I am agreeing that it would
be a worthy goal to aim for -- I do not however think we are there yet.
^ permalink raw reply
* Re: [PATCH 1/2] t/test-lib.sh: Let test_must_fail fail on signals only
From: Jeff King @ 2008-07-17 6:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephan Beyer, Johannes Sixt, git
In-Reply-To: <7vlk01komq.fsf@gitster.siamese.dyndns.org>
On Wed, Jul 16, 2008 at 11:31:41PM -0700, Junio C Hamano wrote:
> > I started to fix the callsites that Stephan mentioned, but it really is
> > convenient to be able to 'return error("foo")' (or even return
> > func_that_calls_error(), and tracking down deep calls is time consuming
> > and error prone). So maybe we should just enhance the change from
> > 2488df84 and special case "-1" into "1"?
>
> Didn't the patch to testsuite that triggered this thread talk about "small
> negative integer" not "-1"? I suspect there might be other negative
> return values from cmd_foo(), although I haven't checked.
It did say that, but I never saw anything in the code except explicit
"return -1" and "return error()". However, some of the diff code may end
up with different values, as I didn't trace it all the way down.
Stephan?
> Is it that somebody do not want 255 exit value, or anything that has 7th
> bit set? 2488df8 (builtin run_command: do not exit with -1., 2007-11-13)
> suggests otherwise at least for Windows runtime, so what we currently have
> that does extra truncation ourselves might be sufficient.
Johannes will have to answer that; however, the truncation there does
leave the extra 7th bit. Maybe & 0x7f would be more appropriate?
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] t/test-lib.sh: Let test_must_fail fail on signals only
From: Junio C Hamano @ 2008-07-17 6:31 UTC (permalink / raw)
To: Jeff King; +Cc: Stephan Beyer, Johannes Sixt, git
In-Reply-To: <20080717060143.GA3338@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> I would however agree that when we do mean 255 we should probably write
>> 255, not (-1). It is easier to document things that way.
>
> I started to fix the callsites that Stephan mentioned, but it really is
> convenient to be able to 'return error("foo")' (or even return
> func_that_calls_error(), and tracking down deep calls is time consuming
> and error prone). So maybe we should just enhance the change from
> 2488df84 and special case "-1" into "1"?
Didn't the patch to testsuite that triggered this thread talk about "small
negative integer" not "-1"? I suspect there might be other negative
return values from cmd_foo(), although I haven't checked.
Is it that somebody do not want 255 exit value, or anything that has 7th
bit set? 2488df8 (builtin run_command: do not exit with -1., 2007-11-13)
suggests otherwise at least for Windows runtime, so what we currently have
that does extra truncation ourselves might be sufficient.
^ permalink raw reply
* Re: [PATCH 1/2] t/test-lib.sh: Let test_must_fail fail on signals only
From: Jeff King @ 2008-07-17 6:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephan Beyer, Johannes Sixt, git
In-Reply-To: <7v3am9m5ne.fsf@gitster.siamese.dyndns.org>
On Wed, Jul 16, 2008 at 10:38:45PM -0700, Junio C Hamano wrote:
> "Only the least significant 8 bits (that is, status & 0377) shall be
> available to a waiting parent process". So it is not just "at least on
> Linux" but is a well defined behaviour.
>
> http://www.opengroup.org/onlinepubs/000095399/functions/exit.html
Ah, thanks. I read that same text in the Linux manpage but didn't think
to check that it was POSIX. However, some of our systems aren't quite
POSIX...check out 2488df84 (builtin run_command: do not exit with -1) by
Johannes Sixt [who is now cc'd]. I assume that was a Windows fallout.
> I would however agree that when we do mean 255 we should probably write
> 255, not (-1). It is easier to document things that way.
I started to fix the callsites that Stephan mentioned, but it really is
convenient to be able to 'return error("foo")' (or even return
func_that_calls_error(), and tracking down deep calls is time consuming
and error prone). So maybe we should just enhance the change from
2488df84 and special case "-1" into "1"?
diff --git a/git.c b/git.c
index 6b600b5..4f28e8c 100644
--- a/git.c
+++ b/git.c
@@ -240,7 +240,7 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
status = p->fn(argc, argv, prefix);
if (status)
- return status & 0xff;
+ return status == -1 ? 1 : status & 0xff;
/* Somebody closed stdout? */
if (fstat(fileno(stdout), &st))
-Peff
^ permalink raw reply related
* Re: [PATCH 1/2] t/test-lib.sh: Let test_must_fail fail on signals only
From: Junio C Hamano @ 2008-07-17 5:38 UTC (permalink / raw)
To: Jeff King; +Cc: Stephan Beyer, git
In-Reply-To: <20080717051833.GA3100@sigio.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Jul 15, 2008 at 10:54:25PM -0700, Junio C Hamano wrote:
>
>> Anything that returns error() from its cmd_xxx() routine, for example,
>> would end up exiting with (-1). Is it "such bogus" error codes, though?
>
> I think it is bogus, because it is being implicitly truncated to an
> unsigned 8-bit value (at least on Linux -- I have no idea what other
> platforms do).
"Only the least significant 8 bits (that is, status & 0377) shall be
available to a waiting parent process". So it is not just "at least on
Linux" but is a well defined behaviour.
http://www.opengroup.org/onlinepubs/000095399/functions/exit.html
I would however agree that when we do mean 255 we should probably write
255, not (-1). It is easier to document things that way.
^ permalink raw reply
* Re: [PATCH 1/2] t/test-lib.sh: Let test_must_fail fail on signals only
From: Jeff King @ 2008-07-17 5:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephan Beyer, git
In-Reply-To: <7v4p6qwezy.fsf@gitster.siamese.dyndns.org>
On Tue, Jul 15, 2008 at 10:54:25PM -0700, Junio C Hamano wrote:
> Anything that returns error() from its cmd_xxx() routine, for example,
> would end up exiting with (-1). Is it "such bogus" error codes, though?
I think it is bogus, because it is being implicitly truncated to an
unsigned 8-bit value (at least on Linux -- I have no idea what other
platforms do). So your -1 is actually 255. Portably speaking, C defines
only the macros EXIT_SUCCESS and EXIT_FAILURE; in practice, I don't know
what is most common.
Bad side effects of not treating your exit codes as unsigned 8-bit
integers:
- the exit values are easily confused with other things, like signal
death. As in this case. We have modified our checking code in the
test scripts, but there may be other, less robust code out there.
- other exit values can be mistaken as success. Obviously 256, -256,
512, -512, etc all produce an erroneous "success". Now we aren't
doing this, as we are using "-1", but it just seems a bit cleaner to
be up front about what is happening (and the 255 we end up with is
unnecessarily confusing; some documents, like this one:
http://tldp.org/LDP/abs/html/exitcodes.html
claim 255 as "out of range").
So what we are doing now isn't terrible, but since it was noted (and did
in fact cause a problem!), I just expected a "let's stop doing that"
patch in the original series.
-Peff
^ permalink raw reply
* Re: [PATCH] gitk - work around stderr redirection on Cygwin
From: Eric Blake @ 2008-07-17 3:55 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Junio C Hamano, paulus, git
In-Reply-To: <487A7949.9050800@gmail.com>
According to Mark Levedahl on 7/13/2008 3:53 PM:
>>>
>>>> Cygwin is *still* shipping with antiquated Tcl 8.4.1
>>>>
>>> Ping. This bug is in 1.5.6.x, and thus also in the current Cygwin git
>>> release: as a result, several gitk context menu items cause
>>> errors. (Let me know if I should resend the patch).
> I certainly have this patch in my tree so the folks I supply git to who
> use Cygwin have this patch. The question of whether to maintain this out
> of tree for the official Cygwin release is up to Eric.
Could you point me to the patch? I need to roll the Cygwin git release of
1.5.6.3 anyway (in part because cygwin has upgraded from perl 5.8 to
5.10). This sounds like something worth me including as a vendor patch,
if it does not get folded into the main repo.
--
Don't work too hard, make some time for fun as well!
Eric Blake ebb9@byu.net
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Stephen Sinclair @ 2008-07-17 3:21 UTC (permalink / raw)
To: Theodore Tso; +Cc: Junio C Hamano, Avery Pennarun, Johannes Schindelin, git
In-Reply-To: <20080716223205.GK2167@mit.edu>
On Wed, Jul 16, 2008 at 6:32 PM, Theodore Tso <tytso@mit.edu> wrote:
> So from a pedagogical perspective, what I would probably do is show
> them how to replicate svn-up, and explain to them how this script
> works:
>
> #!/bin/sh
> # git-up
>
> if git diff --quiet && git diff --quiet --cached ; then
> git pull $*
> else
> git stash ; git pull $*; git stash pop
> fi
Wouldn't this be confusing if they have a few commits on the local
branch that aren't yet pushed to the remote branch?
They could suddenly have conflicts that have nothing to do with the
working tree, which could throw people off. Not to mention the
meaningless merges that clutter the gitk display.
I know I was personally pretty confused the first few times this
happened and I had little trapezoidal patterns in gitk showing 2
commits being automerged between the local and remote branches. This
was before I understood the concepts of local and remote branches.
Perhaps some warning when...
if git diff --quiet origin/master HEAD; then...
Personally I've since learned that git-pull is a command to think
about a little before doing, as opposed to svn up, since you might
have to resolve things you aren't prepared for, and we're trying to
avoid teaching git-reset here. I've had to untrain myself from using
git-pull, switching to git-fetch/merge more and more often, because I
keep doing stupid 3-commit merges by mistake when I didn't intend to.
Some tracking of what's been pushed and what hasn't is helpful to keep
things in the expected order imho.
Steve
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Theodore Tso @ 2008-07-17 2:47 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Avery Pennarun, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0807170218280.4318@eeepc-johanness>
On Thu, Jul 17, 2008 at 02:25:05AM +0200, Johannes Schindelin wrote:
> Well, rev-parse is one of my pet peeves this day. rev-parse is _nothing_
> but plumbing.
Actually the the git man page doesn't list rev-parse as plumbing. :-)
> Actually, the problem arose with a few "tutorials" on the web, and their
> creators violently arguing for their ways (and me being more and more
> uncertain if they are wrong or me).
I know you don't like hearing about SVN, but normally the tutorials I
tend to point people to, in addition to the standard official git
tutorial and git's user manual, are these two web pages. First I tell
people to read first part of:
http://utsl.gen.nz/talks/git-svn/intro.html
which covers the git "philosophy" very nicely, up to the point where
it starts talking about the "git svn" command, and then I tell them to
go read:
http://git.or.cz/course/svn.html
There are no git plumbing commands in either of those two web pages,
because most SVN users would run screaming if they were given a
tutorial that talked about git-read-tree or git-commit-tree. :-)
- Ted
^ permalink raw reply
* Re: [PATCH 5/7] git mv: Support moving submodules
From: Junio C Hamano @ 2008-07-17 2:37 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20080716191129.19772.41903.stgit@localhost>
Petr Baudis <pasky@suse.cz> writes:
> The usage of struct path_list here is a bit abusive,...
I do not think use of path_list->util is particularly bad. It is a data
structure to store a bag of random pointers that can be indexed with
string keys, which is exactly what you are doing here.
> ... The horrid
> index_path_src_sha1 hack is unfortunately much worse,
This one certainly is ugly beyond words.
By the way, why is it even necessary to rehash the contents when you run
"mv"?
IOW,
$ >foo
$ git add foo
$ echo 1 >foo
$ git mv foo bar
makes "foo" disappear from the index and adds "bar", but it makes the
local change added to the index at the same time.
Side note. It actually is a lot worse than that. When you move
something to another existing entry, the code does not even add
the object name of moved entry recorded in the index, nor rehashes
the moved file. This is totally inconsistent!
I personally think these buggy behaviours you are trying to inherit are
making this patch more complex than necessary. Perhaps this needs to be
fixed up even further (you did some fix with the first patch) before
adding new features? For example, I think it is a bug that the
"overwrite" codepath does not work with symlinks.
But let's assume that we do not want to "fix" any of these existing
(mis)behaviour, because doing so would change the semantics.
There are a few cases:
(1) gitlink exists and so is directory but no repository (i.e. the user is
not interested);
(2) gitlink exists and there is a repository. Its HEAD matches what
gitlink records;
(3) gitlink exists and there is a repository. Its HEAD does not match what
gitlink records;
The (mis)behaviour that automatically adds moved files to the index we saw
earlier suggests that in case (3) you would want to update the gitlink in
the index with whatever HEAD the submodule repository has to be
consistent.
So instead of adding a index_path_src_sha1 hack like that, how about
* When you see ce_is_gitlink(j), you keep the original gitlink object
name. That is very good in order to preserve it for not just (1) but
also for (3) once we decide to fix this "auto adding" misbehaviour in
the later round. But you do not have to do this for case (2) if you
are going to rehash anyway, don't you?
So when you see ce_is_gitlink(j), you check if it has repository and
HEAD, and record the path_list->util only when it is case (1).
* Then, only for case (1), you do not call add_file_to_cache() -- because
you know you do not have anything you can rehash; instead, factor out
the codepath "git-update-index --cacheinfo" uses and call that.
^ permalink raw reply
* Re: [PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories
From: Shawn O. Pearce @ 2008-07-17 2:16 UTC (permalink / raw)
To: Alexander Gavrilov; +Cc: git
In-Reply-To: <200807170042.29462.angavrilov@gmail.com>
Alexander Gavrilov <angavrilov@gmail.com> wrote:
> Full copy detection can take quite some time on large repositories, which
> substantially decreases perceived usability of the 'git gui blame' command.
> This set of patches tries to overcome it by:
>
> 1) Allowing the user to disable '-C -C' and/or set the detection threshold.
>
> 2) Explicitly killing back-end processes, which don't produce any output
> during copy detection, and thus normally won't receive SIGPIPE until
> it is finished. Runaway processes are annoying.
>
> 3) To compensate for (1), adding a context menu item to manually invoke
> blame -C -C -C on a group of lines.
This series is nicely done. Works well on revision.c in git.git,
where the blame can be costly to compute with full copy detection.
And getting the incremental from the context menu also works well.
Thanks.
--
Shawn.
^ permalink raw reply
* Re: gitk: Author/Committer name with special characters
From: Paul Mackerras @ 2008-07-16 23:55 UTC (permalink / raw)
To: Torsten Paul; +Cc: git
In-Reply-To: <487E7525.7000708@gmx.de>
Torsten Paul writes:
> I'm tracking a subversion repository which is running on
> a windows machine with git-svn. The user names look like
> DOMAIN\username and that's giving a strange display in
> gitk as the backslash sequence is evaluated.
>
> I'm not sure if I found the correct place to prevent this,
> so I'd like to ask if the following change would be the
> correct thing to prepare as patch...
Something like that, I think, but to be sure I'd like to see the
actual author and committer lines that are causing the problem. Could
you send me the output of "git cat-file commit" on one of the
problematic commits?
Thanks,
Paul.
^ permalink raw reply
* Re: [PATCH (GIT-GUI)] Fix pre-commit hooks under MinGW/MSYS
From: Shawn O. Pearce @ 2008-07-17 1:18 UTC (permalink / raw)
To: Alexander Gavrilov; +Cc: git
In-Reply-To: <200807170012.28769.angavrilov@gmail.com>
Alexander Gavrilov <angavrilov@gmail.com> wrote:
> Apply the work-around for checking the executable
> permission of hook files not only on Cygwin, but on
> Windows in general.
>
> Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
> ---
> This is a fix for msysgit issue #118.
> (http://code.google.com/p/msysgit/issues/detail?id=118)
>
> I've already sent this patch, but resend it as I haven't received any reply,
> and it is not in git-gui.git yet.
Thanks. I'm not sure why I dropped this earlier, but it is
in my git-gui tree now.
> diff --git a/git-gui.sh b/git-gui.sh
> index e6e8890..2d14bf2 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -473,10 +473,10 @@ proc githook_read {hook_name args} {
> set pchook [gitdir hooks $hook_name]
> lappend args 2>@1
>
> - # On Cygwin [file executable] might lie so we need to ask
> + # On Windows [file executable] might lie so we need to ask
> # the shell if the hook is executable. Yes that's annoying.
> #
> - if {[is_Cygwin]} {
> + if {[is_Windows]} {
> upvar #0 _sh interp
> if {![info exists interp]} {
> set interp [_which sh]
--
Shawn.
^ permalink raw reply
* Re: Considering teaching plumbing to users harmful
From: Stephan Beyer @ 2008-07-17 1:01 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807170120170.4318@eeepc-johanness>
Hi,
Johannes Schindelin wrote:
> Hi,
>
> On Thu, 17 Jul 2008, Stephan Beyer wrote:
>
> > I also think that for a user it is totally irrelevant if it is plumbing
> > or porcelain she is using, as long as it works. I mean, if I tought
> > someone using git, I'd never use the words "porcelain" or "plumbing".
>
> So you would say that remembering the name "rev-parse" is just as easy as
> remembering "show"?
"show" is an intuitional name, "rev-parse" is not.[1]
But that wasn't my point. The point was, that it is not important to a
user whether the tool is called "plumbing" or "porcelain"; that these
terms have no value for the user, if she just wants to get a job done.
Of course, "usually" porcelain is more helpful and as I've said (or
at least tried to say), I don't think there is any plumbing that's
useful for a git beginner.
Regards,
Stephan
Footnote:
1. A further comment about the intuitionality or "remembering":
git-apply is plumbing and has an intuitional name (hence easy to
remember), git-am is porcelain and does not have one.
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: GSoC podcast 18: Git
From: Sverre Rabbelier @ 2008-07-17 0:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, spearce
In-Reply-To: <bd6139dc0807161732p70d01b32o39b85c8cbc75c8e9@mail.gmail.com>
On Thu, Jul 17, 2008 at 2:32 AM, Sverre Rabbelier <alturin@gmail.com> wrote:
> Dun dun dun, captain obvious to the rescue! Am listening to it now and
> it seems to be a general talk on Git's participation in GSoC, and how
> Shawn and Dscho experienced GSoC as a mentor as well.
Also the origin of Shawn's forced-upon nickname on #git is explained,
which is rather hilarious :P. (I won't give it away, you should listen
to the podcast!)
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] Use SHELL_PATH
From: SungHyun Nam @ 2008-07-17 0:38 UTC (permalink / raw)
To: git; +Cc: namsh, git, sverre, namsh
In-Reply-To: <487E932A.5080502@gmail.com>
SungHyun Nam wrote:
> And I also need to run a script function below between 'make all'
> and 'make test/install'. I hope GIT does this. Of course, GIT's
> Makefile would use SHELL_PATH and PERL_PATH.
Oops! I'm sorry! Ignore this part. GIT alreay does it, just i didn't
realize that.
Regards,
namsh
^ 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