* Re: git hang with corrupted .pack
From: Shawn O. Pearce @ 2009-11-03 22:28 UTC (permalink / raw)
To: Pascal Obry
Cc: Alex Riesen, Junio C Hamano, Nicolas Pitre, Andy Isaacson, git
In-Reply-To: <4AF0A132.1060103@obry.net>
Pascal Obry <pascal@obry.net> wrote:
> Le 20/10/2009 17:14, Alex Riesen a ??crit :
>> I seem to have problems with this change (on Cygwin). Sometimes
>> accessing an object in a pack fails in unpack_compressed_entry.
>> When it happens, both avail_in and avail_out of the stream are 0,
>> and the reported status is Z_BUF_ERROR.
>> Output with the second attached patch:
>>
>> error: *** inflate error: 0x862380 size=1256, avail_in=0 (was 697),
>> avail_out=0 (was 1256)
>> error: *** unpack_compressed_entry failed
>> error: failed to read object 3296766eb5531ef051ae392114de5d75556f5613
>> at offset 2620741 from
>> .git/objects/pack/pack-996206790aaefbf4d34c86b3ff546bb924546b7c.pack
>> fatal: object 3296766eb5531ef051ae392114de5d75556f5613 is corrupted
>
> I have this problem on some repository on Cygwin too. For now I have
> reverted to Git 1.6.4 which seems to be working fine.
It was fixed in 1.6.5.2 by Junio.
--
Shawn.
^ permalink raw reply
* Re: git hang with corrupted .pack
From: Pascal Obry @ 2009-11-03 22:34 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Alex Riesen, Junio C Hamano, Nicolas Pitre, Andy Isaacson, git
In-Reply-To: <20091103222834.GC10505@spearce.org>
Shawn,
> It was fixed in 1.6.5.2 by Junio.
Hum... I was using today Git from master. Just after I send my message I
tried 1.6.5 and it was working... I went back to master and it is now
working... Really strange... Maybe a file did not get properly
recompiled. I doubt it... but for sure it was failing and I build git
every day from master! Strange!
Anyway, it was a false alarm.
Pascal.
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net - http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Nanako Shiraishi @ 2009-11-03 22:38 UTC (permalink / raw)
To: Erick Mattos; +Cc: Junio C Hamano, git
In-Reply-To: <1257282551-9999-1-git-send-email-erick.mattos@gmail.com>
Quoting Erick Mattos <erick.mattos@gmail.com>
> When we use one of the options above we are normally trying to do mainly
> two things: using the source as a template or recreating a commit with
> corrections.
>
> When they are used, the authorship and timestamp recorded in the newly
> created commit are always taken from the original commit. And they
> should not when we are using it as a template or when our change to the
> code is so significant that we should take over the authorship (with the
> blame for bugs we introduce, of course).
>
> The new --reset-author option is meant to solve this need by
> regenerating the timestamp and setting as the new author the committer
> or the one specified by --author option.
>
> Signed-off-by: Erick Mattos <erick.mattos@gmail.com>
> ---
If you are sending an update to a previous patch (I am comparing
this patch with the "show by example" patch Junio sent on 11/02
http://article.gmane.org/gmane.comp.version-control.git/131893),
it is a common courtesy to summarize what you changed relative
to the base version after the three dashes line, so that people
will know which part can be skipped while reviewing your patch.
I was originally puzzled by Junio's "mine" because I thought
"what does taking over the authorship have to do with digging
diamonds out of earth?". "reset-author" makes it very clear,
and I think it is a much better name for the option.
Your first paragraph mentions "one of the options", but subject
mentions four (-c, -C, --amend and --reset-author) and you want
only the first three of them.
The second paragraph of gmane/131893 says "borrow the commit log
message" instead of repeating "a template". It has the effect of
clarifying that only the message part is borrowed but not the
authorship information.
I think gmane/131893 has much better description than your first
two paragraphs for the these reasons.
> diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
> new file mode 100755
> index 0000000..7290242
> --- /dev/null
> +++ b/t/t7509-commit.sh
I have to say that the test script is much worse than what
gmane/131893 had.
The old test made sure that -C copied the message, with or
without the --mine option. But this test only checks the
author line (and it doesn't even make sure that "^author"
matches only in the header). The messages are unchecked,
and it will let a bug when someone breaks --reset-author
logic in the future in such a way that it corrupts the
message by mistake go unnoticed.
Also the old test was much more readable because it used
shell functions to avoid repeating cat-file and pipe to sed
script. It also tagged the initial commit, which had a nice
effect that a failure in any intermediate test will not change
which earlier commit is reused (eg. yours say "-C HEAD^" but
old test said "-C Initial").
It looks silly to create an "Initial Commit" in the middle
of history, too (^_^).
I think it is much better to replace "--mine" in gmane/131893
with "--reset-author" and make no other change to the test.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: Problem with "From:" line on "git format-patch" generated patches
From: Jonathan Nieder @ 2009-11-03 22:55 UTC (permalink / raw)
To: André Goddard Rosa; +Cc: Santi Béjar, Git Mailing List
In-Reply-To: <b8bf37780911031011v5c8ec684ke6eebc6b0de1a66a@mail.gmail.com>
Hi André,
André Goddard Rosa wrote:
>> I'm not using any specific tool for inputting the git-format-patch,
>> but instead I'm sending the files generated by it through gmail as an
>> inlined patch in the email body.
>>
>> I like the convenience of format-patch for generating the patch files,
>> but in this case, formatting the header as rfc2047 is not necessary
>> and makes a funny/garbled output in my patch submission.
The header fields git format-patch outputs are just intended as a
starting point for the header of your mailing. It is more convenient
to receive an e-mail with
Delivered-to: maintainer@example.com
Received: [...]
Message-ID: <patch.sender.0001@example.com>
Date: Tue, 03 Nov 2009 16:33:54 -0600
From: Patch Sender <patch.sender@example.com>
Subject: [PATCH] Fix one bug, add another
Content-Type: text/plain; charset=us-ascii
Blah blah blah
than one in which the content includes some useless metadata that was
already in the header. So you should just strip the header out from
the body before sending.
There are three common exceptions: 1) you might want to send a patch
written by someone else, 2) you might want to mark a patch as written
before it was sent, and 3) some people like to receive patches as
attachments rather than inlined in messages. For the first two cases,
the solution is to include the header fields to change in the body:
From: Patch Writer <patch.writer@example.com>
Date: Wed, 01 Apr 1970 01:23:45 +0100
Blah blah blah
---
Hi,
Patch Writer wrote this patch a while ago that might be
relevant. It needed a straightforward one-line change to
apply and is otherwise unchanged.
What do you think?
[...]
For the last case, I think it is most common to send unchanged 'git
format-patch' output. But only the From, Date, and Subject fields
are actually needed.
I am not sure how well 'git am' copes with non-ascii characters in
the pseudo-header lines: I would have guessed it could handle them
both rfc2047-encoded and not, but I have not tried.
> I really would like continuing having the convenience of using a web
> access to my gmail for sending the patches, so I just need a way to
> format the patches which makes it easy submitting them later. I'd like
> to avoid using any other email client for that, if possible.
Here, there is another danger: the Gmail web interface does not
consider your whitespace precious, so it is very prone to mangling
patches (especially with long lines).
Documentation/SubmittingPatches [1] has some advice:
| Gmail
| -----
|
| GMail does not appear to have any way to turn off line wrapping in the web
| interface, so this will mangle any emails that you send. You can however
| use any IMAP email client to connect to the google imap server, and forward
| the emails through that. Just make sure to disable line wrapping in that
| email client. Alternatively, use "git send-email" instead.
|
| Submitting properly formatted patches via Gmail is simple now that
| IMAP support is available. First, edit your ~/.gitconfig to specify your
| account settings:
|
| [imap]
| folder = "[Gmail]/Drafts"
| host = imaps://imap.gmail.com
| user = user@gmail.com
| pass = p4ssw0rd
| port = 993
| sslverify = false
|
| You might need to instead use: folder = "[Google Mail]/Drafts" if you get an error
| that the "Folder doesn't exist".
|
| Next, ensure that your Gmail settings are correct. In "Settings" the
| "Use Unicode (UTF-8) encoding for outgoing messages" should be checked.
|
| Once your commits are ready to send to the mailing list, run the following
| command to send the patch emails to your Gmail Drafts folder.
|
| $ git format-patch -M --stdout origin/master | git imap-send
|
| Go to your Gmail account, open the Drafts folder, find the patch email, fill
| in the To: and CC: fields and send away!
Good luck.
Hope that helps,
Jonathan
[1] <http://git.kernel.org/?p=git/git.git;a=blob_plain;f=Documentation/SubmittingPatches>
converting tabs to spaces.
^ permalink raw reply
* [PATCH] gitk: Fix "git gui blame" invocation when called from topdir
From: Markus Heidelberg @ 2009-11-03 23:21 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Markus Heidelberg
In-Reply-To: <19184.2163.760155.285153@cargo.ozlabs.ibm.com>
In this case "git rev-parse --git-dir" doesn't return an absolute path,
but merely ".git", so the selected file has a relative path.
The function make_relative then tries to make the already relative path
relative, which results in a path like "../../../../Makefile" with as
much ".." as the number of parts [pwd] consists of.
This regression was introduced by commit 9712b81 (gitk: Fix bugs in
blaming code, 2008-12-06), which fixed "git gui blame" when called from
subdirs.
This also fixes it for bare repositories.
Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
Paul Mackerras, 03.11.2009:
> Thanks for the patch, but I'd prefer to just add:
>
> if {[file pathtype $f] ne "relative"} {
> return $f
> }
>
> at the start of the function. I think that's easier to read than
> having a big if statement.
Definitely yes. But eq instead of ne.
gitk | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/gitk b/gitk
index 32e4ab0..949abfe 100755
--- a/gitk
+++ b/gitk
@@ -3378,6 +3378,9 @@ proc index_sha1 {fname} {
# Turn an absolute path into one relative to the current directory
proc make_relative {f} {
+ if {[file pathtype $f] eq "relative"} {
+ return $f
+ }
set elts [file split $f]
set here [file split [pwd]]
set ei 0
--
1.6.5.2.155.gaa0e5
^ permalink raw reply related
* Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads
From: Johannes Schindelin @ 2009-11-03 23:38 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: git
In-Reply-To: <1257283802-29726-2-git-send-email-ahaczewski@gmail.com>
Hi,
On Tue, 3 Nov 2009, Andrzej K. Haczewski wrote:
> ---
Could you please add the reasoning from the cover letter to this commit
message? And add a sign-off?
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 02f9246..a8a4f59 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -18,8 +18,12 @@
> #include "refs.h"
>
> #ifdef THREADED_DELTA_SEARCH
> -#include "thread-utils.h"
> -#include <pthread.h>
> +# include "thread-utils.h"
> +# ifndef _WIN32
> +# include <pthread.h>
> +# else
> +# include <winthread.h>
> +# endif
> #endif
>
It is unlikely that an #ifdef "contamination" of this extent will go
through easily, but I have a suggestion that may make your patch both
easier to read and more likely to be accepted into git.git: Try to wrap
the win32 calls into pthread-compatible function signatures. Then you can
add a compat/win32/pthread.h and not even touch core files of git.git at
all.
Oh, and you definitely do not want to copy-paste err_win_to_posix(). You
definitely want to reuse the existing instance.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Erick Mattos @ 2009-11-03 23:51 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Junio C Hamano, git
In-Reply-To: <20091104073822.6117@nanako3.lavabit.com>
2009/11/3 Nanako Shiraishi <nanako3@lavabit.com>:
> If you are sending an update to a previous patch (I am comparing
> this patch with the "show by example" patch Junio sent on 11/02
> http://article.gmane.org/gmane.comp.version-control.git/131893),
> it is a common courtesy to summarize what you changed relative
> to the base version after the three dashes line, so that people
> will know which part can be skipped while reviewing your patch.
I got your point. I will try to improve that.
I have been talking to Junio during the weekend and with a lot of
emails sent to each other. It happens that when he sent gmane/131893
(I had to find out what that code meant because I was using
[marc.info]! :-) ) I had already sent another patch with the
suggestions he made in a previous email.
So his message was late. While I was waiting for his acknowledgment I
started thinking he could be lost on those e-mails so I sent the one
you are replying to make it the last on the queue.
> I have to say that the test script is much worse than what
> gmane/131893 had.
>
> The old test made sure that -C copied the message, with or
> without the --mine option. But this test only checks the
> author line (and it doesn't even make sure that "^author"
> matches only in the header). The messages are unchecked,
> and it will let a bug when someone breaks --reset-author
> logic in the future in such a way that it corrupts the
> message by mistake go unnoticed.
I think you misunderstood something here:
* On his patch (which he sent before seeing mine), while testing -C,
on first check, he is checking author_header only.
* While testing -C on mine I compare both messages without "parent",
"tree" and "committer". Whole!
After check one I did concentrate only on author data. But on mine I
separate the tests between timestamp and author (name and email),
making sure the author was set to the actual committer and that the
timestamp was behaving as expected.
I am not saying mine is better. What I am saying is that I expected
him to notice and change/improve THIS patch. Not the other.
The new option only touches on getting new author or copying the
original so that is why I made the first check in whole and the others
only by author. If people think that this operation is so uncertain,
then everything should be compared: parent, author and message on all
tests.
It is not a problem for me adding more code to the test even if I
consider it unnecessary. I am doing this only to give a pay-back for
all the good service this free software gave to me so I am very
patient to all demands. I will be letting this effort go only at the
real end. No matter how long it takes.
> Also the old test was much more readable because it used
> shell functions to avoid repeating cat-file and pipe to sed
> script. It also tagged the initial commit, which had a nice
> effect that a failure in any intermediate test will not change
> which earlier commit is reused (eg. yours say "-C HEAD^" but
> old test said "-C Initial").
I am so used to scripts that I really haven't thought it difficult to
read but I can do some cosmetic too if it is important. As I said
early, I was waiting for Junio's jugdement over my later patch.
> It looks silly to create an "Initial Commit" in the middle
> of history, too (^_^).
This is something more laborious but which I thought was important to
let murphy's law act on a real case. We never do an amend on an
initial commit so I only did the tests on a later one.
> I think it is much better to replace "--mine" in gmane/131893
> with "--reset-author" and make no other change to the test.
Let's see Junio's opinion... We have changed names a lot since start. ;-)
> --
> Nanako Shiraishi
> http://ivory.ap.teacup.com/nanako3/
>
>
Thank you very much for all your comments. I really appreciate about
being noticed by you people. It is nice for a newcomer!
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Junio C Hamano @ 2009-11-04 0:40 UTC (permalink / raw)
To: Scott Chacon; +Cc: Shawn O. Pearce, git list
In-Reply-To: <d411cc4a0911031405x60ea2396o35eea78a0b07fda9@mail.gmail.com>
Scott Chacon <schacon@gmail.com> writes:
> Would you prefer that I try to incorporate all these comments and
> submit another patch, or are you going to modify it in-tree?
I prefer to not munge other people's patches if the modification will
involve more than ten lines and if I know the original submitter can be
trusted to do the right thing.
I would rewrite other's patch myself heavily _only_ after I tried the
usual "suggest to improve" and found that it would not be worth my time to
wait for yet more rounds that would require more suggestions, major part
of which will eventually be forgotten or ignored again, iow, when I sense
that the communication with the contributor is hopelessly inefficient.
Needless to say, you have been here long enough to be in the "trusted"
category.
Thanks.
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Junio C Hamano @ 2009-11-04 0:40 UTC (permalink / raw)
To: Scott Chacon; +Cc: git list, Shawn O. Pearce
In-Reply-To: <d411cc4a0911011518q15a8267bn642e6937be8c9ab1@mail.gmail.com>
Scott Chacon <schacon@gmail.com> writes:
> diff --git a/Documentation/technical/protocol-capabilities.txt
> b/Documentation/technical/protocol-capabilities.txt
> new file mode 100644
> index 0000000..3c86fc3
> --- /dev/null
> +++ b/Documentation/technical/protocol-capabilities.txt
> @@ -0,0 +1,188 @@
> +Git Protocol Capabilities
> +=========================
> +
> +Servers SHOULD support all capabilities defined in this document.
> +
> +On the very first line of the initial server response, the first
> +reference is followed by a null byte and then a list of space
> +delimited server capabilities. These allow the server to declare
> +what it can and cannot do to the client.
A few paragraphs below the reader will notice that it talks about both,
but it would be better to make it clear upfront that this document intends
to apply to both of the protocols (fetch-pack protocol and send-pack
protocol).
> +Client sends space separated list of capabilities it wants. It
> +SHOULD send a subset of server capabilities, i.e do not send
> +capabilities served does not advertise. The client SHOULD NOT ask
> +for capabilities the server did not say it supports.
The last two sentences are redundant, and "served does not" is probably
"server did not". I'd suggest dropping from "It SHOULD send a subset"
to "does not advertise."
> +Server MUST ignore capabilities it does not understand. Server MUST
> +NOT ignore capabilities that client requested and server advertised.
Hmm, is the first sentence true? Shouldn't it notice and error out?
After all the other end is asking something we do not know how to obey,
and there is no guarantee we do the right thing if we ignore blindly.
> +The 'report-status' and 'delete-refs' capabilities are sent and
> +recognized by the receive-pack (push to server) process.
> +
> +The 'ofs-delta' capability is sent and recognized by both upload-pack
> +and receive-pack protocols.
> +
> +All other capabilities are only recognized by the upload-pack (fetch
> +from server) process.
> +
> +multi_ack
> +---------
> +
> +The 'multi_ack' capability allows the server to return "ACK $SHA1
> +continue" as soon as it finds a commit that it can use as a common
> +base, between the client's wants and the client's have set.
s/$SHA1/obj-id/; to be consistent with the other document.
> +By sending this early, the server can potentially head off the client
> +from walking any further down that particular branch of the client's
> +repository history. The client may still need to walk down other
> +branches, sending have lines for those, until the server has a
> +complete cut across the DAG, or the client has said "done".
> +
> +Without multi_ack, a client sends have lines in --date-order until
> +the server has found a common base. That means the client will send
> +have lines that are already known by the server to be common, because
> +they overlap in time with another branch that the server hasn't found
> +a common base on yet.
> +
> +The client has things in caps that the server doesn't; server has
> +things in lower case.
s/things/commits/g; perhaps?
It's a bit abrupt to omit "For example suppose ...; we will illustrate
what happens." entirely.
> + +---- u ---------------------- x
> + / +----- y
> + / /
> + a -- b -- c -- d -- E -- F
> + \
> + +--- Q -- R -- S
Is it just my mail client, or is the line from 'd' to 'y' misaligned and
does not begin at 'd'?
> +If the client wants x,y and starts out by saying have F,S, the server
> +doesn't know what F,S is. Eventually the client says "have d" and
> +the server sends "ACK d continue" to let the client know to stop
> +walking down that line (so don't send c-b-a), but its not done yet,
> +it needs a base for X. The client keeps going with S-R-Q, until a
s/X/x/;
Otherwise, a nice example that is easy to understand.
> +thin-pack
> +---------
> +
> +Server can send thin packs, i.e. packs which do not contain base
> +elements, if those base elements are available on clients side.
> +Client requests thin-pack capability when it understands how to "thicken"
> +them adding required delta bases making them independent.
We call it "base object" when referring to what a "deltified
representation of an object" is based on. "base element" is a word that
has never been used to describe it elsewhere in our documentation set.
It seems that git-index-pack.txt calls the requirement of a packfile "self
contained and indexable"; I think "self contained" would be a better
wording than "independent" which has never been used to describe this
condition elsewhere in our documentation set.
> +side-band, side-band-64k
> +------------------------
> +
> +This means that server can send, and client understand multiplexed
> +(muxed) progress reports and error info interleaved with the packfile
> +itself.
I don't see need for "(muxed)"; you don't use the abbreviated form
anywhere else in the rest of the document.
> +These two options are mutually exclusive. A client should ask for
> +only one of them, and a modern client always favors side-band-64k.
> +
> +Either mode indicates that the packfile data will be streamed broken
> +up into packets of either 1000 bytes in the case of 'side_band', or
> +65520 bytes in the case of 'side_band_64k'. Each packet is made up of
> +a leading 4-byte pkt-line length of how much data is in the packet,
> +followed by a 1-byte stream code, followed by the actual data.
I think I mumbled something about "up to" in the other document on this
same topic. The same comment applies here.
> +The stream code can be one of:
> +
> + 1 - pack data
> + 2 - progress messages
> + 3 - fatal error message just before stream aborts
> +
> +The "side-band-64k" capability came about as a way for newer clients
> +that can handle much larger packets to request packets that are
> +actually crammed nearly full, while maintaining backward compatibility
> +for the older clients.
> +
> +Further, with side-band and its 1000 byte messages, it's actually
> +999 bytes of payload and 1 byte for the stream code. With side-band-64k,
> +same deal, you have 65519 bytes of data and 1 byte for the stream code.
> +
> +The client MUST send only maximum of one of "side-band" and "side-
> +band-64k". Server MUST favor side-band-64k if client requests both.
Again I think sending both is an error and should be diagnosed as such.
This is not a three-way handshake where I say "I can handle both", you say
"I can too", and then I finally pick "then we'll use this one". There is
no way for the requesting side to tell which one was chosen, and the
requester who sent both assumed that the other end chose "side-band" and
allocated only a 1000-byte buffer like older implementation did, the limit
of buffer will be busted.
Fix the last line "Server MUST favor" to "Server MUST diagnose it as an
error". Also drop "A client should ask for only one of them," near the
beginning of this section, as it is redundant. I think it is fine to keep
"A modern client always favors".
> +ofs-delta
> +---------
> +
> +Server can send, and client understand PACKv2 with delta refering to
> +its base by position in pack rather than by SHA-1. Its that they can
> +send/read OBJ_OFS_DELTA, aka type 6 in a pack file.
"Its that"? EPARSE. Perhaps "That is,"?
> +include-tag
> +-----------
> +
> +The 'include-tag' capability is about sending tags if we are sending
> +objects they point to. If we pack an object to the client, and a tag
> +points exactly at that object, we pack the tag too. In general this
> +allows a client to get all new tags when it fetches a branch, in a
> +single network connection.
> +
> +Clients MAY always send include-tag, hardcoding it into a request.
"... when the server advertises this capability", no?
> +The decision for a client to request include-tag only has to do with
> +the client's desires for tag data, whether or not a server had
> +advertised objects in the refs/tags/* namespace.
> +
> +Clients SHOULD NOT send include-tag if remote.name.tagopt was set to
> +--no-tags, as the client doesn't want tag data.
> +
> +Servers MUST accept include-tag without error or warning, even if the
> +server does not understand or support the option.
Why is this special case here? In the beginning, the servers are required
to support all of these (and include-tag is part of that "all"). If a
server for some justifiable reason does not understand a capability, it
would ignore it anyway with your earlier specification, so there is no
need to spell this out. Since I do not agree with "ignore capability
requests for something we do not know" at all, I cannot agree with this
sentence. If a server does not understand this capability, it is an error
for a client to ask for it, and it should be diagnosed as a protocol
error, no?
> +Servers SHOULD pack the tags if their referrant is packed and the
> +client has requested include-tag.
Sorry, I do not understand the motivation to make make this so weak? If
the server claims to support this capability, and when a referrant is
going to the client, the server MUST do so---if it cannot guarantee, why
claim to support that capability?
Or am I missing something?
> +Clients MUST be prepared for the case where a server has ignored
> +include-tag and has not actually sent tags in the pack. In such
> +cases the client SHOULD issue a subsequent fetch to acquire the tags
> +that include-tag would have otherwise given the client.
> +
> +The server SHOULD send include-tag, if it supports it, irregardless
> +of whether or not there are tags available.
irregardless?
> diff --git a/Documentation/technical/protocol-common.txt
> b/Documentation/technical/protocol-common.txt
> new file mode 100644
> index 0000000..ddf9912
> --- /dev/null
> +++ b/Documentation/technical/protocol-common.txt
> @@ -0,0 +1,96 @@
> +Documentation Common to Pack and Http Protocols
> +===============================================
> +
> +ABNF Notation
> +-------------
> +
> +ABNF notation as described by RFC 5234 is used within the protocol documents,
> +except the following replacement core rules are used:
> +----
> + HEXDIG = DIGIT / "a" / "b" / "c" / "d" / "e" / "f"
> +----
> +
> +We also define the following common rules:
> +----
> + NUL = %x00
> + zero-id = 40*"0"
> + obj-id = 40*(HEXDIGIT)
> +
> + refname = "HEAD"
> + refname /= "refs/" <see discussion below>
> +----
> +
> +A refname is a hierarichal octet string beginning with "refs/" and
> +not violating the 'git-check-ref-format' command's validation rules.
> +More generally, they:
s/generally/specifically/; I think. Also if you end with "they:" and
continue with enumeration, each enumerated sentence should omit "they",
no?
> +. They can include slash `/` for hierarchical (directory)
> + grouping, but no slash-separated component can begin with a
> + dot `.`.
> +...
> +pkt-line Format
> +---------------
> +
> +Much (but not all) of the payload is described around pkt-lines.
> +
> +A pkt-line is a variable length binary string. The first four bytes
> +of the line, the pkt-len, indicates the total length of the line,
> +in hexadecimal. The pkt-len includes the 4 bytes used to contain
> +the length's hexadecimal representation.
> +
> +A pkt-line MAY contain binary data, so implementors MUST ensure
> +pkt-line parsing/formatting routines are 8-bit clean.
> +
> +A non-binary line SHOULD BE terminated by an LF, which if present
> +MUST be included in the total length.
> +
> +The maximum length of a pkt-line's data component is 65520 bytes.
> +Implementations MUST NOT send pkt-line whose length exceeds 65524
> +(65520 bytes of payload + 4 bytes of length data).
> +
> +Implementations SHOULD NOT send an empty pkt-line ("0004").
Not an objection, but where is this coming from?
> +A pkt-line with a length field of 0 ("0000"), called a flush-pkt,
> +is a special case and MUST be handled differently than an empty
> +pkt-line ("0004").
...especially that this sentence makes it sound as if it is perfectly
normal to send "0004" for "an empty line" (and I've always thought that is
Ok), I am quite puzzled by that "SHOULD NOT".
> +----
> + pkt-line = data-pkt / flush-pkt
> +
> + data-pkt = pkt-len pkt-payload
> + pkt-len = 4*(HEXDIG)
> + pkt-payload = (pkt-len - 4)*(OCTET)
> +
> + flush-pkt = "0000"
> +----
> +
> +Examples (as C-style strings):
> +
> +----
> + pkt-line actual value
> + ---------------------------------
> + "0006a\n" "a\n"
> + "0005a" "a"
> + "000bfoobar\n" "foobar\n"
> + "0004" ""
Whew. Sorry for taking more than a few days to give you review comments.
It must have been quite a lot of work to carefully assemble these
documents.
Thanks.
^ permalink raw reply
* Re: Common setting for interoperability repo across windows and unix?
From: Vietor Liu @ 2009-11-04 0:46 UTC (permalink / raw)
To: Dilip M; +Cc: git
In-Reply-To: <c94f8e120911030709h29c5b8edr53df269632990e81@mail.gmail.com>
On Tue, 2009-11-03 at 20:39 +0530, Dilip M wrote:
> Hello,
>
> I have repo in unix. The same repo is cloned onto windows.I have set
> "core.autocrlf=input" in both the repos.
>
> When I do some change to a file in windows and push to unix repo, I
> get file deleted If I do "git status"?
>
> What is the setting to be done if I want an repo to be
> access/push/pulled across windows and unix?
>
>
>
msysgit, test:
core.autocrlf=false
^ permalink raw reply
* Re: [PATCH] gitk: disable checkout of remote branch
From: Tim Mazid @ 2009-11-04 0:48 UTC (permalink / raw)
To: git
In-Reply-To: <fabb9a1e0911030807h6b76b661pef75628a1255356@mail.gmail.com>
Sverre Rabbelier-2 wrote:
>
> On Tue, Nov 3, 2009 at 17:00, Sitaram Chamarty <sitaramc@gmail.com> wrote:
>> At the command line, this gives you a detailed warning message, but the
>> GUI currently allows it without any fuss.
>
> This is even better than an annoying popup dialog, as we all know
> those are just ignored anyway :).
>
Might be better to include a configuration option to allow this, for those
that know what they're doing. Most of the people that know what they're
doing will use the command line, anyway, but it may irritate some people.
--
View this message in context: http://n2.nabble.com/PATCH-gitk-disable-checkout-of-remote-branch-tp3939363p3942366.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Shawn O. Pearce @ 2009-11-04 0:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Scott Chacon, git list
In-Reply-To: <7v4opbp1fa.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Scott Chacon <schacon@gmail.com> writes:
>
> > diff --git a/Documentation/technical/protocol-capabilities.txt
> > +
> > +The client MUST send only maximum of one of "side-band" and "side-
> > +band-64k". Server MUST favor side-band-64k if client requests both.
>
> Again I think sending both is an error and should be diagnosed as such.
> This is not a three-way handshake where I say "I can handle both", you say
> "I can too", and then I finally pick "then we'll use this one". There is
> no way for the requesting side to tell which one was chosen, and the
> requester who sent both assumed that the other end chose "side-band" and
> allocated only a 1000-byte buffer like older implementation did, the limit
> of buffer will be busted.
I think Scott borrowed the above from me. The last sentance with that
server MUST is my error.
> Fix the last line "Server MUST favor" to "Server MUST diagnose it as an
> error". Also drop "A client should ask for only one of them," near the
> beginning of this section, as it is redundant. I think it is fine to keep
> "A modern client always favors".
Ack, I agree.
> > +include-tag
> > +-----------
> > +
> > +The 'include-tag' capability is about sending tags if we are sending
> > +objects they point to. If we pack an object to the client, and a tag
> > +points exactly at that object, we pack the tag too. In general this
> > +allows a client to get all new tags when it fetches a branch, in a
> > +single network connection.
> > +
> > +Clients MAY always send include-tag, hardcoding it into a request.
>
> "... when the server advertises this capability", no?
This is also my fault. Its rooted in the way the C implementation
of upload-pack parses the want line, it doesn't care if there are
unrecognized capabilities requested by the client. This is a bug
in the C implementation.
I agree with you, and disagree with the original text I wrote.
> > +Clients SHOULD NOT send include-tag if remote.name.tagopt was set to
> > +--no-tags, as the client doesn't want tag data.
> > +
> > +Servers MUST accept include-tag without error or warning, even if the
> > +server does not understand or support the option.
>
> Why is this special case here?
Ack, I agree with you, this should be removd.
> > +Servers SHOULD pack the tags if their referrant is packed and the
> > +client has requested include-tag.
>
> Sorry, I do not understand the motivation to make make this so weak? If
> the server claims to support this capability, and when a referrant is
> going to the client, the server MUST do so---if it cannot guarantee, why
> claim to support that capability?
>
> Or am I missing something?
IIRC at one time the C implementation didn't fully ensure the tag is
packed when the referrant is packed. This SHOULD exists because it
may have been possible for a tag to be omitted. But I don't think
I've seen this happen under any condition, and it probably is now
a bug if it occurs, which means we likely can convert this to a MUST.
> > diff --git a/Documentation/technical/protocol-common.txt
> > +...
> > +pkt-line Format
> > +---------------
> > +
> > +Implementations SHOULD NOT send an empty pkt-line ("0004").
>
> Not an objection, but where is this coming from?
Me. I think sending "0004" is stupid.
"0004" must immediately be followed by another pkt-len because there
is no data payload behind it. "0004" is the same as having called
write(fd, buf, 0), which is equally pointless. Such a kernel call
will be a no-op, my point here is that "0004" as a packet is also
stupid and shouldn't be sent.
> > +A pkt-line with a length field of 0 ("0000"), called a flush-pkt,
> > +is a special case and MUST be handled differently than an empty
> > +pkt-line ("0004").
>
> ...especially that this sentence makes it sound as if it is perfectly
> normal to send "0004" for "an empty line" (and I've always thought that is
> Ok), I am quite puzzled by that "SHOULD NOT".
I don't think we ever send an empty packet. If we have no data to
send, why the hell did we create the packet header?
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Junio C Hamano @ 2009-11-04 1:07 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Scott Chacon, git list
In-Reply-To: <20091104005614.GD10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
>> > +A pkt-line with a length field of 0 ("0000"), called a flush-pkt,
>> > +is a special case and MUST be handled differently than an empty
>> > +pkt-line ("0004").
>>
>> ...especially that this sentence makes it sound as if it is perfectly
>> normal to send "0004" for "an empty line" (and I've always thought that is
>> Ok), I am quite puzzled by that "SHOULD NOT".
>
> I don't think we ever send an empty packet. If we have no data to
> send, why the hell did we create the packet header?
Oh, I do not disagree that it is pointless, but the example that followed
the part we are discussing also had "0004". I think it is Ok to allow it.
The original intent of packet_flush() was that everything else could be
kept buffered in-core without going to write() until packet_flush() is
called. The protocol was defined in a way that we won't wait for
listening a response from the other end to an earlier message we "sent"
with packet_write() but haven't called packet_flush() yet hence could
still be in our buffer. We still have this comment:
/*
* If we buffered things up above (we don't, but we should),
* we'd flush it here
*/
void packet_flush(int fd)
And once we start buffering, allowing "0004" packet_write() wouldn't even
be a problem; it can be optimized out in the buffering layer.
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Junio C Hamano @ 2009-11-04 1:12 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Erick Mattos, git
In-Reply-To: <20091104073822.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> I think it is much better to replace "--mine" in gmane/131893
> with "--reset-author" and make no other change to the test.
Heh, I would not claim my tests were perfect, even though I agree with
most of the suggestions and comments, including the parts I did not quote
that are not about the test script.
My first test did not check the message contents, but all the other ones
(except the last one that we expect the command to fail) check output from
both author_header and message_body, so that not just "--mine affects the
authorship" but also "--mine does not mangle the message contents" are
checked.
One thing you did not mention was that I made sure that the command failed
when given both --mine and --author options; I think Erick's last round
fails to test this condition.
Side note. When writing tests for their shiny new toy, people often
forget to test "the other side".
It is just human nature. It is more fun to demonstrate what your new
feature does, than making sure that the new feature does not kick in
when it is not supposed to, nor it does not change what it is not
supposed to change.
Negative tests are not particularly hard to write. The harder part is
to force the habit of writing them on yourself. Right after designing
and implementing a new feature, the list of things it is supposed to
do and when it is supposed to kick in are pretty clear in your mind,
and that is what makes writing positive tests psychologically a lot
easier.
On the other hand, it takes concious effort to list what it is _not_
supposed to do or when it is _not_ supposed to kick in. That is why
people often fail to write negative tests.
I think in addition to the obvious "s/--mine/--reset-author/g"
replacements, we would need this patch on top of mine.
t/t7509-commit.sh | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
index ec13cea..1dad228 100755
--- a/t/t7509-commit.sh
+++ b/t/t7509-commit.sh
@@ -28,6 +28,10 @@ test_expect_success '-C option copies authorship and message' '
git commit -a -C Initial &&
author_header Initial >expect &&
author_header HEAD >actual &&
+ test_cmp expect actual &&
+
+ message_body Initial >expect &&
+ message_body HEAD >actual &&
test_cmp expect actual
'
^ permalink raw reply related
* Re: Unhappy git in a jailshell?
From: Alex MDC @ 2009-11-04 1:17 UTC (permalink / raw)
To: Michael J Gruber, Dmitry Potapov; +Cc: git
In-Reply-To: <4AEEE3D2.9040905@drmicha.warpmail.net>
Hi guys,
I requested the admins allow access to the `git --exec-path` directory
from the jailshell and incredibly, they granted it! Now everything is
working and "git --help --all" lists all the commands as expected, so
that was indeed the problem.
It was just a bit deceiving at first that some git commands were
working but others weren't, but Dmitry explained that one.
Thanks for your help,
Alex
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Shawn O. Pearce @ 2009-11-04 1:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Scott Chacon, git list
In-Reply-To: <7vljinnllj.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > I don't think we ever send an empty packet. If we have no data to
> > send, why the hell did we create the packet header?
>
> Oh, I do not disagree that it is pointless, but the example that followed
> the part we are discussing also had "0004". I think it is Ok to allow it.
If its pointless, why encourage it? Why not discourage it with SHOULD NOT?
> The original intent of packet_flush() was that everything else could be
> kept buffered in-core without going to write() until packet_flush() is
> called. The protocol was defined in a way that we won't wait for
> listening a response from the other end to an earlier message we "sent"
> with packet_write() but haven't called packet_flush() yet hence could
> still be in our buffer. We still have this comment:
>
> /*
> * If we buffered things up above (we don't, but we should),
> * we'd flush it here
> */
> void packet_flush(int fd)
The smart-http series causes fetch-pack to buffer. :-)
> And once we start buffering, allowing "0004" packet_write() wouldn't even
> be a problem; it can be optimized out in the buffering layer.
Sure, but can't packet_write just return early without write()
if format_packet returned 4 (aka vsnprintf returned 0)?
--
Shawn.
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Junio C Hamano @ 2009-11-04 1:23 UTC (permalink / raw)
To: Erick Mattos; +Cc: Nanako Shiraishi, git
In-Reply-To: <55bacdd30911031551k1bfd3151t940864e4793f5a37@mail.gmail.com>
Erick Mattos <erick.mattos@gmail.com> writes:
> ... I had already sent another patch with the
> suggestions he made in a previous email.
That happens in real life with people working in different timezones.
> The new option only touches on getting new author or copying the
> original so that is why I made the first check in whole and the others
> only by author. If people think that this operation is so uncertain,
> then everything should be compared: parent, author and message on all
> tests.
You probably have misunderstood why we write tests; it is not about making
sure _your_ implementation is Ok. If that were the case, using knowledge
of implementation details to short-circuit the tests would perfectly be
acceptable.
We write tests so that long after you get bored and stop visiting the git
project mailing-list, if somebody _else_ changes the program and its
behaviour gets changed in a way _you_ did not expect, such a mistake can
be caught, even if you are not monitoring the mailing list to actively
catch such a bad change to go into the system. So we prefer to test both
sides of the coin without saying "this option only affects this codepath
(currently) so it never can break this part, it is not worth checking this
and that (right now)" when it is not too much trouble. It is a win in the
long run.
In any case, I like --reset-author better than --mine. I didn't think of
diamond-mine, though ;-)
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Junio C Hamano @ 2009-11-04 1:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, Scott Chacon, git list
In-Reply-To: <20091104011802.GE10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>> > I don't think we ever send an empty packet. If we have no data to
>> > send, why the hell did we create the packet header?
>>
>> Oh, I do not disagree that it is pointless, but the example that followed
>> the part we are discussing also had "0004". I think it is Ok to allow it.
>
> If its pointless, why encourage it? Why not discourage it with SHOULD NOT?
Oh, no, I didn't mean to _encourage_ it.
I just thought that it being pointless at the semantic level would already
be an enough discouragement for people who are intelligent enough.
As I said, this was not an objection to start with.
> Sure, but can't packet_write just return early without write()
> if format_packet returned 4 (aka vsnprintf returned 0)?
Ah, that's right.
^ permalink raw reply
* Re: [PATCH] gitk: disable checkout of remote branch
From: Sitaram Chamarty @ 2009-11-04 1:58 UTC (permalink / raw)
To: Tim Mazid; +Cc: git
In-Reply-To: <1257295737457-3942366.post@n2.nabble.com>
On Wed, Nov 4, 2009 at 6:18 AM, Tim Mazid <timmazid@hotmail.com> wrote:
> Might be better to include a configuration option to allow this, for those
> that know what they're doing. Most of the people that know what they're
> doing will use the command line, anyway, but it may irritate some people.
I considered that but found my tcl fu was seriously lacking. These
are literally the first 3 lines of tcl I ever wrote in my life, and
this program is one huge 11,000+ line monolith, so I'm naturally
scared to make more than very, very, small changes :)
In any case, as you said, most people who know what they're doing can
use the CLI to get there anyway...
^ permalink raw reply
* How to ensure a word has been removed from repository?
From: Patrick Higgins @ 2009-11-04 2:12 UTC (permalink / raw)
To: git
Hi all,
I just completed a series of filter-branch commands to remove a couple
of sensitive words from a repository before I publish it. The words
were found in commit messages, directory names, file contents, and
various other places (kind of weird, I know). I believe I have removed
them all, but I would like to double check but don't know how.
Given that much of the repository is stored in compressed packs, I
can't just use grep to look for the words. To get around this, I've
unpacked the objects, use a Perl script (filtinf example script) to
decompress them and then use grep (this has proven to be quite slow).
Is that going to find every possible occurrence if all the relevant
files are plain text?
Is there an easier way to search the repository? The way I'm doing it
has required some awfully deep knowledge to expire and prune
everything. I feel like I must be missing something.
Thanks,
Patrick
^ permalink raw reply
* [PATCH] Require a struct remote in transport_get()
From: Daniel Barkalow @ 2009-11-04 2:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
cmd_ls_remote() was calling transport_get() with a NULL remote and a
non-NULL url in the case where it was run outside a git
repository. This involved a bunch of ill-tested special
cases. Instead, simply get the struct remote for the URL with
remote_get(), which works fine outside a git repository, and can also
take global options into account.
This fixes a tiny and obscure bug where "git ls-remote" without a repo
didn't support global url.*.insteadOf, even though "git clone" and
"git ls-remote" in any repo did.
Also, enforce that all callers provide a struct remote to transport_get().
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
This is sufficient to stop the segfault when tring "git ls-remote
http://..." outside of a repo, but not to make it work, which requires
either something simple but not ideal or something complex.
builtin-ls-remote.c | 6 +++---
transport.c | 7 +++++--
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..b5bad0c 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -86,10 +86,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
pattern[j - i] = p;
}
}
- remote = nongit ? NULL : remote_get(dest);
- if (remote && !remote->url_nr)
+ remote = remote_get(dest);
+ if (!remote->url_nr)
die("remote %s has no configured URL", dest);
- transport = transport_get(remote, remote ? remote->url[0] : dest);
+ transport = transport_get(remote, remote->url[0]);
if (uploadpack != NULL)
transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
diff --git a/transport.c b/transport.c
index 644a30a..298dc46 100644
--- a/transport.c
+++ b/transport.c
@@ -812,6 +812,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
{
struct transport *ret = xcalloc(1, sizeof(*ret));
+ if (!remote)
+ die("No remote provided to transport_get()");
+
ret->remote = remote;
ret->url = url;
@@ -849,10 +852,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
data->thin = 1;
data->conn = NULL;
data->uploadpack = "git-upload-pack";
- if (remote && remote->uploadpack)
+ if (remote->uploadpack)
data->uploadpack = remote->uploadpack;
data->receivepack = "git-receive-pack";
- if (remote && remote->receivepack)
+ if (remote->receivepack)
data->receivepack = remote->receivepack;
}
--
1.6.5.2.142.g063c5.dirty
^ permalink raw reply related
* Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads
From: Joshua Jensen @ 2009-11-04 2:34 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.DEB.1.00.0911040031210.4985@pacific.mpi-cbg.de>
----- Original Message -----
From: Johannes Schindelin
Date: 11/3/2009 4:38 PM
>> #ifdef THREADED_DELTA_SEARCH
>> -#include "thread-utils.h"
>> -#include<pthread.h>
>> +# include "thread-utils.h"
>> +# ifndef _WIN32
>> +# include<pthread.h>
>> +# else
>> +# include<winthread.h>
>> +# endif
>> #endif
>>
>>
> It is unlikely that an #ifdef "contamination" of this extent will go
> through easily, but I have a suggestion that may make your patch both
> easier to read and more likely to be accepted into git.git: Try to wrap
> the win32 calls into pthread-compatible function signatures. Then you can
> add a compat/win32/pthread.h and not even touch core files of git.git at
> all.
>
Pardon my ignorance, but is there a reason to not use Pthreads for
Win32? http://sourceware.org/pthreads-win32/
Josh
^ permalink raw reply
* Re: How to ensure a word has been removed from repository?
From: Nicolas Pitre @ 2009-11-04 2:49 UTC (permalink / raw)
To: Patrick Higgins; +Cc: git
In-Reply-To: <6fb3af8e0911031812j54a9b698xca9f5301ac07442a@mail.gmail.com>
On Tue, 3 Nov 2009, Patrick Higgins wrote:
> Hi all,
>
> I just completed a series of filter-branch commands to remove a couple
> of sensitive words from a repository before I publish it. The words
> were found in commit messages, directory names, file contents, and
> various other places (kind of weird, I know). I believe I have removed
> them all, but I would like to double check but don't know how.
>
> Given that much of the repository is stored in compressed packs, I
> can't just use grep to look for the words. To get around this, I've
> unpacked the objects, use a Perl script (filtinf example script) to
> decompress them and then use grep (this has proven to be quite slow).
>
> Is that going to find every possible occurrence if all the relevant
> files are plain text?
>
> Is there an easier way to search the repository? The way I'm doing it
> has required some awfully deep knowledge to expire and prune
> everything. I feel like I must be missing something.
An easy way to look for the presence of a particular string in all the
repository data is:
git rev-list --all --objects | cut -c -40 | \
git cat-file --batch | grep <string>
Alternatively you can use:
git fast-export --all --signed-tag=verbatim | grep <string>
Nicolas
^ permalink raw reply
* [PATCH] Allow curl helper to work without a local repository
From: Daniel Barkalow @ 2009-11-04 2:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
It's okay to use the curl helper without a local repository, so long
as you don't use "fetch". There aren't any git programs that would try
to use it, and it doesn't make sense to try it (since there's nowhere
to write the results), but we may as well be clear.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
This is the simple change to let remote-curl work without a local
repository for git ls-remote; it leave the transport-helper code assuming
that all helpers can list without a local repo, which happens to be true
of this helper, the only one in current git.
remote-curl.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 2faf1c6..ebdab36 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -82,9 +82,10 @@ int main(int argc, const char **argv)
struct strbuf buf = STRBUF_INIT;
const char *url;
struct walker *walker = NULL;
+ int nongit;
git_extract_argv0_path(argv[0]);
- setup_git_directory();
+ setup_git_directory_gently(&nongit);
if (argc < 2) {
fprintf(stderr, "Remote needed\n");
return 1;
@@ -103,6 +104,8 @@ int main(int argc, const char **argv)
break;
if (!prefixcmp(buf.buf, "fetch ")) {
char *obj = buf.buf + strlen("fetch ");
+ if (nongit)
+ die("Fetch attempted without a local repo");
if (!walker)
walker = get_http_walker(url, remote);
walker->get_all = 1;
--
1.6.5.2.142.g063c5.dirty
^ permalink raw reply related
* Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Erick Mattos @ 2009-11-04 2:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7v639rnkvt.fsf@alter.siamese.dyndns.org>
2009/11/3 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> ... I had already sent another patch with the
>> suggestions he made in a previous email.
>
> That happens in real life with people working in different timezones.
6 hours between you and me!
>> The new option only touches on getting new author or copying the
>> original so that is why I made the first check in whole and the others
>> only by author. If people think that this operation is so uncertain,
>> then everything should be compared: parent, author and message on all
>> tests.
>
> You probably have misunderstood why we write tests; it is not about making
> sure _your_ implementation is Ok. If that were the case, using knowledge
> of implementation details to short-circuit the tests would perfectly be
> acceptable.
>
> We write tests so that long after you get bored and stop visiting the git
> project mailing-list, if somebody _else_ changes the program and its
> behaviour gets changed in a way _you_ did not expect, such a mistake can
> be caught, even if you are not monitoring the mailing list to actively
> catch such a bad change to go into the system. So we prefer to test both
> sides of the coin without saying "this option only affects this codepath
> (currently) so it never can break this part, it is not worth checking this
> and that (right now)" when it is not too much trouble. It is a win in the
> long run.
I really did not get the reason before the other guy argued... :-S
> In any case, I like --reset-author better than --mine. I didn't think of
> diamond-mine, though ;-)
>
So that's it! Diamond... me neither! :-D
I am going to send you another patch in a few minutes. I hope this
time will be almost there.
Regards
^ 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