* Re: [RFC PATCH v4 00/26] Return of smart HTTP
From: Shawn O. Pearce @ 2009-10-29 15:28 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m3my3ad3gp.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > I think this is the final spin of the smart HTTP series.
>
> If it is a final spin, then what of missing RFC-like documentation of
> Git HTTP protocol in Documentation/technical/http-protocol.txt
> (it was present only in first version of new series)?
Its still incomplete. I want to get the code cooking while I work
on the documentation. Its a separate patch anyway, whether or not
its in the middle of the code series or after it is doesn't matter.
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCH v4 14/26] Add stateless RPC options to upload-pack, receive-pack
From: Shawn O. Pearce @ 2009-10-29 15:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd446dfx4.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > When --stateless-rpc is passed [...]
> > When --advertise-refs is passed [...]
>
> Is the idea to first run with --advertise-refs to get the set of refs, and
> then doing another, separate run with --stateless-rpc, assuming that the
> refs the other end advertised does not change in the meantime, to conclude
> the business?
Yes.
> I suspect that two separate invocations on a (supposedly) single
> repository made back-to-back can produce an inconsistent response in
> verious situations (e.g. somebody pushing in the middle, or the same
> hostname served by more than one mirrors) and the other end can get
> confused.
Yes, that can happen.
> I do not think there is any way to avoid it, short of adding to the second
> request some "cookie" that allows the second requestee to verify that the
> request is based on what he would give to the requester if this request
> were the first step of the request made to him, iow, his state did not
> change in the middle since the first request was made (either to him or to
> the equivalent mirror sitting next to himm).
There isn't a lot we can do, you are right.
One approach is to use an HMAC and sign each advertised SHA-1
during the initial --advertise-refs phase. Requesters then present
the SHA-1 and the HAMC signature in each "want" line, and the
--stateless-rpc phase validates the signatures to ensure they came
from a trusted party.
The major problem with this approach is the private key management.
All mirrors of that repository need to have a common private key
so they can generate and later verify that HMAC signature. This is
additional complexity, for perhaps not much gain.
A different approach is to have the --stateless-rpc phase validate
the want lines against its refs (like we do now), but if the validate
fails (no ref exactly matches) support walking backwards through the
last few records in the reflog, or through that ref's own history
for a few hundred commits, to see if the want SHA-1 appears in the
recent prior past.
Obviously when walking the reflog we would need to use a time bound,
e.g. "only examine last record if more recent than 5 minutes ago".
This way a force push to erase something will still erase it, but
a client who had just recently observed the prior SHA-1 can still
complete their fetch request, just as with native git:// they could
do that due to the prior SHA-1 being held in the git-upload-pack's
private memory.
Also, obviously when walking the commit history of a ref (to see
if the want'd SHA-1 is merged into one or more reachable refs)
we don't want to walk backwards too far, as it costs CPU time on
the server side, but we also don't want to walk too few commits.
E.g. pushes for me tend to be in the 20 commit range, while Linus
probably pushes a thousand commits or more in a single merge.
Finding the right balance may be tricky.
> I wouldn't worry too much about this if the only negative effect could be
> that the requestor's second request may result in an error, but I am
> wondering if this can be used to attack the requestee.
I don't think it can be used to attack the server. The only impact
I can see is the client gets confused and gets an error response
from the server when the server aborts due to the invalid "want"
line sent during that 2nd (or any subsequent) request.
--
Shawn.
^ permalink raw reply
* [PATCH] Make t9150 and t9151 test scripts executable
From: Michael J Gruber @ 2009-10-29 15:26 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
so that they can be run individually as
(cd t && ./t9150-svk-mergetickets.sh)
etc. just like all other test scripts.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Not much more to say. Wanted to be a good boy and run the new tests.
One can use sh, of course, but there's a reason all other tests are executable.
0 files changed, 0 insertions(+), 0 deletions(-)
mode change 100644 => 100755 t/t9150-svk-mergetickets.sh
mode change 100644 => 100755 t/t9151-svn-mergeinfo.sh
diff --git a/t/t9150-svk-mergetickets.sh b/t/t9150-svk-mergetickets.sh
old mode 100644
new mode 100755
diff --git a/t/t9151-svn-mergeinfo.sh b/t/t9151-svn-mergeinfo.sh
old mode 100644
new mode 100755
--
1.6.5.86.g0056e
^ permalink raw reply
* Re: [RFC PATCH v4 03/26] pkt-line: Make packet_read_line easier to debug
From: Shawn O. Pearce @ 2009-10-29 15:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhbtidgmp.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > diff --git a/pkt-line.c b/pkt-line.c
> > index bd603f8..893dd3c 100644
> > --- a/pkt-line.c
> > +++ b/pkt-line.c
> > @@ -124,12 +124,14 @@ static int packet_length(const char *linelen)
> > int packet_read_line(int fd, char *buffer, unsigned size)
> > {
> > int len;
> > - char linelen[4];
> > + char linelen[5];
> >
> > safe_read(fd, linelen, 4);
> > len = packet_length(linelen);
> > - if (len < 0)
> > - die("protocol error: bad line length character");
> > + if (len < 0) {
> > + linelen[4] = '\0';
> > + die("protocol error: bad line length character: %s", linelen);
> > + }
>
> Since this is not called recursively, you can make linelen[] static
Sure. But it wasn't static before. It was stack allocated. Its a
5 byte stack allocation. Its not a lot of data to push onto the
stack, why does it suddenly matter that we make it static and charge
everyone for its memory instead of just those who really need it?
> and do
> without the NUL assignment; safe_read() won't read beyond 4 bytes anyway.
The NUL assignment isn't about safe_read(), its about making the
block of 4 bytes read a proper C-style string so we can safely pass
it down into the snprintf that is within die().
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCH v4 05/26] Move "get_ack()" back to fetch-pack
From: Shawn O. Pearce @ 2009-10-29 15:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vljiudgrr.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > In 41cb7488 Linus moved this function to connect.c for reuse inside
> > of the git-clone-pack command. That was 2005, but in 2006 Junio
> > retired git-clone-pack in commit efc7fa53. Since then the only
> > caller has been fetch-pack. Since this ACK/NAK exchange is only
> > used by the fetch-pack/upload-pack protocol we should keep move
> > it back to a private detail of fetch-pack.
>
> Should we keep it there or should we move it? which? ;-)
Right. Fixed.
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCH v4 26/26] test smart http fetch and push
From: Shawn O. Pearce @ 2009-10-29 14:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20091029143702.GU10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > > + s/^Content-Length: .*$/Content-Length: xxxx/
> >
> > This chomped line is so unlike you---what happened?
>
> I was getting different Content-Lengths on different runs of the
> test. I don't know why. Here the Content-Length is of the gzip'd
> request, it shouldn't be varying with each run, but it seemed to be.
FWIW, I found the variablity here. It was triggered by running the
test with -v or without -v, which caused clone to change its first
"want" line to include or exclude progress messages, based on the
tty-ness of stderr.
Adding a --quiet to the clone command means I can avoid this munging
and hard code the length in the test vector. But I'm still against
doing that here in case the protocol capabilities ever change in
the future.
--
Shawn.
^ permalink raw reply
* Re: date change of commit?
From: Rogan Dawes @ 2009-10-29 14:47 UTC (permalink / raw)
To: Alex K; +Cc: Miklos Vajna, Matthieu Moy, git
In-Reply-To: <e4a904790910290555n61bca296g3624c1aced50ed85@mail.gmail.com>
Alex K wrote:
> And how do you actually set those variables? Sorry to ask such a
> trivial question but it's been an hour that i'm going through the doc
> for such a simple feature. I thought those were environment variables
> ... but they are not seen under git var -l. Thank you.
>
As you suspected, they are environment variables.
i.e. :
$ GIT_AUTHOR_DATE="1112911993 -0700" git commit x
Hope that helps.
Rogan
> 2009/10/27 Miklos Vajna <vmiklos@frugalware.org>:
>> On Tue, Oct 27, 2009 at 10:41:47AM +0100, Alex K <spaceoutlet@gmail.com> wrote:
>>> Thank you. And how would you use git-filter-branch to create another
>>> branch with a different time stamp? Is it possible to commit under a
>>> different time stamp than the one provided by your default local time?
>> You can set GIT_AUTHOR_DATE and GIT_COMMITTER_DATE. Both expect a format
>> like: "1112911993 -0700" (unix timestamp + timezone info).
>>
^ permalink raw reply
* Re: [RFC PATCH v4 26/26] test smart http fetch and push
From: Shawn O. Pearce @ 2009-10-29 14:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpr86dgyj.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > +test_expect_success 'clone http repository' '
> > + GIT_CURL_VERBOSE=1 git clone $HTTPD_URL/git/repo.git clone 2>err &&
> > + test_cmp file clone/file &&
> > + egrep "^([<>]|Pragma|Accept|Content-|Transfer-)" err |
> > + egrep -v "^< (Server|Expires|Date|Content-Length:|Transfer-Encoding: chunked)" |
> > + sed -e "
> > + s/
> > //
> > + s/^Content-Length: .*$/Content-Length: xxxx/
> > + " >act &&
>
> This chomped line is so unlike you---what happened?
I was getting different Content-Lengths on different runs of the
test. I don't know why. Here the Content-Length is of the gzip'd
request, it shouldn't be varying with each run, but it seemed to be.
> Also, when the last downstream is sed, why would you even need two egrep
> process?
That's a really good point. This is just stupid, I started with
the two egreps to filter the lines, then found I needed to strip
CRs, and then had to munge the Content-Length, and I just forgot
to merge the egrep cases into the sed script.
I'll fix this. Thanks.
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCH v4 26/26] test smart http fetch and push
From: Shawn O. Pearce @ 2009-10-29 14:32 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: git
In-Reply-To: <20091029003117.GA18299@localhost>
Clemens Buchacher <drizzd@aon.at> wrote:
> On Wed, Oct 28, 2009 at 05:00:48PM -0700, Shawn O. Pearce wrote:
>
> > --- /dev/null
> > +++ b/t/t5541-http-push.sh
> [...]
> > +LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5550'}
>
> This should be 5541. We need different ports to be able to run the tests
> simultenously.
Thanks, I also realized this myself this morning on my way into work.
I've adjusted my two new test scripts, and inserted your patch to
fix the older test script into the series.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 7/7] Factor ref updating out of fetch_with_import
From: Shawn O. Pearce @ 2009-10-29 14:22 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Git List, Johannes Schindelin, Daniel Barkalow, Johan Herland
In-Reply-To: <1256798426-21816-8-git-send-email-srabbelier@gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Also allow the new update_refs to actually update the refs set, this
> way the remote helper can set the value of previously unknown refs.
...
> diff --git a/builtin-clone.c b/builtin-clone.c
> index f281756..768668d 100644
> --- a/builtin-clone.c
> +++ b/builtin-clone.c
> @@ -512,6 +512,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> if (refs) {
> struct ref *ref_cpy = copy_ref_list(refs);
> transport_fetch_refs(transport, ref_cpy);
> + if (transport->update_refs)
> + {
> + ref_cpy = copy_ref_list(refs);
> + transport->update_refs(transport, ref_cpy);
Please define a transport_update_refs wrapper function to implement
this method invocation on the transport instance. Callers should
not be reaching into the struct transport directly.
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index 63a4ff0..3aaac47 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -479,7 +479,11 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
> {
> int ret = quickfetch(ref_map);
> if (ret)
> + {
> ret = transport_fetch_refs(transport, ref_map);
> + if (transport->update_refs)
> + transport->update_refs(transport, ref_map);
I certainly have to wonder... if this is done in both fetch and
clone, why isn't it just part of fetch_refs?
> diff --git a/transport-helper.c b/transport-helper.c
> index a361366..9a98fae 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -264,5 +271,6 @@ int transport_helper_init(struct transport *transport, const char *name)
> transport->get_refs_list = get_refs_list;
> transport->fetch = fetch;
> transport->disconnect = release_helper;
> + transport->update_refs = update_refs;
Please set this before the disconnect method (move it up one line).
--
Shawn.
^ permalink raw reply
* Re: [PATCH] diff --color-words -U0: fix the location of hunk headers
From: Johannes Schindelin @ 2009-10-29 13:27 UTC (permalink / raw)
To: Markus Heidelberg; +Cc: Junio C Hamano, git
In-Reply-To: <200910291222.42598.markus.heidelberg@web.de>
Hi,
On Thu, 29 Oct 2009, Markus Heidelberg wrote:
> Johannes Schindelin, 29.10.2009:
> >
> > I would strongly prefer this fix instead of your 2/3 and 3/3.
> >
> > diff.c | 6 ++++++
> > t/t4034-diff-words.sh | 2 +-
> > 2 files changed, 7 insertions(+), 1 deletions(-)
> >
> > diff --git a/diff.c b/diff.c
> > index 51b5dbb..4eafaf5 100644
> > --- a/diff.c
> > +++ b/diff.c
> > @@ -656,6 +656,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> > for (i = 0; i < len && line[i] == '@'; i++)
> > ;
> > if (2 <= i && i < len && line[i] == ' ') {
> > + /* flush --color-words even for --unified=0 */
> > + if (ecbdata->diff_words &&
> > + (ecbdata->diff_words->minus.text.size ||
> > + ecbdata->diff_words->plus.text.size))
> > + diff_words_show(ecbdata->diff_words);
> > +
> > ecbdata->nparents = i - 1;
> > len = sane_truncate_line(ecbdata, line, len);
> > emit_line(ecbdata->file,
>
> This seems to apply before commit b8d9c1a (diff.c: the builtin_diff()
> deals with only two-file comparison, 2009-09-03).
Yes, sorry, for some reason I worked on a machine where I do not work from
junio's next, but my own fork (which is outdated due to lack of time).
> Indeed my initial fix was in the same fashion:
>
> @@ -772,6 +772,15 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> }
>
> if (line[0] == '@') {
> + if (ecbdata->diff_words) {
> + /*
> + * The content of the previous hunk, necessary for
> + * 0-context.
> + */
> + if (ecbdata->diff_words->minus.text.size ||
> + ecbdata->diff_words->plus.text.size)
> + diff_words_show(ecbdata->diff_words);
> + }
> len = sane_truncate_line(ecbdata, line, len);
> find_lno(line, ecbdata);
> emit_line(ecbdata->file,
>
> But then I thought I should not put the diff output from --color-words
> into the block that deals with the hunk header, but save another place
> where diff_words_show() is called.
I found this paragraph, as well as the patches 2/3 and 3/3, hard to
follow.
And besides, flushing in that block is the correct thing to do. The
function diff_words_show() is a function for that exact purpose.
Ciao,
Dscho
^ permalink raw reply
* Re: How does format-patch determine the filename of the patch?
From: Joshua Roys @ 2009-10-29 13:14 UTC (permalink / raw)
To: Timur Tabi; +Cc: Robin Rosenberg, git
In-Reply-To: <ed82fe3e0910290605j302747f9o6a86acb414427639@mail.gmail.com>
On 10/29/2009 09:05 AM, Timur Tabi wrote:
> On Thu, Oct 15, 2009 at 12:59 PM, Robin Rosenberg
> <robin.rosenberg.lists@dewire.com> wrote:
>
>>> The reason I ask is that I'm writing a script which calls
>>> git-format-patch to create some patches for post-processing. So I
>>> need the name of the file that git-format-patch creates so that I can
>>> open it and examine it. I'd liked to see if there's a way to get the
>>> name of the patch without actually creating the file.
>>
>> It tells you the names on stdout.
>
> Is there a way for it to tell me the name on stdout *without* actually
> creating the patch?
>
Hello,
Is there any way you can just save the names from when you do call
format-patch?
e.g.
...
my @filenames = qx(git format-patch $opts);
...
Josh
^ permalink raw reply
* Re: How does format-patch determine the filename of the patch?
From: Timur Tabi @ 2009-10-29 13:05 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200910151959.47778.robin.rosenberg.lists@dewire.com>
On Thu, Oct 15, 2009 at 12:59 PM, Robin Rosenberg
<robin.rosenberg.lists@dewire.com> wrote:
>> The reason I ask is that I'm writing a script which calls
>> git-format-patch to create some patches for post-processing. So I
>> need the name of the file that git-format-patch creates so that I can
>> open it and examine it. I'd liked to see if there's a way to get the
>> name of the patch without actually creating the file.
>
> It tells you the names on stdout.
Is there a way for it to tell me the name on stdout *without* actually
creating the patch?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: date change of commit?
From: Alex K @ 2009-10-29 12:55 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Matthieu Moy, git
In-Reply-To: <20091027122156.GD6115@genesis.frugalware.org>
And how do you actually set those variables? Sorry to ask such a
trivial question but it's been an hour that i'm going through the doc
for such a simple feature. I thought those were environment variables
... but they are not seen under git var -l. Thank you.
2009/10/27 Miklos Vajna <vmiklos@frugalware.org>:
> On Tue, Oct 27, 2009 at 10:41:47AM +0100, Alex K <spaceoutlet@gmail.com> wrote:
>> Thank you. And how would you use git-filter-branch to create another
>> branch with a different time stamp? Is it possible to commit under a
>> different time stamp than the one provided by your default local time?
>
> You can set GIT_AUTHOR_DATE and GIT_COMMITTER_DATE. Both expect a format
> like: "1112911993 -0700" (unix timestamp + timezone info).
>
^ permalink raw reply
* Re: [PATCH 1/3] add splash screen
From: Paolo Bonzini @ 2009-10-29 12:48 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091029002400.GA1057@sigill.intra.peff.net>
On 10/29/2009 01:24 AM, Jeff King wrote:
> Because bash completion is so slow to start, we need to
> entertain users with a splash screen, so reuse the one from
> git-gui.
I suggest adding "wm overrideredirect . 1; wm geometry . +300+300".
Paolo
^ permalink raw reply
* msysgit won't install as per instructions
From: Anthony W. Youngman @ 2009-10-29 12:38 UTC (permalink / raw)
To: git
I want to do some work on git-daemon under msysgit. And as a newbie to
git it could well be I'm just being stupid, but ...
I installed msysgit, did a clone on the msysgit repository, and followed
the instructions, namely double-click on msys.bat. It falls over with
"no target for make install" - I know make well enough to know that
*sounds* *like* a messed up makefile. But lack of experience means
trying to track how it got there defeats me. The error output is below.
Cheers,
Wol
-------------------------------------------------------
Building and Installing Git
-------------------------------------------------------
make: *** No rule to make target `install'. Stop.
-------------------------
Hello, dear Git developer.
This is a minimal MSYS environment to work on Git.
Your build failed... Please fix it, and give feedback on the Git list.
Welcome to msysGit
Run 'git help git' to display the help index.
Run 'git help <command>' to display help for specific commands.
Run '/share/msysGit/add-shortcut.tcl' to add a shortcut to msysGit.
bash: /git/contrib/completion/git-completion.bash: No such file or
directory
bash: __git_ps1: command not found
--
Anthony W. Youngman - anthony@thewolery.demon.co.uk
^ permalink raw reply
* Re: [PATCH 1/7] Refactor git_remote_cvs to a more generic git_remote_helpers
From: Johan Herland @ 2009-10-29 12:05 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <1256798426-21816-2-git-send-email-srabbelier@gmail.com>
On Thursday 29 October 2009, Sverre Rabbelier wrote:
> This in an effort to allow future remote helpers written in python to
> re-use the non-cvs-specific code.
>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> CC: Johan Herland <johan@herland.net>
> ---
>
> As discussed with Johan Herland, refactored git_remote_cvs into a
> more reusable git_remote_helpers module.
It's hard to review this patch for a couple of reasons:
1. It's over 200K large, which causes it to be blocked by the Git mailing
list (100K limit, I believe).
2. The patch renames some files, but instead of simply stating the rename,
the patch lists the entire file twice (deletion + creation)
Fortunately, you can easily solve both problems by rerolling the patch with
the -M flag to git-format-patch.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH/RFC 2/2] Provide a build time default-editor setting
From: Johannes Sixt @ 2009-10-29 11:50 UTC (permalink / raw)
To: David Roundy; +Cc: Jonathan Nieder, Junio C Hamano, Ben Walton, GIT List
In-Reply-To: <117f2cc80910290336k1e7b5877uc472ad511bb6f5ae@mail.gmail.com>
David Roundy schrieb:
> Any chance this will be exported as plumbing? I know it's pretty
> high-level, but it'd be handy to have be able to write `git editor
> $FILENAME` and just have it do the right thing. This would also mean
> that the perl scripts below could be simplified.
Something like below? Possible usage in shell scripts:
editor=$(git var GIT_EDITOR)
"$editor" "$filename"
-- Hannes
PS: warning: linewrapped.
Subject: [PATCH] Teach git var about GIT_EDITOR
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
cache.h | 1 +
editor.c | 13 +++++++++++--
var.c | 6 ++++++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index a5eeead..3103dda 100644
--- a/cache.h
+++ b/cache.h
@@ -750,6 +750,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const
char *date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *git_editor();
struct checkout {
const char *base_dir;
diff --git a/editor.c b/editor.c
index 4d469d0..bd8c828 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
#include "strbuf.h"
#include "run-command.h"
-int launch_editor(const char *path, struct strbuf *buffer, const char
*const *env)
+const char *git_editor()
{
const char *editor, *terminal;
@@ -16,11 +16,20 @@ int launch_editor(const char *path, struct strbuf
terminal = getenv("TERM");
if (!editor && (!terminal || !strcmp(terminal, "dumb")))
- return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
+ return "/dev/null";
if (!editor)
editor = "vi";
+ return editor;
+}
+
+int launch_editor(const char *path, struct strbuf *buffer, const char
*const *env)
+{
+ const char *editor = git_editor();
+
+ if (!strcmp(editor, "/dev/null"))
+ return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
if (strcmp(editor, ":")) {
size_t len = strlen(editor);
int i = 0;
diff --git a/var.c b/var.c
index 125c0d1..48d8b9a 100644
--- a/var.c
+++ b/var.c
@@ -8,6 +8,11 @@
static const char var_usage[] = "git var [-l | <variable>]";
+static const char *editor(int unused)
+{
+ return git_editor();
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -15,6 +20,7 @@ struct git_var {
static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
{ "GIT_AUTHOR_IDENT", git_author_info },
+ { "GIT_EDITOR", editor },
{ "", NULL },
};
--
1.6.5.rc2.47.g49402
^ permalink raw reply related
* Re: [PATCH] diff --color-words -U0: fix the location of hunk headers
From: Markus Heidelberg @ 2009-10-29 11:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0910291144170.3687@felix-maschine>
Johannes Schindelin, 29.10.2009:
>
> I would strongly prefer this fix instead of your 2/3 and 3/3.
>
> diff.c | 6 ++++++
> t/t4034-diff-words.sh | 2 +-
> 2 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 51b5dbb..4eafaf5 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -656,6 +656,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> for (i = 0; i < len && line[i] == '@'; i++)
> ;
> if (2 <= i && i < len && line[i] == ' ') {
> + /* flush --color-words even for --unified=0 */
> + if (ecbdata->diff_words &&
> + (ecbdata->diff_words->minus.text.size ||
> + ecbdata->diff_words->plus.text.size))
> + diff_words_show(ecbdata->diff_words);
> +
> ecbdata->nparents = i - 1;
> len = sane_truncate_line(ecbdata, line, len);
> emit_line(ecbdata->file,
This seems to apply before commit b8d9c1a (diff.c: the builtin_diff()
deals with only two-file comparison, 2009-09-03).
Indeed my initial fix was in the same fashion:
@@ -772,6 +772,15 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
}
if (line[0] == '@') {
+ if (ecbdata->diff_words) {
+ /*
+ * The content of the previous hunk, necessary for
+ * 0-context.
+ */
+ if (ecbdata->diff_words->minus.text.size ||
+ ecbdata->diff_words->plus.text.size)
+ diff_words_show(ecbdata->diff_words);
+ }
len = sane_truncate_line(ecbdata, line, len);
find_lno(line, ecbdata);
emit_line(ecbdata->file,
But then I thought I should not put the diff output from --color-words
into the block that deals with the hunk header, but save another place
where diff_words_show() is called.
Markus
^ permalink raw reply
* Re: [PATCH 0/3] increase user-friendliness
From: Johannes Schindelin @ 2009-10-29 10:59 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091029002229.GA986@sigill.intra.peff.net>
Hi,
On Wed, 28 Oct 2009, Jeff King wrote:
> Git has a reputation as being unfriendly to users. Let's fix that by
> adding some features implemented by more user-friendly programs.
So the bar for pirate patches has been raised to patch series now?
LOL,
Dscho
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2009, #01; Wed, 07)
From: Johannes Schindelin @ 2009-10-29 10:54 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Shawn O. Pearce, git, vcs-fast-import-devs
In-Reply-To: <fabb9a1e0910281508m3e9bb8a6g7b39abc29fceae78@mail.gmail.com>
Hi,
On Wed, 28 Oct 2009, Sverre Rabbelier wrote:
> On Thu, Oct 8, 2009 at 10:58, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> > I think it makes to ignore options that are not for our vcs, as long
> > as options that change import behavior (such as marks, date-format)
> > are combined with, say, 'feature tool=git'. This way we can be sure
> > that when outputting out a vcs specific stream, it is only parsed by
> > that vcs.
> >
> > Note: yes, I know that marks and date-format are features now, but
> > there's really no other suitable example that I could think of).
> >
> > vcs fast import devs please ack this idea (and perhaps suggest
> > something other than "feature tool=git" if preferable) so that I can
> > reroll my gfi-options series :).
>
> Shawn, what do you want to do with this, it seems the vcs devs are not
> very interested in this feature, should I implement it as described
> above? That is:
> * If you use any option that is stream-changing you should include
> "feature tool=git" in your stream
> * import-marks and export-marks are made into features
> * "option vcs" is ignored if vcs is a different vcs
> * "option vcs" must be recognised if vcs is this vcs
It would be quite nice if this issue moved forward for a change.
As a consequence of it moving forward, I could nudge Sverre into
continuing with his git-remote-hg work that will allow me to work
transparently on a Mercurial repository using Git.
Transparent as in "no hassles".
It also will serve nicely as a perfect excuse to fix some design mistakes
in the foreign vcs stuff.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] diff --color-words -U0: fix the location of hunk headers
From: Johannes Schindelin @ 2009-10-29 10:45 UTC (permalink / raw)
To: Markus Heidelberg; +Cc: Junio C Hamano, git
In-Reply-To: <1256732672-11817-4-git-send-email-markus.heidelberg@web.de>
Colored word diff without context lines firstly printed all the hunk
headers among each other and then printed the diff.
This was due to the code relying on getting at least one context line at
the end of each hunk, where the colored words would be flushed (it is
done that way to be able to ignore rewrapped lines).
Noticed by Markus Heidelberg.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
I would strongly prefer this fix instead of your 2/3 and 3/3.
diff.c | 6 ++++++
t/t4034-diff-words.sh | 2 +-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c
index 51b5dbb..4eafaf5 100644
--- a/diff.c
+++ b/diff.c
@@ -656,6 +656,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
for (i = 0; i < len && line[i] == '@'; i++)
;
if (2 <= i && i < len && line[i] == ' ') {
+ /* flush --color-words even for --unified=0 */
+ if (ecbdata->diff_words &&
+ (ecbdata->diff_words->minus.text.size ||
+ ecbdata->diff_words->plus.text.size))
+ diff_words_show(ecbdata->diff_words);
+
ecbdata->nparents = i - 1;
len = sane_truncate_line(ecbdata, line, len);
emit_line(ecbdata->file,
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 82240cf..21db6e9 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -77,7 +77,7 @@ cat > expect <<\EOF
<GREEN>aeff = aeff * ( aaa )<RESET>
EOF
-test_expect_failure 'word diff without context' '
+test_expect_success 'word diff without context' '
word_diff --color-words --unified=0
--
1.6.4.GIT
^ permalink raw reply related
* Re: [PATCH/RFC 2/2] Provide a build time default-editor setting
From: David Roundy @ 2009-10-29 10:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Ben Walton, GIT List
In-Reply-To: <20091029075021.GC15403@progeny.tock>
Any chance this will be exported as plumbing? I know it's pretty
high-level, but it'd be handy to have be able to write `git editor
$FILENAME` and just have it do the right thing. This would also mean
that the perl scripts below could be simplified.
Same goes for pager, of course...
David
On Thu, Oct 29, 2009 at 3:50 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Provide a DEFAULT_EDITOR knob to allow the fallback editor (to
> use instead of vi if VISUAL, EDITOR, and GIT_EDITOR are unset) to
> be set at build time according to a system’s policy. For
> example, on Debian systems, the default editor should be the
> 'editor' command.
>
> The contrib/fast-import/git-p4 script still uses vi, since it is
> not modified by the Makefile currently, and making it require
> build-time modification would create too much trouble for people
> deploying that script.
>
> This change makes t7005-editor into a mess. Any ideas for fixing
> this?
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Makefile | 10 ++++++++++
> editor.c | 2 +-
> git-add--interactive.perl | 3 ++-
> git-sh-setup.sh | 6 ++++--
> git-svn.perl | 5 +++--
> t/Makefile | 2 ++
> t/t7005-editor.sh | 29 ++++++++++++++++++++++-------
> 7 files changed, 44 insertions(+), 13 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index fc1a461..fae8647 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -203,6 +203,9 @@ all::
> #
> # Define DEFAULT_PAGER to the path of a sensible pager (defaults to "less") if
> # you want to use something different.
> +#
> +# Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you
> +# want to use something different.
>
> GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
> @$(SHELL_PATH) ./GIT-VERSION-GEN
> @@ -1301,6 +1304,11 @@ ifndef DEFAULT_PAGER
> DEFAULT_PAGER = less
> endif
> BASIC_CFLAGS += -DDEFAULT_PAGER='"$(DEFAULT_PAGER)"'
> +ifndef DEFAULT_EDITOR
> + DEFAULT_EDITOR = vi
> +endif
> +export DEFAULT_EDITOR
> +BASIC_CFLAGS += -DDEFAULT_EDITOR='"$(DEFAULT_EDITOR)"'
>
> ifdef USE_NED_ALLOCATOR
> COMPAT_CFLAGS += -DUSE_NED_ALLOCATOR -DOVERRIDE_STRDUP -DNDEBUG -DREPLACE_SYSTEM_ALLOCATOR -Icompat/nedmalloc
> @@ -1435,6 +1443,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
> -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
> -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
> -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
> + -e 's|DEFAULT_EDITOR:=vi|DEFAULT_EDITOR:=$(DEFAULT_EDITOR)|' \
> -e $(BROKEN_PATH_FIX) \
> $@.sh >$@+ && \
> chmod +x $@+ && \
> @@ -1459,6 +1468,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
> -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \
> -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
> -e 's/@@DEFAULT_PAGER@@/$(DEFAULT_PAGER)/g' \
> + -e 's/@@DEFAULT_EDITOR@@/$(DEFAULT_EDITOR)/g' \
> $@.perl >$@+ && \
> chmod +x $@+ && \
> mv $@+ $@
> diff --git a/editor.c b/editor.c
> index 4d469d0..93b8cbb 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -19,7 +19,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
> return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
>
> if (!editor)
> - editor = "vi";
> + editor = DEFAULT_EDITOR;
>
> if (strcmp(editor, ":")) {
> size_t len = strlen(editor);
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 69aeaf0..c3d932c 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1,6 +1,7 @@
> #!/usr/bin/perl -w
>
> use strict;
> +use constant DEFAULT_EDITOR => '@@DEFAULT_EDITOR@@';
> use Git;
>
> binmode(STDOUT, ":raw");
> @@ -988,7 +989,7 @@ EOF
> close $fh;
>
> my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor")
> - || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
> + || $ENV{VISUAL} || $ENV{EDITOR} || DEFAULT_EDITOR;
> system('sh', '-c', $editor.' "$@"', $editor, $hunkfile);
>
> if ($? != 0) {
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index c41c2f7..d053d56 100755
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -99,19 +99,21 @@ set_reflog_action() {
> }
>
> git_editor() {
> + : "${DEFAULT_EDITOR:=vi}"
> : "${GIT_EDITOR:=$(git config core.editor)}"
> : "${GIT_EDITOR:=${VISUAL:-${EDITOR}}}"
> case "$GIT_EDITOR,$TERM" in
> ,dumb)
> echo >&2 "No editor specified in GIT_EDITOR, core.editor, VISUAL,"
> - echo >&2 "or EDITOR. Tried to fall back to vi but terminal is dumb."
> + echo >&2 "or EDITOR. Tried to fall back to $DEFAULT_EDITOR" \
> + "but terminal is dumb."
> echo >&2 "Please set one of these variables to an appropriate"
> echo >&2 "editor or run $0 with options that will not cause an"
> echo >&2 "editor to be invoked (e.g., -m or -F for git-commit)."
> exit 1
> ;;
> esac
> - eval "${GIT_EDITOR:=vi}" '"$@"'
> + eval "${GIT_EDITOR:=$DEFAULT_EDITOR}" '"$@"'
> }
>
> is_bare_repository () {
> diff --git a/git-svn.perl b/git-svn.perl
> index c270b23..b98d378 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3,12 +3,13 @@
> # License: GPL v2 or later
> use warnings;
> use strict;
> -use vars qw/ $AUTHOR $VERSION $DEFAULT_PAGER
> +use vars qw/ $AUTHOR $VERSION $DEFAULT_PAGER $DEFAULT_EDITOR
> $sha1 $sha1_short $_revision $_repository
> $_q $_authors $_authors_prog %users/;
> $AUTHOR = 'Eric Wong <normalperson@yhbt.net>';
> $VERSION = '@@GIT_VERSION@@';
> $DEFAULT_PAGER = '@@DEFAULT_PAGER@@';
> +$DEFAULT_EDITOR = '@@DEFAULT_EDITOR@@';
>
> # From which subdir have we been invoked?
> my $cmd_dir_prefix = eval {
> @@ -1322,7 +1323,7 @@ sub get_commit_entry {
> close $log_fh or croak $!;
>
> if ($_edit || ($type eq 'tree')) {
> - my $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';
> + my $editor = $ENV{VISUAL} || $ENV{EDITOR} || $DEFAULT_EDITOR;
> # TODO: strip out spaces, comments, like git-commit.sh
> system($editor, $commit_editmsg);
> }
> diff --git a/t/Makefile b/t/Makefile
> index bd09390..9174bbb 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -9,6 +9,8 @@
> SHELL_PATH ?= $(SHELL)
> TAR ?= $(TAR)
> RM ?= rm -f
> +DEFAULT_EDITOR ?= vi
> +export DEFAULT_EDITOR
>
> # Shell quote;
> SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
> index b647957..2b76f72 100755
> --- a/t/t7005-editor.sh
> +++ b/t/t7005-editor.sh
> @@ -4,7 +4,18 @@ test_description='GIT_EDITOR, core.editor, and stuff'
>
> . ./test-lib.sh
>
> -for i in GIT_EDITOR core_editor EDITOR VISUAL vi
> +: ${DEFAULT_EDITOR=vi}
> +
> +unset EDITOR VISUAL GIT_EDITOR
> +
> +case "$DEFAULT_EDITOR" in
> +*/* | [A-Z]*)
> + DEFAULT_EDITOR=
> + ;;
> +esac
> +
> +for i in GIT_EDITOR core_editor EDITOR VISUAL \
> + ${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"}
> do
> cat >e-$i.sh <<-EOF
> #!$SHELL_PATH
> @@ -12,15 +23,17 @@ do
> EOF
> chmod +x e-$i.sh
> done
> -unset vi
> -mv e-vi.sh vi
> -unset EDITOR VISUAL GIT_EDITOR
> +
> +if test -n "$DEFAULT_EDITOR"
> +then
> + mv "e-$DEFAULT_EDITOR.sh" "$DEFAULT_EDITOR"
> +fi
>
> test_expect_success setup '
>
> msg="Hand edited" &&
> echo "$msg" >expect &&
> - git add vi &&
> + git add "e-VISUAL.sh" &&
> test_tick &&
> git commit -m "$msg" &&
> git show -s --pretty=oneline |
> @@ -44,7 +57,8 @@ test_expect_success 'dumb should error out when falling back on vi' '
>
> TERM=vt100
> export TERM
> -for i in vi EDITOR VISUAL core_editor GIT_EDITOR
> +for i in ${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"} \
> + EDITOR VISUAL core_editor GIT_EDITOR
> do
> echo "Edited by $i" >expect
> unset EDITOR VISUAL GIT_EDITOR
> @@ -68,7 +82,8 @@ done
>
> unset EDITOR VISUAL GIT_EDITOR
> git config --unset-all core.editor
> -for i in vi EDITOR VISUAL core_editor GIT_EDITOR
> +for i in ${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"} \
> + EDITOR VISUAL core_editor GIT_EDITOR
> do
> echo "Edited by $i" >expect
> case "$i" in
> --
> 1.6.5.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
David Roundy
^ permalink raw reply
* Re: [PATCH] git-rebase -i: improve usage message
From: Thomas Rast @ 2009-10-29 9:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brian Ewins, git, kusmabite
In-Reply-To: <7vtyxiafa9.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
>
> The OPTIONS_SPEC in rebase--interactive is for the interactive mode and
> for nothing else, so it may be a good idea to clearly say so at the
> beginning. The user experience perhaps should look like:
>
> $ git rebase -i -h
> Note: this help is only about the interactive mode;
> see 'git rebase -h' for help on non-interactive mode.
>
> usage: git rebase -i [<options>] [--] <upstream> [<branch>]
> or: git rebase -i (--continue|--abort|--skip)
Nit-pick: the last line is not correct; the user does not have to
specify -i for git-rebase to figure out that an interactive rebase is
in progress.
(I agree with the rest though.)
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH I18N filenames v2 2/3] Use I18N-wrappers everywhere in Git
From: Alex Riesen @ 2009-10-29 9:30 UTC (permalink / raw)
To: Timur Sufiev; +Cc: git
In-Reply-To: <1256752900-2615-2-git-send-email-timur@iris-comp.ru>
On Wed, Oct 28, 2009 at 19:01, Timur Sufiev <timur@iris-comp.ru> wrote:
> Signed-off-by: Timur Sufiev <timur@iris-comp.ru>
> ---
> abspath.c | 1 +
> attr.c | 1 +
> bisect.c | 1 +
Instead of modifying all these files you could just have put that
darn header in compat/mingw.h
^ 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