* Re: [PATCH] Modified the default git help message to be grouped by topic
From: Junio C Hamano @ 2008-12-03 0:10 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, James Pickens, Scott Chacon, git
In-Reply-To: <20081202233004.GA22379@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Dec 02, 2008 at 11:55:03PM +0100, Johannes Schindelin wrote:
>
>> If the whole thing gets longer than 24 lines, we have to leave some things
>> out. Personally, I consider rm and mv unimportant enough that they could
>> be shown in an extended list, but be left out of the summary page.
>
> For the record, the current output is 26 lines, plus you probably want
> to account for 1 line of the user's next shell prompt. So we are 3 lines
> over already.
>
> Scott's proposal is about grouping the commands more sensibly. Many of
> the complaints are about the length of the output. Maybe we should scrap
> having a list of commands altogether and just point at section-specific
> documentation, each of which could discuss basic commands related to it.
>
> I think there has been mention of task-oriented documentation pointers
> before, and I think this is a place where we would want it.
It might not be a bad idea to make this "top page help" into an
interactive hierarchical help topic browser. You would start a page that
might look like this:
Bootstrapping -- preparing an area to work in
init, clone
Basic -- review, undo and record your changes
diff, status, checkout <path>, add, reset, commit
History -- inspect what you have now, and what happened before
log, blame, grep, show
Branching and Merging -- build and use alternate histories
branch, checkout -b, merge, rebase
Working with Others
remote, fetch, pull, push
with each of the command and the heading being a "link" (use ncurses for
that). If you choose the leaf-level command (say, 'diff'), you will get
the git-diff(1) manual page. If you pick one of the headings, say,
"Basic", you may get a more extended description of commands in the
category, that may include other basic commands not in the front page,
perhaps, like this:
(review)
diff HEAD: view what you did since the last commit
diff: view what you did since you last added
diff --cached: view what you already added
status: list what are added and changes yet to be added
(undo)
checkout path...: checkout a copy from the staging area
checkout HEAD path...: checkout a copy from the last commit
reset: undo earlier "git add" to the staging area
reset path...: do so only for the named paths
(record)
mv path1 path2: move the state of path1 to path2
rm path2: remove path2
rm --cached path2: do so without losing the copy in the working tree
add: add the contents to the staging area
add -p: do so interactively, hunk by hunk
commit: record the changes you added to the staging area
commit path...: record all the changes to the paths, ignoring
changes you added to the staging area for other paths.
and again each element can be a "link".
^ permalink raw reply
* Re: git-p4 submit, Can't clobber writable file
From: Gary Yang @ 2008-12-03 0:14 UTC (permalink / raw)
To: Reece Dunn; +Cc: git
In-Reply-To: <3f4fd2640812021551t5451e6e3i91e6d2694c89e085@mail.gmail.com>
Reece,
I understand your explanation. But, git-p4 submit does not work for me. Are you able to re-produce the problem?
Thanks,
Gary
--- On Tue, 12/2/08, Reece Dunn <msclrhd@googlemail.com> wrote:
> From: Reece Dunn <msclrhd@googlemail.com>
> Subject: Re: git-p4 submit, Can't clobber writable file
> To: garyyang6@yahoo.com
> Cc: git@vger.kernel.org
> Date: Tuesday, December 2, 2008, 3:51 PM
> 2008/12/2 Gary Yang <garyyang6@yahoo.com>:
> >
> > I followed the instructions at
> http://modular.math.washington.edu/home/mhansen/git-1.5.5.1/contrib/fast-import/git-p4.txt
> >
> > But, I am not able to git-p4 submit. Any idea?
> >
> > git-p4 clone //build/scripts build_scripts
> > cd build_scripts
> > vi foo.h
> > git commit foo.h
> > git-p4 rebase
> > git-p4 submit
> >
> > from sets import Set;
> > Perforce checkout for depot path //build/scripts/
> located at /home/gyang/workspace/build_scripts/
> > Syncronizing p4 checkout...
>
> This will be running a `p4 sync ...` command to ensure that
> the files
> are up-to-date.
>
> > //build/scripts/foo.h#1 - added as
> /home/gyang/workspace/build_scripts/foo.h
> > Can't clobber writable file
> /home/gyang/workspace/build_scripts/foo.h
> > //build/scripts/foo.c#1 - added as
> /home/gyang/workspace/build_scripts/foo.c
> > Can't clobber writable file
> /user/home/gyang/workspace/build_scripts/foo.c
> > ......
> > command failed: p4 sync ...
>
> Perforce will mark files as readonly when it does a
> checkout. When you
> use `p4 edit` to say you have made changes to the file,
> Perforce
> removes the readonly bit.
>
> What is happening here is that the sync (checkout) command
> is trying
> to write foo.h/c to your build_scripts directory, but
> because they
> already exist there *and* are not readonly, Perforce is
> producing that
> error as it does not want to overwrite any files that have
> changed
> locally.
>
> Not sure what is happening on the git-p4 side, though.
>
> - Reece
^ permalink raw reply
* Re: [PATCH] Modified the default git help message to be grouped by topic
From: Jeff King @ 2008-12-03 0:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, James Pickens, Scott Chacon, git
In-Reply-To: <7vfxl6m84g.fsf@gitster.siamese.dyndns.org>
On Tue, Dec 02, 2008 at 04:10:07PM -0800, Junio C Hamano wrote:
> It might not be a bad idea to make this "top page help" into an
> interactive hierarchical help topic browser. You would start a page that
> might look like this:
>
> Bootstrapping -- preparing an area to work in
> init, clone
> Basic -- review, undo and record your changes
> diff, status, checkout <path>, add, reset, commit
> History -- inspect what you have now, and what happened before
> log, blame, grep, show
> Branching and Merging -- build and use alternate histories
> branch, checkout -b, merge, rebase
> Working with Others
> remote, fetch, pull, push
Yes, that is the sort of thing I was thinking of. And I think your
layout addresses Scott's concern, which is to keep names of commands
available for quick reference. So really we are ditching the
descriptions.
> with each of the command and the heading being a "link" (use ncurses for
> that). If you choose the leaf-level command (say, 'diff'), you will get
I'm not sure we need anything so fancy. I was thinking of something
like:
Bootstrapping -- preparing an area to work in (bootstrapping)
init, clone
...
Working with Others (others)
remote, fetch, pull, push
Use "git help <subject>" for more help on one of these subjects, or
"git help <command>" for help with a specific command.
And maybe the "(bootstrapping)" could be typographically more obvious as
the subject keyword, but I think you get the point (and for
"Bootstrapping", it's obvious what the keyword would be, but for
"Working with Others" it's not).
And obviously something with ncurses would save you typing, but I have
no desire to recreate "info" or "lynx" here (and I also think that "git"
or "git foo" displaying help should remain non-interactive to cause the
least surprise).
-Peff
^ permalink raw reply
* Re: [PATCH] Modified the default git help message to be grouped by topic
From: Jakub Narebski @ 2008-12-03 0:47 UTC (permalink / raw)
To: git
In-Reply-To: <7vfxl6m84g.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> On Tue, Dec 02, 2008 at 11:55:03PM +0100, Johannes Schindelin wrote:
>>
>>> If the whole thing gets longer than 24 lines, we have to leave some things
>>> out. Personally, I consider rm and mv unimportant enough that they could
>>> be shown in an extended list, but be left out of the summary page.
>>
>> For the record, the current output is 26 lines, plus you probably want
>> to account for 1 line of the user's next shell prompt. So we are 3 lines
>> over already.
>>
>> Scott's proposal is about grouping the commands more sensibly. Many of
>> the complaints are about the length of the output. Maybe we should scrap
>> having a list of commands altogether and just point at section-specific
>> documentation, each of which could discuss basic commands related to it.
>>
>> I think there has been mention of task-oriented documentation pointers
>> before, and I think this is a place where we would want it.
>
> It might not be a bad idea to make this "top page help" into an
> interactive hierarchical help topic browser.
Isn't that info?
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: more merge strategies : feature request
From: Leo Razoumov @ 2008-12-03 1:07 UTC (permalink / raw)
To: Caleb Cushing; +Cc: git
In-Reply-To: <81bfc67a0812020546o79906a20jcd04bd42d18dd803@mail.gmail.com>
On 12/2/08, Caleb Cushing <xenoterracide@gmail.com> wrote:
> > I guess that "no-overwrite" can be achieved by
> >
> > git merge -s ours --no-commit
>
>
> no it doesn't. which is why I called it a bad name. no-overwrite would
> still add new lines to the file not in ours (and no-commit isn't
> needed in that case) it just wouldn't overwrite conflicting lines, my
> understanding of ours is that it will keep the files as is.
>
> Caleb Cushing
>
>From your original email in this thread
"no-overwrite: if a change from the branch being merged in would
overwrite something in the current branch don't merge it. (I think it
needs a better name)"
I got the impression that you would like to preserve "ours" branch
whenever other branch tries to overwrite something? Is it
"no-override-conflicting-lines" that you are really after?
--Leo--
^ permalink raw reply
* Re: [PATCH] gitweb: fixes to gitweb feature check code
From: Jakub Narebski @ 2008-12-03 1:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Giuseppe Bilotta, git, Petr Baudis
In-Reply-To: <7v4p1mp7hx.fsf@gitster.siamese.dyndns.org>
Dnia wtorek 2. grudnia 2008 22:55, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > What I like about having all this, i.e. fix, futureproof and style
> > correction in one single patch is the fact that fix doesn't introduce
> > strange looking (gitweb_check_feature('bool_feat'))[0]... well, except
> > encapsulated in a subroutine.
> >
> > From all possible splits of this feature into series of up to three
> > patches I think I like the one with pure subroutine rename from *check*
> > to *get* least...
>
> Well, I have to say that you have a strange taste, sense of priorities,
> and perhaps aversion to logical progression. Let's explain one more
> time.
>
> The case we had at hand was that a callee has a less-than-ideal calling
> convention that has caused a few bugs by callers because they did not
> understand the calling convention. You can argue it is not entirely
> caller's fault that they failed to follow the calling convention, but the
> fact remains that there are bugs taken as a whole.
>
> First we fix the callers, because existing bugs get highest priority.
> This is a pure bugfix patch that could even go to maintenance "bugfix
> only" branch.
I agree here.
> Then we fix the calling convention because we all know that the calling
> convention was less-than-ideal. A large part of the reason the calling
> convention was confusing was because the wording "check" implied it was a
> boolean function. Logically, s/check/get/ would be a major part of fixing
> that.
And here I slightly disagree. The lone change s/check/get/ doesn't
actually do anything. Splitting wrongly named 'check' into checking
if feature is enabled and actual getting config does. And it is IMVHO
quote obvious which calls are to be to new semantic of 'check':
my ($var) = gitweb_check_feature('feature');
and
(gitweb_check_feature('feature'))[0]
and which are to be renamed to 'get':
my ($a, $b) = gitweb_check_feature('feature');
and
my @a = gitweb_check_feature('feature');
Perhaps it is more clear from the point of view of reviewing individual
pieces...
--
Jakub Narebski
Poland
^ permalink raw reply
* X-Debbugs-Cc didn't make it to git@vger.kernel.org
From: jidanni @ 2008-12-03 1:32 UTC (permalink / raw)
To: git
Bummer, on
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507475
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507476
I used X-Debbugs-Cc, and it says Report forwarded to git@vger.kernel.org
but I don't see them here on nntp:gmane.comp.version-control.git .
Perhaps they got filtered out?
^ permalink raw reply
* Re: summaries in git add --patch
From: Junio C Hamano @ 2008-12-03 2:15 UTC (permalink / raw)
To: William Pursell; +Cc: git
In-Reply-To: <49308B4B.3070703@gmail.com>
William Pursell <bill.pursell@gmail.com> writes:
> Junio C Hamano wrote:
>> William Pursell <bill.pursell@gmail.com> writes:
>>
>>> Here's a new patch. Instead of displaying the summary and then
>>> the current hunk, it implements a 'goto' command.
>>
>> I take it that this is for discussion not for immediate inclusion.
>
> Yes. I tend to think of all of my patches as being merely
> for discussion since I'm not terribly familiar with the code
> base and expect to miss many things. I'm flattered that
> you would even consider them for inclusion. For that matter,
> I'm flattered that you have even responded to my submissions!
Thanks. I value contributions from people who are enthused and can make a
good case for the change they propose. I utter comments and sometimes
even send out an alternative implementation to illustrate what might be a
better approach. IOW, I try to help people make progress.
One thing I will not do after such a discussion, unless I am really really
interested in having the new feature personally myself, is to go back to
the discussion thread and assemble the pieces together to make the final
series of patches for inclusion. The responsibility for doing that lies
on the original contributor.
^ permalink raw reply
* Re: [PATCH 2/2 v2] Documentation: describe how to "bisect skip" a range of commits
From: Junio C Hamano @ 2008-12-03 2:17 UTC (permalink / raw)
To: Christian Couder; +Cc: Johannes Schindelin, Johannes Sixt, git, H. Peter Anvin
In-Reply-To: <20081202145351.24d1a1fb.chriscool@tuxfamily.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] User's Manual: remove duplicated url at the end of Appendix B
From: Junio C Hamano @ 2008-12-03 2:17 UTC (permalink / raw)
To: Miklos Vajna; +Cc: J. Bruce Fields, git
In-Reply-To: <1228157402-10961-1-git-send-email-vmiklos@frugalware.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] Makefile: introduce NO_PTHREADS
From: Junio C Hamano @ 2008-12-03 2:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Mike Ralphson
In-Reply-To: <4934E6BC.9040203@viscovery.net>
Thanks, both.
^ permalink raw reply
* Re: [PATCHv3bis] gitweb: fixes to gitweb feature check code
From: Junio C Hamano @ 2008-12-03 2:18 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Petr Baudis
In-Reply-To: <1228214598-26557-1-git-send-email-giuseppe.bilotta@gmail.com>
Thanks, but the ones based on the previous round were already queued in
'next', so I took the change to the check/get comment (the first hunk in
this squashed patch) not to lose the remaining improvements from this
patch.
^ permalink raw reply
* Re: [PATCH] Add a built-in alias for 'stage' to the 'add' command
From: Junio C Hamano @ 2008-12-03 2:18 UTC (permalink / raw)
To: Scott Chacon; +Cc: git, peff
In-Reply-To: <20081202061455.GA48848@agadorsparticus>
Thanks.
^ permalink raw reply
* Re: [PATCH] Add a built-in alias for 'stage' to the 'add' command
From: Junio C Hamano @ 2008-12-03 2:18 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Scott Chacon, git, peff
In-Reply-To: <fcaeb9bf0812020436n69a21c96g4c13976e493372d9@mail.gmail.com>
"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
> On 12/2/08, Scott Chacon <schacon@gmail.com> wrote:
>> diff --git a/git.c b/git.c
>> index 89feb0b..9e5813c 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -266,6 +266,7 @@ static void handle_internal_command(int argc, const char **argv)
>> const char *cmd = argv[0];
>> static struct cmd_struct commands[] = {
>> { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>> + { "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>> { "annotate", cmd_annotate, RUN_SETUP },
>> { "apply", cmd_apply },
>> { "archive", cmd_archive },
>
> Nit-picking. There are some references to "git add" in builtin-add.c,
> like help usage or error message ("Maybe you wanted to say 'git add
> .'?\n"). Should it refer to "git stage" instead as well?
I do not think that is necessary. As git-stage(1) makes it clear, it is
an alias to "add", not the other way around.
^ permalink raw reply
* Re: [PATCH] t4030-diff-textconv: Make octal escape sequence more portable
From: Junio C Hamano @ 2008-12-03 2:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Git Mailing List
In-Reply-To: <4934F245.9020908@viscovery.net>
Thanks.
^ permalink raw reply
* Re: git-p4 submit, Can't clobber writable file
From: Jing Xue @ 2008-12-03 3:07 UTC (permalink / raw)
To: Gary Yang; +Cc: git
In-Reply-To: <40586.80065.qm@web37902.mail.mud.yahoo.com>
On Tue, Dec 02, 2008 at 02:30:51PM -0800, Gary Yang wrote:
>
> I followed the instructions at http://modular.math.washington.edu/home/mhansen/git-1.5.5.1/contrib/fast-import/git-p4.txt
>
> But, I am not able to git-p4 submit. Any idea?
>
> git-p4 clone //build/scripts build_scripts
> cd build_scripts
> vi foo.h
> git commit foo.h
> git-p4 rebase
> git-p4 submit
>
> from sets import Set;
> Perforce checkout for depot path //build/scripts/ located at /home/gyang/workspace/build_scripts/
> Syncronizing p4 checkout...
> //build/scripts/foo.h#1 - added as /home/gyang/workspace/build_scripts/foo.h
> Can't clobber writable file /home/gyang/workspace/build_scripts/foo.h
> //build/scripts/foo.c#1 - added as /home/gyang/workspace/build_scripts/foo.c
> Can't clobber writable file /user/home/gyang/workspace/build_scripts/foo.c
> ......
> command failed: p4 sync ...
You might want to clone to a git working dir different than the p4
working dir.
For instance, if your p4 workspace has the working dir set to
build_scripts/, try 'git p4 clone //build/scripts build_scripts.git'.
You would then normally work under build_scripts.git/. build_scripts/
would only be used by git-p4 at submission time.
Cheers.
--
Jing Xue
^ permalink raw reply
* Re: [PATCH 1/5] avoid parse_sha1_header() accessing memory out of bound
From: Liu Yubao @ 2008-12-03 3:49 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git list
In-Reply-To: <20081202154257.GK23984@spearce.org>
Shawn O. Pearce wrote:
> Liu Yubao <yubao.liu@gmail.com> wrote:
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 6c0e251..efe6967 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -1254,10 +1255,10 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
>> /*
>> * The type can be at most ten bytes (including the
>> * terminating '\0' that we add), and is followed by
>> - * a space.
>> + * a space, at least one byte for size, and a '\0'.
>> */
>> i = 0;
>> - for (;;) {
>> + while (hdr < hdr_end - 2) {
>> char c = *hdr++;
>> if (c == ' ')
>> break;
>> @@ -1265,6 +1266,8 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
>> if (i >= sizeof(type))
>> return -1;
>
> That first hunk I am citing is unnecessary, because of the lines
> right above. All of the callers of this function pass in a buffer
> that is at least 32 bytes in size; this loop aborts if it does not
> find a ' ' within the first 10 bytes of the buffer. We'll never
> access memory outside of the buffer during this loop.
>
> So IMHO your first three hunks here aren't necessary.
>
Seems you missed the cover letter sent as patch 0/5, all patches are explained
in the cover letter, sorry I sent them as separate topics by mistake.
This bound check is mainly for uncompressed loose object, a loose object
that just are uncompressed:
uncompressed loose object = inflate(loose object)
loose object = deflate(typename + <space> + size + '\0' + data)
I'm doing a defensive programming, for uncompressed loose object the mmapped
memory is passed to parse_sha1_header without being checked by inflateInit() first,
so there may be a SIGSEGV crash for a corrupted uncompressed loose object.
>> @@ -1275,7 +1278,7 @@ static int parse_sha1_header(const char *hdr, unsigned long *sizep)
>> if (size > 9)
>> return -1;
>> if (size) {
>> - for (;;) {
>> + while (hdr < hdr_end - 1) {
>> unsigned long c = *hdr - '0';
>> if (c > 9)
>> break;
>
> OK, there's no promise here that we don't roll off the buffer.
>
> This can be fixed in the caller, ensuring we always have the '\0'
> at some point in the initial header buffer we were asked to parse:
>
Isn't it easier to solve this problem in one place and maintain it? Maybe someday
someone forgets parse_sha1_header requires a null terminated buffer, and a corrupted
uncompressed loose object even doesn't have to be null terminated (if there will be
this kind of loose object).
^ permalink raw reply
* Re: [PATCH] git-svn: Make branch use correct svn-remote
From: Junio C Hamano @ 2008-12-03 3:55 UTC (permalink / raw)
To: Eric Wong; +Cc: Deskin Miller, git, gitster
In-Reply-To: <20081202215157.GB9650@hand.yhbt.net>
Eric Wong <normalperson@yhbt.net> writes:
> Deskin Miller <deskinm@umich.edu> wrote:
>> The 'branch' subcommand incorrectly had the svn-remote to use hardcoded
>> as 'svn', the default remote name. This meant that branches derived
>> from other svn-remotes would try to use the branch and tag configuration
>> for the 'svn' remote, potentially copying would-be branches to the wrong
>> place in SVN, into the branch namespace for another project.
>>
>> Fix this by using the remote name extracted from the svn info for the
>> specified git ref. Add a testcase for this behaviour.
>>
>> Signed-off-by: Deskin Miller <deskinm@umich.edu>
>
> Looks alright to me, thanks Deskin.
>
> Acked-by: Eric Wong <normalperson@yhbt.net>
Does not work for me X-<.
* expecting success:
(svn co "$svnrepo" svn &&
cd svn &&
mkdir mirror &&
svn add mirror &&
svn copy trunk tags branches mirror/ &&
svn ci -m "made mirror" ) &&
rm -rf svn &&
git svn init -s -R mirror --prefix=mirror/ "$svnrepo"/mirror &&
git svn fetch -R mirror &&
git checkout mirror/trunk &&
base=$(git rev-parse HEAD:) &&
git svn branch -m "branch in mirror" d &&
test $base = $(git rev-parse remotes/mirror/d:) &&
test_must_fail git rev-parse remotes/d
A svn/trunk
A svn/trunk/foo
A svn/branches
A svn/branches/a
A svn/branches/a/foo
A svn/branches/b
A svn/branches/b/foo
A svn/tags
A svn/tags/tag4
A svn/tags/tag4/foo
A svn/tags/tag1
A svn/tags/tag1/foo
A svn/tags/tag2
A svn/tags/tag2/foo
A svn/tags/tag3
A svn/tags/tag3/foo
Checked out revision 8.
A mirror
svn: Client error in parsing arguments
* FAIL 4: branch uses correct svn-remote
(svn co "$svnrepo" svn &&
cd svn &&
mkdir mirror &&
svn add mirror &&
svn copy trunk tags branches mirror/ &&
svn ci -m "made mirror" ) &&
rm -rf svn &&
git svn init -s -R mirror --prefix=mirror/ "$svnrepo"/mirror &&
git svn fetch -R mirror &&
git checkout mirror/trunk &&
base=$(git rev-parse HEAD:) &&
git svn branch -m "branch in mirror" d &&
test $base = $(git rev-parse remotes/mirror/d:) &&
test_must_fail git rev-parse remotes/d
^ permalink raw reply
* Re: [PATCH] gitweb: Fix handling of non-ASCII characters in inserted HTML files
From: Junio C Hamano @ 2008-12-03 3:55 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Tatsuki Sugiura, Gerrit Pape, Recai Oktas
In-Reply-To: <20081201175741.11133.82393.stgit@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> Use new insert_file() subroutine to insert HTML chunks from external
> files: $site_header, $home_text (by default indextext.html),
> $site_footer, and $projectroot/$project/REAME.html.
>
> All non-ASCII chars of those files will be broken by Perl IO layer
> without decoding to utf8, so insert_file() does to_utf8() on each
> printed line; alternate solution would be to open those files with
> "binmode $fh, ':utf8'", or even all files with "use open qw(:std :utf8)".
>
> Note that inserting README.html lost one of checks for simplicity.
>
> Noticed-by: Tatsuki Sugiura <sugi@nemui.org>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This is more complete solution that the one provided by Tatsuki Sugiura
> in original patch
>
> [PATCH] gitweb: fix encode handling for site_{header,footer}
> Msg-Id: <87vdumbxgc.wl@vaj-k-334-sugi.local.valinux.co.jp>
> http://thread.gmane.org/gmane.comp.version-control.git/101199
It may be more complete but it is obviously untested. Please help me
trust you better with your future patches. Because I personally do not
run gitweb myself, I really need a trustworthy lieutenant(s) in the area.
[Wed Dec 3 01:52:07 2008] gitweb.perl: Global symbol "$fd" requires explicit package name at /git.git/t/../gitweb/gitweb.perl line 4500.
[Wed Dec 3 01:52:07 2008] gitweb.perl: Execution of /git.git/t/../gitweb/gitweb.perl aborted due to compilation errors.
> but it is in principle the same solution.
>
> I think this one as it is a bugfix should go in git 1.6.1
Trading a gitweb with a small bug with a gitweb that does not even pass
its test script does not feel like a good change to me.
I think the breakage is the "close $fd" at the end of this hunk:
> @@ -4472,13 +4475,11 @@ sub git_summary {
> print "</table>\n";
>
> if (-s "$projectroot/$project/README.html") {
> - if (open my $fd, "$projectroot/$project/README.html") {
> - print "<div class=\"title\">readme</div>\n" .
> - "<div class=\"readme\">\n";
> - print $_ while (<$fd>);
> - print "\n</div>\n"; # class="readme"
> - close $fd;
> - }
> + print "<div class=\"title\">readme</div>\n" .
> + "<div class=\"readme\">\n";
> + insert_file("$projectroot/$project/README.html");
> + print "\n</div>\n"; # class="readme"
> + close $fd;
> }
>
> # we need to request one more than 16 (0..15) to check if
I'll queue it to 'pu', with the "close $fd" removed, for now.
^ permalink raw reply
* Re: [PATCH 3/5] optimize parse_sha1_header() a little by detecting object type
From: Liu Yubao @ 2008-12-03 4:06 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git list
In-Reply-To: <20081202155300.GL23984@spearce.org>
Shawn O. Pearce wrote:
> Liu Yubao <yubao.liu@gmail.com> wrote:
>> diff --git a/sha1_file.c b/sha1_file.c
>> index dccc455..79062f0 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -1099,7 +1099,8 @@ static void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
>>
>> if (!fstat(fd, &st)) {
>> *size = xsize_t(st.st_size);
>> - map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
>> + if (*size > 0)
>> + map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
>> }
>> close(fd);
>> }
>
> This has nothing to do with this change description. Why are we
> returning NULL from map_sha1_file when the file length is 0 bytes?
> No loose object should ever be an empty file, there must always be
> some sort of type header present. So it probably is an error to
> have a 0 length file here. But that bug is a different change.
>
Also a defensive programming for uncompressed loose object because the mapped memory
will be passed to parse_sha1_header() directly without being checked by inflateInit().
In fact unpack_sha1_header() in current code calls legacy_loose_object() without checking
mapsize first. If it encounters (very very unlikely) a corrupted empty loose object, it
will crash.
>> @@ -1257,6 +1258,8 @@ static int parse_sha1_header(const char *hdr, unsigned long length, unsigned lon
>> * terminating '\0' that we add), and is followed by
>> * a space, at least one byte for size, and a '\0'.
>> */
>> + if ('b' != *hdr && 'c' != *hdr && 't' != *hdr) /* blob/commit/tag/tree */
>> + return -1;
>> i = 0;
>> while (hdr < hdr_end - 2) {
>> char c = *hdr++;
>
> Oh. I wouldn't do that. Its a cute trick and it works to quickly
> determine if the header is an uncompressed header vs. a zlib header
> vs. a new-style loose object header (which git cannot write anymore,
> but it still can read). But its just asking for trouble when/if a
> new object type was ever added to the type table.
>
I can't agree any more, it's just a trick. I considered adding
a function seems_likely_uncompressed_loose_object(), but I didn't
because this patch series are just my first try, I don't know whether
the idea to support uncompressed loose object is attractive enough.
> Given that we know that no type name can be more than 10 bytes and
> if you use my patch from earlier today you can be certain hdr has a
> '\0' terminator, so you could write a function to test for the type
> against the hdr, stopping on either ' ' or '\0'. Or find the first
> ' ' in the first 10 bytes (which is what this loop does anyway) and
> then test that against the type name table.
>
^ permalink raw reply
* [PATCH] Implement rebase -q to fix pull --rebase -q
From: Tuncer Ayaz @ 2008-12-03 4:06 UTC (permalink / raw)
To: gitster; +Cc: git
This is needed on top of the fetch/pull -q/-v changes
to make
$ git pull --rebase -q
as quiet as expected.
Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
git-pull.sh | 2 +-
git-rebase.sh | 31 +++++++++++++++++++++++--------
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index 1cac898..57fcee9 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -184,6 +184,6 @@ fi
merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
test true = "$rebase" &&
exec git-rebase $strategy_args --onto $merge_head \
- ${oldremoteref:-$merge_head}
+ $verbosity ${oldremoteref:-$merge_head}
exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
"$merge_name" HEAD $merge_head $verbosity
diff --git a/git-rebase.sh b/git-rebase.sh
index 023a6dc..bbfdc2e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -3,7 +3,7 @@
# Copyright (c) 2005 Junio C Hamano.
#
-USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
+USAGE='[--interactive | -i] [-q] [-v] [--onto <newbase>] <upstream> [<branch>]'
LONG_USAGE='git-rebase replaces <branch> with a new branch of the
same name. When the --onto option is provided the new branch starts
out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
@@ -45,7 +45,7 @@ strategy=recursive
do_merge=
dotest="$GIT_DIR"/rebase-merge
prec=4
-verbose=
+verbosity=1
git_am_opt=
continue_merge () {
@@ -135,7 +135,10 @@ move_to_original_branch () {
finish_rb_merge () {
move_to_original_branch
rm -r "$dotest"
- echo "All done."
+ if test $verbosity -gt 0
+ then
+ echo "All done."
+ fi
}
is_interactive () {
@@ -288,8 +291,11 @@ do
esac
do_merge=t
;;
+ -q|--quiet)
+ verbosity=0
+ ;;
-v|--verbose)
- verbose=t
+ verbosity=2
;;
--whitespace=*)
git_am_opt="$git_am_opt $1"
@@ -401,11 +407,14 @@ if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
then
# Lazily switch to the target branch if needed...
test -z "$switch_to" || git checkout "$switch_to"
- echo >&2 "Current branch $branch_name is up to date."
+ if test $verbosity -gt 0
+ then
+ echo >&2 "Current branch $branch_name is up to date."
+ fi
exit 0
fi
-if test -n "$verbose"
+if test $verbosity -gt 1
then
echo "Changes from $mb to $onto:"
# We want color (if set), but no pager
@@ -413,7 +422,10 @@ then
fi
# Detach HEAD and reset the tree
-echo "First, rewinding head to replay your work on top of it..."
+if test $verbosity -gt 0
+then
+ echo "First, rewinding head to replay your work on top of it..."
+fi
git checkout -q "$onto^0" || die "could not detach HEAD"
git update-ref ORIG_HEAD $branch
@@ -421,7 +433,10 @@ git update-ref ORIG_HEAD $branch
# we just fast forwarded.
if test "$mb" = "$branch"
then
- echo >&2 "Fast-forwarded $branch_name to $onto_name."
+ if test $verbosity -gt 0
+ then
+ echo >&2 "Fast-forwarded $branch_name to $onto_name."
+ fi
move_to_original_branch
exit 0
fi
--
1.6.0.2.GIT
^ permalink raw reply related
* Re: [PATCH 4/5] support reading uncompressed loose object
From: Liu Yubao @ 2008-12-03 4:09 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git list
In-Reply-To: <20081202155806.GM23984@spearce.org>
Shawn O. Pearce wrote:
> Liu Yubao <yubao.liu@gmail.com> wrote:
>> Signed-off-by: Liu Yubao <yubao.liu@gmail.com>
>
> I'd like to see a bit more of an explanation of the new loose
> object format you are reading in the commit message. We have a
> long history of explaining *why* the code behaves the way it does
> in our commits, so we can look at it in blame/log and understand
> what the heck went on.
>
I mentioned in the cover letter sent as patch 0/5, indeed I should
have mentioned it in the commit message again.
An uncompressed loose object is just an uncompressed loose object:
loose object = deflate(typename + <space> + size + '\0' + data)
uncompressed object = inflate(loose object)
^ permalink raw reply
* Re: [PATCH] Implement rebase -q to fix pull --rebase -q
From: Tuncer Ayaz @ 2008-12-03 4:09 UTC (permalink / raw)
To: gitster; +Cc: git
In-Reply-To: <1228277212-5917-1-git-send-email-tuncer.ayaz@gmail.com>
On Wed, Dec 3, 2008 at 5:06 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
> This is needed on top of the fetch/pull -q/-v changes
> to make
> $ git pull --rebase -q
> as quiet as expected.
>
> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
> ---
> git-pull.sh | 2 +-
> git-rebase.sh | 31 +++++++++++++++++++++++--------
> 2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 1cac898..57fcee9 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -184,6 +184,6 @@ fi
> merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
> test true = "$rebase" &&
> exec git-rebase $strategy_args --onto $merge_head \
> - ${oldremoteref:-$merge_head}
> + $verbosity ${oldremoteref:-$merge_head}
> exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
> "$merge_name" HEAD $merge_head $verbosity
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 023a6dc..bbfdc2e 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -3,7 +3,7 @@
> # Copyright (c) 2005 Junio C Hamano.
> #
>
> -USAGE='[--interactive | -i] [-v] [--onto <newbase>] <upstream> [<branch>]'
> +USAGE='[--interactive | -i] [-q] [-v] [--onto <newbase>] <upstream> [<branch>]'
> LONG_USAGE='git-rebase replaces <branch> with a new branch of the
> same name. When the --onto option is provided the new branch starts
> out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
> @@ -45,7 +45,7 @@ strategy=recursive
> do_merge=
> dotest="$GIT_DIR"/rebase-merge
> prec=4
> -verbose=
> +verbosity=1
> git_am_opt=
>
> continue_merge () {
> @@ -135,7 +135,10 @@ move_to_original_branch () {
> finish_rb_merge () {
> move_to_original_branch
> rm -r "$dotest"
> - echo "All done."
> + if test $verbosity -gt 0
> + then
> + echo "All done."
> + fi
> }
>
> is_interactive () {
> @@ -288,8 +291,11 @@ do
> esac
> do_merge=t
> ;;
> + -q|--quiet)
> + verbosity=0
> + ;;
> -v|--verbose)
> - verbose=t
> + verbosity=2
> ;;
> --whitespace=*)
> git_am_opt="$git_am_opt $1"
> @@ -401,11 +407,14 @@ if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
> then
> # Lazily switch to the target branch if needed...
> test -z "$switch_to" || git checkout "$switch_to"
> - echo >&2 "Current branch $branch_name is up to date."
> + if test $verbosity -gt 0
> + then
> + echo >&2 "Current branch $branch_name is up to date."
> + fi
If anyone dislikes the additional three lines I could combine
the test with the action on one line. I'm just not sure that would
make it better, especially depending on log message length.
> exit 0
> fi
>
> -if test -n "$verbose"
> +if test $verbosity -gt 1
> then
> echo "Changes from $mb to $onto:"
> # We want color (if set), but no pager
> @@ -413,7 +422,10 @@ then
> fi
>
> # Detach HEAD and reset the tree
> -echo "First, rewinding head to replay your work on top of it..."
> +if test $verbosity -gt 0
> +then
> + echo "First, rewinding head to replay your work on top of it..."
> +fi
> git checkout -q "$onto^0" || die "could not detach HEAD"
> git update-ref ORIG_HEAD $branch
>
> @@ -421,7 +433,10 @@ git update-ref ORIG_HEAD $branch
> # we just fast forwarded.
> if test "$mb" = "$branch"
> then
> - echo >&2 "Fast-forwarded $branch_name to $onto_name."
> + if test $verbosity -gt 0
> + then
> + echo >&2 "Fast-forwarded $branch_name to $onto_name."
> + fi
> move_to_original_branch
> exit 0
> fi
> --
> 1.6.0.2.GIT
>
>
^ permalink raw reply
* Re: [PATCH 5/5] support writing uncompressed loose object
From: Liu Yubao @ 2008-12-03 4:22 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git list
In-Reply-To: <20081202160706.GN23984@spearce.org>
Shawn O. Pearce wrote:
> Liu Yubao <yubao.liu@gmail.com> wrote:
>> Signed-off-by: Liu Yubao <yubao.liu@gmail.com>
>
> IMHO, this needs more description in the commit message.
>
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 05a9fa3..053b564 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -2328,7 +2328,7 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
>> }
>>
>> static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
>> - void *buf, unsigned long len, time_t mtime)
>> + void *buf, unsigned long len, time_t mtime, int dont_deflate)
>
> Passing this as an argument is pointless. It should be a repository
> wide configuration option in core, so you can declare it a static and
> allow git_config to populate it. Defaulting to 1 (no compression)
> like you do elsewhere in the patch isn't good.
>
Aha, sorry again, I sent the patch series as separate topics by mistake.
I considered adding a configuration variable, the patch series are sent
just to see whether the idea is worth.
> I'm still against this file format change. The series itself isn't
> that bad, and the buffer overflow catch in parse_sha1_header()
> may be something worthwhile fixing. But I'm still not sold that
> introducing a new loose object format is worth it.
>
> I'd rather use a binary header encoding like the new-style/in-pack
> format rather than the older style text headers. Its faster to
> parse for one thing.
>
The key point I suggest is to use *uncompressed* loose object, I didn't
change the format of uncompressed loose object because I don't want
to distract your attention and keep the patches small.
> Your changes in the reading code cause a copy of the buffer we
> mmap()'d. That sort of ruins your argument that this change is
> worthwhile because concurrent processes on the same host can mmap the
> same buffer and save memory. We're still copying the buffer anyway.
> I probably should have commented on that in patch 4/5, but I just
> realized it, so I'm saying it here.
>
Yes, I mentioned it in the cover letter(sigh, sorry!)
I didn't use the mapped buffer directly because other functions required
a null terminated buffer to parse data part of loose object. It can be
fixed but I don't want to make the patches too big.
The two big pros of uncompressed loose object are:
*) avoid compressing and uncompressing loose objects (I have implemented it)
*) use memory mapped loose object directly (I havn't implemented it)
Thank you for reviewing my patches, seems the idea to use uncompressed loose
object isn't attractive enough, I will keep the patches locally.
Best regards,
Liu Yubao
^ permalink raw reply
* [PATCH] git-sh-setup: Fix scripts whose PWD is a symlink into a git work-dir
From: Marcel M. Cary @ 2008-12-03 5:27 UTC (permalink / raw)
To: gitster, git; +Cc: jnareb, ae, j.sixt, Marcel M. Cary
In-Reply-To: <7vtz9vk6uj.fsf@gitster.siamese.dyndns.org>
* Before interpretting an upward path (../) in cd_to_toplevel,
cd to a path without symlinks given by /bin/pwd
* Add tests for cd_to_toplevel and "git pull" in a symlinked
directory that failed before this fix, plus constrasting
scenarios that already worked
Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
---
I hope this patch will address concerns both about changes
to existing APIs and speed of the new behavior.
A few notes on implementation choices:
I used /bin/pwd because of this precedent for choosing it over
"cd -P" for compatibility.
http://article.gmane.org/gmane.comp.version-control.git/46918
If cd_to_toplevel had concatenated $(/bin/pwd) with $cdup to
avoid the separate "cd", it would require checking for $cdup
being an absolute path. I wasn't sure how to check that in
a way that is both portable and clearly faster than "cd",
so cd_to_toplevel runs "cd" twice. I'm assuming that
running an external command like expr or grep is slower than
just doing the "cd".
cd_to_toplevel doesn't check $PWD to see whether to do the
first cd, because some shells allegedly don't update it
reliably.
Since cd_to_toplevel doesn't know whether it's at a
symlinked PWD or not, I wrote it to treat the
"cd $(/bin/pwd)" as mandatory, even when it might not
actually be. So on systems without /bin/pwd, it will fail
even when there are no symlinks. I thought that was better
than inconsistent behavior depending on whether /bin/pwd is
available.
The extra "cd" will be skipped when the script is already at
the top of the working tree.
git-sh-setup.sh | 11 +++++++
t/t2300-cd-to-toplevel.sh | 37 +++++++++++++++++++++++++
t/t5521-pull-symlink.sh | 67 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 115 insertions(+), 0 deletions(-)
create mode 100755 t/t2300-cd-to-toplevel.sh
create mode 100755 t/t5521-pull-symlink.sh
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index dbdf209..377700b 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -85,6 +85,17 @@ cd_to_toplevel () {
cdup=$(git rev-parse --show-cdup)
if test ! -z "$cdup"
then
+ # Interpret $cdup relative to the physical, not logical, cwd.
+ # Probably /bin/pwd is more portable than passing -P to cd or pwd.
+ phys="$(/bin/pwd)" || {
+ echo >&2 "Cannot determine the physical path to the current dir"
+ exit 1
+ }
+ cd "$phys" || {
+ echo >&2 "Cannot chdir to the physical path to current dir: $phys"
+ exit 1
+ }
+
cd "$cdup" || {
echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
exit 1
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
new file mode 100755
index 0000000..293dc35
--- /dev/null
+++ b/t/t2300-cd-to-toplevel.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='cd_to_toplevel'
+
+. ./test-lib.sh
+
+test_cd_to_toplevel () {
+ test_expect_success "$2" '
+ (
+ cd '"'$1'"' &&
+ . git-sh-setup &&
+ cd_to_toplevel &&
+ [ "$(pwd -P)" = "$TOPLEVEL" ]
+ )
+ '
+}
+
+TOPLEVEL="$(pwd -P)/repo"
+mkdir -p repo/sub/dir
+mv .git repo/
+SUBDIRECTORY_OK=1
+
+test_cd_to_toplevel repo 'at physical root'
+
+test_cd_to_toplevel repo/sub/dir 'at physical subdir'
+
+ln -s repo symrepo
+test_cd_to_toplevel symrepo 'at symbolic root'
+
+ln -s repo/sub/dir subdir-link
+test_cd_to_toplevel subdir-link 'at symbolic subdir'
+
+cd repo
+ln -s sub/dir internal-link
+test_cd_to_toplevel internal-link 'at internal symbolic subdir'
+
+test_done
diff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh
new file mode 100755
index 0000000..f18fec7
--- /dev/null
+++ b/t/t5521-pull-symlink.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+test_description='pulling from symlinked subdir'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+# The scenario we are building:
+#
+# trash\ directory/
+# clone-repo/
+# subdir/
+# bar
+# subdir-link -> clone-repo/subdir/
+#
+# The working directory is subdir-link.
+#
+test_expect_success setup '
+
+ mkdir subdir &&
+ touch subdir/bar &&
+ git add subdir/bar &&
+ git commit -m empty &&
+ git clone . clone-repo &&
+ # demonstrate that things work without the symlink
+ test_debug "cd clone-repo/subdir/ && git pull; cd ../.." &&
+ ln -s clone-repo/subdir/ subdir-link &&
+ cd subdir-link/ &&
+ test_debug "set +x"
+'
+
+# From subdir-link, pulling should work as it does from
+# clone-repo/subdir/.
+#
+# Instead, the error pull gave was:
+#
+# fatal: 'origin': unable to chdir or not a git archive
+# fatal: The remote end hung up unexpectedly
+#
+# because git would find the .git/config for the "trash directory"
+# repo, not for the clone-repo repo. The "trash directory" repo
+# had no entry for origin. Git found the wrong .git because
+# git rev-parse --show-cdup printed a path relative to
+# clone-repo/subdir/, not subdir-link/. Git rev-parse --show-cdup
+# used the correct .git, but when the git pull shell script did
+# "cd `git rev-parse --show-cdup`", it ended up in the wrong
+# directory. Shell "cd" works a little different from chdir() in C.
+# Bash's "cd -P" works like chdir() in C.
+#
+test_expect_success 'pulling from symlinked subdir' '
+
+ git pull
+'
+
+# Prove that the remote end really is a repo, and other commands
+# work fine in this context.
+#
+test_debug "
+ test_expect_success 'pushing from symlinked subdir' '
+
+ git push
+ '
+"
+cd "$D"
+
+test_done
--
1.6.0.3
^ permalink raw reply related
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